Skip to content
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

feat(api): Delete subscriber channel preference when updating global channel #6767

Merged
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
5e77371
refactor(preferences): enhance validation and structure
rifont Oct 23, 2024
cda73b9
refactor(preferences): remove default values from schema
rifont Oct 23, 2024
856a947
fix(sync): handle undefined workflow preferences
rifont Oct 24, 2024
22346b8
refactor(base.command): improve error mapping logic
rifont Oct 24, 2024
18523d5
refactor(upsert-preferences): remove unused import
rifont Oct 24, 2024
8ba826f
refactor(sync): extract getWorkflowPreferences method
rifont Oct 24, 2024
eaf6080
refactor(inbox): rename GetPreferences to GetInboxPreferences
rifont Oct 24, 2024
3e3ff65
refactor(inbox): rename GetPreferences to GetInboxPreferences
rifont Oct 24, 2024
f5c8d98
refactor(upsert-preferences): simplify preferences handling
rifont Oct 24, 2024
0c41453
`feat(preferences): add unset logic for channel types`
rifont Oct 24, 2024
150e302
refactor(preferences): simplify channel handling
rifont Oct 24, 2024
6a66a12
refactor(sync): update WorkflowPreferences type usage
rifont Oct 24, 2024
5d99857
refactor(inbox): simplify preference retrieval logic
rifont Oct 24, 2024
0fa489a
refactor(preference): simplify global preference logic
rifont Oct 24, 2024
86b5bc6
refactor(get-subscriber): improve preference handling
rifont Oct 24, 2024
56f0d09
refactor(workflows): add default workflow preferences
rifont Oct 24, 2024
70f22b6
refactor(preferences): update storePreferences method
rifont Oct 24, 2024
7aab8b7
refactor(preferences): extract method for clarity
rifont Oct 24, 2024
49b8683
refactor(subscriber): merge fetch and get methods
rifont Oct 24, 2024
a4d5c47
fix: correct enabled flag in preference return object
rifont Oct 24, 2024
1739863
fix: handle undefined enabled property safely
rifont Oct 24, 2024
ae7a520
fix(preference): default enable to true if undefined
rifont Oct 24, 2024
b1abd25
test(get-inbox-preferences): revise test structure
rifont Oct 24, 2024
26c0ffc
test(update-preferences): remove unused channel fields
rifont Oct 24, 2024
4d7479d
test(inbox): update preferences e2e test logic
rifont Oct 24, 2024
a51539c
refactor(get-subscriber-preference): rename variable
rifont Oct 24, 2024
85addb9
refactor(get-preferences): simplify preference mapping
rifont Oct 24, 2024
73fe8bf
feat(upsert-preferences): enhance preferences handling
rifont Oct 24, 2024
7177ae2
refactor(get-preferences): use partial preferences type
rifont Oct 24, 2024
a5ce239
test(inbox): update e2e test for preferences
rifont Oct 24, 2024
8ca830f
refactor(get-preferences): improve preference mapping logic
rifont Oct 24, 2024
3c09ccc
refactor(update-preferences): reorganize preference logic
rifont Oct 24, 2024
c783e5d
test(inbox): add test for updating preferences
rifont Oct 24, 2024
32f7ec6
refactor(upsert-preferences): rename method for clarity
rifont Oct 25, 2024
093365d
refactor: rename method for clarity
rifont Oct 25, 2024
26891fc
refactor(upsert-preferences): simplify update query
rifont Oct 26, 2024
6afe9da
refactor(preferences): use WorkflowPreferencesPartial type
rifont Oct 26, 2024
4b54490
ci(setup-project): update MongoDB to v5.0.29
rifont Oct 26, 2024
a5c0b9b
ci(setup-project): update MongoDB version and action tag
rifont Oct 26, 2024
2ee421d
ci: Update MongoDB version in setup action
rifont Oct 26, 2024
e383ae2
style: Adjust div height in preferences page
rifont Oct 26, 2024
09107f8
refactor(get-preferences): update preferences handling
rifont Oct 27, 2024
33c0d8e
refactor(get-preferences): update source type definitions
rifont Oct 27, 2024
a53724a
refactor(preferences): add caching to preference update
rifont Oct 28, 2024
2137bc3
test: simplify global preference channel assertions
rifont Oct 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/actions/setup-project/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ runs:

- name: 📚 Start MongoDB
if: ${{ inputs.slim == 'false' }}
uses: supercharge/mongodb-github-action@v1.9.0
uses: supercharge/mongodb-github-action@v1.11.0
with:
mongodb-version: 4.2.8
mongodb-version: 5.0.29
Copy link
Contributor Author

@rifont rifont Oct 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This MongoDB version bump to >5 was needed to solve this runtime error that disallows setting empty objects:

Invalid $addFields :: caused by :: an empty object is not a valid value. Found empty object at path preferences.all

Version 5 supports empty object setting per this thread and referenced documentation.

5.0.29 is the latest LTS 5.x release per the MongoDB changelog. MongoDB 5.0 reaches end of life this month, October 2024.

Note, this means that we are effectively dropping support of MongoDB v4 for Novu Self-hosted. MongoDB v4 reached end of life in February 2024.

cc @merrcury

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, Thanks 👍🏼. We will post a note a to community/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call out!


- name: 🛟 Install dependencies
shell: bash
Expand Down
14 changes: 12 additions & 2 deletions apps/api/src/app/bridge/usecases/sync/sync.usecase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@ import {
UpsertPreferences,
UpsertWorkflowPreferencesCommand,
} from '@novu/application-generic';
import { FeatureFlagsKeysEnum, WorkflowCreationSourceEnum, WorkflowOriginEnum, WorkflowTypeEnum } from '@novu/shared';
import {
FeatureFlagsKeysEnum,
WorkflowCreationSourceEnum,
WorkflowOriginEnum,
WorkflowTypeEnum,
WorkflowPreferencesPartial,
} from '@novu/shared';
import { DiscoverOutput, DiscoverStepOutput, DiscoverWorkflowOutput, GetActionEnum } from '@novu/framework/internal';

import { SyncCommand } from './sync.command';
Expand Down Expand Up @@ -186,7 +192,7 @@ export class Sync {
environmentId: savedWorkflow._environmentId,
organizationId: savedWorkflow._organizationId,
templateId: savedWorkflow._id,
preferences: workflow.preferences,
preferences: this.getWorkflowPreferences(workflow),
})
);
}
Expand Down Expand Up @@ -323,6 +329,10 @@ export class Sync {
return notificationGroupId;
}

private getWorkflowPreferences(workflow: DiscoverWorkflowOutput): WorkflowPreferencesPartial {
return workflow.preferences || {};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A fallback empty object is now necessary during sync after strengthening validation of the UpsertWorkflow use-case to disallow undefined preferences.

}

private getWorkflowName(workflow: DiscoverWorkflowOutput): string {
return workflow.name || workflow.workflowId;
}
Expand Down
19 changes: 13 additions & 6 deletions apps/api/src/app/inbox/e2e/get-preferences.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { UserSession } from '@novu/testing';
import { expect } from 'chai';
import { StepTypeEnum } from '@novu/shared';

describe('Get all preferences - /inbox/preferences (GET)', function () {
let session: UserSession;
Expand All @@ -9,22 +10,28 @@ describe('Get all preferences - /inbox/preferences (GET)', function () {
await session.initialize();
});

it('should always get the global preferences even if workflow preferences are not present', async function () {
it('should return no global preferences if workflow preferences are not present', async function () {
const response = await session.testAgent
.get('/v1/inbox/preferences')
.set('Authorization', `Bearer ${session.subscriberToken}`);

const globalPreference = response.body.data[0];

expect(globalPreference.channels.email).to.equal(true);
expect(globalPreference.channels.in_app).to.equal(true);
expect(globalPreference.channels.email).to.equal(undefined);
expect(globalPreference.channels.in_app).to.equal(undefined);
expect(globalPreference.level).to.equal('global');
expect(response.body.data.length).to.equal(1);
});

it('should get both global and workflow preferences if workflow is present', async function () {
it('should get both global preferences for active channels and workflow preferences if workflow is present', async function () {
await session.createTemplate({
noFeedId: true,
steps: [
{
type: StepTypeEnum.EMAIL,
content: 'Test notification content',
},
],
Copy link
Contributor Author

@rifont rifont Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly specifying that only email is present, to assert that in_app should not appear.

});

const response = await session.testAgent
Expand All @@ -34,13 +41,13 @@ describe('Get all preferences - /inbox/preferences (GET)', function () {
const globalPreference = response.body.data[0];

expect(globalPreference.channels.email).to.equal(true);
expect(globalPreference.channels.in_app).to.equal(true);
expect(globalPreference.channels.in_app).to.equal(undefined);
expect(globalPreference.level).to.equal('global');

const workflowPreference = response.body.data[1];

expect(workflowPreference.channels.email).to.equal(true);
expect(workflowPreference.channels.in_app).to.equal(true);
expect(workflowPreference.channels.in_app).to.equal(undefined);
expect(workflowPreference.level).to.equal('template');
});

Expand Down
107 changes: 88 additions & 19 deletions apps/api/src/app/inbox/e2e/update-preferences.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { EmailBlockTypeEnum, PreferenceLevelEnum, StepTypeEnum } from '@novu/shared';
import { UserSession } from '@novu/testing';
import { expect } from 'chai';
import e from 'express';

describe('Update global preferences - /inbox/preferences (PATCH)', function () {
let session: UserSession;
Expand Down Expand Up @@ -38,48 +39,53 @@ describe('Update global preferences - /inbox/preferences (PATCH)', function () {
.set('Authorization', `Bearer ${session.subscriberToken}`);

expect(response.status).to.equal(200);
expect(response.body.data.channels.email).to.equal(true);
expect(response.body.data.channels.in_app).to.equal(true);
expect(response.body.data.channels.sms).to.equal(false);
expect(response.body.data.channels.push).to.equal(false);
expect(response.body.data.channels.chat).to.equal(true);
expect(response.body.data.channels.email).to.equal(undefined);
expect(response.body.data.channels.in_app).to.equal(undefined);
expect(response.body.data.channels.sms).to.equal(undefined);
expect(response.body.data.channels.push).to.equal(undefined);
expect(response.body.data.channels.chat).to.equal(undefined);
expect(response.body.data.level).to.equal(PreferenceLevelEnum.GLOBAL);
});

it('should update the particular channel sent in the body and return all channels', async function () {
it('should update the particular channel sent in the body and return only active channels', async function () {
await session.createTemplate({
noFeedId: true,
steps: [
{
type: StepTypeEnum.IN_APP,
content: 'Test notification content',
},
],
});

const response = await session.testAgent
.patch('/v1/inbox/preferences')
.send({
email: true,
in_app: true,
sms: false,
push: false,
chat: true,
})
.set('Authorization', `Bearer ${session.subscriberToken}`);

expect(response.status).to.equal(200);
expect(response.body.data.channels.email).to.equal(true);
expect(response.body.data.channels.email).to.equal(undefined);
expect(response.body.data.channels.in_app).to.equal(true);
expect(response.body.data.channels.sms).to.equal(false);
expect(response.body.data.channels.push).to.equal(false);
expect(response.body.data.channels.chat).to.equal(true);
expect(response.body.data.channels.sms).to.equal(undefined);
expect(response.body.data.channels.push).to.equal(undefined);
expect(response.body.data.channels.chat).to.equal(undefined);
expect(response.body.data.level).to.equal(PreferenceLevelEnum.GLOBAL);

const responseSecond = await session.testAgent
.patch('/v1/inbox/preferences')
.send({
email: false,
in_app: true,
})
.set('Authorization', `Bearer ${session.subscriberToken}`);

expect(responseSecond.status).to.equal(200);
expect(responseSecond.body.data.channels.email).to.equal(false);
expect(responseSecond.body.data.channels.email).to.equal(undefined);
expect(responseSecond.body.data.channels.in_app).to.equal(true);
expect(responseSecond.body.data.channels.sms).to.equal(false);
expect(responseSecond.body.data.channels.push).to.equal(false);
expect(responseSecond.body.data.channels.chat).to.equal(true);
expect(responseSecond.body.data.channels.sms).to.equal(undefined);
expect(responseSecond.body.data.channels.push).to.equal(undefined);
expect(responseSecond.body.data.channels.chat).to.equal(undefined);
expect(responseSecond.body.data.level).to.equal(PreferenceLevelEnum.GLOBAL);
});
});
Expand Down Expand Up @@ -256,4 +262,67 @@ describe('Update workflow preferences - /inbox/preferences/:workflowId (PATCH)',
expect(responseSecond.body.data.channels.chat).to.equal(true);
expect(responseSecond.body.data.level).to.equal(PreferenceLevelEnum.TEMPLATE);
});

it('should unset the suscribers workflow preference for the specified channels when the global preference is updated', async function () {
const workflow = await session.createTemplate({
noFeedId: true,
steps: [
{
type: StepTypeEnum.IN_APP,
content: 'Test notification content',
},
{
type: StepTypeEnum.EMAIL,
content: 'Test notification content',
},
],
});

const updateWorkflowPrefResponse = await session.testAgent
.patch(`/v1/inbox/preferences/${workflow._id}`)
.send({
email: false,
in_app: false,
})
.set('Authorization', `Bearer ${session.subscriberToken}`);

expect(updateWorkflowPrefResponse.status).to.equal(200);
expect(updateWorkflowPrefResponse.body.data.channels.email).to.equal(false);
expect(updateWorkflowPrefResponse.body.data.channels.in_app).to.equal(false);
expect(updateWorkflowPrefResponse.body.data.channels.sms).to.equal(undefined);
expect(updateWorkflowPrefResponse.body.data.channels.push).to.equal(undefined);
expect(updateWorkflowPrefResponse.body.data.channels.chat).to.equal(undefined);
expect(updateWorkflowPrefResponse.body.data.level).to.equal(PreferenceLevelEnum.TEMPLATE);

const updateGlobalPrefResponse = await session.testAgent
.patch(`/v1/inbox/preferences`)
.send({
email: true,
})
.set('Authorization', `Bearer ${session.subscriberToken}`);

expect(updateGlobalPrefResponse.status).to.equal(200);
expect(updateGlobalPrefResponse.body.data.channels.email).to.equal(true);
expect(updateGlobalPrefResponse.body.data.channels.in_app).to.equal(true);
expect(updateGlobalPrefResponse.body.data.channels.sms).to.equal(undefined);
expect(updateGlobalPrefResponse.body.data.channels.push).to.equal(undefined);
expect(updateGlobalPrefResponse.body.data.channels.chat).to.equal(undefined);
expect(updateGlobalPrefResponse.body.data.level).to.equal(PreferenceLevelEnum.GLOBAL);

const getInboxPrefResponse = await session.testAgent
.get(`/v1/inbox/preferences`)
.set('Authorization', `Bearer ${session.subscriberToken}`);

const workflowPref = getInboxPrefResponse.body.data.find(
(pref) => pref.level === PreferenceLevelEnum.TEMPLATE && pref.workflow.id === workflow._id
);

expect(getInboxPrefResponse.status).to.equal(200);
expect(workflowPref.channels.email).to.equal(true);
expect(workflowPref.channels.in_app).to.equal(false);
expect(workflowPref.channels.sms).to.equal(undefined);
expect(workflowPref.channels.push).to.equal(undefined);
expect(workflowPref.channels.chat).to.equal(undefined);
expect(workflowPref.level).to.equal(PreferenceLevelEnum.TEMPLATE);
});
});
10 changes: 5 additions & 5 deletions apps/api/src/app/inbox/inbox.controller.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and in the following files, we'll see renaming from GetPreferences to GetInboxPreferences to eliminate the duplicated usage of GetPreferences use-case naming.

Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import { UpdateNotificationActionCommand } from './usecases/update-notification-
import { UpdateAllNotificationsRequestDto } from './dtos/update-all-notifications-request.dto';
import { UpdateAllNotificationsCommand } from './usecases/update-all-notifications/update-all-notifications.command';
import { UpdateAllNotifications } from './usecases/update-all-notifications/update-all-notifications.usecase';
import { GetPreferences } from './usecases/get-preferences/get-preferences.usecase';
import { GetPreferencesCommand } from './usecases/get-preferences/get-preferences.command';
import { GetInboxPreferences } from './usecases/get-inbox-preferences/get-inbox-preferences.usecase';
import { GetInboxPreferencesCommand } from './usecases/get-inbox-preferences/get-inbox-preferences.command';
import { GetPreferencesResponseDto } from './dtos/get-preferences-response.dto';
import { UpdatePreferencesRequestDto } from './dtos/update-preferences-request.dto';
import { UpdatePreferences } from './usecases/update-preferences/update-preferences.usecase';
Expand All @@ -46,7 +46,7 @@ export class InboxController {
private markNotificationAsUsecase: MarkNotificationAs,
private updateNotificationActionUsecase: UpdateNotificationAction,
private updateAllNotifications: UpdateAllNotifications,
private getPreferencesUsecase: GetPreferences,
private getInboxPreferencesUsecase: GetInboxPreferences,
private updatePreferencesUsecase: UpdatePreferences
) {}

Expand Down Expand Up @@ -107,8 +107,8 @@ export class InboxController {
@SubscriberSession() subscriberSession: SubscriberEntity,
@Query() query: GetPreferencesRequestDto
): Promise<GetPreferencesResponseDto[]> {
return await this.getPreferencesUsecase.execute(
GetPreferencesCommand.create({
return await this.getInboxPreferencesUsecase.execute(
GetInboxPreferencesCommand.create({
organizationId: subscriberSession._organizationId,
subscriberId: subscriberSession.subscriberId,
environmentId: subscriberSession._environmentId,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { IsArray, IsOptional, IsString } from 'class-validator';
import { EnvironmentWithSubscriber } from '../../../shared/commands/project.command';

export class GetPreferencesCommand extends EnvironmentWithSubscriber {
export class GetInboxPreferencesCommand extends EnvironmentWithSubscriber {
@IsOptional()
@IsArray()
@IsString({ each: true })
Expand Down
Loading
Loading