feat(dashboard): add delay step #7131

ChmaraX · 2024-11-26T12:16:10Z

What changed? Why was the change needed?

Current know issues:

LD flag for disabling steps (will be done in separate PR)
[visual] the error border is missing on the amount field (will be done in separate PR)

Added logic for control value forms which are on the first sidebar:

wf_delay_step.mp4

linear · 2024-11-26T12:16:13Z

NV-4575 Implement Delay step

netlify · 2024-11-26T12:19:53Z

✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!

Name	Link
🔨 Latest commit	`2a08599`
🔍 Latest deploy log	https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/6745d5864c78640008f5a263
😎 Deploy Preview	https://deploy-preview-7131--novu-stg-vite-dashboard-poc.netlify.app
📱 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/dashboard/src/components/number-input-with-select.tsx

apps/dashboard/src/components/workflow-editor/add-step-menu.tsx

apps/dashboard/src/components/workflow-editor/steps/delay/delay-configure.tsx

apps/dashboard/src/components/workflow-editor/workflow-canvas.tsx

...cation-generic/src/usecases/tier-restrictions-validate/tier-restrictions-validate.usecase.ts

ChmaraX · 2024-12-03T15:38:25Z

...shboard/src/components/workflow-editor/steps/configure-inline/configure-step-inline-form.tsx

+};
+
+export const ConfigureStepInlineForm = (props: ConfigureStepInlineFormProps) => {
+  const { workflow, step, issues, update, updateStepCache } = props;


we are drilling the props quite deep, we should think about using the hooks here, but I left it like this for consistency.

ChmaraX · 2024-12-03T15:40:35Z

apps/dashboard/src/components/workflow-editor/steps/step-provider.tsx

+  /**
+   * We need to get the issues from the workflow response
+   * because the step is not re-fetched when workflow is updated
+   *
+   * TODO:
+   * 1. add all step data to workflow response
+   * 2. remove StepProvider and keep just the WorkflowProvider with step value
+   */
+  const issues = useMemo(() => {
+    const newIssues = workflow?.steps.find(
+      (s) =>
+        getEncodedId({ slug: s.slug, divider: STEP_DIVIDER }) ===
+        getEncodedId({ slug: stepSlug, divider: STEP_DIVIDER })
+    )?.issues;
+
+    return { ...newIssues };
+  }, [workflow, stepSlug]);


The fact that we update both step and workflow with one endpoint, but fetch the updated data from 2 endpoints causes a lot of issues regarding stale data, caching etc etc.

We should fix this asap by fetching everything from workflows response as I suggested here in the comment or some other solution.

github-actions · 2024-12-03T15:44:35Z

LaunchDarkly flag references

🔍 1 flag added or modified

Name	Key	Aliases found	Info
IS_ND_DELAY_DIGEST_EMAIL_ENABLED	`IS_ND_DELAY_DIGEST_EMAIL_ENABLED`

pkg-pr-new · 2024-12-03T15:48:47Z

Open in Stackblitz

@novu/client

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

@novu/framework

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

@novu/js

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

@novu/headless

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

@novu/nextjs

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

@novu/node

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

@novu/notification-center

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

novu

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

@novu/providers

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

@novu/react

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

@novu/react-native

npm i https://pkg.pr.new/novuhq/novu/@novu/react-native@7131

@novu/shared

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

commit: 44f83c5

ChmaraX · 2024-12-03T15:55:50Z

apps/dashboard/src/components/workflow-editor/steps/step-provider.tsx


 export type StepEditorContextType = {
  isPending: boolean;
  step?: StepDataDto;
-  refetch: (options?: RefetchOptions) => Promise<QueryObserverResult<StepDataDto, Error>>;


SokratisVidros

I strongly suggest decoupling the form logic from the form wrapper. That is, the form of every step editor should work both in the first side bar or the second sidebar.

Currently, I noticed a lot of duplication between the template form and the inline form. Introducing a StepForm component to host the form handling that is reused from the different sidebar containers should simplify the logic drastically.

SokratisVidros · 2024-12-03T15:48:59Z

apps/dashboard/src/components/number-input-with-select.tsx

+  isReadOnly?: boolean;
+};
+
+export const NumberInputWithSelect = ({


Suggested change

export const NumberInputWithSelect = ({

export const DurationInput = ({

Its a generic component not necessarily a duration, it can be also 10 horses

SokratisVidros · 2024-12-03T15:52:44Z

apps/dashboard/src/components/number-input-with-select.tsx

+import { useMemo } from 'react';
+
+type InputWithSelectProps = {
+  fields: {


Nested in props is always 👿 . how about using two flat props?

unit: 'seconds' | 'minutes' | 'hours' | 'months'

amount: number

This should simplify the logic of the handleChange a lot.

@SokratisVidros this is what we agreed before with @desiprisg and we use this pattern in other places where the component is using two form fields

SokratisVidros · 2024-12-03T15:53:16Z

apps/dashboard/src/components/number-input-with-select.tsx

+    selectKey: string;
+  };
+  options: string[];
+  defaultOption?: string;


Do we need the default option prop? The unit prop should be the default option otherwise its defaultOption[0].

SokratisVidros · 2024-12-03T15:57:50Z

apps/dashboard/src/components/number-input-with-select.tsx

+  placeholder,
+  isReadOnly,
+}: InputWithSelectProps) => {
+  const { getFieldState, setValue, getValues, control } = useFormContext();


I suggest making the DurationInput a primitive and then wiring it to the RHF either in workflow editor or with a wrapper component under /components/workflow-editor if necessary.

It will give us way more flexibility and make the component reusable.

The wrapper component already exists. See here.

I'm not sure we can do something about it because this component uses the two form fields, check the FormField usage.

apps/dashboard/src/components/number-input-with-select.tsx

apps/dashboard/src/components/workflow-editor/add-step-menu.tsx

apps/dashboard/src/components/workflow-editor/workflow-canvas.tsx

SokratisVidros · 2024-12-03T16:05:26Z

...shboard/src/components/workflow-editor/steps/configure-inline/configure-step-inline-form.tsx

+import { SaveFormContext } from '@/components/workflow-editor/steps/save-form-context';
+import { DelayForm } from '@/components/workflow-editor/steps/delay/delay-form';
+
+const STEP_TYPE_TO_INLINE_FORM: Record<StepTypeEnum, (args: StepInlineFormProps) => React.JSX.Element | null> = {


This code along with calculateDefaultValues is duplicated between this new component and configure-step-template-form.tsx We need to make it dry.

I agree.
Also, it will result in the form nested under another form, which we wanted to avoid.
The configure-step-form should just register all the fields under the controls in the form I guess, because this is the shape of the step and that will be updated later with using the update workflow endpoint.

{ "name": "In-App Step", "stepId": "some-step-id", "controls": { ... } }

Later here we will also need to connect custom step controls from the code created workflows.

LetItRock · 2024-12-03T17:10:02Z

apps/dashboard/src/components/number-input-with-select.tsx

+import { useMemo } from 'react';
+
+type InputWithSelectProps = {
+  fields: {


@SokratisVidros this is what we agreed before with @desiprisg and we use this pattern in other places where the component is using two form fields

LetItRock · 2024-12-03T17:12:51Z

apps/dashboard/src/components/number-input-with-select.tsx

+  placeholder,
+  isReadOnly,
+}: InputWithSelectProps) => {
+  const { getFieldState, setValue, getValues, control } = useFormContext();


I'm not sure we can do something about it because this component uses the two form fields, check the FormField usage.

LetItRock · 2024-12-03T17:16:28Z

apps/dashboard/src/components/workflow-editor/add-step-menu.tsx

@@ -73,6 +75,7 @@ export const AddStepMenu = ({
  onMenuItemClick: (stepType: StepTypeEnum) => void;
 }) => {
  const [isPopoverOpen, setIsPopoverOpen] = useState(false);
+  const isDelayDigestEmailEnabled = useFeatureFlag(FeatureFlagsKeysEnum.IS_ND_DELAY_DIGEST_EMAIL_ENABLED);


what ND means? new dashboard?

LetItRock · 2024-12-03T17:26:30Z

...shboard/src/components/workflow-editor/steps/configure-inline/configure-step-inline-form.tsx

+import { SaveFormContext } from '@/components/workflow-editor/steps/save-form-context';
+import { DelayForm } from '@/components/workflow-editor/steps/delay/delay-form';
+
+const STEP_TYPE_TO_INLINE_FORM: Record<StepTypeEnum, (args: StepInlineFormProps) => React.JSX.Element | null> = {


I agree.
Also, it will result in the form nested under another form, which we wanted to avoid.
The configure-step-form should just register all the fields under the controls in the form I guess, because this is the shape of the step and that will be updated later with using the update workflow endpoint.

{ "name": "In-App Step", "stepId": "some-step-id", "controls": { ... } }

LetItRock · 2024-12-03T17:27:40Z

...shboard/src/components/workflow-editor/steps/configure-inline/configure-step-inline-form.tsx

+import { SaveFormContext } from '@/components/workflow-editor/steps/save-form-context';
+import { DelayForm } from '@/components/workflow-editor/steps/delay/delay-form';
+
+const STEP_TYPE_TO_INLINE_FORM: Record<StepTypeEnum, (args: StepInlineFormProps) => React.JSX.Element | null> = {


Later here we will also need to connect custom step controls from the code created workflows.

feat(dashboard): add delay step

146e9e2

Merge branch 'next' into nv-4575-implement-delay-step

4eae38e

ChmaraX temporarily deployed to Linting November 26, 2024 12:27 — with GitHub Actions Inactive

ChmaraX commented Nov 26, 2024

View reviewed changes

apps/dashboard/src/components/number-input-with-select.tsx Outdated Show resolved Hide resolved

ChmaraX commented Nov 26, 2024

View reviewed changes

apps/dashboard/src/components/workflow-editor/add-step-menu.tsx Outdated Show resolved Hide resolved

ChmaraX commented Nov 26, 2024

View reviewed changes

apps/dashboard/src/components/workflow-editor/steps/delay/delay-configure.tsx Outdated Show resolved Hide resolved

feat(dashboard): add delay step

b1c6f9f

ChmaraX temporarily deployed to Linting November 26, 2024 12:35 — with GitHub Actions Inactive

feat(dashboard): add delay step

5c1378e

ChmaraX temporarily deployed to Linting November 26, 2024 12:38 — with GitHub Actions Inactive

ChmaraX commented Nov 26, 2024

View reviewed changes

apps/dashboard/src/components/workflow-editor/workflow-canvas.tsx Show resolved Hide resolved

ChmaraX commented Nov 26, 2024

View reviewed changes

...cation-generic/src/usecases/tier-restrictions-validate/tier-restrictions-validate.usecase.ts Show resolved Hide resolved

ChmaraX requested review from LetItRock, SokratisVidros, djabarovgeorge and desiprisg November 26, 2024 12:50

feat(dashboard): add delay step

07642c1

ChmaraX temporarily deployed to Linting November 26, 2024 12:57 — with GitHub Actions Inactive

ChmaraX temporarily deployed to Linting November 26, 2024 12:58 — with GitHub Actions Inactive

feat(dashboard): isolate the actual delay component from the sidebar

c2b3c52

ChmaraX temporarily deployed to Linting November 26, 2024 13:34 — with GitHub Actions Inactive

ChmaraX temporarily deployed to Linting December 3, 2024 14:41 — with GitHub Actions Inactive

Merge branch 'next' into nv-4575-implement-delay-step

ef822ac

ChmaraX temporarily deployed to Linting December 3, 2024 14:53 — with GitHub Actions Inactive

feat(dashboard): get step issues from workflow

4c59b6f

ChmaraX temporarily deployed to Linting December 3, 2024 15:17 — with GitHub Actions Inactive

ChmaraX had a problem deploying to Linting December 3, 2024 15:17 — with GitHub Actions Failure

chore: spellcheck

2bbe057

ChmaraX temporarily deployed to Linting December 3, 2024 15:19 — with GitHub Actions Inactive

feat(dashboard): node step issue

b76624f

ChmaraX temporarily deployed to Linting December 3, 2024 15:23 — with GitHub Actions Inactive

ChmaraX requested a review from LetItRock December 3, 2024 15:24

ChmaraX commented Dec 3, 2024

View reviewed changes

feat(dashboard): add ld flag for email, digest, delay (#7205)

44f83c5

ChmaraX temporarily deployed to Linting December 3, 2024 15:43 — with GitHub Actions Inactive

github-actions bot added the @novu/shared label Dec 3, 2024

ChmaraX commented Dec 3, 2024

View reviewed changes

SokratisVidros requested changes Dec 3, 2024

View reviewed changes

LetItRock reviewed Dec 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dashboard): add delay step #7131

feat(dashboard): add delay step #7131

ChmaraX commented Nov 26, 2024 •

edited

Loading

linear bot commented Nov 26, 2024

netlify bot commented Nov 26, 2024 •

edited

Loading

ChmaraX Dec 3, 2024

ChmaraX Dec 3, 2024

github-actions bot commented Dec 3, 2024

pkg-pr-new bot commented Dec 3, 2024

ChmaraX Dec 3, 2024

SokratisVidros left a comment

SokratisVidros Dec 3, 2024

ChmaraX Dec 4, 2024

SokratisVidros Dec 3, 2024

LetItRock Dec 3, 2024

SokratisVidros Dec 3, 2024

SokratisVidros Dec 3, 2024

SokratisVidros Dec 3, 2024

LetItRock Dec 3, 2024

SokratisVidros Dec 3, 2024

LetItRock Dec 3, 2024

LetItRock Dec 3, 2024

LetItRock Dec 3, 2024

LetItRock Dec 3, 2024

LetItRock Dec 3, 2024

ChmaraX Dec 4, 2024

LetItRock Dec 3, 2024

LetItRock Dec 3, 2024

	export const NumberInputWithSelect = ({
	export const DurationInput = ({

feat(dashboard): add delay step #7131

Are you sure you want to change the base?

feat(dashboard): add delay step #7131

Conversation

ChmaraX commented Nov 26, 2024 • edited Loading

What changed? Why was the change needed?

linear bot commented Nov 26, 2024

netlify bot commented Nov 26, 2024 • edited Loading

✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 3, 2024

LaunchDarkly flag references

🔍 1 flag added or modified

pkg-pr-new bot commented Dec 3, 2024

Choose a reason for hiding this comment

SokratisVidros 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChmaraX commented Nov 26, 2024 •

edited

Loading

netlify bot commented Nov 26, 2024 •

edited

Loading