diff --git a/CHANGELOG.md b/CHANGELOG.md index d89c41786c0..617db7aa6ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## Solidus 2.2.0 (master, unreleased) +* Promotion and Shipping calculators can be created or have their type + changed without saving and reloading the page. [#1618](https://github.com/solidusio/solidus/pull/1618) + ## Solidus 2.1.0 (unreleased) * The OrderUpdater (as used by `order.update!`) now fully updates taxes. diff --git a/backend/app/assets/javascripts/spree/backend/calculator.js b/backend/app/assets/javascripts/spree/backend/calculator.js index fd39f36cda6..001f62abe5f 100644 --- a/backend/app/assets/javascripts/spree/backend/calculator.js +++ b/backend/app/assets/javascripts/spree/backend/calculator.js @@ -1,16 +1,26 @@ +Spree.CalculatorEditView = Backbone.View.extend({ + events: { + "change .js-calculator-type": "render", + }, + + initialize: function() { + this.render(); + }, + + render: function() { + var selected_class = this.$('.js-calculator-type option:selected').val(); + this.$('.js-calculator-preferences').each(function() { + var selected = ($(this).data('calculator-type') === selected_class); + $(this).find(':input').prop("disabled", !selected); + $(this).toggle(selected); + }); + } +}) + $(function() { - var calculator_select = $('select#calc_type') - var original_calc_type = calculator_select.prop('value'); - $('.calculator-settings-warning').hide(); - calculator_select.change(function() { - if (calculator_select.prop('value') == original_calc_type) { - $('div.calculator-settings').show(); - $('.calculator-settings-warning').hide(); - $('.calculator-settings').find('input,textarea').prop("disabled", false); - } else { - $('div.calculator-settings').hide(); - $('.calculator-settings-warning').show(); - $('.calculator-settings').find('input,texttarea').prop("disabled", true); - } + $('.js-calculator-fields').each(function() { + new Spree.CalculatorEditView({ + el: this + }); }); -}) +}); diff --git a/backend/app/assets/javascripts/spree/backend/promotions.js.coffee b/backend/app/assets/javascripts/spree/backend/promotions.js.coffee index 6605fab3e59..fcfaadc36e8 100644 --- a/backend/app/assets/javascripts/spree/backend/promotions.js.coffee +++ b/backend/app/assets/javascripts/spree/backend/promotions.js.coffee @@ -1,4 +1,49 @@ -window.initProductActions = -> + +# +# Tiered Calculator +# +TieredCalculatorView = Backbone.View.extend + initialize: -> + @calculatorName = @$('.js-tiers').data('calculator') + @tierFieldsTemplate = HandlebarsTemplates["promotions/calculators/fields/#{@calculatorName}"] + @originalTiers = @$('.js-tiers').data('original-tiers') + @formPrefix = @$('.js-tiers').data('form-prefix') + + for base, value of @originalTiers + @$('.js-tiers').append @tierFieldsTemplate + baseField: + value: base + valueField: + name: @tierInputName(base) + value: value + + events: + 'click .js-add-tier': 'onAdd' + 'click .js-remove-tier': 'onRemove' + 'change .js-base-input': 'onChange' + + tierInputName: (base) -> + "#{@formPrefix}[calculator_attributes][preferred_tiers][#{base}]" + + onAdd: (event) -> + event.preventDefault() + @$('.js-tiers').append @tierFieldsTemplate(valueField: name: null) + + onRemove: (event) -> + event.preventDefault() + $(event.target).parents('.tier').remove() + + onChange: (event) -> + valueInput = $(event.target).parents('.tier').find('.js-value-input') + valueInput.attr 'name', @tierInputName($(event.target).val()) + +initTieredCalculators = -> + $('.js-tiered-calculator').each -> + if !$(this).data('has-view') + $(this).data('has-view', true) + new TieredCalculatorView(el: this) + +window.initPromotionActions = -> # Add classes on promotion items for design $(document).on 'mouseover', 'a.delete', (event) -> $(this).parent().addClass 'action-remove' @@ -8,23 +53,6 @@ window.initProductActions = -> $('#promotion-filters').find('.variant_autocomplete').variantAutocomplete() - $('.calculator-fields').each -> - $fields_container = $(this) - $type_select = $fields_container.find('.type-select') - $settings = $fields_container.find('.settings') - $warning = $fields_container.find('.warning') - originalType = $type_select.val() - $warning.hide() - $type_select.change -> - if $(this).val() == originalType - $warning.hide() - $settings.show() - $settings.find('input').removeProp 'disabled' - else - $warning.show() - $settings.hide() - $settings.find('input').prop 'disabled', 'disabled' - # # Option Value Promo Rule # @@ -66,36 +94,6 @@ window.initProductActions = -> optionValueSelect.prop('disabled', $(this).val() == '').select2 'val', '' return - # - # Tiered Calculator - # - if $('.js-tiers').length - calculatorName = $('.js-tiers').data('calculator') - tierFieldsTemplate = HandlebarsTemplates["promotions/calculators/fields/#{calculatorName}"] - originalTiers = $('.js-tiers').data('original-tiers') - formPrefix = $('.js-tiers').data('form-prefix') - - tierInputName = (base) -> - "#{formPrefix}[calculator_attributes][preferred_tiers][#{base}]" - - $.each originalTiers, (base, value) -> - $('.js-tiers').append tierFieldsTemplate - baseField: - value: base - valueField: - name: tierInputName(base) - value: value - - $(document).on 'click', '.js-add-tier', (event) -> - event.preventDefault() - $('.js-tiers').append tierFieldsTemplate(valueField: name: null) - - $(document).on 'click', '.js-remove-tier', (event) -> - event.preventDefault() - $(this).parents('.tier').remove() - - $(document).on 'change', '.js-base-input', (event) -> - valueInput = $(this).parents('.tier').find('.js-value-input') - valueInput.attr 'name', tierInputName($(this).val()) + initTieredCalculators() -$ initProductActions +$ initPromotionActions diff --git a/backend/app/views/spree/admin/promotion_actions/create.js.erb b/backend/app/views/spree/admin/promotion_actions/create.js.erb index 9700fc67170..ea8371ec7d0 100644 --- a/backend/app/views/spree/admin/promotion_actions/create.js.erb +++ b/backend/app/views/spree/admin/promotion_actions/create.js.erb @@ -5,9 +5,10 @@ $(document).ready(function(){ //enable select2 functions for recently added box $('.type-select.select2').last().select2(); }); -initProductActions(); +initPromotionActions(); $('#<%= dom_id @promotion_action %>').hide(); $('#<%= dom_id @promotion_action %>').fadeIn(); +new Spree.CalculatorEditView({el: $('#<%= dom_id @promotion_action %> .js-calculator-fields')}); diff --git a/backend/app/views/spree/admin/promotions/actions/_promotion_calculators_with_custom_fields.html.erb b/backend/app/views/spree/admin/promotions/actions/_promotion_calculators_with_custom_fields.html.erb index 47c8b7560b5..7aa14bc5cd4 100644 --- a/backend/app/views/spree/admin/promotions/actions/_promotion_calculators_with_custom_fields.html.erb +++ b/backend/app/views/spree/admin/promotions/actions/_promotion_calculators_with_custom_fields.html.erb @@ -1,5 +1,5 @@
-
+
@@ -7,28 +7,27 @@ <%= label_tag field_name, Spree::Calculator.model_name.human %> <%= select_tag field_name, options_from_collection_for_select(calculators, :to_s, :description, promotion_action.calculator.type), - :class => 'type-select select2 fullwidth' %> - <% if promotion_action.calculator.respond_to?(:preferences) %> - <%= Spree.t(:calculator_settings_warning) %> - <% end %> + :class => 'type-select js-calculator-type select2 fullwidth' %>
- <% unless promotion_action.new_record? %> -
-
- <% type_name = promotion_action.calculator.type.demodulize.underscore %> - <% if lookup_context.exists?("fields", - ["spree/admin/promotions/calculators/#{type_name}"], true) %> - <%= render "spree/admin/promotions/calculators/#{type_name}/fields", - calculator: promotion_action.calculator, prefix: param_prefix %> - <% else %> - <%= render "spree/admin/promotions/calculators/default_fields", - calculator: promotion_action.calculator, prefix: param_prefix %> - <% end %> - <%= hidden_field_tag "#{param_prefix}[calculator_attributes][id]", promotion_action.calculator.id %> -
+
+
+ <% calculators.each do |calculator_class| %> + <% calculator = promotion_action.calculator.class == calculator_class ? promotion_action.calculator : calculator_class.new %> +
+ <% type_name = calculator.type.demodulize.underscore %> + <% if lookup_context.exists?("fields", + ["spree/admin/promotions/calculators/#{type_name}"], true) %> + <%= render "spree/admin/promotions/calculators/#{type_name}/fields", + calculator: calculator, prefix: param_prefix %> + <% else %> + <%= render "spree/admin/promotions/calculators/default_fields", + calculator: calculator, prefix: param_prefix %> + <% end %> +
+ <% end %>
- <% end %> +
diff --git a/backend/app/views/spree/admin/promotions/calculators/tiered_flat_rate/_fields.html.erb b/backend/app/views/spree/admin/promotions/calculators/tiered_flat_rate/_fields.html.erb index c8376bf2d3a..de91dec5511 100644 --- a/backend/app/views/spree/admin/promotions/calculators/tiered_flat_rate/_fields.html.erb +++ b/backend/app/views/spree/admin/promotions/calculators/tiered_flat_rate/_fields.html.erb @@ -6,9 +6,11 @@ type: calculator.preference_type(:base_amount)) %> <%= label_tag nil, Spree.t(:tiers) %> -<%= content_tag :div, nil, class: "js-tiers", data: { - 'original-tiers' => Hash[calculator.preferred_tiers.sort], - 'form-prefix' => prefix, - 'calculator' => 'tiered_flat_rate' -} %> - +
+ <%= content_tag :div, nil, class: "js-tiers", data: { + 'original-tiers' => Hash[calculator.preferred_tiers.sort], + 'form-prefix' => prefix, + 'calculator' => 'tiered_flat_rate' + } %> + +
diff --git a/backend/app/views/spree/admin/promotions/calculators/tiered_percent/_fields.html.erb b/backend/app/views/spree/admin/promotions/calculators/tiered_percent/_fields.html.erb index 7e2e773be6c..fd1765c8e10 100644 --- a/backend/app/views/spree/admin/promotions/calculators/tiered_percent/_fields.html.erb +++ b/backend/app/views/spree/admin/promotions/calculators/tiered_percent/_fields.html.erb @@ -6,9 +6,11 @@ type: calculator.preference_type(:base_percent)) %> <%= label_tag nil, Spree.t(:tiers) %> -<%= content_tag :div, nil, class: "js-tiers", data: { - 'original-tiers' => Hash[calculator.preferred_tiers.sort], - 'form-prefix' => prefix, - 'calculator' => 'tiered_percent' -} %> - +
+ <%= content_tag :div, nil, class: "js-tiers", data: { + 'original-tiers' => Hash[calculator.preferred_tiers.sort], + 'form-prefix' => prefix, + 'calculator' => 'tiered_percent' + } %> + +
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 eecdcb37eb0..c05e6747e54 100644 --- a/backend/app/views/spree/admin/shared/_calculator_fields.html.erb +++ b/backend/app/views/spree/admin/shared/_calculator_fields.html.erb @@ -1,20 +1,17 @@ -
+
<%= Spree::Calculator.model_name.human %> -
+
- <%= f.label(:calculator_type, Spree::Calculator.model_name.human, :for => 'calc_type') %> - <%= f.select(:calculator_type, @calculators.map { |c| [c.description, c.name] }, {}, {:id => 'calc_type', :class => 'select2 fullwidth'}) %> + <%= f.label(:calculator_type, Spree::Calculator.model_name.human) %> + <%= f.select(:calculator_type, @calculators.map { |c| [c.description, c.name] }, {}, {class: 'select2 fullwidth js-calculator-type'}) %>
- <% if !@object.new_record? %> -
-
- <%= f.fields_for :calculator do |calculator_form| %> - <%= preference_fields(@object.calculator, calculator_form) %> - <% end %> -
- <% if @object.calculator.respond_to?(:preferences) %> - <%= Spree.t(:calculator_settings_warning) %> + + <% @calculators.each do |calculator_class| %> + <% calculator = f.object.calculator.class == calculator_class ? f.object.calculator : calculator_class.new %> +
+ <%= f.fields_for :calculator, calculator do |calculator_form| %> + <%= preference_fields(calculator, calculator_form) %> <% end %>
<% end %> diff --git a/backend/spec/features/admin/configuration/shipping_methods_spec.rb b/backend/spec/features/admin/configuration/shipping_methods_spec.rb index abba4b58756..bf068d624df 100644 --- a/backend/spec/features/admin/configuration/shipping_methods_spec.rb +++ b/backend/spec/features/admin/configuration/shipping_methods_spec.rb @@ -5,16 +5,7 @@ let!(:zone) { create(:global_zone) } let!(:shipping_method) { create(:shipping_method, zones: [zone]) } - after do - Capybara.ignore_hidden_elements = true - end - before do - Capybara.ignore_hidden_elements = false - # HACK: To work around no email prompting on check out - allow_any_instance_of(Spree::Order).to receive_messages(require_email: false) - create(:check_payment_method) - visit spree.admin_path click_link "Settings" click_link "Shipping" @@ -31,7 +22,7 @@ end end - context "create" do + context "create", js: true do it "should be able to create a new shipping method" do click_link "New Shipping Method" @@ -48,17 +39,34 @@ # Regression test for https://github.com/spree/spree/issues/1331 context "update" do + it "can update the existing calculator", js: true do + within("#listing_shipping_methods") do + click_icon :edit + end + + fill_in 'Amount', with: 20 + + click_button "Update" + + expect(page).to have_content 'successfully updated' + expect(page).to have_field 'Amount', with: '20.0' + end + it "can change the calculator", js: true do within("#listing_shipping_methods") do click_icon :edit end - expect(find(:css, ".calculator-settings-warning")).not_to be_visible select2_search('Flexible Rate', from: 'Calculator') - expect(find(:css, ".calculator-settings-warning")).to be_visible + + fill_in 'First Item', with: 10 + fill_in 'Additional Item', with: 20 click_button "Update" - expect(page).not_to have_content("Shipping method is not found") + + expect(page).to have_content 'successfully updated' + expect(page).to have_field 'First Item', with: '10.0' + expect(page).to have_field 'Additional Item', with: '20.0' end end end diff --git a/backend/spec/features/admin/promotion_adjustments_spec.rb b/backend/spec/features/admin/promotion_adjustments_spec.rb index 00ca4c9b67e..29b13f25fae 100644 --- a/backend/spec/features/admin/promotion_adjustments_spec.rb +++ b/backend/spec/features/admin/promotion_adjustments_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe "Promotion Adjustments", type: :feature do +describe "Promotion Adjustments", type: :feature, js: true do stub_authorization! context "coupon promotions", js: true do diff --git a/core/app/models/concerns/spree/calculated_adjustments.rb b/core/app/models/concerns/spree/calculated_adjustments.rb index ef59ee212e7..fc3a7c72be6 100644 --- a/core/app/models/concerns/spree/calculated_adjustments.rb +++ b/core/app/models/concerns/spree/calculated_adjustments.rb @@ -4,7 +4,7 @@ module CalculatedAdjustments included do has_one :calculator, class_name: "Spree::Calculator", as: :calculable, inverse_of: :calculable, dependent: :destroy, autosave: true - accepts_nested_attributes_for :calculator + accepts_nested_attributes_for :calculator, update_only: true validates :calculator, presence: true end diff --git a/core/spec/lib/calculated_adjustments_spec.rb b/core/spec/lib/calculated_adjustments_spec.rb index d0e66a0f16f..07618a497d6 100644 --- a/core/spec/lib/calculated_adjustments_spec.rb +++ b/core/spec/lib/calculated_adjustments_spec.rb @@ -1,7 +1,111 @@ require 'spec_helper' describe Spree::CalculatedAdjustments do + let(:calculator_class) { Spree::Calculator::FlatRate } + + with_model :Calculable, scope: :all do + model do + include Spree::CalculatedAdjustments + end + end + it "should add has_one :calculator relationship" do - assert Spree::ShippingMethod.reflect_on_all_associations(:has_one).map(&:name).include?(:calculator) + expect(Calculable.reflect_on_all_associations(:has_one).map(&:name)).to include(:calculator) + end + + describe 'initialization' do + context 'with no calculator' do + subject { Calculable.new } + + it 'can be initialized' do + expect(subject.calculator).to be nil + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:calculator]).to eq ["can't be blank"] + end + end + + context 'with calculator object' do + subject { Calculable.new(calculator: calculator_class.new) } + + it 'can be initialized' do + expect(subject.calculator).to be_a(calculator_class) + expect(calculator_class.count).to eq 0 # not yet saved + end + + it 'can be created' do + subject.save! + expect(subject.calculator).to be_a(calculator_class) + expect(calculator_class.count).to eq 1 # saved in database + end + end + + context 'with calculator_type' do + subject { Calculable.new(calculator_type: calculator_class.to_s) } + + it 'can be initialized' do + expect(subject.calculator).to be_a(calculator_class) + expect(calculator_class.count).to eq 0 # not yet saved + end + + it 'can be created' do + subject.save! + expect(subject.calculator).to be_a(calculator_class) + expect(calculator_class.count).to eq 1 # saved in database + end + end + + context 'with calculator_type and calculator_attributes' do + subject { Calculable.new(calculator_type: calculator_class.to_s, calculator_attributes: {preferred_amount: 123}) } + + it 'can be initialized' do + expect(subject.calculator).to be_a(calculator_class) + expect(subject.calculator.preferred_amount).to eq 123 + expect(calculator_class.count).to eq 0 # not yet saved + end + + it 'can be created' do + subject.save! + expect(subject.calculator).to be_a(calculator_class) + expect(subject.calculator.preferred_amount).to eq 123 + expect(calculator_class.count).to eq 1 # saved in database + end + end + end + + describe 'update' do + subject { Calculable.create!(calculator_type: calculator_class.to_s) } + + it "can update calculator attributes with id" do + subject.update!(calculator_attributes: { + id: subject.calculator.id, + preferred_amount: 123 + }) + expect(subject.calculator.preferred_amount).to eq(123) + subject.reload + expect(subject.calculator.preferred_amount).to eq(123) + end + + it "can update calculator attributes without id" do + subject.update!(calculator_attributes: { + preferred_amount: 123 + }) + expect(subject.calculator.preferred_amount).to eq(123) + subject.reload + expect(subject.calculator.preferred_amount).to eq(123) + end + + it "can update both calculator type and attributes" do + subject.update!(calculator_type: 'Spree::Calculator::FlexiRate', calculator_attributes: { + preferred_first_item: 123 + }) + expect(subject.calculator.class).to eq(Spree::Calculator::FlexiRate) + expect(subject.calculator.preferred_first_item).to eq(123) + subject.reload + expect(subject.calculator.class).to eq(Spree::Calculator::FlexiRate) + expect(subject.calculator.preferred_first_item).to eq(123) + end end end