fix: migrate payment profiles to MUI components#925
Open
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughThis PR consolidates payment profile and fee-type management from three separate route-based pages into a single unified list page with an embedded modal dialog. Changes include updated Redux state tracking for pagination and search terms, API action refactoring to use snackbar-based notifications, simplified routing, and a new reusable PaymentProfileDialog component. ChangesPayment Profile Dialog-Based Management
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
Comment on lines
+213
to
+220
| <Dialog | ||
| open | ||
| onClose={handleClose} | ||
| maxWidth="md" | ||
| fullWidth | ||
| disableEnforceFocus | ||
| disableAutoFocus | ||
| disableRestoreFocus |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/pages/tickets/payment-profile/components/payment-profile-dialog.js | head -250Repository: fntechgit/summit-admin
Length of output: 9796
🏁 Script executed:
cat -n src/pages/tickets/payment-profile/components/payment-profile-dialog.js | sed -n '250,400p'Repository: fntechgit/summit-admin
Length of output: 6630
🏁 Script executed:
cat -n src/pages/tickets/payment-profile/components/payment-profile-dialog.js | sed -n '400,600p'Repository: fntechgit/summit-admin
Length of output: 9347
Re-enable the dialog's focus management.
The three focus-related props (disableEnforceFocus, disableAutoFocus, disableRestoreFocus) remove the modal's focus trap and break keyboard/screen-reader navigation. If this is working around a specific field issue, apply the workaround to that field instead of disabling it globally for the entire dialog.
🤖 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 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.
Comment on lines
+100
to
+104
| const handleSaveFeeType = (entity) => | ||
| savePaymentFeeType(entity).then(() => { | ||
| getPaymentFeeTypes(currentPaymentProfile.id); | ||
| setPaymentProfilePopup(false); | ||
| }); |
There was a problem hiding this comment.
Keep the payment-profile dialog open after saving a fee type.
This closes the entire parent dialog after an inline fee-type save, so users cannot review the refreshed fee list or continue editing the payment profile. It also leaves PaymentProfileDialog’s own then path resetting fee-type state after the parent has already unmounted it.
Suggested fix
const handleSaveFeeType = (entity) =>
savePaymentFeeType(entity).then(() => {
- getPaymentFeeTypes(currentPaymentProfile.id);
- setPaymentProfilePopup(false);
+ return getPaymentFeeTypes(currentPaymentProfile.id);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleSaveFeeType = (entity) => | |
| savePaymentFeeType(entity).then(() => { | |
| getPaymentFeeTypes(currentPaymentProfile.id); | |
| setPaymentProfilePopup(false); | |
| }); | |
| const handleSaveFeeType = (entity) => | |
| savePaymentFeeType(entity).then(() => { | |
| return getPaymentFeeTypes(currentPaymentProfile.id); | |
| }); |
🤖 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/payment-profile-list-page.js` around lines
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.
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.
ref: https://app.clickup.com/t/86b9tgt19
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Improvements