Merged
Conversation
📝 WalkthroughWalkthroughPackage version bumped to 5.0.17-beta.4. DashboardCard component replaced with three new card variants (InlineCard, ListCard, TableCard). Three new table row components added (FeeRow, PaymentRow, RefundRow). Supporting utilities, exports, tests, i18n strings, and webpack configuration updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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: 5
🧹 Nitpick comments (4)
src/components/mui/cards/TableCard/index.js (1)
37-50: Prefer stable keys over index-derived keys for mapped items.On Line 39, Line 45, and Line 49, index-based keys can cause reconciliation issues when data order changes. Use stable data keys where possible.
💡 Suggested fix
- {columns.map((col, i) => ( + {columns.map((col) => ( // eslint-disable-next-line react/no-array-index-key - <Heading key={`col-${col.key}-${i}`}>{col.label}</Heading> + <Heading key={`col-${col.key}`}>{col.label}</Heading> ))} @@ - {rows.map((row, i) => ( + {rows.map((row, i) => ( // eslint-disable-next-line react/no-array-index-key - <React.Fragment key={`row-${i}`}> + <React.Fragment key={row.id ?? row.ref ?? `row-${i}`}> @@ - {columns.map((col, j) => ( + {columns.map((col) => ( // eslint-disable-next-line react/no-array-index-key - <Value key={`col-val-${col.key}-${j}`}>{row[col.key]}</Value> + <Value key={`col-val-${col.key}`}>{row[col.key]}</Value> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/cards/TableCard/index.js` around lines 37 - 50, Replace the index-derived keys used in the mapped JSX for columns and rows with stable unique identifiers: use col.key alone for column Heading and Value keys, and use a stable unique property from each row (e.g., row.id or another unique field) for the row React.Fragment key (e.g., key={`row-${row.id}`}) and for any per-cell keys that need to combine row and column (e.g., key={`cell-${row.id}-${col.key}`}); update the mappings in the TableCard component (the columns.map, rows.map, and inner columns.map) to remove the index-based keys and the eslint-disable comments.src/components/mui/__tests__/refund-row.test.js (1)
37-70: Add coverage forcolGapandtrailingcell rendering.The suite currently validates content, but not the dynamic spacer/trailing columns. A focused assertion on rendered
<td>count would lock down this new behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/__tests__/refund-row.test.js` around lines 37 - 70, The tests for RefundRow (use renderInTable to mount) are missing assertions that the dynamic spacer/trailing cells are rendered; update the suite to, after rendering via renderInTable({ refund: ... }), assert the number of <td> elements (container.querySelectorAll("td")) matches the expected count and also assert the existence of the spacer and trailing cells by selecting their identifying classes or attributes (e.g., a cell with class/name colGap and a cell with class/name trailing) so colGap and trailing cell rendering is covered.src/components/mui/__tests__/fee-row.test.js (1)
37-62: Consider adding coverage forcolGapandtrailinglayout behavior.The suite is good, but one focused test for gap/trailing cell counts would lock in the new layout contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/__tests__/fee-row.test.js` around lines 37 - 62, Add a focused test in the FeeRow suite to assert the new layout contract for colGap and trailing: render the component via renderInTable with a fee and with colGap and trailing props set (e.g., colGap: X, trailing: Y) and assert the expected number of cells/columns and spacing behavior by querying container.querySelectorAll("td") or specific test ids and checking computed styles or class names; update or add tests named like "renders correct cells for colGap and trailing" referencing renderInTable and FeeRow so the suite verifies both cell count and the trailing-gap layout.src/components/mui/table/extra-rows/TotalRow.jsx (1)
21-21: Use nullish coalescing forlabelprop for consistency with best practices.The current
||operator treats""as absent, while??only treatsnullandundefinedas absent. Although no callers currently pass empty strings, using??is the more precise operator for this pattern.♻️ Proposed change
- const totalLabel = label || T.translate("mui_table.total"); + const totalLabel = label ?? T.translate("mui_table.total");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/table/extra-rows/TotalRow.jsx` at line 21, Replace the fallback using the logical OR with nullish coalescing for the label prop: update the expression that defines totalLabel (currently using `label || T.translate("mui_table.total")`) to use `??` so it only falls back when `label` is null or undefined (i.e., `totalLabel = label ?? T.translate("mui_table.total")`), keeping the rest of the component and translation call unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/mui/__tests__/payment-row.test.js`:
- Around line 28-29: The inline comment for the PAYMENT_TIMESTAMP constant is
incorrect; update the comment that currently reads "2026-01-15 00:00:00 UTC" to
the correct date "2025-01-15 00:00:00 UTC" so it matches the numeric value
1736899200 in the PAYMENT_TIMESTAMP constant used in the payment-row.test.js
file.
In `@src/components/mui/cards/ListCard/index.js`:
- Around line 29-33: The ListItem key using row.id ?? row.label can produce
duplicate keys when ids are missing; fix by ensuring the data source always
provides row.id where possible, and when a fallback is necessary use a stable,
namespaced fallback key (e.g., include the label plus the index like
"row-{label}-{i}") and mirror the repo pattern by adding a single-line eslint
disable for react/no-array-index-key above the ListItem key; update the mapping
that renders rows (the rows array/map and the ListItem JSX) to use this
namespaced fallback so keys are unique and consistent.
In `@src/components/mui/table/extra-rows/FeeRow.jsx`:
- Around line 21-47: In FeeRow, guard the colGap and trailing props before
calling Array(colGap)/Array(trailing): ensure colGap and trailing are normalized
to safe non-negative integers (e.g., coerce with parseInt/Number and fall back
to 0, or use Math.max(0, Math.floor(value))) at the top of the FeeRow component
so the [...Array(...)] maps never receive negative or non-integer lengths;
update references in the mapping expressions (the two [...Array(colGap)] and
[...Array(trailing)] usages) to use these sanitized variables.
In `@src/components/mui/table/extra-rows/PaymentRow.jsx`:
- Around line 23-63: PaymentRow currently constructs arrays with Array(colGap)
and Array(trailing) and calls moment(payment.created...) without validating
inputs; add defensive validation: coerce and clamp colGap and trailing at the
top of PaymentRow (e.g. let safeColGap = Math.max(0, Number.isFinite(colGap) ?
Math.floor(colGap) : 0) and same for trailing) and replace
Array(colGap)/Array(trailing) usage with Array(safeColGap)/Array(safeTrailing);
also validate payment.created before using moment (e.g. const ts =
Number.isFinite(payment?.created) ? payment.created * MILLISECONDS_IN_SECOND :
null and render a safe fallback when ts is null); apply the same validation
pattern to FeeRow, RefundRow, and TotalRow for their colGap/trailing and any
timestamp usage to avoid RangeError and invalid date formatting.
In `@src/components/mui/table/extra-rows/RefundRow.jsx`:
- Around line 46-61: The RefundRow component should sanitize its colGap and
trailing props at entry to avoid RangeError from Array(colGap)/Array(trailing);
inside the RefundRow function/component, coerce each prop to a safe non-negative
integer (e.g., let safeColGap = Math.max(0, Math.floor(Number(colGap) || 0)) and
let safeTrailing = Math.max(0, Math.floor(Number(trailing) || 0))) and then use
safeColGap and safeTrailing in the two [...Array(...)] map calls; this prevents
negative, non-integer, or non-numeric inputs from causing runtime crashes while
preserving the existing rendering logic (refer to the RefundRow component and
its uses of colGap/trailing).
---
Nitpick comments:
In `@src/components/mui/__tests__/fee-row.test.js`:
- Around line 37-62: Add a focused test in the FeeRow suite to assert the new
layout contract for colGap and trailing: render the component via renderInTable
with a fee and with colGap and trailing props set (e.g., colGap: X, trailing: Y)
and assert the expected number of cells/columns and spacing behavior by querying
container.querySelectorAll("td") or specific test ids and checking computed
styles or class names; update or add tests named like "renders correct cells for
colGap and trailing" referencing renderInTable and FeeRow so the suite verifies
both cell count and the trailing-gap layout.
In `@src/components/mui/__tests__/refund-row.test.js`:
- Around line 37-70: The tests for RefundRow (use renderInTable to mount) are
missing assertions that the dynamic spacer/trailing cells are rendered; update
the suite to, after rendering via renderInTable({ refund: ... }), assert the
number of <td> elements (container.querySelectorAll("td")) matches the expected
count and also assert the existence of the spacer and trailing cells by
selecting their identifying classes or attributes (e.g., a cell with class/name
colGap and a cell with class/name trailing) so colGap and trailing cell
rendering is covered.
In `@src/components/mui/cards/TableCard/index.js`:
- Around line 37-50: Replace the index-derived keys used in the mapped JSX for
columns and rows with stable unique identifiers: use col.key alone for column
Heading and Value keys, and use a stable unique property from each row (e.g.,
row.id or another unique field) for the row React.Fragment key (e.g.,
key={`row-${row.id}`}) and for any per-cell keys that need to combine row and
column (e.g., key={`cell-${row.id}-${col.key}`}); update the mappings in the
TableCard component (the columns.map, rows.map, and inner columns.map) to remove
the index-based keys and the eslint-disable comments.
In `@src/components/mui/table/extra-rows/TotalRow.jsx`:
- Line 21: Replace the fallback using the logical OR with nullish coalescing for
the label prop: update the expression that defines totalLabel (currently using
`label || T.translate("mui_table.total")`) to use `??` so it only falls back
when `label` is null or undefined (i.e., `totalLabel = label ??
T.translate("mui_table.total")`), keeping the rest of the component and
translation call unchanged.
🪄 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: a9c2ca24-de37-44d7-ba35-e2cae99cf15e
📒 Files selected for processing (22)
package.jsonsrc/components/index.jssrc/components/mui/DashboardCard/index.jssrc/components/mui/__tests__/dashboard-card.test.jssrc/components/mui/__tests__/fee-row.test.jssrc/components/mui/__tests__/payment-row.test.jssrc/components/mui/__tests__/refund-row.test.jssrc/components/mui/__tests__/table-card.test.jssrc/components/mui/cards/InlineCard/index.jssrc/components/mui/cards/ListCard/index.jssrc/components/mui/cards/TableCard/index.jssrc/components/mui/cards/components/Heading.jssrc/components/mui/cards/components/Value.jssrc/components/mui/cards/index.jssrc/components/mui/table/extra-rows/FeeRow.jsxsrc/components/mui/table/extra-rows/PaymentRow.jsxsrc/components/mui/table/extra-rows/RefundRow.jsxsrc/components/mui/table/extra-rows/TotalRow.jsxsrc/components/mui/table/extra-rows/index.jssrc/i18n/en.jsonsrc/utils/constants.jswebpack.common.js
💤 Files with no reviewable changes (2)
- src/components/mui/tests/dashboard-card.test.js
- src/components/mui/DashboardCard/index.js
Comment on lines
+28
to
+29
| // 2026-01-15 00:00:00 UTC in seconds | ||
| const PAYMENT_TIMESTAMP = 1736899200; |
There was a problem hiding this comment.
Fix the timestamp comment to match the constant value.
1736899200 is 2025-01-15 00:00:00 UTC, not 2026. The assertion logic is fine, but this comment is misleading.
Suggested patch
-// 2026-01-15 00:00:00 UTC in seconds
+// 2025-01-15 00:00:00 UTC in seconds
const PAYMENT_TIMESTAMP = 1736899200;📝 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.
Suggested change
| // 2026-01-15 00:00:00 UTC in seconds | |
| const PAYMENT_TIMESTAMP = 1736899200; | |
| // 2025-01-15 00:00:00 UTC in seconds | |
| const PAYMENT_TIMESTAMP = 1736899200; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/__tests__/payment-row.test.js` around lines 28 - 29, The
inline comment for the PAYMENT_TIMESTAMP constant is incorrect; update the
comment that currently reads "2026-01-15 00:00:00 UTC" to the correct date
"2025-01-15 00:00:00 UTC" so it matches the numeric value 1736899200 in the
PAYMENT_TIMESTAMP constant used in the payment-row.test.js file.
Comment on lines
+29
to
+33
| {(rows ?? []).map((row, i) => ( | ||
| <ListItem | ||
| key={row.id ?? row.label} | ||
| divider={i < (rows ?? []).length - 1} | ||
| sx={{ display: "flex", height: 50 }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/mui/cards/ListCard/index.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 1856
🏁 Script executed:
# Check for prop types definition
fd -t f "ListCard" --exec grep -l "propTypes\|interface ListCard\|type ListCard" {} \;Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
# Search for ListCard usage in the codebase
rg "ListCard" --type jsx --type js --type tsx --type ts -B 2 -A 5Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 112
🏁 Script executed:
# Search for ListCard usage more broadly
rg "ListCard" -B 2 -A 5Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2575
🏁 Script executed:
# Also check if there are TypeScript definitions or interfaces
fd -t f "\\.tsx?$" | xargs grep -l "ListCard" 2>/dev/null | head -20Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
# Find all files that import or use ListCard/MuiListCard
rg "import.*ListCard|MuiListCard" -B 2 -A 10Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1105
🏁 Script executed:
# Also check if there's any test file or documentation
fd -t f "ListCard" -o "list-card"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 222
🏁 Script executed:
# Look for README or documentation about components
find . -name "README*" -o -name "CONTRIBUTING*" | head -10Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
# Check similar cards to see if they have same pattern
cat -n src/components/mui/cards/InlineCard/index.js 2>/dev/null | head -50Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2087
🏁 Script executed:
# Check if there's a shared pattern in other list/table implementations
rg "\.map.*key=" src/components --type js -A 1 -B 1Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
# Check TableCard for comparison
cat -n src/components/mui/cards/TableCard/index.js 2>/dev/null | head -80 | tail -50Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1925
🏁 Script executed:
# Search for react/no-array-index-key eslint disable comments
rg "no-array-index-key" -B 2 -A 2Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 4969
Duplicate keys possible when row.id is missing, but suggested fix introduces worse anti-pattern.
The concern about row.id ?? row.label is valid—if multiple rows share the same label, React cannot distinguish them, causing unstable reconciliation. However, the suggested fix using ${row.label}-${i} introduces array index keys, which is a React anti-pattern that causes reordering bugs and contradicts the codebase's own approach (see InlineCard and TableCard, which explicitly disable react/no-array-index-key when index-based keys are necessary).
Better alternatives:
- Ensure
row.idis always populated in the data source (most reliable) - If fallback is needed, follow the codebase pattern:
key={row-${row.label}-${i}}with// eslint-disable-next-line react/no-array-index-key - Or implement a stable fallback:
key={row.id ??${row.label}-${indexOf}}where indexOf is a stable identifier from data context
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/cards/ListCard/index.js` around lines 29 - 33, The
ListItem key using row.id ?? row.label can produce duplicate keys when ids are
missing; fix by ensuring the data source always provides row.id where possible,
and when a fallback is necessary use a stable, namespaced fallback key (e.g.,
include the label plus the index like "row-{label}-{i}") and mirror the repo
pattern by adding a single-line eslint disable for react/no-array-index-key
above the ListItem key; update the mapping that renders rows (the rows array/map
and the ListItem JSX) to use this namespaced fallback so keys are unique and
consistent.
Comment on lines
+21
to
+47
| const FeeRow = ({ fee, colGap = 3, trailing = 0 }) => { | ||
| if (!fee) return null; | ||
|
|
||
| return ( | ||
| <TableRow> | ||
| <TableCell>{T.translate("mui_table.payfee")}</TableCell> | ||
| <TableCell> | ||
| <Typography variant="body2" sx={{ fontWeight: 500 }}> | ||
| {fee.title} | ||
| </Typography> | ||
| </TableCell> | ||
| {[...Array(colGap)].map((_, i) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <TableCell key={`fee-col-gap-${i}`} /> | ||
| ))} | ||
| <TableCell> | ||
| <Typography | ||
| variant="body2" | ||
| sx={{ color: "success.main", fontWeight: 500 }} | ||
| > | ||
| {currencyAmountFromCents(fee.amount)} | ||
| </Typography> | ||
| </TableCell> | ||
| {[...Array(trailing)].map((_, i) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <TableCell key={`fee-trailing-col-${i}`} sx={{ width: 40 }} /> | ||
| ))} |
There was a problem hiding this comment.
Guard colGap/trailing before using Array(...).
Array(colGap)/Array(trailing) can throw for invalid values (e.g., negative or non-integer). Consider normalizing these props to safe non-negative integers.
Suggested patch
const FeeRow = ({ fee, colGap = 3, trailing = 0 }) => {
if (!fee) return null;
+ const safeColGap = Number.isInteger(colGap) && colGap > 0 ? colGap : 0;
+ const safeTrailing = Number.isInteger(trailing) && trailing > 0 ? trailing : 0;
return (
<TableRow>
@@
- {[...Array(colGap)].map((_, i) => (
+ {[...Array(safeColGap)].map((_, i) => (
// eslint-disable-next-line react/no-array-index-key
<TableCell key={`fee-col-gap-${i}`} />
))}
@@
- {[...Array(trailing)].map((_, i) => (
+ {[...Array(safeTrailing)].map((_, i) => (
// eslint-disable-next-line react/no-array-index-key
<TableCell key={`fee-trailing-col-${i}`} sx={{ width: 40 }} />
))}📝 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.
Suggested change
| const FeeRow = ({ fee, colGap = 3, trailing = 0 }) => { | |
| if (!fee) return null; | |
| return ( | |
| <TableRow> | |
| <TableCell>{T.translate("mui_table.payfee")}</TableCell> | |
| <TableCell> | |
| <Typography variant="body2" sx={{ fontWeight: 500 }}> | |
| {fee.title} | |
| </Typography> | |
| </TableCell> | |
| {[...Array(colGap)].map((_, i) => ( | |
| // eslint-disable-next-line react/no-array-index-key | |
| <TableCell key={`fee-col-gap-${i}`} /> | |
| ))} | |
| <TableCell> | |
| <Typography | |
| variant="body2" | |
| sx={{ color: "success.main", fontWeight: 500 }} | |
| > | |
| {currencyAmountFromCents(fee.amount)} | |
| </Typography> | |
| </TableCell> | |
| {[...Array(trailing)].map((_, i) => ( | |
| // eslint-disable-next-line react/no-array-index-key | |
| <TableCell key={`fee-trailing-col-${i}`} sx={{ width: 40 }} /> | |
| ))} | |
| const FeeRow = ({ fee, colGap = 3, trailing = 0 }) => { | |
| if (!fee) return null; | |
| const safeColGap = Number.isInteger(colGap) && colGap > 0 ? colGap : 0; | |
| const safeTrailing = Number.isInteger(trailing) && trailing > 0 ? trailing : 0; | |
| return ( | |
| <TableRow> | |
| <TableCell>{T.translate("mui_table.payfee")}</TableCell> | |
| <TableCell> | |
| <Typography variant="body2" sx={{ fontWeight: 500 }}> | |
| {fee.title} | |
| </Typography> | |
| </TableCell> | |
| {[...Array(safeColGap)].map((_, i) => ( | |
| // eslint-disable-next-line react/no-array-index-key | |
| <TableCell key={`fee-col-gap-${i}`} /> | |
| ))} | |
| <TableCell> | |
| <Typography | |
| variant="body2" | |
| sx={{ color: "success.main", fontWeight: 500 }} | |
| > | |
| {currencyAmountFromCents(fee.amount)} | |
| </Typography> | |
| </TableCell> | |
| {[...Array(safeTrailing)].map((_, i) => ( | |
| // eslint-disable-next-line react/no-array-index-key | |
| <TableCell key={`fee-trailing-col-${i}`} sx={{ width: 40 }} /> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/table/extra-rows/FeeRow.jsx` around lines 21 - 47, In
FeeRow, guard the colGap and trailing props before calling
Array(colGap)/Array(trailing): ensure colGap and trailing are normalized to safe
non-negative integers (e.g., coerce with parseInt/Number and fall back to 0, or
use Math.max(0, Math.floor(value))) at the top of the FeeRow component so the
[...Array(...)] maps never receive negative or non-integer lengths; update
references in the mapping expressions (the two [...Array(colGap)] and
[...Array(trailing)] usages) to use these sanitized variables.
Comment on lines
+23
to
+63
| const PaymentRow = ({ payment, colGap = 1, trailing = 0 }) => { | ||
|
|
||
| if (!payment) return null; | ||
|
|
||
| return ( | ||
| <TableRow sx={{backgroundColor: "#2E7D3214"}}> | ||
| <TableCell>{T.translate("mui_table.pay")}</TableCell> | ||
| <TableCell> | ||
| <Typography | ||
| variant="body2" | ||
| sx={{ color: "success.main", fontWeight: 500 }} | ||
| > | ||
| {T.translate("mui_table.payment")} | ||
| </Typography> | ||
| </TableCell> | ||
| <TableCell> | ||
| <Typography variant="body2" sx={{ color: "text.secondary" }}> | ||
| {T.translate("mui_table.paid_via")} {payment.method} | ||
| </Typography> | ||
| </TableCell> | ||
| <TableCell> | ||
| <Typography variant="body2" sx={{ color: "text.secondary" }}> | ||
| {moment(payment.created * MILLISECONDS_IN_SECOND).format(DATETIME_FORMAT)} | ||
| </Typography> | ||
| </TableCell> | ||
| {[...Array(colGap)].map((_, i) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <TableCell key={`pay-col-gap-${i}`} /> | ||
| ))} | ||
| <TableCell> | ||
| <Typography | ||
| variant="body2" | ||
| sx={{ color: "success.main", fontWeight: 500 }} | ||
| > | ||
| -{currencyAmountFromCents(payment.amount)} | ||
| </Typography> | ||
| </TableCell> | ||
| {[...Array(trailing)].map((_, i) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <TableCell key={`pay-trailing-col-${i}`} sx={{ width: 40 }} /> | ||
| ))} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect PaymentRow callsites and passed props to confirm whether created/colGap/trailing are always sanitized.
rg -nP --type=js --type=jsx -C3 '<(Mui)?PaymentRow\b'
rg -nP --type=js --type=jsx -C3 '\bcolGap\s*=|\btrailing\s*=|\bcreated\b'Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 140
🏁 Script executed:
# Search for PaymentRow callsites and prop usage without file type restrictions
rg -n 'PaymentRow' --type js -C3 .Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 3941
🏁 Script executed:
# Find the PaymentRow component file to confirm it exists
find . -name "PaymentRow.jsx" -o -name "PaymentRow.js"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 137
🏁 Script executed:
# Search for how colGap and trailing are passed when calling PaymentRow
rg -n 'colGap|trailing' --type js -C2 .Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 7070
🏁 Script executed:
cat -n ./src/components/mui/__tests__/payment-row.test.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 2941
🏁 Script executed:
# Search for actual usage of PaymentRow or MuiPaymentRow in the codebase
rg -n 'MuiPaymentRow|<PaymentRow' --type js -C3 .Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1416
🏁 Script executed:
# Check where payment objects are created/passed to understand the data flow
rg -n 'payment\s*[:=]' --type js -B2 -A2 . | head -100Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2098
🏁 Script executed:
# Search for actual usage in app components
rg -n 'MuiPaymentRow' --type js -C3 .Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 932
🏁 Script executed:
# Look for places that might construct payment objects
rg -n 'payment.*created|created.*payment' --type js -B2 -A2 . | head -80Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2595
🏁 Script executed:
# Test Array() behavior with various invalid inputs
node -e "
console.log('Array(1):', Array(1));
console.log('Array(0):', Array(0));
console.log('Array(-1):', Array(-1));
console.log('Array(NaN):', Array(NaN));
console.log('Array(Infinity):', Array(Infinity));
console.log('Array({}):', Array({}));
try { console.log('[...Array(-1)]:', [...Array(-1)]); } catch(e) { console.log('Error:', e.message); }
"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 652
🏁 Script executed:
# Check test coverage for other row components with similar pattern
cat -n ./src/components/mui/__tests__/fee-row.test.js 2>/dev/null || echo "File not found"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2601
🏁 Script executed:
cat -n ./src/components/mui/__tests__/refund-row.test.js 2>/dev/null || echo "File not found"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 3059
🏁 Script executed:
# Search for where colGap/trailing are actually passed in the component tree
rg -n 'colGap\s*=' --type js -B3 -A1 . | grep -v test | head -60Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1507
🏁 Script executed:
# Check payment data source/validation
rg -n 'payment' --type js | grep -i 'schema\|validate\|type\|interface' | head -30Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
Add defensive validation for numeric props to prevent runtime crashes.
Array(colGap) and Array(trailing) will throw a RangeError: Invalid array length if passed negative or non-finite values, breaking rendering. payment.created is also used without validation, risking invalid date formatting if the value is null or malformed. Sanitizing these inputs prevents edge-case crashes:
- Validate
colGapandtrailingwithMath.max(0, Number.isFinite(...) ? Math.floor(...) : 0) - Validate
payment.createdwithNumber.isFinite()before passing to moment
This pattern should also be applied to similar components: FeeRow, RefundRow, and TotalRow use the same unsafe Array() pattern.
Suggested fix
const PaymentRow = ({ payment, colGap = 1, trailing = 0 }) => {
if (!payment) return null;
+ const safeColGap = Math.max(0, Number.isFinite(colGap) ? Math.floor(colGap) : 0);
+ const safeTrailing = Math.max(0, Number.isFinite(trailing) ? Math.floor(trailing) : 0);
+ const createdAt = Number(payment.created);
+ const createdLabel = Number.isFinite(createdAt)
+ ? moment.unix(createdAt).format(DATETIME_FORMAT)
+ : "";
return (
<TableRow sx={{backgroundColor: "#2E7D3214"}}>
@@
<TableCell>
<Typography variant="body2" sx={{ color: "text.secondary" }}>
- {moment(payment.created * MILLISECONDS_IN_SECOND).format(DATETIME_FORMAT)}
+ {createdLabel}
</Typography>
</TableCell>
- {[...Array(colGap)].map((_, i) => (
+ {[...Array(safeColGap)].map((_, i) => (
// eslint-disable-next-line react/no-array-index-key
<TableCell key={`pay-col-gap-${i}`} />
))}
@@
- {[...Array(trailing)].map((_, i) => (
+ {[...Array(safeTrailing)].map((_, i) => (
// eslint-disable-next-line react/no-array-index-key
<TableCell key={`pay-trailing-col-${i}`} sx={{ width: 40 }} />
))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/table/extra-rows/PaymentRow.jsx` around lines 23 - 63,
PaymentRow currently constructs arrays with Array(colGap) and Array(trailing)
and calls moment(payment.created...) without validating inputs; add defensive
validation: coerce and clamp colGap and trailing at the top of PaymentRow (e.g.
let safeColGap = Math.max(0, Number.isFinite(colGap) ? Math.floor(colGap) : 0)
and same for trailing) and replace Array(colGap)/Array(trailing) usage with
Array(safeColGap)/Array(safeTrailing); also validate payment.created before
using moment (e.g. const ts = Number.isFinite(payment?.created) ?
payment.created * MILLISECONDS_IN_SECOND : null and render a safe fallback when
ts is null); apply the same validation pattern to FeeRow, RefundRow, and
TotalRow for their colGap/trailing and any timestamp usage to avoid RangeError
and invalid date formatting.
Comment on lines
+46
to
+61
| {[...Array(colGap)].map((_, i) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <TableCell key={`ref-col-gap-${i}`} /> | ||
| ))} | ||
| <TableCell> | ||
| <Typography | ||
| variant="body2" | ||
| sx={{ color: "warning.main", fontWeight: 500 }} | ||
| > | ||
| -{currencyAmountFromCents(refund.amount)} | ||
| </Typography> | ||
| </TableCell> | ||
| {[...Array(trailing)].map((_, i) => ( | ||
| // eslint-disable-next-line react/no-array-index-key | ||
| <TableCell key={`ref-trailing-col-${i}`} sx={{ width: 40 }} /> | ||
| ))} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "RefundRow.jsx" -o -name "RefundRow.js" -o -name "RefundRow.tsx"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 136
🏁 Script executed:
fd -t f "RefundRow\.(jsx|js|tsx|ts)$"Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 134
🏁 Script executed:
cat -n src/components/mui/table/extra-rows/RefundRow.jsxRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 2792
🏁 Script executed:
rg "RefundRow" --type jsx --type js --type tsx --type ts -A 3 -B 1Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 112
🏁 Script executed:
rg "RefundRow" -A 3 -B 1Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 2521
🏁 Script executed:
cat -n src/components/mui/__tests__/refund-row.test.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 3059
🏁 Script executed:
rg "RefundRow" --no-heading -B 2 -A 5Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 3551
🏁 Script executed:
rg "MuiRefundRow|<RefundRow" -B 2 -A 2Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 1029
Add input validation for colGap and trailing props to prevent runtime crashes.
On lines 46 and 58, Array(colGap) and Array(trailing) can throw RangeError if values are negative, non-integers, or invalid. Although defaults are safe (1 and 0), callers can pass invalid values. Add validation to normalize these props at component entry.
Suggested fix
const RefundRow = ({ refund, colGap = 1, trailing = 0 }) => {
if (!refund) return null;
+ const safeColGap = Number.isInteger(colGap) && colGap > 0 ? colGap : 0;
+ const safeTrailing = Number.isInteger(trailing) && trailing > 0 ? trailing : 0;
return (
{[...Array(safeColGap)].map((_, i) => (
<TableCell key={`ref-col-gap-${i}`} />
))}
- {[...Array(trailing)].map((_, i) => (
+ {[...Array(safeTrailing)].map((_, i) => (
<TableCell key={`ref-trailing-col-${i}`} sx={{ width: 40 }} />
))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/table/extra-rows/RefundRow.jsx` around lines 46 - 61, The
RefundRow component should sanitize its colGap and trailing props at entry to
avoid RangeError from Array(colGap)/Array(trailing); inside the RefundRow
function/component, coerce each prop to a safe non-negative integer (e.g., let
safeColGap = Math.max(0, Math.floor(Number(colGap) || 0)) and let safeTrailing =
Math.max(0, Math.floor(Number(trailing) || 0))) and then use safeColGap and
safeTrailing in the two [...Array(...)] map calls; this prevents negative,
non-integer, or non-numeric inputs from causing runtime crashes while preserving
the existing rendering logic (refer to the RefundRow component and its uses of
colGap/trailing).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://app.clickup.com/t/86b66n5k4
Summary by CodeRabbit
New Features
Chores