feat: Integrate Google Sheets API with new routes and components for …#38
Conversation
…credential management
WalkthroughThis PR integrates Google OAuth authentication and Google Sheets support throughout the backend and frontend. It adds the googleapis dependency, creates OAuth callback and sheets routes in the backend, refactors credential handling to use credential IDs instead of node IDs, and updates frontend components to fetch and manage credentials dynamically. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Web as Web Browser
participant BackendAuth as Backend<br/>(googleAuth)
participant GoogleOAuth as Google OAuth
participant Service as GoogleOAuthService
participant DB as Database
User->>Web: Click "Connect Google"
Web->>BackendAuth: Navigate to /google/callback?code=AUTH_CODE&state=USER_ID
BackendAuth->>GoogleOAuth: Exchange code for tokens
GoogleOAuth-->>BackendAuth: Return access_token, refresh_token
BackendAuth->>Service: saveCredentials(userId, tokens)
Service->>DB: Store credential with userId & tokens
DB-->>Service: Credential saved
Service-->>BackendAuth: Success
BackendAuth->>Web: Redirect to /workflow
Web-->>User: Show workflow page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Vamsi-o
left a comment
Vamsi-o
left a comment
There was a problem hiding this comment.
GOOD WORK , MAKE SURE TESTING IS DONE
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/nodes/src/common/google-oauth-service.ts (1)
77-101: Critical: Restore the userId filter to prevent unauthorized credential access.The commented-out
userIdfilter on Line 84 creates a serious security vulnerability. Without this filter, any user who knows acredIdcan retrieve credentials belonging to other users, potentially exposing sensitive OAuth tokens.🔎 Apply this diff to restore the security filter:
async getCredentials(userId: string, credId: string): Promise<{id: string, tokens: OAuthTokens} | null>{ try{ console.log("user id: ",userId," & ",credId," from oauth service") const credentials = await this.prisma.credential.findFirst({ where:{ id:credId, - // userId: userId, + userId: userId, type:'google_oauth', }, orderBy:{ id: 'desc' } });apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
14-24: Dead interface and unusual prop typing.
- The
GoogleSheetFormClientPropsinterface (lines 14-22) is no longer used since the function signature changed.- The prop type
type:{type: string}is unusual. Consider using a proper interface or simplified typing.🔎 Clean up the interface:
-interface GoogleSheetFormClientProps { - initialData: { - credentials: Array<{ id: string }>; - authUrl?: string; - hasCredentials: boolean; - }; - userId: string; - nodeId: string; -} +interface GoogleSheetFormClientProps { + type: string; +} -export function GoogleSheetFormClient(type:{type: string}) { +export function GoogleSheetFormClient({ type }: GoogleSheetFormClientProps) {packages/nodes/src/google-sheets/google-sheets.executor.ts (1)
5-10: Verify all callers ofNodeExecutionContexthave been updated to usecredId.The interface change from
nodeIdtocredIdis incomplete. API routes (documents/route.ts, tabs/route.ts) still passnodeIdto executor methods expecting the updatedcredIdfield. Additionally, google-oauth-service.ts and GoogleSheetFormClient.tsx still referencenodeIdand need updates to match the new interface definition.
🧹 Nitpick comments (8)
packages/nodes/src/common/google-oauth-service.ts (1)
80-80: Remove debug console.log statements.Debug logging statements should be removed before merging to production.
🔎 Apply this diff to remove console.log statements:
async getCredentials(userId: string, credId: string): Promise<{id: string, tokens: OAuthTokens} | null>{ try{ - console.log("user id: ",userId," & ",credId," from oauth service") const credentials = await this.prisma.credential.findFirst({ where:{ id:credId, userId: userId, type:'google_oauth', }, orderBy:{ id: 'desc' } }); - console.log("credentails from oauth service: ",credentials) if(!credentials) return null;async getAllCredentials(userId: string, type: string): Promise<Array<any>>{ try{ const credentials = await this.prisma.credential.findMany({ where:{ userId: userId, type: type }, }); - console.log("logs from service - ",credentials) return credentials;Also applies to: 91-91, 111-111
packages/nodes/src/google-sheets/google-sheets.node.ts (1)
36-36: Remove commented console.log statements.Either remove these commented logs or uncomment them if they're needed for debugging.
Also applies to: 38-38
apps/web/app/workflow/lib/config.ts (1)
27-45: Remove unnecessary variable assignment.Line 38 creates a
Datavariable by stringifying the response but never uses it. The function returnsresponse.data.Datadirectly.🔎 Apply this diff to remove the unused variable:
export const getCredentials = async(type: string)=>{ try{ const response = await axios.get( `${BACKEND_URL}/user/getCredentials/${type}`, { withCredentials: true, } ); console.log("response from config: ",response); - const Data = JSON.stringify(response.data.Data); return response.data.Data; } catch(e){ console.error("Error fetching credentials:", e); throw e; } }apps/web/app/hooks/useCredential.ts (1)
5-5: Add proper TypeScript typing for the hook.The hook return type is currently
any, which defeats the purpose of TypeScript. Consider defining a proper interface for the return value.🔎 Example implementation:
interface UseCredentialsReturn { cred: any; // Replace with specific credential type authUrl: string | undefined; } export const useCredentials = (type: string): UseCredentialsReturn => { const [cred, setCred] = useState<any>() const [authUrl, setAuthUrl] = useState<string>() // ... rest of implementation return { cred, authUrl } }Also applies to: 31-32
packages/nodes/src/google-sheets/google-sheets.executor.ts (1)
54-66: Mixed return types reduce API clarity and error handling loses context.
The method returns either an array of credentials or a string (auth URL), which makes the caller's type handling awkward and error-prone (as seen in userRoutes.ts).
Line 63-64: On error, the method logs but returns an empty array, losing the error context. This could mask underlying issues.
🔎 Consider returning a consistent structure:
- async getAllCredentials(userId: string, type: string) { + async getAllCredentials(userId: string, type: string): Promise<{ credentials: any[]; authUrl?: string }> { try { - const credentials = await this.oauthService.getAllCredentials(userId, type); console.log("log from executor - ",credentials) - if(credentials.length > 0) return credentials - else return (this.oauthService.getAuthUrl(userId)) - + if(credentials.length > 0) { + return { credentials }; + } + return { credentials: [], authUrl: this.oauthService.getAuthUrl(userId) }; } catch (e) { console.log(`Error in fetching credentials: ${e}`); - return []; + throw e; // Let caller handle the error appropriately } }apps/web/app/components/nodes/CreateWorkFlow.tsx (1)
11-12: Unused import.
useCredentialsis imported but not used in this component. It's used insideGoogleSheetFormClientinstead.🔎 Remove unused import:
import { GoogleSheetFormClient } from "./GoogleSheetFormClient"; -import { useCredentials } from "@/app/hooks/useCredential";apps/http-backend/src/routes/google_callback.ts (1)
4-4: Unused import.
userMiddlewareis imported but not used in this file.-import { userMiddleware } from './userRoutes/userMiddleware.js';apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
40-44: Remove debug console.log statements.Multiple console.log statements should be removed before merging to production.
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")
📜 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 (17)
apps/http-backend/package.json(1 hunks)apps/http-backend/src/index.ts(2 hunks)apps/http-backend/src/routes/google_callback.ts(1 hunks)apps/http-backend/src/routes/nodes.routes.ts(1 hunks)apps/http-backend/src/routes/userRoutes/userRoutes.ts(7 hunks)apps/web/app/api/auth/google/callback/route.ts(0 hunks)apps/web/app/components/nodes/CreateWorkFlow.tsx(6 hunks)apps/web/app/components/nodes/GoogleSheetForm.tsx(0 hunks)apps/web/app/components/nodes/GoogleSheetFormClient.tsx(7 hunks)apps/web/app/components/nodes/actions.ts(1 hunks)apps/web/app/form/page.tsx(0 hunks)apps/web/app/hooks/useCredential.ts(1 hunks)apps/web/app/workflow/lib/config.ts(2 hunks)packages/nodes/src/common/google-oauth-service.ts(2 hunks)packages/nodes/src/google-sheets/google-sheets.executor.ts(3 hunks)packages/nodes/src/google-sheets/google-sheets.node.ts(1 hunks)packages/nodes/src/registry/node-registry.ts(2 hunks)
💤 Files with no reviewable changes (3)
- apps/web/app/api/auth/google/callback/route.ts
- apps/web/app/components/nodes/GoogleSheetForm.tsx
- apps/web/app/form/page.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
apps/http-backend/src/routes/google_callback.ts (2)
packages/nodes/src/google-sheets/token.js (1)
code(10-10)packages/nodes/src/common/google-oauth-service.ts (2)
GoogleOAuthService(151-151)OAuthTokens(152-152)
apps/web/app/components/nodes/CreateWorkFlow.tsx (3)
apps/web/app/workflow/lib/config.ts (1)
getCredentials(27-45)packages/nodes/src/common/google-oauth-service.ts (1)
getCredentials(77-101)apps/web/app/components/nodes/GoogleSheetFormClient.tsx (1)
GoogleSheetFormClient(24-281)
apps/web/app/workflow/lib/config.ts (2)
packages/nodes/src/common/google-oauth-service.ts (1)
getCredentials(77-101)packages/common/src/index.ts (1)
BACKEND_URL(4-4)
apps/http-backend/src/index.ts (2)
apps/http-backend/src/routes/nodes.routes.ts (1)
sheetRouter(6-6)apps/http-backend/src/routes/google_callback.ts (1)
googleAuth(7-7)
apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
apps/http-backend/src/routes/userRoutes/userMiddleware.ts (2)
userMiddleware(18-42)AuthRequest(8-10)
apps/http-backend/src/routes/nodes.routes.ts (1)
apps/http-backend/src/routes/userRoutes/userMiddleware.ts (2)
userMiddleware(18-42)AuthRequest(8-10)
packages/nodes/src/common/google-oauth-service.ts (1)
packages/nodes/src/index.ts (1)
OAuthTokens(5-5)
apps/web/app/components/nodes/GoogleSheetFormClient.tsx (3)
apps/web/app/hooks/useCredential.ts (1)
useCredentials(5-33)packages/common/src/index.ts (1)
BACKEND_URL(4-4)packages/ui/src/components/select.tsx (1)
SelectItem(180-180)
packages/nodes/src/registry/node-registry.ts (1)
packages/db/src/index.ts (1)
prismaClient(17-18)
apps/web/app/hooks/useCredential.ts (2)
apps/web/app/workflow/lib/config.ts (1)
getCredentials(27-45)packages/nodes/src/common/google-oauth-service.ts (1)
getCredentials(77-101)
🔇 Additional comments (7)
apps/http-backend/src/index.ts (1)
11-12: LGTM!The new routes are properly imported and mounted. The route paths
/nodeandAlso applies to: 38-39
packages/nodes/src/registry/node-registry.ts (2)
15-15: LGTM!The
registerTriggermethod follows the same pattern as the existingregistermethod with proper:
- Deduplication using the
triggersSet- Upsert logic for database operations
- Error handling and logging
Also applies to: 44-68
38-38: Good improvement to the log message.Using
definition.nameinstead of logging the entire definition object makes the console output cleaner and more readable.apps/web/app/components/nodes/actions.ts (1)
14-26: Verify that the executor acceptscredIdin the context.The refactor from
nodeIdtocredentialIdis correct. The GoogleSheetsNodeExecutor interface explicitly definescredIdas a required field inNodeExecutionContext, and the executor properly accesses it when fetching credentials.apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
68-91: LGTM!Route path formatting normalized. No functional changes.
packages/nodes/src/google-sheets/google-sheets.executor.ts (1)
89-104: LGTM!The
ensureSheetServicemethod correctly uses the renamedcredIdfield.apps/web/app/components/nodes/CreateWorkFlow.tsx (1)
231-234: LGTM!Conditional rendering of
GoogleSheetFormClientwhenloadSheetis true.
| "cors": "^2.8.5", | ||
| "dotenv": "^17.2.3", | ||
| "express": "^5.1.0", | ||
| "googleapis": "^166.0.0", |
There was a problem hiding this comment.
Update googleapis to the latest version.
The current version of googleapis is 169.0.0, published 2 days ago. Version ^166.0.0 is outdated and will not receive updates beyond the 166.x range. Consider updating to ^169.0.0 for the latest bug fixes and improvements.
🤖 Prompt for AI Agents
In apps/http-backend/package.json at line 34 the dependency "googleapis":
"^166.0.0" is pinned to an outdated minor range; update the version string to
the current release range (for example "^169.0.0") in package.json, then run
your package manager to update the lockfile (npm install or yarn install) and
commit the updated package.json and lockfile so CI uses the new version.
| googleAuth.get('/callback', async(req: Request, res: Response)=>{ | ||
| const code = req.query.code; | ||
| const state = req.query.state; // userId | ||
| const Oauth = new GoogleOAuthService; |
There was a problem hiding this comment.
Missing parentheses in constructor call.
new GoogleOAuthService should be new GoogleOAuthService(). While JavaScript may still work without parentheses for parameterless constructors, it's inconsistent with the rest of the codebase and could cause issues.
- const Oauth = new GoogleOAuthService;
+ const Oauth = new GoogleOAuthService();📝 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 Oauth = new GoogleOAuthService; | |
| const Oauth = new GoogleOAuthService(); |
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/google_callback.ts around line 12, the
constructor invocation is missing parentheses; change the instantiation from
using "new GoogleOAuthService" to "new GoogleOAuthService()" to match the
codebase style and ensure a proper parameterless constructor call.
| if (!code || typeof code !== 'string') { | ||
| return res.json({ error: 'Missing or invalid authorization code' }); | ||
| } |
There was a problem hiding this comment.
Missing HTTP status code for error response.
Line 14 returns a JSON error but doesn't set an appropriate HTTP status code (defaults to 200). This should return a 400 Bad Request.
if (!code || typeof code !== 'string') {
- return res.json({ error: 'Missing or invalid authorization code' });
+ return res.status(400).json({ error: 'Missing or invalid authorization code' });
}🤖 Prompt for AI Agents
In apps/http-backend/src/routes/google_callback.ts around lines 13 to 15, the
handler returns a JSON error for a missing/invalid authorization code but fails
to set an HTTP status, implicitly sending 200; change the response to return a
400 Bad Request by setting the status before sending JSON (e.g., use
res.status(400).json({ error: 'Missing or invalid authorization code' })) so
clients receive the correct HTTP status.
| // Save tokens to database if userId (state) is provided | ||
| if (state && typeof state === 'string') { | ||
|
|
||
| await Oauth.saveCredentials(state, tokens as OAuthTokens) | ||
| } |
There was a problem hiding this comment.
State parameter must be validated against session-stored value to prevent CSRF attacks.
Clients MUST prevent Cross-Site Request Forgery (CSRF) where requests to the redirection endpoint do not originate at the authorization server but at a malicious third party. Currently, any value passed in state is trusted without verification. Implement state validation:
- Before redirecting to Google OAuth, generate a cryptographically secure random state and store it in the user session
- On callback, verify the returned state matches the stored value
- Only then trust the userId and proceed with token saving
This is required by OAuth 2.0 security standards and is a preventable vulnerability.
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/google_callback.ts around lines 26-30, the
callback is trusting any returned state and saving tokens immediately; update
the handler to first retrieve the expected state from the user session, compare
it to the incoming state using a constant-time equality check (or strict === if
stored as plain string), and if they match proceed to save credentials; if they
do not match or the session value is missing, reject the request (return
400/unauthorized), do not save tokens, log the mismatch, and clear the session
state token; additionally ensure (elsewhere on the initial OAuth redirect) a
cryptographically secure random state is generated and stored in the session
before redirecting to Google.
| } | ||
|
|
||
| // Redirect to success page | ||
| return res.redirect('http://localhost:3000/workflow'); |
There was a problem hiding this comment.
Hardcoded localhost URL.
The redirect URL http://localhost:3000/workflow is hardcoded, which will break in production environments.
🔎 Use environment variable:
- return res.redirect('http://localhost:3000/workflow');
+ const frontendUrl = process.env.FRONTEND_URL || 'http://localhost:3000';
+ return res.redirect(`${frontendUrl}/workflow`);Similarly for line 37-38:
- return res.redirect(
- `http://localhost:3000/workflow?google=error&msg=${encodeURIComponent(err?.message ?? 'Token exchange failed')}`);
+ const frontendUrl = process.env.FRONTEND_URL || 'http://localhost:3000';
+ return res.redirect(
+ `${frontendUrl}/workflow?google=error&msg=${encodeURIComponent(err?.message ?? 'Token exchange failed')}`);📝 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.
| return res.redirect('http://localhost:3000/workflow'); | |
| const frontendUrl = process.env.FRONTEND_URL || 'http://localhost:3000'; | |
| return res.redirect(`${frontendUrl}/workflow`); |
🤖 Prompt for AI Agents
In apps/http-backend/src/routes/google_callback.ts around line 33, the redirect
target is hardcoded to 'http://localhost:3000/workflow'; change this to use a
configurable environment variable (e.g. process.env.FRONTEND_URL or app config)
with a sensible default for local dev, and apply the same approach to the other
hardcoded redirects at lines ~37-38 so all redirect URLs come from
configuration; validate the env value (ensure it includes protocol) before
redirecting and fall back to a safe internal route if missing or invalid.
| const data = await response.json(); | ||
| if (data.success) { | ||
| setSheets(data.data || []); | ||
| // console.log(data," from clint") | ||
| if (data.files.data.length > 0) { | ||
| setSheets(data.files.data || []); | ||
| } |
There was a problem hiding this comment.
Missing null safety check may cause runtime error.
Accessing data.files.data.length without null checks will throw if data.files or data.files.data is undefined.
🔎 Add null safety:
const data = await response.json();
- // console.log(data," from clint")
- if (data.files.data.length > 0) {
- setSheets(data.files.data || []);
+ if (data.files?.data?.length > 0) {
+ setSheets(data.files.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 = await response.json(); | |
| if (data.success) { | |
| setSheets(data.data || []); | |
| // console.log(data," from clint") | |
| if (data.files.data.length > 0) { | |
| setSheets(data.files.data || []); | |
| } | |
| const data = await response.json(); | |
| if (data.files?.data?.length > 0) { | |
| setSheets(data.files.data); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/components/nodes/GoogleSheetFormClient.tsx around lines 91 to
95, the code reads data.files.data.length without null-safety; add guards using
optional chaining or existence checks before accessing length (e.g., if
(data?.files?.data && data.files.data.length > 0) ) and fall back to an empty
array when calling setSheets (e.g., setSheets(data?.files?.data ?? [])); ensure
response.json() result is validated before accessing nested properties to avoid
runtime errors.
| {authUrl ? ( | ||
| // <SelectItem value="create-new"> | ||
| <a href={authUrl} target="_blank" rel="noopener noreferrer" className="text-blue-500"> | ||
| + Create new credential | ||
| </Link> | ||
| </SelectItem> | ||
| </a> | ||
| // </SelectItem> | ||
| ) : ( | ||
| <> | ||
| {initialData.credentials.map((cred) => ( | ||
| { | ||
| response?.map((cred: any) => ( | ||
| <SelectItem key={cred.id} value={cred.id}> | ||
| Google Account ({cred.id.slice(0, 8)}...) | ||
| </SelectItem> | ||
| ))} | ||
| {initialData.authUrl && ( | ||
| <SelectItem value="create-new"> | ||
| <a href={initialData.authUrl} target="_blank" rel="noopener noreferrer" className="text-blue-500"> | ||
| + Add another account | ||
| </a> | ||
| </SelectItem> | ||
| )} | ||
| )) | ||
| } | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Accessibility issue: Anchor inside Select dropdown.
Placing an anchor tag (<a>) directly inside SelectContent without wrapping it in a SelectItem breaks the Select component's expected structure and may cause accessibility issues. Users navigating with keyboard may not be able to interact with this link properly.
🔎 Consider alternative UX patterns:
Instead of mixing a link inside the Select dropdown, consider:
- Show the credential selector only when credentials exist
- Show a separate "Connect Google Account" button when no credentials exist
+ {authUrl && !response?.length && (
+ <div className="mb-4">
+ <a href={authUrl} target="_blank" rel="noopener noreferrer" className="text-blue-500 underline">
+ + Connect Google Account
+ </a>
+ </div>
+ )}
+
{/* 1. Credentials Select */}
<div>
<Label htmlFor="credential">Credential to connect with</Label>
- <Select value={selectedCredential} onValueChange={handleCredentialChange}>
+ <Select value={selectedCredential} onValueChange={handleCredentialChange} disabled={!response?.length}>
<SelectTrigger className="w-full mt-2">
<SelectValue placeholder="Select credential" />
</SelectTrigger>
<SelectContent>
- {authUrl ? (
- // <SelectItem value="create-new">
- <a href={authUrl} target="_blank" rel="noopener noreferrer" className="text-blue-500">
- + Create new credential
- </a>
- // </SelectItem>
- ) : (
- <>
- {
- response?.map((cred: any) => (
- <SelectItem key={cred.id} value={cred.id}>
- Google Account ({cred.id.slice(0, 8)}...)
- </SelectItem>
- ))
- }
- </>
- )}
+ {response?.map((cred: any) => (
+ <SelectItem key={cred.id} value={cred.id}>
+ Google Account ({cred.id.slice(0, 8)}...)
+ </SelectItem>
+ ))}
</SelectContent>
</Select>
</div>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/components/nodes/GoogleSheetFormClient.tsx around lines 140 to
156, the issue is that an <a> tag is being rendered directly inside the Select
dropdown which breaks the Select structure and harms keyboard/accessibility;
replace this pattern by not placing a bare anchor inside SelectContent — when
credentials exist render only SelectItems, and when no credentials render a
separate, focusable control (e.g., a button placed outside the Select) that
opens the authUrl in a new tab or triggers the connect flow; alternatively, if
you must show an option inside the dropdown, implement it as a proper SelectItem
that, on selection, opens the authUrl (via window.open or navigation handler) to
preserve keyboard/mouse behavior and ARIA expectations.
| const fetchCred = async()=>{ | ||
| try{ | ||
| if(!type) return {} | ||
| const response = await getCredentials(type) | ||
| if(response){ | ||
| console.log(typeof(response)) | ||
| if(typeof(response) === 'string') setAuthUrl(response) | ||
| else setCred(response) | ||
|
|
||
| // console.log(response[0].nodeId) | ||
| console.log(response) | ||
| return cred | ||
|
|
||
| } | ||
| else return {} | ||
| } | ||
| catch(e){ | ||
| console.log(e instanceof Error ? e.message : "unknow error from useCredentials hook") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix hook logic issues.
Several issues with the hook implementation:
- Line 20: Returning
credinside the async function serves no purpose—the return value is discarded andcredwon't have the updated state value yet. - Lines 14-16: Type checking with
typeof(response) === 'string'is fragile. If the backend returns different shapes, this could break. - Line 26: Typo "unknow" should be "unknown".
🔎 Apply this diff to fix the issues:
const fetchCred = async()=>{
try{
if(!type) return {}
const response = await getCredentials(type)
if(response){
console.log(typeof(response))
if(typeof(response) === 'string') setAuthUrl(response)
else setCred(response)
- // console.log(response[0].nodeId)
console.log(response)
- return cred
}
- else return {}
}
catch(e){
- console.log(e instanceof Error ? e.message : "unknow error from useCredentials hook")
+ console.error(e instanceof Error ? e.message : "unknown error from useCredentials hook")
}
} 📝 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 fetchCred = async()=>{ | |
| try{ | |
| if(!type) return {} | |
| const response = await getCredentials(type) | |
| if(response){ | |
| console.log(typeof(response)) | |
| if(typeof(response) === 'string') setAuthUrl(response) | |
| else setCred(response) | |
| // console.log(response[0].nodeId) | |
| console.log(response) | |
| return cred | |
| } | |
| else return {} | |
| } | |
| catch(e){ | |
| console.log(e instanceof Error ? e.message : "unknow error from useCredentials hook") | |
| } | |
| } | |
| const fetchCred = async()=>{ | |
| try{ | |
| if(!type) return {} | |
| const response = await getCredentials(type) | |
| if(response){ | |
| console.log(typeof(response)) | |
| if(typeof(response) === 'string') setAuthUrl(response) | |
| else setCred(response) | |
| console.log(response) | |
| } | |
| } | |
| catch(e){ | |
| console.error(e instanceof Error ? e.message : "unknown error from useCredentials hook") | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/hooks/useCredential.ts around lines 9 to 28, the async fetchCred
incorrectly returns the state variable, uses a fragile typeof string check, and
contains a typo in the error message; remove the useless "return cred" (state
updates are async and the return is discarded), replace the fragile type branch
with robust shape checks (e.g., test for specific properties like
response.authUrl or use Array.isArray(response) / response?.nodeId to detect
credential shapes and set either setAuthUrl or setCred accordingly), and correct
the catch message typo from "unknow" to "unknown"; ensure the function simply
sets state and returns nothing (or a meaningful value derived from the response)
and logs the full error message.
| @@ -1,4 +1,5 @@ | |||
| import { BACKEND_URL } from "@repo/common/zod"; | |||
| import { GoogleSheetsNodeExecutor } from "@repo/nodes"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused import.
The GoogleSheetsNodeExecutor import is not used anywhere in this file.
🔎 Apply this diff to remove the unused import:
import { BACKEND_URL } from "@repo/common/zod";
-import { GoogleSheetsNodeExecutor } from "@repo/nodes";
import axios from "axios"📝 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 { GoogleSheetsNodeExecutor } from "@repo/nodes"; | |
| import { BACKEND_URL } from "@repo/common/zod"; | |
| import axios from "axios" |
🤖 Prompt for AI Agents
In apps/web/app/workflow/lib/config.ts around line 2, the import
"GoogleSheetsNodeExecutor" is unused; remove that import statement (or delete
the identifier from the import list) so the file no longer imports
GoogleSheetsNodeExecutor and run/verify the linter/build to ensure no other
references remain.
| static async register(){ | ||
| await NodeRegistry.register(this.definition) | ||
| // console.log(`✅ Registered node: ${this.definition.name}`); | ||
| await NodeRegistry.registerTrigger(this.definition) | ||
| // console.log(`✅ Registered Trigger: ${this.definition.name}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the structure and look for other node files
find packages/nodes/src -name "*.node.ts" -type f | head -20
# Search for registerTrigger calls across all node files
echo "=== Files with registerTrigger ==="
rg -nP "registerTrigger" --type=ts -g "*node.ts"
# Search for register(this.definition) pattern
echo "=== Files with register\\(this.definition\\) ==="
rg -nP "register\(this\.definition\)" --type=ts -g "*node.ts"
# Check if there are nodes with trigger registration
echo "=== Check specific node files for registration patterns ==="
rg -nP "static.*register|registerTrigger" --type=ts -g "*node.ts" -A2 -B2Repository: Dev-Pross/BuildFlow
Length of output: 1251
🏁 Script executed:
#!/bin/bash
# Find all node files
echo "=== Total node files ==="
find packages/nodes/src -name "*.node.ts" -type f | wc -l
# Show all node files
echo "=== All node files ==="
find packages/nodes/src -name "*.node.ts" -type f | sort
# Check their registration patterns
echo "=== Registration patterns in all nodes ==="
for file in $(find packages/nodes/src -name "*.node.ts" -type f | sort); do
echo "--- $file ---"
grep -A5 "static.*register\|register()" "$file" | head -10
done
# Find NodeRegistry implementation
echo "=== NodeRegistry implementation ==="
find . -name "*registry*" -o -name "*Registry*" | grep -i registry | grep -v node_modules | head -20
</parameter>Repository: Dev-Pross/BuildFlow
Length of output: 802
🏁 Script executed:
#!/bin/bash
# Read the NodeRegistry implementation
cat -n packages/nodes/src/registry/node-registry.ts
# Also check the google-sheets definition to see if it has trigger properties
echo "=== Google Sheets node definition ==="
grep -B20 "definition =" packages/nodes/src/google-sheets/google-sheets.node.ts | tail -30Repository: Dev-Pross/BuildFlow
Length of output: 2720
🏁 Script executed:
#!/bin/bash
# Get the full definition of the Google Sheets node
sed -n '6,35p' packages/nodes/src/google-sheets/google-sheets.node.tsRepository: Dev-Pross/BuildFlow
Length of output: 878
Remove redundant commented console.logs.
The NodeRegistry.register() and NodeRegistry.registerTrigger() methods already include built-in logging (lines 38 and 64 in node-registry.ts), so the commented logs at lines 36 and 38 are redundant and should be removed.
🤖 Prompt for AI Agents
In packages/nodes/src/google-sheets/google-sheets.node.ts around lines 34 to 39,
remove the two redundant commented console.log statements (the lines commented
out after NodeRegistry.register and NodeRegistry.registerTrigger) because
NodeRegistry already logs registration; simply delete those commented lines and
run the linter/format step to ensure no trailing whitespace remains.
3 participants
…credential management
Summary by CodeRabbit
Release Notes
New Features
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.