-
-
Notifications
You must be signed in to change notification settings - Fork 852
Fix payment processor selection persistence #34180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
| else if (cj('input[name="payment_processor_id"]:checked').length === 0) { | ||
| cj('input[name="payment_processor_id"][checked=checked]').prop('checked', true); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To further optimize how about :
diff --git a/templates/CRM/common/paymentBlock.tpl b/templates/CRM/common/paymentBlock.tpl
index 8a3c00436b..842880ce41 100644
--- a/templates/CRM/common/paymentBlock.tpl
+++ b/templates/CRM/common/paymentBlock.tpl
@@ -24,7 +24,13 @@
// might be that the unselecting of the processor should cause it
// to be hidden (or removed) in which case it can go from this function.
var billing_block = cj("div#billing-payment-block");
+ let payment_processor_options = cj('input[name="payment_processor_id"]');
if (isHide) {
+ let checked = payment_processor_options.filter(':checked').val();
+ if (checked) {
+ billing_block.data('saved-ppid', checked);
+ }
+
payment_options.hide();
payment_processor.hide();
payment_information.hide();
@@ -32,7 +38,7 @@
// Ensure that jquery validation doesn't block submission when we don't need to fill in the billing details section
cj('#billing-payment-block select.crm-select2').addClass('crm-no-validate');
// also unset selected payment methods
- cj('input[name="payment_processor_id"]').removeProp('checked');
+ payment_processor_options.removeProp('checked');
}
else {
payment_options.show();
@@ -41,7 +47,16 @@
billing_block.show();
cj('#billing-payment-block select.crm-select2').removeClass('crm-no-validate');
// also set selected payment methods
- cj('input[name="payment_processor_id"][checked=checked]').prop('checked', true);
+
+ let saved = billing_block.data('saved-ppid');
+ if (saved && payment_processor_options.filter('[value="' + saved + '"]').length) {
+ payment_processor_options.filter('[value="' + saved + '"]').prop('checked', true);
+ billing_block.removeData('saved-ppid');
+ }
+ else if (!payment_processor_options.is(':checked')) {
+ // Fallback to default
+ payment_processor_options.filter('[checked=checked]').prop('checked', true);
+ }
}
}just removed var declaration with let and use more generic variable to reduce variable declaration like restored_input and current_selection
|
Thanks for alternate PR. suggested a optimized patch~ |
Overview
Fixes #32342 along with original fix included #10826
From #32342:
Before
The payment processor selection switches to CC from pay-later and billing section is not exposed. I can replicate this issue in dmaster:
After
Fixed
Technical Details
On Hide: We now capture the currently selected payment_processor_id and store it in a data attribute on the billing block.
On Show: check for that data attribute.
If found: restore that specific processor.
If NOT found (and no other processor is currently active), fall back to the default behavior (selecting the processor marked checked=checked).
This preserves the fix for CRM-21038 (ensuring some processor is selected) while respecting explicit user choices during form updates.
Comments
Anything else you would like the reviewer to note