Worflow UI#45
Conversation
…ooltip components and mobile hook - Implemented `Collapsible`, `CollapsibleTrigger`, and `CollapsibleContent` components for collapsible UI sections. - Created `DropdownMenu` components including `DropdownMenuPortal`, `DropdownMenuTrigger`, `DropdownMenuContent`, and various item types for enhanced dropdown functionality. - Added `Separator` component for visual separation in layouts. - Developed a comprehensive `Sidebar` component with context management, mobile responsiveness, and various subcomponents for structured navigation. - Introduced `Skeleton` component for loading states. - Implemented `Tooltip` components for contextual hints and information. - Added `useIsMobile` hook to detect mobile device state for responsive design.
📝 WalkthroughWalkthroughThis PR introduces a sidebar-based workflow editor layout with Radix UI components, expands user state management to include email and name, refactors GoogleSheetFormClient to accept initial data, updates the WorkflowSchema by removing UserId, and adds multiple new UI primitives (collapsible, dropdown-menu, separator, tooltip, sidebar system, skeleton, and mobile hook). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/app/components/nodes/CreateWorkFlow.tsx (1)
123-151: Dead code:getEmptyWorkflowIDis defined but never called.The function
getEmptyWorkflowID(lines 124-138) is defined but the call on line 149 is commented out. Consider removing the dead code or uncommenting if the functionality is needed.useEffect(()=>{ - async function getEmptyWorkflowID(){ - const workflow = await getEmptyWorkflow() - - if(workflow){ - const {id, isEmpty} = workflow - dispatch(workflowActions.setWorkflowId(id)) - dispatch(workflowActions.setWorkflowStatus(isEmpty)) - } - else{ - if (!userId) return - const newWorkflow = await createWorkflow() - dispatch(workflowActions.setWorkflowId(newWorkflow.id)) - dispatch(workflowActions.setWorkflowStatus(newWorkflow.isEmpty)) - } - } async function getWorkflowData(){ if(!workflowId) return const workflow = await getworkflowData(workflowId) if(workflow.success){ dispatch(workflowActions.setWorkflowStatus(false)) dispatch(workflowActions.setWorkflowNodes(workflow.data.nodes)) dispatch(workflowActions.setWorkflowTrigger(workflow.data.Trigger)) } } - // if(!workflowId) getEmptyWorkflowID() getWorkflowData(); },[dispatch, userId, workflowId])apps/web/app/components/nodes/GoogleSheetFormClient.tsx (2)
150-154: Potential runtime error: unsafe property access.
data.files.lengthwill throw ifdata.filesis undefined. The initial fetch effect (line 76) uses optional chaining (data.files?.length), but this handler doesn't.const data = await response.json(); - if (data.files.length >0) { + if (data.files?.length > 0) { setDocuments(data.files || []); }
177-180: Potential runtime error: unsafe nested property access.
data.files.data.lengthwill throw ifdata.filesordata.files.datais undefined.const data = await response.json(); - if (data.files.data.length > 0) { + if (data.files?.data?.length > 0) { setSheets(data.files.data || []); }
🧹 Nitpick comments (9)
packages/ui/src/components/skeleton.tsx (1)
3-11: Consider adding accessibility attributes for loading states.Skeleton components represent loading placeholders and should communicate this state to assistive technologies. Adding
aria-busy="true"oraria-hidden="true"improves the experience for screen reader users.🔎 Proposed enhancement
function Skeleton({ className, ...props }: React.ComponentProps<"div">) { return ( <div data-slot="skeleton" className={cn("bg-accent animate-pulse rounded-md", className)} + aria-busy="true" + role="status" {...props} /> ) }Alternatively, use
aria-hidden="true"if the skeleton should be completely hidden from screen readers.packages/ui/src/hooks/use-mobile.ts (1)
8-16: Reorder initial state update to avoid potential race condition.The current implementation adds the event listener (line 13) before setting the initial state (line 14). While unlikely in practice, this creates a theoretical race condition where a viewport change could occur between these two operations, potentially causing the state to be overwritten incorrectly.
🔎 Recommended ordering
React.useEffect(() => { const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`) const onChange = () => { setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) } - mql.addEventListener("change", onChange) setIsMobile(window.innerWidth < MOBILE_BREAKPOINT) + mql.addEventListener("change", onChange) return () => mql.removeEventListener("change", onChange) }, [])apps/web/app/components/ui/app-sidebar.tsx (4)
46-59: Potential stale data and unnecessary fetches due to useEffect dependency.The effect depends on
selectedWorkflow, but internally it only runs fetches when!credsor!workflowis truthy. This means:
- Changing
selectedWorkflowwon't refetch data if it's already loaded- The dependency doesn't match actual intent
Consider removing
selectedWorkflowfrom deps if refetch isn't intended, or refetch unconditionally if fresh data is needed on workflow change.useEffect(()=>{ async function getWorkflows(){ const flows = await getAllWorkflows(); if(flows) setWorkflow(flows) } async function getCreds(){ const credentials = await getAllCredentials(); if(credentials) setCreds(credentials) } - if(!creds) getCreds() - if(!workflow) getWorkflows() - },[selectedWorkflow]) + getCreds() + getWorkflows() + },[]) // Run once on mount, or add proper refetch trigger
64-67: Empty handler function.
credHandleris defined but does nothing. Either implement the intended behavior or remove it to avoid confusion.
76-93: Missing error handling increateNewWorkflow.If
createWorkflow()throws, the error is unhandled and could cause a poor UX. Consider wrapping in try/catch with appropriate user feedback.🔎 Suggested improvement
const createNewWorkflow = async()=>{ + try { const workflow = await getEmptyWorkflow() if(workflow){ const {id, isEmpty} = workflow dispatch(workflowActions.setWorkflowId(id)) dispatch(workflowActions.setWorkflowStatus(isEmpty)) toast.info("You are in empty workflow") } else{ const newWorkflow = await createWorkflow() dispatch(workflowActions.clearWorkflow()) setSelectedWorkflow(null) dispatch(workflowActions.setWorkflowId(newWorkflow.id)) dispatch(workflowActions.setWorkflowStatus(newWorkflow.isEmpty)) toast.success("Workflow created") } + } catch (error) { + console.error("Failed to create workflow:", error) + toast.error("Failed to create workflow") + } }
44-45: Consider clearer variable naming.
workflow(line 44) stores an array of workflows, which can be confusing alongsideflow(single workflow from Redux). Consider renaming toworkflowsfor clarity.- const [workflow, setWorkflow] = useState<Array<any>>() + const [workflows, setWorkflows] = useState<Array<any>>([])packages/ui/src/components/tooltip.tsx (1)
21-29: EachTooltipcreates its ownTooltipProvider.Wrapping each
Tooltipinstance in its ownTooltipProviderworks, but can be inefficient with many tooltips. Radix recommends a single provider at the app root. However, this pattern does ensure each tooltip is self-contained.If many tooltips will be used, consider having consumers wrap with a single
TooltipProviderat a higher level, and havingTooltipjust useTooltipPrimitive.Rootdirectly.apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
63-113: Sharedloadingstate between two async effects can cause UI inconsistency.Both
fetchInitialDocumentsandfetchInitialSheetsset/clear the sameloadingstate independently. If both run simultaneously, the first to finish will setloading = falsewhile the other is still fetching.Consider using separate loading states or a counter-based approach.
🔎 Suggested approach
+ const [loadingDocuments, setLoadingDocuments] = useState(false); + const [loadingSheets, setLoadingSheets] = useState(false); + const loading = loadingDocuments || loadingSheets; useEffect(() => { const fetchInitialDocuments = async () => { if (!initialData?.credentialId) return; - setLoading(true); + setLoadingDocuments(true); try { // ...fetch logic } finally { - setLoading(false); + setLoadingDocuments(false); } }; fetchInitialDocuments(); }, [initialData?.credentialId]);packages/ui/src/components/sidebar.tsx (1)
85-86: Cookie missingSameSiteattribute.Modern browsers default to
SameSite=Lax, but explicitly setting it is a best practice for clarity and cross-browser consistency.- document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}` + document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Lax`
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/http-backend/src/routes/userRoutes/userRoutes.tsapps/web/app/components/nodes/CreateWorkFlow.tsxapps/web/app/components/nodes/GoogleSheetFormClient.tsxapps/web/app/components/providers.tsxapps/web/app/components/ui/app-sidebar.tsxapps/web/app/workflow/lib/config.tsapps/web/app/workflow/page.tsxapps/web/store/slices/userSlice.tspackages/common/src/index.tspackages/ui/package.jsonpackages/ui/src/components/button.tsxpackages/ui/src/components/collapsible.tsxpackages/ui/src/components/dropdown-menu.tsxpackages/ui/src/components/separator.tsxpackages/ui/src/components/sidebar.tsxpackages/ui/src/components/skeleton.tsxpackages/ui/src/components/tooltip.tsxpackages/ui/src/hooks/use-mobile.ts
💤 Files with no reviewable changes (1)
- packages/common/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (9)
packages/ui/src/components/collapsible.tsx (3)
packages/ui/src/components/label.tsx (1)
Label(8-22)packages/ui/src/components/sheet.tsx (3)
Sheet(9-11)SheetTrigger(13-17)SheetTitle(104-115)packages/ui/src/components/select.tsx (4)
SelectScrollUpButton(140-156)SelectTrigger(27-51)Select(9-13)SelectScrollDownButton(158-174)
packages/ui/src/components/tooltip.tsx (4)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/select.tsx (1)
SelectTrigger(27-51)packages/ui/src/components/sheet.tsx (2)
SheetOverlay(31-45)Sheet(9-11)packages/ui/src/components/label.tsx (1)
Label(8-22)
apps/web/store/slices/userSlice.ts (2)
apps/web/app/login/page.tsx (2)
e(83-83)apps/web/store/slices/workflowSlice.ts (1)
WorkflowSlice(19-24)
apps/web/app/workflow/lib/config.ts (3)
packages/nodes/src/common/google-oauth-service.ts (2)
getAllCredentials(103-117)getCredentials(77-101)packages/common/src/index.ts (1)
BACKEND_URL(4-4)apps/hooks/src/index.ts (1)
req(7-53)
apps/web/app/components/providers.tsx (1)
apps/web/store/slices/userSlice.ts (1)
userAction(41-41)
packages/ui/src/components/dropdown-menu.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/web/app/components/nodes/CreateWorkFlow.tsx (4)
apps/web/app/hooks/redux.ts (1)
useAppSelector(7-7)apps/web/app/workflow/lib/config.ts (1)
createWorkflow(78-100)apps/web/app/types/workflow.types.ts (1)
NodeType(1-11)apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
GoogleSheetFormClient(29-371)
packages/ui/src/components/separator.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (2)
apps/http-backend/src/routes/userRoutes/userMiddleware.ts (2)
userMiddleware(18-42)AuthRequest(8-10)packages/db/src/index.ts (1)
prismaClient(17-18)
🔇 Additional comments (20)
packages/ui/src/components/skeleton.tsx (1)
13-13: LGTM!The export follows standard conventions for the UI component library.
apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
369-369: Good catch on the typo fix!Correcting
error.meesagetoerror.messagefixes a potential bug where the error message would be undefined.packages/ui/src/hooks/use-mobile.ts (1)
5-19: Consider SSR hydration implications of initialundefinedstate.The hook initializes
isMobileasundefined(line 6) and returns!!isMobile(line 18), which coercesundefinedtofalseon the initial render. This means:
- Server-side: Always returns
false(sinceuseEffectdoesn't run on server)- Client first render: Returns
false(before effect runs)- After effect: Returns actual viewport state
This pattern is intentional for SSR safety but can cause:
- Potential hydration mismatch warnings if server renders assuming desktop
- Brief layout shift when the effect updates the state post-hydration
If this is intentional, consider documenting this behavior. Alternatively, you could return
undefinedexplicitly until the effect runs, allowing consumers to handle the loading state.Alternative approach (explicit undefined)
- return !!isMobile + return isMobileThis would return
false | true | undefined, allowing consumers to distinguish between "checked and is desktop" vs "not yet checked". Update the return type accordingly:export function useIsMobile(): boolean | undefined {apps/web/app/workflow/lib/config.ts (1)
48-61: LGTM!The
getAllCredentialsfunction follows the established patterns in this file with proper error handling and consistent structure.packages/ui/src/components/button.tsx (1)
7-60: LGTM! Well-structured button enhancements.The changes improve the button component:
- New size variants (
icon-sm,icon-lg) provide more granular icon button sizing options- Explicit default values in props destructuring improve code clarity
- Data attributes (
data-variant,data-size) enable better debugging, testing, and CSS targeting- Styling refinements to hover states appear intentional and consistent
All changes follow established patterns and maintain backward compatibility.
apps/web/app/components/providers.tsx (1)
19-22: LGTM! Consistent extension of user state management.The additions of
setEmailandsetUserNamedispatches follow the same pattern as the existingsetUserIddispatch, with appropriate optional chaining and null fallbacks. Removing the debug console.log also cleans up the code.apps/web/app/workflow/page.tsx (1)
13-20: Verify SidebarProvider scope.
SidebarProvidertypically should wrap both the sidebar and main content area to enable proper state sharing (e.g., collapse/expand state, responsive behavior). Currently, it only wrapsAppSidebar, which may limit functionality.Consider this structure instead:
🔎 Suggested restructure
- <div className=" w-auto h-full text-black"> - <SidebarProvider> - <AppSidebar /> - - {/* {children} */} - </SidebarProvider> - </div> - <div className=" flex -col h-[95vh] w-full fitems-center justify-center bg-[#0f0f1a]"> - <CreateWorkFlow/> - </div> + <SidebarProvider> + <div className="w-auto h-full text-black"> + <AppSidebar /> + </div> + <div className="flex-col h-[95vh] w-full items-center justify-center bg-[#0f0f1a]"> + <CreateWorkFlow/> + </div> + </SidebarProvider>packages/ui/src/components/separator.tsx (1)
1-28: LGTM!The
Separatorcomponent is well-implemented and follows the established pattern in the codebase for wrapping Radix UI primitives. The use of sensible defaults (orientation="horizontal",decorative=true), proper prop forwarding, and responsive className logic via data attributes is appropriate.apps/web/store/slices/userSlice.ts (1)
7-8: LGTM!The additions to the
UserSliceinterface and reducers are well-implemented. The newnamefields with their correspondingsetEmailandsetUserNamereducers follow the existing pattern, maintain type consistency (string | null), and integrate properly with theclearUserreducer.Also applies to: 14-15, 28-33
packages/ui/src/components/collapsible.tsx (1)
1-33: LGTM!The
Collapsiblecomponent wrappers are well-implemented and follow the established pattern in the codebase for wrapping Radix UI primitives (as seen in similar components likeSheet,Select, andLabel). All three components (Collapsible,CollapsibleTrigger,CollapsibleContent) properly adddata-slotattributes and use correct TypeScript typing viaReact.ComponentProps.packages/ui/package.json (1)
10-17: Package versions verified and secure.All specified Radix UI packages (@radix-ui/react-collapsible@1.1.12, @radix-ui/react-dialog@1.1.15, @radix-ui/react-dropdown-menu@2.1.16, @radix-ui/react-label@2.1.8, @radix-ui/react-select@2.2.6, @radix-ui/react-separator@1.1.8, @radix-ui/react-slot@1.2.3, and @radix-ui/react-tooltip@1.2.8) exist on the npm registry and have no known security vulnerabilities.
packages/ui/src/components/tooltip.tsx (1)
1-61: LGTM!The Tooltip component implementation follows established patterns in the UI package (data-slot attributes, cn utility for className composition). The animation classes and Portal usage are correctly applied.
apps/web/app/components/nodes/CreateWorkFlow.tsx (3)
343-343: Position calculation may be incorrect for editing existing nodes.
position={existingNodes.length + 1}always calculates position based on total nodes count. When editing an existing node, this should probably use the node's actual position, not append position.Verify that the
positionprop is used correctly inGoogleSheetFormClientand whether it represents the node's ordinal position or is used for new node placement only.
153-220: LGTM on workflow graph rebuilding logic.The guard condition correctly prevents rebuilding when there's no stored data, and the node/edge generation logic properly handles triggers, actions, and placeholder positioning.
344-349: The current implementation is correct and does not need changes.The code properly reads from
selectedNodeConfig.spreadsheetId(lowercase 's') from the backend and assigns it tospreadSheetId(uppercase 'S') as expected byGoogleSheetFormClient. The backend config (peractions.ts) returns the property with lowercase 's', and the component interface (perGoogleSheetFormClient.tsxline 24) expects uppercase 'S' for theinitialDataprop. The mapping on line 348 ofCreateWorkFlow.tsxcorrectly bridges this difference.Changing to
selectedNodeConfig.spreadSheetIdwould result inundefinedbeing passed to the component, breaking initial data population instead of fixing it.apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
29-40: LGTM on state initialization from initialData.The state variables are correctly initialized with values from
initialDatawhen present, with appropriate fallbacks.packages/ui/src/components/dropdown-menu.tsx (1)
1-257: LGTM!The DropdownMenu component set is well-implemented following established UI package patterns:
- Consistent use of
data-slotattributes for styling hooks- Proper className composition with
cnutility- Correct use of Portal for main content
- Appropriate animation and styling classes
The implementation aligns with other Radix UI wrapper components in the package.
packages/ui/src/components/sidebar.tsx (3)
47-54: Good practice: Context with error boundary.The
useSidebarhook correctly throws a descriptive error when used outside aSidebarProvider, making debugging easier.
183-206: Good accessibility: Mobile sidebar uses Sheet with sr-only header.The mobile implementation correctly includes visually hidden but screen-reader accessible title and description via
SheetHeaderwithsr-onlyclass.
1-726: LGTM overall!This is a comprehensive, well-structured sidebar component library with:
- Proper controlled/uncontrolled state management
- Cookie persistence for user preferences
- Keyboard shortcut support (Ctrl/Cmd+B)
- Mobile-responsive design with Sheet fallback
- Tooltip integration for collapsed state
- Extensive composable subcomponents
The implementation follows established patterns and provides good accessibility features.
| await prismaClient.workflow.update({ | ||
| where:{ id: dataSafe.data.WorkflowId }, | ||
| data:{ | ||
| isEmpty: false | ||
| } | ||
| }) |
There was a problem hiding this comment.
Add error handling for workflow update.
The workflow update operation at lines 401-406 lacks error handling. If this update fails after the trigger has been successfully created, it would leave the system in an inconsistent state where:
- The trigger exists in the database
- The workflow's
isEmptyflag remainstrue(incorrect)
This could lead to unexpected behavior in the UI or other parts of the system that rely on the isEmpty flag.
🔎 Proposed fix
})
- await prismaClient.workflow.update({
- where:{ id: dataSafe.data.WorkflowId },
- data:{
- isEmpty: false
- }
- })
+ try {
+ await prismaClient.workflow.update({
+ where:{ id: dataSafe.data.WorkflowId },
+ data:{
+ isEmpty: false
+ }
+ })
+ } catch (updateError) {
+ console.log("Warning: Failed to update workflow isEmpty flag:", updateError);
+ // Note: trigger was created successfully but isEmpty flag may be stale
+ }
if(createdTrigger){📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await prismaClient.workflow.update({ | |
| where:{ id: dataSafe.data.WorkflowId }, | |
| data:{ | |
| isEmpty: false | |
| } | |
| }) | |
| try { | |
| await prismaClient.workflow.update({ | |
| where:{ id: dataSafe.data.WorkflowId }, | |
| data:{ | |
| isEmpty: false | |
| } | |
| }) | |
| } catch (updateError) { | |
| console.log("Warning: Failed to update workflow isEmpty flag:", updateError); | |
| // Note: trigger was created successfully but isEmpty flag may be stale | |
| } |
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/userRoutes/userRoutes.ts around lines 401-406,
the prismaClient.workflow.update call has no error handling which can leave a
created trigger in DB while the workflow.isEmpty stays true; wrap the update in
error handling or (preferably) perform the trigger creation and workflow update
inside a prisma transaction so both succeed or both rollback; if you cannot
convert to a transaction, catch errors from the update, delete the newly created
trigger to rollback the partial change, log the failure with context, and
rethrow or return an appropriate error response.
| console.log('initial data: ', initialData) | ||
| console.log('initial document: ', selectedDocument) | ||
| console.log('initial sheet: ', selectedSheet) | ||
| console.log('initial range: ', range) | ||
| console.log("initial operation: ",operation) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statements.
Multiple debug logs should be removed before merging to production.
- console.log('initial data: ', initialData)
- console.log('initial document: ', selectedDocument)
- console.log('initial sheet: ', selectedSheet)
- console.log('initial range: ', range)
- console.log("initial operation: ",operation)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('initial data: ', initialData) | |
| console.log('initial document: ', selectedDocument) | |
| console.log('initial sheet: ', selectedSheet) | |
| console.log('initial range: ', range) | |
| console.log("initial operation: ",operation) |
🤖 Prompt for AI Agents
In apps/web/app/components/nodes/GoogleSheetFormClient.tsx around lines 42 to
46, remove the five debug console.log statements (console.log('initial data...',
selectedDocument, selectedSheet, range, operation)) so no debug output is left
in production; if you need runtime inspection, replace them with a proper logger
behind a development-only check or use the existing debug/log utility, otherwise
simply delete these console.log lines.
| {creds ? (creds.length === 0 ? | ||
| (<SidebarMenuSubItem key={0}> | ||
| <SidebarMenuSubButton> | ||
| <span>No credentials avaliable</span> |
There was a problem hiding this comment.
Typo in user-facing text.
"avaliable" should be "available".
- <span>No credentials avaliable</span>
+ <span>No credentials available</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span>No credentials avaliable</span> | |
| <span>No credentials available</span> |
🤖 Prompt for AI Agents
In apps/web/app/components/ui/app-sidebar.tsx around line 157, there's a typo in
the user-facing string "No credentials avaliable"; update the text to "No
credentials available" to fix the misspelling, keeping casing and spacing
consistent with surrounding UI copy.
| import { BACKEND_URL } from "@repo/common/zod"; | ||
| import { GoogleSheetsNodeExecutor } from "@repo/nodes"; | ||
| import axios from "axios" | ||
| const date = new Date() |
There was a problem hiding this comment.
Critical: Move date instantiation inside createWorkflow function.
The date constant is declared at the module level (line 4), which means it's evaluated once when the module loads, not when createWorkflow is called. This causes all workflows created in the same session to receive identical timestamps in their names (line 82: workflow-${date.getTime()}), defeating the purpose of using timestamps for uniqueness.
🔎 Proposed fix
-const date = new Date()
export const getAvailableTriggers = async () => {And in createWorkflow:
export const createWorkflow = async()=>{
try{
+ const date = new Date()
const response = await axios.post(`${BACKEND_URL}/user/create/workflow`,
{
Name:`workflow-${date.getTime()}`,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/workflow/lib/config.ts around line 4 (and referenced at line
82), the module-level const date = new Date() causes the same timestamp to be
reused for every workflow; move the Date instantiation inside createWorkflow so
a fresh new Date() (or Date.now()) is created during each call and replace
usages of the module-level date (e.g., workflow-${date.getTime()}) with the
locally constructed timestamp to ensure unique, per-call names.
| export const getAllWorkflows = async()=>{ | ||
| try{ | ||
| const res = await axios.get(`${BACKEND_URL}/user/workflows`,{ | ||
| withCredentials: true | ||
| }) | ||
| const data = res.data.Data | ||
| console.log("workflows: ",data) | ||
| return data | ||
| }catch(e){ | ||
| console.error("Error fetching credentials:", e); | ||
| throw e; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix copy-paste error in error message.
Line 73 logs "Error fetching credentials" but this function is getAllWorkflows. The error message should reference workflows, not credentials.
🔎 Proposed fix
}catch(e){
- console.error("Error fetching credentials:", e);
+ console.error("Error fetching workflows:", e);
throw e;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getAllWorkflows = async()=>{ | |
| try{ | |
| const res = await axios.get(`${BACKEND_URL}/user/workflows`,{ | |
| withCredentials: true | |
| }) | |
| const data = res.data.Data | |
| console.log("workflows: ",data) | |
| return data | |
| }catch(e){ | |
| console.error("Error fetching credentials:", e); | |
| throw e; | |
| } | |
| } | |
| export const getAllWorkflows = async()=>{ | |
| try{ | |
| const res = await axios.get(`${BACKEND_URL}/user/workflows`,{ | |
| withCredentials: true | |
| }) | |
| const data = res.data.Data | |
| console.log("workflows: ",data) | |
| return data | |
| }catch(e){ | |
| console.error("Error fetching workflows:", e); | |
| throw e; | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/workflow/lib/config.ts around lines 64 to 76, the catch block
logs "Error fetching credentials" which is a copy-paste mistake for
getAllWorkflows; change the console.error call to reference workflows (e.g.,
"Error fetching workflows:" or "Error fetching all workflows:") and keep passing
the caught error object so the log shows details, then rethrow the error as
currently implemented.
| @@ -1,15 +1,27 @@ | |||
| "use client"; | |||
|
|
|||
| import { SidebarProvider, SidebarTrigger } from "@workspace/ui/components/sidebar"; | |||
There was a problem hiding this comment.
Remove unused import.
SidebarTrigger is imported but never used in the component.
🔎 Proposed fix
-import { SidebarProvider, SidebarTrigger } from "@workspace/ui/components/sidebar";
+import { SidebarProvider } from "@workspace/ui/components/sidebar";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { SidebarProvider, SidebarTrigger } from "@workspace/ui/components/sidebar"; | |
| import { SidebarProvider } from "@workspace/ui/components/sidebar"; |
🤖 Prompt for AI Agents
In apps/web/app/workflow/page.tsx around line 3, the import statement brings in
SidebarTrigger which is not used; remove SidebarTrigger from the import so only
SidebarProvider is imported (or delete the entire import if SidebarProvider is
also unused), and run a quick search to ensure there are no remaining references
to SidebarTrigger before committing.
| {/* {children} */} | ||
| </SidebarProvider> | ||
| </div> | ||
| <div className=" flex -col h-[95vh] w-full fitems-center justify-center bg-[#0f0f1a]"> |
There was a problem hiding this comment.
Fix CSS class typos.
The className string contains two typos:
fitems-centershould beitems-centerflex -colshould beflex-col
🔎 Proposed fix
- <div className=" flex -col h-[95vh] w-full fitems-center justify-center bg-[#0f0f1a]">
+ <div className="flex flex-col h-[95vh] w-full items-center justify-center bg-[#0f0f1a]">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className=" flex -col h-[95vh] w-full fitems-center justify-center bg-[#0f0f1a]"> | |
| <div className="flex flex-col h-[95vh] w-full items-center justify-center bg-[#0f0f1a]"> |
🤖 Prompt for AI Agents
In apps/web/app/workflow/page.tsx around line 21 the className has typos:
replace "flex -col" with "flex-col" and "fitems-center" with "items-center" so
the final className uses "flex-col items-center justify-center" (keeping the
other utility classes and spacing intact).
| @@ -0,0 +1,13 @@ | |||
| import { cn } from "@workspace/ui/lib/utils" | |||
There was a problem hiding this comment.
Add missing React type import.
The component uses React.ComponentProps<"div"> on line 3, but React types are not imported. This will cause a TypeScript compilation error: "Cannot find namespace 'React'."
🔎 Proposed fix
+import type React from "react"
import { cn } from "@workspace/ui/lib/utils"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { cn } from "@workspace/ui/lib/utils" | |
| import type React from "react" | |
| import { cn } from "@workspace/ui/lib/utils" |
🤖 Prompt for AI Agents
packages/ui/src/components/skeleton.tsx around line 1: the file uses
React.ComponentProps<"div"> but doesn't import React, causing a TS error; add a
React type import at the top (for example a type-only import like `import type *
as React from "react"` or a regular `import React from "react"`) so the React
namespace/types are available for ComponentProps; keep existing imports and
ensure the import is placed before any usage of React types.
Vamsi-o
left a comment
Vamsi-o
left a comment
There was a problem hiding this comment.
Good work , Just move the Date inside to try catch in createWorkflow
TejaBudumuru3
dismissed
Vamsi-o’s stale review
The merge-base changed after approval.
3 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.