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

Nv 1576 make default layout changes dependent #2711

Merged
merged 6 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 apps/api/src/app/change/e2e/promote-changes.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ describe('Promote changes', () => {
_organizationId: session.organization._id,
enabled: false,
_entityId: layoutId,
type: ChangeEntityTypeEnum.LAYOUT,
type: ChangeEntityTypeEnum.DEFAULT_LAYOUT,
},
'',
{
Expand All @@ -641,7 +641,7 @@ describe('Promote changes', () => {

expect(changes.length).to.eql(1);
expect(changes[0]._entityId).to.eql(layoutId);
expect(changes[0].type).to.eql(ChangeEntityTypeEnum.LAYOUT);
expect(changes[0].type).to.eql(ChangeEntityTypeEnum.DEFAULT_LAYOUT);
expect(changes[0].change).to.deep.include({ op: 'add', path: ['_id'], val: layoutId });

await session.applyChanges({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ export class GetChanges {
if (change.type === ChangeEntityTypeEnum.LAYOUT) {
item = await this.getTemplateDataForLayout(change._entityId, command.environmentId);
}
if (change.type === ChangeEntityTypeEnum.DEFAULT_LAYOUT) {
item = await this.getTemplateDataForDefaultLayout(change._entityId, command.environmentId);
}

list.push({
...change,
Expand Down Expand Up @@ -196,4 +199,22 @@ export class GetChanges {
templateName: item.name,
};
}

private async getTemplateDataForDefaultLayout(
entityId: string,
environmentId: string
): Promise<IViewEntity | Record<string, unknown>> {
const currentDefaultLayout = await this.getTemplateDataForLayout(entityId, environmentId);

const defaultLayout = await this.layoutRepository.findOne({
_environmentId: environmentId,
isDefault: true,
_id: { $ne: entityId },
});

return {
templateName: currentDefaultLayout?.templateName,
previousDefaultLayout: defaultLayout?.name,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export class PromoteChangeToEnvironment {
await this.promoteFeedChange.execute(typeCommand);
break;
case ChangeEntityTypeEnum.LAYOUT:
case ChangeEntityTypeEnum.DEFAULT_LAYOUT:
await this.promoteLayoutChange.execute(typeCommand);
break;
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@ export class PromoteLayoutChange {
constructor(private layoutRepository: LayoutRepository) {}

async execute(command: PromoteTypeChangeCommand) {
const item = await this.layoutRepository.findOne({
let item: LayoutEntity | undefined = await this.layoutRepository.findOne({
_environmentId: command.environmentId,
_parentId: command.item._id,
});

// For the scenario where the layout is deleted and an active default layout change was pending
if (!item) {
item = await this.layoutRepository.findDeletedByParentId(command.item._id, command.environmentId);
}

const newItem = command.item as LayoutEntity;

if (!item) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { IsDefined, IsString, IsOptional } from 'class-validator';
import { EnvironmentWithUserCommand } from '../../../shared/commands/project.command';
import { LayoutId } from '../../types';

export class CreateDefaultLayoutChangeCommand extends EnvironmentWithUserCommand {
@IsString()
@IsDefined()
layoutId: LayoutId;

@IsString()
@IsOptional()
changeId?: string;

@IsString()
@IsOptional()
parentChangeId?: string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Injectable } from '@nestjs/common';
import { ChangeRepository, LayoutEntity, LayoutRepository } from '@novu/dal';
import { ChangeEntityTypeEnum } from '@novu/shared';
import { CreateChange, CreateChangeCommand } from '../../../change/usecases';
import { LayoutDto } from '../../dtos';
import { FindDeletedLayoutCommand, FindDeletedLayoutUseCase } from '../find-deleted-layout';
import { CreateDefaultLayoutChangeCommand } from './create-default-layout-change.command';

type GetChangeId = {
environmentId: string;
layoutId: string;
};

@Injectable()
export class CreateDefaultLayoutChangeUseCase {
constructor(
private createChange: CreateChange,
private findDeletedLayout: FindDeletedLayoutUseCase,
private layoutRepository: LayoutRepository,
private changeRepository: ChangeRepository
) {}

async execute(command: CreateDefaultLayoutChangeCommand): Promise<void> {
let item: LayoutEntity | LayoutDto = await this.layoutRepository.findOne({
_id: command.layoutId,
_environmentId: command.environmentId,
_organizationId: command.organizationId,
});

const changeId = command.changeId || (await this.getChangeId(command));

if (!item) {
item = await this.findDeletedLayout.execute(FindDeletedLayoutCommand.create(command));
}

if (item) {
await this.createChange.execute(
CreateChangeCommand.create({
organizationId: command.organizationId,
environmentId: command.environmentId,
userId: command.userId,
type: ChangeEntityTypeEnum.DEFAULT_LAYOUT,
parentChangeId: command.parentChangeId,
changeId,
item,
})
);
}
}

private async getChangeId(command: GetChangeId) {
return await this.changeRepository.getChangeId(
command.environmentId,
ChangeEntityTypeEnum.DEFAULT_LAYOUT,
command.layoutId
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ export class CreateLayoutUseCase {
userId: dto._creatorId,
});
await this.setDefaultLayout.execute(setDefaultLayoutCommand);
} else {
await this.createChange(command, dto._id);
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what would happen if there is an update/create of layout(name,content) + setting to default. Would it create 2 changes? One for default layout change and one for layout change? If so, are we sure they are also dependent on promote?

Copy link
Member Author

Choose a reason for hiding this comment

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

In that case, It will create a single(1) change under DefaultLayout,
This way it maintains the dependency between other layout changes too.

}

await this.createChange(command, dto._id);

return dto;
}

Expand Down
2 changes: 2 additions & 0 deletions apps/api/src/app/layouts/usecases/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CreateDefaultLayoutChangeUseCase } from './create-default-layout-change/create-default-layout-change.usecase';
import { CheckLayoutIsUsedUseCase } from './check-layout-is-used/check-layout-is-used.use-case';
import { CreateLayoutUseCase } from './create-layout/create-layout.use-case';
import { CreateLayoutChangeUseCase } from './create-layout-change/create-layout-change.use-case';
Expand All @@ -22,6 +23,7 @@ export * from './set-default-layout';
export * from './update-layout';

export const USE_CASES = [
CreateDefaultLayoutChangeUseCase,
CheckLayoutIsUsedUseCase,
CreateDefaultLayout,
CreateLayoutChangeUseCase,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,40 +1,53 @@
import { LayoutEntity, LayoutRepository } from '@novu/dal';
import { Injectable, Logger } from '@nestjs/common';

import { SetDefaultLayoutCommand } from './set-default-layout.command';

import { CreateLayoutChangeCommand, CreateLayoutChangeUseCase } from '../create-layout-change';
import { GetLayoutCommand, GetLayoutUseCase } from '../get-layout';
import { ChangeRepository, LayoutRepository } from '@novu/dal';
import { ChangeEntityTypeEnum } from '@novu/shared';
import { EnvironmentId, LayoutId, OrganizationId } from '../../types';
import { CreateDefaultLayoutChangeCommand } from '../create-default-layout-change/create-default-layout-change.command';
import { CreateDefaultLayoutChangeUseCase } from '../create-default-layout-change/create-default-layout-change.usecase';
import { GetLayoutUseCase } from '../get-layout';
import { SetDefaultLayoutCommand } from './set-default-layout.command';

@Injectable()
export class SetDefaultLayoutUseCase {
constructor(
private getLayout: GetLayoutUseCase,
private createLayoutChange: CreateLayoutChangeUseCase,
private layoutRepository: LayoutRepository
private createDefaultLayoutChange: CreateDefaultLayoutChangeUseCase,
private layoutRepository: LayoutRepository,
private changeRepository: ChangeRepository
) {}

async execute(command: SetDefaultLayoutCommand) {
const layout = await this.getLayout.execute(command);

const existingDefaultLayoutId = await this.findExistingDefaultLayoutId(
layout._id,
layout._id as string,
command.environmentId,
command.organizationId
);

if (!existingDefaultLayoutId) {
await this.createDefaultChange(command);

return;
}

try {
if (existingDefaultLayoutId) {
await this.setIsDefaultForLayout(existingDefaultLayoutId, command.environmentId, command.organizationId, false);
await this.createLayoutChangeForPreviousDefault(command, existingDefaultLayoutId);
}
await this.setIsDefaultForLayout(existingDefaultLayoutId, command.environmentId, command.organizationId, false);

const existingParentChangeId = await this.getParentChangeId(command.environmentId, existingDefaultLayoutId);
const previousDefaultLayoutChangeId = await this.changeRepository.getChangeId(
command.environmentId,
ChangeEntityTypeEnum.DEFAULT_LAYOUT,
existingDefaultLayoutId
);

await this.createLayoutChangeForPreviousDefault(command, existingDefaultLayoutId, previousDefaultLayoutChangeId);

await this.setIsDefaultForLayout(layout._id, command.environmentId, command.organizationId, true);
await this.setIsDefaultForLayout(layout._id as string, command.environmentId, command.organizationId, true);
await this.createDefaultChange({
...command,
parentChangeId: existingParentChangeId || previousDefaultLayoutChangeId,
});
} catch (error) {
Logger.error(error);
// TODO: Rollback through transactions
Expand All @@ -43,27 +56,10 @@ export class SetDefaultLayoutUseCase {

private async createLayoutChangeForPreviousDefault(
command: SetDefaultLayoutCommand,
layoutId: LayoutId
): Promise<void> {
const createLayoutChangeCommand = CreateLayoutChangeCommand.create({
environmentId: command.environmentId,
layoutId,
organizationId: command.organizationId,
userId: command.userId,
});

await this.createLayoutChange.execute(createLayoutChangeCommand);
}

private mapToEntity(
domainEntity: SetDefaultLayoutCommand
): Pick<LayoutEntity, '_id' | '_environmentId' | '_organizationId' | '_creatorId'> {
return {
_id: domainEntity.userId,
_environmentId: domainEntity.environmentId,
_organizationId: domainEntity.organizationId,
_creatorId: domainEntity.userId,
};
layoutId: LayoutId,
changeId: string
) {
await this.createDefaultChange({ ...command, layoutId, changeId });
}

private async findExistingDefaultLayoutId(
Expand Down Expand Up @@ -93,4 +89,27 @@ export class SetDefaultLayoutUseCase {
): Promise<void> {
await this.layoutRepository.updateIsDefault(layoutId, environmentId, organizationId, isDefault);
}

private async createDefaultChange(command: CreateDefaultLayoutChangeCommand) {
const createLayoutChangeCommand = CreateDefaultLayoutChangeCommand.create({
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you might be creating this command twice :) here and in line 55

Copy link
Member Author

Choose a reason for hiding this comment

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

😅 Removing them

environmentId: command.environmentId,
layoutId: command.layoutId,
organizationId: command.organizationId,
userId: command.userId,
changeId: command.changeId,
parentChangeId: command.parentChangeId,
});

await this.createDefaultLayoutChange.execute(createLayoutChangeCommand);
}

private async getParentChangeId(environmentId: string, layoutId: string) {
const parentChangeId = await this.changeRepository.getParentId(
environmentId,
ChangeEntityTypeEnum.DEFAULT_LAYOUT,
layoutId
);

return parentChangeId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ export class UpdateLayoutUseCase {
userId: dto._creatorId,
});
await this.setDefaultLayout.execute(setDefaultLayoutCommand);
} else {
await this.createChange(command);
}

await this.createChange(command);

return dto;
}

Expand Down
10 changes: 9 additions & 1 deletion apps/web/src/components/changes/ChangesTableLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const ChangesTable = ({
{
accessor: 'change',
Header: 'Change',
Cell: ({ type, templateName, messageType }: any) => (
Cell: ({ type, templateName, messageType, previousDefaultLayout }: any) => (
<div data-test-id="change-type">
{type === ChangeEntityTypeEnum.NOTIFICATION_TEMPLATE && (
<Text color={colorScheme === 'dark' ? colors.B40 : colors.B70}>Template Change</Text>
Expand All @@ -70,6 +70,14 @@ export const ChangesTable = ({
{type === ChangeEntityTypeEnum.LAYOUT && (
<Text color={colorScheme === 'dark' ? colors.B40 : colors.B70}>Layout Change</Text>
)}
{type === ChangeEntityTypeEnum.DEFAULT_LAYOUT && (
<Text color={colorScheme === 'dark' ? colors.B40 : colors.B70}>Default Layout Change</Text>
)}
Comment on lines +73 to +75
Copy link
Contributor

@LetItRock LetItRock Feb 16, 2023

Choose a reason for hiding this comment

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

similar to the line above

{previousDefaultLayout && (
<Text data-test-id="previous-default-layout-content" rows={1} mt={5}>
Previous Default Layout: {previousDefaultLayout}
</Text>
)}
<Text data-test-id="change-content" rows={1} mt={5}>
{templateName}
{messageType ? `, ${messageType}` : null}
Expand Down
22 changes: 22 additions & 0 deletions libs/dal/src/repositories/change/change.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,26 @@ export class ChangeRepository extends BaseRepository<EnforceEnvironmentQuery, Ch

return { totalCount: totalItemsCount, data: this.mapEntities(items) };
}

public async getParentId(
environmentId: string,
entityType: ChangeEntityTypeEnum,
entityId: string
): Promise<string | null> {
const change = await this.findOne(
{
_environmentId: environmentId,
_entityId: entityId,
type: entityType,
enabled: false,
_parentId: { $exists: true },
},
'_parentId'
);
if (change?._parentId) {
return change._parentId;
}

return null;
}
}
13 changes: 13 additions & 0 deletions libs/dal/src/repositories/layout/layout.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,19 @@ export class LayoutRepository extends BaseRepository<EnforceEnvironmentQuery, La
return this.mapEntity(deletedLayout);
}

async findDeletedByParentId(parentId: LayoutId, environmentId: EnvironmentId): Promise<LayoutEntity | undefined> {
const deletedLayout = await this.layout.findOneDeleted({
_parentId: this.convertStringToObjectId(parentId),
_environmentId: this.convertStringToObjectId(environmentId),
});

if (!deletedLayout?._id) {
return undefined;
}

return this.mapEntity(deletedLayout);
}

async filterLayouts(
query: EnforceEnvironmentQuery,
pagination: { limit: number; skip: number; sortBy?: string; orderBy?: OrderDirectionEnum }
Expand Down
1 change: 1 addition & 0 deletions libs/shared/src/entities/change/change.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export enum ChangeEntityTypeEnum {
FEED = 'Feed',
MESSAGE_TEMPLATE = 'MessageTemplate',
LAYOUT = 'Layout',
DEFAULT_LAYOUT = 'DefaultLayout',
NOTIFICATION_TEMPLATE = 'NotificationTemplate',
NOTIFICATION_GROUP = 'NotificationGroup',
}