Skip to content

fix: migrate payment profiles to MUI components#925

Open
tomrndom wants to merge 8 commits into
masterfrom
fix/payment-profile-mui
Open

fix: migrate payment profiles to MUI components#925
tomrndom wants to merge 8 commits into
masterfrom
fix/payment-profile-mui

Conversation

@tomrndom

@tomrndom tomrndom commented May 8, 2026

Copy link
Copy Markdown

ref: https://app.clickup.com/t/86b9tgt19

image image image image

Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com

Summary by CodeRabbit

  • New Features

    • Added a Ticket payment profile list page with a create/edit modal to manage profile details and associated fee types, including search, sorting, and full pagination.
  • Improvements

    • Payment operations now show consistent snackbar success/error notifications.
    • Updated payment profile UI text: improved delete confirmation (includes profile name), added search placeholder, and clarified fee fields (“Max Amount/Min Amount”).
  • Removed

    • Legacy standalone routes/pages for listing and editing payment profiles and fee types (replaced by the consolidated list + modal flow).
  • Tests

    • Added/expanded unit tests for actions, reducer behavior, and dialog/list interactions.

Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Consolidates 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, removes legacy edit pages, simplifies routing, and adds tests, i18n updates, and constants.

Changes

Payment Profile Dialog-Based Management

Layer / File(s) Summary
Payment profile action updates
src/actions/ticket-actions.js
Imports snackbar handlers; getPaymentProfiles adds term parameter and backend filter; payment actions switch from authErrorHandler to snackbarErrorHandler; savePaymentFeeType uses snackbar success on both create/update with .finally() for cleanup.
Reducer pagination state
src/reducers/payment_profiles/payment-profile-list-reducer.js
Adds term, currentPage, lastPage, perPage to state; REQUEST_PAYMENT_PROFILES stores search/pagination params; RECEIVE_PAYMENT_PROFILES reads pagination metadata from API responses.
Unified payment profile dialog
src/pages/tickets/payment-profile/components/payment-profile-dialog.js
MUI Dialog with two Formik forms: profile form (application type/provider/active/test flags/provider-specific fields) and fee-type form (kind/value/min/max/payment method). Fee-type editor only renders when editing existing Stripe SponsorServices profiles. Kind switching alters value input mode (percentage vs cents).
Consolidated list page
src/pages/tickets/payment-profile/payment-profile-list-page.js
Connected page combining listing, search, pagination, sort, and CRUD. Opens PaymentProfileDialog for edit/create, persists via action creators, reloads list and fee types after changes. Fetches initial list on currentSummit.id availability.
Layout simplification and page removal
src/layouts/payment-profile-layout.js, src/pages/tickets/edit-payment-profile-page.js, src/pages/tickets/edit-payment-fee-type-page.js, src/pages/tickets/payment-profile-list-page.js (old)
Layout now routes only to list page with NoMatchPage fallback. Removed legacy standalone edit pages and old list page; functionality migrated to new list + dialog architecture.
Action creator tests
src/actions/__tests__/payment-profile-actions.test.js
Tests for getPaymentProfiles validate dispatch order, summit URL construction, filter expression escaping, extraPayload propagation with term/page/perPage/order/orderDir, and order parameter +/- prefixing.
Dialog and list page tests
src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js, src/pages/tickets/payment-profile/__tests__/payment-profile-list-page.test.js
Dialog tests cover profile form validation, provider-specific field rendering, fee-type table/deletion, kind switching with value mode changes, and kind-dependent constraints. List page tests validate dialog popup visibility after save outcomes.
Reducer tests
src/reducers/payment_profiles/__tests__/payment-profile-list-reducer.test.js
Tests for REQUEST_PAYMENT_PROFILES and RECEIVE_PAYMENT_PROFILES handling of search/pagination parameters and list updates.
Localization and constants
src/i18n/en.json, src/utils/constants.js
i18n adds placeholders.search_profiles, updates delete confirmation to reference #{name}, adds payment_type_fees labels. Constants exports TEN_THOUSAND (10000) and MAX_CENTS (99).

Sequence Diagram

sequenceDiagram
  participant User
  participant ListPage as Payment Profile<br/>List Page
  participant Dialog
  participant Redux
  participant API as Purchases API
  participant Snack as Snackbar
  User->>ListPage: load/search/paginate/click edit
  ListPage->>Redux: dispatch getPaymentProfiles(term,page,perPage,order,orderDir)
  Redux->>API: GET /summits/{id}/payment-profiles?filter[]=...&page=&per_page=
  API-->>Redux: response with data and pagination metadata
  Redux-->>ListPage: REQUEST + RECEIVE actions with list and params
  ListPage->>User: render table with profiles
  User->>ListPage: click edit or add payment profile
  ListPage->>Redux: dispatch getPaymentProfile(id) or getPaymentFeeTypes(profile_id)
  Redux->>API: fetch entity details
  API-->>Redux: return entity
  Redux-->>ListPage: update currentPaymentProfile or paymentFeeTypes
  ListPage->>Dialog: open(entity, paymentFeeTypes, isSaving)
  Dialog->>User: show profile and fee-type forms
  User->>Dialog: submit profile form
  Dialog->>Redux: dispatch savePaymentProfile(entity)
  Redux->>API: POST/PUT /summits/{id}/payment-profiles
  alt success
    API-->>Redux: success response
    Redux->>Snack: snackbarSuccessHandler
    Redux-->>ListPage: RECEIVE and close dialog
  else error
    API-->>Redux: error response
    Redux->>Snack: snackbarErrorHandler
  end
  Snack->>User: show notification
  User->>Dialog: manage fee types (add/edit/delete)
  Dialog->>Redux: dispatch savePaymentFeeType or deletePaymentFeeType
  Redux->>API: save/delete fee type
  API-->>Redux: success or error
  Redux->>Snack: snackbar notification
  Redux-->>Dialog: reload paymentFeeTypes
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Suggested reviewers

  • smarcet

Poem

A rabbit hops through form and state,
One dialog makes profiles great,
Search and pages dance as one,
Snackbars chirp when saves are done,
Fee types twirl in modal light—hooray! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: migrate payment profiles to MUI components' directly and accurately summarizes the main objective of this PR: migrating the payment profiles feature from its previous implementation to Material-UI components.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/payment-profile-mui

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Persist the requested term and perPage in list state.

Right now only order/orderDir are stored here. After a search or rows-per-page change, Redux still holds the previous term/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

📥 Commits

Reviewing files that changed from the base of the PR and between bda55af and 1aad0b6.

📒 Files selected for processing (9)
  • src/actions/ticket-actions.js
  • src/i18n/en.json
  • src/layouts/payment-profile-layout.js
  • 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
  • src/pages/tickets/payment-profile/components/payment-profile-dialog.js
  • src/pages/tickets/payment-profile/payment-profile-list-page.js
  • src/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

Comment thread src/pages/tickets/payment-profile/payment-profile-list-page.js
@tomrndom tomrndom requested a review from smarcet May 11, 2026 15:53
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Fix PropTypes mismatch: paymentFeeTypes is an object, not an array.

The paymentFeeTypes prop is passed as an object from currentPaymentFeeListTypeState (defined in src/reducers/payment_profiles/payment-fee-types-list-reducer.js) with properties order, orderDir, totalPaymentFeeTypes, and paymentFeeTypes. It is currently typed as PropTypes.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

📥 Commits

Reviewing files that changed from the base of the PR and between 1aad0b6 and cadb9f9.

📒 Files selected for processing (2)
  • src/pages/tickets/payment-profile/components/payment-profile-dialog.js
  • src/pages/tickets/payment-profile/payment-profile-list-page.js

@smarcet smarcet requested a review from santipalenque May 28, 2026 02:30
@smarcet

smarcet commented May 28, 2026

Copy link
Copy Markdown

@tomrndom there are a couple of ux issues at fee types ( pre existents)
amount should be sent in cents and converted to usd at ui ( price fiekd)
rate should be sent in BPS and converted to percentaje ( allow 2 decimals from 0.01 to 100.00 )
max cents should be a price and could be zero ( send cents and convert to usd)
min cents should be a price ( send cents and convert to usd)

@smarcet smarcet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomrndom please review

@smarcet smarcet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/pages/tickets/payment-profile/components/payment-profile-dialog.js Outdated
Comment thread src/pages/tickets/payment-profile/payment-profile-list-page.js
Comment thread src/pages/tickets/payment-profile/payment-profile-list-page.js Outdated
Comment thread src/pages/tickets/payment-profile/payment-profile-list-page.js
Comment thread src/reducers/payment_profiles/payment-profile-list-reducer.js
component={EditPaymentFeeTypePage}
/>

<Route component={NoMatchPage} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [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.

Comment thread src/actions/ticket-actions.js Outdated
if (term) {
const escapedTerm = escapeFilterValue(term);
filter.push(
`provider=@${escapedTerm},id=@${escapedTerm},application_type=@${escapedTerm}`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [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 =

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [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:

  • getPaymentProfiles with term — URL/params construction with non-empty term (verify escapeFilterValue, the filter[] array shape, and the new REQUEST_PAYMENT_PROFILES payload).
  • Reducer REQUEST_PAYMENT_PROFILES — asserts that term, currentPage, perPage land in state (catches the bug flagged in payment-profile-list-reducer.js#L46).
  • PaymentProfileDialog flow
    • 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 (.catch behaviour),
    • validation prevents submit when required fields are empty (once the schema is re-enabled).

This is a UX-critical surface (payment configuration). At minimum the action + reducer should be covered.

@santipalenque santipalenque left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no further flags other than seba's

Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_cents are 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 for max_cents and min_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 win

Extend the formatting test to assert max_cents/min_cents display conversion too.

This test already guards Rate/Amount value formatting; 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

📥 Commits

Reviewing files that changed from the base of the PR and between cadb9f9 and fe7e71b.

📒 Files selected for processing (8)
  • src/actions/__tests__/payment-profile-actions.test.js
  • src/pages/tickets/payment-profile/__tests__/payment-profile-list-page.test.js
  • src/pages/tickets/payment-profile/components/__tests__/payment-profile-dialog.test.js
  • src/pages/tickets/payment-profile/components/payment-profile-dialog.js
  • src/pages/tickets/payment-profile/payment-profile-list-page.js
  • src/reducers/payment_profiles/__tests__/payment-profile-list-reducer.test.js
  • src/reducers/payment_profiles/payment-profile-list-reducer.js
  • src/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

Comment thread src/pages/tickets/payment-profile/components/payment-profile-dialog.js Outdated
tomrndom added 2 commits June 5, 2026 00:30
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Comment thread src/pages/tickets/payment-profile/payment-profile-list-page.js Outdated
Comment thread src/pages/tickets/payment-profile/payment-profile-list-page.js Outdated
@smarcet smarcet requested a review from santipalenque June 17, 2026 17:59

@smarcet smarcet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomrndom please review comments

Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Comment thread src/actions/ticket-actions.js Outdated
if (term) {
const escapedTerm = escapeFilterValue(term);
filter.push(
`provider=@${escapedTerm},id=@${escapedTerm},application_type=@${escapedTerm}`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] id is a numeric field — the API only supports exact match (id==<number>) on it, not the contains operator (=@).

The current filter string id=@${escapedTerm} will be silently ignored or cause an API error, meaning search by profile ID won't work.

Use OR concatenation to keep the numeric filter separate:

filter.push(`provider=@${escapedTerm},application_type=@${escapedTerm}`);

// numeric id: only add an exact-match filter if the term looks like an integer
const numericId = parseInt(escapedTerm, 10);
if (!Number.isNaN(numericId)) {
  filter.push(`id==`);
}

This way string searches match on provider / application_type, and a purely numeric term also matches on id.

setIsSaving(true);
onSave(values)
.catch(() => {})
.finally(() => setIsSaving(false));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] This deviates from the popup-dialog pattern — the dialog should close itself on success, not the parent.

The current chain never calls onClose():

onSave(values)
  .catch(() => {})
  .finally(() => setIsSaving(false));  // dialog never closes itself

Meanwhile handleSave in the list page calls setPaymentProfilePopup(false) directly in .then(), which unmounts the dialog while isSaving is still truesetIsSaving(false) then fires on an unmounted component (React 16 warning).

Fix: add .then(() => onClose()) here, and remove setPaymentProfilePopup(false) from the parent's handleSave:

onSave(values)
  .then(() => onClose())   // popup closes itself on success
  .catch(() => {})          // stays open on error
  .finally(() => setIsSaving(false));

This is the same deviation being fixed in ticket 86bag2zk7 (page-templates, inventory, sponsorship-types). The fee-type save at line 171 already does this correctly — align the profile save to match.


if (entity.id) {
putRequest(
return putRequest(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

savePaymentProfile is missing startLoading/stopLoading dispatches. Compare with savePaymentFeeType added in this same PR, which correctly wraps the request:

dispatch(startLoading());

if (entity.id) {
  return putRequest(...)(params)(dispatch)
    .then(...)
    .finally(() => dispatch(stopLoading()));
}

return postRequest(...)(params)(dispatch)
  .then(...)
  .finally(() => dispatch(stopLoading()));

getPaymentProfile in this file also calls dispatch(startLoading()) explicitly, confirming the foundation primitives don't handle it automatically. Without it, the global spinner never fires during profile create/update — the user sees frozen-but-disabled buttons with no feedback if the API is slow.

@smarcet smarcet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomrndom please review

tomrndom added 2 commits June 19, 2026 02:36
…nsability

Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants