feat: update Google OAuth routes and enhance Google Sheets integration#60
feat: update Google OAuth routes and enhance Google Sheets integration#60
Conversation
📝 WalkthroughWalkthroughThis PR introduces a Google Sheets integration feature with dynamic field option loading. Changes span backend route configuration, new API endpoints for Google document/sheet retrieval, updated node configuration with credential type renaming, type system extensions, and enhanced ConfigModal component logic to support dependent field options loading. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/workflows/[id]/components/ConfigModal.tsx (2)
26-68: ResetdynamicOptionswhen node changes to avoid stale dropdowns.
Right now onlyconfigis reset in theselectedNodeeffect. If the user switches nodes,dynamicOptionsfrom the previous node can leak into the new form and show wrong options.🛠️ Suggested fix
useEffect(() => { setConfig({}); + setDynamicOptions({}); // We no longer set local credentials here; handled by useCredentials! }, [selectedNode]);
183-205: Fix inverted field type handling —field.type === "text"currently renders a<textarea>, whilefield.type === "textarea"renders invalid<input type="textarea">.The code branches incorrectly: when
field.type === "text", it renders a<textarea>element; whenfield.type === "textarea", it falls through to the default case and renders<input type={field.type} />, producing invalid HTML.This is confirmed by actual usage in
apps/web/app/lib/nodeConfigs/gmail.action.tsline 41 where a field is configured astype: "textarea"for the email body. Either swap the conditions, rename the field types to match their rendering, or add explicit handling for the"textarea"type.
🤖 Fix all issues with AI agents
In `@apps/http-backend/src/index.ts`:
- Line 36: Remove the inline debug comment from the app.use('/auth/google',
googleAuth) line and ensure the route migration is complete: update any frontend
references to the old '/oauth/google' prefix to the new '/auth/google' prefix.
Specifically, change occurrences of '/oauth/google/initiate' in the
useCredential hook (useCredential) to '/auth/google/initiate' and change
'/oauth/google/callback' in the google-oauth-service module
(google-oauth-service) to '/auth/google/callback' so the OAuth flow matches the
backend's app.use('/auth/google', googleAuth) route.
In `@apps/http-backend/tsconfig.tsbuildinfo`:
- Line 1: The repo is tracking TypeScript build cache files like
apps/http-backend/tsconfig.tsbuildinfo because .gitignore entries are malformed;
update .gitignore to add a single glob rule **/*.tsbuildinfo (replacing the
broken entries such as apps/http-backend/tsbuildinfo) and then remove
already-tracked tsbuildinfo files from git with git rm --cached <path> (or git
rm --cached **/*.tsbuildinfo) and commit the change so files like
tsconfig.tsbuildinfo are no longer tracked.
In `@apps/web/app/lib/nodeConfigs/googleSheet.action.ts`:
- Around line 49-55: The Range field object uses inconsistent casing and keys;
change the object so its internal key is camelCase name: "range", its human
label is label: "Range" (capitalized), and replace value: "A1:Z100" with
defaultValue: "A1:Z100" to match the pattern used by other fields (e.g.,
credentialId, spreadsheetId, sheetName, and action) and keep naming/defaults
consistent in googleSheet.action.ts.
In `@apps/web/app/workflow/lib/config.ts`:
- Around line 39-40: The snippet declares an unused const Data (const Data =
JSON.stringify(response.data.Data)) but returns response.data.data (lowercase),
so remove the unused variable and normalize the response field access; update
the function to return the correct payload by using a single source like
response.data.Data if backend uses uppercase, or safely handle both casings
(e.g., prefer response.data.Data || response.data.data) to avoid breakage. Check
and align other functions (getAvailableTriggers, getAllWorkflows,
getEmptyWorkflow) to the same casing or apply the same tolerant access pattern
so all endpoints are handled consistently.
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx:
- Around line 35-53: handleFieldChange currently awaits fetchFn(updatedConfig)
without error handling; wrap the fetch call in a try/catch around the await
fetchFn(updatedConfig) (inside the loop over dependentFields) to catch network
or runtime failures, log the error (include fetchFn and depField.name), and set
a safe fallback into setDynamicOptions (e.g., empty array or previous options)
so the UI doesn’t break; ensure fetchFn is obtained from fetchOptionsMap and
continue processing other dependent fields even if one fetch fails.
🧹 Nitpick comments (2)
apps/web/app/lib/types/node.types.ts (1)
31-32: Use consistent punctuation and clarify the purpose ofvalue.Two issues:
- Lines 31-32 use commas while the rest of the interface uses semicolons - this should be consistent.
- The
valuefield's purpose is unclear given thatdefaultValuealready exists on line 29. Consider documenting howvaluediffers fromdefaultValue, or consolidate if they serve the same purpose.Suggested fix for consistency
- fetchOptions?: string, - value? : string, + fetchOptions?: string; // API method name to fetch dynamic options (e.g., "google.getDocuments") + value?: string; // Current field value (if different from defaultValue)apps/web/app/lib/api.ts (1)
93-110: Remove debug logging and fix parameter naming inconsistency.
Remove debug log: Line 100 contains a
console.logthat should be removed from production code.Inconsistent parameter naming:
CredentialIduses PascalCase whiledocumentIduses camelCase. Use consistent camelCase for all parameters.Inconsistent return structure:
getDocumentsreturnsdata.data.fileswhilegetSheetsreturnsdata.data.files.data. This asymmetry may confuse consumers. Consider documenting or normalizing the response structure.Suggested fix
google: { - getDocuments: async (CredentialId : string) => { - const data = await axios.get(`${BACKEND_URL}/node/getDocuments/${CredentialId}`,{ + getDocuments: async (credentialId: string) => { + const response = await axios.get(`${BACKEND_URL}/node/getDocuments/${credentialId}`, { withCredentials: true, - headers: {"Content-Type" : "application/json"}, + headers: { "Content-Type": "application/json" }, }) - - console.log(data.data.files) - return data.data.files + return response.data.files }, - getSheets: async (documentId: string, CredentialId: string) => { - const data = await axios.get(`${BACKEND_URL}/node/getSheets/${CredentialId}/${documentId}`,{ + getSheets: async (documentId: string, credentialId: string) => { + const response = await axios.get(`${BACKEND_URL}/node/getSheets/${credentialId}/${documentId}`, { withCredentials: true, - headers: {"Content-Type":"application/json"} + headers: { "Content-Type": "application/json" }, }) - return data.data.files.data + return response.data.files.data }, }
| app.use("/user" , userRouter) | ||
| app.use('/node', sheetRouter) | ||
| app.use('/oauth/google', googleAuth) // ← CHANGED THIS LINE! | ||
| app.use('/auth/google', googleAuth) // ← CHANGED THIS LINE! |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the actual state of the file at line 36
head -40 apps/http-backend/src/index.ts | tail -10Repository: Dev-Pross/BuildFlow
Length of output: 283
🏁 Script executed:
# Search for references to the old OAuth route path
rg -n '/oauth/google' --type ts --type tsx --type js --type json 2>/dev/nullRepository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
# Search for references to the new OAuth route path to understand scope
rg -n '/auth/google' --type ts --type tsx --type js --type json 2>/dev/nullRepository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
# Check if there are any client-side files that might reference OAuth endpoints
fd -e ts -e tsx -e js | xargs grep -l -i 'oauth\|google.*auth' 2>/dev/null | head -20Repository: Dev-Pross/BuildFlow
Length of output: 960
🏁 Script executed:
# Check the useCredential hook for OAuth endpoint references
cat -n apps/web/app/hooks/useCredential.tsRepository: Dev-Pross/BuildFlow
Length of output: 1948
🏁 Script executed:
# Check the google-oauth-service for endpoint configuration
cat -n packages/nodes/src/common/google-oauth-service.tsRepository: Dev-Pross/BuildFlow
Length of output: 6420
🏁 Script executed:
# Check the google_callback routes to see what endpoints are defined
cat -n apps/http-backend/src/routes/google_callback.tsRepository: Dev-Pross/BuildFlow
Length of output: 11196
Remove debug comment and fix incomplete route migration.
The inline comment // ← CHANGED THIS LINE! must be removed before merging.
More critically, the route path change from /oauth/google to /auth/google is incomplete. Frontend code still references the old paths and will fail:
apps/web/app/hooks/useCredential.ts(line 31) references/oauth/google/initiatepackages/nodes/src/common/google-oauth-service.ts(line 19) references/oauth/google/callback
Both frontend files must be updated to use /auth/google/initiate and /auth/google/callback respectively, or the OAuth flow will break.
🤖 Prompt for AI Agents
In `@apps/http-backend/src/index.ts` at line 36, Remove the inline debug comment
from the app.use('/auth/google', googleAuth) line and ensure the route migration
is complete: update any frontend references to the old '/oauth/google' prefix to
the new '/auth/google' prefix. Specifically, change occurrences of
'/oauth/google/initiate' in the useCredential hook (useCredential) to
'/auth/google/initiate' and change '/oauth/google/callback' in the
google-oauth-service module (google-oauth-service) to '/auth/google/callback' so
the OAuth flow matches the backend's app.use('/auth/google', googleAuth) route.
| @@ -1 +1 @@ | |||
| {"root":["./src/index.ts","./src/routes/google_callback.ts","./src/routes/nodes.routes.ts","./src/routes/userRoutes/userMiddleware.ts","./src/routes/userRoutes/userRoutes.ts","./src/scheduler/token-scheduler.ts","./src/services/token-refresh.service.ts"],"version":"5.7.3"} No newline at end of file | |||
| {"root":["./src/index.ts","./src/routes/google_callback.ts","./src/routes/nodes.routes.ts","./src/routes/userroutes/usermiddleware.ts","./src/routes/userroutes/userroutes.ts","./src/scheduler/token-scheduler.ts","./src/services/token-refresh.service.ts"],"version":"5.7.3"} No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if tsbuildinfo is in gitignore
echo "=== Checking .gitignore for tsbuildinfo ==="
if [ -f .gitignore ]; then
cat .gitignore | grep -i 'tsbuild' && echo "Found tsbuildinfo reference" || echo "No tsbuildinfo reference found"
else
echo ".gitignore not found"
fi
echo ""
echo "=== Full .gitignore content ==="
cat .gitignore 2>/dev/null | head -50
echo ""
echo "=== Checking for *.tsbuildinfo files in repo ==="
find . -name "*.tsbuildinfo" -type f 2>/dev/null | head -20
echo ""
echo "=== Git status for tsbuildinfo files ==="
git ls-files | grep tsbuildinfo || echo "No tsbuildinfo files tracked in git"Repository: Dev-Pross/BuildFlow
Length of output: 1677
Fix .gitignore to properly exclude all tsconfig.tsbuildinfo files from version control.
The .gitignore has incomplete and malformed entries for .tsbuildinfo files. While some specific entries exist, they don't cover all packages and some are incorrectly formatted (e.g., apps/http-backend/tsbuildinfo is missing the tsconfig. prefix). Additionally, these auto-generated build cache files are currently tracked in git.
Replace scattered entries with a single glob pattern **/*.tsbuildinfo to exclude all TypeScript build info files across the entire monorepo, then remove the tracked files from git history using git rm --cached.
🤖 Prompt for AI Agents
In `@apps/http-backend/tsconfig.tsbuildinfo` at line 1, The repo is tracking
TypeScript build cache files like apps/http-backend/tsconfig.tsbuildinfo because
.gitignore entries are malformed; update .gitignore to add a single glob rule
**/*.tsbuildinfo (replacing the broken entries such as
apps/http-backend/tsbuildinfo) and then remove already-tracked tsbuildinfo files
from git with git rm --cached <path> (or git rm --cached **/*.tsbuildinfo) and
commit the change so files like tsconfig.tsbuildinfo are no longer tracked.
| { | ||
| name: "Range", | ||
| type: "text", | ||
| label: "range", | ||
| value: "A1:Z100", | ||
| required: true | ||
| } |
There was a problem hiding this comment.
Fix naming conventions and use defaultValue consistently.
The new Range field has several inconsistencies:
-
Field name casing:
name: "Range"uses PascalCase while all other fields use camelCase (e.g.,credentialId,spreadsheetId,sheetName). -
Label/name appear swapped: Typically
nameis the internal key (lowercase) andlabelis the human-readable display text (capitalized). Here it's reversed. -
Use
defaultValueinstead ofvalue: Theactionfield usesdefaultValueon line 46. For consistency, this field should also usedefaultValue.
Suggested fix
{
- name: "Range",
+ name: "range",
type: "text",
- label: "range",
- value: "A1:Z100",
+ label: "Range",
+ defaultValue: "A1:Z100",
required: true
}📝 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.
| { | |
| name: "Range", | |
| type: "text", | |
| label: "range", | |
| value: "A1:Z100", | |
| required: true | |
| } | |
| { | |
| name: "range", | |
| type: "text", | |
| label: "Range", | |
| defaultValue: "A1:Z100", | |
| required: true | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/app/lib/nodeConfigs/googleSheet.action.ts` around lines 49 - 55, The
Range field object uses inconsistent casing and keys; change the object so its
internal key is camelCase name: "range", its human label is label: "Range"
(capitalized), and replace value: "A1:Z100" with defaultValue: "A1:Z100" to
match the pattern used by other fields (e.g., credentialId, spreadsheetId,
sheetName, and action) and keep naming/defaults consistent in
googleSheet.action.ts.
| const Data = JSON.stringify(response.data.Data); | ||
| return response.data.Data; | ||
| return response.data.data; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file around lines 39-40 with context
cat -n apps/web/app/workflow/lib/config.ts | sed -n '30,50p'Repository: Dev-Pross/BuildFlow
Length of output: 707
🏁 Script executed:
# Search for all usages of response.data.Data and response.data.data in the file
rg -n 'response\.data\.[Dd]ata' apps/web/app/workflow/lib/config.tsRepository: Dev-Pross/BuildFlow
Length of output: 388
🏁 Script executed:
# Check if the Data variable on line 39 is used anywhere after its declaration
rg -n '\bData\b' apps/web/app/workflow/lib/config.ts | head -20Repository: Dev-Pross/BuildFlow
Length of output: 595
🏁 Script executed:
# Get full context of the function containing lines 39-40
sed -n '25,55p' apps/web/app/workflow/lib/config.tsRepository: Dev-Pross/BuildFlow
Length of output: 748
Remove unused variable and verify response field casing consistency.
The Data variable on line 39 is unused after the return statement was changed to use response.data.data. This should be removed.
Additionally, this function now uses response.data.data (lowercase) while other functions in this file use response.data.Data (uppercase) — e.g., getAvailableTriggers (lines 13, 16, 18, 19), getAllWorkflows (line 69), getEmptyWorkflow (lines 261, 275, 278). Verify whether the backend response structure is consistent, or if different endpoints use different casing.
Suggested fix
console.log("response from config: ",response);
-
- const Data = JSON.stringify(response.data.Data);
return response.data.data;📝 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.
| const Data = JSON.stringify(response.data.Data); | |
| return response.data.Data; | |
| return response.data.data; | |
| return response.data.data; |
🤖 Prompt for AI Agents
In `@apps/web/app/workflow/lib/config.ts` around lines 39 - 40, The snippet
declares an unused const Data (const Data = JSON.stringify(response.data.Data))
but returns response.data.data (lowercase), so remove the unused variable and
normalize the response field access; update the function to return the correct
payload by using a single source like response.data.Data if backend uses
uppercase, or safely handle both casings (e.g., prefer response.data.Data ||
response.data.data) to avoid breakage. Check and align other functions
(getAvailableTriggers, getAllWorkflows, getEmptyWorkflow) to the same casing or
apply the same tolerant access pattern so all endpoints are handled
consistently.
| const handleFieldChange = async (fieldName: string, value: string, nodeConfig: any) => { | ||
| // Update config with new value | ||
| const updatedConfig = ({ ...config, [fieldName]: value }) | ||
| console.log(fieldName, " ", value, " ", nodeConfig) | ||
| console.log(config, "from handle field function - 1") | ||
| setConfig((prev) => ({ ...prev, [fieldName]: value })); | ||
| console.log(config, "from handle field fun - 2") | ||
| console.log({ ...config, [fieldName]: value }, "what we're setting") | ||
| // Find fields that depend on this field | ||
| const dependentFields = nodeConfig.fields.filter((f:any) => f.dependsOn === fieldName); | ||
|
|
||
| for (const depField of dependentFields) { | ||
| const fetchFn = depField.fetchOptions ? fetchOptionsMap[depField.fetchOptions] : undefined; | ||
| console.log(fetchFn, "fecth FN") | ||
| if (fetchFn) { | ||
| const options = await fetchFn(updatedConfig); | ||
| // console.log(({ ...config, [depField.name]: options }), "optiops setting") | ||
| setDynamicOptions((prev) => ({ ...prev, [depField.name]: options })); | ||
| } |
There was a problem hiding this comment.
Handle option fetch failures to prevent unhandled rejections.
handleFieldChange awaits fetchFn without try/catch; a network failure will reject and leave the UI in a broken state without feedback.
🛠️ Suggested fix
for (const depField of dependentFields) {
const fetchFn = depField.fetchOptions ? fetchOptionsMap[depField.fetchOptions] : undefined;
console.log(fetchFn, "fecth FN")
if (fetchFn) {
- const options = await fetchFn(updatedConfig);
- // console.log(({ ...config, [depField.name]: options }), "optiops setting")
- setDynamicOptions((prev) => ({ ...prev, [depField.name]: options }));
+ try {
+ const options = await fetchFn(updatedConfig);
+ setDynamicOptions((prev) => ({ ...prev, [depField.name]: options }));
+ } catch (err) {
+ console.error("Failed to load dependent options", err);
+ toast.error(`Failed to load ${depField.label ?? depField.name} options`);
+ setDynamicOptions((prev) => ({ ...prev, [depField.name]: [] }));
+ }
}
}📝 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.
| const handleFieldChange = async (fieldName: string, value: string, nodeConfig: any) => { | |
| // Update config with new value | |
| const updatedConfig = ({ ...config, [fieldName]: value }) | |
| console.log(fieldName, " ", value, " ", nodeConfig) | |
| console.log(config, "from handle field function - 1") | |
| setConfig((prev) => ({ ...prev, [fieldName]: value })); | |
| console.log(config, "from handle field fun - 2") | |
| console.log({ ...config, [fieldName]: value }, "what we're setting") | |
| // Find fields that depend on this field | |
| const dependentFields = nodeConfig.fields.filter((f:any) => f.dependsOn === fieldName); | |
| for (const depField of dependentFields) { | |
| const fetchFn = depField.fetchOptions ? fetchOptionsMap[depField.fetchOptions] : undefined; | |
| console.log(fetchFn, "fecth FN") | |
| if (fetchFn) { | |
| const options = await fetchFn(updatedConfig); | |
| // console.log(({ ...config, [depField.name]: options }), "optiops setting") | |
| setDynamicOptions((prev) => ({ ...prev, [depField.name]: options })); | |
| } | |
| const handleFieldChange = async (fieldName: string, value: string, nodeConfig: any) => { | |
| // Update config with new value | |
| const updatedConfig = ({ ...config, [fieldName]: value }) | |
| console.log(fieldName, " ", value, " ", nodeConfig) | |
| console.log(config, "from handle field function - 1") | |
| setConfig((prev) => ({ ...prev, [fieldName]: value })); | |
| console.log(config, "from handle field fun - 2") | |
| console.log({ ...config, [fieldName]: value }, "what we're setting") | |
| // Find fields that depend on this field | |
| const dependentFields = nodeConfig.fields.filter((f:any) => f.dependsOn === fieldName); | |
| for (const depField of dependentFields) { | |
| const fetchFn = depField.fetchOptions ? fetchOptionsMap[depField.fetchOptions] : undefined; | |
| console.log(fetchFn, "fecth FN") | |
| if (fetchFn) { | |
| try { | |
| const options = await fetchFn(updatedConfig); | |
| setDynamicOptions((prev) => ({ ...prev, [depField.name]: options })); | |
| } catch (err) { | |
| console.error("Failed to load dependent options", err); | |
| toast.error(`Failed to load ${depField.label ?? depField.name} options`); | |
| setDynamicOptions((prev) => ({ ...prev, [depField.name]: [] })); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 35 - 53,
handleFieldChange currently awaits fetchFn(updatedConfig) without error
handling; wrap the fetch call in a try/catch around the await
fetchFn(updatedConfig) (inside the loop over dependentFields) to catch network
or runtime failures, log the error (include fetchFn and depField.name), and set
a safe fallback into setDynamicOptions (e.g., empty array or previous options)
so the UI doesn’t break; ensure fetchFn is obtained from fetchOptionsMap and
continue processing other dependent fields even if one fetch fails.
3 participants
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.