feat(api): add full step data to workflow dto; refactor #7235

ChmaraX · 2024-12-07T19:25:08Z

What changed? Why was the change needed?

TL;DR:

The external workflow DTO now includes full step data (with control values etc). This supports multiple upcoming features and simplifies both the frontend and backend significantly.
A unified external step DTO replaces previously inconsistent step DTOs returned by GET step and GET workflow endpoints

💡 These changes do not affect FE-BE compatibility in any way - only new fields were added to workflow.steps, no external DTO property was removed or renamed.

Key DTO changes:

File renames and moves:

step-data.dto ➔ step.dto.ts (better reflects contents and avoids circular imports).
All step-related DTOs consolidated in step.dto.ts.
workflow.dto.ts ➔ workflow.deprecated.dto (contains only deprecated DTOs).
workflow-response.dto ➔ workflow.dto.ts.
All workflow-related DTOs consolidated in workflow.dto.ts.

Simplified Step DTO:

Replaced StepResponseDto with StepDataDto (already used by GET step).
This unification reduces redundancy and simplifies the codebase.

Refactoring:

sync-to-environment.usecase - improved naming, less code, clearer intent
Removed redundant workflow e2e code (full step data is now available without extra fetches).

Next steps:

Update the frontend to use step data from the GET workflow endpoint.
1.1 this will enable us to get rid of react-query cache invalidation code, step hooks etc
Remove unused API GET step endpoint, (I think we should also remove PATCH step + usecases) since we are currently not using them - if we decide to use them again in future we can find them in git history
Simplify API e2e tests

linear · 2024-12-07T19:25:12Z

NV-4974 Add step data to GET workflow endpoint

netlify · 2024-12-07T19:25:32Z

✅ Deploy Preview for dashboard-v2-novu-staging ready!

Name	Link
🔨 Latest commit	`01e4a62`
🔍 Latest deploy log	https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/67570f5351e84500088817b3
😎 Deploy Preview	https://deploy-preview-7235.dashboard-v2.novu-staging.co
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2024-12-07T19:25:33Z

✅ Deploy Preview for dev-web-novu ready!

Name	Link
🔨 Latest commit	`01e4a62`
🔍 Latest deploy log	https://app.netlify.com/sites/dev-web-novu/deploys/67570f53ae2355000835e25c
😎 Deploy Preview	https://deploy-preview-7235.dashboard.novu-staging.co
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

pkg-pr-new · 2024-12-07T20:01:29Z

Open in Stackblitz

@novu/client

npm i https://pkg.pr.new/novuhq/novu/@novu/client@7235

@novu/headless

npm i https://pkg.pr.new/novuhq/novu/@novu/headless@7235

@novu/framework

npm i https://pkg.pr.new/novuhq/novu/@novu/framework@7235

@novu/node

npm i https://pkg.pr.new/novuhq/novu/@novu/node@7235

@novu/notification-center

npm i https://pkg.pr.new/novuhq/novu/@novu/notification-center@7235

novu

npm i https://pkg.pr.new/novuhq/novu@7235

@novu/providers

npm i https://pkg.pr.new/novuhq/novu/@novu/providers@7235

@novu/shared

npm i https://pkg.pr.new/novuhq/novu/@novu/shared@7235

commit: 01e4a62

djabarovgeorge

looks good! :)
left a couple of comments for your review.

apps/api/src/app/workflows-v2/mappers/notification-template-mapper.ts

djabarovgeorge · 2024-12-08T11:56:28Z

apps/api/src/app/workflows-v2/mappers/notification-template-mapper.ts

@@ -30,7 +33,7 @@ export function toResponseWorkflowDto(workflow: WorkflowInternalResponseDto): Wo
    tags: workflow.tags,
    active: workflow.active,
    preferences: preferencesDto,
-    steps: getSteps(workflow),
+    steps: fullSteps,


I believe we should keep the mapping here for two key reasons:

We’re adding extra parameters, such as slug, which are only exposed to the API client.

We’re mapping the types—for instance, if the entity allows undefined, we convert those to stricter types in the DTO response.

the first point is not relevant, i see that you added the slug to the BuildStepDataUsecase.

I see your point but the stricter types in this case are already coming from the BuildStepDataUsecase.

The return type from the BuildStepDataUsecase doesn't allow undefined and does explicit checks for missing values, therefore having additional response type for step and mapping, e.g. like this:

name: step.name || 'Missing step name'

would be redundant . I think we should keep it simple and when the usecase emerges then we can have some mapping.

apps/api/src/app/workflows-v2/usecases/get-workflow/get-workflow.usecase.ts

packages/shared/src/dto/workflows/workflow-deprecated.dto.ts

tatarco · 2024-12-09T10:05:11Z

apps/api/src/app/workflows-v2/mappers/notification-template-mapper.ts

@@ -15,7 +15,10 @@ import { NotificationStepEntity, NotificationTemplateEntity } from '@novu/dal';
 import { WorkflowInternalResponseDto } from '@novu/application-generic';
 import { buildSlug } from '../../shared/helpers/build-slug';

-export function toResponseWorkflowDto(workflow: WorkflowInternalResponseDto): WorkflowResponseDto {
+export function toResponseWorkflowDto(


the point was letting this map to dto, if you send the DTO pre cooked I think it's taking the string out of the funciton no? I would expect to get the entity list and maps of the additinoal data you want to bake in

It still maps the workflow to DTO as name of the method suggests, only the steps are coming in already in "DTO form".

We simply don't need to do any mapping for the steps for now, its already in viable form, which is good. Each layer of mapping just adds an overhead

apps/api/src/app/workflows-v2/usecases/get-workflow/get-workflow.usecase.ts

apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts

packages/shared/src/dto/workflows/workflow-deprecated.dto.ts

tatarco

It's looking OK, I'm missing some tests and added some comments, lets please resolve / reply to them before merging

ChmaraX · 2024-12-09T14:25:31Z

apps/api/src/app/workflows-v2/mappers/notification-template-mapper.ts

-function toStepResponseDto(persistedStep: NotificationStepEntity): StepResponseDto {
-  const stepName = persistedStep.name || 'Missing Name';
-
-  return {
-    _id: persistedStep._templateId,
-    slug: buildSlug(stepName, ShortIsPrefixEnum.STEP, persistedStep._templateId),
-    name: stepName,
-    stepId: persistedStep.stepId || 'Missing Step Id',
-    type: persistedStep.template?.type || StepTypeEnum.EMAIL,
-    issues: persistedStep.issues,
-  } satisfies StepResponseDto;
-}


I moved this directly to the build-step-data use-case, because its always used together with the use-case.

If the usecase for unsanitized step data emerges, then we can split it again or create GetStepDataInternal usecase.

Now I don't think its even possible to create step without name or stepId or without type, it seems to be here only because of tests. These things should be validated upon creation and not rely on fallbacks anyway.

feat(api): add full step data to workflow dto; refactor

d829484

github-actions bot added @novu/api @novu/shared labels Dec 7, 2024

Merge branch 'next' into nv-4974-add-step-data-to-get-workflow-endpoint

546801d

ChmaraX temporarily deployed to Linting December 7, 2024 19:56 — with GitHub Actions Inactive

ChmaraX requested review from SokratisVidros, djabarovgeorge and tatarco December 7, 2024 19:57

ChmaraX marked this pull request as ready for review December 7, 2024 20:09

djabarovgeorge reviewed Dec 8, 2024

View reviewed changes

tatarco reviewed Dec 9, 2024

View reviewed changes

apps/api/src/app/workflows-v2/usecases/get-workflow/get-workflow.usecase.ts Show resolved Hide resolved

tatarco reviewed Dec 9, 2024

View reviewed changes

apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts Outdated Show resolved Hide resolved

tatarco reviewed Dec 9, 2024

View reviewed changes

packages/shared/src/dto/workflows/workflow-deprecated.dto.ts Show resolved Hide resolved

tatarco approved these changes Dec 9, 2024

View reviewed changes

feat(dashboard): minor fixes and improvements

0302cd8

ChmaraX temporarily deployed to Linting December 9, 2024 11:11 — with GitHub Actions Inactive

github-actions bot added the @novu/dashboard label Dec 9, 2024

ChmaraX commented Dec 9, 2024

View reviewed changes

feat(dashboard): improvements & fixes

01e4a62

ChmaraX temporarily deployed to Linting December 9, 2024 15:40 — with GitHub Actions Inactive

ChmaraX merged commit d87ec5f into next Dec 10, 2024
43 checks passed

ChmaraX deleted the nv-4974-add-step-data-to-get-workflow-endpoint branch December 10, 2024 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): add full step data to workflow dto; refactor #7235

feat(api): add full step data to workflow dto; refactor #7235

ChmaraX commented Dec 7, 2024 •

edited

Loading

linear bot commented Dec 7, 2024

netlify bot commented Dec 7, 2024 •

edited

Loading

netlify bot commented Dec 7, 2024 •

edited

Loading

pkg-pr-new bot commented Dec 7, 2024 •

edited

Loading

djabarovgeorge left a comment

djabarovgeorge Dec 8, 2024

djabarovgeorge Dec 8, 2024

ChmaraX Dec 9, 2024

tatarco Dec 9, 2024

ChmaraX Dec 9, 2024

tatarco left a comment

ChmaraX Dec 9, 2024

feat(api): add full step data to workflow dto; refactor #7235

feat(api): add full step data to workflow dto; refactor #7235

Conversation

ChmaraX commented Dec 7, 2024 • edited Loading

What changed? Why was the change needed?

TL;DR:

Key DTO changes:

Refactoring:

Next steps:

linear bot commented Dec 7, 2024

netlify bot commented Dec 7, 2024 • edited Loading

✅ Deploy Preview for dashboard-v2-novu-staging ready!

netlify bot commented Dec 7, 2024 • edited Loading

✅ Deploy Preview for dev-web-novu ready!

pkg-pr-new bot commented Dec 7, 2024 • edited Loading

djabarovgeorge left a comment

Choose a reason for hiding this comment

djabarovgeorge Dec 8, 2024

Choose a reason for hiding this comment

djabarovgeorge Dec 8, 2024

Choose a reason for hiding this comment

ChmaraX Dec 9, 2024

Choose a reason for hiding this comment

tatarco Dec 9, 2024

Choose a reason for hiding this comment

ChmaraX Dec 9, 2024

Choose a reason for hiding this comment

tatarco left a comment

Choose a reason for hiding this comment

ChmaraX Dec 9, 2024

Choose a reason for hiding this comment

ChmaraX commented Dec 7, 2024 •

edited

Loading

netlify bot commented Dec 7, 2024 •

edited

Loading

netlify bot commented Dec 7, 2024 •

edited

Loading

pkg-pr-new bot commented Dec 7, 2024 •

edited

Loading