Feature: order grid revamp#262
Conversation
📝 WalkthroughWalkthroughThis PR refactors the SponsorOrderGrid component from a line-item–based table to an order-based ledger grid with reconciliation and per-item balance tracking. New utility components (BalanceValue, TransactionType) provide consistent transaction rendering. Footer rows now display running balances and use the unified transaction type system. Optional reconciliation UI (ReconciliationBox, TotalFooter) and cancelled-items header replace the prior non-reconciliation total row when enabled. ChangesSponsor Order Grid Ledger Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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: 7
🧹 Nitpick comments (6)
src/i18n/en.json (1)
143-150: ⚡ Quick winInconsistent spelling: "canceled" vs "Cancelled".
Line 143 uses American spelling ("canceled") twice:
- "Active order items can be canceled"
- "Canceled items show an undo action"
However, lines 144-150 use British spelling ("Cancelled"):
- Line 144: "Cancelled {date} by {user}"
- Line 145: "Cancelled items ({count})"
- Line 147: "Cancelled"
Choose one spelling convention (either American "canceled" or British "cancelled") and apply it consistently across all
sponsor_order_gridtranslation keys.🔤 Suggested fix for American English consistency
- "cancel_info_note": "Active order items can be canceled. Canceled items show an undo action to restore them. Refund and payment rows are display-only.", - "cancelled_by": "Cancelled {date} by {user}", - "cancelled_items": "Cancelled items ({count}):", + "cancel_info_note": "Active order items can be canceled. Canceled items show an undo action to restore them. Refund and payment rows are display-only.", + "cancelled_by": "Canceled {date} by {user}", + "cancelled_items": "Canceled items ({count}):", "reconciliation": "Reconciliation", - "cancelled": "Cancelled", + "cancelled": "Canceled",Alternatively, if British English is preferred, update line 143 to use "cancelled" throughout.
🤖 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/i18n/en.json` around lines 143 - 150, The translation entries under sponsor_order_grid use mixed American and British spellings; update the keys/values for consistency by choosing one spelling and applying it across the affected keys: change the "cancel_info_note" value to use the same spelling as "cancelled_by", "cancelled_items", and "cancelled" (or vice versa), and ensure the displayed phrase in "cancel_info_note" and the label in "cancelled_items" both use the chosen spelling; specifically review and update the values for "cancel_info_note", "cancelled_by", "cancelled_items", and "cancelled" so they all consistently use either "canceled"/"Canceled" or "cancelled"/"Cancelled".src/components/mui/InfoNote/index.jsx (1)
11-11: 💤 Low valuePrefer theme-based or numeric fontSize values.
The hardcoded
"13px"string bypasses MUI's theme sizing system and uses absolute pixel units, which can reduce accessibility for users with custom font-size preferences. Consider using a theme-relative value or a numeric pixel value for better integration with MUI's design system.♻️ Suggested alternatives
Option 1 (preferred): Use MUI theme typography sizing:
- <Typography variant="body1" color="text.secondary" sx={{fontSize: "13px"}}> + <Typography variant="body2" color="text.secondary">Option 2: Use numeric value (still absolute, but more idiomatic):
- <Typography variant="body1" color="text.secondary" sx={{fontSize: "13px"}}> + <Typography variant="body1" color="text.secondary" sx={{fontSize: 13}}>Option 3: Use theme spacing with rem units:
- <Typography variant="body1" color="text.secondary" sx={{fontSize: "13px"}}> + <Typography variant="body1" color="text.secondary" sx={{fontSize: "0.8125rem"}}>🤖 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/mui/InfoNote/index.jsx` at line 11, Replace the hardcoded string font size "13px" in the InfoNote component's Typography sx with a theme-aware or numeric value; e.g., use a theme callback like sx={(theme) => ({ fontSize: theme.typography.pxToRem(13) })} or switch to a built-in size/variant (e.g., variant="body2") or use numeric pixels (fontSize: 13) so the Typography (in InfoNote) integrates with MUI's theme system and accessibility settings.src/components/mui/SponsorOrderGrid/index.js (3)
227-243: 💤 Low valueMisleading
key="balance"on the action cell.The action
TableCellreuseskey="balance"(copy-pasted from the balance column). It doesn't collide within the row, but it's confusing and brittle; key it on the row id/action instead.🔧 Proposed fix
cols.push( <TableCell - key="balance" + key={`grid-col-${row.id}-action`} align="right" >🤖 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/mui/SponsorOrderGrid/index.js` around lines 227 - 243, The action TableCell added inside the canCancel branch is using a misleading static key "balance"; update the key to be unique per row/action (e.g. use row.id or a composite like `action-${row.id}`) so the element identity is stable and clear; modify the cols.push call that creates the TableCell (the block referencing canCancel, TableCell, row.cancelled, onUndoCancelForm and onCancelForm) to set its key to a row-specific value instead of "balance".
96-101: ⚖️ Poor tradeoff
calculateBalancemutates closure state during render.The running
balanceis mutated as a side effect while building JSX. It works under React 17's synchronous single pass, but it's order-dependent and would break under StrictMode double-invocation or concurrent rendering. Consider precomputing balances inmapOrderData/a memoized pass and passing values down, keeping render pure.Also applies to: 184-263
🤖 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/mui/SponsorOrderGrid/index.js` around lines 96 - 101, The calculateBalance function mutates the outer balance variable during render (balance and calculateBalance), which is unsafe under StrictMode; instead compute running balances ahead of rendering—e.g., in mapOrderData or a useMemo that iterates the original data once, producing a new array with a computed balance field on each row (attach as row.runningBalance or similar). Replace calls to calculateBalance in the JSX with the precomputed property and remove the closure mutation and calculateBalance side effects so rendering becomes pure and order-independent.
80-91: 💤 Low valueDestructuring
orderwithout a default crashes when the prop is absent.
const {...} = order;throws a TypeError iforderis undefined/null. Sinceorderis now required (replacing the prior individual props), add a default to fail soft during the migration.🛡️ Proposed fix
title = T.translate("sponsor_order_grid.title"), - order, + order = {}, withReconciliation = false,🤖 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/mui/SponsorOrderGrid/index.js` around lines 80 - 91, The destructuring of the prop "order" in SponsorOrderGrid (the const { forms, fees, payments, refunds, notes, total, retained, credited_to_payment_method: credited, cancelled_total: cancelledTotal, refunds_total: refundsTotal } = order;) will throw if order is undefined; change the destructuring to provide a safe default object (or set a default param for the component) so the destructure reads from {} when order is missing and avoid runtime TypeError—keep the same property names/aliases so downstream code continues to work.src/components/mui/SponsorOrderGrid/components/CancelledItems.jsx (1)
21-23: 💤 Low valueGuard against an undefined
cancelledItems.
cancelledItems.lengththrows if the prop is omitted. A default empty array keeps the component safe when reused outside the current call site.🛡️ Proposed fix
-const CancelledItems = ({cancelledItems, sx = {}}) => { +const CancelledItems = ({cancelledItems = [], sx = {}}) => {🤖 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/mui/SponsorOrderGrid/components/CancelledItems.jsx` around lines 21 - 23, The component CancelledItems currently assumes cancelledItems is defined and calls cancelledItems.length; update its props handling to guard against undefined by defaulting cancelledItems to an empty array or validating it before use (e.g., change the function signature to use cancelledItems = [] or add an Array.isArray check) so checks like cancelledItems.length won't throw when the prop is omitted.
🤖 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/mui/SponsorOrderGrid/components/CancelledItems.jsx`:
- Around line 31-39: The mapped Link elements in the CancelledItems component
are missing a React key which causes warnings and unstable reconciliation;
update the cancelledItems.map callback (in CancelledItems.jsx) to add a stable
key prop to each <Link> using item.id (or another unique identifier) so each
child has a unique key.
In `@src/components/mui/SponsorOrderGrid/components/ReconciliationBox.jsx`:
- Around line 21-54: Reconcile the displayed amount with the label in
ReconciliationBox: compute a totalValue (e.g., const totalValue = retained > 0 ?
retained : (credited > 0 ? credited : retained)) alongside totalLabel and
replace the hard-coded currencyAmountFromCents(retained) in the bottom-right
cell with currencyAmountFromCents(totalValue) so that when totalLabel is
"credited" the credited amount is shown; update the variables near where
totalLabel is defined (in the ReconciliationBox component) and render
currencyAmountFromCents(totalValue) instead of retained.
In `@src/components/mui/SponsorOrderGrid/components/TransactionType.js`:
- Around line 30-38: TransactionType currently dereferences iconMap[type]
without checking existence which crashes when type is missing; update the
component to guard access to iconMap by reading a safe entry (e.g. const meta =
iconMap[type] || {icon: DefaultIcon, color: 'inherit'}), use meta.icon as Icon
and meta.color for the sx color, and ensure children/type text still renders
when meta is the fallback; reference the TransactionType component, iconMap,
Icon and children when applying this change.
- Around line 22-28: The module crashes because SPONSOR_ORDER_GRID_ITEM_TYPES is
undefined when Jest mocks "../../../../utils/constants" in tests, so update the
tests' mocks to return SPONSOR_ORDER_GRID_ITEM_TYPES (or replace the mock with
jest.requireActual("../../../../utils/constants") spread) so TransactionType can
import it, and also harden TransactionType by guarding access to iconMap entries
(in TransactionType.js: check iconMap[type] exists before accessing
iconMap[type].icon / .color and provide a sensible default icon/color) to
prevent module-eval failures when the constant is missing.
In `@src/components/mui/SponsorOrderGrid/index.js`:
- Around line 252-260: The DiscountRow is passing mismatched fields
(discountCents={form.discount_total}) while calculateBalance uses
form.discount_in_cents, which can be undefined and produces NaN; update the
DiscountRow to use a single canonical cents value (e.g., const discountCents =
form.discount_in_cents ?? form.discount_total ?? 0) and pass that same
discountCents into both the DiscountRow prop and calculateBalance(discountCents,
-1), ensuring a default of 0 to avoid NaN and crashes (also ensure any
downstream callers like BalanceValue/currencyAmountFromCents receive an
integer). Additionally, rename the action column TableCell key from "balance" to
a clearer key like "action" to avoid the misleading identifier.
In `@src/components/mui/table/extra-rows/DiscountRow.jsx`:
- Around line 25-27: The guard in DiscountRow currently checks `discountCents
=== 0`, which misses null/undefined and allows
`currencyAmountFromCents(discountCents)` to print NaN; change the early-return
to a falsy check (e.g., `if (!discountCents) return null;`) so the row doesn't
render for 0, null, or undefined values and ensure `currencyAmountFromCents`
only receives a valid number.
In `@src/components/mui/table/extra-rows/TotalRow.jsx`:
- Around line 23-26: The TotalRow/TotalFooter crash happens because
currencyAmountFromCents is called with Math.abs(total) when total can be
non-integer/NaN/nullish/or a preformatted string; update the rendering logic
around total (variables isNegative, sign, totalStr) to sanitize/guard before
calling currencyAmountFromCents: set isNegative = (typeof total === "number" &&
total < 0), if typeof total === "number" and Number.isFinite(total) compute
sanitizedCents = Math.round(Math.abs(total)) and pass that to
currencyAmountFromCents, if typeof total === "string" treat it as a preformatted
string and render it directly (skip currencyAmountFromCents), and for
null/NaN/other types fallback sanitizedCents = 0; apply the same fix in
TotalFooter (same currencyAmountFromCents(Math.abs(total)) usage).
---
Nitpick comments:
In `@src/components/mui/InfoNote/index.jsx`:
- Line 11: Replace the hardcoded string font size "13px" in the InfoNote
component's Typography sx with a theme-aware or numeric value; e.g., use a theme
callback like sx={(theme) => ({ fontSize: theme.typography.pxToRem(13) })} or
switch to a built-in size/variant (e.g., variant="body2") or use numeric pixels
(fontSize: 13) so the Typography (in InfoNote) integrates with MUI's theme
system and accessibility settings.
In `@src/components/mui/SponsorOrderGrid/components/CancelledItems.jsx`:
- Around line 21-23: The component CancelledItems currently assumes
cancelledItems is defined and calls cancelledItems.length; update its props
handling to guard against undefined by defaulting cancelledItems to an empty
array or validating it before use (e.g., change the function signature to use
cancelledItems = [] or add an Array.isArray check) so checks like
cancelledItems.length won't throw when the prop is omitted.
In `@src/components/mui/SponsorOrderGrid/index.js`:
- Around line 227-243: The action TableCell added inside the canCancel branch is
using a misleading static key "balance"; update the key to be unique per
row/action (e.g. use row.id or a composite like `action-${row.id}`) so the
element identity is stable and clear; modify the cols.push call that creates the
TableCell (the block referencing canCancel, TableCell, row.cancelled,
onUndoCancelForm and onCancelForm) to set its key to a row-specific value
instead of "balance".
- Around line 96-101: The calculateBalance function mutates the outer balance
variable during render (balance and calculateBalance), which is unsafe under
StrictMode; instead compute running balances ahead of rendering—e.g., in
mapOrderData or a useMemo that iterates the original data once, producing a new
array with a computed balance field on each row (attach as row.runningBalance or
similar). Replace calls to calculateBalance in the JSX with the precomputed
property and remove the closure mutation and calculateBalance side effects so
rendering becomes pure and order-independent.
- Around line 80-91: The destructuring of the prop "order" in SponsorOrderGrid
(the const { forms, fees, payments, refunds, notes, total, retained,
credited_to_payment_method: credited, cancelled_total: cancelledTotal,
refunds_total: refundsTotal } = order;) will throw if order is undefined; change
the destructuring to provide a safe default object (or set a default param for
the component) so the destructure reads from {} when order is missing and avoid
runtime TypeError—keep the same property names/aliases so downstream code
continues to work.
In `@src/i18n/en.json`:
- Around line 143-150: The translation entries under sponsor_order_grid use
mixed American and British spellings; update the keys/values for consistency by
choosing one spelling and applying it across the affected keys: change the
"cancel_info_note" value to use the same spelling as "cancelled_by",
"cancelled_items", and "cancelled" (or vice versa), and ensure the displayed
phrase in "cancel_info_note" and the label in "cancelled_items" both use the
chosen spelling; specifically review and update the values for
"cancel_info_note", "cancelled_by", "cancelled_items", and "cancelled" so they
all consistently use either "canceled"/"Canceled" or "cancelled"/"Cancelled".
🪄 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: 39ee3ca9-abe1-4254-b608-4642fd6138d6
📒 Files selected for processing (16)
package.jsonsrc/components/mui/InfoNote/index.jsxsrc/components/mui/SponsorOrderGrid/components/BalanceValue.jsxsrc/components/mui/SponsorOrderGrid/components/CancelledItems.jsxsrc/components/mui/SponsorOrderGrid/components/ReconciliationBox.jsxsrc/components/mui/SponsorOrderGrid/components/TotalFooter.jsxsrc/components/mui/SponsorOrderGrid/components/TransactionType.jssrc/components/mui/SponsorOrderGrid/index.jssrc/components/mui/table/extra-rows/DiscountRow.jsxsrc/components/mui/table/extra-rows/FeeRow.jsxsrc/components/mui/table/extra-rows/NotesRow.jsxsrc/components/mui/table/extra-rows/PaymentRow.jsxsrc/components/mui/table/extra-rows/RefundRow.jsxsrc/components/mui/table/extra-rows/TotalRow.jsxsrc/i18n/en.jsonsrc/utils/constants.js
| {cancelledItems.map((item) => ( | ||
| <Link | ||
| variant="body1" | ||
| href={`#item-${item.id}`} | ||
| sx={{mr: 1, color: "text.disabled", textDecorationColor: "rgba(0, 0, 0, 0.38)"}} | ||
| > | ||
| {item.formCode} - {item.itemCode} | ||
| </Link> | ||
| ))} |
There was a problem hiding this comment.
Missing key on mapped Link.
Each child in a list needs a stable key; otherwise React emits warnings and reconciliation suffers. Use item.id.
🔧 Proposed fix
{cancelledItems.map((item) => (
<Link
+ key={`cancelled-item-${item.id}`}
variant="body1"
href={`#item-${item.id}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {cancelledItems.map((item) => ( | |
| <Link | |
| variant="body1" | |
| href={`#item-${item.id}`} | |
| sx={{mr: 1, color: "text.disabled", textDecorationColor: "rgba(0, 0, 0, 0.38)"}} | |
| > | |
| {item.formCode} - {item.itemCode} | |
| </Link> | |
| ))} | |
| {cancelledItems.map((item) => ( | |
| <Link | |
| key={`cancelled-item-${item.id}`} | |
| variant="body1" | |
| href={`#item-${item.id}`} | |
| sx={{mr: 1, color: "text.disabled", textDecorationColor: "rgba(0, 0, 0, 0.38)"}} | |
| > | |
| {item.formCode} - {item.itemCode} | |
| </Link> | |
| ))} |
🤖 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/mui/SponsorOrderGrid/components/CancelledItems.jsx` around
lines 31 - 39, The mapped Link elements in the CancelledItems component are
missing a React key which causes warnings and unstable reconciliation; update
the cancelledItems.map callback (in CancelledItems.jsx) to add a stable key prop
to each <Link> using item.id (or another unique identifier) so each child has a
unique key.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| const ReconciliationBox = ({cancelledTotal, refundsTotal, retained, credited}) => { | ||
| const totalColor = retained > 0 ? "error.dark" : "success.dark"; | ||
| const totalLabel = retained > 0 ? "retained" : (credited > 0 ? "credited" : "balance"); | ||
|
|
||
| return ( | ||
| <Box sx={{maxWidth: 400, mt: 2, mb: 3}}> | ||
| <Typography variant="body2" sx={{mb: 3}}> | ||
| {T.translate("sponsor_order_grid.reconciliation")} | ||
| </Typography> | ||
| <Box sx={{display: "flex", justifyContent: "space-between"}}> | ||
| <Typography variant="body1" sx={{color: "text.secondary"}}> | ||
| {T.translate("sponsor_order_grid.cancelled")} | ||
| </Typography> | ||
| <Typography variant="body1"> | ||
| {currencyAmountFromCents(cancelledTotal)} | ||
| </Typography> | ||
| </Box> | ||
| <Box sx={{display: "flex", justifyContent: "space-between"}}> | ||
| <Typography variant="body1" sx={{color: "text.secondary"}}> | ||
| {T.translate("sponsor_order_grid.refunded")} | ||
| </Typography> | ||
| <Typography variant="body1"> | ||
| {currencyAmountFromCents(refundsTotal)} | ||
| </Typography> | ||
| </Box> | ||
| <Divider sx={{my: 1}}/> | ||
| <Box sx={{display: "flex", justifyContent: "space-between"}}> | ||
| <Typography variant="body2"> | ||
| {T.translate(`sponsor_order_grid.${totalLabel}`)} | ||
| </Typography> | ||
| <Typography variant="body2" sx={{color: totalColor}}> | ||
| {currencyAmountFromCents(retained)} | ||
| </Typography> | ||
| </Box> |
There was a problem hiding this comment.
Total value doesn't match the chosen label.
totalLabel can resolve to credited, but the amount cell on Line 52 always renders currencyAmountFromCents(retained). When retained <= 0 && credited > 0, the row shows the "credited" label next to the retained value (typically 0), so the credited amount is never displayed. Derive the displayed value alongside the label.
🐛 Proposed fix
const ReconciliationBox = ({cancelledTotal, refundsTotal, retained, credited}) => {
const totalColor = retained > 0 ? "error.dark" : "success.dark";
const totalLabel = retained > 0 ? "retained" : (credited > 0 ? "credited" : "balance");
+ const totalValue = retained > 0 ? retained : (credited > 0 ? credited : retained);
...
<Typography variant="body2" sx={{color: totalColor}}>
- {currencyAmountFromCents(retained)}
+ {currencyAmountFromCents(totalValue)}
</Typography>🤖 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/mui/SponsorOrderGrid/components/ReconciliationBox.jsx` around
lines 21 - 54, Reconcile the displayed amount with the label in
ReconciliationBox: compute a totalValue (e.g., const totalValue = retained > 0 ?
retained : (credited > 0 ? credited : retained)) alongside totalLabel and
replace the hard-coded currencyAmountFromCents(retained) in the bottom-right
cell with currencyAmountFromCents(totalValue) so that when totalLabel is
"credited" the credited amount is shown; update the variables near where
totalLabel is defined (in the ReconciliationBox component) and render
currencyAmountFromCents(totalValue) instead of retained.
| const iconMap = { | ||
| [SPONSOR_ORDER_GRID_ITEM_TYPES.CHARGE]: {icon: ArrowUpwardIcon, color: "warning.light"}, | ||
| [SPONSOR_ORDER_GRID_ITEM_TYPES.PAYMENT]: {icon: ArrowDownwardIcon, color: "success.light"}, | ||
| [SPONSOR_ORDER_GRID_ITEM_TYPES.DISCOUNT]: {icon: ArrowDownwardIcon, color: "success.light"}, | ||
| [SPONSOR_ORDER_GRID_ITEM_TYPES.REFUND]: {icon: RefreshIcon, color: "warning.light"}, | ||
| [SPONSOR_ORDER_GRID_ITEM_TYPES.CANCELLED]: {icon: DoNotDisturbIcon, color: "default"}, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Confirm the relative path resolves to the file that actually exports the constant
fd -t f 'constants.js' src/utils
# 2) Verify the export exists exactly as imported
rg -nP 'export const SPONSOR_ORDER_GRID_ITEM_TYPES' src/utils/constants.js
# 3) Look for any imports inside constants.js that could create a cycle
rg -nP "^\s*import\b" src/utils/constants.js || echo "constants.js has no imports"
# 4) Trace who imports TransactionType and whether that path loops back through constants
rg -nP "import\s+TransactionType" -g '*.js' -g '*.jsx'
# 5) Check jest moduleNameMapper / transform config that could shadow utils/constants
fd -t f -i 'jest.config*' ; fd -t f 'package.json' -d 2 --exec rg -n 'moduleNameMapper|jest' {}Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1256
🏁 Script executed:
#!/bin/bash
# Inspect TransactionType.js imports + iconMap usage
fd -t f 'TransactionType.js' src/components/mui/SponsorOrderGrid/components/TransactionType.js -0
# If fd can't find with composite, list matching file
fd -t f 'TransactionType.js' src/components/mui/SponsorOrderGrid/components | head -n 20
# Print the relevant section with line numbers
python3 - <<'PY'
from pathlib import Path
paths = [
Path('src/components/mui/SponsorOrderGrid/components/TransactionType.js')
]
for p in paths:
if p.exists():
print('---', p, '---')
import itertools
with p.open('r', encoding='utf-8') as f:
lines = f.readlines()
start, end = 1, min(len(lines), 120)
for i in range(start, end+1):
pass
# print around 1-80 and around 15-60
def dump(a,b):
for idx in range(a-1,b):
if 0<=idx<len(lines):
print(f"{idx+1:4d}: {lines[idx].rstrip()}")
dump(1,60)
dump(60,120)
PY
# Find where SPONSOR_ORDER_GRID_ITEM_TYPES is imported in TransactionType.js
rg -n "SPONSOR_ORDER_GRID_ITEM_TYPES" src/components/mui/SponsorOrderGrid/components/TransactionType.js
# Show the exact import statement for constants in TransactionType.js
rg -n "constants" src/components/mui/SponsorOrderGrid/components/TransactionType.js
# Confirm Jest config doesn't remap this utils/constants path unexpectedly
# (we already saw moduleNameMapper exists; now extract the relevant part)
rg -n "moduleNameMapper" -S package.json jest.config* 2>/dev/null || true
fd -t f -i 'jest.config*' . | head -n 20Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 3053
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact export block for SPONSOR_ORDER_GRID_ITEM_TYPES
python3 - <<'PY'
import itertools
from pathlib import Path
p = Path("src/utils/constants.js")
lines = p.read_text(encoding="utf-8").splitlines()
for i,l in enumerate(lines, start=1):
if "export const SPONSOR_ORDER_GRID_ITEM_TYPES" in l:
start=i-10
end=i+60
start=max(1,start)
end=min(len(lines),end)
print(f"--- {p} (around line {i}) ---")
for ln in range(start, end+1):
print(f"{ln:4d}: {lines[ln-1]}")
break
else:
raise SystemExit("SPONSOR_ORDER_GRID_ITEM_TYPES export not found")
PY
# Extract jest moduleNameMapper/transform config section for context
python3 - <<'PY'
import json
from pathlib import Path
pkg = json.loads(Path("package.json").read_text(encoding="utf-8"))
jest = pkg.get("jest", {})
print("--- package.json jest config keys ---")
print(sorted(jest.keys()))
print("\n--- moduleNameMapper ---")
print(jest.get("moduleNameMapper"))
print("\n--- transform ---")
print(jest.get("transform"))
print("\n--- setupFilesAfterEnv ---")
print(jest.get("setupFilesAfterEnv"))
PYRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 1111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where TransactionType is rendered and what `type` prop is passed
rg -n "<TransactionType" src -g'*.js' -g'*.jsx'
# Find how the extra-row components set the type prop
rg -n "TransactionType" src/components/mui/SponsorOrderGrid -g'*.js' -g'*.jsx'
rg -n "type=" src/components/mui/SponsorOrderGrid -g'*.js' -g'*.jsx'
# Search for Jest mocking of constants
rg -n "jest\.mock\(.+utils/constants|jest\.mock\(.+constants" -S src -g'*.js' -g'*.jsx' -g'*.ts' -g'*.tsx'
rg -n "SPONSOR_ORDER_GRID_ITEM_TYPES" -S src -g'*.js' -g'*.jsx' -g'*.ts' -g'*.tsx'
# Search tests specifically
fd -t f -e 'test.js' -e 'test.jsx' -e 'spec.js' -e 'spec.jsx' test tests __tests__ src -x echo 2>/dev/null | head -n 50 || true
fd -t f -i 'test|spec|__tests__' . || true
fd -t f -i '__tests__' . || true
# More targeted: list test files and grep for constants mocks
find . -maxdepth 6 -type f \( -name '*test*.js' -o -name '*test*.jsx' -o -name '*spec*.js' -o -name '*spec*.jsx' \) 2>/dev/null > /tmp/testfiles.txt
wc -l /tmp/testfiles.txt
head -n 20 /tmp/testfiles.txt
if [ -s /tmp/testfiles.txt ]; then
rg -n "SPONSOR_ORDER_GRID_ITEM_TYPES|utils/constants|jest\.mock" -g'*.js' -g'*.jsx' -S -f /tmp/testfiles.txt || true
fiRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 12910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Jest mocks that reference ../../../../utils/constants
for f in \
src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js \
src/components/mui/FormItemTable/__tests__/helpers.test.js
do
echo "===== $f ====="
python3 - <<PY
from pathlib import Path
p = Path("$f")
txt = p.read_text(encoding="utf-8").splitlines()
# Print region around jest.mock calls
for i,line in enumerate(txt, start=1):
if "jest.mock" in line and "utils/constants" in line:
start=max(1,i-10); end=min(len(txt), i+40)
for ln in range(start,end+1):
print(f"{ln:4d}: {txt[ln-1]}")
break
else:
# fallback: print first 120 lines if mock not found (still useful)
for ln in range(1, min(len(txt),120)+1):
print(f"{ln:4d}: {txt[ln-1]}")
PY
done
# Also inspect the Icon usage risk: iconMap[type] fallback
rg -n "iconMap\\[type\\]" -n src/components/mui/SponsorOrderGrid/components/TransactionType.js
rg -n "TransactionType\\s*\\(" src/components/mui/table/extra-rows/*.jsxRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate jest.mock for utils/constants in the two test files and print small surrounding context
for f in \
src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js \
src/components/mui/FormItemTable/__tests__/helpers.test.js
do
echo "===== $f ====="
# show jest.mock block region (up to ~80 lines around first match)
line=$(rg -n "jest\.mock\(\s*\"(\.\./\.\./\.\./\.\./)?utils/constants\"|jest\.mock\(\s*\"\.{0,2}/?.*utils/constants\"" "$f" || true | head -n 1 | cut -d: -f1)
if [ -n "${line:-}" ]; then
start=$((line-5)); [ $start -lt 1 ] && start=1
end=$((line+80))
sed -n "${start},${end}p" "$f" | nl -ba -w2 -s': ' | head -n 120
else
# fallback: just show lines containing jest.mock and file text around them
rg -n "jest\.mock" "$f" | head -n 20 || true
fi
done
# Also show TransactionType icon access
nl -ba src/components/mui/SponsorOrderGrid/components/TransactionType.js | sed -n '15,60p'
# And show iconMap[type] usage
rg -n "const Icon = iconMap\\[type\\]" -n src/components/mui/SponsorOrderGrid/components/TransactionType.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
files = [
"src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js",
"src/components/mui/FormItemTable/__tests__/helpers.test.js",
]
def dump_region(path: str, needle_substrings):
p = Path(path)
txt = p.read_text(encoding="utf-8").splitlines()
matches = []
for i, line in enumerate(txt, start=1):
if all(s in line for s in needle_substrings):
matches.append(i)
if not matches:
print(f"\n===== {path} =====")
print("No matching jest.mock line found for:", needle_substrings)
return
# print around first match
i = matches[0]
start = max(1, i-12)
end = min(len(txt), i+90)
print(f"\n===== {path} (around line {i}) =====")
for ln in range(start, end+1):
print(f"{ln:4d}: {txt[ln-1]}")
for f in files:
dump_region(f, ["jest.mock", "utils/constants"])
PY
# Print TransactionType iconMap access with line numbers (no `nl` needed)
python3 - <<'PY'
from pathlib import Path
p = Path("src/components/mui/SponsorOrderGrid/components/TransactionType.js")
txt = p.read_text(encoding="utf-8").splitlines()
for i in range(14, 45):
if 1 <= i <= len(txt):
print(f"{i:4d}: {txt[i-1]}")
PYRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 9456
Blocking: Jest mocks replace utils/constants without exporting SPONSOR_ORDER_GRID_ITEM_TYPES, making TransactionType crash at module load.
src/components/mui/SponsorOrderGrid/components/TransactionType.js imports SPONSOR_ORDER_GRID_ITEM_TYPES and immediately uses computed keys like [SPONSOR_ORDER_GRID_ITEM_TYPES.CHARGE] in iconMap (lines ~22-28). In src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js Jest mocks ../../../../utils/constants and returns only SPONSOR_FORMS_METAFIELD_CLASS (lines ~23-25), so SPONSOR_ORDER_GRID_ITEM_TYPES becomes undefined and the module throws when evaluating .CHARGE. A similar incomplete mock exists in src/components/mui/FormItemTable/__tests__/helpers.test.js (lines ~24-26) with only MILLISECONDS_IN_SECOND.
Fix options:
- Update those Jest mocks to include
SPONSOR_ORDER_GRID_ITEM_TYPES(or spreadjest.requireActual("../../../../utils/constants")). - Add a runtime guard in
TransactionTypefor unknown/absenttypesoiconMap[type]can’t crash (currentlyiconMap[type].iconis unguarded).
🧰 Tools
🪛 GitHub Actions: jest / 0_build.txt
[error] 23-23: Test suite failed to run: Cannot read properties of undefined (reading 'CHARGE') in iconMap initialization.
🪛 GitHub Actions: jest / build
[error] 23-23: Test suite failed to run: TypeError Cannot read properties of undefined (reading 'CHARGE') at iconMap construction.
🤖 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/mui/SponsorOrderGrid/components/TransactionType.js` around
lines 22 - 28, The module crashes because SPONSOR_ORDER_GRID_ITEM_TYPES is
undefined when Jest mocks "../../../../utils/constants" in tests, so update the
tests' mocks to return SPONSOR_ORDER_GRID_ITEM_TYPES (or replace the mock with
jest.requireActual("../../../../utils/constants") spread) so TransactionType can
import it, and also harden TransactionType by guarding access to iconMap entries
(in TransactionType.js: check iconMap[type] exists before accessing
iconMap[type].icon / .color and provide a sensible default icon/color) to
prevent module-eval failures when the constant is missing.
| const TransactionType = ({type, children}) => { | ||
| const Icon = iconMap[type].icon; | ||
| return ( | ||
| <Box sx={{display: "flex", alignItems: "center", gap: 1}}> | ||
| <Icon sx={{color: iconMap[type].color}} /> | ||
| {children || type} | ||
| </Box> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Guard against unmapped type values.
iconMap[type] is undefined for any type not in the map (or undefined), so iconMap[type].icon throws and crashes the row. Add a fallback before dereferencing.
🛡️ Proposed guard
-const TransactionType = ({type, children}) => {
- const Icon = iconMap[type].icon;
+const TransactionType = ({type, children}) => {
+ const entry = iconMap[type];
+ const Icon = entry?.icon;
return (
<Box sx={{display: "flex", alignItems: "center", gap: 1}}>
- <Icon sx={{color: iconMap[type].color}} />
+ {Icon && <Icon sx={{color: entry.color}} />}
{children || type}
</Box>
);
}🤖 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/mui/SponsorOrderGrid/components/TransactionType.js` around
lines 30 - 38, TransactionType currently dereferences iconMap[type] without
checking existence which crashes when type is missing; update the component to
guard access to iconMap by reading a safe entry (e.g. const meta = iconMap[type]
|| {icon: DefaultIcon, color: 'inherit'}), use meta.icon as Icon and meta.color
for the sx color, and ensure children/type text still renders when meta is the
fallback; reference the TransactionType component, iconMap, Icon and children
when applying this change.
| rows.push( | ||
| <DiscountRow | ||
| key={`discount-row-${form.id}`} | ||
| discount={form.discount} | ||
| discountCents={form.discount_total} | ||
| trailing={trailingCols} | ||
| balance={calculateBalance(form.discount_in_cents, -1)} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify which discount field the order/form payload actually provides.
rg -nP '\bdiscount_(total|in_cents)\b' -g '!**/node_modules/**' -C2Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find all usages of discount_in_cents and discount_total in the target component
rg -nP '\bdiscount_in_cents\b|\bdiscount_total\b' src/components/mui/SponsorOrderGrid/index.js
# 2) Inspect calculateBalance implementation (definition + how it handles undefined/NaN)
rg -nP 'function\s+calculateBalance|const\s+calculateBalance|calculateBalance\s*=\s*' src/components/mui/SponsorOrderGrid/index.js
rg -nP 'calculateBalance\s*\(' src/components/mui/SponsorOrderGrid/index.js
# 3) Check the order destructuring around the claimed line ~80
sed -n '60,110p' src/components/mui/SponsorOrderGrid/index.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 2050
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find mapOrderData and inspect how it builds `form` objects (especially discount fields)
rg -nP '\bmapOrderData\b' src/components/mui/SponsorOrderGrid/index.js
rg -nP '\bfunction\s+mapOrderData\b|const\s+mapOrderData\s*=' src/components/mui/SponsorOrderGrid/index.js
# Print a focused window around mapOrderData definition
python3 - <<'PY'
import re, pathlib
p=pathlib.Path('src/components/mui/SponsorOrderGrid/index.js')
txt=p.read_text()
m=re.search(r'(const\s+mapOrderData\s*=\s*\(|function\s+mapOrderData\s*\()', txt)
print("mapOrderData_pos", m.start() if m else None)
PY
# If found, dump around it (lines ~120-200 likely)
sed -n '1,220p' src/components/mui/SponsorOrderGrid/index.js
# Search for discount_in_cents anywhere else in this component/repo
rg -nP '\bdiscount_in_cents\b' src/components/mui/SponsorOrderGrid -S
# Expand the existing test to see expected form/order shape
sed -n '1,120p' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 12055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) See ordering of calculateBalance calls relative to DiscountRow
sed -n '200,330p' src/components/mui/SponsorOrderGrid/index.js
# 2) Search repo for discount_in_cents usages
rg -nP '\bdiscount_in_cents\b' . -S
# 3) Inspect DiscountRow implementation and how it uses balance/discountCents
rg -nP 'function\s+DiscountRow|const\s+DiscountRow|export\s+.*DiscountRow' src/components/mui -S
fd -a 'extra-rows' src/components/mui/SponsorOrderGrid -t f 2>/dev/null || true
# more direct:
rg -nP 'DiscountRow' src/components/mui/SponsorOrderGrid -S
rg -nP 'export\s*\{\s*DiscountRow' -S src/components/mui/SponsorOrderGrid -S || true
rg -nP 'DiscountRow\s*=' -S src/components/mui/SponsorOrderGrid -S || true
# Find the file imported as ../table/extra-rows
fd -a 'extra-rows' src/components/mui -t f | head
# Now show contents around DiscountRow in that file
EXTR=$(fd -a 'extra-rows.*' src/components/mui -t f | head -n 1)
echo "EXTRA_ROWS_FILE=$EXTR"
if [ -n "$EXTR" ]; then
rg -nP 'DiscountRow' "$EXTR" -n
# show around DiscountRow definition
# (approx: print 1/2 windows around match)
L=$(rg -nP 'DiscountRow' "$EXTR" | head -n1 | cut -d: -f1)
start=$((L-40)); end=$((L+120))
sed -n "${start},${end}p" "$EXTR"
fi
# 4) Inspect BalanceValue component for handling of NaN/undefined
fd -a 'BalanceValue*' src/components/mui/SponsorOrderGrid -t f | head
fd -a 'BalanceValue*' src/components/mui -t f | head
BV=$(fd -a 'BalanceValue*' src/components/mui -t f | head -n 1)
echo "BALANCE_FILE=$BV"
if [ -n "$BV" ]; then
sed -n '1,200p' "$BV"
fiRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 6594
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect DiscountRow to see how discountCents/balance are used and whether it guards undefined/NaN
sed -n '1,140p' src/components/mui/table/extra-rows/DiscountRow.jsx
# Find any other usage of discount_total/discount_in_cents across the repo
rg -nP '\bdiscount_total\b' src -S
rg -nP '\bdiscount_in_cents\b' src -S
# Look for how SponsorOrderGrid tests assert discount/balance rows (to see expected field names)
rg -nP 'DiscountRow|discount_total|discount_in_cents|discountCents|amount_due|balance' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js -S
sed -n '120,260p' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 4645
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check SponsorOrderGrid tests for whether they actually pass `order` prop (not just `lines`)
rg -nP '\border\b\s*[:=]' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js
rg -nP '\b<\s*SponsorOrderGrid\b' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js -n
# 2) Inspect default export component prop expectations (quick scan)
sed -n '1,120p' src/components/mui/SponsorOrderGrid/index.js
# 3) Inspect currencyAmountFromCents handling of null/NaN
rg -nP 'export\s+function\s+currencyAmountFromCents|function\s+currencyAmountFromCents|const\s+currencyAmountFromCents' src/utils/money.* src/**/money.* -S || true
fd -t f money src -a | head -n 20
MONEY_FILE=$(fd -a 'money' src -t f | head -n 20 | head -n 1)
echo "MONEY_FILE=$MONEY_FILE"
if [ -n "$MONEY_FILE" ]; then
sed -n '1,200p' "$MONEY_FILE"
fiRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) See how tests render SponsorOrderGrid: whether they pass `order`
rg -nP '\border\b' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js
# 2) Inspect money util for currencyAmountFromCents implementation
rg -nP 'currencyAmountFromCents' src/utils -S || true
fd -a '*money*' src -t f | head -n 20
# 3) Print the actual implementation file we find for currencyAmountFromCents
MONEY_FILE=$(rg -nP 'currencyAmountFromCents' src/utils -S --files-with-matches | head -n 1 || true)
echo "MONEY_FILE=$MONEY_FILE"
if [ -n "$MONEY_FILE" ]; then
sed -n '1,200p' "$MONEY_FILE"
fi
# 4) Inspect SponsorOrderGrid component prop handling (check if it tolerates missing `order`)
rg -nP 'const SponsorOrderGrid\s*=\s*\(' -n src/components/mui/SponsorOrderGrid/index.js
sed -n '110,200p' src/components/mui/SponsorOrderGrid/index.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect how tests render SponsorOrderGrid: whether they pass `order`
echo "---- Test file SponsorOrderGrid.test.js relevant lines ----"
rg -nP '\b<SponsorOrderGrid\b|lines\s*=\s*|order\s*=|defaultProps|render\(<SponsorOrderGrid' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js || true
echo "---- Component invocation blocks ----"
sed -n '1,220p' src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js
# 2) Locate currencyAmountFromCents implementation
echo "---- Locate currencyAmountFromCents ----"
MONEY_FILES=$(rg -nP 'currencyAmountFromCents' src/utils -S --files-with-matches || true)
echo "$MONEY_FILES"
# if nothing in src/utils, widen search
if [ -z "${MONEY_FILES:-}" ]; then
MONEY_FILES=$(rg -nP 'currencyAmountFromCents' src -S --files-with-matches || true)
echo "$MONEY_FILES"
fi
MONEY_FILE=$(echo "$MONEY_FILES" | head -n 1 || true)
echo "MONEY_FILE=$MONEY_FILE"
if [ -n "${MONEY_FILE:-}" ] && [ -f "$MONEY_FILE" ]; then
sed -n '1,220p' "$MONEY_FILE"
fi
# 3) Inspect SponsorOrderGrid prop destructuring and defaults for `order`
echo "---- SponsorOrderGrid component props destructuring ----"
sed -n '1,140p' src/components/mui/SponsorOrderGrid/index.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 18164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for keys/props expected by tests vs actual component
rg -nP '\blines\b|\bamountDue\b|\bwithDescription\b' src/components/mui/SponsorOrderGrid/index.js
# See whether component maps props like `lines` into `order`
rg -nP '\border\b' src/components/mui/SponsorOrderGrid/index.js
# Check how the component exports default and whether there is a wrapper
rg -nP 'export\s+default|module\.exports' src/components/mui/SponsorOrderGrid/index.js
# Repo-wide search for discount_in_cents and discount_total usage (to infer payload field)
rg -nP '\bdiscount_in_cents\b' src -S || true
rg -nP '\bdiscount_total\b' src -S || true
# Locate where forms are expected to come from (order.forms vs props.forms/lines)
sed -n '130,230p' src/components/mui/SponsorOrderGrid/index.js
sed -n '230,320p' src/components/mui/SponsorOrderGrid/index.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
#!/bin/bash
set -u
# List SponsorOrderGrid folder to see if there is a wrapper/index different from the file reviewed
ls -R src/components/mui/SponsorOrderGrid
# Check component export entrypoints
rg -nP 'export\s+default\s+SponsorOrderGrid|export\s*\{.*SponsorOrderGrid|export\s+default' src/components/mui/SponsorOrderGrid -S
# Re-run the earlier searches, but tolerate no matches
set +e
rg -nP '\blines\b|\bamountDue\b|\bwithDescription\b' src/components/mui/SponsorOrderGrid/index.js || true
rg -nP '\border\b' src/components/mui/SponsorOrderGrid/index.js || true
rg -nP '\bdiscount_in_cents\b' src || true
rg -nP '\bdiscount_total\b' src || true
set -e
# Inspect the bottom of SponsorOrderGrid to see how it maps props/uses order
tail -n 80 src/components/mui/SponsorOrderGrid/index.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 3832
Fix DiscountRow discount/balance field mismatch to prevent NaN/render crashes
In src/components/mui/SponsorOrderGrid/index.js, DiscountRow shows discountCents={form.discount_total} but updates the running balance with calculateBalance(form.discount_in_cents, -1). If discount_in_cents is missing/undefined, calculateBalance produces NaN, and BalanceValue/DiscountRow call currencyAmountFromCents, which requires an integer cents value—so rendering can throw and the running balance can be corrupted. Use the same discount cents field for both (and default missing values to 0).
- The action column
TableCelluseskey="balance"(misleading); rename to something likekey="action".
🤖 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/mui/SponsorOrderGrid/index.js` around lines 252 - 260, The
DiscountRow is passing mismatched fields (discountCents={form.discount_total})
while calculateBalance uses form.discount_in_cents, which can be undefined and
produces NaN; update the DiscountRow to use a single canonical cents value
(e.g., const discountCents = form.discount_in_cents ?? form.discount_total ?? 0)
and pass that same discountCents into both the DiscountRow prop and
calculateBalance(discountCents, -1), ensuring a default of 0 to avoid NaN and
crashes (also ensure any downstream callers like
BalanceValue/currencyAmountFromCents receive an integer). Additionally, rename
the action column TableCell key from "balance" to a clearer key like "action" to
avoid the misleading identifier.
| const DiscountRow = ({ discount, discountCents, balance, colGap = 0, trailing = 0 }) => { | ||
|
|
||
| if (discountTotal === 0) return null; | ||
| if (discountCents === 0) return null; |
There was a problem hiding this comment.
Null discount slips past the === 0 guard.
The supplied context indicates discountCents is null when there is no discount, but null === 0 is false, so the row still renders and currencyAmountFromCents(discountCents) (Line 56) prints $NaN. Use a falsy check to cover 0, null, and undefined.
🐛 Proposed fix
- if (discountCents === 0) return null;
+ if (!discountCents) return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const DiscountRow = ({ discount, discountCents, balance, colGap = 0, trailing = 0 }) => { | |
| if (discountTotal === 0) return null; | |
| if (discountCents === 0) return null; | |
| const DiscountRow = ({ discount, discountCents, balance, colGap = 0, trailing = 0 }) => { | |
| if (!discountCents) return null; |
🤖 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/mui/table/extra-rows/DiscountRow.jsx` around lines 25 - 27,
The guard in DiscountRow currently checks `discountCents === 0`, which misses
null/undefined and allows `currencyAmountFromCents(discountCents)` to print NaN;
change the early-return to a falsy check (e.g., `if (!discountCents) return
null;`) so the row doesn't render for 0, null, or undefined values and ensure
`currencyAmountFromCents` only receives a valid number.
| const isNegative = total < 0; | ||
| const sign = isNegative ? "-" : ""; | ||
| const color = isNegative ? "primary.dark" : (total === 0 ? "text.primary" : "error.main"); | ||
| const totalStr = `${sign}${currencyAmountFromCents(Math.abs(total))}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show TotalRow.jsx around the reported lines
FILE="src/components/mui/table/extra-rows/TotalRow.jsx"
echo "---- $FILE (context) ----"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '1,120p'
else
echo "Missing file: $FILE" >&2
fi
# 2) Locate currencyAmountFromCents
echo "---- Searching currencyAmountFromCents usage ----"
rg -n "currencyAmountFromCents" -S src || true
# 3) Find the definition of currencyAmountFromCents (any of several common names)
echo "---- Searching for its implementation ----"
rg -n "function currencyAmountFromCents|const currencyAmountFromCents|currencyAmountFromCents\s*=\s*\(" -S src || true
rg -n "currencyAmountFromCents" -S . || true
# 4) Read the definition file(s) if found
# (We capture candidate files first to avoid huge reads.)
CANDIDATE_FILES="$(rg -l "currencyAmountFromCents" -S src || true)"
echo "---- Candidate files ----"
echo "$CANDIDATE_FILES" | sed 's/^/ - /' || true
# For each candidate file, print line range around definition patterns
for f in $CANDIDATE_FILES; do
echo "---- Inspecting $f (possible implementation area) ----"
# Print just the matching lines with a bit of context
nl -ba "$f" | rg -n "currencyAmountFromCents" -n -C 3 || true
done
# 5) Trace how TotalRow is used and where `total` comes from
echo "---- Searching TotalRow props usage ----"
rg -n "<TotalRow\b" -S src || true
rg -n "TotalRow\s*\(" -S src || true
rg -n "total\s*=" -S src/components/mui/table || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/mui/table/extra-rows/TotalRow.jsx"
echo "Repo files exist?"
ls -la src/components/mui/table/extra-rows || true
ls -la "src/components/mui/table/extra-rows/TotalRow.jsx" || true
echo "---- $FILE (context) ----"
if [ -f "$FILE" ]; then
awk 'NR<=140{printf "%6d %s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE" >&2
fi
echo "---- Searching currencyAmountFromCents usage (in src) ----"
rg -n "currencyAmountFromCents" -S src || true
echo "---- Searching for currencyAmountFromCents implementation ----"
rg -n "function currencyAmountFromCents|const currencyAmountFromCents|currencyAmountFromCents\s*=\s*\(" -S src || true
CANDIDATE_FILES="$(rg -l "currencyAmountFromCents" -S src || true)"
echo "---- Candidate files (contain currencyAmountFromCents) ----"
echo "$CANDIDATE_FILES" | sed 's/^/ - /' || true
for f in $CANDIDATE_FILES; do
echo "---- Inspecting $f (match contexts) ----"
awk '{
if (NR>=1 && NR<=4000) {}
}' "$f" >/dev/null 2>&1 || true
# Print a small window around each match
rg -n "currencyAmountFromCents" -S "$f" -C 8 || true
done
echo "---- Searching for TotalRow usage to trace `total` origin ----"
rg -n "<TotalRow\b" -S src || true
rg -n "TotalRow\s*\(" -S src || true
rg -n "total\s*=" -S src/components/mui/table || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 35890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect TotalRow call site(s)
FILE="src/components/mui/SponsorOrderGrid/index.js"
echo "---- $FILE: TotalRow call context ----"
if [ -f "$FILE" ]; then
# Print a reasonable window around the first "TotalRow" occurrence
FIRST=$(rg -n "TotalRow" "$FILE" | head -n 1 | cut -d: -f1)
echo "First TotalRow line: $FIRST"
START=$((FIRST-60))
if [ "$START" -lt 1 ]; then START=1; fi
END=$((FIRST+120))
awk -v s="$START" -v e="$END" 'NR>=s && NR<=e{printf "%6d %s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE" >&2
fi
# Find any other TotalRow usage
echo "---- All TotalRow occurrences in src/components/mui ----"
rg -n "TotalRow" src/components/mui -S || true
# Inspect TotalRow unit test (if it exists)
TEST_FILE="src/components/mui/__tests__/total-row.test.js"
echo "---- $TEST_FILE (context) ----"
if [ -f "$TEST_FILE" ]; then
awk 'NR<=200{printf "%6d %s\n", NR, $0}' "$TEST_FILE"
else
echo "Missing file: $TEST_FILE" >&2
fi
# Inspect remaining part of TotalRow test if file longer
if [ -f "$TEST_FILE" ]; then
LINES=$(wc -l < "$TEST_FILE")
echo "Test file lines: $LINES"
fi
# Inspect currency helpers used to compute totals (look for "amount" sums)
echo "---- Searching for 'cancelledTotal'/'retained'/'credited'/'total' computations ----"
rg -n "cancelledTotal|refundsTotal|retained|credited|total\s*:" src/components/mui/SponsorOrderGrid/index.js -S || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 10461
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/mui/SponsorOrderGrid/index.js"
# Show around the TotalRow render (line ~299)
START=260
END=360
echo "---- $FILE (lines $START-$END) ----"
awk -v s="$START" -v e="$END" 'NR>=s && NR<=e{printf "%6d %s\n", NR, $0}' "$FILE"
echo "---- Re-check TotalRow unit test file for currencyAmountFromCents mocking ----"
TEST_FILE="src/components/mui/__tests__/total-row.test.js"
awk 'NR<=120{printf "%6d %s\n", NR, $0}' "$TEST_FILE" | rg -n "currencyAmountFromCents|mock\\(\"\\.{0,3}/utils/money|money\\\"" || true
echo "---- Inspect src/utils/money.js amountFromCents (for rounding expectations) ----"
MONEY_FILE="src/utils/money.js"
# Print around amountFromCents too
rg -n "function amountFromCents|amountFromCents\s*=" "$MONEY_FILE" || true
LINE=$(rg -n "amountFromCents" "$MONEY_FILE" | head -n 1 | cut -d: -f1)
START=$((LINE-20)); END=$((LINE+60))
if [ "$START" -lt 1 ]; then START=1; fi
awk -v s="$START" -v e="$END" 'NR>=s && NR<=e{printf "%6d %s\n", NR, $0}' "$MONEY_FILE"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 6201
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all occurrences where the order object shape is mentioned/constructed
rg -n "cancelled_total|refunds_total|credited_to_payment_method|retained|amount_due|order\.total|total\W*:" -S src || true
# Specifically track "order" passed into SponsorOrderGrid
rg -n "<SponsorOrderGrid|SponsorOrderGrid\\s*\\(" -S src || true
# If we find a caller, inspect its logic around constructing/transforming "order"
# (We limit output by printing only relevant windows around matches.)
CALLER_FILES="$(rg -l "<SponsorOrderGrid|SponsorOrderGrid\\s*\\(" src || true)"
echo "---- Caller files ----"
echo "$CALLER_FILES" | sed 's/^/ - /' || true
for f in $CALLER_FILES; do
echo "---- Inspecting $f (around SponsorOrderGrid usage) ----"
rg -n "<SponsorOrderGrid|SponsorOrderGrid\\s*\\(" -S "$f" -n -C 6 || true
done
# Track if any API/mapper converts money to cents using parseFloat/rounding
rg -n "amount.*(cents|cent)|toCents|fromCents|parseFloat|Math\.round|Math\.floor|Math\.ceil|/\\s*100|\\*\\s*100" -S src || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 16247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect TotalFooter.jsx
FILE="src/components/mui/SponsorOrderGrid/components/TotalFooter.jsx"
echo "---- $FILE ----"
if [ -f "$FILE" ]; then
awk 'NR<=120{printf "%6d %s\n", NR, $0}' "$FILE"
else
echo "Missing file: $FILE" >&2
fi
# 2) Find non-test usages of SponsorOrderGrid and inspect what is passed as order/total
echo "---- Searching for SponsorOrderGrid usage (excluding __tests__) ----"
rg -n "SponsorOrderGrid" src -S -g'!*__tests__*' || true
# 3) Also search for ".total" passed with SponsorOrderGrid
rg -n "SponsorOrderGrid[\\s\\S]*total=" src -S -g'!*__tests__*' || true
# 4) Look for an 'order' object being passed to SponsorOrderGrid
rg -n "<SponsorOrderGrid[\\s\\S]*order=" src -S -g'!*__tests__*' || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 3538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find Jest manual mocks for money
rg -n "__mocks__/.*money|jest\\.mock\\(.+money|money\\.js" -S . || true
# Specifically inspect total-row.test.js for any money mock/setup
rg -n "mock\\(|currencyAmountFromCents" src/components/mui/__tests__/total-row.test.js || true
ls -la src/components/mui/__tests__ || true
# Search for jest setup files
rg -n "setupFilesAfterEnv|setupFiles" package.json jest.config.* jest.* -S . || true
fd -t f "jest.config.*" . || true
fd -t f "*setup*jest*" . || true
# List __mocks__ directories
fd -t d "__mocks__" . || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 6599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Listing src/__mocks__ ----"
ls -la src/__mocks__ || true
echo "---- Searching for money mocks ----"
fd -t f "__mocks__" -a src/__mocks__ . 2>/dev/null || true
rg -n "currencyAmountFromCents" -S src/__mocks__ || true
rg -n "__mocks__" -S src || true
rg -n "jest\\.mock\\(" -S src/components/mui/__tests__/total-row.test.js || true
echo "---- Searching for a manual mock file for utils/money ----"
fd -t f "*money*mock*" src . 2>/dev/null || true
fd -t f "money.*" src/__mocks__ . 2>/dev/null || true
find src -type f \( -name "*money*.js" -o -path "*__mocks__*" \) | head -n 200
echo "---- Grep for explicit module mocking of utils/money ----"
rg -n "utils/money" -S src/components/mui/__tests__/total-row.test.js src/components/mui/__tests__ || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 1003
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show total-row.test.js fully (it's small)
TEST_FILE="src/components/mui/__tests__/total-row.test.js"
echo "---- $TEST_FILE ----"
wc -l "$TEST_FILE" || true
awk '{printf "%6d %s\n", NR, $0}' "$TEST_FILE" || true
# 2) Locate Jest config / setup files
echo "---- Searching for jest config files ----"
ls -la | rg -n "jest|jasmine" || true
fd -t f "jest*.js" . || true
fd -t f "*jest*.config*" . || true
fd -t f "*jest*" . -g"*.json" -g"*.js" -g"*.cjs" -g"*.mjs" || true
# If package.json exists, print relevant jest fields
if [ -f "package.json" ]; then
echo "---- package.json (jest fields) ----"
node -e "const p=require('./package.json'); console.log(Object.keys(p).filter(k=>k.toLowerCase().includes('jest')||k==='jest').map(k=>k+':').join('\n'));"
node -e "const p=require('./package.json'); console.log(JSON.stringify(p.jest||{},null,2));" || true
fi
# 3) Search for global money mocks in Jest setup
echo "---- Searching for global mocks of utils/money ----"
rg -n "jest\\.mock\\(.*utils/money|jest\\.mock\\(.*money" -S . || true
rg -n "moduleNameMapper|__mocks__|setupFiles.*jest|setupFilesAfterEnv" -S . || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 5219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search in total-row.test.js for string totals (already seen) and numeric cases
echo "---- total-row.test.js totals ----"
rg -n "total\s*:" src/components/mui/__tests__/total-row.test.js || true
# Search in SponsorOrderGrid tests for non-integer / nullish totals
echo "---- SponsorOrderGrid.test.js totals ----"
rg -n "total\s*:" src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js || true
# Search for cancelled_total/refunds_total/retained used as "total" in UI
echo "---- SponsorOrderGrid.test.js monetary fields ----"
rg -n "cancelled_total|refunds_total|retained|credited_to_payment_method|discount_total|total\s*:" src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js || true
# Look for any other tests referencing TotalRow/TotalFooter formatting with non-integers
echo "---- any other TotalFooter/TotalRow money usage in tests ----"
rg -n "TotalFooter|TotalRow|currencyAmountFromCents\\(Math\\.abs\\(total\\)|Math\\.abs\\(total\\)" src/components/mui/__tests__ src/components/mui/SponsorOrderGrid/__tests__ -S || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any instantiation of SponsorOrderGrid (excluding its own tests)
rg -n "SponsorOrderGrid" src -S -g'!*__tests__*' || true
# Find any instantiation of the exported alias MuiSponsorOrderGrid
rg -n "MuiSponsorOrderGrid" src -S -g'!*__tests__*' || true
# Search for "amount_due" / "cancelled_total" / "refunds_total" usage with SponsorOrderGrid
rg -n "cancelled_total|refunds_total|amount_due|credited_to_payment_method|retained" src -S -g'!*__tests__*' || trueRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 2870
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TEST_FILE="src/components/mui/SponsorOrderGrid/__tests__/SponsorOrderGrid.test.js"
echo "---- $TEST_FILE (head 220 lines) ----"
awk 'NR<=220{printf "%6d %s\n", NR, $0}' "$TEST_FILE"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 7026
Fix TotalRow/TotalFooter crash when total isn’t an integer cents number
currencyAmountFromCentsthrows unlesscentsis anumberandNumber.isInteger(cents).src/components/mui/table/extra-rows/TotalRow.jsxformatscurrencyAmountFromCents(Math.abs(total))at line 26; iftotalis fractional/NaN/nullish (or a formatted string like"$1,234"fromsrc/components/mui/__tests__/total-row.test.js),Math.abs(total)becomesNaNand the component crashes.- Add a guard/type handling before calling
currencyAmountFromCents(fallback to0for nullish/NaN; only round whentypeof total === "number"; iftotalcan be a preformatted string, render it directly). - Same risk exists in
src/components/mui/SponsorOrderGrid/components/TotalFooter.jsx(usescurrencyAmountFromCents(Math.abs(total))).
Upstream contract decision: SponsorOrderGrid forwards order.total without conversion—either enforce “integer cents” at that boundary or handle coercion safely in the view.
🧰 Tools
🪛 GitHub Actions: jest / 0_build.txt
[error] 26-26: TotalRow render crashed at currency formatting call: currencyAmountFromCents (from src/utils/money.js:163).
🪛 GitHub Actions: jest / build
[error] 26-26: TotalRow crashed while rendering due to currencyAmountFromCents error (non-integer cents).
🤖 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/mui/table/extra-rows/TotalRow.jsx` around lines 23 - 26, The
TotalRow/TotalFooter crash happens because currencyAmountFromCents is called
with Math.abs(total) when total can be non-integer/NaN/nullish/or a preformatted
string; update the rendering logic around total (variables isNegative, sign,
totalStr) to sanitize/guard before calling currencyAmountFromCents: set
isNegative = (typeof total === "number" && total < 0), if typeof total ===
"number" and Number.isFinite(total) compute sanitizedCents =
Math.round(Math.abs(total)) and pass that to currencyAmountFromCents, if typeof
total === "string" treat it as a preformatted string and render it directly
(skip currencyAmountFromCents), and for null/NaN/other types fallback
sanitizedCents = 0; apply the same fix in TotalFooter (same
currencyAmountFromCents(Math.abs(total)) usage).
https://app.clickup.com/t/86ba505mg
Summary by CodeRabbit
New Features
Refactor
Chores