Merged
Conversation
…api-journeys-override-skill
…odify existing iconBlockCreate mutation for modern API
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)
WalkthroughAdds an Apollo Federation override for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as API_Gateway
participant Subgraph as API_JOURNEYS_MODERN
participant Auth as Authorization
participant DB as Database
Client->>Gateway: iconBlockUpdate(id,input,journeyId)
Gateway->>Subgraph: Route mutation (override -> API_JOURNEYS_MODERN)
Subgraph->>Auth: authorizeBlockUpdate(id, user)
Auth-->>Subgraph: allowed / forbidden
alt allowed
Subgraph->>DB: tx.block.update(where:{id}, data:{...input})
DB-->>Subgraph: updatedBlock
Subgraph->>DB: tx.journey.update(...) (side-effect)
DB-->>Subgraph: journeyUpdated
Subgraph-->>Gateway: { iconBlockUpdate: updatedBlock }
Gateway-->>Client: response with updated iconBlock
else forbidden
Subgraph-->>Gateway: error (not allowed)
Gateway-->>Client: error response (data:null)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 ff85123
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 86d34f4
☁️ 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.
|
![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: 2
🧹 Nitpick comments (3)
.cursor/skills/create-api-journeys-override/SKILL.md (1)
251-256: Expand Step 6 to include the required migration quality gates.The guide currently omits lint/type-check and schema validation gates that are required for this migration workflow.
Based on learnings: "Run quality gates before committing: lint, type-check, and test for api-journeys and api-journeys-modern; regenerate GraphQL schema and validate subgraph against Hive; run Prisma generation after schema changes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/skills/create-api-journeys-override/SKILL.md around lines 251 - 256, Update Step 6 to add the required migration quality gates: before regenerating the schema run lint and type-check for both projects (invoke nx run api-journeys:lint, nx run api-journeys:build or nx run api-journeys:tsc and the same for api-journeys-modern), then run tests (nx test api-journeys-modern --testPathPattern="<fieldName>" and nx test api-journeys), then run nx run api-journeys-modern:generate-graphql, validate the generated subgraph against Hive (subgraph validation command used in your repo), and finally run Prisma generation (prisma generate or nx run api-journeys-modern:prisma-generate) after schema changes; ensure the guide lists these commands in order and mentions failing early on any gate.apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.ts (2)
58-73: Unusedactionmock properties.The
action: { upsert, delete }mocks are defined but never verified in any test. IconBlock updates don't involve action mutations, so these can be removed for clarity.♻️ Simplified tx mock
const tx = { block: { update: jest.fn().mockResolvedValue({ id, typename: 'IconBlock', journeyId: 'journeyId', parentBlockId: 'parentId', parentOrder: 0, name: 'CheckCircleRounded', color: 'primary', size: 'md' }) }, - action: { upsert: jest.fn(), delete: jest.fn() }, journey: { update: jest.fn().mockResolvedValue({ id: 'journeyId' }) } }🤖 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 58 - 73, The tx mock includes unused action properties (action.upsert and action.delete) that aren’t used by the IconBlock update tests; remove the entire action: { upsert: jest.fn(), delete: jest.fn() } entry from the tx mock so the mock only contains block.update and journey.update (references: the tx constant and its block.update and journey.update mocks in iconBlockUpdate.mutation.spec.ts).
16-48: Consider adding a test case for block not found scenario.The test suite covers authorized, unauthorized, and partial input cases, but doesn't test what happens when
fetchBlockWithJourneyAclreturnsnull(block doesn't exist). This edge case should return an appropriate error.📝 Suggested test case
it('returns error when block not found', async () => { fetchBlockWithJourneyAcl.mockResolvedValue(null) const result = await authClient({ document: ICON_BLOCK_UPDATE, variables: { id: 'nonexistent', input: { name: 'CheckCircleRounded' } } }) expect(result.errors).toBeDefined() expect(result.data?.iconBlockUpdate).toBeNull() })🤖 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 16 - 48, Add a test that mocks fetchBlockWithJourneyAcl to return null and asserts the mutation returns an error and null data; specifically, in iconBlockUpdate.mutation.spec.ts use fetchBlockWithJourneyAcl.mockResolvedValue(null), call authClient with the ICON_BLOCK_UPDATE mutation and variables (id: 'nonexistent', input: { name: 'CheckCircleRounded' }), then assert result.errors is defined and result.data?.iconBlockUpdate is null to cover the "block not found" edge case.
🤖 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 149-151: The query template uses withAuth({ isAuthenticated: true,
isAnonymous: true }) which doesn't follow the project's $any convention; update
the builder.queryField call for exampleQuery so the withAuth invocation uses the
$any pattern (i.e., wrap isAuthenticated/isAnonymous inside $any) before calling
prismaField, ensuring the snippet matches other schema files' usage of withAuth.
In `@apis/api-journeys-modern/schema.graphql`:
- Around line 1370-1376: The return type for the iconBlockUpdate field is
incorrectly nullable; update the GraphQL field definition for iconBlockUpdate to
return a non-nullable IconBlock (IconBlock!) to match the original api-journeys
contract while keeping the existing arguments and `@override`(from:
"api-journeys") directive unchanged.
---
Nitpick comments:
In @.cursor/skills/create-api-journeys-override/SKILL.md:
- Around line 251-256: Update Step 6 to add the required migration quality
gates: before regenerating the schema run lint and type-check for both projects
(invoke nx run api-journeys:lint, nx run api-journeys:build or nx run
api-journeys:tsc and the same for api-journeys-modern), then run tests (nx test
api-journeys-modern --testPathPattern="<fieldName>" and nx test api-journeys),
then run nx run api-journeys-modern:generate-graphql, validate the generated
subgraph against Hive (subgraph validation command used in your repo), and
finally run Prisma generation (prisma generate or nx run
api-journeys-modern:prisma-generate) after schema changes; ensure the guide
lists these commands in order and mentions failing early on any gate.
In
`@apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.ts`:
- Around line 58-73: The tx mock includes unused action properties
(action.upsert and action.delete) that aren’t used by the IconBlock update
tests; remove the entire action: { upsert: jest.fn(), delete: jest.fn() } entry
from the tx mock so the mock only contains block.update and journey.update
(references: the tx constant and its block.update and journey.update mocks in
iconBlockUpdate.mutation.spec.ts).
- Around line 16-48: Add a test that mocks fetchBlockWithJourneyAcl to return
null and asserts the mutation returns an error and null data; specifically, in
iconBlockUpdate.mutation.spec.ts use
fetchBlockWithJourneyAcl.mockResolvedValue(null), call authClient with the
ICON_BLOCK_UPDATE mutation and variables (id: 'nonexistent', input: { name:
'CheckCircleRounded' }), then assert result.errors is defined and
result.data?.iconBlockUpdate is null to cover the "block not found" edge case.
🪄 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: a0f2cbf6-cfbc-471c-944c-9b58bd7ba790
📒 Files selected for processing (6)
.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.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.cursor/skills/create-api-journeys-override/SKILL.md (1)
93-93: Capture the return value fromauthorizeBlockUpdateto match established patterns.The template calls
authorizeBlockUpdatebut discards its return value. However, the actual implementation incardBlockUpdate.mutation.tscaptures the returned block:const block = await authorizeBlockUpdate(id, context.user)While the simple template might not use the block object, developers following this guide should see the correct pattern. Update to:
- await authorizeBlockUpdate(id, context.user) + const block = await authorizeBlockUpdate(id, context.user)This aligns with the codebase convention and gives developers access to the authorized block if needed for validation logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/skills/create-api-journeys-override/SKILL.md at line 93, The call to authorizeBlockUpdate currently discards its return value; update the code to capture the returned block (e.g., const block = await authorizeBlockUpdate(id, context.user)) so it follows the established pattern used in cardBlockUpdate.mutation.ts and provides the authorized block for any downstream validation or use; ensure any existing references to the previous call are replaced with this captured variable where needed.
🤖 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 66-105: Update the block mutation template to document both
authorization patterns: explain that simple blocks should call
authorizeBlockUpdate(id, context.user) before update(), while complex blocks
should fetch the block via fetchBlockWithJourneyAcl(id) and perform manual
ability(Action.Update, ...) checks with custom error messages before calling
update(); include a short note describing when to use Pattern 2 (e.g., when you
need custom validation or custom error wording prior to authorization) and add
an inline example snippet reference showing fetchBlockWithJourneyAcl +
ability(Action.Update, ...) so readers can copy that flow for
video/multiselect/multiselectOption blocks.
---
Nitpick comments:
In @.cursor/skills/create-api-journeys-override/SKILL.md:
- Line 93: The call to authorizeBlockUpdate currently discards its return value;
update the code to capture the returned block (e.g., const block = await
authorizeBlockUpdate(id, context.user)) so it follows the established pattern
used in cardBlockUpdate.mutation.ts and provides the authorized block for any
downstream validation or use; ensure any existing references to the previous
call are replaced with this captured variable where needed.
🪄 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: f494a995-070c-433e-ba36-0cdc1bc914a0
⛔ Files ignored due to path filters (3)
apps/journeys-admin/__generated__/IconBlockColorUpdate.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/IconBlockNameUpdate.tsis excluded by!**/__generated__/**libs/shared/gql/src/__generated__/graphql-env.d.tsis excluded by!**/__generated__/**
📒 Files selected for processing (5)
.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.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apis/api-journeys-modern/schema.graphql
- apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.ts
- apis/api-journeys-modern/src/schema/block/icon/iconBlockUpdate.mutation.spec.ts
- apis/api-gateway/schema.graphql
This comment has been minimized.
This comment has been minimized.
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 8, 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
Documentation
Tests
Schema