chore: radiooptionblockupdate modern#8983
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
|
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)
WalkthroughThe pull request migrates radio option block mutations from the api-journeys graph to api-journeys-modern. This includes updating the API gateway routing, extending the modern service's schema with new mutation fields, and implementing the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 698fc0b
☁️ 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.
|
|
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: 9
🤖 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: The test invocation in the docs for api-journeys-modern
runs tests in parallel; update the test command to run sequentially by adding
the Jest flag --runInBand to the existing command shown with nx test
api-journeys-modern --testPathPattern="<fieldName>" (keep the generate step nx
run api-journeys-modern:generate-graphql unchanged); modify the line in SKILL.md
so the test command becomes the same invocation but including --runInBand.
- Around line 227-236: Update the migration docs to add regenerating the gateway
supergraph after the subgraph: run the command nx run
api-gateway:generate-graphql immediately after nx run
api-journeys-modern:generate-graphql (as part of Step 6), and instruct verifying
the composed schema at apis/api-gateway/schema.graphql contains the new field
annotated with `@join__field`(graph: API_JOURNEYS_MODERN, override:
"api-journeys"); reference the exact command name (nx run
api-gateway:generate-graphql) and the schema path so reviewers can locate and
validate the change.
In
`@apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.ts`:
- Around line 179-186: The current test in iconBlockUpdate.mutation.spec.ts only
asserts tx.block.update was called with data containing name and doesn't prevent
omitted fields from being set to null; update the assertion to also ensure the
update data does not include/overwrite omitted fields (e.g., color or size) by
asserting tx.block.update was called with a data object that contains the name
and explicitly does NOT contain keys like color or size (use negative/absence
assertions against tx.block.update's data argument) so partial updates leave
unspecified fields untouched.
In
`@apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.spec.ts`:
- Around line 189-193: The test currently asserts the soft-delete timestamp with
expect.any(String) which is brittle; update the assertion in the tx.block.update
expectation (the objectContaining with where: { id: 'oldCoverId' }) to be
representation-agnostic by replacing expect.any(String) with a
non-format-specific matcher such as expect.anything() or expect.not.toBeNull so
the test only verifies that deletedAt was set without enforcing a string format.
In
`@apis/api-journeys-modern/src/schema/block/image/imageBlockCreate.mutation.ts`:
- Around line 53-80: The ImageBlock creation path writes a block that may modify
the block's `customizable` flag but only calls setJourneyUpdatedAt(tx, block);
after the DB write and after setJourneyUpdatedAt(tx, block) you must also invoke
the journey customizable recalculation routine (the function used elsewhere to
recompute journey-level `customizable` aggregates) passing the transaction and
the journey/block context (e.g., tx and block or input.journeyId) so the journey
aggregate is updated; locate this change near the ImageBlock create block and
call the existing recalculation helper immediately after setJourneyUpdatedAt.
In
`@apis/api-journeys-modern/src/schema/block/image/imageBlockUpdate.mutation.spec.ts`:
- Around line 16-18: The global mock of transformInput hides real
metadata-preserving behavior so the "alt-only" partial-update test doesn't catch
regressions clearing width/height/blurhash; update the tests so the alt-only
case uses the real transformInput (e.g., use jest.requireActual or jest.unmock
for transformInput just for that test) or add explicit assertions in the
alt-only test that width, height and blurhash are preserved after update; target
the mocked transformInput helper and the "alt-only" partial-update test so it
exercises/validates metadata preservation.
In `@apis/api-journeys-modern/src/schema/block/image/transformInput.ts`:
- Around line 30-43: The code in transformInput.ts currently calls
fetch(input.src) (and processes the response) which enables SSRF and resource
abuse; modify the implementation to stop trusting arbitrary URLs by either
requiring a pre-signed/uploaded file or routing image fetches through a trusted
proxy/allowlist service, and add strict safeguards: validate input.src against a
configured host allowlist, enforce network timeouts, check Content-Type and
Content-Length before reading, and enforce a maximum image size and decode
limits before passing buffer to sharp (update the logic around fetch(input.src),
Buffer.from(await response.arrayBuffer()), and subsequent sharp(...) calls).
Ensure these checks are centralized so callers supply only validated/resolved
image buffers or a safe URL.
- Around line 22-29: The current code always overwrites image metadata with
width:0/height:0/blurhash:'' before checking input.src, treating an absent
property like an explicit null; update transformInput (in transformInput.ts) so
you only clear metadata when input.src is explicitly null (input.src === null),
and if src is omitted (property not present / undefined) leave existing
width/height/blurhash untouched; move or conditionalize the assignments to
transformedInput (or build transformedInput differently) so the zeroing happens
only on explicit null while returning the original/input-spread otherwise.
In
`@apis/api-journeys-modern/src/schema/block/radioOption/radioOptionBlockUpdate.mutation.spec.ts`:
- Around line 168-175: The test currently only asserts that tx.block.update's
data includes label, which allows other fields to be sent as null; update the
assertion on tx.block.update (the call that includes where: { id }) so it
verifies the data payload is strictly a partial update with only the intended
keys: either assert data equals the exact object { label: 'Only label' } or, if
other keys exist on the mocked block, assert that those specific keys (e.g.,
description, options, etc. from the mocked block) are not present or not set to
null via negative assertions (e.g., expect.not.toHaveProperty or
expect.not.objectContaining) to guarantee no extra fields are sent as null when
calling tx.block.update for id.
🪄 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: e55f2344-344c-472d-a275-7d2e690fa281
📒 Files selected for processing (17)
.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.ts
…Cursor, documenting Prisma and GraphQL update processes
|
Found 1 test failure on Blacksmith runners: Failure
|
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Tests