refactor(api-service): issues management #7344

djabarovgeorge · 2024-12-21T13:51:31Z

What changed? Why was the change needed?

Issue Management:

Responsibility for issues has been shifted to schemas and LiquidJS instead of custom code , that make us aligned with the preview flow.

https://www.loom.com/share/710a5a211e4a4cbaa236fbbcf882008a?provider=slack-openid-connect

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

# Conflicts: # apps/api/package.json # apps/api/src/app/environments-v1/usecases/output-renderers/render-email-output.usecase.ts # apps/api/src/app/workflows-v2/shared/schemas/email-control.schema.ts

# Conflicts: # apps/api/src/app/workflows-v2/shared/schemas/email-control.schema.ts

# Conflicts: # apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts

netlify · 2024-12-21T13:51:51Z

✅ Deploy Preview for dev-web-novu ready!

Name	Link
🔨 Latest commit	`e78a239`
🔍 Latest deploy log	https://app.netlify.com/sites/dev-web-novu/deploys/67692454eb8a91000892875f
😎 Deploy Preview	https://deploy-preview-7344.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.

netlify · 2024-12-21T13:51:51Z

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

Name	Link
🔨 Latest commit	`e78a239`
🔍 Latest deploy log	https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/676924547516e400088edc43
😎 Deploy Preview	https://deploy-preview-7344.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.

apps/api/src/app/environments-v1/usecases/output-renderers/hydrate-email-schema.usecase.ts

SokratisVidros · 2024-12-23T08:06:06Z

apps/api/src/app/environments-v1/usecases/output-renderers/render-email-output.usecase.ts

+    const valueEscapedNewLines = valueSingleQuotes.replace(/\n/g, '\\n');
+
+    return valueEscapedNewLines;
+  } else {


else is not required.

Suggested change

} else {

}

SokratisVidros · 2024-12-23T08:06:35Z

apps/api/src/app/environments-v1/usecases/output-renderers/render-email-output.usecase.ts

    return this.expandEmailEditorSchemaUseCase.execute({ emailEditorJson: body, fullPayloadForRender });
  }
 }
+
+export const stringifyDataStructureWithSingleQuotes = (value: unknown, spaces: number = 0): string => {


@djabarovgeorge can you please elaborate with an example? Why do we need this extra handling on single quotes?

A few reasons:

Alignment with the Liquid.js client used in the framework.

I noticed that when a variable is missing, the framework currently parses it as unknown (we have it today in the framework). This option causes this behavior. I believe it's an issue, but I couldn't fix it quickly, so I suggest we address it separately.
For now, we've ensured consistent behavior across channel steps.

SokratisVidros · 2024-12-23T08:09:25Z

apps/api/src/app/workflows-v2/shared/step-type-to-control.mapper.ts

@@ -1,11 +1,12 @@
 import { ActionStepEnum, ChannelStepEnum } from '@novu/framework/internal';


Nit-picky: This file could be replaced by a mapper returned from the ./schemas/index.ts to avoid introducing a new filename, mapper.

I like the idea and have used this pattern myself, but I noticed that not everyone on the team is aligned with it. I suggest introducing it to the team first.

SokratisVidros · 2024-12-23T08:12:15Z

apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts

      );

      return {
        result: {
          preview: executeOutput.outputs as any,
          type: stepData.type as unknown as ChannelTypeEnum,
        },
-        previewPayloadExample: finalVariablesExample,
+        previewPayloadExample: mergedVariablesExample,


As per lexicon, these are the preview control data. Let's add a TODO for the renaming in a follow up PR.

I also have a task in my list, to remove the nesting added by the result attribute from this endpoint.

apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts

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

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

djabarovgeorge added 23 commits December 12, 2024 15:51

feat(api): cherry pick liquid parser refactor

0a89674

feat(api): poc of none email preview

5e6754e

feat(api): wip email preview

7b898d7

feat(api): add email preview

13806f1

chore(api): refactor preview data structure

02ba6e0

chore(api): remove stale code

35e341b

Merge remote-tracking branch 'origin/next' into preview-usecase

4c8b0e3

# Conflicts: # apps/api/package.json # apps/api/src/app/environments-v1/usecases/output-renderers/render-email-output.usecase.ts # apps/api/src/app/workflows-v2/shared/schemas/email-control.schema.ts

chore(api): remove redundant deps

472f610

Merge remote-tracking branch 'origin/next' into preview-usecase

8ad7d1c

# Conflicts: # apps/api/src/app/workflows-v2/shared/schemas/email-control.schema.ts

chore(api): remove redundant second variables compute

ef47067

refactor(api): replace invalid variable with empty string

48f5434

Merge remote-tracking branch 'origin/next' into preview-usecase

660ac49

fix(api): wip fix tests

266a5ef

fix(api): tests

7a5899c

refactor(api): execute preview per control value

57c531d

refactor(api): wip generate tiptap variables

2bdf193

Merge remote-tracking branch 'origin/next' into preview-usecase

6513589

# Conflicts: # apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts

feat(api): tmp skip for bug bash

e6a1720

Merge remote-tracking branch 'origin/next' into preview-usecase

bc72eb1

feat(api): polish

0a9605d

feat(api): refactor issues generation

a024b6c

fix(api): after refactor

335b82d

feat(api): add tier issue

0bfbbb8

djabarovgeorge temporarily deployed to Linting December 21, 2024 13:51 — with GitHub Actions Inactive

djabarovgeorge had a problem deploying to Linting December 21, 2024 13:51 — with GitHub Actions Error

github-actions bot added the @novu/api label Dec 21, 2024

Merge remote-tracking branch 'origin/next' into preview-usecase

841ef80

feat(api): update after pr comments

bdb9dc1

djabarovgeorge temporarily deployed to Linting December 22, 2024 21:00 — with GitHub Actions Inactive

feat(api): update after pr comments

8e0126b

djabarovgeorge had a problem deploying to Linting December 22, 2024 21:40 — with GitHub Actions Error

djabarovgeorge temporarily deployed to Linting December 22, 2024 21:40 — with GitHub Actions Inactive

feat(api): update after pr comments

62e5ed6

djabarovgeorge temporarily deployed to Linting December 22, 2024 21:41 — with GitHub Actions Inactive

feat(api): update after pr comments

8a001f0

djabarovgeorge temporarily deployed to Linting December 22, 2024 22:25 — with GitHub Actions Inactive

SokratisVidros approved these changes Dec 23, 2024

View reviewed changes

feat(api): update after pr comments

411a187

djabarovgeorge temporarily deployed to Linting December 23, 2024 08:42 — with GitHub Actions Inactive

github-actions bot added the @novu/api-service label Dec 23, 2024

Merge remote-tracking branch 'origin/next' into preview-usecase

e78a239

djabarovgeorge changed the title ~~refactor(api): preview usecase~~ refactor(api-service): preview usecase Dec 23, 2024

djabarovgeorge temporarily deployed to Linting December 23, 2024 08:50 — with GitHub Actions Inactive

djabarovgeorge merged commit aa64780 into next Dec 23, 2024
43 of 45 checks passed

djabarovgeorge deleted the preview-usecase branch December 23, 2024 09:27

djabarovgeorge changed the title ~~refactor(api-service): preview usecase~~ refactor(api-service): issues management Dec 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(api-service): issues management #7344

refactor(api-service): issues management #7344

djabarovgeorge commented Dec 21, 2024 •

edited

Loading

netlify bot commented Dec 21, 2024 •

edited

Loading

netlify bot commented Dec 21, 2024 •

edited

Loading

SokratisVidros Dec 23, 2024

SokratisVidros Dec 23, 2024

djabarovgeorge Dec 23, 2024

SokratisVidros Dec 23, 2024

djabarovgeorge Dec 23, 2024

SokratisVidros Dec 23, 2024

djabarovgeorge Dec 23, 2024

		@@ -1,11 +1,12 @@
		import { ActionStepEnum, ChannelStepEnum } from '@novu/framework/internal';

refactor(api-service): issues management #7344

refactor(api-service): issues management #7344

Conversation

djabarovgeorge commented Dec 21, 2024 • edited Loading

What changed? Why was the change needed?

Screenshots

Related enterprise PR

Special notes for your reviewer

netlify bot commented Dec 21, 2024 • edited Loading

✅ Deploy Preview for dev-web-novu ready!

netlify bot commented Dec 21, 2024 • edited Loading

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

SokratisVidros Dec 23, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 23, 2024

Choose a reason for hiding this comment

djabarovgeorge Dec 23, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 23, 2024

Choose a reason for hiding this comment

djabarovgeorge Dec 23, 2024

Choose a reason for hiding this comment

SokratisVidros Dec 23, 2024

Choose a reason for hiding this comment

djabarovgeorge Dec 23, 2024

Choose a reason for hiding this comment

djabarovgeorge commented Dec 21, 2024 •

edited

Loading

netlify bot commented Dec 21, 2024 •

edited

Loading

netlify bot commented Dec 21, 2024 •

edited

Loading