feat: order summary chart visibility #346
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughAdds optional chart visibility and layout controls to orders-summary and payments-summary blocks; augments models and mappers to propagate Changes
Sequence Diagram(s)sequenceDiagram
participant Strapi/CMS
participant Mapper
participant Component
participant UI
Strapi/CMS->>Mapper: Return block including chart.showChart and layout
Mapper->>Component: Supply OrdersSummaryBlock / PaymentsSummaryBlock (with chart, layout)
alt chart.showChart == true
Component->>UI: Render chart card + info cards (layout may affect ordering)
else
Component->>UI: Render info cards only (use layout to arrange)
end
sequenceDiagram
participant CMS
participant PaymentsMapper
participant mapChartData
participant PaymentsComponent
CMS->>PaymentsMapper: Provide invoices, chart config, layout
PaymentsMapper->>mapChartData: Compute BarData[] with months window and segments
mapChartData-->>PaymentsMapper: Return BarData[]
PaymentsMapper->>PaymentsComponent: Return PaymentsSummaryBlock with chart & layout
alt chart.showChart == true
PaymentsComponent->>UI: Dynamically import StackedBarChart and render chart card + info cards
else
PaymentsComponent->>UI: Render info cards arranged by layout only
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/blocks/orders-summary/src/frontend/OrdersSummary.client.tsx (1)
128-209: Guard against missing chart data before readingshowChart.
Line 129 dereferencesdata.chart.showChartwithout confirmingdata.chartexists. When CMS disables the chart entirely (which this PR is supposed to support),data.chartcan beundefined, causing a runtime crash on render. Please deriveshowChartvia optional chaining and only render the chart block once the chart object is present.@@ - return ( + const chart = data.chart; + const showChart = chart?.showChart ?? false; + + return ( @@ - <div - className={cn('w-full flex gap-6', data.chart.showChart ? 'flex-col lg:flex-row' : 'flex-col')} - > - {data.chart.showChart ? ( + <div className={cn('w-full flex gap-6', showChart ? 'flex-col lg:flex-row' : 'flex-col')}> + {showChart && chart ? ( @@ - <InfoCard + <InfoCard @@ - <Card className="h-full w-full"> + <Card className="h-full w-full"> @@ - <Typography variant="subtitle">{data.chart.title}</Typography> + <Typography variant="subtitle">{chart.title}</Typography> @@ - <DoubleLineChart - chartData={data.chart.data} - legend={data.chart.legend} + <DoubleLineChart + chartData={chart.data} + legend={chart.legend}
🧹 Nitpick comments (1)
packages/blocks/payments-summary/src/frontend/PaymentsSummary.client.stories.tsx (1)
70-164: Consider reducing story duplication.The
WithChartstory duplicates most of theDefaultstory configuration (overdue, toBePaid, routing, accessToken, locale). To improve maintainability, consider extracting common configuration into a shared base object.Example refactor:
+const basePaymentsConfig = { + routing: baseRouting, + __typename: 'PaymentsSummaryBlock' as const, + currency: 'EUR', + layout: 'vertical' as const, + overdue: { + title: 'Overdue', + link: { + label: 'Pay online', + icon: 'ArrowUpRight', + url: '', + }, + description: '599 days overdue', + value: { + value: 5544.828051951577, + currency: 'EUR', + }, + isOverdue: true, + icon: 'ThumbsUp', + }, + toBePaid: { + title: 'To be paid', + link: { + label: 'Pay online', + icon: 'ArrowUpRight', + url: '', + }, + description: 'New and overdue', + value: { + value: 1234.56, + currency: 'EUR', + }, + icon: 'CreditCard', + }, + accessToken: baseAccessToken, + locale: 'en', +}; export const Default: Story = { - args: { - routing: baseRouting, - __typename: 'PaymentsSummaryBlock', - id: 'payments-summary-1', - currency: 'EUR', - layout: 'vertical', - overdue: { ... }, - toBePaid: { ... }, - accessToken: baseAccessToken, - locale: 'en', + args: { + ...basePaymentsConfig, + id: 'payments-summary-1', }, }; export const WithChart: Story = { args: { - routing: baseRouting, - __typename: 'PaymentsSummaryBlock', - id: 'payments-summary-7', - currency: 'EUR', - layout: 'vertical', - overdue: { ... }, - toBePaid: { ... }, + ...basePaymentsConfig, + id: 'payments-summary-7', chart: { title: '6-months history', // ... chart config }, - accessToken: baseAccessToken, - locale: 'en', }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/integrations/strapi-cms/generated/strapi.tsis excluded by!**/generated/**
📒 Files selected for processing (17)
packages/blocks/orders-summary/src/api-harmonization/orders-summary.mapper.ts(1 hunks)packages/blocks/orders-summary/src/api-harmonization/orders-summary.model.ts(1 hunks)packages/blocks/orders-summary/src/frontend/OrdersSummary.client.stories.tsx(1 hunks)packages/blocks/orders-summary/src/frontend/OrdersSummary.client.tsx(2 hunks)packages/blocks/payments-summary/src/api-harmonization/payments-summary.mapper.ts(3 hunks)packages/blocks/payments-summary/src/api-harmonization/payments-summary.model.ts(3 hunks)packages/blocks/payments-summary/src/frontend/PaymentsSummary.client.stories.tsx(2 hunks)packages/blocks/payments-summary/src/frontend/PaymentsSummary.client.tsx(2 hunks)packages/framework/src/modules/cms/models/blocks/orders-summary.model.ts(1 hunks)packages/framework/src/modules/cms/models/blocks/payments-summary.model.ts(1 hunks)packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.orders-summary.mapper.ts(3 hunks)packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts(2 hunks)packages/integrations/mocked/src/modules/cms/mappers/mocks/pages/invoice-list.page.ts(3 hunks)packages/integrations/strapi-cms/src/modules/cms/graphql/fragments/blocks/OrdersSummary.graphql(1 hunks)packages/integrations/strapi-cms/src/modules/cms/graphql/fragments/blocks/PaymentsSummary.graphql(1 hunks)packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.orders-summary.mapper.ts(1 hunks)packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts (1)
packages/integrations/strapi-cms/src/modules/cms/mappers/cms.information-card.mapper.ts (1)
mapInfoCard(5-17)
packages/blocks/orders-summary/src/frontend/OrdersSummary.client.tsx (3)
packages/ui/src/lib/utils.ts (1)
cn(5-7)packages/framework/src/utils/models/info-card.ts (1)
InfoCard(3-9)packages/framework/src/utils/models/price.ts (1)
Price(3-7)
packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts (2)
packages/framework/src/modules/cms/models/blocks/payments-summary.model.ts (1)
PaymentsSummaryBlock(13-18)packages/blocks/payments-summary/src/api-harmonization/payments-summary.model.ts (1)
PaymentsSummaryBlock(5-44)
packages/framework/src/modules/cms/models/blocks/payments-summary.model.ts (2)
packages/blocks/payments-summary/src/api-harmonization/payments-summary.model.ts (1)
PaymentsSummaryBlock(5-44)packages/framework/src/utils/models/info-card.ts (1)
InfoCard(3-9)
packages/blocks/payments-summary/src/api-harmonization/payments-summary.mapper.ts (3)
packages/framework/src/modules/cms/models/blocks/payments-summary.model.ts (1)
PaymentsSummaryBlock(13-18)packages/blocks/payments-summary/src/api-harmonization/payments-summary.model.ts (2)
PaymentsSummaryBlock(5-44)BarData(46-52)packages/framework/src/modules/invoices/invoices.model.ts (2)
Invoices(24-24)Invoice(7-22)
packages/blocks/payments-summary/src/frontend/PaymentsSummary.client.tsx (4)
packages/ui/src/lib/utils.ts (1)
cn(5-7)packages/framework/src/utils/models/info-card.ts (1)
InfoCard(3-9)packages/framework/src/utils/models/price.ts (1)
Price(3-7)packages/ui/src/components/DynamicIcon/DynamicIcon.tsx (1)
DynamicIcon(8-38)
⏰ 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 (15)
packages/integrations/mocked/src/modules/cms/mappers/mocks/pages/invoice-list.page.ts (3)
71-78: Consistent structural change across locales.The DE locale configuration mirrors the EN locale changes correctly—
PaymentsSummaryBlockmoved to the top slot with empty left/right slots.
117-124: Consistent structural change across all locales.The PL locale configuration correctly maintains the same structure as EN and DE locales—
PaymentsSummaryBlockin the top slot with empty left/right slots. This consistency ensures uniform behavior across all supported locales.
25-32: The mock data properly demonstrates the new chart configuration—no changes needed.The new optional fields (
showChart,layout) are correctly configured in the mapper file (cms.payments-summary.mapper.ts). The invoice-list.page.ts file appropriately defines only the page structure (slots and block references), while the mapper file handles block-level configuration with the new fields properly set (showChart: truefor EN,layout: 'horizontal'for PL,layout: 'vertical'for DE). The structural reorganization is correct and aligns with the PR objectives.packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts (1)
27-35: Verify intentional locale configuration differences.The EN block includes chart configuration while PL and DE blocks don't. Similarly, PL and DE blocks have layout configuration while EN doesn't. If this is intentional to test different configurations, consider adding comments to clarify the test scenarios.
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.orders-summary.mapper.ts (1)
38-38: LGTM!The mapping of
showChartfrom the component to the chart object is straightforward and aligns with the GraphQL fragment changes.packages/blocks/orders-summary/src/frontend/OrdersSummary.client.stories.tsx (1)
85-85: LGTM!Adding
showChart: trueto the story provides appropriate test data for the new chart visibility feature.packages/integrations/strapi-cms/src/modules/cms/graphql/fragments/blocks/OrdersSummary.graphql (1)
9-9: LGTM!The
showChartfield addition extends the GraphQL fragment appropriately and aligns with the corresponding mapper that nests this field within the chart object.packages/framework/src/modules/cms/models/blocks/orders-summary.model.ts (1)
20-20: LGTM!The optional
showChartfield is appropriately typed and maintains backward compatibility with existing code.packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.orders-summary.mapper.ts (1)
25-25: LGTM!The
showChart: truefield is consistently added across all locale blocks (EN, DE, PL), providing uniform test data.Also applies to: 73-73, 121-121
packages/integrations/strapi-cms/src/modules/cms/graphql/fragments/blocks/PaymentsSummary.graphql (2)
4-4: LGTM!The
layoutfield addition enables layout configuration for the payments summary block.
11-19: LGTM!The
chartobject with comprehensive configuration fields (labels, visibility, and display options) properly extends the fragment to support chart integration into the payments summary block.packages/blocks/orders-summary/src/api-harmonization/orders-summary.model.ts (1)
36-36: LGTM!The optional
showChartfield in the chart object extends the public API appropriately while maintaining backward compatibility.packages/blocks/payments-summary/src/frontend/PaymentsSummary.client.stories.tsx (3)
14-27: LGTM! Good refactoring to centralize shared configuration.The introduction of
baseRoutingandbaseAccessTokenconstants effectively reduces duplication across stories. The JWT token is appropriately a mock token with test data (Jane Doe, example.com), which is suitable for Storybook development context.
36-50: Question: Is the 'ThumbsUp' icon intentional for overdue payments?The
overdueblock usesicon: 'ThumbsUp'(line 49) while indicating a negative state withisOverdue: trueand'599 days overdue'. The ThumbsUp icon typically conveys approval or positive sentiment, which seems semantically inconsistent with overdue debt.If this is placeholder data for Storybook demonstration purposes, consider using a more contextually appropriate icon (e.g., 'AlertCircle', 'Clock', or 'AlertTriangle').
106-160: LGTM! Chart configuration is well-structured.The chart configuration is complete and correctly structured:
- String values for monetary amounts appropriately avoid floating-point precision issues
monthsToShow: 6correctly matches the chartData array length- All required fields (title, labels, chartData, showChart) are present
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts
Show resolved
Hide resolved
michnowak
force-pushed
the
feature/order-summary-chart-visibility
branch
from
47ba11e to
35e2d6b
Compare
...ges/integrations/strapi-cms/src/modules/cms/graphql/fragments/blocks/PaymentsSummary.graphql
Outdated
Show resolved
Hide resolved
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.payments-summary.mapper.ts
Outdated
Show resolved
Hide resolved
packages/framework/src/modules/cms/models/blocks/orders-summary.model.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/integrations/strapi-cms/generated/strapi.tsis excluded by!**/generated/**
📒 Files selected for processing (1)
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.orders-summary.mapper.ts(1 hunks)
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.orders-summary.mapper.ts
Show resolved
Hide resolved
marcinkrasowski
changed the title
What does this PR do?
Key Changes
Layout Configuration: Added support for vertical/horizontal layout configuration in Payments Summary Block. The layout can now be controlled via CMS configuration field, allowing cards to be displayed either stacked vertically or side-by-side horizontally.
Optional Fields Support: Made
overdueandtoBePaidfields optional in the Payments Summary Block. If a field is not configured in CMS, the corresponding card is not displayed. This allows displaying only one card when needed (e.g., only overdue or only toBePaid).Chart Integration: Integrated chart functionality directly into Payments Summary Block (similar to Orders Summary Block architecture). The chart is now an optional part of the Payments Summary Block instead of being a separate Payments History Block. This provides better layout control:
showChartistrue: Cards and chart are displayed side-by-side (flex-col lg:flex-row)showChartisfalseor chart is not configured: Cards stretch to full width (flex-col md:flex-row), allowing payments-summary to take full width when payments-history block is removedSummary by CodeRabbit
New Features
Changes