Conversation
…api-journeys-override-skill
…odify existing iconBlockCreate mutation for modern API
…tions for iconBlock mutations
…ization patterns in API journeys
…ck mutation in API gateway
…d override for API journeys
…I gateway schema for consistency
…ensure consistency across schemas
…API gateway schema for consistency
…consistency across schemas
…eway schema for consistency
…consistency across schemas
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 0 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR implements Apollo Federation override mutations in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
View your CI Pipeline Execution ↗ for commit db83559
☁️ Nx Cloud last updated this comment at |
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.spec.ts (1)
151-213: Cover the invalid-cover branches too.This suite exercises the successful cover replacement path, but it never asserts the two explicit guardrails in the resolver:
isCoverwithoutparentBlockId, andparentBlockIdpointing at a missing block. Those are the branches most likely to regress because they only show up on malformed input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.spec.ts` around lines 151 - 213, Add two unit tests in imageBlockCreate.mutation.spec.ts to exercise the resolver's guard clauses: (1) call authClient with input { isCover: true } but without parentBlockId and assert the mutation returns the expected validation/error response for "isCover without parentBlockId"; (2) mock tx.block.findUnique to return null for the given parentBlockId (simulating a missing parent) and call authClient with that parentBlockId and isCover true, asserting the resolver returns the expected error for "parentBlockId points at a missing block". Use the same patterns as existing tests (authClient with document IMAGE_BLOCK_CREATE, prismaMock.$transaction mocking tx, and assertions on result/error shape) and reference tx.block.findUnique and the imageBlockCreate mutation behavior so the new specs cover those invalid-cover branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/skills/create-api-journeys-override/SKILL.md:
- Around line 316-319: Update the documented test command for the
api-journeys-modern target so it runs sequentially: change the example "nx test
api-journeys-modern --testPathPattern=\"<fieldName>\"" to include the Jest
runInBand flag (i.e., add --runInBand) so the invocation becomes "nx test
api-journeys-modern --runInBand --testPathPattern=\"<fieldName>\""; ensure any
other examples in the same section that invoke nx test api-journeys-modern also
include --runInBand to enforce non-parallel execution as required by the
apis/api-journeys-modern spec.
In
`@apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.ts`:
- Around line 53-80: The mutation writes the block's customizable flag but never
recalculates Journey.customizable, leaving the journey stale; after awaiting
tx.block.create(...) and setJourneyUpdatedAt(tx, block), call the existing
recalculation function (e.g., recalculateJourneyCustomizability or whatever
project convention you have) with the transaction and input.journeyId to update
Journey.customizable immediately (i.e., invoke
recalculateJourneyCustomizability(tx, input.journeyId) right after
setJourneyUpdatedAt).
- Around line 26-29: Move the authorization call before any processing of
untrusted input: in the resolve function call authorizeBlockCreate using
args.input.journeyId and context.user before invoking transformInput so we don't
run transformInput on attacker-controlled src; specifically, replace the current
order in the resolve method so authorizeBlockCreate(args.input.journeyId,
context.user) runs first, then call transformInput({ ...args.input }) and
continue with the rest of the logic.
In `@apis/api-journeys-modern/src/schema/block/image/transformInput.ts`:
- Around line 44-50: The catch block in transformInput (catch (ex) { ... })
swallows non-Error exceptions causing transformedInput to be returned with empty
values; change the handler to surface unknown throws by either rethrowing the
original exception or converting it to a GraphQLError (e.g., stringify ex) and
include it in processLogger/error output so callers see the failure instead of
silently receiving an empty transformedInput; ensure all branches throw or log
and do not fall through to returning transformedInput on unexpected exception
types.
In
`@apis/api-journeys-modern/src/schema/block/radioQuestion/radioQuestionBlockUpdate.mutation.ts`:
- Around line 1-19: The resolver must validate the target block’s type and the
new parent before calling the generic update: after authorizeBlockUpdate(id,
context.user) but before calling update(id, {...}), fetch and assert the
existing block is a RadioQuestionBlock (e.g. check its typename or use
RadioQuestionBlock.isTypeOf logic) and call validateParentBlock(parentBlockId)
(outside any prisma.$transaction) to ensure the parent exists/belongs to the
same journey; only if both checks pass proceed to update(id, { parentBlockId,
gridView }).
In
`@apis/api-journeys-modern/src/schema/block/signUp/signUpBlockUpdate.mutation.spec.ts`:
- Around line 187-230: The tests for invalid submitIconId and submitIconId not
child of block are missing assertions/mocks for the authorization path, so
update the two it(...) cases to also mock fetchBlockWithJourneyAcl (or the
helper used to fetch the block with ACL) and assert ability is called or that
fetchBlockWithJourneyAcl is invoked, ensuring the authorization path is
exercised; specifically, in signUpBlockUpdate.mutation.spec.ts augment the
setups around SIGN_UP_BLOCK_UPDATE (and the prismaMock.block.findUnique mocks)
to stub/spy fetchBlockWithJourneyAcl (or the ability check) to return the
expected block/ability and add expectations that fetchBlockWithJourneyAcl (or
ability) was called, so the tests fail if auth is not enforced.
In
`@apis/api-journeys-modern/src/schema/block/signUp/signUpBlockUpdate.mutation.ts`:
- Around line 25-40: Before calling authorizeBlockUpdate or update, load the
target block (using prisma.block.findUnique for id) and assert it exists and has
type 'SignUpBlock'; if not, throw a GraphQLError. Do the same when validating
input.submitIconId: fetch the submitIcon via prisma.block.findUnique, ensure
submitIcon.parentBlockId === id AND submitIcon.type is the expected icon block
type (e.g., 'IconBlock'), otherwise throw NOT_FOUND/invalid error. Only after
these type checks pass call authorizeBlockUpdate(id, context.user) and then
update(id, { ...input }).
---
Nitpick comments:
In
`@apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.spec.ts`:
- Around line 151-213: Add two unit tests in imageBlockCreate.mutation.spec.ts
to exercise the resolver's guard clauses: (1) call authClient with input {
isCover: true } but without parentBlockId and assert the mutation returns the
expected validation/error response for "isCover without parentBlockId"; (2) mock
tx.block.findUnique to return null for the given parentBlockId (simulating a
missing parent) and call authClient with that parentBlockId and isCover true,
asserting the resolver returns the expected error for "parentBlockId points at a
missing block". Use the same patterns as existing tests (authClient with
document IMAGE_BLOCK_CREATE, prismaMock.$transaction mocking tx, and assertions
on result/error shape) and reference tx.block.findUnique and the
imageBlockCreate mutation behavior so the new specs cover those invalid-cover
branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0ec6bbb9-3c6f-46ba-bab3-e318948bbb22
📒 Files selected for processing (30)
.cursor/skills/create-api-journeys-override/SKILL.mdapis/api-gateway/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/icon/index.tsapis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.tsapis/api-journeys-modern/src/schema/block/image/imageBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/image/imageBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/image/index.tsapis/api-journeys-modern/src/schema/block/image/transformInput.tsapis/api-journeys-modern/src/schema/block/radioOption/index.tsapis/api-journeys-modern/src/schema/block/radioOption/radioOptionBlockCreate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/radioOption/radioOptionBlockCreate.mutation.tsapis/api-journeys-modern/src/schema/block/radioOption/radioOptionBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/radioOption/radioOptionBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/radioQuestion/index.tsapis/api-journeys-modern/src/schema/block/radioQuestion/radioQuestionBlockCreate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/radioQuestion/radioQuestionBlockCreate.mutation.tsapis/api-journeys-modern/src/schema/block/radioQuestion/radioQuestionBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/radioQuestion/radioQuestionBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/signUp/index.tsapis/api-journeys-modern/src/schema/block/signUp/signUpBlockCreate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/signUp/signUpBlockCreate.mutation.tsapis/api-journeys-modern/src/schema/block/signUp/signUpBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/signUp/signUpBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/spacer/index.tsapis/api-journeys-modern/src/schema/block/spacer/spacerBlockCreate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/spacer/spacerBlockCreate.mutation.ts
…-00-MA-chore-spacerblockcreate-modern
Summary by CodeRabbit
New Features
Tests