Merge pull request #1618 from jhawthorn/smart_calculator

Allow changing calculator types and attributes without reloading page
solidusio · Dec 12, 2016 · 1b77bc4 · 1b77bc4
2 parents 7b4d48b + b2bcc7b
commit 1b77bc4
Show file tree

Hide file tree

Showing 12 changed files with 250 additions and 126 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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/...p/views/spree/admin/promotions/actions/_promotion_calculators_with_custom_fields.html.erb b/...p/views/spree/admin/promotions/actions/_promotion_calculators_with_custom_fields.html.erb
@@ -1,34 +1,33 @@
 <div class="col-xs-12">
-  <div class="calculator-fields row">
+  <div class="calculator-fields js-calculator-fields row">
 
     <div class="col-xs-6">
       <div class="field">
         <% field_name = "#{param_prefix}[calculator_type]" %>
         <%= 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) %>
-          <span class="warning info"><%= Spree.t(:calculator_settings_warning) %></span>
-        <% end %>
+                      :class => 'type-select js-calculator-type select2 fullwidth' %>
       </div>
     </div>
 
-    <% unless promotion_action.new_record? %>
-      <div class="col-xs-6">
-        <div class="settings field">
-          <% 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 %>
-        </div>
+    <div class="col-xs-6">
+      <div class="settings field">
+        <% calculators.each do |calculator_class| %>
+          <% calculator = promotion_action.calculator.class == calculator_class ? promotion_action.calculator : calculator_class.new %>
+          <div class="js-calculator-preferences" data-calculator-type="<%= calculator_class %>">
+            <% 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 %>
+          </div>
+        <% end %>
       </div>
-    <% end %>
+    </div>
   </div>
 </div>
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
@@ -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'
-} %>
-<button class="button js-add-tier"><%= Spree.t('actions.add') %></button>
+<div class="js-tiered-calculator">
+  <%= content_tag :div, nil, class: "js-tiers", data: {
+    'original-tiers' => Hash[calculator.preferred_tiers.sort],
+    'form-prefix' => prefix,
+    'calculator' => 'tiered_flat_rate'
+  } %>
+  <button class="button js-add-tier"><%= Spree.t('actions.add') %></button>
+</div>
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
@@ -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'
-} %>
-<button class="button js-add-tier"><%= Spree.t('actions.add') %></button>
+<div class="js-tiered-calculator">
+  <%= content_tag :div, nil, class: "js-tiers", data: {
+    'original-tiers' => Hash[calculator.preferred_tiers.sort],
+    'form-prefix' => prefix,
+    'calculator' => 'tiered_percent'
+  } %>
+  <button class="button js-add-tier"><%= Spree.t('actions.add') %></button>
+</div>
diff --git 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 @@
-<fieldset id="calculator_fields" data-hook class="no-border-bottom">
+<fieldset id="calculator_fields" class="js-calculator-fields no-border-bottom">
   <legend align="center"><%= Spree::Calculator.model_name.human %></legend>
 
-  <div id="preference-settings" data-hook>
+  <div id="preference-settings">
     <div class="field">
-      <%= 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'}) %>
     </div>
-    <% if !@object.new_record? %>
-      <div class="field">
-        <div class="calculator-settings">
-          <%= f.fields_for :calculator do |calculator_form| %>
-            <%= preference_fields(@object.calculator, calculator_form) %>
-          <% end %>
-        </div>
-        <% if @object.calculator.respond_to?(:preferences) %>
-          <span class="calculator-settings-warning info warning"><%= Spree.t(:calculator_settings_warning) %></span>
+
+    <% @calculators.each do |calculator_class| %>
+      <% calculator = f.object.calculator.class == calculator_class ? f.object.calculator : calculator_class.new %>
+      <div class="js-calculator-preferences" data-calculator-type="<%= calculator_class %>">
+        <%= f.fields_for :calculator, calculator do |calculator_form| %>
+          <%= preference_fields(calculator, calculator_form) %>
         <% end %>
       </div>
     <% end %>

diff --git 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
@@ -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
@@ -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