fix: migrate payment profiles to MUI components#925
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughConsolidates payment profile and fee-type management into a single list page with a modal dialog, adds search/pagination to Redux, standardizes snackbar-based API notifications, simplifies routing, and adds tests, i18n updates, and small constants. ChangesPayment Profile Dialog-Based Management
Sequence DiagramsequenceDiagram
participant User
participant ListPage
participant Dialog
participant Redux
participant API
participant Snack as Snackbar
User->>ListPage: search / paginate / open edit
ListPage->>Redux: dispatch getPaymentProfiles / getPaymentProfile / getPaymentFeeTypes
Redux->>API: http requests
API-->>Redux: responses (with pagination)
Redux->>ListPage: update state
ListPage->>Dialog: open with entity & feeTypes
Dialog->>Redux: dispatch savePaymentProfile / savePaymentFeeType
Redux->>API: save request
API-->>Redux: success / error
Redux->>Snack: snackbarSuccessHandler / snackbarErrorHandler
Snack->>User: show notification
Redux->>ListPage: refresh list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/reducers/payment_profiles/payment-profile-list-reducer.js (1)
43-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist the requested
termandperPagein list state.Right now only
order/orderDirare stored here. After a search or rows-per-page change, Redux still holds the previousterm/perPage, so subsequent paging/sorting re-queries with stale values and drops the active filter/page size.Suggested fix
case REQUEST_PAYMENT_PROFILES: { - const { order, orderDir } = payload; - return { ...state, order, orderDir }; + const { term, page, perPage, order, orderDir } = payload; + return { + ...state, + term, + currentPage: page, + perPage, + order, + orderDir + }; }🤖 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/reducers/payment_profiles/payment-profile-list-reducer.js` around lines 43 - 45, The REQUEST_PAYMENT_PROFILES case in payment-profile-list-reducer.js currently only persists order and orderDir; update it to also read term and perPage from the action payload and include them in the returned state (i.e., return { ...state, order, orderDir, term, perPage }) so searches and rows-per-page changes are stored on the list state; modify the REQUEST_PAYMENT_PROFILES branch where order/orderDir are extracted to also extract term and perPage from payload and add them to the new state object.
🤖 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/pages/tickets/payment-profile/components/payment-profile-dialog.js`:
- Around line 315-354: The live_secret_key and test_secret_key fields are
rendered as plain text; change their MuiFormikTextField usage to mask values by
setting type="password" (for example add type="password" prop on the
MuiFormikTextField for name="live_secret_key" and name="test_secret_key") and
add a toggleable visibility affordance using an endAdornment
(IconButton/visibility icons) so admins can reveal values when needed; keep
existing props (required/fullWidth/variant) and ensure the visibility toggle
updates the input type and includes accessible labels.
- Around line 213-220: Remove the three focus-disabling props from the Dialog
(disableEnforceFocus, disableAutoFocus, disableRestoreFocus) so the modal
regains its focus trap and keyboard/screen-reader navigation; keep the Dialog
open/onClose handling (Dialog, handleClose) as-is. If a specific input or
control inside the dialog is causing focus issues, revert the global changes and
apply a targeted workaround to that control only (for example, render
problematic popovers/menus in a portal, use the control's built-in
disableAutoFocus or disableEnforceFocus flags, or adjust its focus handling)
rather than disabling focus management on the entire Dialog.
In `@src/pages/tickets/payment-profile/payment-profile-list-page.js`:
- Around line 100-104: The inline fee-type save handler handleSaveFeeType should
refresh the fee list but must not close the parent PaymentProfileDialog; remove
the setPaymentProfilePopup(false) call from handleSaveFeeType and keep calling
getPaymentFeeTypes(currentPaymentProfile.id) after savePaymentFeeType(entity)
resolves so the dialog stays open and shows refreshed data, and ensure you do
not trigger any additional unmount/reset logic that conflicts with
PaymentProfileDialog’s own then path.
---
Outside diff comments:
In `@src/reducers/payment_profiles/payment-profile-list-reducer.js`:
- Around line 43-45: The REQUEST_PAYMENT_PROFILES case in
payment-profile-list-reducer.js currently only persists order and orderDir;
update it to also read term and perPage from the action payload and include them
in the returned state (i.e., return { ...state, order, orderDir, term, perPage
}) so searches and rows-per-page changes are stored on the list state; modify
the REQUEST_PAYMENT_PROFILES branch where order/orderDir are extracted to also
extract term and perPage from payload and add them to the new state object.
🪄 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: 2529cc2e-39a0-40a8-a92f-524919ed0f65
📒 Files selected for processing (9)
src/actions/ticket-actions.jssrc/i18n/en.jsonsrc/layouts/payment-profile-layout.jssrc/pages/tickets/edit-payment-fee-type-page.jssrc/pages/tickets/edit-payment-profile-page.jssrc/pages/tickets/payment-profile-list-page.jssrc/pages/tickets/payment-profile/components/payment-profile-dialog.jssrc/pages/tickets/payment-profile/payment-profile-list-page.jssrc/reducers/payment_profiles/payment-profile-list-reducer.js
💤 Files with no reviewable changes (3)
- src/pages/tickets/edit-payment-fee-type-page.js
- src/pages/tickets/edit-payment-profile-page.js
- src/pages/tickets/payment-profile-list-page.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/tickets/payment-profile/components/payment-profile-dialog.js (1)
588-600:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix PropTypes mismatch:
paymentFeeTypesis an object, not an array.The
paymentFeeTypesprop is passed as an object fromcurrentPaymentFeeListTypeState(defined insrc/reducers/payment_profiles/payment-fee-types-list-reducer.js) with propertiesorder,orderDir,totalPaymentFeeTypes, andpaymentFeeTypes. It is currently typed asPropTypes.array, which fails to validate the actual object structure. Additionally, no default value is provided, risking undefined property access.Suggested fix
PaymentProfileDialog.propTypes = { onClose: PropTypes.func.isRequired, onSave: PropTypes.func.isRequired, entity: PropTypes.object, - paymentFeeTypes: PropTypes.array, + paymentFeeTypes: PropTypes.shape({ + order: PropTypes.string, + orderDir: PropTypes.number, + totalPaymentFeeTypes: PropTypes.number, + paymentFeeTypes: PropTypes.arrayOf(PropTypes.object) + }), isSaving: PropTypes.bool, onSaveFeeType: PropTypes.func.isRequired, onDeleteFeeType: PropTypes.func.isRequired }; PaymentProfileDialog.defaultProps = { - entity: {} + entity: {}, + paymentFeeTypes: { + order: "id", + orderDir: 1, + totalPaymentFeeTypes: 0, + paymentFeeTypes: [] + } };🤖 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/pages/tickets/payment-profile/components/payment-profile-dialog.js` around lines 588 - 600, PaymentProfileDialog's propTypes incorrectly declare paymentFeeTypes as PropTypes.array while the component receives an object (from currentPaymentFeeListTypeState) with keys order, orderDir, totalPaymentFeeTypes, and paymentFeeTypes; update PaymentProfileDialog.propTypes to use PropTypes.shape({ order: PropTypes.number, orderDir: PropTypes.string, totalPaymentFeeTypes: PropTypes.number, paymentFeeTypes: PropTypes.array }) (mark fields required as appropriate) and add a matching default in PaymentProfileDialog.defaultProps (e.g., an empty shape with sensible defaults) to avoid undefined access.
🤖 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.
Outside diff comments:
In `@src/pages/tickets/payment-profile/components/payment-profile-dialog.js`:
- Around line 588-600: PaymentProfileDialog's propTypes incorrectly declare
paymentFeeTypes as PropTypes.array while the component receives an object (from
currentPaymentFeeListTypeState) with keys order, orderDir, totalPaymentFeeTypes,
and paymentFeeTypes; update PaymentProfileDialog.propTypes to use
PropTypes.shape({ order: PropTypes.number, orderDir: PropTypes.string,
totalPaymentFeeTypes: PropTypes.number, paymentFeeTypes: PropTypes.array })
(mark fields required as appropriate) and add a matching default in
PaymentProfileDialog.defaultProps (e.g., an empty shape with sensible defaults)
to avoid undefined access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2536394-67a8-4c22-af80-ae594c1a9dae
📒 Files selected for processing (2)
src/pages/tickets/payment-profile/components/payment-profile-dialog.jssrc/pages/tickets/payment-profile/payment-profile-list-page.js
|
@tomrndom there are a couple of ux issues at fee types ( pre existents) |
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
Deep review — 8 findings (2 HIGH, 5 MEDIUM, 1 LOW) posted inline. Verdict: needs changes. Highlights: validation schema is commented out (HIGH); saving a fee type closes the entire profile dialog (HIGH); tests for the new dialog/list page and the new term filter are missing.
| test_secret_key: initialEntity?.test_secret_key || "", | ||
| test_publishable_key: initialEntity?.test_publishable_key || "" | ||
| }, | ||
| // validationSchema: yup.object().shape({ |
There was a problem hiding this comment.
🔴 [HIGH] Validation schema is commented out on the main profile form
Users can save a profile with empty application_type, provider, and (for Stripe) empty live keys — onSubmit runs unconditionally and the previous form's required-field guards are silently regressed.
Fix: Re-enable this Yup schema (or remove the commented block and add the live one) before merging.
| savePaymentFeeType(entity).then(() => { | ||
| getPaymentFeeTypes(currentPaymentProfile.id); | ||
| setPaymentProfilePopup(false); | ||
| }); |
There was a problem hiding this comment.
🔴 [HIGH] Saving a fee type closes the entire payment-profile dialog
setPaymentProfilePopup(false) here unmounts the parent dialog after every fee-type save, dropping any unsaved profile edits and forcing the user to re-open the profile.
Fix: After a fee-type save, only refresh getPaymentFeeTypes(currentPaymentProfile.id) and reset the inner fee-type form. Do NOT close the profile dialog — mirror handleDeleteFeeType below, which is correct.
Also: there is no .catch here, so the popup-dialog rule (.claude/rules/summit-admin-popup-dialog-pattern.md) — keep dialog open on error — is not honoured.
| setIsSaving(true); | ||
| savePaymentProfile(entity) | ||
| .then(() => { | ||
| getPaymentProfiles("", DEFAULT_CURRENT_PAGE, perPage); |
There was a problem hiding this comment.
🟡 [MEDIUM] Active search term is silently dropped on every refresh
handleSave (here), handleDelete (line 82), and the mount useEffect (line 56) all call getPaymentProfiles("", DEFAULT_CURRENT_PAGE, perPage) with an empty term. After saving or deleting a profile while a search filter is active, results jump back to the unfiltered first page with no UI cue and the SearchInput desyncs from the data.
Fix: Pass the persisted term (and order / orderDir) — or extract a refreshList() helper that reads them from props.
Also: handleSave has no .catch. Per .claude/rules/summit-admin-popup-dialog-pattern.md the dialog should stay open on error to preserve user input.
| const handleDeleteFeeType = (feeTypeId) => | ||
| deletePaymentFeeType(feeTypeId).then(() => { | ||
| getPaymentFeeTypes(currentPaymentProfile.id); | ||
| }); |
There was a problem hiding this comment.
🟡 [MEDIUM] handleDeleteFeeType and handleSaveFeeType swallow failures
.then(...) with no .catch — if the underlying request rejects, the success branch (refresh, and for save: close dialog) still chains, leaving the UI inconsistent with the server. snackbarErrorHandler will show a toast but the refresh and any state cleanup still runs.
Fix: Add an explicit .catch(() => { /* no-op */ }), and only refresh / close on the success path.
| @@ -41,13 +45,19 @@ const paymentProfileListReducer = (state = DEFAULT_STATE, action) => { | |||
| return { ...state, order, orderDir }; | |||
| } | |||
There was a problem hiding this comment.
🟡 [MEDIUM] Reducer drops term, currentPage, and perPage from REQUEST_PAYMENT_PROFILES payload
The action now dispatches { term, page, perPage, order, orderDir }, but this case only reads { order, orderDir }. As a result term is never written to redux — the term value spread by mapStateToProps stays "" and any consumer relying on it is stale. This also breaks the list-state contract documented in .claude/rules/summit-admin-redux-actions.md.
Fix:
case REQUEST_PAYMENT_PROFILES: {
const { term, currentPage, perPage, order, orderDir } = payload;
return { ...state, term, currentPage, perPage, order, orderDir };
}| component={EditPaymentFeeTypePage} | ||
| /> | ||
|
|
||
| <Route component={NoMatchPage} /> |
There was a problem hiding this comment.
🟡 [MEDIUM] Deleted routes silently 404 bookmarks and deep links
/payment-profiles/new, /payment-profiles/:id, and /payment-profiles/:id/payment-fee-type/... are removed with no redirect. Any bookmarked URL, emailed link, or in-app history.push to those paths will now hit NoMatchPage.
Fix: Either add a Redirect for legacy paths (/:id → / and optionally open the dialog via a query param), or grep the codebase for any residual history.push(".../payment-profiles/${id}") callers and update them.
| if (term) { | ||
| const escapedTerm = escapeFilterValue(term); | ||
| filter.push( | ||
| `provider=@${escapedTerm},id=@${escapedTerm},application_type=@${escapedTerm}` |
There was a problem hiding this comment.
🔵 [LOW] Search filter uses =@ (contains) on enum & id fields
provider=@x,id=@x,application_type=@x does a substring match on enum-like fields (Registration, BookableRooms, SponsorServices) and on a numeric id — typing 1 matches ids 1, 10, 11, 12…, and Sponsor matches SponsorServices even when the user meant something narrower.
Fix: Use == for id and application_type; keep =@ only on provider if free-text matching is actually intended.
| @@ -1186,6 +1187,7 @@ export const deleteRefundPolicy = | |||
|
|
|||
| export const getPaymentProfiles = | |||
There was a problem hiding this comment.
🟡 [MEDIUM] Missing test coverage for the new behavior
No __tests__ updates ship with this PR for any of the new surfaces. The following should be covered before merge:
getPaymentProfileswithterm— URL/params construction with non-emptyterm(verifyescapeFilterValue, thefilter[]array shape, and the newREQUEST_PAYMENT_PROFILESpayload).- Reducer
REQUEST_PAYMENT_PROFILES— asserts thatterm,currentPage,perPageland in state (catches the bug flagged inpayment-profile-list-reducer.js#L46). PaymentProfileDialogflow —- parent stays open after a fee-type save (catches the bug flagged in
payment-profile-list-page.js#L110), - dialog stays open if save rejects (
.catchbehaviour), - validation prevents submit when required fields are empty (once the schema is re-enabled).
- parent stays open after a fee-type save (catches the bug flagged in
This is a UX-critical surface (payment configuration). At minimum the action + reducer should be covered.
santipalenque
left a comment
santipalenque
left a comment
There was a problem hiding this comment.
no further flags other than seba's
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/tickets/payment-profile/components/payment-profile-dialog.js (1)
217-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
max_cents/min_centsare edited as price but displayed as raw cents in the table.You converted form inputs to price fields (
inCents), but the table still renders raw stored integers formax_centsandmin_cents. This leaves a unit mismatch in the UI.Suggested fix
{ columnKey: "max_cents", - header: T.translate("edit_payment_profile.payment_type_fee_max_cents") + header: T.translate("edit_payment_profile.payment_type_fee_max_cents"), + render: (row) => currencyAmountFromCents(row.max_cents || 0) }, { columnKey: "min_cents", - header: T.translate("edit_payment_profile.payment_type_fee_min_cents") + header: T.translate("edit_payment_profile.payment_type_fee_min_cents"), + render: (row) => currencyAmountFromCents(row.min_cents || 0) }Also applies to: 584-604
🤖 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/pages/tickets/payment-profile/components/payment-profile-dialog.js` around lines 217 - 223, Table columns for "max_cents" and "min_cents" show raw integer cents while the form and inputs use price units; update the table column renderers for columnKey "max_cents" and "min_cents" to convert cents to a human price string (e.g., divide by 100 and format with the app's currency formatter or existing helper used elsewhere—look for any formatPrice/formatCurrency or inCents utilities) so the table displays the same price units as the form; ensure the conversion is applied both where these columns are defined and in the other occurrence noted (lines ~584-604) so all table displays match the edited price inputs.
🧹 Nitpick comments (1)
src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js (1)
318-325: ⚡ Quick winExtend the formatting test to assert
max_cents/min_centsdisplay conversion too.This test already guards Rate/Amount
valueformatting; adding max/min assertions will catch cents-vs-USD regressions in the same path.Suggested test extension
const amountFeeType = { id: 2, name: "Flat Fee", kind: "Amount", payment_method: "card", value: 1500, // $15.00 — stored in cents - max_cents: 0, - min_cents: 0 + max_cents: 2500, // $25.00 + min_cents: 500 // $5.00 }; @@ test("formats Rate value as a percentage and Amount value as money", () => { renderDialog({ entity: feeTypeSectionEntity, paymentFeeTypes }); // rateFeeType row 0: value = 250 → 250/100 = 2.5% expect(screen.getByTestId("cell-value-0")).toHaveTextContent("2.5%"); // amountFeeType row 1: value = 1500 cents → $15.00 expect(screen.getByTestId("cell-value-1")).toHaveTextContent("$15.00"); + expect(screen.getByTestId("cell-max_cents-1")).toHaveTextContent("$25.00"); + expect(screen.getByTestId("cell-min_cents-1")).toHaveTextContent("$5.00"); });🤖 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/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js` around lines 318 - 325, Update the "formats Rate value as a percentage and Amount value as money" test (the test block that calls renderDialog with feeTypeSectionEntity and paymentFeeTypes) to also assert the formatted display for the min_cents and max_cents cells: add expectations using screen.getByTestId for "cell-min_cents-0"/"cell-max_cents-0" (rateFeeType row 0) and "cell-min_cents-1"/"cell-max_cents-1" (amountFeeType row 1); for the Rate row assert min/max are shown as percentages (value/100 with a "%" suffix) and for the Amount row assert min/max are shown as USD money strings (cents → dollars with two decimals, e.g. 1500 → "$15.00"). Ensure these new assertions follow the existing pattern in the test that checks "cell-value-0" and "cell-value-1".
🤖 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/pages/tickets/payment-profile/components/payment-profile-dialog.js`:
- Around line 163-170: The max_cents/min_cents validation is capping cent values
at 99 which conflicts with MuiFormikPriceField's inCents usage; update the
decimalValidation() calls for max_cents and min_cents to either remove the
.max(...) cap or replace MAX_CENTS with the actual backend-aligned cents limit
(e.g., BACKEND_MAX_CENTS), and adjust the T.translate message to show the
correct unit (use cents or convert MAX_CENTS to dollars if the translation
expects dollars); locate the validation block containing max_cents, min_cents,
decimalValidation(), and MAX_CENTS and modify it accordingly so cent inputs are
not incorrectly blocked.
- Around line 172-177: The onSubmit handler currently calls
onSaveFeeType(values).then(...) but doesn’t return the resulting promise,
preventing Formik from tracking async submission; update the onSubmit in the fee
type Formik config to return the promise from onSaveFeeType(values).then(...),
keeping the existing then callback (feeTypeFormik.resetForm();
setShowFeeTypeForm(false)); this ensures Formik’s isSubmitting lifecycle is
properly managed when onSaveFeeType/handleSaveFeeType/savePaymentFeeType resolve
or reject.
---
Outside diff comments:
In `@src/pages/tickets/payment-profile/components/payment-profile-dialog.js`:
- Around line 217-223: Table columns for "max_cents" and "min_cents" show raw
integer cents while the form and inputs use price units; update the table column
renderers for columnKey "max_cents" and "min_cents" to convert cents to a human
price string (e.g., divide by 100 and format with the app's currency formatter
or existing helper used elsewhere—look for any formatPrice/formatCurrency or
inCents utilities) so the table displays the same price units as the form;
ensure the conversion is applied both where these columns are defined and in the
other occurrence noted (lines ~584-604) so all table displays match the edited
price inputs.
---
Nitpick comments:
In
`@src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js`:
- Around line 318-325: Update the "formats Rate value as a percentage and Amount
value as money" test (the test block that calls renderDialog with
feeTypeSectionEntity and paymentFeeTypes) to also assert the formatted display
for the min_cents and max_cents cells: add expectations using screen.getByTestId
for "cell-min_cents-0"/"cell-max_cents-0" (rateFeeType row 0) and
"cell-min_cents-1"/"cell-max_cents-1" (amountFeeType row 1); for the Rate row
assert min/max are shown as percentages (value/100 with a "%" suffix) and for
the Amount row assert min/max are shown as USD money strings (cents → dollars
with two decimals, e.g. 1500 → "$15.00"). Ensure these new assertions follow the
existing pattern in the test that checks "cell-value-0" and "cell-value-1".
🪄 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: b38ccc6b-74da-4c4e-9e38-7559d82e59ad
📒 Files selected for processing (8)
src/actions/__tests__/payment-profile-actions.test.jssrc/pages/tickets/payment-profile/__tests__/payment-profile-list-page.test.jssrc/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.jssrc/pages/tickets/payment-profile/components/payment-profile-dialog.jssrc/pages/tickets/payment-profile/payment-profile-list-page.jssrc/reducers/payment_profiles/__tests__/payment-profile-list-reducer.test.jssrc/reducers/payment_profiles/payment-profile-list-reducer.jssrc/utils/constants.js
✅ Files skipped from review due to trivial changes (1)
- src/utils/constants.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/reducers/payment_profiles/payment-profile-list-reducer.js
- src/pages/tickets/payment-profile/payment-profile-list-page.js
| max_cents: decimalValidation().max( | ||
| MAX_CENTS, | ||
| T.translate("validation.maximum", { maximum: 99 }) | ||
| ), | ||
| min_cents: decimalValidation().max( | ||
| MAX_CENTS, | ||
| T.translate("validation.maximum", { maximum: 99 }) | ||
| ) |
There was a problem hiding this comment.
max_cents/min_cents are capped with the wrong unit scale.
These fields are now edited via MuiFormikPriceField with inCents, but validation caps them at 99, which effectively blocks many valid cent amounts. Either remove this cap or replace it with a backend-aligned cents limit.
Suggested fix
- max_cents: decimalValidation().max(
- MAX_CENTS,
- T.translate("validation.maximum", { maximum: 99 })
- ),
- min_cents: decimalValidation().max(
- MAX_CENTS,
- T.translate("validation.maximum", { maximum: 99 })
- )
+ max_cents: decimalValidation(),
+ min_cents: decimalValidation()📝 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.
| max_cents: decimalValidation().max( | |
| MAX_CENTS, | |
| T.translate("validation.maximum", { maximum: 99 }) | |
| ), | |
| min_cents: decimalValidation().max( | |
| MAX_CENTS, | |
| T.translate("validation.maximum", { maximum: 99 }) | |
| ) | |
| max_cents: decimalValidation(), | |
| min_cents: decimalValidation() |
🤖 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/pages/tickets/payment-profile/components/payment-profile-dialog.js`
around lines 163 - 170, The max_cents/min_cents validation is capping cent
values at 99 which conflicts with MuiFormikPriceField's inCents usage; update
the decimalValidation() calls for max_cents and min_cents to either remove the
.max(...) cap or replace MAX_CENTS with the actual backend-aligned cents limit
(e.g., BACKEND_MAX_CENTS), and adjust the T.translate message to show the
correct unit (use cents or convert MAX_CENTS to dollars if the translation
expects dollars); locate the validation block containing max_cents, min_cents,
decimalValidation(), and MAX_CENTS and modify it accordingly so cent inputs are
not incorrectly blocked.
| onSubmit: (values) => { | ||
| onSaveFeeType(values).then(() => { | ||
| feeTypeFormik.resetForm(); | ||
| setShowFeeTypeForm(false); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify feeTypeFormik onSubmit currently does not return the promise chain
cat -n src/pages/tickets/payment-profile/components/payment-profile-dialog.js | sed -n '168,182p'Repository: fntechgit/summit-admin
Length of output: 495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect full feeTypeFormik initialization area =="
sed -n '130,220p' src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Locate onSaveFeeType references in this file =="
rg -n "onSaveFeeType" src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Locate feeTypeFormik error handling (catch/then) in this file =="
rg -n "feeTypeFormik|onSubmit|catch\\(|validate|setErrors|setStatus" src/pages/tickets/payment-profile/components/payment-profile-dialog.jsRepository: fntechgit/summit-admin
Length of output: 4046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find feeTypeFormik isSubmitting usage =="
rg -n "feeTypeFormik\\.isSubmitting|isSubmitting" src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Inspect fee-type submit button area (handleSubmit) =="
sed -n '600,720p' src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Inspect onSaveFeeType prop / how it's defined =="
sed -n '40,140p' src/pages/tickets/payment-profile/components/payment-profile-dialog.jsRepository: fntechgit/summit-admin
Length of output: 110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find how onSaveFeeType is passed in other files =="
rg -n "onSaveFeeType=" -S .
echo
rg -n "onSaveFeeType:" -S .
echo
rg -n "onSaveFeeType\\(" -S srcRepository: fntechgit/summit-admin
Length of output: 743
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect handleSaveFeeType implementation in payment-profile-list-page.js =="
sed -n '180,320p' src/pages/tickets/payment-profile/payment-profile-list-page.js
echo
echo "== Inspect dialog unit test mocking onSaveFeeType =="
sed -n '150,260p' src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js
echo
echo "== Inspect test assertions around Formik async submit/reset =="
rg -n "resetForm|handleSubmit|isSubmitting|onSubmit|onSaveFeeType" src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js -nRepository: fntechgit/summit-admin
Length of output: 6029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate handleSaveFeeType in payment-profile-list-page.js =="
rg -n "handleSaveFeeType" src/pages/tickets/payment-profile/payment-profile-list-page.js
echo
echo "== Print handleSaveFeeType definition =="
python3 - <<'PY'
import re, pathlib
path = pathlib.Path("src/pages/tickets/payment-profile/payment-profile-list-page.js")
txt = path.read_text()
m = re.search(r'const\s+handleSaveFeeType\s*=\s*async\s*\([^)]*\)\s*=>\s*\{', txt)
if not m:
m = re.search(r'const\s+handleSaveFeeType\s*=\s*\([^)]*\)\s*=>\s*\{', txt)
print("match at", m.start() if m else None)
PY
sed -n '1,260p' src/pages/tickets/payment-profile/payment-profile-list-page.js | rg -n "handleSaveFeeType|savePaymentFeeType|return|catch|try" -n
echo
echo "== Locate save_fee_type button and its disabled/loading logic in dialog file =="
rg -n "save_fee_type|Save Fee|save fee type|handleSubmit" src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Inspect fee type form button area around save_fee_type =="
sed -n '420,700p' src/pages/tickets/payment-profile/components/payment-profile-dialog.jsRepository: fntechgit/summit-admin
Length of output: 12119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect handleSaveFeeType implementation lines around 100-140 =="
sed -n '90,150p' src/pages/tickets/payment-profile/payment-profile-list-page.js
echo
echo "== Search for useScrollToError usage for feeTypeFormik =="
rg -n "useScrollToError\\(([^)]*feeTypeFormik|feeTypeFormik" src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Search for setErrors/setStatus/catch in feeTypeFormik flow =="
rg -n "catch\\(|setErrors\\(|setStatus\\(|onSaveFeeType\\(" src/pages/tickets/payment-profile/components/payment-profile-dialog.jsRepository: fntechgit/summit-admin
Length of output: 1945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for useScrollToError usage of feeTypeFormik =="
rg -n "useScrollToError\\(.*feeTypeFormik|feeTypeFormik.*useScrollToError" src/pages/tickets/payment-profile/components/payment-profile-dialog.js
echo
echo "== Inspect fee-type tests in payment-profile-dialog.test.js =="
rg -n "Fee Type|fee type|save_fee_type|onSaveFeeType|resetForm|setShowFeeTypeForm" src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.jsRepository: fntechgit/summit-admin
Length of output: 123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js
echo
echo "----"
sed -n '260,520p' src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.jsRepository: fntechgit/summit-admin
Length of output: 12129
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find savePaymentFeeType implementation =="
rg -n "savePaymentFeeType" src/pages/tickets/payment-profile -S
echo
echo "== Find savePaymentFeeType in store/actions =="
rg -n "savePaymentFeeType" src -S
echo
echo "== Inspect savePaymentFeeType definition (file + promise behavior) =="
# Grab the first few matching definitions with context
rg -n "const savePaymentFeeType|function savePaymentFeeType|savePaymentFeeType\s*=\s*\(" src -S | head -n 20Repository: fntechgit/summit-admin
Length of output: 1978
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1320,1415p' src/actions/ticket-actions.jsRepository: fntechgit/summit-admin
Length of output: 2819
src/pages/tickets/payment-profile/components/payment-profile-dialog.js: Return the fee-type save promise from onSubmit.
onSubmit starts onSaveFeeType(values).then(...) but doesn’t return that promise, so Formik can’t await it / manage the async submit lifecycle (isSubmitting). Since handleSaveFeeType/savePaymentFeeType return promises, return the chain from onSubmit.
Suggested fix
- onSubmit: (values) => {
- onSaveFeeType(values).then(() => {
- feeTypeFormik.resetForm();
- setShowFeeTypeForm(false);
- });
- }
+ onSubmit: (values) =>
+ onSaveFeeType(values).then(() => {
+ feeTypeFormik.resetForm();
+ setShowFeeTypeForm(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/pages/tickets/payment-profile/components/payment-profile-dialog.js`
around lines 172 - 177, The onSubmit handler currently calls
onSaveFeeType(values).then(...) but doesn’t return the resulting promise,
preventing Formik from tracking async submission; update the onSubmit in the fee
type Formik config to return the promise from onSaveFeeType(values).then(...),
keeping the existing then callback (feeTypeFormik.resetForm();
setShowFeeTypeForm(false)); this ensures Formik’s isSubmitting lifecycle is
properly managed when onSaveFeeType/handleSaveFeeType/savePaymentFeeType resolve
or reject.
3 participants
ref: https://app.clickup.com/t/86b9tgt19
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Improvements
Removed
Tests