perf(api): Lookup subscriber preferences with a single database query #7119
Conversation
✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| // TODO: replace this runtime mapping with a single query to the database | ||
| const subscriberWorkflowPreferences = await Promise.all( | ||
| templateList.map(async (template) => | ||
| this.getSubscriberTemplatePreferenceUsecase.execute( | ||
| GetSubscriberTemplatePreferenceCommand.create({ | ||
| organizationId: command.organizationId, | ||
| subscriberId: command.subscriberId, | ||
| environmentId: command.environmentId, | ||
| template, | ||
| subscriber, | ||
| includeInactiveChannels: command.includeInactiveChannels, | ||
| }), | ||
| ), | ||
| ), | ||
| ); | ||
| const allPreferences = await this.preferencesRepository.find({ | ||
| _environmentId: command.environmentId, | ||
| $or: [ | ||
| { | ||
| _subscriberId: command.subscriberId, | ||
| type: PreferencesTypeEnum.SUBSCRIBER_WORKFLOW, | ||
| }, | ||
| { | ||
| _templateId: { $in: workflowList.map((template) => template._id) }, | ||
| type: { | ||
| $in: [ | ||
| PreferencesTypeEnum.WORKFLOW_RESOURCE, | ||
| PreferencesTypeEnum.USER_WORKFLOW, | ||
| ], | ||
| }, | ||
| }, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
Et voila, a single database query!
rifont
changed the title
| }), | ||
| ), | ||
| ), | ||
| const allPreferences = await this.preferencesRepository.find({ |
There was a problem hiding this comment.
❓ Is it maybe faster to split this into these into parallel queries, one for each Preference type?
| * | ||
| * If the subscriber has no preferences, the workflow preferences are returned. | ||
| */ | ||
| export class MergePreferences { |
There was a problem hiding this comment.
😍 This is exactly what we needed in the code. A central place to do the resolution.
| ], | ||
| }); | ||
|
|
||
| const subscriberGlobalPreferences = allPreferences.filter( |
There was a problem hiding this comment.
If we split it into separate Mongo queries, we can fetch this set of preferences from Mongo.
| preference.type === PreferencesTypeEnum.SUBSCRIBER_GLOBAL, | ||
| ); | ||
|
|
||
| const workflowPreferenceSets = allPreferences.reduce< |
There was a problem hiding this comment.
Ditto. If we split it into separate Mongo queries, we can fetch this set of preferences from Mongo.
| command.includeInactiveChannels, | ||
| ); | ||
|
|
||
| const initialChannels = filteredPreference( |
There was a problem hiding this comment.
Should we move these two processing functions into the MergeCommand to have everything in one place?
| ); | ||
|
|
||
| return nonCriticalWorkflowPreferences; | ||
| } | ||
|
|
||
| private getChannels( |
There was a problem hiding this comment.
| private getChannels( | |
| private getActiveChannels( |
| workflowResourcePreferences, | ||
| workflowUserPreferences, | ||
| ] | ||
| .filter((preference) => preference !== undefined) |
There was a problem hiding this comment.
| .filter((preference) => preference !== undefined) | |
| .reduce((preference) => ...) |
| */ | ||
| export class MergePreferences { | ||
| public static merge( | ||
| command: MergePreferencesCommand, |
There was a problem hiding this comment.
I feel the code would be simplified if we passed each type of preference separately to this use case. Currently, we fetch from Mongo, we do some mini-processing before calling the Merge use case, and then we combine them and split them again inside the use case.
My suggestion is:
- Fetch the four types of preferences separately (with a single or separate queries from Mongo).
- Feed them to MergeUseCase separately via the command.
- Do all the processing and merging in the MergePreferences use case.
| const readOnlyFlag = workflowPreferences?.all?.readOnly; | ||
|
|
||
| // Determine the most specific preference applied | ||
| let mostSpecificPreference: PreferencesTypeEnum | undefined; |
There was a problem hiding this comment.
We can get this automatically from deepMerge if we provide it during function invocation. The last type will win.
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer