diff --git a/backend/app/views/spree/admin/payment_methods/_form.html.erb b/backend/app/views/spree/admin/payment_methods/_form.html.erb index 9d9eb3c46e1..b6b4a32248a 100644 --- a/backend/app/views/spree/admin/payment_methods/_form.html.erb +++ b/backend/app/views/spree/admin/payment_methods/_form.html.erb @@ -19,9 +19,10 @@ <% if @object.preference_source.present? %> <%= t('spree.preference_source_using', name: @object.preference_source) %> <% elsif !@object.new_record? %> - <% @object.preferences.keys.each do |key| %> - <%= render "spree/admin/shared/preference_fields/#{@object.preference_type(key)}", - form: f, attribute: "preferred_#{key}", label: t(key, scope: 'spree') %> + <% @object.admin_form_preference_names.each do |name| %> + <%= render "spree/admin/shared/preference_fields/#{@object.preference_type(name)}", + form: f, attribute: "preferred_#{name}", + label: t(name, scope: 'spree', default: name.to_s.humanize) %> <% end %> <% end %> diff --git a/backend/app/views/spree/admin/promotions/calculators/_default_fields.html.erb b/backend/app/views/spree/admin/promotions/calculators/_default_fields.html.erb index 78273ae01cf..0bdcfa9286e 100644 --- a/backend/app/views/spree/admin/promotions/calculators/_default_fields.html.erb +++ b/backend/app/views/spree/admin/promotions/calculators/_default_fields.html.erb @@ -1,5 +1,6 @@ -<% calculator.preferences.keys.map do |key| %> - <%= render "spree/admin/shared/preference_fields/#{calculator.preference_type(key)}", - name: "#{prefix}[calculator_attributes][preferred_#{key}]", - value: calculator.get_preference(key), label: t(key.to_s, scope: 'spree') %> +<% calculator.admin_form_preference_names.map do |name| %> + <%= render "spree/admin/shared/preference_fields/#{calculator.preference_type(name)}", + name: "#{prefix}[calculator_attributes][preferred_#{name}]", + value: calculator.get_preference(name), + label: t(name.to_s, scope: 'spree', default: name.to_s.humanize) %> <% end %> diff --git a/backend/app/views/spree/admin/shared/_calculator_fields.html.erb b/backend/app/views/spree/admin/shared/_calculator_fields.html.erb index 1bfc3a2ea64..3d078f24f30 100644 --- a/backend/app/views/spree/admin/shared/_calculator_fields.html.erb +++ b/backend/app/views/spree/admin/shared/_calculator_fields.html.erb @@ -11,9 +11,10 @@ <% calculator = f.object.calculator.class == calculator_class ? f.object.calculator : calculator_class.new %>
<%= f.fields_for :calculator, calculator do |calculator_form| %> - <% calculator.preferences.keys.each do |key| %> - <%= render "spree/admin/shared/preference_fields/#{calculator.preference_type(key)}", - form: calculator_form, attribute: "preferred_#{key}", label: t(key, scope: 'spree') %> + <% calculator.admin_form_preference_names.each do |name| %> + <%= render "spree/admin/shared/preference_fields/#{calculator.preference_type(name)}", + form: calculator_form, attribute: "preferred_#{name}", + label: t(name, scope: 'spree', default: name.to_s.humanize) %> <% end %> <% end %>
diff --git a/backend/spec/features/admin/configuration/payment_methods_spec.rb b/backend/spec/features/admin/configuration/payment_methods_spec.rb index 1d93b9aca49..f56003e93f5 100644 --- a/backend/spec/features/admin/configuration/payment_methods_spec.rb +++ b/backend/spec/features/admin/configuration/payment_methods_spec.rb @@ -45,8 +45,9 @@ end context "admin editing a payment method" do - before(:each) do - create(:check_payment_method) + let!(:payment_method) { create(:check_payment_method) } + + before do click_link "Payments" expect(page).to have_link 'Payment Methods' within("table#listing_payment_methods") do @@ -66,6 +67,22 @@ click_button "Update" expect(page).to have_content("Name can't be blank") end + + context 'with payment method having hash and array as preference' do + class ComplexPayments < Spree::PaymentMethod + preference :name, :string + preference :mapping, :hash + preference :recipients, :array + end + + let!(:payment_method) { ComplexPayments.create!(name: 'Complex Payments') } + + it "does not display preference fields that are hash or array" do + expect(page).to have_field("Name") + expect(page).to_not have_field("Mapping") + expect(page).to_not have_field("Recipients") + end + end end context "changing type and payment_source", js: true do diff --git a/backend/spec/features/admin/configuration/shipping_methods_spec.rb b/backend/spec/features/admin/configuration/shipping_methods_spec.rb index e63700a4174..63446a6964d 100644 --- a/backend/spec/features/admin/configuration/shipping_methods_spec.rb +++ b/backend/spec/features/admin/configuration/shipping_methods_spec.rb @@ -35,6 +35,46 @@ click_on "Create" expect(current_path).to eql(spree.edit_admin_shipping_method_path(Spree::ShippingMethod.last)) end + + context 'with shipping method having a calculator with array or hash preference type' do + before do + class ComplexShipments < Spree::ShippingCalculator + preference :amount, :decimal + preference :currency, :string + preference :mapping, :hash + preference :list, :array + + def self.description + "Complex Shipments" + end + end + @calculators = Rails.application.config.spree.calculators.shipping_methods + Rails.application.config.spree.calculators.shipping_methods = [ComplexShipments] + end + + after do + Rails.application.config.spree.calculators.shipping_methods = @calculators + end + + it "does not show array and hash form fields" do + click_link "New Shipping Method" + + fill_in "shipping_method_name", with: "bullock cart" + + within("#shipping_method_categories_field") do + check first("input[type='checkbox']")["name"] + end + + click_on "Create" + select 'Complex Shipments', from: 'Base Calculator' + click_on "Update" + + expect(page).to have_field('Amount') + expect(page).to have_field('Currency') + expect(page).to_not have_field('Mapping') + expect(page).to_not have_field('List') + end + end end # Regression test for https://github.com/spree/spree/issues/1331 diff --git a/backend/spec/features/admin/promotion_adjustments_spec.rb b/backend/spec/features/admin/promotion_adjustments_spec.rb index dca1b3dff62..c165ad1dd5d 100644 --- a/backend/spec/features/admin/promotion_adjustments_spec.rb +++ b/backend/spec/features/admin/promotion_adjustments_spec.rb @@ -242,5 +242,48 @@ expect(first_action.calculator.class).to eq(Spree::Calculator::FlatRate) expect(first_action.calculator.preferred_amount).to eq(5) end + + context 'creating a promotion with promotion action that has a calculator with complex preferences' do + before do + class ComplexCalculator < Spree::Calculator + preference :amount, :decimal + preference :currency, :string + preference :mapping, :hash + preference :list, :array + + def self.description + "Complex Calculator" + end + end + @calculators = Rails.application.config.spree.calculators.promotion_actions_create_item_adjustments + Rails.application.config.spree.calculators.promotion_actions_create_item_adjustments = [ComplexCalculator] + end + + after do + Rails.application.config.spree.calculators.promotion_actions_create_item_adjustments = @calculators + end + + it "does not show array and hash form fields" do + fill_in "Name", with: "SAVE SAVE SAVE" + choose "Apply to all orders" + click_button "Create" + expect(page).to have_title("SAVE SAVE SAVE - Promotions") + + select "Create per-line-item adjustment", from: "Add action of type" + within('#action_fields') do + click_button "Add" + select "Complex Calculator", from: "Base Calculator" + end + within('#actions_container') { click_button "Update" } + expect(page).to have_text 'successfully updated' + + within('#action_fields') do + expect(page).to have_field('Amount') + expect(page).to have_field('Currency') + expect(page).to_not have_field('Mapping') + expect(page).to_not have_field('List') + end + end + end end end diff --git a/core/lib/spree/preferences/preferable.rb b/core/lib/spree/preferences/preferable.rb index fd28f979eb1..36008856a54 100644 --- a/core/lib/spree/preferences/preferable.rb +++ b/core/lib/spree/preferences/preferable.rb @@ -1,43 +1,55 @@ -# Preferable allows defining preference accessor methods. -# -# A class including Preferable must implement #preferences which should return -# an object responding to .fetch(key), []=(key, val), and .delete(key). -# -# The generated writer method performs typecasting before assignment into the -# preferences object. -# -# Examples: -# -# # Spree::Base includes Preferable and defines preferences as a serialized -# # column. -# class Settings < Spree::Base -# preference :color, :string, default: 'red' -# preference :temperature, :integer, default: 21 -# end -# -# s = Settings.new -# s.preferred_color # => 'red' -# s.preferred_temperature # => 21 -# -# s.preferred_color = 'blue' -# s.preferred_color # => 'blue' -# -# # Typecasting is performed on assignment -# s.preferred_temperature = '24' -# s.preferred_color # => 24 -# -# # Modifications have been made to the .preferences hash -# s.preferences #=> {color: 'blue', temperature: 24} -# -# # Save the changes. All handled by activerecord -# s.save! - require 'spree/preferences/preferable_class_methods' require 'active_support/concern' require 'active_support/core_ext/hash/keys' module Spree module Preferences + # Preferable allows defining preference accessor methods. + # + # A class including Preferable must implement #preferences which should return + # an object responding to .fetch(key), []=(key, val), and .delete(key). + # + # The generated writer method performs typecasting before assignment into the + # preferences object. + # + # Examples: + # + # # Spree::Base includes Preferable and defines preferences as a serialized + # # column. + # class Settings < Spree::Base + # preference :color, :string, default: 'red' + # preference :temperature, :integer, default: 21 + # end + # + # s = Settings.new + # s.preferred_color # => 'red' + # s.preferred_temperature # => 21 + # + # s.preferred_color = 'blue' + # s.preferred_color # => 'blue' + # + # # Typecasting is performed on assignment + # s.preferred_temperature = '24' + # s.preferred_color # => 24 + # + # # Modifications have been made to the .preferences hash + # s.preferences #=> {color: 'blue', temperature: 24} + # + # # Save the changes. All handled by activerecord + # s.save! + # + # Each preference gets rendered as a form field in Solidus backend. + # + # As not all supported preference types are representable as a form field, only + # some of them get rendered per default. Arrays and Hashes for instance are + # supported preference field types, but do not represent well as a form field. + # + # Overwrite +allowed_admin_form_preference_types+ in your class if you want to + # provide more fields. If you do so, you also need to provide a preference field + # partial that lives in: + # + # +app/views/spree/admin/shared/preference_fields/+ + # module Preferable extend ActiveSupport::Concern @@ -101,6 +113,30 @@ def default_preferences ] end + # Preference names representable as form fields in Solidus backend + # + # Not all preferences are representable as a form field. + # + # Arrays and Hashes for instance are supported preference field types, + # but do not represent well as a form field. + # + # As these kind of preferences are mostly developer facing + # and not admin facing we should not render them. + # + # Overwrite +allowed_admin_form_preference_types+ in your class that + # includes +Spree::Preferable+ if you want to provide more fields. + # If you do so, you also need to provide a preference field partial + # that lives in: + # + # +app/views/spree/admin/shared/preference_fields/+ + # + # @return [Array] + def admin_form_preference_names + defined_preferences.keep_if do |type| + preference_type(type).in? self.class.allowed_admin_form_preference_types + end + end + private def convert_preference_value(value, type) diff --git a/core/lib/spree/preferences/preferable_class_methods.rb b/core/lib/spree/preferences/preferable_class_methods.rb index 2abcf14d1c1..823e0f6e703 100644 --- a/core/lib/spree/preferences/preferable_class_methods.rb +++ b/core/lib/spree/preferences/preferable_class_methods.rb @@ -1,5 +1,14 @@ module Spree::Preferences module PreferableClassMethods + DEFAULT_ADMIN_FORM_PREFERENCE_TYPES = %i( + boolean + decimal + integer + password + string + text + ) + def defined_preferences [] end @@ -60,5 +69,18 @@ def preference_default_getter_method(name) def preference_type_getter_method(name) "preferred_#{name}_type".to_sym end + + # List of preference types allowed as form fields in the Solidus admin + # + # Overwrite this method in your class that includes +Spree::Preferable+ + # if you want to provide more fields. If you do so, you also need to provide + # a preference field partial that lives in: + # + # +app/views/spree/admin/shared/preference_fields/+ + # + # @return [Array] + def allowed_admin_form_preference_types + DEFAULT_ADMIN_FORM_PREFERENCE_TYPES + end end end diff --git a/core/spec/models/spree/preferences/preferable_spec.rb b/core/spec/models/spree/preferences/preferable_spec.rb index ebec0c347ba..783e7d61eff 100644 --- a/core/spec/models/spree/preferences/preferable_spec.rb +++ b/core/spec/models/spree/preferences/preferable_spec.rb @@ -105,6 +105,50 @@ class B < A }) end + describe '#admin_form_preference_names' do + subject do + ComplexPreferableClass.new.admin_form_preference_names + end + + before do + class ComplexPreferableClass + include Spree::Preferences::Preferable + preference :name, :string + preference :password, :password + preference :mapping, :hash + preference :recipients, :array + end + end + + it "returns an array of preference names excluding preferences not presentable as form field" do + is_expected.to contain_exactly(:name, :password) + end + + context 'with overwritten allowed_admin_form_preference_types class method' do + subject do + ComplexOverwrittenPreferableClass.new.admin_form_preference_names + end + + before do + class ComplexOverwrittenPreferableClass + include Spree::Preferences::Preferable + preference :name, :string + preference :password, :password + preference :mapping, :hash + preference :recipients, :array + + def self.allowed_admin_form_preference_types + %i(string password hash array) + end + end + end + + it 'returns these types instead' do + is_expected.to contain_exactly(:name, :password, :mapping, :recipients) + end + end + end + context "converts integer preferences to integer values" do before do A.preference :is_integer, :integer