Open
Conversation
📝 WalkthroughWalkthroughRefactors rate determination by moving time-zone logic into helpers and passing a computed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant EditForm as Edit Form
participant Helper as getCurrentApplicableRate()
participant FormTable as FormItemTable
participant UI as Inputs / UnderlyingAlertNote
EditForm->>Helper: call getCurrentApplicableRate(showTimeZone, showMetadata)
Helper-->>EditForm: "early_bird" | "standard" | "onsite" | "expired"
EditForm->>FormTable: render with currentApplicableRate
FormTable->>UI: render rows (price cells via formatRate)
FormTable->>UI: render UnderlyingAlertNote when items incomplete
UI-->>FormTable: user interactions (quantity/inputs, may be disabled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/mui/formik-inputs/mui-formik-discountfield.js (1)
105-108: Adddisabledto propTypes for completeness.The
disabledprop is accepted and used but not declared in propTypes.🔧 Proposed fix
MuiFormikDiscountField.propTypes = { name: PropTypes.string.isRequired, - label: PropTypes.string + label: PropTypes.string, + disabled: PropTypes.bool };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/formik-inputs/mui-formik-discountfield.js` around lines 105 - 108, MuiFormikDiscountField's propTypes are missing the disabled prop declaration; update the MuiFormikDiscountField.propTypes object to include disabled: PropTypes.bool so the component's accepted props are accurately declared (keep existing name and label entries intact).src/components/mui/formik-inputs/mui-formik-timepicker.js (1)
65-67: Adddisabledto propTypes for completeness.The
disabledprop is accepted and used but not declared in propTypes.🔧 Proposed fix
MuiFormikTimepicker.propTypes = { - name: PropTypes.string.isRequired + name: PropTypes.string.isRequired, + disabled: PropTypes.bool };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/formik-inputs/mui-formik-timepicker.js` around lines 65 - 67, MuiFormikTimepicker.propTypes currently only declares name; add a PropTypes entry for the optional boolean disabled to the MuiFormikTimepicker.propTypes object so the component's accepted prop is documented and type-checked. Locate the MuiFormikTimepicker.propTypes declaration and add disabled: PropTypes.bool (not required) alongside name to reflect that the component reads and uses the disabled prop.src/components/mui/FormItemTable/index.js (1)
78-81: Consider using nullish coalescing for rate calculation.The
||operator treats0as falsy, so a valid custom rate of $0 would incorrectly fall back to the row rate. Additionally, if both values are undefined, the multiplication would produceNaN. Using??would be more precise.💡 Suggested improvement
const customRate = values[`i-${row.form_item_id}-c-global-f-custom_rate`]; - const rate = customRate || row.rates[currentApplicableRate]; + const rate = customRate ?? row.rates[currentApplicableRate] ?? 0; return qty * rate;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/FormItemTable/index.js` around lines 78 - 81, The calculation uses the || operator which treats 0 as falsy; update the logic so customRate uses nullish coalescing and add a safe fallback to avoid NaN: compute rate as customRate ?? row.rates[currentApplicableRate] ?? 0 (referencing customRate, row.rates and currentApplicableRate) and ensure the multiplier uses a safe qty fallback (e.g., qty ?? 0) before returning the product so a 0 customRate is preserved and undefined values don't yield NaN.
🤖 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/FormItemTable/components/UnderlyingAlertNote.js`:
- Around line 10-14: The Typography in UnderlyingAlertNote.js uses an invalid
MUI palette path "error.warning"; update the sx color to a valid theme key (for
example use "warning.main" or "error.main" depending on intended semantics) so
the color is applied correctly—locate the Typography element in the
UnderlyingAlertNote component and replace "error.warning" with the chosen valid
palette path (e.g., "warning.main").
---
Nitpick comments:
In `@src/components/mui/formik-inputs/mui-formik-discountfield.js`:
- Around line 105-108: MuiFormikDiscountField's propTypes are missing the
disabled prop declaration; update the MuiFormikDiscountField.propTypes object to
include disabled: PropTypes.bool so the component's accepted props are
accurately declared (keep existing name and label entries intact).
In `@src/components/mui/formik-inputs/mui-formik-timepicker.js`:
- Around line 65-67: MuiFormikTimepicker.propTypes currently only declares name;
add a PropTypes entry for the optional boolean disabled to the
MuiFormikTimepicker.propTypes object so the component's accepted prop is
documented and type-checked. Locate the MuiFormikTimepicker.propTypes
declaration and add disabled: PropTypes.bool (not required) alongside name to
reflect that the component reads and uses the disabled prop.
In `@src/components/mui/FormItemTable/index.js`:
- Around line 78-81: The calculation uses the || operator which treats 0 as
falsy; update the logic so customRate uses nullish coalescing and add a safe
fallback to avoid NaN: compute rate as customRate ??
row.rates[currentApplicableRate] ?? 0 (referencing customRate, row.rates and
currentApplicableRate) and ensure the multiplier uses a safe qty fallback (e.g.,
qty ?? 0) before returning the product so a 0 customRate is preserved and
undefined values don't yield NaN.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6e62329-af1b-4de9-8387-a4959fb061e5
📒 Files selected for processing (11)
src/components/mui/FormItemTable/components/GlobalQuantityField.jssrc/components/mui/FormItemTable/components/ItemTableField.jssrc/components/mui/FormItemTable/components/UnderlyingAlertNote.jssrc/components/mui/FormItemTable/helpers.jssrc/components/mui/FormItemTable/index.jssrc/components/mui/formik-inputs/mui-formik-datepicker.jssrc/components/mui/formik-inputs/mui-formik-discountfield.jssrc/components/mui/formik-inputs/mui-formik-timepicker.jssrc/i18n/en.jsonsrc/pages/sponsors/sponsor-cart-tab/components/edit-form/index.jssrc/reducers/sponsors/sponsor-page-cart-list-reducer.js
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/mui/FormItemTable/__tests__/FormItemTable.test.js (1)
483-542: Add an explicit test for theexpiredrate path.This updated suite covers null/mixed rates well, but it doesn’t directly assert the
expiredbranch introduced insrc/components/mui/FormItemTable/index.js(Line 76), where row totals should be forced to$0.00.✅ Suggested test case
+ it("shows $0.00 totals when currentApplicableRate is expired", () => { + render( + <FormItemTableWrapper + data={MOCK_FORM_A.items} + currentApplicableRate="expired" + initialValues={{ + "i-1-c-Form-f-1": 2, + "i-1-c-Form-f-2": 2 + }} + timeZone="America/New_York" + onNotesClick={mockOnNotesClick} + onSettingsClick={mockOnSettingsClick} + /> + ); + + expect(screen.getAllByText("$0.00").length).toBeGreaterThan(0); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/FormItemTable/__tests__/FormItemTable.test.js` around lines 483 - 542, Add a new test in the "N/A Rates" describe that covers the expired branch in the FormItemTable component: render FormItemTableWrapper with currentApplicableRate="expired" (use MOCK_ITEMS_WITH_MIXED_RATES so some items have non-null rates) and assert that row totals and the grand total are forced to "$0.00" (e.g., expect(screen.getAllByText("$0.00").length).toBeGreaterThan(0)) and that previously visible dollar amounts (like "$100.00") are not rendered, ensuring the expired path in FormItemTable/index.js is exercised.
🤖 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/FormItemTable/__tests__/FormItemTable.test.js`:
- Around line 268-310: In the two mocked components (the default mock exports
for the mui-formik-pricefield and mui-formik-discountfield) replace the value
expression that currently uses logical OR (field.value || "") with nullish
coalescing so zero is preserved (field.value ?? ""); update both occurrences
inside the default functional components that render the <input> (look for
data-testid={`pricefield-${name}`} and data-testid={`discountfield-${name}`} to
find the exact spots).
In `@src/components/mui/FormItemTable/index.js`:
- Around line 78-81: The calculation incorrectly treats a customRate of 0 as
falsy because it uses ||; update the expression that sets rate (currently using
customRate || row.rates[currentApplicableRate]) to use nullish coalescing
(customRate ?? row.rates[currentApplicableRate]) so explicit 0 custom rates are
respected; ensure you keep the subsequent null check (if (rate == null || qty ==
null) return 0) unchanged and reference variables customRate, rate, values,
row.rates, currentApplicableRate, and qty when making the change.
In `@src/pages/sponsors/sponsor-cart-tab/components/cart-view.js`:
- Line 208: The disabled check currently uses the exact equality
cart?.net_amount === 0 which misses missing, string, NaN or negative values;
update the disabled prop on the button in cart-view.js so it treats any
non-positive or non-finite amount as disabled (e.g., coerce cart?.net_amount to
a number using parseFloat or Number, guard with Number.isFinite, and disable
when value <= 0 or !Number.isFinite(value)); look for the
disabled={cart?.net_amount === 0} usage and replace it with a robust check such
as computing a netAmount variable from cart.net_amount and using
disabled={!Number.isFinite(netAmount) || netAmount <= 0}.
---
Nitpick comments:
In `@src/components/mui/FormItemTable/__tests__/FormItemTable.test.js`:
- Around line 483-542: Add a new test in the "N/A Rates" describe that covers
the expired branch in the FormItemTable component: render FormItemTableWrapper
with currentApplicableRate="expired" (use MOCK_ITEMS_WITH_MIXED_RATES so some
items have non-null rates) and assert that row totals and the grand total are
forced to "$0.00" (e.g.,
expect(screen.getAllByText("$0.00").length).toBeGreaterThan(0)) and that
previously visible dollar amounts (like "$100.00") are not rendered, ensuring
the expired path in FormItemTable/index.js is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8df452ff-68e9-4317-b986-7ad30ad5d799
📒 Files selected for processing (4)
src/components/mui/FormItemTable/__tests__/FormItemTable.test.jssrc/components/mui/FormItemTable/components/UnderlyingAlertNote.jssrc/components/mui/FormItemTable/index.jssrc/pages/sponsors/sponsor-cart-tab/components/cart-view.js
✅ Files skipped from review due to trivial changes (1)
- src/components/mui/FormItemTable/components/UnderlyingAlertNote.js
Comment on lines
+268
to
+310
| jest.mock("../../formik-inputs/mui-formik-pricefield", () => { | ||
| const { useField } = require("formik"); | ||
| return { | ||
| __esModule: true, | ||
| default: ({ name, label, ...props }) => { | ||
| const [field] = useField(name); | ||
| return ( | ||
| <input | ||
| data-testid={`pricefield-${name}`} | ||
| name={name} | ||
| type="number" | ||
| placeholder={label} | ||
| value={field.value || ""} | ||
| onChange={field.onChange} | ||
| onBlur={field.onBlur} | ||
| {...props} // eslint-disable-line react/jsx-props-no-spreading | ||
| /> | ||
| ); | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| jest.mock("../../formik-inputs/mui-formik-discountfield", () => { | ||
| const { useField } = require("formik"); | ||
| return { | ||
| __esModule: true, | ||
| default: ({ name, label, discountType, ...props }) => { | ||
| const [field] = useField(name); | ||
| return ( | ||
| <input | ||
| data-testid={`discountfield-${name}`} | ||
| name={name} | ||
| type="number" | ||
| placeholder={label} | ||
| value={field.value || ""} | ||
| onChange={field.onChange} | ||
| onBlur={field.onBlur} | ||
| {...props} // eslint-disable-line react/jsx-props-no-spreading | ||
| /> | ||
| ); | ||
| } | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate value fallback patterns in FormItemTable tests:"
rg -nP 'value=\{field\.value\s*\|\|\s*""\}' src/components/mui/FormItemTable/__tests__/FormItemTable.test.jsRepository: fntechgit/summit-admin
Length of output: 240
🏁 Script executed:
cat -n src/components/mui/FormItemTable/__tests__/FormItemTable.test.js | sed -n '160,180p;270,320p'Repository: fntechgit/summit-admin
Length of output: 2378
🏁 Script executed:
rg -n "pricefield|discountfield" src/components/mui/FormItemTable/__tests__/FormItemTable.test.js | head -20Repository: fntechgit/summit-admin
Length of output: 303
🏁 Script executed:
rg -n "pricefield-|discountfield-" src/components/mui/FormItemTable/__tests__/FormItemTable.test.js | head -30Repository: fntechgit/summit-admin
Length of output: 166
🏁 Script executed:
rg -n "initialValues|price|discount" src/components/mui/FormItemTable/__tests__/FormItemTable.test.js | grep -E "(0|zero|initialValues)" | head -20Repository: fntechgit/summit-admin
Length of output: 786
🏁 Script executed:
sed -n '320,330p;710,720p' src/components/mui/FormItemTable/__tests__/FormItemTable.test.jsRepository: fntechgit/summit-admin
Length of output: 618
Use nullish coalescing (??) instead of logical OR (||) to preserve zero values in mock inputs.
Lines 280 and 302 use field.value || "", which converts numeric 0 to "" (empty string). Since tests initialize fields with discount_amount: 0 and other zero values, this pattern causes those valid zero values to be erased, potentially hiding regressions.
Proposed fix
- value={field.value || ""}
+ value={field.value ?? ""}Apply to both lines 280 and 302.
📝 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
| jest.mock("../../formik-inputs/mui-formik-pricefield", () => { | |
| const { useField } = require("formik"); | |
| return { | |
| __esModule: true, | |
| default: ({ name, label, ...props }) => { | |
| const [field] = useField(name); | |
| return ( | |
| <input | |
| data-testid={`pricefield-${name}`} | |
| name={name} | |
| type="number" | |
| placeholder={label} | |
| value={field.value || ""} | |
| onChange={field.onChange} | |
| onBlur={field.onBlur} | |
| {...props} // eslint-disable-line react/jsx-props-no-spreading | |
| /> | |
| ); | |
| } | |
| }; | |
| }); | |
| jest.mock("../../formik-inputs/mui-formik-discountfield", () => { | |
| const { useField } = require("formik"); | |
| return { | |
| __esModule: true, | |
| default: ({ name, label, discountType, ...props }) => { | |
| const [field] = useField(name); | |
| return ( | |
| <input | |
| data-testid={`discountfield-${name}`} | |
| name={name} | |
| type="number" | |
| placeholder={label} | |
| value={field.value || ""} | |
| onChange={field.onChange} | |
| onBlur={field.onBlur} | |
| {...props} // eslint-disable-line react/jsx-props-no-spreading | |
| /> | |
| ); | |
| } | |
| }; | |
| }); | |
| jest.mock("../../formik-inputs/mui-formik-pricefield", () => { | |
| const { useField } = require("formik"); | |
| return { | |
| __esModule: true, | |
| default: ({ name, label, ...props }) => { | |
| const [field] = useField(name); | |
| return ( | |
| <input | |
| data-testid={`pricefield-${name}`} | |
| name={name} | |
| type="number" | |
| placeholder={label} | |
| value={field.value ?? ""} | |
| onChange={field.onChange} | |
| onBlur={field.onBlur} | |
| {...props} // eslint-disable-line react/jsx-props-no-spreading | |
| /> | |
| ); | |
| } | |
| }; | |
| }); | |
| jest.mock("../../formik-inputs/mui-formik-discountfield", () => { | |
| const { useField } = require("formik"); | |
| return { | |
| __esModule: true, | |
| default: ({ name, label, discountType, ...props }) => { | |
| const [field] = useField(name); | |
| return ( | |
| <input | |
| data-testid={`discountfield-${name}`} | |
| name={name} | |
| type="number" | |
| placeholder={label} | |
| value={field.value ?? ""} | |
| onChange={field.onChange} | |
| onBlur={field.onBlur} | |
| {...props} // eslint-disable-line react/jsx-props-no-spreading | |
| /> | |
| ); | |
| } | |
| }; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/FormItemTable/__tests__/FormItemTable.test.js` around
lines 268 - 310, In the two mocked components (the default mock exports for the
mui-formik-pricefield and mui-formik-discountfield) replace the value expression
that currently uses logical OR (field.value || "") with nullish coalescing so
zero is preserved (field.value ?? ""); update both occurrences inside the
default functional components that render the <input> (look for
data-testid={`pricefield-${name}`} and data-testid={`discountfield-${name}`} to
find the exact spots).
Comment on lines
78
to
+81
| const customRate = values[`i-${row.form_item_id}-c-global-f-custom_rate`]; | ||
| const rate = customRate || row.rates[currentApplicableRate]; | ||
|
|
||
| if (rate == null || qty == null) return 0; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect custom-rate fallback in FormItemTable:"
sed -n '68,92p' src/components/mui/FormItemTable/index.js
echo
echo "Inspect price field implementation to verify whether 0 is stored as number/string:"
fd 'mui-formik-pricefield.*' src --exec sed -n '1,220p' {}Repository: fntechgit/summit-admin
Length of output: 2931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for other uses of customRate in the codebase ==="
rg "custom_rate|customRate" src --type js -B 2 -A 2 | head -100
echo
echo "=== Check for validation or constraint on custom_rate ==="
rg "custom_rate" src --type js | grep -i "validat\|required\|constraint\|min\|max" || echo "No validation constraints found"
echo
echo "=== Check FormItemTable context for business logic ==="
sed -n '1,120p' src/components/mui/FormItemTable/index.js | tail -50Repository: fntechgit/summit-admin
Length of output: 9587
Use nullish coalescing to respect explicit 0 custom rates when computing row totals.
Line 79 uses customRate || row.rates[currentApplicableRate], which incorrectly treats numeric 0 as missing and falls back to the default rate. When a user explicitly sets a custom rate of 0 (e.g., for a complimentary sponsorship item), this causes the row total to be calculated with the wrong rate, inflating the amount.
Proposed fix
- const rate = customRate || row.rates[currentApplicableRate];
+ const rate = customRate ?? row.rates?.[currentApplicableRate];📝 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 customRate = values[`i-${row.form_item_id}-c-global-f-custom_rate`]; | |
| const rate = customRate || row.rates[currentApplicableRate]; | |
| if (rate == null || qty == null) return 0; | |
| const customRate = values[`i-${row.form_item_id}-c-global-f-custom_rate`]; | |
| const rate = customRate ?? row.rates?.[currentApplicableRate]; | |
| if (rate == null || qty == null) return 0; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/FormItemTable/index.js` around lines 78 - 81, The
calculation incorrectly treats a customRate of 0 as falsy because it uses ||;
update the expression that sets rate (currently using customRate ||
row.rates[currentApplicableRate]) to use nullish coalescing (customRate ??
row.rates[currentApplicableRate]) so explicit 0 custom rates are respected;
ensure you keep the subsequent null check (if (rate == null || qty == null)
return 0) unchanged and reference variables customRate, rate, values, row.rates,
currentApplicableRate, and qty when making the change.
| variant="contained" | ||
| color="primary" | ||
| style={{ minWidth: 250 }} | ||
| disabled={cart?.net_amount === 0} |
There was a problem hiding this comment.
Harden the disable condition for non-positive/invalid totals.
At Line 208, cart?.net_amount === 0 only handles one exact case. If net_amount is missing, string-typed, NaN, or negative, the button can still be enabled.
Suggested fix
- disabled={cart?.net_amount === 0}
+ disabled={
+ !Number.isFinite(Number(cart?.net_amount)) ||
+ Number(cart?.net_amount) <= 0
+ }📝 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
| disabled={cart?.net_amount === 0} | |
| disabled={ | |
| !Number.isFinite(Number(cart?.net_amount)) || | |
| Number(cart?.net_amount) <= 0 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-cart-tab/components/cart-view.js` at line 208, The
disabled check currently uses the exact equality cart?.net_amount === 0 which
misses missing, string, NaN or negative values; update the disabled prop on the
button in cart-view.js so it treats any non-positive or non-finite amount as
disabled (e.g., coerce cart?.net_amount to a number using parseFloat or Number,
guard with Number.isFinite, and disable when value <= 0 or
!Number.isFinite(value)); look for the disabled={cart?.net_amount === 0} usage
and replace it with a robust check such as computing a netAmount variable from
cart.net_amount and using disabled={!Number.isFinite(netAmount) || netAmount <=
0}.
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
1 participant
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/86b8z5cj5
Summary by CodeRabbit
New Features
Improvements
Tests