chore: radiooptionblockcreate modern#8982
Merged
mikeallisonJS merged 10 commits intomainfrom Apr 9, 2026
Merged
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
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis pull request routes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as API Gateway
participant Modern as API Journeys<br/>Modern
participant Auth as Authorization<br/>Service
participant DB as Database<br/>(Prisma)
Client->>Gateway: radioOptionBlockCreate(input)
Gateway->>Modern: Route to API_JOURNEYS_MODERN
Modern->>Auth: Authenticate user
Auth-->>Modern: User authenticated
Modern->>Auth: authorizeBlockCreate(journeyId)
Auth-->>Modern: Permission granted
Modern->>DB: prisma.$transaction()
DB->>DB: Create RadioOptionBlock record
DB->>DB: Connect to journey & parentBlock
DB->>DB: Calculate parentOrder
DB->>DB: Update journey.updatedAt
DB-->>Modern: Block created
Modern-->>Gateway: RadioOptionBlock
Gateway-->>Client: radioOptionBlockCreate response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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🧪 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 5ad1314
☁️ Nx Cloud last updated this comment at |
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
Contributor
|
The latest updates on your projects.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.ts (2)
119-143: Strengthen unauthorized-path test by asserting no DB transaction/write.Add an explicit
expect(prismaMock.$transaction).not.toHaveBeenCalled()assertion to guarantee denied requests never reach mutation writes.🤖 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/icon/iconBlockUpdate.mutation.spec.ts` around lines 119 - 143, In the 'returns FORBIDDEN when unauthorized' test (where fetchBlockWithJourneyAcl is mocked, mockAbility returns false and authClient runs ICON_BLOCK_UPDATE), add an assertion to ensure no DB writes occur by asserting prismaMock.$transaction was not called (expect(prismaMock.$transaction).not.toHaveBeenCalled()); this confirms denied requests do not trigger any mutation/transaction.
74-74: Replaceanytransaction callback types with explicit callback types.Using
anyhere drops useful type checks and conflicts with the TS guideline.♻️ Typed alternative
- prismaMock.$transaction.mockImplementation(async (cb: any) => await cb(tx)) + prismaMock.$transaction.mockImplementation( + async (cb: (transaction: typeof tx) => Promise<unknown>) => await cb(tx) + ) ... - prismaMock.$transaction.mockImplementation(async (cb: any) => await cb(tx)) + prismaMock.$transaction.mockImplementation( + async (cb: (transaction: typeof tx) => Promise<unknown>) => await cb(tx) + )As per coding guidelines:
**/*.{ts,tsx}: Define a type if possible.Also applies to: 169-169
🤖 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/icon/iconBlockUpdate.mutation.spec.ts` at line 74, The mock uses a loose any for the transaction callback (prismaMock.$transaction.mockImplementation(async (cb: any) => await cb(tx))) which loses type-safety; replace the any with an explicit callback type matching Prisma's $transaction signature (use the appropriate PrismaClient/TransactionClient callback type and a generic return type) for the parameter named cb and ensure tx is typed accordingly; update both occurrences (the mockImplementation around prismaMock.$transaction and the second instance at the other noted location) so the mock's callback parameter and tx variable have explicit Prisma types instead of any.
🤖 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 314-319: Update the test command in the "Regenerate schema and
test" step so contributors run api-journeys-modern tests with the sequential
flag: change the nx test invocation for API Journey Modern (the line with nx
test api-journeys-modern --testPathPattern="<fieldName>") to include the
--runInBand flag so tests execute without parallelization.
In
`@apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.ts`:
- Around line 53-80: The mutation creates an ImageBlock but only calls
setJourneyUpdatedAt so Journey.customizable can remain stale; after the
tx.block.create (and after any cover-replacement logic) await the journey
customizable recalculation function (e.g., await
recalculateJourneyCustomizable(tx, input.journeyId)) to recompute and persist
Journey.customizable, and import/use the existing recalculation helper (name it
consistently, e.g., recalculateJourneyCustomizable) alongside
setJourneyUpdatedAt so every create/cover-replacement path updates customizable
immediately.
- Around line 27-30: Move the authorization check to run before any input
transformation: call authorizeBlockCreate(args.input.journeyId, context.user)
prior to invoking transformInput({ ...args.input }) so that authorizeBlockCreate
executes before transformInput and prevents anonymous/unauthorized callers from
triggering metadata/blurhash work; use the existing authorizeBlockCreate,
transformInput, args.input and context.user symbols to locate and reorder the
calls.
- Around line 31-42: The create mutation must validate input.parentBlockId
against the target journey before entering prisma.$transaction: call
validateParentBlock(journeyId, input.parentBlockId) (or similar existing helper)
outside the transaction and ensure that validateParentBlock queries include
deletedAt: null so it cannot return a deleted or cross-journey block; remove the
reliance on tx.block.findUnique alone (in the isCover branch) and use the
validated parentBlock result (or re-query inside the transaction only after
validation) to enforce the parent belongs to the same journey.
In `@apis/api-journeys-modern/src/schema/block/image/transformInput.ts`:
- Around line 44-50: The catch block in transformInput.ts currently only handles
instances of Error and silently swallows other throwables, leading to a
partially defaulted payload; update the catch in the function that processes
image input (the catch surrounding the image processing logic in transformInput)
to either rethrow non-Error exceptions or convert them into a GraphQLError with
the original value serialized (e.g., JSON.stringify or String(ex)) included in
extensions or the message; specifically ensure that if ex is not an instance of
Error you do not fall through — throw ex (or throw new GraphQLError(String(ex),
{ extensions: { code: 'BAD_USER_INPUT', original: ex } })) so all failures are
surfaced.
- Around line 31-33: The code in transformInput.ts calls fetch(input.src)
directly (see the fetch(input.src) / response / buffer sequence), allowing SSRF;
before fetching validate and restrict the URL: parse input.src with URL, reject
non-http(s) schemes, forbid loopback/localhost and private/CIDR ranges by
resolving the hostname to IP(s) and checking against RFC1918/loopback/mapped
ranges, or apply a strict hostname allowlist; only proceed to call fetch when
the URL passes these checks and throw a validation error otherwise. Ensure this
validation is performed early in the transformInput flow so response = await
fetch(input.src) is only reached for safe, approved destinations.
---
Nitpick comments:
In
`@apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.ts`:
- Around line 119-143: In the 'returns FORBIDDEN when unauthorized' test (where
fetchBlockWithJourneyAcl is mocked, mockAbility returns false and authClient
runs ICON_BLOCK_UPDATE), add an assertion to ensure no DB writes occur by
asserting prismaMock.$transaction was not called
(expect(prismaMock.$transaction).not.toHaveBeenCalled()); this confirms denied
requests do not trigger any mutation/transaction.
- Line 74: The mock uses a loose any for the transaction callback
(prismaMock.$transaction.mockImplementation(async (cb: any) => await cb(tx)))
which loses type-safety; replace the any with an explicit callback type matching
Prisma's $transaction signature (use the appropriate
PrismaClient/TransactionClient callback type and a generic return type) for the
parameter named cb and ensure tx is typed accordingly; update both occurrences
(the mockImplementation around prismaMock.$transaction and the second instance
at the other noted location) so the mock's callback parameter and tx variable
have explicit Prisma types instead of any.
🪄 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: 372fad35-c14d-4954-bc3c-eae6a57167a6
📒 Files selected for processing (15)
.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.ts
Contributor
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
Contributor
|
Merge conflict attempting to merge this into stage. Please fix manually. |
csiyang
approved these changes
Apr 9, 2026
tanflem
pushed a commit
that referenced
this pull request
Apr 13, 2026
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor