-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(api, application-generic): Encapsulate preference mutations in workflow use-cases #7005
base: next
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@novu/client
@novu/framework
@novu/nextjs
@novu/headless
@novu/node
@novu/js
@novu/notification-center
novu
@novu/providers
@novu/react
@novu/react-native
@novu/shared
commit: |
@@ -40,7 +40,7 @@ export class Channels { | |||
export class PreferencesDto { | |||
@ValidateNested({ each: true }) | |||
@Type(() => WorkflowPreference) | |||
workflow: WorkflowPreference; | |||
all: WorkflowPreference; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was just completely wrong before.
organizationId: command.organizationId, | ||
identifierOrInternalId: command.id, | ||
}), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always use the GetWorkflow
command at the end. Not the most performant, but the endpoint is infrequently used so it's a suitable compromise to avoid an even bigger refactor.
{ | ||
$set: updatePayload, | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The database mutation should happen before invalidating.
defaultPreferences: DEFAULT_WORKFLOW_PREFERENCES, | ||
userPreferences: | ||
body.preferenceSettings && | ||
buildWorkflowPreferencesFromPreferenceChannels(body.critical ?? false, body.preferenceSettings), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Internally, all Workflow DTOs now carry a defaultPreferences
and userPreferences
object now.
preferenceSettings
and critical
are removed from all internal DTOs in preparation for their full removal.
…://github.com/novuhq/novu into nv-4752-start-writing-to-v2-preferences-for-all
What changed? Why was the change needed?
CreateWorkflow
,UpdateWorkflow
, andDeleteWorkflow
use-casesUpsertPreferences
use-caseScreenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer