Feature/direct navigate to bubble with missing creds#248
Conversation
…g them in a ref and accessing callbacks through optionsRef.current instead of relying on closures. It also removes options from dependency arrays to prevent unnecessary re-creations and cleans up all debug console.log statements.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughTracks a target bubble to focus during flow execution: Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant FlowIDEView
participant useRunExecution
participant flowValidation
participant FlowVisualizer
participant ReactFlow
User->>FlowIDEView: Click "Run Flow"
FlowIDEView->>useRunExecution: runFlow(..., onFocusBubble)
useRunExecution->>flowValidation: validateCredentials(...)
alt missing credentials (returns bubbleVariableId)
flowValidation-->>useRunExecution: { bubbleVariableId }
useRunExecution->>FlowIDEView: onFocusBubble(bubbleVariableId)
FlowIDEView->>FlowIDEView: set bubbleToFocus=bubbleVariableId
FlowIDEView->>FlowVisualizer: prop bubbleToFocus
FlowVisualizer->>ReactFlow: getNodes() / resolve target node
FlowVisualizer->>ReactFlow: setCenter(...) / pan to computed position
FlowVisualizer->>FlowVisualizer: highlight node (auto-clear ~3s)
FlowVisualizer-->>FlowIDEView: onFocusComplete()
else credentials valid
useRunExecution->>FlowIDEView: continue execution
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
Suggested PR title from PearlTitle: Body: SummaryAutomatically navigates to and highlights bubbles that have missing credentials when flow execution fails validation. This improves user experience by directing users to the exact location where action is needed. ChangesCore Functionality
Flow Visualizer
Technical Improvements
Other
Benefits
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsx (1)
114-118: Consider simplifying the conditional options object.The current pattern works, but could be simplified since
onFocusBubblebeingundefinedis already handled by the hook.🔎 Optional simplification
- // Get runFlow function with callback - const { runFlow } = useRunExecution( - flowId, - onFocusBubble ? { onFocusBubble } : {} - ); + // Get runFlow function with callback + const { runFlow } = useRunExecution(flowId, { onFocusBubble });Passing
undefinedforonFocusBubbleis safe since the hook uses optional chaining (optionsRef.current.onFocusBubble?.()).apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsx (1)
64-68: Same simplification opportunity as CronScheduleNode.For consistency, consider the same simplification mentioned in
CronScheduleNode.tsx.🔎 Optional simplification
- // Get runFlow function with callback - const { runFlow } = useRunExecution( - flowId, - onFocusBubble ? { onFocusBubble } : {} - ); + // Get runFlow function with callback + const { runFlow } = useRunExecution(flowId, { onFocusBubble });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/bubble-studio/src/components/FlowIDEView.tsxapps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsxapps/bubble-studio/src/hooks/useRunExecution.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to apps/bubble-studio/src/components/flow_visualizer/README.md : Refer to apps/bubble-studio/src/components/flow_visualizer/README.md for documentation on flow visualizer architecture and node rendering
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to apps/bubble-studio/src/components/flow_visualizer/README.md : Refer to apps/bubble-studio/src/components/flow_visualizer/README.md for documentation on flow visualizer architecture and node rendering
Applied to files:
apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsxapps/bubble-studio/src/components/FlowIDEView.tsxapps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsxapps/bubble-studio/src/components/FlowIDEView.tsxapps/bubble-studio/src/hooks/useRunExecution.tsapps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/packages/bubble-shared-schemas/**/*.{ts,tsx} : Write shared schemas between frontend and backend in `/packages/bubble-shared-schemas` directory
Applied to files:
apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsx
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to apps/bubble-studio/src/components/MonacoEditor.tsx : The Monaco Editor integration in `apps/bubble-studio/src/components/MonacoEditor.tsx` must fetch the bundled types from `/bubble-types.txt`, wrap them in a module declaration for `bubblelab/bubble-core`, and add them to Monaco's TypeScript type system
Applied to files:
apps/bubble-studio/src/components/FlowIDEView.tsx
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/src/index.ts : All new types and classes added to bubble-core must be exported from `packages/bubble-core/src/index.ts` to ensure they are included in the generated bundle
Applied to files:
apps/bubble-studio/src/components/FlowIDEView.tsx
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-runtime/src/injection/README.md : Refer to packages/bubble-runtime/src/injection/README.md for information about credential injection and bubble parameter reinitialization
Applied to files:
apps/bubble-studio/src/hooks/useRunExecution.ts
🧬 Code graph analysis (4)
apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsx (1)
apps/bubble-studio/src/hooks/useRunExecution.ts (1)
useRunExecution(61-634)
apps/bubble-studio/src/components/FlowIDEView.tsx (2)
apps/bubble-studio/src/hooks/useBubbleFlow.ts (1)
useBubbleFlow(32-327)apps/bubble-studio/src/hooks/useRunExecution.ts (1)
useRunExecution(61-634)
apps/bubble-studio/src/hooks/useRunExecution.ts (1)
apps/bubble-studio/src/stores/executionStore.ts (1)
getExecutionStore(728-731)
apps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx (2)
apps/bubble-studio/src/components/flow_visualizer/flowLayoutConstants.ts (1)
FLOW_LAYOUT(7-68)apps/bubble-studio/src/stores/executionStore.ts (1)
getExecutionStore(728-731)
🔇 Additional comments (13)
apps/bubble-studio/src/components/FlowIDEView.tsx (2)
67-76: LGTM! Clean state management for bubble focus navigation.The
bubbleToFocusstate andonFocusBubblecallback wiring correctly lifts the focus state to the parent component, enabling coordination between the execution hook and the visualizer.
666-670: Proper prop threading to FlowVisualizer.The focus-related props are correctly passed down, and
onFocusCompleteappropriately clears the state to prevent stale focus behavior.apps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsx (1)
38-38: LGTM! Interface extension for focus callback.The optional
onFocusBubblecallback is correctly added to the data interface.apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsx (1)
43-43: LGTM! Consistent interface extension.The optional
onFocusBubblecallback matches the pattern inCronScheduleNode.apps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx (4)
49-51: LGTM! Props interface correctly extended.The new props for bubble focus navigation are properly typed and optional.
97-104: LGTM! Component signature updated with new props and getNodes access.The
getNodesfunction fromuseReactFlowis correctly retrieved for fallback node lookup.
953-954: LGTM! onFocusBubble correctly wired to entry nodes.Both
CronScheduleNodeandInputSchemaNodereceive theonFocusBubblecallback through their data props.Also applies to: 974-974
2138-2210: Timing concern aboutonFocusCompleteis not a practical risk.The
setCenteranimation has a duration of 300ms, butonFocusCompleteis called immediately after. While this timing exists, rapid successive focus requests cannot occur because the execution flow is serialized: the UI button is disabled during execution (isRunningflag), and credential validation (which triggersonFocusBubble) happens only once perrunFlowinvocation. Even if bubbleToFocus were to change during the 300ms animation, the effect's cleanup properly handles it by clearing the previous highlight timer and restarting with the new target.The fallback node lookup and absolute position calculation for nested nodes are well-implemented.
apps/bubble-studio/src/hooks/useRunExecution.ts (5)
1-1: LGTM! Added useRef for callback stability.The
useRefimport is needed for theoptionsRefpattern.
49-49: LGTM! Interface extended with onFocusBubble callback.This enables callers to receive navigation events when credentials are missing.
65-67: Good pattern: Using ref for latest callback references.The
optionsRefpattern ensures that async callbacks (streaming events, error handlers) always invoke the latest callback versions, avoiding stale closure issues. This is particularly important sinceexecuteWithStreamingis a long-running async operation where the component's props may have changed by the time events arrive.
444-455: LGTM! Focus navigation on credential validation failure.The implementation correctly:
- Checks for
bubbleVariableIdfrom validation- Uses
optionsRef.currentto get the latest callback- Calls
onFocusBubbleto trigger navigation
546-579: LGTM! validateCredentials now returns the offending bubble ID.The function correctly:
- Initializes
bubbleVariableIdas null- Captures the first bubble with missing credentials
- Returns the ID along with validation results
This enables the caller to navigate users directly to the problematic bubble.
PR.demo.mp4Demo for my changes |
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 (1)
apps/bubble-studio/src/hooks/useRunExecution.ts (1)
464-464: Fix the function signature mismatch causing pipeline failure.The local
validateInputsfunction expects 1 argument (currentFlow), but you're calling it with 2 arguments (flowId, flowToValidate). This matches the pipeline error:TS2554: Expected 1 arguments, but got 2.🔎 Proposed fix
Remove the
flowIdargument:- const inputValidation = validateInputs(flowId, flowToValidate); + const inputValidation = validateInputs(flowToValidate);Better yet, see the next comment about removing these duplicate functions entirely.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/bubble-studio/src/components/FlowIDEView.tsxapps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsxapps/bubble-studio/src/hooks/useRunExecution.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to apps/bubble-studio/src/components/flow_visualizer/README.md : Refer to apps/bubble-studio/src/components/flow_visualizer/README.md for documentation on flow visualizer architecture and node rendering
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to apps/bubble-studio/src/components/flow_visualizer/README.md : Refer to apps/bubble-studio/src/components/flow_visualizer/README.md for documentation on flow visualizer architecture and node rendering
Applied to files:
apps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsxapps/bubble-studio/src/components/FlowIDEView.tsxapps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsxapps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
apps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsxapps/bubble-studio/src/components/FlowIDEView.tsxapps/bubble-studio/src/hooks/useRunExecution.tsapps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsxapps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to apps/bubble-studio/src/components/MonacoEditor.tsx : The Monaco Editor integration in `apps/bubble-studio/src/components/MonacoEditor.tsx` must fetch the bundled types from `/bubble-types.txt`, wrap them in a module declaration for `bubblelab/bubble-core`, and add them to Monaco's TypeScript type system
Applied to files:
apps/bubble-studio/src/components/FlowIDEView.tsx
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-runtime/src/injection/README.md : Refer to packages/bubble-runtime/src/injection/README.md for information about credential injection and bubble parameter reinitialization
Applied to files:
apps/bubble-studio/src/hooks/useRunExecution.ts
📚 Learning: 2025-12-19T03:17:06.825Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.825Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
apps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx
🧬 Code graph analysis (2)
apps/bubble-studio/src/components/FlowIDEView.tsx (1)
apps/bubble-studio/src/hooks/useRunExecution.ts (1)
useRunExecution(64-662)
apps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx (2)
apps/bubble-studio/src/components/flow_visualizer/flowLayoutConstants.ts (1)
FLOW_LAYOUT(7-68)apps/bubble-studio/src/stores/executionStore.ts (1)
getExecutionStore(728-731)
🪛 GitHub Actions: CI
apps/bubble-studio/src/hooks/useRunExecution.ts
[error] 464-464: tsc --noEmit failed with TS2554: Expected 1 arguments, but got 2 in src/hooks/useRunExecution.ts(464,56).
🔇 Additional comments (8)
apps/bubble-studio/src/hooks/useRunExecution.ts (2)
1-1: LGTM! Ref pattern is correct.The
useRefpattern to capture the latest options without causing re-renders is a solid approach to avoid stale closures in async callbacks and event handlers.Also applies to: 52-52, 68-70
483-493: LGTM! Focus navigation is correctly implemented.The navigation logic properly checks for both the
bubbleVariableIdand the callback, and uses the ref pattern to ensure it captures the latestonFocusBubblecallback.apps/bubble-studio/src/components/FlowIDEView.tsx (1)
67-76: LGTM! Focus state management is clean.The bubble focus state is properly managed:
onFocusBubbleupdates the state when credentials are missing, the state flows down toFlowVisualizer, andonFocusCompleteclears it after navigation completes.Also applies to: 672-676
apps/bubble-studio/src/components/flow_visualizer/nodes/InputSchemaNode.tsx (1)
47-47: LGTM! Callback wiring is correct.The
onFocusBubblecallback is properly threaded through touseRunExecutionwith appropriate conditional handling.Also applies to: 55-55, 68-72
apps/bubble-studio/src/components/flow_visualizer/nodes/CronScheduleNode.tsx (1)
38-38: LGTM! Callback wiring matches InputSchemaNode.The
onFocusBubblecallback is properly threaded through with the same pattern asInputSchemaNode.Also applies to: 91-91, 114-118
apps/bubble-studio/src/components/flow_visualizer/FlowVisualizer.tsx (3)
53-55: LGTM! Props interface extended correctly.The new focus-related props are properly added to the interface and signature, and
getNodesis appropriately retrieved fromuseReactFlowfor the fallback search.Also applies to: 101-108
974-974: LGTM! Focus callback propagated to entry nodes.The
onFocusBubblecallback is correctly passed to bothCronScheduleNodeandInputSchemaNodedata, enabling focus navigation from scheduled and manual executions.Also applies to: 995-995
2334-2406: LGTM! Focus navigation implementation is solid.The effect correctly:
- Finds the target node with a fallback search by
variableIdorbubbleKey- Calculates absolute positions for bubbles inside parent containers
- Centers and zooms the viewport smoothly
- Highlights the bubble for 3 seconds
- Cleans up the highlight timer on unmount or re-trigger
The dependency array is appropriate (excluding
onFocusBubblewhich would cause unnecessary re-runs).
|
Hi @zhubzy could you please take a look into this PR? My test cases works fine in local but when running github action CI it fails. |
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 (1)
apps/bubble-studio/src/utils/flowValidation.ts (1)
123-171: Critical: validateFlow must propagate bubbleVariableId from credential validation.The
validateFlowfunction returnsValidationResult, which now includesbubbleVariableId, but line 170 returns onlyisValidandreasons. This means callers ofvalidateFlowwon't receive the navigation target even thoughvalidateCredentialscomputed it. While the current execution path may callvalidateCredentialsdirectly (per the PR summary), this inconsistency breaks the interface contract and will cause silent bugs ifvalidateFlowis used elsewhere.🔎 Proposed fix to propagate bubbleVariableId
export function validateFlow( flowId: number | null, currentFlow: BubbleFlowDetailsResponse | undefined, options?: { executionInputs?: Record<string, unknown>; pendingCredentials?: Record<string, Record<string, number>>; checkRunning?: boolean; checkValidating?: boolean; } ): ValidationResult { if (!flowId) { return { isValid: false, reasons: ['No flow selected'] }; } const reasons: string[] = []; + let bubbleVariableId: string | undefined = undefined; if (!currentFlow) { reasons.push('No flow selected'); return { isValid: false, reasons }; } if (options?.checkRunning && getExecutionStore(flowId).isRunning) { reasons.push('Already executing'); } if (options?.checkValidating && getExecutionStore(flowId).isValidating) { reasons.push('Currently validating'); } const inputValidation = validateInputs( flowId, currentFlow, options?.executionInputs ); if (!inputValidation.isValid) { reasons.push(...inputValidation.reasons); } const credentialValidation = validateCredentials( flowId, currentFlow, options?.pendingCredentials ); if (!credentialValidation.isValid) { reasons.push(...credentialValidation.reasons); + // Capture bubbleVariableId from credential validation for navigation + if (credentialValidation.bubbleVariableId) { + bubbleVariableId = credentialValidation.bubbleVariableId; + } } - return { isValid: reasons.length === 0, reasons }; + return { isValid: reasons.length === 0, reasons, bubbleVariableId }; }
🧹 Nitpick comments (1)
apps/bubble-studio/src/utils/flowValidation.ts (1)
54-58: Clarify JSDoc: specify when bubbleVariableId is returned.The updated JSDoc explains the new behavior well, but could be more explicit that
bubbleVariableIdis only populated when validation fails and captures the first bubble with missing credentials for navigation purposes.🔎 Suggested enhancement
/** * Validates that all required credentials are configured. * Iterates over bubbleParameters to find clones (bubbles with invocationCallSiteKey) - * and validates their credentials. Returns the clone's variableId for navigation. + * and validates their credentials. When validation fails, returns the first bubble's + * variableId (as a string) to enable navigation to the problematic bubble. */
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/bubble-studio/src/utils/flowValidation.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-22T09:55:47.873Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.873Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
apps/bubble-studio/src/utils/flowValidation.ts
🧬 Code graph analysis (1)
apps/bubble-studio/src/utils/flowValidation.ts (3)
packages/bubble-shared-schemas/src/bubbleflow-schema.ts (1)
BubbleFlowDetailsResponse(395-397)apps/bubble-studio/src/stores/executionStore.ts (1)
getExecutionStore(728-731)packages/bubble-shared-schemas/src/credential-schema.ts (1)
SYSTEM_CREDENTIALS(37-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
apps/bubble-studio/src/utils/flowValidation.ts (3)
6-10: LGTM: Clean interface extension.The optional
bubbleVariableIdfield appropriately extendsValidationResultto support navigation to problematic bubbles.
117-117: LGTM: Return statement correctly includes bubbleVariableId.The return statement properly propagates the navigation target to callers.
59-118: Approve overall approach with suggested refinements.The refactored
validateCredentialslogic correctly:
- Iterates over
bubbleParametersto access bubble metadata- Skips design-time bubbles that have clones, validating only runtime clones
- Enriches error messages with
invocationCallSiteKeycontext for better clone identification- Captures the first problematic bubble's
variableIdfor UI navigationThis aligns well with the PR's objective to navigate to bubbles with missing credentials. Address the flagged issues (type consistency, null checks, and
validateFlowpropagation) to ensure robustness.
| for (const bubble of Object.values(bubbleParameters)) { | ||
| // Skip design-time bubbles that have clones (only validate clones) | ||
| if (!bubble.invocationCallSiteKey && clonedFromSet.has(bubble.variableId)) { | ||
| continue; | ||
| } | ||
|
|
||
| const variableIdKey = String(bubble.variableId); | ||
| // requiredCredentials is keyed by variableId (as string) | ||
| const credTypes = required[variableIdKey] || []; | ||
|
|
||
| for (const credType of credTypes) { | ||
| if (SYSTEM_CREDENTIALS.has(credType as CredentialType)) continue; | ||
|
|
||
| const selectedForBubble = credentials[bubbleKey] || {}; | ||
| // pendingCredentials is keyed by variableId (as string) | ||
| const selectedForBubble = credentials[variableIdKey] || {}; | ||
| const selectedId = selectedForBubble[credType]; | ||
|
|
||
| if (selectedId === undefined || selectedId === null) { | ||
| reasons.push(`Missing credential for ${bubbleKey}: ${credType}`); | ||
| // Include invocationCallSiteKey in message for clones | ||
| const context = bubble.invocationCallSiteKey | ||
| ? ` (${bubble.invocationCallSiteKey})` | ||
| : ''; | ||
| reasons.push(`Missing ${credType} for ${bubble.bubbleName}${context}`); | ||
|
|
||
| // Capture the first clone's variableId for navigation | ||
| if (!bubbleVariableId) { | ||
| bubbleVariableId = String(bubble.variableId); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Add defensive null checks for bubble.variableId and bubble.bubbleName.
Lines 91 and 111 convert bubble.variableId to a string without verifying it exists, and line 107 uses bubble.bubbleName directly. If these properties can be undefined or null (e.g., malformed data, schema changes), the code could produce incorrect keys like "undefined" or throw errors.
🔎 Suggested safeguards
for (const bubble of Object.values(bubbleParameters)) {
+ // Guard against missing identifiers
+ if (!bubble.variableId) {
+ continue;
+ }
+
// Skip design-time bubbles that have clones (only validate clones)
if (!bubble.invocationCallSiteKey && clonedFromSet.has(bubble.variableId)) {
continue;
}
const variableIdKey = String(bubble.variableId);
// requiredCredentials is keyed by variableId (as string)
const credTypes = required[variableIdKey] || [];
for (const credType of credTypes) {
if (SYSTEM_CREDENTIALS.has(credType as CredentialType)) continue;
// pendingCredentials is keyed by variableId (as string)
const selectedForBubble = credentials[variableIdKey] || {};
const selectedId = selectedForBubble[credType];
if (selectedId === undefined || selectedId === null) {
// Include invocationCallSiteKey in message for clones
const context = bubble.invocationCallSiteKey
? ` (${bubble.invocationCallSiteKey})`
: '';
- reasons.push(`Missing ${credType} for ${bubble.bubbleName}${context}`);
+ const bubbleName = bubble.bubbleName || 'Unknown Bubble';
+ reasons.push(`Missing ${credType} for ${bubbleName}${context}`);
// Capture the first clone's variableId for navigation
if (!bubbleVariableId) {
bubbleVariableId = String(bubble.variableId);
}
}
}
}🤖 Prompt for AI Agents
In apps/bubble-studio/src/utils/flowValidation.ts around lines 85 to 115, add
defensive checks so bubble.variableId and bubble.bubbleName are validated before
being used: skip the bubble if variableId is null/undefined (to avoid creating
"undefined" keys) and only call String(bubble.variableId) after confirming it
exists; when building the missing-credential message ensure bubble.bubbleName is
present (use a safe fallback like '<unknown>' or skip reporting that bubble) and
only set bubbleVariableId the first time with a confirmed variableId string.
Ensure the loop continues/returns appropriately when required properties are
missing instead of proceeding with invalid values.
|
@ashutoshbhandari5 I am changing some logic to make it work with the new flow visualizer layout (basically there are design-time bubble and cloned bubble, but your code is only highlghting the design time version and not the cloned version which won't work (if you try it on one of the template) ). In addition, that overall state managment should ideally be inside executionStore to avoid prop drilling or callbacks. but I suppose we can leave this refactor for later (flow visualizer is not the most organized state anyway) |
…g variableId, preventing validation errors from malformed data.
Gotcha, thank you! |
3 participants
Summary
Implements automatic navigation to bubbles with missing credentials when users attempt to run a flow. Previously, users only saw a toast error without knowing which bubble required attention. Now the flow visualizer automatically scrolls and zooms to highlight the problematic bubble, significantly improving the user experience.
Related Issues
Fixes #244
Type of Change
Implementation Details
Changes Made
1. Enhanced Credential Validation (
useRunExecution.ts)validateCredentials()to returnbubbleVariableIdof the bubble with missing credentialsonFocusBubblecallback option to hook interface2. Navigation State Management (
FlowIDEView.tsx)bubbleToFocusstate to track which bubble needs attentiononFocusBubblecallback that triggers when validation fails3. Camera Navigation (
FlowVisualizer.tsx)4. Entry Point Integration
InputSchemaNode.tsxto passonFocusBubblecallbackCronScheduleNode.tsxto passonFocusBubblecallbackTechnical Approach
The implementation follows a clean callback pattern:
bubbleVariableIduseRunExecutionhook callsonFocusBubble(bubbleVariableId)Checklist
pnpm checkand all tests passSummary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.