refactor: improve user routes, logging, and component formatting #55
Conversation
- Streamlined error handling in userRoutes for better clarity and consistency. - Cleaned up unnecessary whitespace and improved console logging in WorkflowCanvas and ConfigModal components. - Updated Prisma schema to enhance relationships and added optional fields for better flexibility in Credential and Node models.
📝 WalkthroughWalkthroughThis pull request updates the Node creation request payload construction in the backend, modifies the ConfigModal component to retrieve userId from Redux store, adjusts Prisma schema to establish Credential-Trigger and Credential-Node relationships with new optional fields, and includes minor formatting adjustments. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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: 1
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 (1)
11-11: Critical:onSavesignature mismatch between interface and usage.The interface declares:
onSave: (config: any, userId: string) => Promise<void>;But in
page.tsx(line 428), the callback is:onSave={async (nodeId, config) => { ... }}And
handleSave(line 31) calls:await onSave({ HOOKS_URL }, userId);The first argument passed is
config({ HOOKS_URL }), butpage.tsxexpectsnodeIdas the first argument. This will causenodeIdto be{ HOOKS_URL }andconfigto be theuserIdstring, breaking the update logic.🐛 Proposed fix: align the signature
Either update the interface and call site:
interface ConfigModalProps { isOpen: boolean; selectedNode: any | null; onClose: () => void; - onSave: (config: any, userId: string) => Promise<void>; + onSave: (nodeId: string, config: any, userId: string) => Promise<void>; }And in
handleSave:- await onSave({ HOOKS_URL }, userId); + await onSave(selectedNode.id, { HOOKS_URL }, userId);Or keep the existing
page.tsxsignature and update the interface accordingly.apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
413-413: Order nodes bystageinstead of the JSONpositionfield.The query orders nodes by
position: "asc", butpositionis aJson?field. PostgreSQL/Prisma will perform lexicographic string comparison on the serialized JSON, producing unpredictable ordering for coordinate objects. The same pattern is repeated inapps/worker/src/engine/executor.ts:17andapps/hooks/src/index.ts:18.Use the
stage: Intfield instead to maintain consistent workflow sequence ordering across all three locations.
🤖 Fix all issues with AI agents
In `@packages/db/prisma/schema.prisma`:
- Around line 71-75: The schema change made AvailableNode mandatory on the Node
model (AvailableNode changed from optional to required), which will break
existing rows lacking AvailableNodeID; create a migration to handle existing
data before applying this schema change: either backfill Node.AvailableNodeID
with a valid AvailableNode.id (e.g., pick a default/placeholder AvailableNode
row) or remove Node rows without a matching AvailableNode, then apply the schema
change; if you prefer a safer multi-step deploy, revert the schema to keep
AvailableNode optional, add a script to populate/fix AvailableNodeID for all
Node rows, run that migration/script in production, and only then change
AvailableNode to required. Also verify the dual credential relations
(credentials: Credential[] and Credential_Node_CredentialsIDToCredential:
Credential?) meet requirements and that CredentialsID foreign keys are
consistent before deploying.
🧹 Nitpick comments (2)
apps/web/app/workflows/[id]/components/ConfigModal.tsx (1)
21-24: Remove debug logging and review the type cast.
The
console.logon line 24 should be removed before merging.The
as unknown as stringcast suggestsstate.user.userIdmay not be typed asstring. If the Redux state type is incorrect, fix it at the source rather than casting here. IfuserIdcould beundefined, handle that case explicitly.♻️ Suggested improvement
const userId = useAppSelector( (state) => state.user.userId - ) as unknown as string; - console.log("we are getting this userId from ConfigModal", userId); + ); + + // Handle missing userId gracefully + if (!userId) { + // Return early or show appropriate UI + }apps/http-backend/src/routes/userRoutes/userRoutes.ts (1)
508-508: Remove debug logging.This
console.logshould be removed before merging to avoid polluting server logs in production.- console.log(" from http-backeden", data);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/http-backend/src/routes/userRoutes/userRoutes.tsapps/web/app/workflows/[id]/components/ConfigModal.tsxapps/web/app/workflows/[id]/page.tsxpackages/db/prisma/schema.prisma
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/app/workflows/[id]/page.tsx (1)
apps/web/app/lib/api.ts (1)
api(7-71)
apps/web/app/workflows/[id]/components/ConfigModal.tsx (1)
apps/web/app/hooks/redux.ts (1)
useAppSelector(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (4)
packages/db/prisma/schema.prisma (1)
39-42: LGTM!The optional
CredentialsIDfield and correspondingCredentialrelation onTriggerare well-defined, enabling triggers to optionally reference credentials.apps/web/app/workflows/[id]/page.tsx (1)
428-436: LGTM!Formatting change only; no functional impact.
apps/http-backend/src/routes/userRoutes/userRoutes.ts (2)
521-530: Position data is not captured from the request.The
positionfield is hardcoded to{}, but the frontend (page.tsxline 238) sets node positions like{ x: 350, y: 400 }. This position data is never sent to the backend during node creation, meaning positions are only saved later via the update endpoint when nodes are dragged.If initial positions should be persisted, consider:
- Adding
PositiontoNodeSchemavalidation- Passing it in the create payload:
position: dataSafe.data.Position || {}Otherwise, if the current behavior (positions saved only on drag) is intentional, this is fine.
236-238: LGTM!Formatting improvement for the error response object.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| CredentialsID String? | ||
| position Json? | ||
| credentials Credential[] | ||
| AvailableNode AvailableNode? @relation(fields: [AvailableNodeID], references: [id]) | ||
| // Credential_Node_CredentialsIDToCredential Credential @relation("Node_CredentialsIDToCredential", fields: [CredentialsID], references: [id], onDelete: Cascade) | ||
| AvailableNode AvailableNode @relation(fields: [AvailableNodeID], references: [id]) | ||
| Credential_Node_CredentialsIDToCredential Credential? @relation("Node_CredentialsIDToCredential", fields: [CredentialsID], references: [id]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/db/prisma/schema.prismaRepository: Dev-Pross/BuildFlow
Length of output: 4928
🏁 Script executed:
find packages/db -type f \( -name "*.sql" -o -name "*.ts" \) -path "*/migration*" | head -20Repository: Dev-Pross/BuildFlow
Length of output: 45
🏁 Script executed:
find packages/db -type d -name "migrations" -o -name "prisma"
ls -la packages/db/prisma/Repository: Dev-Pross/BuildFlow
Length of output: 345
🏁 Script executed:
ls -la packages/db/prisma/migrations/ && echo "---" && find packages/db/prisma/migrations -type f -name "*.sql" | sortRepository: Dev-Pross/BuildFlow
Length of output: 243
🏁 Script executed:
cat packages/db/prisma/migrations/migration_lock.toml
echo "---"
git log --oneline packages/db/prisma/schema.prisma | head -10Repository: Dev-Pross/BuildFlow
Length of output: 267
🏁 Script executed:
git show 83ab84d:packages/db/prisma/schema.prisma | grep -A 20 "model Node"
echo "---CURRENT---"
git show HEAD:packages/db/prisma/schema.prisma | grep -A 20 "model Node"Repository: Dev-Pross/BuildFlow
Length of output: 4300
🏁 Script executed:
git log --oneline packages/db/prisma/schema.prisma | head -20Repository: Dev-Pross/BuildFlow
Length of output: 135
🏁 Script executed:
git branch -a
echo "---"
git log --all --oneline -- packages/db/prisma/schema.prisma
echo "---"
git status packages/db/prisma/Repository: Dev-Pross/BuildFlow
Length of output: 1734
🏁 Script executed:
# Check the schema in a few earlier commits to see AvailableNodeID state
git show cd00170:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 15 "model Node" || echo "File not found in cd00170"
echo "---"
git show be91f24:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 15 "model Node" || echo "File not found in be91f24"
echo "---"
git show 85a5ad6:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 15 "model Node" || echo "File not found in 85a5ad6"Repository: Dev-Pross/BuildFlow
Length of output: 1636
🏁 Script executed:
# Check more recent commits
git show 81945da:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 20 "model Node" || echo "File not found in 81945da"
echo "---"
git show 5b8ae59:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 20 "model Node" || echo "File not found in 5b8ae59"
echo "---"
git show 2c89b31:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 20 "model Node" || echo "File not found in 2c89b31"Repository: Dev-Pross/BuildFlow
Length of output: 4631
🏁 Script executed:
# Check when CredentialsID and credential relations were added
git show f556da3:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 20 "model Node" || echo "File not found in f556da3"
echo "---"
git show 83ab84d:packages/db/prisma/schema.prisma 2>/dev/null | grep -A 20 "model Node" || echo "File not found in 83ab84d"
echo "---CURRENT---"
head -85 packages/db/prisma/schema.prisma | tail -20Repository: Dev-Pross/BuildFlow
Length of output: 5358
Address breaking change: AvailableNode relation is now mandatory.
The schema was updated to make AvailableNode required (changed from AvailableNode? to AvailableNode). This is a breaking change—existing Node records without a valid AvailableNodeID will violate constraints. Create a database migration to handle this before deploying:
- Set default
AvailableNodeIDvalues for existing records, or - Delete nodes without valid references, or
- Make this change on a fresh database only.
Credential relationships are intentionally dual-pattern. Node now has two credential associations:
credentials Credential[](many credentials per node vianodeIdFK on Credential)Credential_Node_CredentialsIDToCredential Credential?(optional primary credential viaCredentialsIDFK on Node)
This allows a node to reference multiple credentials while optionally designating one as primary. Confirm this design matches your requirements.
🤖 Prompt for AI Agents
In `@packages/db/prisma/schema.prisma` around lines 71 - 75, The schema change
made AvailableNode mandatory on the Node model (AvailableNode changed from
optional to required), which will break existing rows lacking AvailableNodeID;
create a migration to handle existing data before applying this schema change:
either backfill Node.AvailableNodeID with a valid AvailableNode.id (e.g., pick a
default/placeholder AvailableNode row) or remove Node rows without a matching
AvailableNode, then apply the schema change; if you prefer a safer multi-step
deploy, revert the schema to keep AvailableNode optional, add a script to
populate/fix AvailableNodeID for all Node rows, run that migration/script in
production, and only then change AvailableNode to required. Also verify the dual
credential relations (credentials: Credential[] and
Credential_Node_CredentialsIDToCredential: Credential?) meet requirements and
that CredentialsID foreign keys are consistent before deploying.
Summary by CodeRabbit
Release Notes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.