🪟🔧 Refactor feature service #14559
🪟🔧 Refactor feature service #14559
Conversation
timroes
added
area/frontend
technical-debt
| @@ -38,7 +38,9 @@ const Services: React.FC = ({ children }) => ( | |||
| <NotificationServiceProvider> | |||
| <ConfirmationModalService> | |||
| <FormChangeTrackerService> | |||
| <FeatureService> | |||
| <FeatureService | |||
| features={[FeatureItem.AllowOAuthConnector, FeatureItem.AllowCreateConnection, FeatureItem.AllowSync]} | |||
There was a problem hiding this comment.
Like OSS, maybe this should be moved to a defaultFeatures const?
There was a problem hiding this comment.
I'd prefer to have it inline, given that we're not needing to import it anywhere else, it would just make the code more fragmented and requires you to follow into other files to see the feature set. I'd also have kept the OSS one inline if we wouldn't have needed to import it for the tests (and even considered just duplicate it into the tests).
| return () => { | ||
| setWorkspaceFeatures(undefined); | ||
| }; | ||
| }, [cloudWorkspace, setWorkspaceFeatures]); |
There was a problem hiding this comment.
Just need the cloudWorkspace.creditStatus as a dep
| <FeatureService>{children}</FeatureService> | ||
| </ConfigContext.Provider> | ||
| </TestWrapper> | ||
| <FeatureService features={[FeatureItem.AllowCreateConnection, FeatureItem.AllowSync]}>{children}</FeatureService> |
There was a problem hiding this comment.
Might be better to have the features in a constant such as defaultFeatures so that it can be referenced later in the tests.
There was a problem hiding this comment.
I personally would prefer keeping it that way. Most of the tests tests different combinations of enabling/disabling, why we can't spread that anyway simply into it. Also given that those tests all test whether overwriting etc work, I'd prefer if someone ever adjust this line, manually also needs to adjust the expect cases below, to sanity check that the overwrite logic is really working as expected.
…rbytehq-master * 'master' of https://github.com/airbytehq/airbyte: (1141 commits) pass USE_STREAM_CAPABLE_STATE env var to containers/deployments (airbytehq#14737) Bump mqtt connector (airbytehq#14648) Add error code to ManualOperationResult (airbytehq#14657) Bump elasticsearch version (airbytehq#14640) Ryan/sync oracle version number (airbytehq#14736) Fixed linter issue with add_fields.py comments (airbytehq#14742) 🎉Redshift, Databricks, Snowflake, S3 Destinations: Make S3 output filename configurable (airbytehq#14494) 🐛Source-mssql: aligned regular and cdc syncs and its datatype tests (airbytehq#14379) 🎉 Source Amazon Seller Partner: Add new streams (airbytehq#13604) bump source-file-secure (airbytehq#14704) 🎉 New source: Timely airbytehq#13292 (airbytehq#14335) 🪟🔧 Refactor feature service (airbytehq#14559) [low code cdk] add a transformation for adding fields into an outgoing record (airbytehq#14638) Bump destination-postgres to 0.3.21 (airbytehq#14479) Remove `additionalProperties: false` from JDBC destination connectors (airbytehq#14618) 🎉 Source Notion: add OAuth authorization for source-notion connector (airbytehq#14706) Use the configuration diff calculation in the update endpoint (airbytehq#14626) 🪟 🐛 Fix input validation on blur and cleanup signup error handling (airbytehq#14724) lower sleep after wait for successful job (airbytehq#14725) Add configuration diff (airbytehq#14603) ...
What
Closes #10974
This refactors the feature service and detaches it from the
configService(to unblock #10955). The PR does the following things:idproperty anymore, but can use a simpler format.hasFeatureis nowuseFeature. It will cause a re-render if a feature changed. Earlier thehasFeaturemethod itself would be recreated (and thus have a changed reference), when a feature changed, which imho isn't the most intuitive to understand.WithFeaturecomponent toOnlyWithFeatureto make the name express a bit clearer what it does.