feat(api): create a layout change #2521

p-fernandez · 2023-01-18T00:27:32Z

What change does this PR introduce?

Adds functionality for creating a layout change

Why was this change needed?

To be able to promote the layouts for the production environment, first we need to create said change in order to be able to apply the changes in production, either individually or in bulk.

Other information (Screenshots)

linear · 2023-01-18T00:27:35Z

NV-1502 Changes - create a layout change

p-fernandez · 2023-01-18T00:28:17Z

apps/api/src/app/change/usecases/create-change/index.ts

+export * from './create-change.command';
+export * from './create-change.usecase';


Following the same pattern of the use case we apply everywhere, I moved to dedicated folder to easier export.

p-fernandez · 2023-01-18T00:29:09Z

apps/api/src/app/layouts/use-cases/create-layout-change/create-layout-change.use-case.ts

+    const item = await this.layoutRepository.findById(command.layoutId, command.environmentId);
+
+    if (item) {
+      const changeId = LayoutRepository.createObjectId();
+
+      await this.createChange.execute(
+        CreateChangeCommand.create({
+          organizationId: command.organizationId,
+          environmentId: command.environmentId,
+          userId: command.userId,
+          type: ChangeEntityTypeEnum.LAYOUT,
+          item,
+          changeId,
+        })
+      );
+    }
+  }


To review. I assume we don't need any parentChangeId as the layout changes are not based on another change.

p-fernandez · 2023-01-18T00:29:51Z

apps/api/src/app/layouts/use-cases/create-layout-change/create-layout-change.use-case.ts

+import { CreateChange, CreateChangeCommand } from '../../../change/usecases';
+
+@Injectable()
+export class CreateLayoutChangeUseCase {


Dedicate use case for layout so it can be used in the needed use cases of the layout feature.

ainouzgali

Looks good.

ainouzgali · 2023-01-18T08:29:22Z

apps/api/src/app/change/usecases/create-change/index.ts

+export * from './create-change.command';
+export * from './create-change.usecase';


ainouzgali · 2023-01-18T08:42:01Z

libs/shared/src/entities/change/change.interface.ts

@@ -1,6 +1,7 @@
 export enum ChangeEntityTypeEnum {


Its also in the first pr. Notice for conflicts

ainouzgali · 2023-01-18T08:46:11Z

apps/api/src/app/layouts/use-cases/create-layout-change/create-layout-change.use-case.ts

+    const item = await this.layoutRepository.findById(command.layoutId, command.environmentId);
+
+    if (item) {
+      const changeId = LayoutRepository.createObjectId();
+
+      await this.createChange.execute(
+        CreateChangeCommand.create({
+          organizationId: command.organizationId,
+          environmentId: command.environmentId,
+          userId: command.userId,
+          type: ChangeEntityTypeEnum.LAYOUT,
+          item,
+          changeId,
+        })
+      );
+    }
+  }


p-fernandez · 2023-01-22T16:34:38Z

...c/app/change/usecases/promote-change-to-environment/promote-change-to-environment.usecase.ts

@@ -14,6 +14,7 @@ export class PromoteChangeToEnvironment {
  constructor(
    private changeRepository: ChangeRepository,
    private environmentRepository: EnvironmentRepository,
+    @Inject(forwardRef(() => PromoteNotificationTemplateChange))


There is a circular dependency for PromoteNotificationTemplateChange. It tells me that we probably we would need to rethink some of the dependencies between the ChangeModule, LayoutModuleand NotificationTemplateModule.

p-fernandez · 2023-01-22T16:34:55Z

apps/api/src/app/invites/e2e/bulk-invite.e2e.ts

@@ -28,7 +28,7 @@ describe('Bulk invite members - /invites/bulk (POST)', async () => {
      .send({
        invitees: [
          {
-            email: 'asdasda',
+            email: 'email@bad',


To avoid CSpell to complain.

p-fernandez · 2023-01-22T16:35:39Z

apps/api/src/app/layouts/layouts.module.ts

 import { MessageTemplateModule } from '../message-template/message-template.module';
 import { SharedModule } from '../shared/shared.module';

 @Module({
-  imports: [SharedModule, MessageTemplateModule],
+  imports: [SharedModule, ChangeModule, MessageTemplateModule],


We need CreateChange use case inside Layout module for being able to create all the changes through CreateLayoutChange.

p-fernandez · 2023-01-22T16:36:07Z

apps/api/src/app/layouts/usecases/create-layout-change/create-layout-change.use-case.ts

+export class CreateLayoutChangeUseCase {
+  constructor(private createChange: CreateChange, private layoutRepository: LayoutRepository) {}
+
+  async execute(command: CreateLayoutChangeCommand): Promise<void> {
+    const item = await this.layoutRepository.findById(command.layoutId, command.environmentId);


Generic use case to be reused in all the Layout use cases where we need to track a change.

In the case of create change after delete-layout, this will not find the deleted one @p-fernandez

How do we handle deletions? With the soft delete should be able to pick it up. Don't know how mongo-delete works 100%.

I made a suggestion in this pr for a find soft deleted layouts. Let me know what you think :)

So the change creation for a deletion would need to use that new method?

Maybe if he doesn't find the item like this, then do the findDeleted

p-fernandez · 2023-01-22T16:38:16Z

apps/api/src/app/layouts/usecases/set-default-layout/set-default-layout.use-case.ts

@@ -19,6 +21,7 @@ export class SetDefaultLayoutUseCase {
    try {
      if (defaultLayoutId) {
        await this.setIsDefaultForLayout(defaultLayoutId, command.environmentId, command.organizationId, false);
+        await this.createLayoutChangeForPreviousDefault(command, defaultLayoutId);


As we make non default the default layout, we need to keep track of that change. Not totally happy as this could be easily overlooked in a change of logic. Worth discuss it.

p-fernandez · 2023-01-22T16:42:20Z

Tests will go in the promote layout changes PR.

ainouzgali · 2023-01-23T08:43:36Z

apps/api/src/app/layouts/usecases/delete-layout/delete-layout.use-case.ts

+    await this.createChange(command);
+  }
+
+  private async createChange(command: DeleteLayoutCommand): Promise<void> {


Why is this function needed? Doesn't it do the same thing by creating the command?
Maybe you meant to remove the first CreateLayoutChangeCommand in line 39?

It just holds the create layout change logic to not clutter the execute method. Nothing else. 🙂

p-fernandez requested a review from ainouzgali January 18, 2023 00:27

p-fernandez self-assigned this Jan 18, 2023

github-actions bot added @novu/api @novu/shared labels Jan 18, 2023

p-fernandez commented Jan 18, 2023

View reviewed changes

ainouzgali approved these changes Jan 18, 2023

View reviewed changes

p-fernandez force-pushed the nv-1502-changes-create-a-layout-change branch 4 times, most recently from f4d6d2d to 8949d3d Compare January 22, 2023 16:32

p-fernandez commented Jan 22, 2023

View reviewed changes

p-fernandez marked this pull request as ready for review January 22, 2023 16:40

p-fernandez force-pushed the nv-1502-changes-create-a-layout-change branch from 8949d3d to f755eff Compare January 22, 2023 16:41

p-fernandez requested review from ainouzgali and scopsy January 22, 2023 16:45

p-fernandez mentioned this pull request Jan 22, 2023

feat(api): promote changes of layouts #2520

Merged

ainouzgali approved these changes Jan 23, 2023

View reviewed changes

feat(api): create a layout change

505db9f

p-fernandez force-pushed the nv-1502-changes-create-a-layout-change branch from f755eff to d5b7959 Compare January 23, 2023 11:39

github-actions bot added the @novu/dal label Jan 23, 2023

feat(api): add find deleted use case, dal and integrate it in change

a9b47e0

p-fernandez force-pushed the nv-1502-changes-create-a-layout-change branch from d5b7959 to a9b47e0 Compare January 23, 2023 11:41

p-fernandez requested a review from ainouzgali January 23, 2023 11:42

ainouzgali approved these changes Jan 23, 2023

View reviewed changes

p-fernandez merged commit 23b7425 into feat-layouts Jan 23, 2023

p-fernandez deleted the nv-1502-changes-create-a-layout-change branch January 23, 2023 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): create a layout change #2521

feat(api): create a layout change #2521

p-fernandez commented Jan 18, 2023

linear bot commented Jan 18, 2023

p-fernandez Jan 18, 2023

ainouzgali Jan 18, 2023

p-fernandez Jan 18, 2023

ainouzgali Jan 18, 2023

p-fernandez Jan 18, 2023

ainouzgali left a comment

ainouzgali Jan 18, 2023

ainouzgali Jan 18, 2023

ainouzgali Jan 18, 2023

p-fernandez Jan 22, 2023

p-fernandez Jan 22, 2023

p-fernandez Jan 22, 2023

p-fernandez Jan 22, 2023

ainouzgali Jan 23, 2023 •

edited

p-fernandez Jan 23, 2023

ainouzgali Jan 23, 2023

p-fernandez Jan 23, 2023

ainouzgali Jan 23, 2023

p-fernandez Jan 22, 2023

p-fernandez commented Jan 22, 2023

ainouzgali Jan 23, 2023

p-fernandez Jan 23, 2023

		export * from './create-change.command';
		export * from './create-change.usecase';

feat(api): create a layout change #2521

feat(api): create a layout change #2521

Conversation

p-fernandez commented Jan 18, 2023

What change does this PR introduce?

Why was this change needed?

Other information (Screenshots)

linear bot commented Jan 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ainouzgali left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ainouzgali Jan 23, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-fernandez commented Jan 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ainouzgali Jan 23, 2023 •

edited