From 283d13eb35fbfac5fa9beb4703df2b28e80163b3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 13 Jan 2026 14:16:38 +1100 Subject: [PATCH 1/5] Move payment method display logic to helper So we don't need to convert strings into classes to then only convert it into the same string again. --- .../spree/admin/payment_methods_controller.rb | 2 +- app/helpers/spree/admin/payment_methods_helper.rb | 14 ++++++++++++++ .../admin/payment_methods/_providers.html.haml | 2 +- .../spree/admin/payment_methods/index.html.haml | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 app/helpers/spree/admin/payment_methods_helper.rb diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb index 8073fb544a..b8ad17a608 100644 --- a/app/controllers/spree/admin/payment_methods_controller.rb +++ b/app/controllers/spree/admin/payment_methods_controller.rb @@ -138,7 +138,7 @@ module Spree providers.delete("Spree::Gateway::StripeSCA") unless show_stripe? - providers.map(&:constantize) + providers end # Show Stripe as an option if enabled, or if the diff --git a/app/helpers/spree/admin/payment_methods_helper.rb b/app/helpers/spree/admin/payment_methods_helper.rb new file mode 100644 index 0000000000..575a205b71 --- /dev/null +++ b/app/helpers/spree/admin/payment_methods_helper.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Spree + module Admin + module PaymentMethodsHelper + def payment_method_type_name(class_name) + scope = "spree.admin.payment_methods.providers" + key = class_name.demodulize.downcase + + I18n.t(key, scope:) + end + end + end +end diff --git a/app/views/spree/admin/payment_methods/_providers.html.haml b/app/views/spree/admin/payment_methods/_providers.html.haml index 15bd4ef979..b645c5c083 100644 --- a/app/views/spree/admin/payment_methods/_providers.html.haml +++ b/app/views/spree/admin/payment_methods/_providers.html.haml @@ -3,6 +3,6 @@ .alpha.four.columns = label :payment_method, :type, t('.provider') .omega.twelve.columns - = collection_select(:payment_method, :type, @providers, :to_s, :clean_name, {}, { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) + = select(:payment_method, :type, @providers.map { |p| [payment_method_type_name(p), p] }, {}, { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) %div{"ng-include" => "include_html" } diff --git a/app/views/spree/admin/payment_methods/index.html.haml b/app/views/spree/admin/payment_methods/index.html.haml index 1952c60c06..61eb28c63a 100644 --- a/app/views/spree/admin/payment_methods/index.html.haml +++ b/app/views/spree/admin/payment_methods/index.html.haml @@ -37,7 +37,7 @@ - method.distributors.each do |distributor| = distributor.name %br/ - %td= method.class.clean_name + %td= payment_method_type_name(method.class.name) - if spree_current_user.admin? %td.align-center= method.environment.to_s.titleize %td.align-center= method.display_on.blank? ? t('.both') : t('.' + method.display_on.to_s) From 3be0cca230ad45d85e0684df6738de2baf3eb448 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Feb 2026 11:47:01 +1100 Subject: [PATCH 2/5] Move options code into helper --- app/helpers/spree/admin/payment_methods_helper.rb | 4 ++++ app/views/spree/admin/payment_methods/_providers.html.haml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/helpers/spree/admin/payment_methods_helper.rb b/app/helpers/spree/admin/payment_methods_helper.rb index 575a205b71..4d94a7ad52 100644 --- a/app/helpers/spree/admin/payment_methods_helper.rb +++ b/app/helpers/spree/admin/payment_methods_helper.rb @@ -9,6 +9,10 @@ module Spree I18n.t(key, scope:) end + + def payment_method_type_options(providers) + providers.map { |p| [payment_method_type_name(p), p] } + end end end end diff --git a/app/views/spree/admin/payment_methods/_providers.html.haml b/app/views/spree/admin/payment_methods/_providers.html.haml index b645c5c083..fbbb6fec96 100644 --- a/app/views/spree/admin/payment_methods/_providers.html.haml +++ b/app/views/spree/admin/payment_methods/_providers.html.haml @@ -3,6 +3,6 @@ .alpha.four.columns = label :payment_method, :type, t('.provider') .omega.twelve.columns - = select(:payment_method, :type, @providers.map { |p| [payment_method_type_name(p), p] }, {}, { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) + = select(:payment_method, :type, payment_method_type_options(@providers), {}, { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) %div{"ng-include" => "include_html" } From 51df612c519e90d3862c1695244a73e78f5b31cb Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Feb 2026 11:47:27 +1100 Subject: [PATCH 3/5] Spec current default selection And fix the matcher! This matcher was written over ten years ago in 2015. But it never failed wrong tests. --- spec/support/matchers/select2_matchers.rb | 4 +--- spec/support/request/web_helper.rb | 2 -- spec/system/admin/adjustments_spec.rb | 2 +- spec/system/admin/payment_method_spec.rb | 3 +++ 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/spec/support/matchers/select2_matchers.rb b/spec/support/matchers/select2_matchers.rb index 8e7be8f87e..bdafd93ae9 100644 --- a/spec/support/matchers/select2_matchers.rb +++ b/spec/support/matchers/select2_matchers.rb @@ -92,9 +92,7 @@ RSpec::Matchers.define :have_select2 do |id, options = {}| end def selected_option_is(from, text) - within find(from) do - find("a.select2-choice").text == text - end + find("#{from} a.select2-choice", text:) end def with_select2_open(from) diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index 9087d59981..de10dc79c6 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -69,8 +69,6 @@ module WebHelper .find(:css, '.select2-drop-active .select2-result-label', text: options[:select_text] || value) .click - - expect(page).to have_select2 options[:from], selected: options[:select_text] || value end def open_select2(selector) diff --git a/spec/system/admin/adjustments_spec.rb b/spec/system/admin/adjustments_spec.rb index 64ce6a5ad0..8cb0cf9956 100644 --- a/spec/system/admin/adjustments_spec.rb +++ b/spec/system/admin/adjustments_spec.rb @@ -152,7 +152,7 @@ RSpec.describe ' click_link 'Adjustments' page.find('tr', text: 'Extra Adjustment').find('a.icon-edit').click - expect(page).to have_select2 :adjustment_tax_category_id, selected: [] + expect(page).to have_select2 :adjustment_tax_category_id, selected: "None" # When I edit the adjustment, setting a tax rate select2_select 'GST', from: :adjustment_tax_category_id diff --git a/spec/system/admin/payment_method_spec.rb b/spec/system/admin/payment_method_spec.rb index 1a6c519a59..582cdec8ac 100644 --- a/spec/system/admin/payment_method_spec.rb +++ b/spec/system/admin/payment_method_spec.rb @@ -20,6 +20,9 @@ RSpec.describe ' click_link 'Payment Methods' click_link 'New Payment Method' + # Selects the first one by default: + expect(page).to have_select2 "payment_method_type", selected: "PayPal Express" + fill_in 'payment_method_name', with: 'Cheque payment method' check "payment_method_distributor_ids_#{@distributors[0].id}" From 9da3b0ea01d7fab54b0efc062ce401dbf9203964 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 12 Feb 2026 13:02:06 +1100 Subject: [PATCH 4/5] User must choose payment method type on creation --- .../spree/admin/payment_methods/_providers.html.haml | 2 +- spec/system/admin/payment_method_spec.rb | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/views/spree/admin/payment_methods/_providers.html.haml b/app/views/spree/admin/payment_methods/_providers.html.haml index fbbb6fec96..942a2a0c8f 100644 --- a/app/views/spree/admin/payment_methods/_providers.html.haml +++ b/app/views/spree/admin/payment_methods/_providers.html.haml @@ -3,6 +3,6 @@ .alpha.four.columns = label :payment_method, :type, t('.provider') .omega.twelve.columns - = select(:payment_method, :type, payment_method_type_options(@providers), {}, { class: 'select2 fullwidth', 'provider-prefs-for' => "#{@object.id}"}) + = select(:payment_method, :type, payment_method_type_options(@providers), {}, { class: 'select2 fullwidth', required: true, placeholder: t("admin.choose"), 'provider-prefs-for' => "#{@object.id}"}) %div{"ng-include" => "include_html" } diff --git a/spec/system/admin/payment_method_spec.rb b/spec/system/admin/payment_method_spec.rb index 582cdec8ac..2ee60b3c2b 100644 --- a/spec/system/admin/payment_method_spec.rb +++ b/spec/system/admin/payment_method_spec.rb @@ -20,10 +20,11 @@ RSpec.describe ' click_link 'Payment Methods' click_link 'New Payment Method' - # Selects the first one by default: - expect(page).to have_select2 "payment_method_type", selected: "PayPal Express" + expect(page).to have_select2 "payment_method_type", selected: "Choose..." fill_in 'payment_method_name', with: 'Cheque payment method' + cash_name = "Cash/EFT/etc. (payments for which automatic validation is not required)" + select2_select cash_name, from: "payment_method_type" check "payment_method_distributor_ids_#{@distributors[0].id}" click_button 'Create' @@ -246,6 +247,9 @@ RSpec.describe ' it "creates payment methods" do visit spree.new_admin_payment_method_path fill_in 'payment_method_name', with: 'Cheque payment method' + cash_name = "Cash/EFT/etc. (payments for which automatic validation is not required)" + select2_select cash_name, from: "payment_method_type" + expect(page).to have_field 'payment_method_description' expect(page).to have_select 'payment_method_display_on' From 482c2a4a6ee6c1e85276583c99cd19edb46def53 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 13 Feb 2026 12:36:26 +1100 Subject: [PATCH 5/5] Delete now unused method clean_name --- app/models/spree/payment_method.rb | 5 ----- spec/models/spree/payment_method_spec.rb | 7 ------- 2 files changed, 12 deletions(-) diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 77474fb277..7f845f1160 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -113,11 +113,6 @@ module Spree distributors.include?(distributor) end - def self.clean_name - scope = "spree.admin.payment_methods.providers" - I18n.t(name.demodulize.downcase, scope:) - end - private def distributor_validation diff --git a/spec/models/spree/payment_method_spec.rb b/spec/models/spree/payment_method_spec.rb index d84d6e4ca3..44ec84cce7 100644 --- a/spec/models/spree/payment_method_spec.rb +++ b/spec/models/spree/payment_method_spec.rb @@ -131,13 +131,6 @@ RSpec.describe Spree::PaymentMethod do expect(pm.errors.to_a).to eq(["Name can't be blank", "At least one hub must be selected"]) end - it "generates a clean name for known Payment Method types" do - expect(Spree::PaymentMethod::Check.clean_name) - .to eq('Cash/EFT/etc. (payments for which automatic validation is not required)') - expect(Spree::Gateway::PayPalExpress.clean_name).to eq('PayPal Express') - expect(Spree::Gateway::StripeSCA.clean_name).to eq('Stripe SCA') - end - it "computes the amount of fees" do order = create(:order)