feat: add order invoice component#282
Conversation
|
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:
📝 WalkthroughWalkthroughAdds a new order invoice PDF module with row construction, React-PDF rendering, reconciliation display, browser download and preview helpers, webpack entry wiring, dependency updates, and comprehensive Jest coverage. ChangesOrder PDF invoice generation
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Caller
participant previewPDF
participant OrderPdf
participant BrowserTab
Caller->>previewPDF: order, summit, options
previewPDF->>BrowserTab: open blank tab synchronously
previewPDF->>OrderPdf: render invoice to PDF blob
OrderPdf-->>previewPDF: PDF blob
previewPDF->>BrowserTab: navigate to blob URL
previewPDF-->>BrowserTab: revoke blob URL after delay
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/__tests__/order-pdf-.test.js`:
- Line 16: The test import is wrong in order-pdf-.test.js: it only brings in
buildOrderPdfRows and OrderPdf from the components barrel, but the suite
actually calls buildRows, and importing via the barrel also pulls in react-dnd
through SortableTable. Update the test to import the order-pdf module directly
and use the matching exported helper name (or import the alias used by the
tests) so buildRows is defined and the react-dnd dependency chain is avoided.
In `@src/components/order-pdf/index.js`:
- Around line 82-101: The item filtering in the order PDF is inconsistent with
the displayed default quantity: the current filter in the item iteration drops
rows when quantity is missing, but the row rendering in the same block (`qty` in
the `form.items` processing) treats missing quantity as 1. Update the
`form.items` filter logic so optional/missing quantities are still included when
they should render as default quantity 1, and keep the balance calculation and
PDF rows aligned with the `rows.push` item mapping.
- Around line 804-805: The previewPDF flow is creating a blob URL and opening it
in a new tab without releasing it afterward, which can leak memory on repeated
previews. Update the URL handling around the object URL creation in previewPDF
so the URL is revoked after the new window/tab has been opened, while keeping
the current open-in-new-tab behavior intact.
- Around line 32-33: The formatDate helper currently calls .format()
unconditionally on the result of epochToMomentTimeZone, which can be falsy for
missing or 0 epochs and crash invoice rendering. Update formatDate to guard the
value returned by epochToMomentTimeZone before formatting, and return a safe
fallback when no valid moment-like value exists. Keep the fix localized to
formatDate so callers of the order PDF rendering path continue working without
changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0cf3870-2447-4bcc-95d2-77eafc9856b4
📒 Files selected for processing (5)
package.jsonsrc/components/__tests__/order-pdf-.test.jssrc/components/index.jssrc/components/order-pdf/index.jswebpack.common.js
There was a problem hiding this comment.
Pull request overview
Adds a reusable “order invoice PDF” component to uicore, enabling apps that consume this library to generate and preview an order invoice PDF (with line items, fees, discounts, payments/refunds, notes, reconciliation, and amount due).
Changes:
- Introduces
OrderPdf+ helpers (buildRows,generateInvoicePDF,previewPDF) undersrc/components/order-invoice-pdf/. - Exposes the new component/helpers via the central
src/components/index.jsexport surface and adds a webpack entrypoint. - Updates
@react-pdf/rendererto v4.x and adds unit tests for row-building and rendering.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
webpack.common.js |
Adds a new webpack entry for the invoice PDF component bundle. |
src/components/order-invoice-pdf/index.js |
Implements PDF layout, row building, and generate/preview helpers. |
src/components/order-invoice-pdf/__tests__/order-invoice-pdf.test.js |
Adds tests for row formatting and basic render behavior. |
src/components/index.js |
Exports the new invoice PDF APIs from the library’s main component index. |
package.json |
Bumps @react-pdf/renderer devDependency and broadens peer dependency to allow v4. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/order-invoice-pdf/helpers.js (1)
88-114: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winQuantity filter still drops items that the row renderer treats as qty=1 — previously flagged, now regressed.
Line 90 filters out any item with a falsy
quantity(coversundefined/null/0), yet Line 108 renders missing quantity as1viaitem.quantity ?? 1. Since the filter runs first, items withundefined/nullquantity never reach the render step, so this fallback is dead code and those legacy/optional-quantity items silently disappear from both the invoice table and the running balance (Line 99balanceCents += item.amount) instead of showing with default qty 1. This is the exact issue previously raised on an earlier revision of this component (then atsrc/components/order-pdf/index.js) and marked resolved — it appears to have regressed during the file split intohelpers.js/styles.js/components.js/index.js.🐛 Proposed fix
(form.items || []) - .filter((item) => item.quantity) + .filter((item) => (item.quantity ?? 1) > 0) .forEach((item) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/order-invoice-pdf/helpers.js` around lines 88 - 114, The item iteration in the order invoice row builder is filtering out any entry with a falsy quantity before rendering, which causes legacy items with missing quantity to disappear even though the row logic already defaults qty to 1. Update the item-processing path in the form/items loop so that undefined or null quantity values are allowed through while still excluding only truly invalid cases if needed, and keep the existing qty fallback used when building each row. Ensure the fix preserves the balance update and row creation for those items in the helpers.js item row construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/order-invoice-pdf/index.js`:
- Around line 36-108: The OrderPdf component is only partially guarded: the
totals destructure uses order || {}, but later reads still access order.client,
order.address, order.number, order.purchased_date, summit.time_zone_id, and
summit.name directly. Update OrderPdf to consistently handle missing order and
summit data by defaulting the props up front or using optional
chaining/fallbacks at each access so rendering does not throw when either prop
is absent.
---
Duplicate comments:
In `@src/components/order-invoice-pdf/helpers.js`:
- Around line 88-114: The item iteration in the order invoice row builder is
filtering out any entry with a falsy quantity before rendering, which causes
legacy items with missing quantity to disappear even though the row logic
already defaults qty to 1. Update the item-processing path in the form/items
loop so that undefined or null quantity values are allowed through while still
excluding only truly invalid cases if needed, and keep the existing qty fallback
used when building each row. Ensure the fix preserves the balance update and row
creation for those items in the helpers.js item row construction.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc0a5572-6fe6-46c0-a3ca-9e4a0688b776
📒 Files selected for processing (8)
package.jsonsrc/components/index.jssrc/components/order-invoice-pdf/__tests__/order-invoice-pdf.test.jssrc/components/order-invoice-pdf/components.jssrc/components/order-invoice-pdf/helpers.jssrc/components/order-invoice-pdf/index.jssrc/components/order-invoice-pdf/styles.jswebpack.common.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/order-invoice-pdf/index.js (1)
176-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate PDF-blob construction across
generateInvoicePDFandpreviewPDF.Both functions build the identical
pdf(<OrderPdf order={order} summit={summit} logoSrc={logoSrc} theme={theme} />).toBlob()call. Extracting a shared helper (e.g.buildInvoicePdfBlob(order, summit, options)) would remove the duplication and keep the two call sites in sync going forward.♻️ Proposed refactor
+const buildInvoicePdfBlob = (order, summit, { logoSrc, theme } = {}) => + pdf( + <OrderPdf order={order} summit={summit} logoSrc={logoSrc} theme={theme} /> + ).toBlob(); + export const generateInvoicePDF = async ( order, summit, { logoSrc, theme } = {} ) => { try { - const blob = await pdf( - <OrderPdf - order={order} - summit={summit} - logoSrc={logoSrc} - theme={theme} - /> - ).toBlob(); + const blob = await buildInvoicePdfBlob(order, summit, { logoSrc, theme });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/order-invoice-pdf/index.js` around lines 176 - 230, Extract the duplicated OrderPdf-to-blob construction from generateInvoicePDF and previewPDF into a shared helper such as buildInvoicePdfBlob, accepting order, summit, and the logoSrc/theme options. Replace both inline pdf(...).toBlob() calls with the helper while preserving each function’s existing download, preview, cleanup, and error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/order-invoice-pdf/index.js`:
- Around line 206-221: Update window.open in previewPDF to use only the blank
URL and _blank target, removing the noreferrer feature so it returns a
WindowProxy. Preserve the existing newTab.location.href navigation after the PDF
blob URL is created.
---
Nitpick comments:
In `@src/components/order-invoice-pdf/index.js`:
- Around line 176-230: Extract the duplicated OrderPdf-to-blob construction from
generateInvoicePDF and previewPDF into a shared helper such as
buildInvoicePdfBlob, accepting order, summit, and the logoSrc/theme options.
Replace both inline pdf(...).toBlob() calls with the helper while preserving
each function’s existing download, preview, cleanup, and error behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2b88bf2-022d-4a63-8853-0609987037c6
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
package.jsonsrc/components/index.jssrc/components/order-invoice-pdf/__tests__/order-invoice-pdf.test.jssrc/components/order-invoice-pdf/components/field-row.jssrc/components/order-invoice-pdf/components/pdf-icon.jssrc/components/order-invoice-pdf/components/pdf-table-row.jssrc/components/order-invoice-pdf/components/reconciliation-block.jssrc/components/order-invoice-pdf/helpers.jssrc/components/order-invoice-pdf/index.jssrc/components/order-invoice-pdf/styles.jswebpack.common.js
🚧 Files skipped from review as they are similar to previous changes (2)
- webpack.common.js
- src/components/order-invoice-pdf/styles.js
| const rows = buildRows(order, summit); | ||
|
|
||
| const { | ||
| amount_due: total = 0, |
There was a problem hiding this comment.
@tomrndom Quick question on total vs amount_due: OrderPdf reads order.amount_due here for the AMOUNT DUE line, but SponsorOrderGrid (src/components/mui/SponsorOrderGrid/index.js:86) reads order.total for the same concept. I checked purchases-api's purchase_v2_serializer.py — the raw v2 response has amount_due (computed as max(raw_amount - discount_amount + charges_amount - payments_total, 0)) but no total field at all, so amount_due matches the backend contract exactly, same as the other 4 reconciliation fields this component already reads (cancelled_total, refunds_total, retained, credited_to_payment_method).
Can you confirm whether the order prop this component receives is the raw purchases-api v2 response, or whether it goes through the same mapping/normalization used to build SponsorOrderGrid's order prop (which would carry total instead)? If it's the latter, AMOUNT DUE will always render $0.00 in production and amount_due should be total here.
There was a problem hiding this comment.
This takes the raw amount_due prop directly from the order API response where total not exists, based on this change at sponsor-services https://github.com/fntechgit/sponsor-services/pull/100/changes
This components is migrated from sponsor-services, and reused part of the structure from the order-details page. On a deep look, that page the data comes from the reducer after a normalizeOrder function that assign the prop total from the value of amount_due or net_amount (https://github.com/fntechgit/sponsor-services/blob/main/src/utils/methods.js#L92), We should probably need to adjust this line to support both options like the reducer
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
…s, add tests Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
ref: https://app.clickup.com/t/9014802374/86baq9key
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit