-
Notifications
You must be signed in to change notification settings - Fork 166
Add visual indicator for sensitive actions #1736
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
Add visual indicator for sensitive actions #1736
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces risk and validation indicators to the step-node component. The previously internal Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx (1)
133-152: Use the extracted variables for consistency.Lines 143-144 re-access
data.step?.settings.blockNameanddata.step?.settings.actionNameinstead of using the already extractedblockNameandactionNamevariables from lines 134-135.const actionMetadata = getActionMetadata( actionsMetadata, - data.step?.settings.blockName, - data.step?.settings.actionName, + blockName, + actionName, );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx(4 hunks)packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.tspackages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.tspackages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.tspackages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx
🔇 Additional comments (3)
packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.ts (1)
10-25: LGTM!The export addition is clean and minimal. The function logic remains unchanged, and exporting it enables reuse in the step-node component for risk assessment.
packages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx (2)
279-321: LGTM!The UI indicators are well-structured:
- Invalid step indicator with appropriate tooltip
- Risk indicator with a distinctive badge design and clear tooltip explaining the impact
- Proper use of
cnutility and consistent Tailwind styling- Correct Tooltip component usage with
asChildpropThe consolidated layout improves the visual hierarchy by grouping related indicators together.
92-95: No action needed. The hook already implements proper caching via react-query with a query key that includes bothsearchQueryandtype, ensuring that multiple step nodes callinguseAllStepsMetadatawith identical parameters (searchQuery: ''andtype: 'action') will share the same cached response rather than triggering separate API calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds visual indicators to highlight risky steps in the flow canvas. When a step is configured to perform a high-risk action, a shield badge now appears with a tooltip explaining the potential environmental impact. The implementation also improves the layout by consolidating validation indicators alongside the new risk warnings.
Key Changes:
- Exported
getActionMetadatautility function for reuse across components - Added risk level detection logic to identify high-risk steps
- Introduced shield badge UI component to visually indicate risky actions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.ts | Exported getActionMetadata function to enable reuse in the step node component |
| packages/react-ui/src/app/features/builder/flow-canvas/nodes/step-node.tsx | Added risk level detection, shield badge indicator, and reorganized validation indicators for better visual grouping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile OverviewGreptile SummaryAdded visual risk indicators to flow canvas nodes to highlight steps that may make changes to the environment. The implementation fetches action metadata to check risk levels and displays a shield badge with a tooltip for high-risk steps. Key Changes:
Implementation Details:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant StepNode as WorkflowStepNode
participant BlocksHook as blocksHooks
participant Utils as getActionMetadata
participant UI as UI Components
User->>StepNode: View flow canvas
StepNode->>BlocksHook: useAllStepsMetadata({type: 'action'})
BlocksHook-->>StepNode: Return actionsMetadata
StepNode->>StepNode: useMemo: Check isRiskyStep
StepNode->>Utils: getActionMetadata(actionsMetadata, blockName, actionName)
Utils-->>StepNode: Return actionMetadata with riskLevel
StepNode->>StepNode: Check if riskLevel === RiskLevel.HIGH
alt Step is risky
StepNode->>UI: Render ShieldHalf icon in destructive badge
StepNode->>UI: Add tooltip: "This step may make changes to your environment"
UI-->>User: Display risk indicator badge
else Step is not risky
StepNode->>UI: Skip risk indicator rendering
end
alt Step has invalid settings
StepNode->>UI: Render InvalidStepIcon with tooltip
UI-->>User: Display validation warning
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, no comments
| const actionName = data.step?.settings.actionName; | ||
| const blockName = data.step?.settings.blockName; | ||
|
|
||
| if (!actionsMetadata || !actionName || !blockName) { | ||
| return false; | ||
| } | ||
|
|
||
| const actionMetadata = getActionMetadata( | ||
| actionsMetadata, | ||
| blockName, | ||
| actionName, | ||
| ); | ||
|
|
||
| return actionMetadata?.riskLevel === RiskLevel.HIGH; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be cool to have this logic extracted from the component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, a custom hook placed outside this module would make mocking this dependency very convenient in case we add unit/sb tests in the future.
…ce redundant implementations
| const getActionMetadata = ( | ||
| metadata: StepMetadataWithSuggestions[] | undefined, | ||
| blockName: string, | ||
| actionName: string | undefined, | ||
| ): ActionBase | undefined => { | ||
| const blockStepMetadata = metadata?.find( | ||
| (stepMetadata: StepMetadataWithSuggestions) => | ||
| stepMetadata.type === ActionType.BLOCK && | ||
| (stepMetadata as BlockStepMetadataWithSuggestions).blockName === | ||
| blockName, | ||
| ) as BlockStepMetadataWithSuggestions | undefined; | ||
|
|
||
| return blockStepMetadata?.suggestedActions?.find( | ||
| (suggestedAction) => suggestedAction.name === actionName, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just moved from packages/react-ui/src/app/features/flows/components/execute-risky-flow-dialog/utils.ts
|



Fixes OPS-2810
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.