Skip to content

Commit

Permalink
fix(api): fix deletion of issues from persistence once no issues are …
Browse files Browse the repository at this point in the history
…found (#6956)
  • Loading branch information
tatarco authored Nov 13, 2024
1 parent 899d78a commit c7047ef
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import {
NotificationGroupRepository,
NotificationStepEntity,
NotificationTemplateEntity,
PreferencesEntity,
} from '@novu/dal';
import {
CreateWorkflow as CreateWorkflowGeneric,
CreateWorkflowCommand,
GetPreferences,
GetPreferencesCommand,
GetPreferencesResponseDto,
GetWorkflowByIdsCommand,
GetWorkflowByIdsUseCase,
NotificationStep,
shortId,
UpdateWorkflow,
Expand All @@ -22,8 +23,6 @@ import {
UpsertPreferences,
UpsertUserWorkflowPreferencesCommand,
UpsertWorkflowPreferencesCommand,
GetWorkflowByIdsUseCase,
GetWorkflowByIdsCommand,
} from '@novu/application-generic';
import {
CreateWorkflowDto,
Expand All @@ -46,6 +45,7 @@ import { StepUpsertMechanismFailedMissingIdException } from '../../exceptions/st
import { toResponseWorkflowDto } from '../../mappers/notification-template-mapper';
import { stepTypeToDefaultDashboardControlSchema } from '../../shared';
import { ValidateAndPersistWorkflowIssuesUsecase } from './validate-and-persist-workflow-issues.usecase';
import { ValidateWorkflowCommand } from './validate-workflow.command';

function buildUpsertControlValuesCommand(
command: UpsertWorkflowCommand,
Expand Down Expand Up @@ -80,12 +80,14 @@ export class UpsertWorkflowUseCase {
const workflow = await this.createOrUpdateWorkflow(workflowForUpdate, command);
const stepIdToControlValuesMap = await this.upsertControlValues(workflow, command);
const preferences = await this.upsertPreference(command, workflow);
const validatedWorkflowWithIssues = await this.validateWorkflowUsecase.execute({
user: command.user,
workflow,
preferences,
stepIdToControlValuesMap,
});
const validatedWorkflowWithIssues = await this.validateWorkflowUsecase.execute(
ValidateWorkflowCommand.create({
user: command.user,
workflow,
preferences,
stepIdToControlValuesMap,
})
);

return toResponseWorkflowDto(validatedWorkflowWithIssues, preferences);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class ValidateAndPersistWorkflowIssuesUsecase {
if (step.template?.controls) {
const { issuesMissingValues } = this.buildDefaultControlValuesUsecase.execute({
controlSchema: step.template?.controls,
controlValues: stepIdToControlValuesMap,
controlValues: stepIdToControlValuesMap[step._templateId].controls,
});
// eslint-disable-next-line no-param-reassign
stepIssues.controls = issuesMissingValues;
Expand Down Expand Up @@ -194,6 +194,8 @@ export class ValidateAndPersistWorkflowIssuesUsecase {
for (const step of workflow.steps) {
if (stepIssuesMap[step._templateId]) {
step.issues = stepIssuesMap[step._templateId];
} else {
step.issues = undefined;
}
}

Expand Down
18 changes: 16 additions & 2 deletions apps/api/src/app/workflows-v2/workflow.controller.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,20 @@ describe('Workflow Controller E2E API Testing', () => {
expect(issues.body.name?.issueType, JSON.stringify(issues)).to.be.equal(StepIssueEnum.MISSING_REQUIRED_VALUE);
}
});
it('should remove issues when no longer', async () => {
const inAppStep = { ...buildInAppStep(), controlValues: { body: 'some body' }, name: '' };
const workflowCreated = await createWorkflowAndReturn({ steps: [inAppStep] });
const novuRestResult = await workflowsClient.updateWorkflow(workflowCreated._id, {
...workflowCreated,
steps: [{ ...inAppStep, name: 'New Name' }],
});
if (!novuRestResult.isSuccessResult()) {
throw new Error(novuRestResult.error!.responseText);
}
const updatedWorkflow = novuRestResult.value;
expect(updatedWorkflow.steps[0].issues?.body, JSON.stringify(updatedWorkflow.steps[0].issues)).to.be.empty;
expect(updatedWorkflow.steps[0].issues?.controls, JSON.stringify(updatedWorkflow.steps[0].issues)).to.be.empty;
});
});
describe('Workflow Step content Issues', () => {
it('should show control value required when missing', async () => {
Expand Down Expand Up @@ -352,7 +366,7 @@ describe('Workflow Controller E2E API Testing', () => {
description: 'Updated Description',
// modify existing Email Step, add new InApp Steps, previously existing InApp Step is removed
steps: [
{ ...stepToUpdate, name: 'Updated Email Step' },
{ ...buildEmailStep(), _id: devWorkflow.steps[0]._id, name: 'Updated Email Step' },
{ ...buildInAppStep(), name: 'New InApp Step' },
],
};
Expand All @@ -373,7 +387,7 @@ describe('Workflow Controller E2E API Testing', () => {
// Verify updated properties
expect(prodWorkflowUpdated.name).to.equal('Updated Name');
expect(prodWorkflowUpdated.description).to.equal('Updated Description');

console.log('prodWorkflowUpdated\n', JSON.stringify(prodWorkflowUpdated, null, 2));
// Verify unchanged properties
['status', 'type', 'origin'].forEach((prop) => {
expect(prodWorkflowUpdated[prop]).to.deep.equal(prodWorkflowCreated[prop], `Property ${prop} should match`);
Expand Down

0 comments on commit c7047ef

Please sign in to comment.