Conversation
… and related products
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughReplaces product-details action links with coming-soon tooltips; adds FilterViewMode toggle and list view to ProductList, renames/extends product-list label keys, defaults product-list request type to PHYSICAL, adds pagination defaults in mocked mappers, and clears permissions in mocked CMS page fixtures. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as ProductList.client
participant Filters as FiltersSection
participant API as ProductListService
participant Renderer as DataList/ProductGrid
User->>Filters: toggle viewMode / change filters
Filters->>UI: onChange(viewMode / filters)
UI->>API: fetch products (filters, type='PHYSICAL', offset/limit)
API-->>UI: return product data (paged)
UI->>Renderer: render DataList (list) or ProductGrid (grid)
Renderer-->>User: display products
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-26T11:57:00.632ZApplied to files:
🧬 Code graph analysis (2)packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.product-list.mapper.ts (2)
packages/ui/src/components/Filters/Filters.tsx (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (8)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/blocks/product-list/src/api-harmonization/product-list.service.ts`:
- Line 31: The code in product-list.service.ts hardcodes type: 'PHYSICAL' which
ignores any caller-provided value on GetProductListBlockQuery.query.type; either
use the incoming value (e.g., set type: query.type ?? 'PHYSICAL') so callers can
override it, or remove the optional type from GetProductListBlockQuery (and any
related callsites) to make the contract explicit; mirror the behavior used in
service-list.service.ts if you intend to accept caller types, and update typings
and/or documentation accordingly so the inconsistency is resolved.
🧹 Nitpick comments (3)
packages/integrations/mocked/src/modules/products/products.mapper.ts (2)
70-70: Inconsistent type coercion foroffset.The slice uses
offsetdirectly for the start index but wraps it inNumber()for the end calculation. Ifoffsetcomes from query parameters as a string (e.g.,"5"), JavaScript's implicit coercion inslice()handles it, but this is fragile and inconsistent.♻️ Suggested fix for consistent type handling
return { - data: data.slice(offset, Number(offset) + Number(limit)), + data: data.slice(Number(offset), Number(offset) + Number(limit)), total: data.length, };
122-122: Same type coercion inconsistency asmapProducts.Apply the same fix here for consistency.
♻️ Suggested fix
return { - data: data.slice(offset, Number(offset) + Number(limit)), + data: data.slice(Number(offset), Number(offset) + Number(limit)), total: data.length, };packages/blocks/product-details/src/frontend/ProductDetails.client.tsx (1)
19-26: Remove the unusedroutingprop from destructuring.The
routingprop is destructured on line 21 but never referenced in the component since Link-based navigation was replaced withTooltipHover. Remove it from the parameter destructuring and consider removing it from theProductDetailsPurePropstype definition if parent components can be updated accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (21)
packages/integrations/mocked/public/images/accesories_cable.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/accesories_drill.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/accesories_saw.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-004_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-004_1.pngis excluded by!**/*.pngpackages/integrations/mocked/public/images/prd-005_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-006_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-007_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-008_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-009_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-010_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-011_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-012_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-013_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-014_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-015_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-016_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-017_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-018_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-019_1.jpgis excluded by!**/*.jpgpackages/integrations/mocked/public/images/prd-020_1.jpgis excluded by!**/*.jpg
📒 Files selected for processing (6)
packages/blocks/product-details/src/frontend/ProductDetails.client.tsxpackages/blocks/product-list/src/api-harmonization/product-list.service.tspackages/integrations/mocked/src/modules/cms/mappers/mocks/pages/product-details.page.tspackages/integrations/mocked/src/modules/cms/mappers/mocks/pages/product-list.page.tspackages/integrations/mocked/src/modules/products/products.mapper.tspackages/integrations/mocked/src/modules/resources/mock/products.mock.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-13T15:40:10.528Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 348
File: packages/blocks/notification-summary/src/frontend/NotificationSummary.renderer.tsx:1-15
Timestamp: 2025-11-13T15:40:10.528Z
Learning: In next-intl 3.0+, hook-style APIs like useLocale(), useTranslations(), and useFormatter() can be used in non-async Server Components without the 'use client' directive. The library provides server-optimized implementations automatically. Only async Server Components need to use the await-based APIs like getLocale() from next-intl/server.
Applied to files:
packages/blocks/product-details/src/frontend/ProductDetails.client.tsx
📚 Learning: 2025-12-03T12:34:20.707Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 419
File: apps/frontend/package.json:52-52
Timestamp: 2025-12-03T12:34:20.707Z
Learning: In the openselfservice repository, the application does not use Next.js cache components (use cache/cacheComponents), so next-intl compatibility limitations related to those experimental features are not a concern.
Applied to files:
packages/blocks/product-details/src/frontend/ProductDetails.client.tsx
📚 Learning: 2025-11-26T11:57:00.632Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 411
File: packages/framework/src/modules/cms/models/blocks/product-list.model.ts:29-46
Timestamp: 2025-11-26T11:57:00.632Z
Learning: In the framework layer (packages/framework/src/modules/cms/models/blocks/*.model.ts), block classes like ProductListBlock should NOT include explicit __typename discriminators. The __typename field is added at the API harmonization layer (packages/blocks/*/src/api-harmonization/*.model.ts) where it's needed for discriminated unions. The framework maintains normalized data models without these discriminators.
Applied to files:
packages/blocks/product-list/src/api-harmonization/product-list.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-preview
🔇 Additional comments (4)
packages/integrations/mocked/src/modules/cms/mappers/mocks/pages/product-list.page.ts (1)
22-22: Permissions removed from mock pages.Setting
permissions: []makes these product list pages accessible without role restrictions in the mocked environment. Ensure this aligns with testing requirements and doesn't inadvertently expose functionality that should remain protected during integration testing.packages/integrations/mocked/src/modules/cms/mappers/mocks/pages/product-details.page.ts (1)
20-20: Permissions removed consistently with product-list mock.The change is consistent with the parallel updates in
product-list.page.ts. No concerns for mock data.packages/blocks/product-details/src/frontend/ProductDetails.client.tsx (2)
167-182: LGTM - TooltipHover implementation for desktop action button.The tooltip pattern correctly disables the action while providing user feedback. Using
useTranslations()for the "coming soon" message ensures proper localization.
200-212: Consistent implementation for mobile action button.The mobile tooltip mirrors the desktop implementation appropriately.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
What does this PR do?
Related Ticket(s)
Key Changes
How to test
Media (Loom or gif)
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.