refactor: update CMS service methods to use getBlockConfig for block retrieval#759
Merged
lukasz-hycom merged 6 commits intomainfrom Mar 31, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization 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)
WalkthroughConsolidates CMS access by replacing many block-specific CmsService getters with a single generic getBlockConfig(options) dispatcher; request types gained a blockType discriminator and controllers/integrations/block consumers were updated to call getBlockConfig with corresponding blockType. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client"
participant Controller as "CMSController"
participant Service as "CmsService"
participant Provider as "IntegrationProvider"
participant Cache as "MapperCache"
Client->>Controller: GET /cms/block?id,locale,blockType
Controller->>Service: getBlockConfig({ id, locale, blockType })
Service->>Provider: fetch raw entry / GraphQL by id/locale
Provider->>Cache: check cache / map entry
Cache-->>Provider: mapped block (or miss)
Provider-->>Service: Observable<T> (mapped block)
Service-->>Controller: Observable<T>
Controller-->>Client: HTTP response with block payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![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: 3
🧹 Nitpick comments (1)
packages/blocks/surveyjs-form/src/api-harmonization/surveyjs.service.ts (1)
22-22: Minor:forkJoinwrapping a single observable is unnecessary.Since there's only one observable source, you could simplify to
cms.pipe(map(...))directly. However, if this pattern is intentional for consistency across block services or future extensibility, feel free to keep it.♻️ Optional simplification
- return forkJoin([cms]).pipe(map(([cms]) => mapSurveyjs(cms, headers['x-locale']))); + return cms.pipe(map((cmsResult) => mapSurveyjs(cmsResult, headers['x-locale'])));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blocks/surveyjs-form/src/api-harmonization/surveyjs.service.ts` at line 22, The code uses forkJoin to wrap a single observable (cms) which is unnecessary; replace the forkJoin([cms]).pipe(map(([cms]) => mapSurveyjs(cms, headers['x-locale']))) with a direct cms.pipe(map(cms => mapSurveyjs(cms, headers['x-locale']))) call (or keep forkJoin if you intentionally want uniformity), locating the change around the forkJoin usage in surveyjs.service.ts where mapSurveyjs and headers['x-locale'] are referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/framework/src/modules/cms/cms.controller.ts`:
- Around line 103-109: The getArticleDetailsBlock handler currently calls
this.cms.getBlockConfig with blockType hard-coded to 'ArticleListBlock' (see
getArticleDetailsBlock and Model.ArticleListBlock.ArticleListBlock) which is
incorrect; update the blockType argument to the correct value (e.g.
'ArticleDetailsBlock') in the this.cms.getBlockConfig call so the
/blocks/article-details route dispatches to the proper block, or if the
ArticleDetails block isn't implemented yet, restore/keep the old implementation
used previously instead of calling getBlockConfig.
In `@packages/integrations/contentful-cms/src/modules/cms/cms.service.ts`:
- Around line 452-523: The getBlockConfig method is missing cases for the
CmsBlockType variants TicketSummaryBlock, NotificationSummaryBlock,
BentoGridBlock, CtaSectionBlock, DocumentListBlock, FeatureSectionBlock,
FeatureSectionGridBlock, HeroSectionBlock, MediaSectionBlock, and
PricingSectionBlock which causes valid requests to hit the default
NotFoundException branch; update getBlockConfig to add case branches for each
missing block and route them to the appropriate mapping functions (e.g.,
mapTicketSummaryBlock, mapNotificationSummaryBlock, mapBentoGridBlock,
mapCtaSectionBlock, mapDocumentListBlock, mapFeatureSectionBlock,
mapFeatureSectionGridBlock, mapHeroSectionBlock, mapMediaSectionBlock,
mapPricingSectionBlock) using the same pattern as other blocks (either
of(mapX(...)) when no network call is needed or this.getCachedBlock(key, () =>
this.getBlock(options).pipe(map(mapX))) when mapping requires the fetched
block), and ensure the corresponding map* functions are imported or created to
restore parity with the shared CmsBlockType contract.
In `@packages/integrations/strapi-cms/src/modules/cms/cms.service.ts`:
- Around line 299-416: getBlockConfig is missing switch cases for ten CMS block
types causing NotFoundException; add cases for BentoGridBlock, CtaSectionBlock,
DocumentListBlock, FeatureSectionBlock, FeatureSectionGridBlock,
HeroSectionBlock, MediaSectionBlock, NotificationSummaryBlock,
PricingSectionBlock, and TicketSummaryBlock inside the getBlockConfig<T> method
so each returns the appropriate Observable (use this.getCachedBlock(key, () =>
this.getBlock(options).pipe(map(mapXBlock))) or of(mapXBlock(...)) consistent
with existing patterns); reference the existing mapping helper names (e.g.,
mapBentoGridBlock, mapCtaSectionBlock, mapDocumentListBlock,
mapFeatureSectionBlock, mapFeatureSectionGridBlock, mapHeroSectionBlock,
mapMediaSectionBlock, mapNotificationSummaryBlock, mapPricingSectionBlock,
mapTicketSummaryBlock) and ensure locales/baseUrl are passed when other mappings
require them, keeping the default NotFoundException fallback intact.
---
Nitpick comments:
In `@packages/blocks/surveyjs-form/src/api-harmonization/surveyjs.service.ts`:
- Line 22: The code uses forkJoin to wrap a single observable (cms) which is
unnecessary; replace the forkJoin([cms]).pipe(map(([cms]) => mapSurveyjs(cms,
headers['x-locale']))) with a direct cms.pipe(map(cms => mapSurveyjs(cms,
headers['x-locale']))) call (or keep forkJoin if you intentionally want
uniformity), locating the change around the forkJoin usage in
surveyjs.service.ts where mapSurveyjs and headers['x-locale'] are referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 751bddfd-6f9d-4d0c-ba18-3cc2f7e52953
📒 Files selected for processing (42)
apps/api-harmonization/src/modules/organizations/organizations.service.tspackages/blocks/article-list/src/api-harmonization/article-list.service.tspackages/blocks/article-search/src/api-harmonization/article-search.service.tspackages/blocks/bento-grid/src/api-harmonization/bento-grid.service.tspackages/blocks/category-list/src/api-harmonization/category-list.service.tspackages/blocks/category/src/api-harmonization/category.service.tspackages/blocks/cta-section/src/api-harmonization/cta-section.service.tspackages/blocks/document-list/src/api-harmonization/document-list.service.tspackages/blocks/faq/src/api-harmonization/faq.service.tspackages/blocks/feature-section-grid/src/api-harmonization/feature-section-grid.service.tspackages/blocks/feature-section/src/api-harmonization/feature-section.service.tspackages/blocks/featured-service-list/src/api-harmonization/featured-service-list.service.tspackages/blocks/hero-section/src/api-harmonization/hero-section.service.tspackages/blocks/invoice-list/src/api-harmonization/invoice-list.service.tspackages/blocks/media-section/src/api-harmonization/media-section.service.tspackages/blocks/notification-details/src/api-harmonization/notification-details.service.tspackages/blocks/notification-list/src/api-harmonization/notification-list.service.tspackages/blocks/notification-summary/src/api-harmonization/notification-summary.service.tspackages/blocks/order-details/src/api-harmonization/order-details.service.tspackages/blocks/order-list/src/api-harmonization/order-list.service.tspackages/blocks/orders-summary/src/api-harmonization/orders-summary.service.tspackages/blocks/payments-history/src/api-harmonization/payments-history.service.tspackages/blocks/payments-summary/src/api-harmonization/payments-summary.service.tspackages/blocks/pricing-section/src/api-harmonization/pricing-section.service.tspackages/blocks/product-details/src/api-harmonization/product-details.service.tspackages/blocks/product-list/src/api-harmonization/product-list.service.tspackages/blocks/quick-links/src/api-harmonization/quick-links.service.tspackages/blocks/recommended-products/src/api-harmonization/recommended-products.service.tspackages/blocks/service-details/src/api-harmonization/service-details.service.tspackages/blocks/service-list/src/api-harmonization/service-list.service.tspackages/blocks/surveyjs-form/src/api-harmonization/surveyjs.service.tspackages/blocks/ticket-details/src/api-harmonization/ticket-details.service.tspackages/blocks/ticket-list/src/api-harmonization/ticket-list.service.tspackages/blocks/ticket-recent/src/api-harmonization/ticket-recent.service.tspackages/blocks/ticket-summary/src/api-harmonization/ticket-summary.service.tspackages/blocks/user-account/src/api-harmonization/user-account.service.tspackages/framework/src/modules/cms/cms.controller.tspackages/framework/src/modules/cms/cms.request.tspackages/framework/src/modules/cms/cms.service.tspackages/integrations/contentful-cms/src/modules/cms/cms.service.tspackages/integrations/mocked/src/modules/cms/cms.service.tspackages/integrations/strapi-cms/src/modules/cms/cms.service.ts
Contributor
… feature/refactor-cms-getblockconfig
… feature/refactor-cms-getblockconfig
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/eleven-rabbits-laugh.md (1)
8-8: Expand the changeset description to clearly communicate the main API refactoring.The current description focuses narrowly on checkout blocks and mocked-dxp, but doesn't explain the fundamental API change affecting all four packages. Consumers need to understand that block-specific methods (e.g.,
getFaqBlock,getTicketListBlock) have been replaced with a single genericgetBlockConfig<T>(options)method that accepts ablockTypeparameter.📝 Suggested comprehensive changeset description
-Added checkout block type cases (Cart, CheckoutCompanyData, CheckoutShippingAddress, CheckoutBillingPayment, CheckoutSummary, OrderConfirmation) to the getBlockConfig switch in mocked CMS service. Refactored mocked-dxp to use getBlockConfig override instead of individual block method overrides. +Refactored CMS service API to consolidate block-specific methods into a single generic `getBlockConfig<T>(options)` method. All CMS integrations (Contentful, Strapi, Mocked) now use a `blockType` parameter to dispatch to the appropriate block configuration. + +**Breaking change**: Replace calls to specific methods (e.g., `getFaqBlock()`, `getTicketListBlock()`) with `getBlockConfig({ blockType: 'FaqBlock', ... })`. + +Additional changes: +- Added checkout block type cases (Cart, CheckoutCompanyData, CheckoutShippingAddress, CheckoutBillingPayment, CheckoutSummary, OrderConfirmation) to mocked CMS service +- Updated mocked-dxp to use `getBlockConfig` override pattern with `super.getBlockConfig()` delegation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/eleven-rabbits-laugh.md at line 8, Update the changeset text to clearly describe the API refactor: explain that block-specific methods such as getFaqBlock and getTicketListBlock have been removed and replaced across all four packages with a single generic getBlockConfig<T>(options) method that takes a blockType parameter; mention that the mocked-dxp implementation was refactored to override getBlockConfig instead of individual block methods and list the newly added checkout block types (Cart, CheckoutCompanyData, CheckoutShippingAddress, CheckoutBillingPayment, CheckoutSummary, OrderConfirmation) so consumers understand the breaking change and how to migrate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/quick-hoops-cheer.md:
- Around line 2-42: The changeset adds many block packages but cms.service.ts's
getBlockConfig (in
packages/integrations/contentful-cms/src/modules/cms/cms.service.ts) currently
throws for several valid CmsBlockType values (defined in
packages/framework/src/modules/cms/cms.request.ts) such as TicketSummaryBlock,
NotificationSummaryBlock, BentoGridBlock, CtaSectionBlock, DocumentListBlock,
FeatureSectionBlock, FeatureSectionGridBlock, HeroSectionBlock,
MediaSectionBlock, PricingSectionBlock, CartBlock, CheckoutCompanyDataBlock,
CheckoutShippingAddressBlock, CheckoutBillingPaymentBlock, and
OrderConfirmationBlock; either extend getBlockConfig to handle these
CmsBlockType enum values by adding corresponding case branches that return the
proper block config (or a safe default) for each named block type, or
remove/defer the related package bumps from the changeset so the release does
not include blocks that cms.service.ts cannot handle.
In @.changeset/tired-brooms-start.md:
- Around line 2-3: The changeset currently marks '@o2s/framework' and
'@o2s/api-harmonization' as minor, but replacing many CmsService abstract
methods with a single getBlockConfig<T>() is a breaking API change; update the
changeset to use "major" for '@o2s/framework' (and also set
'@o2s/api-harmonization' to "major" if its public API is affected), update the
summary text to reflect the breaking change to CmsService and
getBlockConfig<T>(), and re-run your changeset/versioning commands so the
package releases reflect a major bump.
---
Nitpick comments:
In @.changeset/eleven-rabbits-laugh.md:
- Line 8: Update the changeset text to clearly describe the API refactor:
explain that block-specific methods such as getFaqBlock and getTicketListBlock
have been removed and replaced across all four packages with a single generic
getBlockConfig<T>(options) method that takes a blockType parameter; mention that
the mocked-dxp implementation was refactored to override getBlockConfig instead
of individual block methods and list the newly added checkout block types (Cart,
CheckoutCompanyData, CheckoutShippingAddress, CheckoutBillingPayment,
CheckoutSummary, OrderConfirmation) so consumers understand the breaking change
and how to migrate.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6dcef44a-173c-4a0d-a4be-5a7dd59a8452
📒 Files selected for processing (3)
.changeset/eleven-rabbits-laugh.md.changeset/quick-hoops-cheer.md.changeset/tired-brooms-start.md
Contributor
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 `@apps/docs/docs/guides/integrations/extending-framework-modules.md`:
- Around line 330-332: The example snippet uses headers[H.Locale] but does not
import H; update the snippet header to add the missing import for H so the
reference resolves (ensure the snippet shows the import alongside other
imports), then verify the example including cmsService.getBlockConfig and
CMS.Model.TicketListBlock.TicketListBlock still compiles when copy-pasted.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9269734-a7c0-4d44-aa9b-ef2a857c709c
📒 Files selected for processing (2)
apps/docs/docs/guides/integrations/extending-framework-modules.mdapps/docs/docs/integrations/cms/strapi/blocks.md
marcinkrasowski
approved these changes
Mar 31, 2026
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.
Key Changes
Summary by CodeRabbit