UN-2610 [FEAT] Implement simplified workflow studio UI #1442
Conversation
* UN-2541 Text Extractor Adapter edit update * UN-2541 Text Extractor Adapter edit update#2 * UN-2591 update method for adapter * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Adding code replaced during refactoring * Adding partial TRUE for update serializer * Pre-commit resolution commit * Refactoring as per git comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…lified-workflow-studio
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughA new workflow builder UI was implemented in the Agency component, replacing the previous layout and workflow execution interface. Supporting components and CSS were introduced or updated, including a new WorkflowCard component and related styles. The DsSettingsCard component was simplified by removing icons and adjusting its CSS. The SidePanel component was simplified by removing tabs and showing a static tool settings view. The ToolSettings component now displays a user-friendly empty state when no settings are available. Backend API viewsets were enhanced to support filtering by workflow ID. Frontend deployment and pipeline services were extended to fetch data filtered by workflow. Deployment modals now support optional callbacks after creation. The Steps component removed a redundant API call to avoid race conditions. New CSS files provide scoped styles for the workflow builder, workflow cards, settings card, and side panel header. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AgencyComponent
participant API
participant PostHog
participant Modal
User->>AgencyComponent: Visit workflow builder page
AgencyComponent->>API: Fetch workflow endpoint details
AgencyComponent->>API: Check if workflow updates allowed
AgencyComponent->>API: Fetch prompt studio projects
AgencyComponent->>AgencyComponent: Render workflow builder cards
User->>AgencyComponent: Select source/output/prompt project
User->>AgencyComponent: Click "Deploy" button
AgencyComponent->>PostHog: Track deploy button click
AgencyComponent->>AgencyComponent: Determine deployment type
AgencyComponent->>Modal: Open corresponding deployment modal
User->>Modal: Confirm deployment
Modal->>API: Create deployment
Modal->>AgencyComponent: Invoke onDeploymentCreated callback (if provided)
sequenceDiagram
participant User
participant AgencyComponent
participant DebugPanel
User->>AgencyComponent: Toggle Debug Panel button
AgencyComponent->>DebugPanel: Show/hide debug panel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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 (6)
frontend/src/components/agency/ds-settings-card/DsSettingsCard.css (1)
9-11: Consider removing!importantand using more specific selectors.The
!importantdeclaration suggests potential CSS specificity conflicts. Consider using more specific selectors or reviewing the cascade to avoid forcing styles.-.ds-set-card-select{ - width: 200px !important; -} +.workflow-card .ds-set-card-select, +.ds-settings-card .ds-set-card-select { + width: 200px; +}frontend/src/components/agency/agency/Agency.css (3)
61-68: Consider overflow handling for viewport height layout.The
height: 100vhwithoverflow: autois good, but consider addingmin-heightto prevent content cutoff on smaller screens..workflow-builder-layout { height: 100vh; + min-height: 600px; background: #f5f5f5; display: flex; flex-direction: column; overflow: auto; }
163-171: Fixed width may impact responsiveness.The
width: 200px !importantwith!importantcould cause issues on smaller screens. Consider using responsive units or media queries..workflow-select { - width: 200px !important; + width: 200px; + min-width: 150px; + max-width: 100%; }
200-211: Fixed sidebar width should be responsive.The fixed
width: 400pxmay be too wide on smaller screens. Consider responsive design..workflow-sidebar { position: fixed; top: 0; right: 0; - width: 400px; + width: min(400px, 90vw); height: 100vh; background: white; border-left: 1px solid #d9d9d9; box-shadow: -2px 0 8px rgba(0, 0, 0, 0.15); z-index: 1000; overflow-y: auto; }frontend/src/components/agency/agency/Agency.jsx (2)
23-26: Consider removing or documenting unused state variablesThere are several state variables marked with
eslint-disable-next-line no-unused-varscomments (inputMd,outputMd,activeToolId). If these are intended for future use, please add a comment explaining their purpose. Otherwise, consider removing them to keep the code clean.Also applies to: 46-47
451-457: Use a proper close icon for better consistencyConsider using an Ant Design icon for the close button instead of the × character for better visual consistency.
+import { CloseOutlined } from "@ant-design/icons"; <Button className="close-sidebar-btn" onClick={() => setShowSidebar(false)} + icon={<CloseOutlined />} + type="text" size="small" - > - × - </Button> + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (6)
frontend/src/components/agency/agency/Agency.css(1 hunks)frontend/src/components/agency/agency/Agency.jsx(3 hunks)frontend/src/components/agency/ds-settings-card/DsSettingsCard.css(1 hunks)frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx(2 hunks)frontend/src/components/agency/workflow-card/WorkflowCard.css(1 hunks)frontend/src/components/agency/workflow-card/WorkflowCard.jsx(1 hunks)
🔇 Additional comments (14)
frontend/src/components/agency/ds-settings-card/DsSettingsCard.css (1)
13-15: Good use of flexbox alignment.The
align-self: flex-endproperty appropriately aligns the button within its flex container.frontend/src/components/agency/workflow-card/WorkflowCard.css (1)
1-19: Well-structured component-specific styles.The CSS follows good practices with:
- Proper scoping using
.workflow-cardprefix- Consistent spacing and padding
- Appropriate width adjustments for nested components
The override of base styles is justified for component-specific layout requirements.
frontend/src/components/agency/agency/Agency.css (1)
70-228: Comprehensive and well-structured workflow builder styles.The CSS implementation demonstrates good practices:
- Semantic class naming convention
- Consistent spacing and color usage
- Proper flexbox layouts
- Appropriate use of z-index for layering
- Good visual hierarchy
frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx (3)
359-368: Good simplification of button UI.The change from a text button with icon to a primary button with clear "Configure" text improves usability and aligns with the simplified design goals.
335-348: Select component simplified appropriately.Removing the
size="small"prop allows the component to use default sizing, which works better with the new card layout and consistent styling.
14-14: Unused imports successfully cleaned up in DsSettingsCard.jsxAll import statements were reviewed—there are no remaining imports from
@ant-design/iconsortitleCase. No further cleanup is needed.frontend/src/components/agency/workflow-card/WorkflowCard.jsx (4)
7-15: Well-designed component props interface.The component accepts a flexible set of props that support both custom content and the default DsSettingsCard integration. The prop structure is intuitive and follows React best practices.
26-33: Good conditional rendering pattern.The conditional rendering logic allows for flexibility between custom content and the default DsSettingsCard, making the component reusable across different workflow steps.
40-48: Comprehensive prop validation.The PropTypes definitions are thorough and appropriate, with required props clearly marked and optional props having correct types.
16-37: Clean component structure with semantic HTML.The component uses semantic div elements with appropriate CSS classes and integrates well with Ant Design's Flex and Typography components. The layout structure supports the card-based design effectively.
frontend/src/components/agency/agency/Agency.jsx (4)
1-2: Import organization looks good!The new imports are well-organized and align with the PR objectives for implementing the simplified workflow studio UI.
Also applies to: 11-19
83-143: API functions are well-implementedThe API functions have consistent error handling and properly update the store with fetched data. Good job maintaining consistency across all three functions.
145-189: Deployment logic is well-structuredGood implementation of the deployment flow with proper validation and analytics tracking. The silent error handling for PostHog events is appropriate.
423-445: Debug panel implementation looks goodThe conditional rendering and toggle functionality are well-implemented. The placeholder text appropriately indicates that functionality will be added later.
🔄 PR Overview: Simplified Workflow Studio UI📋 Purpose SummaryThis PR implements a complete UI overhaul of the workflow studio page, transforming it from a complex multi-sidebar interface into a simplified, card-based workflow builder. The new design organizes workflow configuration into 4 logical steps presented in a 2x2 grid layout, with integrated deployment options, conditional debug panel, and a toggleable configuration sidebar. This modernization improves user experience by reducing cognitive load while maintaining all existing functionality. 📊 File Change Statistics
Detailed Breakdown:
🔄 Key Workflow SequencesequenceDiagram
participant User
participant WorkflowBuilder
participant API
participant PostHog
participant DeploymentModal
User->>WorkflowBuilder: Load workflow studio page
WorkflowBuilder->>API: getWfEndpointDetails()
WorkflowBuilder->>API: canUpdateWorkflow()
WorkflowBuilder->>API: getPromptStudioProjects()
API-->>WorkflowBuilder: Return configuration data
WorkflowBuilder->>WorkflowBuilder: Render 4-step card layout
Note over WorkflowBuilder: 1. Source Connector<br/>2. Output Destination<br/>3. Prompt Studio Project<br/>4. Deploy Workflow
User->>WorkflowBuilder: Configure source/destination connectors
User->>WorkflowBuilder: Select prompt studio project
User->>WorkflowBuilder: Choose deployment type (API/ETL/TASK)
WorkflowBuilder->>PostHog: Track deployment button click
WorkflowBuilder->>DeploymentModal: Open appropriate modal
User->>DeploymentModal: Complete deployment configuration
DeploymentModal->>API: Create deployment
🎯 UI Flow Architectureflowchart TD
A[Workflow Builder Layout] --> B[Header Section]
A --> C[Progress Section]
A --> D[2x2 Grid Container]
A --> E[Debug Panel]
A --> F[Conditional Sidebar]
B --> B1[Back Button + Title]
B --> B2[Actions Dropdown]
C --> C1[Progress Label]
C --> C2[Progress Bar]
D --> D1[Card 1: Source Connector]
D --> D2[Card 2: Output Destination]
D --> D3[Card 3: Prompt Studio Project]
D --> D4[Card 4: Deploy Workflow]
D1 --> D1A[DsSettingsCard Component]
D2 --> D2A[DsSettingsCard Component]
D3 --> D3A[Custom Select + Configure Button]
D4 --> D4A[Deployment Type Select + Deploy Button]
E --> E1[Collapsible Debug Panel]
E1 --> E2[Debug Information Display]
F --> F1[SidePanel Component]
F --> F2[Close Button]
D3A --> F[Triggers Sidebar]
D4A --> G[Opens Deployment Modals]
G --> G1[CreateApiDeploymentModal]
G --> G2[EtlTaskDeploy - ETL]
G --> G3[EtlTaskDeploy - TASK]
🏗️ Architecture ChangesMajor Transformations:
New Components:
Conditional Rendering:
🎨 Design System UpdatesCSS Architecture:
Visual Improvements:
This overview was automatically generated to help reviewers understand the scope and impact of the changes. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…lified-workflow-studio
- Remove all console.log statements from Agency and SidePanel components - Fix deployment button behavior to show single Deploy Workflow button - Remove view existing deployment functionality from dropdown selection - Clean up debugging code while maintaining all functionality - Improve code readability by removing development logging 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
frontend/src/components/agency/agency/Agency.jsx (5)
80-85: Add cleanup for API calls to prevent potential memory leaks.The useEffect hook triggers multiple API calls on mount, but there's no cleanup mechanism if the component unmounts while requests are in progress.
This matches a previous review concern. The API calls should implement cleanup using an AbortController or mounted flag to prevent state updates on unmounted components.
260-262: Handle PostHog tracking errors appropriately.The empty catch block ignores PostHog tracking errors, which can make debugging analytics issues difficult.
As suggested in previous reviews, add proper error logging:
} catch (err) { - // If an error occurs while setting custom posthog event, ignore it and continue + // Log PostHog errors for debugging while not breaking the main flow + console.warn('PostHog event tracking failed:', err); }
265-270: Remove redundant deployment trigger from dropdown.The deployment is triggered both when selecting from the dropdown (onChange) and when clicking the Deploy button, creating confusing UX.
const handleDeploymentTypeChange = (value) => { setSelectedDeploymentType(value); - if (value) { - handleDeployBtnClick(value); - } };Users should explicitly click the "Deploy Workflow" button to trigger deployment.
435-448: Implement functionality for actions menu items.The actions dropdown menu items have no click handlers, making them non-functional.
As suggested in previous reviews, add click handlers or remove non-functional items:
const actionsMenuItems = [ { key: "clear-cache", label: "Clear Cache", + onClick: () => { + // Implement cache clearing logic + console.log("Clear cache clicked"); + } }, // ... other items with onClick handlers ];
493-508: Add safety checks for array access.The hardcoded array indices could cause runtime errors if
sourceTypes.connectorsdoesn't have the expected elements.-type={sourceTypes.connectors[0]} +type={sourceTypes.connectors?.[0] || "input"}-type={sourceTypes.connectors[1]} +type={sourceTypes.connectors?.[1] || "output"}
🧹 Nitpick comments (2)
frontend/src/components/agency/tool-settings/ToolSettings.jsx (1)
32-34: Consider consolidating duplicate utility functions.This
isObjectEmptyfunction is similar to the one inSidePanel.jsx. Consider moving it to a shared utilities file to avoid code duplication.You could create a shared utilities file like
utils/objectUtils.js:+// utils/objectUtils.js +export const isObjectEmpty = (obj) => { + return obj && Object.keys(obj).length === 0; +};Then import it in both files:
+import { isObjectEmpty } from '../../../utils/objectUtils'; -const isObjectEmpty = (obj) => { - return obj && Object.keys(obj).length === 0; -};frontend/src/components/agency/agency/Agency.jsx (1)
34-77: Consider consolidating related state variables.The component now manages many state variables (14+ new ones). Consider grouping related states into objects or using a reducer pattern to improve maintainability:
+const [uiState, setUiState] = useState({ + showDebug: false, + showSidebar: false, + selectedDeploymentType: null, + workflowProgress: 0 +}); + +const [modalState, setModalState] = useState({ + openAddApiModal: false, + openAddETLModal: false, + openAddTaskModal: false +});This would reduce the number of individual state setters and group related functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (6)
frontend/src/components/agency/agency/Agency.css(1 hunks)frontend/src/components/agency/agency/Agency.jsx(3 hunks)frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx(2 hunks)frontend/src/components/agency/side-panel/SidePanel.css(1 hunks)frontend/src/components/agency/side-panel/SidePanel.jsx(4 hunks)frontend/src/components/agency/tool-settings/ToolSettings.jsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/agency/side-panel/SidePanel.css
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx
- frontend/src/components/agency/agency/Agency.css
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/agency/tool-settings/ToolSettings.jsx (2)
frontend/src/components/agency/side-panel/SidePanel.jsx (1)
isObjectEmpty(20-22)frontend/src/components/input-output/configure-ds/ConfigureDs.jsx (2)
isFormValid(73-82)formRef(36-36)
🪛 GitHub Check: SonarCloud
frontend/src/components/agency/agency/Agency.jsx
[notice] 221-223: Exceptions should not be ignored
Handle this exception or don't catch it at all.See more on SonarQube Cloud
[notice] 260-262: Exceptions should not be ignored
Handle this exception or don't catch it at all.See more on SonarQube Cloud
⏰ 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: build
🔇 Additional comments (6)
frontend/src/components/agency/tool-settings/ToolSettings.jsx (2)
38-38: Good cleanup removing unused parameter.Simplifying the callback signature by removing the unused
valuesparameter improves code clarity.
80-103: Excellent empty state implementation.The conditional rendering provides a clear, user-friendly message when tools don't have configurable settings. The logic correctly checks for loading state and spec emptiness, and the messaging guides users to the appropriate interface.
frontend/src/components/agency/side-panel/SidePanel.jsx (2)
1-9: Well-executed simplification of the SidePanel component.The removal of tabbed interface and focus on tool settings alone makes the component cleaner and more maintainable. The Typography import supports the new static header approach.
46-58: Improved error handling and documentation.The added comments provide valuable context about Prompt Studio tools and API error handling, making the code more maintainable and easier to understand.
frontend/src/components/agency/agency/Agency.jsx (2)
388-433: Excellent progress calculation implementation.The dynamic progress calculation properly addresses the previous concern about hardcoded values. It correctly checks all four workflow steps and provides meaningful feedback to users.
524-598: Well-implemented tool selection and management logic.The tool selection handler comprehensively manages both new tool additions and replacements. The logic properly handles cleanup of existing tools, API interactions, and store updates with appropriate error handling.
- Replace numbered circles with green checkmarks for completed steps - Add CheckOutlined icon from Ant Design for completed state - Update CSS to show green background (#52c41a) for completed steps - Simplify step indicators to only show checkmark without text - Improve visual feedback for workflow progress 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…pport - Add Run Workflow, Clear Cache, and Clear Processed File History options to Actions dropdown - Implement proper workflow execution logic with file upload modal support for API workflows - Add workflow validation to prevent execution without complete configuration - Remove all console.log statements for clean production code - Change progress bar color to green when workflow reaches 100% completion - Clean up unused activeToolId state variable and related setters - Maintain compatibility with existing workflow store and alert systems 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
frontend/src/components/agency/agency/Agency.jsx (5)
95-101: Add cleanup mechanism to prevent memory leaks in API calls.The useEffect hook triggers multiple API calls without cleanup, which could cause memory leaks if the component unmounts while requests are in progress.
Consider implementing an AbortController or mounted flag pattern as suggested in previous reviews to prevent state updates on unmounted components.
326-328: Empty catch block should include error logging.The empty catch block ignores PostHog tracking errors. Add at least a console warning for debugging:
} catch (err) { - // If an error occurs while setting custom posthog event, ignore it and continue + // Log PostHog errors for debugging while not breaking the main flow + console.warn('PostHog event tracking failed:', err); }
331-336: Remove redundant deployment trigger from dropdown onChange.The deployment is triggered both when selecting from the dropdown and when clicking the Deploy button, creating confusing UX. Remove the automatic trigger:
const handleDeploymentTypeChange = (value) => { setSelectedDeploymentType(value); - if (value) { - handleDeployBtnClick(value); - } };This ensures deployment only happens when users explicitly click "Deploy Workflow".
868-869: Potential array access error with hardcoded indices.Add safety checks for array access to prevent runtime errors:
- type={sourceTypes.connectors[0]} + type={sourceTypes.connectors?.[0] || "input"}
886-887: Potential array access error with hardcoded indices.Add safety checks for array access:
- type={sourceTypes.connectors[1]} + type={sourceTypes.connectors?.[1] || "output"}
🧹 Nitpick comments (1)
frontend/src/components/agency/agency/Agency.jsx (1)
902-977: Complex tool selection logic needs error handling improvements.The tool selection handler is quite complex with multiple async operations. Consider extracting this logic into a separate function and adding better error boundaries:
const handleToolSelection = async (functionName) => { try { setSelectedTool(functionName); await replaceOrCreateToolInstance(functionName); } catch (err) { // Reset selection on error setSelectedTool(null); setAlertDetails(handleException(err, "Failed to update tool")); } }; const replaceOrCreateToolInstance = async (functionName) => { // Extract the existing logic here // This makes testing and maintenance easier };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (9)
backend/api_v2/api_deployment_views.py(1 hunks)backend/pipeline_v2/views.py(1 hunks)frontend/src/components/agency/agency/Agency.css(2 hunks)frontend/src/components/agency/agency/Agency.jsx(3 hunks)frontend/src/components/agency/steps/Steps.jsx(1 hunks)frontend/src/components/deployments/api-deployment/api-deployments-service.js(1 hunks)frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx(4 hunks)frontend/src/components/pipelines-or-deployments/etl-task-deploy/EtlTaskDeploy.jsx(3 hunks)frontend/src/components/pipelines-or-deployments/pipeline-service.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/agency/agency/Agency.css
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/api_v2/api_deployment_views.py (1)
backend/api_v2/models.py (1)
APIDeployment(26-129)
⏰ 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: build
🔇 Additional comments (16)
frontend/src/components/agency/steps/Steps.jsx (1)
39-40: LGTM! Good architectural improvement.Removing the redundant
canUpdateWorkflow()call prevents race conditions with the parentAgency.jsxcomponent that now centrally manages workflow state. The comments clearly explain the rationale for this change.frontend/src/components/pipelines-or-deployments/pipeline-service.js (1)
90-96: LGTM! Clean service method addition.The new
getPipelinesByWorkflowIdmethod follows the established patterns in this service file and correctly constructs the GET request with the workflow query parameter to leverage the backend filtering capability.frontend/src/components/pipelines-or-deployments/etl-task-deploy/EtlTaskDeploy.jsx (3)
38-38: LGTM! Good callback prop addition.The new
onDeploymentCreatedcallback prop enhances component integration capabilities and follows proper optional prop patterns.
252-255: LGTM! Proper callback invocation.The callback is correctly invoked conditionally after successful deployment creation and workflow state updates. The placement and conditional logic are appropriate.
404-404: LGTM! Proper PropTypes definition.The PropTypes definition correctly marks the callback as optional using
PropTypes.func.frontend/src/components/deployments/api-deployment/api-deployments-service.js (1)
92-98: LGTM! Consistent service method implementation.The new
getDeploymentsByWorkflowIdmethod follows the established patterns in this service file using theoptionsvariable approach and correctly constructs the GET request with the workflow query parameter.backend/api_v2/api_deployment_views.py (1)
133-140: LGTM! Well-implemented queryset filtering.The workflow filtering enhancement maintains backward compatibility and correctly uses
workflow_idto filter by the foreign key relationship. The conditional filtering approach is clean and follows Django ORM best practices.backend/pipeline_v2/views.py (1)
50-53: LGTM! Clean implementation of workflow-based filtering.The addition of workflow ID filtering follows the established pattern and integrates well with the workflow-centric UI changes. The implementation correctly checks for parameter presence and applies appropriate filtering.
frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx (3)
30-30: LGTM! Well-structured callback prop addition.The new
onDeploymentCreatedcallback prop enables better integration with the workflow-centric UI by allowing parent components to refresh deployment information after creation.
123-126: Proper callback implementation with appropriate guards.The callback is correctly invoked only when present and within the workflow context, ensuring no breaking changes to existing usage patterns.
299-299: PropTypes properly updated for the new callback prop.frontend/src/components/agency/agency/Agency.jsx (5)
476-504: Progress calculation logic is well-implemented.The dynamic progress calculation properly considers all four workflow steps with appropriate status checks. The logic correctly handles deployed workflows and tool instance validation.
830-833: Progress display now correctly uses calculated values.The hardcoded progress values have been replaced with dynamic calculation based on workflow state. This addresses the previous concern about showing actual progress to users.
840-848: Actions dropdown functionality is properly implemented.The dropdown menu now has working click handlers that properly execute workflow actions like run, clear cache, and clear history. This addresses the previous concern about non-functional menu items.
1162-1163: Excellent integration of callback for deployment refresh.The
onDeploymentCreated={fetchDeploymentInfo}callback properly integrates the modal with the parent component's state management, ensuring the UI reflects deployment changes immediately.
1013-1013: Deployment type selection handles existing deployments correctly.The logic properly shows existing deployment types while disabling changes when deployments are active, providing clear user feedback about workflow state.
- Configure button now properly disabled when workflow is deployed via ETL/Task pipelines - Deploy workflow dropdown correctly shows selected deployment type (ETL/TASK) - Pipeline name and redirection link now display for ETL/Task deployments - Changed pipeline detection logic to include all workflow pipelines regardless of active status 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Added new ToolSelectionSidebar component to replace dropdown - Updated Agency.css with sidebar and loading styles - Improved tool selection UX with visual cards and search functionality 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
frontend/src/components/agency/agency/Agency.jsx (3)
358-360: LGTM: PostHog error handling implementedThe empty catch block now has proper context from past reviews. This is acceptable for non-critical analytics tracking where failures shouldn't break the main flow.
363-368: Deployment UX issue persistsThe redundant deployment trigger issue identified in past reviews still exists. The dropdown onChange still triggers deployment immediately, which could confuse users who expect to click the Deploy button explicitly.
const handleDeploymentTypeChange = (value) => { setSelectedDeploymentType(value); - if (value) { - handleDeployBtnClick(value); - } };
981-994: Potential runtime error with array accessThe hardcoded array access issue identified in past reviews still exists.
sourceTypes.connectors[0]could cause runtime errors if the array is undefined or doesn't have expected elements.- type={sourceTypes.connectors[0]} + type={sourceTypes.connectors?.[0] || "input"}
🧹 Nitpick comments (3)
frontend/src/components/agency/tool-selection-sidebar/ToolSelectionSidebar.jsx (2)
30-34: Consider performance optimization for search filteringThe filtering logic is recalculated on every render. For better performance with large tool lists, consider using useMemo to memoize the filtered results.
+ import { useState, useEffect, useMemo } from "react"; - const filteredTools = tools.filter( - (tool) => - tool.name?.toLowerCase().includes(searchTerm.toLowerCase()) || - tool.description?.toLowerCase().includes(searchTerm.toLowerCase()) - ); + const filteredTools = useMemo(() => + tools.filter( + (tool) => + tool.name?.toLowerCase().includes(searchTerm.toLowerCase()) || + tool.description?.toLowerCase().includes(searchTerm.toLowerCase()) + ), [tools, searchTerm] + );
108-114: Redundant fallback in category displayThe category tag has a redundant fallback that will never be used since the condition already checks for
tool.categoryexistence.{tool.category && ( <div className="tool-category"> <Tag className="tool-category-tag"> - {tool.category || "Financial Documents"} + {tool.category} </Tag> </div> )}frontend/src/components/agency/agency/Agency.jsx (1)
1067-1098: Complex conditional logic could be simplifiedThe deployment options rendering logic is quite complex and could benefit from extraction into a helper function for better readability and maintainability.
+ const getDeploymentOptions = () => { + const baseOptions = [ + { + value: "API", + label: getDeploymentStatusText("API"), + disabled: false, + } + ]; + + if (!apiOpsPresent || deploymentInfo) { + baseOptions.push( + { + value: "ETL", + label: getDeploymentStatusText("ETL"), + disabled: false, + }, + { + value: "TASK", + label: getDeploymentStatusText("TASK"), + disabled: false, + } + ); + } + + return baseOptions; + }; <Select className="workflow-select" placeholder="Select Deployment Type" value={ deploymentInfo ? deploymentInfo.type : selectedDeploymentType } onChange={handleDeploymentTypeChange} disabled={deploymentInfo || !allowChangeEndpoint} - options={[ - { - value: "API", - label: getDeploymentStatusText("API"), - disabled: false, - }, - ...(apiOpsPresent && !deploymentInfo - ? [] - : [ - { - value: "ETL", - label: getDeploymentStatusText("ETL"), - disabled: false, - }, - { - value: "TASK", - label: getDeploymentStatusText("TASK"), - disabled: false, - }, - ]), - ]} + options={getDeploymentOptions()} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (5)
frontend/src/components/agency/agency/Agency.css(2 hunks)frontend/src/components/agency/agency/Agency.jsx(3 hunks)frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx(2 hunks)frontend/src/components/agency/tool-selection-sidebar/ToolSelectionSidebar.css(1 hunks)frontend/src/components/agency/tool-selection-sidebar/ToolSelectionSidebar.jsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/agency/tool-selection-sidebar/ToolSelectionSidebar.css
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx
- frontend/src/components/agency/agency/Agency.css
⏰ 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: build
🔇 Additional comments (6)
frontend/src/components/agency/tool-selection-sidebar/ToolSelectionSidebar.jsx (2)
20-27: LGTM: Proper state synchronizationThe useEffect correctly synchronizes the temporary selected tool state when the sidebar becomes visible or when the selected tool changes. The dependency array is appropriate and the state resets are handled properly.
138-145: LGTM: Comprehensive PropTypes validationThe PropTypes validation covers all required props with appropriate types. The optional
selectedToolprop is correctly marked as not required.frontend/src/components/agency/agency/Agency.jsx (4)
100-122: Improved: API cleanup mechanism implementedThe refactored initialization with Promise.all and proper error handling addresses the previous concern about memory leaks. The timeout for preventing content flash is a good UX touch.
508-536: Excellent: Dynamic progress calculation implementedThis addresses the previous hardcoded progress issue perfectly. The logic properly calculates completion based on actual workflow state and provides meaningful feedback to users.
789-858: Robust tool selection implementationThe tool selection logic properly handles existing tool cleanup, creates new instances, and updates both workflow and tool settings stores. Good error handling and user feedback are implemented.
910-924: Improved: Actions menu with functional handlersThe actions menu now has proper functionality implemented, addressing the previous concern about non-functional menu items. Each action has a corresponding handler.
…d UX - Replace tool dropdown with interactive sidebar for better tool selection - Add search functionality and detailed tool information display - Fix InputOutput component to properly receive workflow execution data - Add real-time status message display in debug panel - Remove deprecated Actions component and related unused files - Clean up stepLoader state (step execution is disabled) - Improve workflow execution feedback with auto-opening debug panel - Update workflow card styling for completed steps (light blue theme) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove redundant deployment trigger from dropdown onChange handler - Add AbortController to prevent memory leaks in API calls during component unmount - Fix useEffect dependency array to include correct deployment state variables - Extract nested ternary operations into clean generateDeploymentMessage function - Add proper cleanup functions to abort ongoing requests when component unmounts - Update all API functions to accept and handle abort signals properly Fixes three critical issues: 1. Deployment now only triggers on explicit button click, not dropdown selection 2. API calls are properly cancelled preventing memory leaks and race conditions 3. useEffect runs correctly when deployment-related dependencies change 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…lity - Added getConfigureTooltipMessage() function to replace complex nested ternary - Improves code maintainability and Sonar compliance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
jagadeeswaran-zipstack
requested review from
Deepak-Kesavan,
athul-rs and
chandrasekharan-zipstack
There was a problem hiding this comment.
@jagadeeswaran-zipstack added minor comment. resolve those. everything else looks okay to me.
…ncy component - Import useRequestUrl hook and initialize getUrl function - Replace all 8 hardcoded URL constructions with getUrl calls - Makes URL handling consistent with rest of the codebase - Addresses PR review comment: #1442 (comment) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/components/agency/agency/Agency.jsx (1)
1057-1057: Potential runtime error with hardcoded array indicesThe code assumes
sourceTypes.connectorshas at least 2 elements, which could cause runtime errors if the array is shorter than expected.- type={sourceTypes.connectors[0]} + type={sourceTypes.connectors?.[0] || "input"} - type={sourceTypes.connectors[1]} + type={sourceTypes.connectors?.[1] || "output"}Also applies to: 1075-1075
🧹 Nitpick comments (2)
frontend/src/components/agency/agency/Agency.jsx (2)
43-96: Consider consolidating complex state managementThe component manages 25+ state variables, which increases complexity and potential for inconsistencies. While the current approach works, consider using
useReducerfor related state groups to improve maintainability.For example, group deployment-related states:
- const [selectedDeploymentType, setSelectedDeploymentType] = useState(null); - const [openAddApiModal, setOpenAddApiModal] = useState(false); - const [openAddETLModal, setOpenAddETLModal] = useState(false); - const [openAddTaskModal, setOpenAddTaskModal] = useState(false); - const [deploymentName, setDeploymentName] = useState(""); - const [deploymentType, setDeploymentType] = useState(""); + const [deploymentState, dispatchDeployment] = useReducer(deploymentReducer, { + selectedType: null, + modals: { api: false, etl: false, task: false }, + name: "", + type: "" + });
1227-1238: Sidebar implementation is clean but could be improvedThe conditional sidebar rendering works well, but the close button positioning might benefit from better styling integration.
Consider improving the close button styling:
<Button className="close-sidebar-btn" onClick={() => setShowSidebar(false)} + type="text" + icon={<CloseOutlined />} size="small" > - × </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (2)
frontend/src/components/agency/agency/Agency.jsx(4 hunks)frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx
⏰ 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: build
🔇 Additional comments (10)
frontend/src/components/agency/agency/Agency.jsx (10)
1-42: Imports are well-organized and comprehensiveThe import structure is logical, grouping external dependencies (Ant Design, React) before internal components and utilities. All imports appear to be utilized in the component implementation.
97-133: Excellent cleanup implementation for API callsThe initialization useEffect properly implements AbortController for cleanup, includes comprehensive error handling, and prevents state updates on unmounted components. The timeout delay to prevent content flash is a nice UX touch.
161-192: Deployment logic is well-structured but complexThe effect properly manages deployment type availability based on connector configurations. The logic correctly prevents invalid deployment selections when switching between API and other types.
273-337: Deployment info fetching is robust and well-implementedThe parallel API calls using
Promise.all, proper error handling, and signal abort checking demonstrate good async programming practices. The logic for prioritizing deployment types (ETL > TASK) is clearly documented and sensible.
568-597: Progress calculation logic is comprehensiveThe dynamic progress calculation properly accounts for all workflow steps including connector configuration, tool selection, and deployment status. This addresses the previous hardcoded progress issue effectively.
854-924: Tool selection implementation handles edge cases wellThe function properly manages replacing existing tools, updating multiple stores (workflow and tool settings), and provides appropriate user feedback. The error handling ensures consistency even if operations fail partway through.
992-1001: Loading state implementation is clean and user-friendlyThe loading spinner with descriptive text provides good user feedback during initial data load. Using IslandLayout maintains consistency with the overall application structure.
1043-1184: Card-based workflow grid is intuitive and well-structuredThe 2x2 grid layout with numbered workflow cards provides excellent visual hierarchy and user guidance. The dynamic status indicators (✓ vs numbers) clearly show completion status, and the conditional rendering based on workflow state is implemented correctly.
1185-1195: Deployment alert provides helpful user feedbackThe conditional alert with dynamic message generation and proper linking to deployment pages gives users clear information about their workflow's deployment status.
1212-1224: Debug panel implementation enhances developer experienceThe collapsible debug panel with status messages and input/output display provides valuable debugging capabilities while keeping the UI clean when not needed.
|
|
What
Complete UI overhaul of the workflow studio page
Why
The previous workflow studio interface was complex and overwhelming for users, with multiple
sidebars and scattered configuration options. This simplified approach:
How
Major Changes:
Notes on Testing
Testing Scenarios:
Screenshots