chore(root): Remove IS_WORKFLOW_PREFERENCES_ENABLED feature flag #6636
Conversation
The feature is now released in V2, so the flag is not required anymore.
LaunchDarkly flag references❌ 1 flag removed
|
✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Looks good, the only thing i am not sure about is the TODO we have in send message use case, do we need to convert the GetSubscriberGlobalPreferenceUsecase GetPreferences Usecase?
/*
* TODO: Remove this after we deprecate V1 preferences, global subscriber
* preferences are handled in `GetPreferences` for V2 preferences.
*
* This is actually a bug because it can allow for Global Preferences to disable
* delivery of Workflows with read-only preferences.
*/
I think for future TODO's or Feature Flag removal we need to make sure we have a dedicated ticket in linear with more details of what the definition of done, in addition, it will help us to not think then we need to remove them as we will have it as part of the cycle
| key: FeatureFlagsKeysEnum.IS_WORKFLOW_PREFERENCES_ENABLED, | ||
| }) | ||
| ); | ||
|
|
||
| /* | ||
| * TODO: Remove this after we deprecate V1 preferences, global subscriber |
There was a problem hiding this comment.
@rifont Can you please share more context on this? Do you see any issues with backwards compatibility between V1 and V2 after the removal of the feature flag?
There was a problem hiding this comment.
We haven't performed a migration of the SubscriberPreferenceRepository into the Preferences repository (see the current lookup in get-subscriber-global-preference.usecase.ts here), therefore the current effect of this change in the absence of a migration will be that Subscribers who have modified Global Preferences will need to revisit the <Inbox /> and set their preferences again for them to become active.
Today there are 118,719 global preferences set in Prod US, all of those Subscriber preferences would not be respected without a data migration.
There was a problem hiding this comment.
Regarding Workflow level preferences, this change is backwards compatible because get-subscriber-template-preference.usecase.ts falls back to V1 preferences if V2 doesn't exist.
There was a problem hiding this comment.
In summary, I suggest applying a strangler pattern here - keep the current Global Preferences lookup in place in the Send-Message usecase, then modify the get-subscriber-global-preference.usecase.ts to fetch Subscriber Global preferences with a similar fallback approach as used for Subscriber Workflow preferences.
We can then officially deprecate the V1 preferences use-cases now, and after some amount of pre-determined time, we can fully delete the V1 preference use-cases.
There was a problem hiding this comment.
I have applied the strangler pattern approach as listed above:
keep the current Global Preferences lookup in place in the Send-Message usecase, then modify the get-subscriber-global-preference.usecase.ts to fetch Subscriber Global preferences with a similar fallback approach as used for Subscriber Workflow preferences.
I will create a ticket for us to remove the V1 Preferences lookups in both of GetSubscriberGlobalPreference and GetSubscriberTemplatePreference, and remove this eager Global Preference lookup in SendMessage in 6 months time. That should give plenty of time for new preferences to be set so we can safely shift to V2 only Preference lookups.
@novu/client
@novu/framework
@novu/js
@novu/headless
@novu/nextjs
@novu/node
@novu/notification-center
novu
@novu/providers
@novu/react
@novu/react-native
@novu/shared
commit: |
What changed? Why was the change needed?
The feature is now released in V2, so the flag is not required anymore.