fix: Add job check to get correct linkage#235
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/api/src/router/resources.ts (1)
435-439: Consider handling undefined matchesJobId conditionThe current implementation might pass
undefinedto theand()function whenjobIdis null. While this might work due to theisPresentfilter elsewhere in the codebase, it would be more explicit to handle this case.Consider this alternative:
const matchesJobId = jobId == null ? undefined : eq(schema.jobResourceRelationship.jobId, jobId); const parent = await db.query.jobResourceRelationship.findFirst({ - where: and(matchesIdentifier, matchesJobId), + where: matchesJobId ? and(matchesIdentifier, matchesJobId) : matchesIdentifier, });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/api/src/router/resources.ts(3 hunks)
🔇 Additional comments (3)
packages/api/src/router/resources.ts (3)
410-414: LGTM: Function signature updated correctly
The addition of the optional jobId parameter is well-typed and aligns with the PR objective.
528-528: LGTM: Correctly propagating job context
The update properly passes the job ID when retrieving child nodes, ensuring consistent filtering throughout the resource tree.
410-414: Verify all function calls are updated
Let's verify that all calls to getNodeDataForResource have been updated to handle the new parameter.
✅ Verification successful
All calls to getNodeDataForResource are properly updated
The verification shows two calls to getNodeDataForResource:
- Line 528: Correctly passes
db,r.resource.id, andr.jobId - Line 540: Correctly passes
dbandresourceId(jobId is optional parameter)
Both usages are consistent with the function signature which declares jobId as an optional parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all calls to getNodeDataForResource
ast-grep --pattern 'getNodeDataForResource($$$)'
Length of output: 253
Summary by CodeRabbit
New Features
Bug Fixes