Conversation
|
View your CI Pipeline Execution ↗ for commit 17e926b
☁️ Nx Cloud last updated this comment at |
|
Warning Rate limit exceeded
⌛ 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 (2)
WalkthroughButtonBlock creation was moved from the legacy api-journeys to the api-journeys-modern graph; the API gateway now routes buttonBlockCreate to the MODERN graph. The MODERN variant gained a new buttonBlockUpdate mutation and resolver + tests; legacy create mutation, input type, and resolver/tests were removed. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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)
📝 Coding Plan
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 |
|
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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.spec.ts (1)
23-33: Add one compatibility case that still sendsjourneyId.The schema keeps
journeyIdduring the override window, but this suite never exercises that call shape, so an accidental removal would slip through without a failing test.🤖 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/button/buttonBlockUpdate.mutation.spec.ts` around lines 23 - 33, Add a compatibility test that calls the existing BUTTON_BLOCK_UPDATE GraphQL mutation with the legacy variable shape including journeyId to ensure the schema still accepts it; update the test suite in buttonBlockUpdate.mutation.spec.ts to include a new it(...) case that sends variables containing journeyId alongside id and input (use the same BUTTON_BLOCK_UPDATE constant), assert the response matches expected fields and that no errors occur, and clean up any mock/setup to accommodate this extra call shape.apis/api-journeys-modern/src/schema/block/button/buttonBlockCreate.mutation.spec.ts (2)
107-116: Consider assertingparentOrderin the response.The GraphQL query requests
parentOrder(line 29), and the mock returnsparentOrder: 0(line 69), but the response assertion doesn't verify this field. Adding this assertion would ensure the field is properly returned.♻️ Suggested addition
expect(result).toEqual({ data: { buttonBlockCreate: expect.objectContaining({ id: 'blockId', journeyId: 'journeyId', parentBlockId: 'parentId', - label: 'Button Label' + label: 'Button Label', + parentOrder: 0 }) } })🤖 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/button/buttonBlockCreate.mutation.spec.ts` around lines 107 - 116, The test for buttonBlockCreate is missing an assertion for the parentOrder field returned by the mutation; update the expectation on result to include parentOrder: 0 alongside id/journeyId/parentBlockId/label so the test verifies the field requested in the GraphQL query (refer to the result object and buttonBlockCreate in buttonBlockCreate.mutation.spec.ts and the mock that returns parentOrder: 0).
35-38: Consider usingjest.mocked()helper for better type safety.Using
jest.mocked()instead ofrequire+ type casting provides better TypeScript support and is the recommended approach in modern Jest.♻️ Suggested refactor
- const { - fetchJourneyWithAclIncludes - } = require('../../../lib/auth/fetchJourneyWithAclIncludes') - const mockAbility = ability as jest.MockedFunction<typeof ability> + const mockFetchJourneyWithAclIncludes = jest.mocked( + (await import('../../../lib/auth/fetchJourneyWithAclIncludes')).fetchJourneyWithAclIncludes + ) + const mockAbility = jest.mocked(ability)Alternatively, if the import approach doesn't work with your setup,
jest.mocked()can still be used for the ability cast:- const mockAbility = ability as jest.MockedFunction<typeof ability> + const mockAbility = jest.mocked(ability)🤖 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/button/buttonBlockCreate.mutation.spec.ts` around lines 35 - 38, Replace the manual require and cast pattern with Jest's typed helper: import or require the module exporting fetchJourneyWithAclIncludes and use jest.mocked(fetchJourneyWithAclIncludes) (or jest.mocked(ability) for the ability helper) to create typed mocks; update references to mockAbility to use the value returned by jest.mocked(ability) and remove the explicit "as jest.MockedFunction" cast so the mock has proper TypeScript types and IntelliSense.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apis/api-journeys-modern/src/schema/block/button/buttonBlockCreate.mutation.ts`:
- Around line 45-60: Before creating the ButtonBlock, verify that when
input.parentBlockId is provided the referenced parent block belongs to the same
journey (input.journeyId): inside the prisma.$transaction (use tx), query the
parent block (e.g., tx.block.findUnique/findFirst) filtering by id and journeyId
and if not found throw/return a validation error; only proceed to
tx.block.create and connect parentBlock by id after this check so
getSiblingsInternal and parentOrder remain consistent; reference
getSiblingsInternal, tx.block.create, input.parentBlockId, and input.journeyId
when adding this guard.
In
`@apis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.ts`:
- Around line 48-76: Reorder the checks so legacy icon-validation runs before
loading the target block and ACL: move the validateBlock checks for
input.startIconId and input.endIconId to occur before the call to
fetchBlockWithJourneyAcl(id). Keep the same error types and messages (throw
GraphQLError with extensions { code: 'BAD_USER_INPUT' } when validateBlock
fails) and leave the subsequent ability(...) / fetchBlockWithJourneyAcl(id)
logic (including Action.Update and abilitySubject('Journey', block.journey))
unchanged to preserve wire-compatible behavior.
---
Nitpick comments:
In
`@apis/api-journeys-modern/src/schema/block/button/buttonBlockCreate.mutation.spec.ts`:
- Around line 107-116: The test for buttonBlockCreate is missing an assertion
for the parentOrder field returned by the mutation; update the expectation on
result to include parentOrder: 0 alongside id/journeyId/parentBlockId/label so
the test verifies the field requested in the GraphQL query (refer to the result
object and buttonBlockCreate in buttonBlockCreate.mutation.spec.ts and the mock
that returns parentOrder: 0).
- Around line 35-38: Replace the manual require and cast pattern with Jest's
typed helper: import or require the module exporting fetchJourneyWithAclIncludes
and use jest.mocked(fetchJourneyWithAclIncludes) (or jest.mocked(ability) for
the ability helper) to create typed mocks; update references to mockAbility to
use the value returned by jest.mocked(ability) and remove the explicit "as
jest.MockedFunction" cast so the mock has proper TypeScript types and
IntelliSense.
In
`@apis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.spec.ts`:
- Around line 23-33: Add a compatibility test that calls the existing
BUTTON_BLOCK_UPDATE GraphQL mutation with the legacy variable shape including
journeyId to ensure the schema still accepts it; update the test suite in
buttonBlockUpdate.mutation.spec.ts to include a new it(...) case that sends
variables containing journeyId alongside id and input (use the same
BUTTON_BLOCK_UPDATE constant), assert the response matches expected fields and
that no errors occur, and clean up any mock/setup to accommodate this extra call
shape.
🪄 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: 1c0aeb61-55ad-43f0-8bf5-dbccd5bdcfb8
📒 Files selected for processing (7)
apis/api-gateway/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys-modern/src/schema/block/button/buttonBlockCreate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/button/buttonBlockCreate.mutation.tsapis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/button/index.ts
- Updated documentation to clarify the process for adding customizable fields to block and action types, emphasizing the need for synchronization between legacy and modern API implementations. - Added new paths for modern API schemas in customizable blocks. - Implemented logic to ensure recalculation of customizable fields after mutations affecting blocks and actions. - Removed deprecated adminJourneys query from legacy API and mirrored it in modern API. - Added tests for customizable journey logic to ensure proper functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.ts`:
- Around line 11-20: The query in validateBlock uses prisma.block.findUnique
with a non-unique field (deletedAt); replace the findUnique call in
validateBlock with prisma.block.findFirst and include the compound condition in
the where clause (e.g., where: { id, parentBlockId, deletedAt: null }) so you
can simply return whether a matching block exists; update the call site in the
validateBlock function accordingly.
🪄 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: 429d3b3a-842a-45fb-82f1-361061bc7959
📒 Files selected for processing (3)
apis/api-journeys-modern/src/schema/block/button/buttonBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/service.tsapis/api-journeys/src/app/modules/block/button/button.resolver.spec.ts
💤 Files with no reviewable changes (1)
- apis/api-journeys/src/app/modules/block/button/button.resolver.spec.ts
- Changed the block retrieval method from `findUnique` to `findFirst` to include parent block ID in the query. - Simplified validation logic to ensure the existence of the block without checking the parent block ID directly.
2 participants
Summary by CodeRabbit