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

fix(framework): Stop validating controls for non previewed step #6876

Conversation

rifont
Copy link
Contributor

@rifont rifont commented Nov 6, 2024

What changed? Why was the change needed?

  • Due to some code misconfiguration for skip step logical execution, non-previewed steps were having controls validated. This was a bug as no controls are ever supplied to the non-executed step. This change removes controls validation for non-previewed steps

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

Copy link

linear bot commented Nov 6, 2024

status: 'success',
},
},
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An empty outputs should have been supplied for this test, ensuring that a hydration can still occur for skipped steps. This fixes the assertion.

Copy link

netlify bot commented Nov 6, 2024

Deploy Preview for novu-stg-vite-dashboard-poc failed. Why did it fail? →

Name Link
🔨 Latest commit 0fd3267
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/672bf327ba21070008494d56

// Only set the result when the step is the current step.
// Only evaluate a skip condition when the step is the current step and not in preview mode.
if (!isPreview && stepId === event.stepId) {
const controls = await this.createStepControls(step, event);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this change, we ensure step controls are only validated during execution where the controls are guaranteed to be available for the current step being executed.

Copy link

pkg-pr-new bot commented Nov 6, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/novuhq/novu/@novu/framework@6876

commit: 7957854

Copy link

netlify bot commented Nov 6, 2024

Deploy Preview for novu-stg-vite-dashboard-poc failed. Why did it fail? →

Name Link
🔨 Latest commit 7957854
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/672bf36e53170a0008a06c18

return {} as any;
/*
* Return an empty object for results when a step is skipped.
* TODO: fix typings when `skip` is specified to return `Partial<T_Result>`
Copy link
Contributor

Choose a reason for hiding this comment

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

just to make sure it doesn't bypass the second PR and results will be supplied if i provide them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this code branch is only for the non-preview case.

@rifont rifont merged commit f6ed024 into next Nov 7, 2024
35 of 44 checks passed
@rifont rifont deleted the nv-4602-stop-validating-controls-for-non-previewed-step-in-framework branch November 7, 2024 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants