update/show-hide-code-for-pipeline#330
Conversation
📝 WalkthroughWalkthroughThis PR adds inline YAML editing for pipelines and unsaved-change handling when switching between visual and code views. PipelineGraph emits editing state and serialized content, PipelineView forwards the updated callbacks, and ProjectDetails manages draft tabs, discard confirmation, and log minimization. ChangesPipeline YAML edit mode
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/screens/projectDetails/index.tsx`:
- Around line 234-237: The effect in projectDetails/index.tsx is clearing
pipeline edit state too aggressively by resetting pipelineDraftTab whenever
activePipelineFilePath changes, which causes unsaved YAML edits to be lost on
tab switches. Update this logic so tab/file changes are gated by the same
unsaved-change confirmation flow used for the Visual editor, or store the draft
by pipeline path before switching; locate the reset in the React.useEffect tied
to activePipelineFilePath and adjust setPipelineCodeMode/setPipelineDraftTab
behavior accordingly.
- Around line 239-253: Guard entering pipeline code mode until the pipeline
content has finished loading, since handleEnterPipelineCodeMode currently falls
back to an empty string when activePipelineContent is undefined and can create
an editable draft for the real pipeline file too early. Update
handleEnterPipelineCodeMode in projectDetails to only build the writable draft
once content is available, and otherwise keep the view in a loading/disabled
state. Also make PipelineView withhold or disable onEdit while
activePipelineContent is undefined so the YAML editor cannot open empty or allow
an accidental empty save.
- Around line 1255-1263: The visual graph save path in the `onSave` handler only
calls `refetchPipelineContent()` after `projectsServices.saveFileContent`, so
the Git/sidebar status stays stale. Update this save flow to also invoke
`updateStatuses()` after a successful save, matching the inline editor behavior
and ensuring pipeline YAML changes are reflected immediately.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2adf882e-51f0-4f76-824b-69d1cbb25e28
📒 Files selected for processing (3)
src/renderer/components/pipelineView/PipelineGraph.tsxsrc/renderer/components/pipelineView/PipelineView.tsxsrc/renderer/screens/projectDetails/index.tsx
confirmation bugfix: pipeline visual editor opening empty draft before file content loads bugfix: git status not refreshing after saving pipeline from visual editor update: minimizing cloud logs when pipeline enters YAML or visual edit mode
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/renderer/components/pipelineView/PipelineGraph.tsx (2)
303-307: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the unsaved-changes dialog open when save fails.
handleUnsavedSavecallssetShowUnsavedDialog(false)unconditionally afterawait handleSave(), even whenhandleSavereturnsnull(validation or save error). The dialog closes silently and the user is left in edit mode with only the headervalidationErroras feedback — easy to miss after the dialog disappears. Move the close call inside the success branch.💚 Proposed fix
const handleUnsavedSave = useCallback(async () => { const content = await handleSave(); - setShowUnsavedDialog(false); - if (content !== null) onEdit?.(content); + if (content !== null) { + setShowUnsavedDialog(false); + onEdit?.(content); + } }, [handleSave, onEdit]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/pipelineView/PipelineGraph.tsx` around lines 303 - 307, In handleUnsavedSave, only call setShowUnsavedDialog(false) when handleSave() returns non-null content; keep the dialog open when saving or validation fails, while still invoking onEdit?.(content) on success.
303-313: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDisable the unsaved-changes dialog while save is in flight
src/renderer/components/editor/unsavedChangesDialog/index.tsxstill leaves Don't Save and the close icon active duringawait handleSave(), so a discard can race with the lateronEdit(content)call. Pass anisSavingflag through and disable the dialog actions until the save finishes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/components/pipelineView/PipelineGraph.tsx` around lines 303 - 313, Track the save-in-progress state in PipelineGraph’s handleUnsavedSave flow and pass it as an isSaving prop to UnsavedChangesDialog. Update the dialog component to disable the “Don’t Save” action and close icon while isSaving is true, and clear the state after handleSave completes so actions are restored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/components/pipelineView/PipelineGraph.tsx`:
- Around line 213-216: Update the useEffect in PipelineGraphContent to return a
cleanup function that calls onEditingChange?.(false) on unmount, while retaining
the existing isEditing synchronization.
---
Outside diff comments:
In `@src/renderer/components/pipelineView/PipelineGraph.tsx`:
- Around line 303-307: In handleUnsavedSave, only call
setShowUnsavedDialog(false) when handleSave() returns non-null content; keep the
dialog open when saving or validation fails, while still invoking
onEdit?.(content) on success.
- Around line 303-313: Track the save-in-progress state in PipelineGraph’s
handleUnsavedSave flow and pass it as an isSaving prop to UnsavedChangesDialog.
Update the dialog component to disable the “Don’t Save” action and close icon
while isSaving is true, and clear the state after handleSave completes so
actions are restored.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 875fa913-382d-4857-bcd6-eddff33bc98c
📒 Files selected for processing (3)
src/renderer/components/pipelineView/PipelineGraph.tsxsrc/renderer/components/pipelineView/PipelineView.tsxsrc/renderer/screens/projectDetails/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/screens/projectDetails/index.tsx
| useEffect(() => { | ||
| onEditingChange?.(isEditing); | ||
| }, [isEditing, onEditingChange]); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether PipelineView/PipelineGraph unmounts when switching away from a pipeline tab.
# Look for the conditional rendering of PipelineView in ProjectDetails.
# Map the ProjectDetails file to find PipelineView rendering context.
ast-grep outline src/renderer/screens/projectDetails/index.tsx --match 'PipelineView' --view expanded
# Search for conditional rendering around PipelineView usage.
rg -n -C10 'PipelineView' src/renderer/screens/projectDetails/index.tsxRepository: rosettadb/dbt-studio
Length of output: 2035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the pipeline graph edit-state effect and the parent state that consumes it.
grep -n -C 20 'onEditingChange' src/renderer/components/pipelineView/PipelineGraph.tsx
grep -n -C 20 'pipelineVisualEditing\|handleEnterPipelineCodeMode\|handleExitPipelineCodeMode' src/renderer/screens/projectDetails/index.tsx
# Inspect the component boundary to see whether PipelineGraphContent is unmounted when leaving pipeline mode.
grep -n -C 20 'PipelineGraphContent\|PipelineView\|isEditing' src/renderer/components/pipelineView/*.tsxRepository: rosettadb/dbt-studio
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every place pipelineVisualEditing is updated.
rg -n -C 3 'pipelineVisualEditing|setPipelineVisualEditing' src/renderer/screens/projectDetails/index.tsx
# Show the pipeline-view render branch around the conditional that mounts/unmounts PipelineView.
sed -n '1240,1310p' src/renderer/screens/projectDetails/index.tsx
# Show the PipelineView implementation to confirm whether it forwards onEditingChange only from the graph component.
sed -n '1,220p' src/renderer/components/pipelineView/PipelineView.tsxRepository: rosettadb/dbt-studio
Length of output: 10474
Reset editing state on unmount Add a cleanup to this effect so onEditingChange(false) runs when PipelineGraphContent unmounts; otherwise pipelineVisualEditing can stay true after leaving the pipeline view and keep cloud logs minimized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/pipelineView/PipelineGraph.tsx` around lines 213 -
216, Update the useEffect in PipelineGraphContent to return a cleanup function
that calls onEditingChange?.(false) on unmount, while retaining the existing
isEditing synchronization.
PIpeline: show/hide code button
Summary by CodeRabbit
New Features
Bug Fixes
UI Improvements