Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 73 additions & 3 deletions apps/web/app/components/nodes/CreateWorkFlow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import ActionSideBar from "../Actions/ActionSidebar";
import ActionNode from "../Actions/ActionNode";
import { GoogleSheetFormClient } from "./GoogleSheetFormClient";
import { useDispatch } from "react-redux";
import { workflowActions, workflowReducer } from "@/store/slices/workflowSlice";
import { workflowActions } from "@/store/slices/workflowSlice";
import { createWorkflow, getEmptyWorkflow, getworkflowData } from "@/app/workflow/lib/config";
import { useAppSelector } from '@/app/hooks/redux';

Expand Down Expand Up @@ -42,9 +42,9 @@ export const CreateWorkFlow = () => {
const dispatch = useDispatch();
const userId = useAppSelector(s=>s.user.userId)
const workflowId = useAppSelector(s=>s.workflow.workflow_id)
const existingTrigger = useAppSelector(s=>s.workflow.trigger?.name)
const existingTrigger = useAppSelector(s=>s.workflow.trigger)
const existingNodes = useAppSelector(s=>s.workflow.nodes)
console.log(`workflow from redux, TRigger: ${existingTrigger}, Nodes: ${existingNodes}`)
// console.log(`workflow from redux, TRigger: ${existingTrigger}, Nodes: ${existingNodes}`)

const [nodes, setNodes] = useState<NodeType[]>([
{
Expand Down Expand Up @@ -132,6 +132,76 @@ export const CreateWorkFlow = () => {
getEmptyWorkflowID();
}, [dispatch]);

useEffect(()=>{
// Guard: only rebuild nodes/edges if there's actual stored data
if (existingNodes.length === 0 && !existingTrigger) {
return; // Keep the current placeholder state
}

function loadWorkflow(){
const START_X = 100;
const GAP = 250;
const Y = 200;

const newNodes: NodeType[] = [];
const newEdges: EdgeType[] = [];
const type = existingTrigger?.name.split(" - ")[0]
if(existingTrigger){
newNodes.push({
id: `trigger-${existingTrigger.dbId}`,
type: 'trigger' as const,
position: { x: START_X, y: Y},
data:{
label: existingTrigger.name,
name: existingTrigger.name,
type: type,
icon: '📊',
config: existingTrigger.config,
}
})
}

existingNodes.forEach((node, index)=>{
const nodeId = `action-${node.dbId}-${index}`;
const type = node.name.split(" - ")[0]
newNodes.push({
id: nodeId,
type: 'action' as const,
position: { x: START_X + (index + 1) * GAP, y: Y},
data:{
label: node.name,
name: node.name,
icon: '⚙️',
type: type,
config: node.config,
}
});
});

const placeholderIndex = newNodes.length;
newNodes.push({
id: `placeholder-${Date.now()}`,
type: 'placeholder' as const,
position: { x: START_X + placeholderIndex * GAP, y: Y},
data: { label: '+' }
});

for(let i=0; i<newNodes.length - 1; i++){
newEdges.push({
id: `edge-${i}`,
source: newNodes[i]?.id || '',
target: newNodes[i+1]?.id || ''
});
}

setNodes(newNodes);
setEdges(newEdges)
}

loadWorkflow()
},[existingNodes, existingTrigger])
Comment on lines +135 to +202

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Previous review feedback not addressed: Stop deriving type from name.

The previous review (lines 152-219) specifically requested using existingTrigger.type and node.type properties from Redux instead of extracting from name.split(" - ")[0]. Lines 165, 174, 183, and 192 still use the fragile name-splitting approach, which will break if the naming convention changes.

Additionally, Line 165 executes outside the if(existingTrigger) check on line 166. Given the guard on line 154 only returns when BOTH are empty, if existingNodes has items but existingTrigger is null, line 165 will set type to undefined, which then gets assigned on line 174.

🔎 Proposed fix using Redux type properties
   function loadWorkflow(){
     const START_X = 100;
     const GAP = 250;
     const Y = 200;

     const newNodes: NodeType[] = [];
     const newEdges: EdgeType[] = [];
-    const type = existingTrigger?.name.split(" - ")[0]
     if(existingTrigger){
       newNodes.push({
         id: `trigger-${existingTrigger.dbId}`,
         type: 'trigger' as const,
         position: { x: START_X, y: Y},
         data:{
           label: existingTrigger.name,
           name: existingTrigger.name,
-          type: type,
+          type: existingTrigger.type,
           icon: '📊',
           config: existingTrigger.config,
         }
       })
     }

     existingNodes.forEach((node, index)=>{
       const nodeId = `action-${node.dbId}-${index}`;
-      const type = node.name.split(" - ")[0]
       newNodes.push({
         id: nodeId,
         type: 'action' as const,
         position: { x: START_X + (index + 1) * GAP, y: Y},
         data:{
           label: node.name,
           name: node.name,
           icon: '⚙️',
-          type: type,
+          type: node.type,
           config: node.config,
         }
       });
     });
📝 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.

Suggested change
useEffect(()=>{
// Guard: only rebuild nodes/edges if there's actual stored data
if (existingNodes.length === 0 && !existingTrigger) {
return; // Keep the current placeholder state
}
function loadWorkflow(){
const START_X = 100;
const GAP = 250;
const Y = 200;
const newNodes: NodeType[] = [];
const newEdges: EdgeType[] = [];
const type = existingTrigger?.name.split(" - ")[0]
if(existingTrigger){
newNodes.push({
id: `trigger-${existingTrigger.dbId}`,
type: 'trigger' as const,
position: { x: START_X, y: Y},
data:{
label: existingTrigger.name,
name: existingTrigger.name,
type: type,
icon: '📊',
config: existingTrigger.config,
}
})
}
existingNodes.forEach((node, index)=>{
const nodeId = `action-${node.dbId}-${index}`;
const type = node.name.split(" - ")[0]
newNodes.push({
id: nodeId,
type: 'action' as const,
position: { x: START_X + (index + 1) * GAP, y: Y},
data:{
label: node.name,
name: node.name,
icon: '⚙️',
type: type,
config: node.config,
}
});
});
const placeholderIndex = newNodes.length;
newNodes.push({
id: `placeholder-${Date.now()}`,
type: 'placeholder' as const,
position: { x: START_X + placeholderIndex * GAP, y: Y},
data: { label: '+' }
});
for(let i=0; i<newNodes.length - 1; i++){
newEdges.push({
id: `edge-${i}`,
source: newNodes[i]?.id || '',
target: newNodes[i+1]?.id || ''
});
}
setNodes(newNodes);
setEdges(newEdges)
}
loadWorkflow()
},[existingNodes, existingTrigger])
useEffect(()=>{
// Guard: only rebuild nodes/edges if there's actual stored data
if (existingNodes.length === 0 && !existingTrigger) {
return; // Keep the current placeholder state
}
function loadWorkflow(){
const START_X = 100;
const GAP = 250;
const Y = 200;
const newNodes: NodeType[] = [];
const newEdges: EdgeType[] = [];
if(existingTrigger){
newNodes.push({
id: `trigger-${existingTrigger.dbId}`,
type: 'trigger' as const,
position: { x: START_X, y: Y},
data:{
label: existingTrigger.name,
name: existingTrigger.name,
type: existingTrigger.type,
icon: '📊',
config: existingTrigger.config,
}
})
}
existingNodes.forEach((node, index)=>{
const nodeId = `action-${node.dbId}-${index}`;
newNodes.push({
id: nodeId,
type: 'action' as const,
position: { x: START_X + (index + 1) * GAP, y: Y},
data:{
label: node.name,
name: node.name,
icon: '⚙️',
type: node.type,
config: node.config,
}
});
});
const placeholderIndex = newNodes.length;
newNodes.push({
id: `placeholder-${Date.now()}`,
type: 'placeholder' as const,
position: { x: START_X + placeholderIndex * GAP, y: Y},
data: { label: '+' }
});
for(let i=0; i<newNodes.length - 1; i++){
newEdges.push({
id: `edge-${i}`,
source: newNodes[i]?.id || '',
target: newNodes[i+1]?.id || ''
});
}
setNodes(newNodes);
setEdges(newEdges)
}
loadWorkflow()
},[existingNodes, existingTrigger])
🤖 Prompt for AI Agents
In apps/web/app/components/nodes/CreateWorkFlow.tsx around lines 152 to 219, the
code still derives node types by splitting the name (lines ~165,174,183,192) and
declares the trigger type outside the existingTrigger guard which can produce
undefined; replace uses of name.split(" - ")[0] with the authoritative
existingTrigger.type and node.type values from Redux, move the trigger type
declaration inside the if(existingTrigger) block so it’s only read when
existingTrigger exists, and ensure each newNodes.data.type is assigned from
those Redux properties (existingTrigger.type for the trigger node and node.type
for action nodes) instead of parsing the name.



const handleSelectAction = (action: {
id: string;
name: string;
Expand Down
100 changes: 80 additions & 20 deletions apps/web/app/components/nodes/GoogleSheetFormClient.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@
import { Button } from '@workspace/ui/components/button';
import { Input } from '@workspace/ui/components/input';
import { Label } from '@workspace/ui/components/label';
import React, { useState, useTransition } from 'react';
import React, { useEffect, useState, useTransition } from 'react';
import Link from 'next/link';
import { toast } from 'sonner';
import { handleSaveConfig } from './actions';
Expand All @@ -13,42 +13,102 @@ import { BACKEND_URL } from '@repo/common/zod';
import { useAppSelector } from '@/app/hooks/redux';

interface GoogleSheetFormClientProps {
initialData: {
credentials: Array<{ id: string }>;
authUrl?: string;
hasCredentials: boolean;
type: string;
nodeType: string;
initialData?: {
range?: string;
operation?: string;
sheetName?: string;
spreadSheetId?: string;
credentialId?: string;
};
userId: string;
nodeId: string;
}

export function GoogleSheetFormClient(type:{type: string, nodeType: string}) {
const [selectedCredential, setSelectedCredential] = useState<string>('');
export function GoogleSheetFormClient({ type, nodeType, initialData }: GoogleSheetFormClientProps) {
const [selectedCredential, setSelectedCredential] = useState<string>(initialData?.credentialId || '');
const [documents, setDocuments] = useState<Array<{ id: string; name: string }>>([]);
const [selectedDocument, setSelectedDocument] = useState<string>('');
const [selectedDocument, setSelectedDocument] = useState<string>(initialData?.spreadSheetId || '');
const [sheets, setSheets] = useState<Array<{ id: number; name: string }>>([]);
const [selectedSheet, setSelectedSheet] = useState<string>('');
const [operation, setOperation] = useState<string>('read_rows');
const [range, setRange] = useState<string>('A1:Z100');
const [selectedSheet, setSelectedSheet] = useState<string>(initialData?.sheetName || '');
const [operation, setOperation] = useState<string>(initialData?.operation || 'read_rows');
const [range, setRange] = useState<string>(initialData?.range || 'A1:Z100');
const [loading, setLoading] = useState(false);
const [isPending, startTransition] = useTransition();
const [result, setResult] = useState<any>(null);
const [credId, setCredId] = useState<string>();
const [credId, setCredId] = useState<string>(initialData?.credentialId || '');
// const [authUrl, setAuthUrl] = useState<string>()
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)

const userId = useAppSelector(s=>s.user.userId) || ""
const workflowId = useAppSelector(s=>s.workflow.workflow_id) || ''
console.log(userId, 'id from client')
const credType = type.type
const nodeType = type.nodeType.split("~")[0] || ""
console.log('checking nodeType: ', nodeType);
const credType = type
const nodeTypeParsed = nodeType.split("~")[0] || ""
console.log('checking nodeType: ', nodeTypeParsed);

const nodeId = type.nodeType.split("~")[1] || ""
const nodeId = nodeType.split("~")[1] || ""
console.log('checking node id: ',nodeId)
const {cred: response, authUrl} = useCredentials(credType)
console.log('response from form client', typeof(response))

console.log(response," response from client after hook")
console.log(authUrl," authurl")

// Fetch documents when there's initial credentialId
useEffect(() => {
const fetchInitialDocuments = async () => {
if (!initialData?.credentialId) return;

setLoading(true);
try {
const res = await fetch(`${BACKEND_URL}/node/getDocuments/${initialData.credentialId}`, {
method: 'GET',
credentials: "include",
headers: { 'Content-Type': 'application/json' },
});
const data = await res.json();
if (data.files?.length > 0) {
setDocuments(data.files);
}
} catch (error) {
console.error('Failed to fetch initial documents:', error);
} finally {
setLoading(false);
}
};

fetchInitialDocuments();
}, [initialData?.credentialId]);
Comment on lines +62 to +85

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add HTTP response status check to prevent silent failures.

The fetch does not check res.ok before parsing JSON. If the server returns a 4xx/5xx error, res.json() may fail or return unexpected data, causing silent failures or misleading UI states.

🔎 Proposed fix
       const res = await fetch(`${BACKEND_URL}/node/getDocuments/${initialData.credentialId}`, {
         method: 'GET',
         credentials: "include",
         headers: { 'Content-Type': 'application/json' },
       });
+      if (!res.ok) {
+        throw new Error(`Failed to fetch documents: ${res.status}`);
+      }
       const data = await res.json();
📝 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.

Suggested change
useEffect(() => {
const fetchInitialDocuments = async () => {
if (!initialData?.credentialId) return;
setLoading(true);
try {
const res = await fetch(`${BACKEND_URL}/node/getDocuments/${initialData.credentialId}`, {
method: 'GET',
credentials: "include",
headers: { 'Content-Type': 'application/json' },
});
const data = await res.json();
if (data.files?.length > 0) {
setDocuments(data.files);
}
} catch (error) {
console.error('Failed to fetch initial documents:', error);
} finally {
setLoading(false);
}
};
fetchInitialDocuments();
}, [initialData?.credentialId]);
useEffect(() => {
const fetchInitialDocuments = async () => {
if (!initialData?.credentialId) return;
setLoading(true);
try {
const res = await fetch(`${BACKEND_URL}/node/getDocuments/${initialData.credentialId}`, {
method: 'GET',
credentials: "include",
headers: { 'Content-Type': 'application/json' },
});
if (!res.ok) {
throw new Error(`Failed to fetch documents: ${res.status}`);
}
const data = await res.json();
if (data.files?.length > 0) {
setDocuments(data.files);
}
} catch (error) {
console.error('Failed to fetch initial documents:', error);
} finally {
setLoading(false);
}
};
fetchInitialDocuments();
}, [initialData?.credentialId]);
🤖 Prompt for AI Agents
In apps/web/app/components/nodes/GoogleSheetFormClient.tsx around lines 62 to
85, the fetch call parses res.json() without checking HTTP status which can
cause silent failures on 4xx/5xx responses; update the code to inspect res.ok
after the fetch, and if not ok read any error body (await res.text() or
res.json() guarded) and log or surface that error (and avoid calling
setDocuments), otherwise parse and use the JSON; ensure loading is cleared in
error and non-ok branches and return early on non-ok to prevent using invalid
data.


// Fetch sheets when there's initial spreadSheetId
useEffect(() => {
const fetchInitialSheets = async () => {
if (!initialData?.credentialId || !initialData?.spreadSheetId) return;

setLoading(true);
try {
const res = await fetch(`${BACKEND_URL}/node/getSheets/${initialData.credentialId}/${initialData.spreadSheetId}`, {
method: 'GET',
headers: { 'Content-Type': 'application/json' },
credentials: "include",
});
const data = await res.json();
if (data.files?.data?.length > 0) {
setSheets(data.files.data);
}
} catch (error) {
console.error('Failed to fetch initial sheets:', error);
} finally {
setLoading(false);
}
};

fetchInitialSheets();
}, [initialData?.credentialId, initialData?.spreadSheetId]);
Comment on lines +88 to +111

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add HTTP response status check (same issue as above).

Same concern applies here—missing res.ok check before parsing. Also note the response structure differs here (data.files?.data?.length) versus the documents fetch (data.files?.length), which could indicate inconsistent API responses or a bug in one of the checks.

🔎 Proposed fix
       const res = await fetch(`${BACKEND_URL}/node/getSheets/${initialData.credentialId}/${initialData.spreadSheetId}`, {
         method: 'GET',
         headers: { 'Content-Type': 'application/json' },
         credentials: "include",
       });
+      if (!res.ok) {
+        throw new Error(`Failed to fetch sheets: ${res.status}`);
+      }
       const data = await res.json();
📝 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.

Suggested change
useEffect(() => {
const fetchInitialSheets = async () => {
if (!initialData?.credentialId || !initialData?.spreadSheetId) return;
setLoading(true);
try {
const res = await fetch(`${BACKEND_URL}/node/getSheets/${initialData.credentialId}/${initialData.spreadSheetId}`, {
method: 'GET',
headers: { 'Content-Type': 'application/json' },
credentials: "include",
});
const data = await res.json();
if (data.files?.data?.length > 0) {
setSheets(data.files.data);
}
} catch (error) {
console.error('Failed to fetch initial sheets:', error);
} finally {
setLoading(false);
}
};
fetchInitialSheets();
}, [initialData?.credentialId, initialData?.spreadSheetId]);
useEffect(() => {
const fetchInitialSheets = async () => {
if (!initialData?.credentialId || !initialData?.spreadSheetId) return;
setLoading(true);
try {
const res = await fetch(`${BACKEND_URL}/node/getSheets/${initialData.credentialId}/${initialData.spreadSheetId}`, {
method: 'GET',
headers: { 'Content-Type': 'application/json' },
credentials: "include",
});
if (!res.ok) {
throw new Error(`Failed to fetch sheets: ${res.status}`);
}
const data = await res.json();
if (data.files?.data?.length > 0) {
setSheets(data.files.data);
}
} catch (error) {
console.error('Failed to fetch initial sheets:', error);
} finally {
setLoading(false);
}
};
fetchInitialSheets();
}, [initialData?.credentialId, initialData?.spreadSheetId]);
🤖 Prompt for AI Agents
In apps/web/app/components/nodes/GoogleSheetFormClient.tsx around lines 88 to
111, the effect is parsing the fetch response without checking res.ok and
assumes a specific response shape (data.files?.data?.length) which differs from
other fetches; update the code to first check if (!res.ok) and handle/log the
non-2xx response (optionally parse error body) and return early, then call
res.json() only for successful responses, and normalize the payload when setting
sheets to accept both shapes (e.g., prefer data.files.data if present, otherwise
data.files) before calling setSheets; ensure setLoading is still toggled in
finally.


const openAuthWindow = (url: string) => {
if (!url) return;
Expand Down Expand Up @@ -129,8 +189,8 @@ export function GoogleSheetFormClient(type:{type: string, nodeType: string}) {
userId:userId,
node_Trigger:nodeId,
workflowId,
type:nodeType,
name: `Google sheet - ${nodeType}`,
type:nodeTypeParsed,
name: `Google sheet - ${nodeTypeParsed}`,
credentialId: selectedCredential,
spreadsheetId: selectedDocument,
sheetName: selectedSheet,
Expand Down