feat: update iconBlockCreate mutation to use modern API and import ne…#8883
Open
mikeallisonJS wants to merge 20 commits intomainfrom
Open
feat: update iconBlockCreate mutation to use modern API and import ne…#8883mikeallisonJS wants to merge 20 commits intomainfrom
mikeallisonJS wants to merge 20 commits intomainfrom
Conversation
- Moved `buttonBlockUpdate` mutation to the modern API schema and adjusted its input type. - Updated `cardBlockCreate` mutation to use the modern API. - Removed deprecated `journeyId` parameter from `buttonBlockUpdate` in the legacy schema. - Cleaned up related resolver and input files for consistency and clarity.
…for modern API - Removed the `cardBlockCreate` mutation from the legacy API and adjusted related resolver tests. - Updated the `cardBlockUpdate` mutation to include the `journeyId` parameter in the modern API schema. - Cleaned up input types and resolver files for consistency across APIs.
- Eliminated the `CardBlockCreateInput` type from the resolver and related test files. - Streamlined the `card.resolver.ts` and `card.resolver.spec.ts` for improved clarity and consistency.
- Changed the `cardBlockUpdate` mutation in both legacy and modern API schemas to make the return type non-nullable. - Updated the `journeyId` parameter in the `cardBlockUpdate` mutation to be non-nullable in the modern API schema. - Ensured consistency across API schemas by aligning mutation definitions.
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)
WalkthroughThe pull request updates the GraphQL federation routing of the Changes
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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run journeys-e2e:e2e |
❌ Failed | 3m 25s | View ↗ |
nx run journeys-admin-e2e:e2e |
✅ Succeeded | 31s | View ↗ |
nx run watch-e2e:e2e |
✅ Succeeded | 27s | View ↗ |
nx run resources-e2e:e2e |
✅ Succeeded | 26s | View ↗ |
nx run player-e2e:e2e |
✅ Succeeded | 3s | View ↗ |
nx run videos-admin-e2e:e2e |
✅ Succeeded | 4s | View ↗ |
nx run short-links-e2e:e2e |
✅ Succeeded | 3s | View ↗ |
nx run-many --target=vercel-alias --projects=jo... |
✅ Succeeded | 2s | View ↗ |
Additional runs (20) |
✅ Succeeded | ... | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-23 23:19:06 UTC
|
View your CI Pipeline Execution ↗ for commit 8435fed
☁️ 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.
|
…00-MA-refactor-iconblockcreate-modern
…00-MA-refactor-iconblockcreate-modern
![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: 1
🧹 Nitpick comments (1)
apis/api-journeys-modern/src/schema/block/card/cardBlockUpdate.mutation.spec.ts (1)
141-212: Add a failingparentBlockIdcase.The new
validateParentBlock()branch only has a happy-path assertion here. If that validation call is accidentally removed, this suite still stays green. Add a case where the parent is missing or from another journey and assert the mutation fails.🧪 Suggested coverage
+ it('returns BAD_USER_INPUT when parentBlockId is outside the journey', async () => { + fetchBlockWithJourneyAcl.mockResolvedValue({ + id, + journeyId: 'journeyId', + journey: { id: 'journeyId' } + }) + mockAbility.mockReturnValue(true) + prismaMock.block.findFirst.mockResolvedValueOnce(null) + + const result = await authClient({ + document: CARD_BLOCK_UPDATE, + variables: { id, input: { parentBlockId: 'otherParentId' } } + }) + + expect(result).toEqual({ + data: null, + errors: [ + expect.objectContaining({ + message: 'parent block not found in journey' + }) + ] + }) + })🤖 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/card/cardBlockUpdate.mutation.spec.ts` around lines 141 - 212, Add a negative test that asserts validateParentBlock() actually blocks invalid parents: in cardBlockUpdate.mutation.spec.ts create a new it() where fetchBlockWithJourneyAcl.mockResolvedValue(...) and mockAbility.mockReturnValue(true) as before, then set prismaMock.block.findFirst.mockResolvedValue(null) (or a block with a different journeyId) to simulate a missing/wrong parent and call authClient({ document: CARD_BLOCK_UPDATE, variables: { id, input: { parentBlockId: 'badParentId', ... } } }); assert the mutation fails (expect an error response or that authClient throws) and that tx.block.update is not called—this ensures validateParentBlock() path is covered and will fail the test if removed.
🤖 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-gateway/schema.graphql`:
- Around line 354-362: The gateway schema still uses `@join__field`(... override:
"api-journeys") for the cardBlockCreate and cardBlockUpdate fields (symbols:
cardBlockCreate, cardBlockUpdate, `@join__field` override: "api-journeys"), but
the legacy service no longer exposes those mutations; remove the override
attribute from the `@join__field` directives on cardBlockCreate and
cardBlockUpdate so ownership is explicit (or alternatively restore the legacy
mutations in the legacy service if you need a fallback), ensuring the gateway no
longer references a non-existent override path.
---
Nitpick comments:
In
`@apis/api-journeys-modern/src/schema/block/card/cardBlockUpdate.mutation.spec.ts`:
- Around line 141-212: Add a negative test that asserts validateParentBlock()
actually blocks invalid parents: in cardBlockUpdate.mutation.spec.ts create a
new it() where fetchBlockWithJourneyAcl.mockResolvedValue(...) and
mockAbility.mockReturnValue(true) as before, then set
prismaMock.block.findFirst.mockResolvedValue(null) (or a block with a different
journeyId) to simulate a missing/wrong parent and call authClient({ document:
CARD_BLOCK_UPDATE, variables: { id, input: { parentBlockId: 'badParentId', ... }
} }); assert the mutation fails (expect an error response or that authClient
throws) and that tx.block.update is not called—this ensures
validateParentBlock() path is covered and will fail the test if removed.
🪄 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: ddd9f4a6-569c-4efb-bce2-9629fb7e3a55
⛔ Files ignored due to path filters (47)
apis/api-journeys/src/__generated__/graphql.tsis excluded by!**/__generated__/**apis/api-journeys/src/app/__generated__/graphql.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/BlockActionDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/BlockDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/BlockDeleteForCoverImage.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/BlockOrderUpdate.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardCtaDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardFormDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardIntroDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardPollDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardQuoteDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardVideoDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CardVideoRestore.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CoverBlockDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/CoverBlockRestore.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/DuplicatedBlock.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/GetStepBlocksWithPosition.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/JourneyImageBlockDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/MenuBlockDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/MultiselectWithButtonDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/MultiselectWithButtonRestore.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/NewBlock.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/PosterImageBlockDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/PosterImageBlockRestore.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/RadioOptionImageDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/RadioOptionImageRestore.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/RestoredBlock.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockCreateFromAction.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockCreateFromSocialPreview.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockDeleteFromAction.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockDeleteFromActionWithoutAction.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockDeleteFromSocialPreview.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockDeleteFromStep.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockRestoreFromAction.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/StepBlockRestoreFromSocialPreview.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/TestJourney.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/TextResponseWithButtonDelete.tsis excluded by!**/__generated__/**apps/journeys-admin/__generated__/TranslatedJourney.tsis excluded by!**/__generated__/**apps/journeys/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/journeys/__generated__/TranslatedJourney.tsis excluded by!**/__generated__/**apps/resources/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/resources/__generated__/TranslatedJourney.tsis excluded by!**/__generated__/**apps/watch/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**apps/watch/__generated__/TranslatedJourney.tsis excluded by!**/__generated__/**libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/__generated__/JourneyAiTranslateCreateSubscription.tsis excluded by!**/__generated__/**libs/journeys/ui/src/libs/useJourneyAiTranslateSubscription/__generated__/TranslatedJourney.tsis excluded by!**/__generated__/**
📒 Files selected for processing (11)
apis/api-gateway/schema.graphqlapis/api-journeys-modern/schema.graphqlapis/api-journeys-modern/src/schema/block/card/cardBlockUpdate.mutation.spec.tsapis/api-journeys-modern/src/schema/block/card/cardBlockUpdate.mutation.tsapis/api-journeys-modern/src/schema/block/card/index.tsapis/api-journeys/schema.graphqlapis/api-journeys/src/app/modules/block/block.module.tsapis/api-journeys/src/app/modules/block/card/card.graphqlapis/api-journeys/src/app/modules/block/card/card.resolver.spec.tsapis/api-journeys/src/app/modules/block/card/card.resolver.tsapis/api-journeys/src/app/modules/journey/journey.resolver.spec.ts
💤 Files with no reviewable changes (5)
- apis/api-journeys/src/app/modules/block/block.module.ts
- apis/api-journeys/src/app/modules/journey/journey.resolver.spec.ts
- apis/api-journeys/src/app/modules/block/card/card.graphql
- apis/api-journeys/src/app/modules/block/card/card.resolver.ts
- apis/api-journeys/src/app/modules/block/card/card.resolver.spec.ts
✅ Files skipped from review due to trivial changes (1)
- apis/api-journeys-modern/src/schema/block/card/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apis/api-journeys-modern/schema.graphql
apis/api-gateway/schema.graphql
Outdated
Comment on lines
+354
to
+362
| cardBlockCreate(input: CardBlockCreateInput!) : CardBlock! @join__field(graph: API_JOURNEYS_MODERN, override: "api-journeys") | ||
| cardBlockUpdate( | ||
| id: ID! | ||
| input: CardBlockUpdateInput! | ||
| """ | ||
| drop this parameter after merging teams | ||
| """ | ||
| journeyId: ID | ||
| ): CardBlock! @join__field(graph: API_JOURNEYS_MODERN, override: "api-journeys") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Legacy card block mutations in apis/api-journeys/schema.graphql:\n'
rg -n '^\s*cardBlock(Create|Update)\s*\(' apis/api-journeys/schema.graphql || true
printf '\nOverride declarations in the migration path:\n'
rg -n "cardBlock(Create|Update)|override:\s*['\"]api-journeys['\"]" \
apis/api-gateway/schema.graphql \
apis/api-journeys-modern/src/schema/block/cardRepository: JesusFilm/core
Length of output: 3280
Remove override: "api-journeys" or restore the legacy mutations.
cardBlockCreate and cardBlockUpdate in the gateway still carry override: "api-journeys", but apis/api-journeys/schema.graphql no longer defines these mutations. The override directive implies a fallback path that no longer exists—this is a hard cutover disguised as a migration. Either restore the mutations in the legacy service to enable true fallback routing, or remove the override declarations to make the service ownership transfer explicit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apis/api-gateway/schema.graphql` around lines 354 - 362, The gateway schema
still uses `@join__field`(... override: "api-journeys") for the cardBlockCreate
and cardBlockUpdate fields (symbols: cardBlockCreate, cardBlockUpdate,
`@join__field` override: "api-journeys"), but the legacy service no longer exposes
those mutations; remove the override attribute from the `@join__field` directives
on cardBlockCreate and cardBlockUpdate so ownership is explicit (or
alternatively restore the legacy mutations in the legacy service if you need a
fallback), ensuring the gateway no longer references a non-existent override
path.
…nblockcreate-modern
Contributor
|
Found 1 test failure on Blacksmith runners: Failure
|
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
1 participant
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.
…w mutation file
Summary by CodeRabbit