feat(dashboard): show read-only step controls in non-dev environments fixes NV-7362#10715
feat(dashboard): show read-only step controls in non-dev environments fixes NV-7362#10715
Conversation
Co-authored-by: Dima Grossman <dima@grossman.io>
📝 WalkthroughWalkthroughThis PR replaces the “unavailable” UX with read-only frames: new Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Step/Layout Editor
participant ReadOnly as ReadOnly Frame
participant EnvSvc as Environment Hook
participant Router as Router
Editor->>ReadOnly: render children (editor fields)
alt environment !== DEV
ReadOnly->>EnvSvc: compute opposite/development environment
ReadOnly->>EnvSvc: switchEnvironment(devSlug)
EnvSvc-->>ReadOnly: success
ReadOnly->>Router: navigate to edit route (envSlug, layout/workflowSlug)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@cursoragent make the editing not available note more compact, and try to use some of the existing minimal banner compomnents in one line with proper padding above and below |
|
Here’s what changed in response to the review. Update
return (
<div className={cn('flex h-full min-h-0 flex-col', className)}>
<div className="shrink-0 px-3 py-2.5">
<InlineToast
variant="info"
className="w-full"
title="Read-only"
description="Editing is available in development."
ctaLabel={switchAction?.label}
onCtaClick={switchAction?.onClick}
/>
</div>
<div className="pointer-events-none min-h-0 flex-1 select-text overflow-auto">{children}</div>
</div>
);Committed and pushed to the PR branch; PR #10715 description was refreshed to mention   |
…NV-7362 Co-authored-by: Dima Grossman <dima@grossman.io>
|
@cursoragent the inputs on in app and other areas are still clickable and editable, also make the read only banner in less poppy color like regular grey we have in other palces in the sysmte, and also there seem to be an extra padding on left and right there |
|
Here is what was wrong and what we changed. Why controls stayed editable
Fix
Banner: calmer + less horizontal padding
return (
<div className={cn('flex h-full min-h-0 flex-col', className)}>
<div className="shrink-0 py-2.5">
<InlineToast
variant="tip"
className="w-full shadow-none"
title="Read-only"
description="Editing is available in development."
ctaLabel={switchAction?.label}
onCtaClick={switchAction?.onClick}
ctaClassName="text-foreground-600 hover:text-foreground-950"
/>
</div>
<div className="min-h-0 flex-1 select-text overflow-auto" inert>
{children}
</div>
</div>
);Pushed on branch |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/dashboard/src/components/workflow-editor/steps/push/push-editor.tsx (1)
14-31: Consider consolidating the repeated read-only wrap pattern.The same
const xFields = (...); if (!DEV) return <WorkflowReadOnlyStepEditor>{xFields}</WorkflowReadOnlyStepEditor>; return xFields;block is duplicated across push / sms / chat / in-app / http-request / email editors. A tiny helper (e.g.useEditorReadOnlyWrap(children)or a wrapper component that always renders and internally decides whether to apply the frame) would remove six copies of the same env check. Optional; safe to defer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/workflow-editor/steps/push/push-editor.tsx` around lines 14 - 31, The push editor repeats the DEV check/readonly wrapping pattern for pushFields; extract that logic into a small helper or wrapper (e.g. useEditorReadOnlyWrap or EditorReadOnlyWrapper) and replace the pattern in push-editor: move the currentEnvironment/EnvironmentTypeEnum.DEV check inside the helper so callers just return useEditorReadOnlyWrap(pushFields) or <EditorReadOnlyWrapper>{pushFields}</EditorReadOnlyWrapper>; ensure the helper uses the same WorkflowReadOnlyStepEditor when currentEnvironment?.type !== EnvironmentTypeEnum.DEV and otherwise returns children unmodified so you can reuse it across push/sms/chat/in-app/http-request/email editors referencing pushFields and WorkflowReadOnlyStepEditor.apps/dashboard/src/components/workflow-editor/steps/email/email-editor.tsx (1)
49-57: Minor: collapse the always-present wrapper.The outer
<div className="flex h-full flex-col">is rendered in both branches, so the DEV/non-DEV ternary can be inlined inside a single wrapper (orWorkflowReadOnlyStepEditorcan itself carry the flex container) to remove one layer of indirection. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/workflow-editor/steps/email/email-editor.tsx` around lines 49 - 57, The outer div with className="flex h-full flex-col" is duplicated across the DEV/non-DEV branches; collapse it by rendering that div once and moving the currentEnvironment?.type === EnvironmentTypeEnum.DEV ? ... : ... ternary inside it so it returns either emailFields or <WorkflowReadOnlyStepEditor>{emailFields}</WorkflowReadOnlyStepEditor>; update the JSX in email-editor.tsx to remove the branch that wraps the whole div and ensure WorkflowReadOnlyStepEditor still receives emailFields only for non-DEV environments.apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx (1)
48-48: Usetype === EnvironmentTypeEnum.DEVinstead of matching by name.Checking
oppositeEnvironment?.name === 'Development'is inconsistent with the entire codebase, where environment type is determined viaenvironment.type === EnvironmentTypeEnum.DEV. This pattern is used in 50+ places across the dashboard includingenvironment-dropdown.tsxand all step editors. Matching by name is fragile and breaks if the environment is renamed or has different casing.♻️ Proposed change
- const developmentEnvironment = oppositeEnvironment?.name === 'Development' ? oppositeEnvironment : null; + const developmentEnvironment = + oppositeEnvironment?.type === EnvironmentTypeEnum.DEV ? oppositeEnvironment : null;Add the import:
+import { EnvironmentTypeEnum } from '@novu/shared';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx` at line 48, Replace the fragile name check in the assignment to developmentEnvironment by using the environment type enum: instead of comparing oppositeEnvironment?.name === 'Development', check oppositeEnvironment?.type === EnvironmentTypeEnum.DEV; add the missing import for EnvironmentTypeEnum if not present and update the variable assignment inside read-only-step-editor.tsx (symbol: developmentEnvironment, oppositeEnvironment) accordingly so the code follows the codebase pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/layouts/layout-read-only-editor-frame.tsx`:
- Line 18: Replace the brittle name string match for development detection with
a type-safe enum check: change the logic that sets developmentEnvironment
(currently using oppositeEnvironment?.name === 'Development') to test
oppositeEnvironment?.type === EnvironmentTypeEnum.DEV, ensuring
EnvironmentTypeEnum is imported/available in this module and preserving the same
behavior (assign oppositeEnvironment or null). Update the const
developmentEnvironment assignment and import references so the component follows
the same enum-based branching as layout-editor.tsx and other editors.
- Around line 26-31: The route call to ROUTES.LAYOUTS_EDIT is passing the wrong
layout identifier; replace the use of layout.layoutId with layout.slug when
building the route (the callsite is the navigate(buildRoute(ROUTES.LAYOUTS_EDIT,
{...})) invocation that currently sets layoutSlug: layout.layoutId) so that
buildRoute receives layoutSlug: layout.slug (keep developmentEnvironment.slug
unchanged).
In
`@apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx`:
- Around line 56-61: The navigation call uses the wrong workflow identifier:
replace the route param workflow.workflowId with workflow.slug when calling
navigate(buildRoute(ROUTES.EDIT_WORKFLOW, { environmentSlug:
developmentEnvironment.slug, workflowSlug: ... })); update the workflowSlug
value to workflow.slug so it matches other uses of ROUTES.EDIT_WORKFLOW and the
route pattern.
---
Nitpick comments:
In `@apps/dashboard/src/components/workflow-editor/steps/email/email-editor.tsx`:
- Around line 49-57: The outer div with className="flex h-full flex-col" is
duplicated across the DEV/non-DEV branches; collapse it by rendering that div
once and moving the currentEnvironment?.type === EnvironmentTypeEnum.DEV ? ... :
... ternary inside it so it returns either emailFields or
<WorkflowReadOnlyStepEditor>{emailFields}</WorkflowReadOnlyStepEditor>; update
the JSX in email-editor.tsx to remove the branch that wraps the whole div and
ensure WorkflowReadOnlyStepEditor still receives emailFields only for non-DEV
environments.
In `@apps/dashboard/src/components/workflow-editor/steps/push/push-editor.tsx`:
- Around line 14-31: The push editor repeats the DEV check/readonly wrapping
pattern for pushFields; extract that logic into a small helper or wrapper (e.g.
useEditorReadOnlyWrap or EditorReadOnlyWrapper) and replace the pattern in
push-editor: move the currentEnvironment/EnvironmentTypeEnum.DEV check inside
the helper so callers just return useEditorReadOnlyWrap(pushFields) or
<EditorReadOnlyWrapper>{pushFields}</EditorReadOnlyWrapper>; ensure the helper
uses the same WorkflowReadOnlyStepEditor when currentEnvironment?.type !==
EnvironmentTypeEnum.DEV and otherwise returns children unmodified so you can
reuse it across push/sms/chat/in-app/http-request/email editors referencing
pushFields and WorkflowReadOnlyStepEditor.
In
`@apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx`:
- Line 48: Replace the fragile name check in the assignment to
developmentEnvironment by using the environment type enum: instead of comparing
oppositeEnvironment?.name === 'Development', check oppositeEnvironment?.type ===
EnvironmentTypeEnum.DEV; add the missing import for EnvironmentTypeEnum if not
present and update the variable assignment inside read-only-step-editor.tsx
(symbol: developmentEnvironment, oppositeEnvironment) accordingly so the code
follows the codebase pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8cc4bfe-8053-44cd-a7d5-9ec7ce9852d8
📒 Files selected for processing (10)
apps/dashboard/src/components/layouts/layout-editor.tsxapps/dashboard/src/components/layouts/layout-read-only-editor-frame.tsxapps/dashboard/src/components/workflow-editor/steps/chat/chat-editor.tsxapps/dashboard/src/components/workflow-editor/steps/email/email-editor.tsxapps/dashboard/src/components/workflow-editor/steps/http-request/http-request-editor.tsxapps/dashboard/src/components/workflow-editor/steps/in-app/in-app-editor.tsxapps/dashboard/src/components/workflow-editor/steps/push/push-editor.tsxapps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsxapps/dashboard/src/components/workflow-editor/steps/sms/sms-editor.tsxapps/dashboard/src/components/workflow-editor/steps/step-editor-unavailable.tsx
💤 Files with no reviewable changes (1)
- apps/dashboard/src/components/workflow-editor/steps/step-editor-unavailable.tsx
| const navigate = useNavigate(); | ||
| const { switchEnvironment, oppositeEnvironment } = useEnvironment(); | ||
| const { layout } = useLayoutEditor(); | ||
| const developmentEnvironment = oppositeEnvironment?.name === 'Development' ? oppositeEnvironment : null; |
There was a problem hiding this comment.
Prefer EnvironmentTypeEnum.DEV over name string match.
Matching oppositeEnvironment?.name === 'Development' is brittle — any rename, localization, or casing drift silently hides the "Switch to Development" CTA. The rest of this PR (e.g. layout-editor.tsx, every step editor) branches on currentEnvironment?.type === EnvironmentTypeEnum.DEV; doing the same here keeps semantics consistent and type-safe.
Proposed change
-import { useEnvironment } from '@/context/environment/hooks';
+import { EnvironmentTypeEnum } from '@novu/shared';
+import { useEnvironment } from '@/context/environment/hooks';
@@
- const developmentEnvironment = oppositeEnvironment?.name === 'Development' ? oppositeEnvironment : null;
+ const developmentEnvironment =
+ oppositeEnvironment?.type === EnvironmentTypeEnum.DEV ? oppositeEnvironment : null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const developmentEnvironment = oppositeEnvironment?.name === 'Development' ? oppositeEnvironment : null; | |
| import { EnvironmentTypeEnum } from '@novu/shared'; | |
| import { useEnvironment } from '@/context/environment/hooks'; | |
| // ... other code ... | |
| const developmentEnvironment = | |
| oppositeEnvironment?.type === EnvironmentTypeEnum.DEV ? oppositeEnvironment : null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/layouts/layout-read-only-editor-frame.tsx` at
line 18, Replace the brittle name string match for development detection with a
type-safe enum check: change the logic that sets developmentEnvironment
(currently using oppositeEnvironment?.name === 'Development') to test
oppositeEnvironment?.type === EnvironmentTypeEnum.DEV, ensuring
EnvironmentTypeEnum is imported/available in this module and preserving the same
behavior (assign oppositeEnvironment or null). Update the const
developmentEnvironment assignment and import references so the component follows
the same enum-based branching as layout-editor.tsx and other editors.
| navigate( | ||
| buildRoute(ROUTES.LAYOUTS_EDIT, { | ||
| environmentSlug: developmentEnvironment.slug, | ||
| layoutSlug: layout.layoutId, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the route definition, slug shape on the layout type, and existing call sites building LAYOUTS_EDIT.
rg -nP -C2 '\bLAYOUTS_EDIT\b'
rg -nP -C3 '\blayoutSlug\b'
ast-grep --pattern 'type Layout = { $$$ }'
ast-grep --pattern 'interface Layout { $$$ }'Repository: novuhq/novu
Length of output: 27736
Use layout.slug instead of layout.layoutId for the route parameter.
The route LAYOUTS_EDIT expects a layoutSlug parameter (as defined in routes.ts), but the code passes layout.layoutId. All other call sites in the codebase use layout.slug:
layout-row.tsxline 129:layoutSlug: layout.sluglayouts-list-upgrade-cta.tsxline 105:layoutSlug: layout.sluglayout-editor-settings-drawer.tsxline 107:layoutSlug: layout?.slug
Change line 29 from layoutSlug: layout.layoutId, to layoutSlug: layout.slug,.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/layouts/layout-read-only-editor-frame.tsx`
around lines 26 - 31, The route call to ROUTES.LAYOUTS_EDIT is passing the wrong
layout identifier; replace the use of layout.layoutId with layout.slug when
building the route (the callsite is the navigate(buildRoute(ROUTES.LAYOUTS_EDIT,
{...})) invocation that currently sets layoutSlug: layout.layoutId) so that
buildRoute receives layoutSlug: layout.slug (keep developmentEnvironment.slug
unchanged).
| navigate( | ||
| buildRoute(ROUTES.EDIT_WORKFLOW, { | ||
| environmentSlug: developmentEnvironment.slug, | ||
| workflowSlug: workflow.workflowId, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how EDIT_WORKFLOW is navigated to elsewhere and what identifier is passed.
rg -nP --type=ts --type=tsx -C3 'ROUTES\.EDIT_WORKFLOW|EDIT_WORKFLOW' apps/dashboard/src
rg -nP --type=ts --type=tsx -C2 'workflowSlug\s*:' apps/dashboard/src | head -n 80Repository: novuhq/novu
Length of output: 110
🏁 Script executed:
# Search for ROUTES definition and EDIT_WORKFLOW route
fd -t f "route" apps/dashboard/src | head -20
rg -n "EDIT_WORKFLOW" apps/dashboard/src --type ts 2>/dev/null || rg -n "EDIT_WORKFLOW" apps/dashboard/srcRepository: novuhq/novu
Length of output: 4674
🏁 Script executed:
# Search for the routes constant definition
rg -n "ROUTES\s*=" apps/dashboard/src -A 20 | head -100Repository: novuhq/novu
Length of output: 2961
🏁 Script executed:
# Find and examine the specific file mentioned in the review
cat -n apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx | sed -n '50,70p'Repository: novuhq/novu
Length of output: 777
🏁 Script executed:
# Look for how EDIT_WORKFLOW is used elsewhere - find all usages with context
rg -n "EDIT_WORKFLOW" apps/dashboard/src -B2 -A2Repository: novuhq/novu
Length of output: 18925
Use workflow.slug instead of workflow.workflowId for the workflowSlug route parameter.
The route expects a workflowSlug parameter, but you're passing workflow.workflowId. Throughout the codebase, every other navigation to ROUTES.EDIT_WORKFLOW uses workflow.slug. This inconsistency will likely cause routing failures since workflowId is a database identifier, not the slug used by the route pattern /env/:environmentSlug/workflows/:workflowSlug.
Diff
navigate(
buildRoute(ROUTES.EDIT_WORKFLOW, {
environmentSlug: developmentEnvironment.slug,
- workflowSlug: workflow.workflowId,
+ workflowSlug: workflow.slug,
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| navigate( | |
| buildRoute(ROUTES.EDIT_WORKFLOW, { | |
| environmentSlug: developmentEnvironment.slug, | |
| workflowSlug: workflow.workflowId, | |
| }) | |
| ); | |
| navigate( | |
| buildRoute(ROUTES.EDIT_WORKFLOW, { | |
| environmentSlug: developmentEnvironment.slug, | |
| workflowSlug: workflow.slug, | |
| }) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx`
around lines 56 - 61, The navigation call uses the wrong workflow identifier:
replace the route param workflow.workflowId with workflow.slug when calling
navigate(buildRoute(ROUTES.EDIT_WORKFLOW, { environmentSlug:
developmentEnvironment.slug, workflowSlug: ... })); update the workflowSlug
value to workflow.slug so it matches other uses of ROUTES.EDIT_WORKFLOW and the
route pattern.
…anner fixes NV-7362 Co-authored-by: Dima Grossman <dima@grossman.io>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx (1)
59-64:⚠️ Potential issue | 🟠 MajorUse
workflow.slugfor theworkflowSlugroute param.The route
ROUTES.EDIT_WORKFLOWpattern uses:workflowSlug, and every other navigation site in the codebase passesworkflow.slug. Passingworkflow.workflowIdhere will produce a URL that doesn't match the slug-based loader and will break navigation into the development environment.Proposed fix
navigate( buildRoute(ROUTES.EDIT_WORKFLOW, { environmentSlug: developmentEnvironment.slug, - workflowSlug: workflow.workflowId, + workflowSlug: workflow.slug, }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx` around lines 59 - 64, The route navigation is using the wrong workflow identifier: change the second param passed to buildRoute for ROUTES.EDIT_WORKFLOW from workflow.workflowId to workflow.slug so the route uses the slug-based loader; update the call inside the navigate(... buildRoute(...)) block (references: navigate, buildRoute, ROUTES.EDIT_WORKFLOW, developmentEnvironment.slug, workflow.workflowId -> replace with workflow.slug).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx`:
- Around line 59-64: The route navigation is using the wrong workflow
identifier: change the second param passed to buildRoute for
ROUTES.EDIT_WORKFLOW from workflow.workflowId to workflow.slug so the route uses
the slug-based loader; update the call inside the navigate(... buildRoute(...))
block (references: navigate, buildRoute, ROUTES.EDIT_WORKFLOW,
developmentEnvironment.slug, workflow.workflowId -> replace with workflow.slug).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 908c3493-de6b-4e69-b5c0-1c509d8a12f2
📒 Files selected for processing (1)
apps/dashboard/src/components/workflow-editor/steps/read-only-step-editor.tsx
2 participants
Summary
In staging/production (non-development environments), workflow step editors and the layout editor no longer replace the editor with a full-screen lock. Users see the same control values as in dev, with a compact InlineToast banner (same minimal pattern as the workflow canvas read-only notice) explaining that editing requires a development environment, plus an optional Switch to Development CTA when that environment exists.
The editor content is wrapped in a container with
pointer-events-noneandselect-textso values remain visible and copyable without interaction.Changes
ReadOnlyStepEditorFrameandWorkflowReadOnlyStepEditorfor workflow steps;LayoutReadOnlyEditorFramefor the layout editor (avoids circular imports withlayout-editor-provider).InlineToast(variant="info") in one line with vertical padding (py-2.5on the wrapper).StepEditorUnavailable.step-editor-unavailable.tsx.Testing
pnpm exec tsc -binapps/dashboardbiome checkon touched filesLinear Issue: NV-7362
What changed
In non-development environments (staging/production) workflow step editors and the layout editor no longer show a full-screen "unavailable" lock; they render the actual editor controls in a read-only frame with a compact InlineToast banner explaining edits require a development environment and an optional "Switch to Development" CTA. The read-only container is inert (non-interactive) but preserves text selection/copy so users can view and copy control values. This fixes inconsistent clickable inputs and provides a minimal, neutral notice instead of a disruptive lock UI.
Affected areas
@novu/dashboard: Replaced the full-screen StepEditorUnavailable pattern with read-only wrappers across the workflow and layout editors — added ReadOnlyStepEditorFrame and WorkflowReadOnlyStepEditor for steps and LayoutReadOnlyEditorFrame for the layout editor; updated email, SMS, push, chat, in-app, and HTTP request editors to render the read-only wrapped content; removed the obsolete StepEditorUnavailable component. Banner styling was made neutral and inert; read-only frames optionally include environment-switching logic when a Development environment exists.
Key technical decisions
Testing
Type-check and lint checks were applied: run pnpm exec tsc -b in apps/dashboard and biome check on touched files; changes verified manually to ensure controls are non-interactive yet selectable/copyable. No new unit or e2e tests were added.