Fix promotions admin UI inconsistency

promotions/app/models/solidus_promotions/promotion.rb

@@ -138,6 +138,18 @@ def eligibility_results
       @eligibility_results ||= SolidusPromotions::EligibilityResults.new(self)
     end
 
+    def can_change_apply_automatically?
+      path.blank? && codes.empty?
+    end
+
+    def can_change_path?
+      !apply_automatically? && codes.empty?
+    end
+
+    def can_change_codes?
+      !apply_automatically? && path.blank?
+    end
+
     private
 
     def normalize_blank_values

promotions/config/locales/en.yml

@@ -167,6 +167,10 @@ en:
           general: General
           starts_at_placeholder: Immediately
           codes_present: This promotion has promotion codes defined. You cannot select the apply automatically option.
+          automatic: "Automatic"
+          path: "Path"
+          single_code: "Single code"
+          multiple_codes: "Multiple codes"
         edit:
           order_conditions: Order Conditions
         calculator:

promotions/lib/controllers/backend/solidus_promotions/admin/promotions_controller.rb

@@ -11,8 +11,7 @@ class PromotionsController < BaseController
 
       def create
         @promotion = model_class.new(permitted_resource_params)
-        @promotion.codes.new(value: params[:single_code]) if params[:single_code].present?
-
+        @promotion.codes.new(promotion: @promotion, value: params[:single_code]) if params[:single_code].present?
         if params[:code_batch]
           @code_batch = @promotion.code_batches.new(code_batch_params)
         end

promotions/lib/views/backend/solidus_promotions/admin/promotion_code_batches/_form_fields.html.erb

@@ -1,22 +1,22 @@
 <div class="form-group">
   <%= batch.label :base_code, class: "required" %>
-  <%= batch.text_field :base_code, class: "fullwidth", required: true %>
+  <%= batch.text_field :base_code, class: "fullwidth #{'enable' if promotion.can_change_codes?}", required: true, disabled: disabled %>
 </div>
 <div class="form-group">
   <%= batch.label :number_of_codes, class: "required" %>
-  <%= batch.number_field :number_of_codes, class: "fullwidth", min: 1, required: true %>
+  <%= batch.number_field :number_of_codes, class: "fullwidth #{'enable' if promotion.can_change_codes?}", min: 1, required: true, disabled: disabled %>
 </div>
 <div class="form-group">
   <%= batch.label :join_characters %>
-  <%= batch.text_field :join_characters, class: "fullwidth" %>
+  <%= batch.text_field :join_characters, class: "fullwidth #{'enable' if promotion.can_change_codes?}", disabled: disabled %>
 </div>
 <% unless promotion_id %>
   <div class="form-group">
     <%= f.label :per_code_usage_limit %>
-    <%= f.text_field :per_code_usage_limit, class: "fullwidth" %>
+    <%= f.text_field :per_code_usage_limit, class: "fullwidth #{'enable' if promotion.can_change_codes?}", disabled: disabled %>
   </div>
 <% end %>
 <div class="form-group">
   <%= batch.label :email %>
-  <%= batch.text_field :email, class: "fullwidth" %>
+  <%= batch.text_field :email, class: "fullwidth #{'enable' if promotion.can_change_codes?}", disabled: disabled %>
 </div>

promotions/lib/views/backend/solidus_promotions/admin/promotion_code_batches/new.html.erb

@@ -3,6 +3,6 @@
 <% admin_breadcrumb(plural_resource_name(SolidusPromotions::PromotionCodeBatch)) %>
 <%= form_for :promotion_code_batch, url: collection_url do |batch| %>
   <%= batch.hidden_field :promotion_id, value: params[:promotion_id] %>
-  <%= render partial: 'form_fields', locals: {batch: batch, promotion_id: params[:promotion_id]} %>
+  <%= render partial: 'form_fields', locals: {batch: batch, promotion: @promotion, disabled: false, promotion_id: params[:promotion_id]} %>
   <%= batch.submit t('spree.actions.create'), class: 'btn btn-primary' %>
 <% end %>

promotions/lib/views/backend/solidus_promotions/admin/promotions/_form.html.erb

@@ -85,40 +85,81 @@
   </div>
 </fieldset>
 
-<fieldset class="form-group row no-border-bottom">
+<fieldset class="no-border-bottom">
   <legend><%= t '.activation' %></legend>
-
-  <div class="col-4">
-    <%= f.field_container :apply_automatically do %>
-      <%= f.label :apply_automatically do %>
-        <%= f.check_box :apply_automatically, disabled: f.object.codes.any? || f.object.path.present? %>
-        <%= SolidusPromotions::Promotion.human_attribute_name(:apply_automatically) %>
-        <%= f.field_hint :promo_code_will_be_disabled %>
-      <% end %>
-    <% end %>
-  </div>
-
-  <% if f.object.new_record? || f.object.present? %>
-    <div class="col-4">
-      <%= f.field_container :path do %>
-        <%= f.label :path %>
-        <%= f.text_field :path, class: "fullwidth", disabled: f.object.apply_automatically || f.object.codes.present? %>
-      <% end %>
+  <div class="row">
+    <div class="col-3">
+      <div class="nav flex-column nav-pills" id="v-pills-tab" role="tablist" aria-orientation="vertical">
+        <button class="nav-link mb-1 active" id="v-pills-automatic-tab" data-toggle="pill" data-target="#v-pills-automatic" type="button" role="tab" aria-controls="v-pills-automatic" aria-selected="true">
+          <%= t(".automatic") %>
+        </button>
+        <button class="nav-link mb-1" id="v-pills-path-tab" data-toggle="pill" data-target="#v-pills-path" type="button" role="tab" aria-controls="v-pills-path" aria-selected="false">
+          <%= t(".path") %>
+        </button>
+        <button class="nav-link mb-1" id="v-pills-single-code-tab" data-toggle="pill" data-target="#v-pills-single-code" type="button" role="tab" aria-controls="v-pills-single-code" aria-selected="false">
+          <%= t(".single_code") %>
+        </button>
+        <button class="nav-link mb-1" id="v-pills-multiple-codes-tab" data-toggle="pill" data-target="#v-pills-multiple-codes" type="button" role="tab" aria-controls="v-pills-multiple-codes" aria-selected="false">
+          <%= t(".multiple_codes") %>
+        </button>
+      </div>
     </div>
-  <% end %>
-
-    <div class="col-4">
-      <% if f.object.new_record? %>
-        <div id="promotion_single_code" class="form-group">
-          <%= label_tag :single_code, SolidusPromotions::PromotionCode.model_name.human %>
-          <%= text_field_tag :single_code, @promotion.codes.first.try!(:value), class: "fullwidth", disabled: f.object.apply_automatically || f.object.path.present? %>
+    <div class="col-9">
+      <div class="tab-content" id="v-pills-tabContent">
+        <div class="tab-pane fade show active" id="v-pills-automatic" role="tabpanel" aria-labelledby="v-pills-automatic-tab">
+          <%= f.field_container :apply_automatically do %>
+            <%= f.label :apply_automatically do %>
+              <%= f.check_box :apply_automatically, {disabled: !f.object.can_change_apply_automatically?, class: "#{'enable' if f.object.can_change_apply_automatically?}"} %>
+              <%= SolidusPromotions::Promotion.human_attribute_name(:apply_automatically) %>
+              <%= f.field_hint :promo_code_will_be_disabled %>
+            <% end %>
+            <% if f.object.codes.present? %>
+              <div class="codes-present">
+                <p>
+                  <%= t('.codes_present') %>
+                </p>
+              </div>
+            <% end %>
+          <% end %>
         </div>
-      <% else %>
-        <div class="codes-present">
-          <p>
-            <%= t('.codes_present') %>
-          </p>
+        <div class="tab-pane fade" id="v-pills-path" role="tabpanel" aria-labelledby="v-pills-path-tab">
+          <%= f.field_container :path do %>
+            <%= f.label :path %>
+            <%= f.text_field :path, class: "fullwidth #{'enable' if f.object.can_change_path?}", disabled: true %>
+          <% end %>
         </div>
-      <% end %>
+        <div class="tab-pane fade" id="v-pills-single-code" role="tabpanel" aria-labelledby="v-pills-single-code-tab">
+          <%= f.field_container :single_code do%>
+            <%= label_tag :single_code, SolidusPromotions::PromotionCode.model_name.human %>
+            <%= text_field_tag :single_code, @promotion.codes.first.try!(:value), class: "fullwidth #{'enable' if f.object.can_change_codes?}", disabled: true %>
+          <% end %>
+        </div>
+        <div class="tab-pane fade" id="v-pills-multiple-codes" role="tabpanel" aria-labelledby="v-pills-multiple-codes-tab">
+          <%= fields_for :promotion_code_batch, @promotion.code_batches.new do |batch| %>
+            <%= render partial: 'solidus_promotions/admin/promotion_code_batches/form_fields', locals: {f: f, batch: batch, promotion: @promotion, disabled: true, promotion_id: params[:promotion_id]} %>
+          <% end %>
+        </div>
+      </div>
     </div>
+  </div>
 </fieldset>
+
+<script>
+  function disableInactiveTabFields(event) {
+    const panes = document.querySelectorAll(".tab-pane");
+    panes.forEach(pane => {
+      if (pane.id == event.target.dataset.target.slice(1)) {
+        pane.querySelectorAll("input.enable, select.enable").forEach(input => {
+          input.removeAttribute("disabled");
+        });
+      } else {
+        pane.querySelectorAll("input, select").forEach(input => {
+          input.setAttribute("disabled", true);
+        });
+      }
+    });
+  }
+  document.addEventListener("DOMContentLoaded", function() {
+    $(".nav-link").on("show.bs.tab", disableInactiveTabFields);
+  });
+</script>

promotions/spec/models/solidus_promotions/promotion_spec.rb

@@ -686,4 +686,82 @@
       expect(subject).to be_nil
     end
   end
+
+  describe "#can_change_apply_automatically?" do
+    subject { promotion.can_change_apply_automatically? }
+
+    let(:promotion) { create :solidus_promotion }
+
+    context "when the promotion has a path" do
+      before { promotion.path = "foo" }
+
+      it { is_expected.to be false }
+    end
+
+    context "when the promotion has a code" do
+      before { promotion.codes.new(value: "foo") }
+
+      it { is_expected.to be false }
+    end
+
+    context "when the promotion has neither a path nor a code" do
+      it { is_expected.to be true }
+    end
+  end
+
+  describe "#can_change_path?" do
+    subject { promotion.can_change_path? }
+
+    let(:promotion) { create :solidus_promotion }
+
+    context "when the promotion has a code" do
+      before { promotion.codes.new(value: "foo") }
+
+      it { is_expected.to be false }
+    end
+
+    context "when the promotion has a path" do
+      before { promotion.path = "foo" }
+
+      it { is_expected.to be true }
+    end
+
+    context "when the promotion has neither a path nor a code" do
+      it { is_expected.to be true }
+    end
+
+    context "when the promotion applies automatically" do
+      before { promotion.apply_automatically = true }
+
+      it { is_expected.to be false }
+    end
+  end
+
+  describe "#can_change_codes?" do
+    subject { promotion.can_change_codes? }
+
+    let(:promotion) { create :solidus_promotion }
+
+    context "when the promotion has a code" do
+      before { promotion.codes.new(value: "foo") }
+
+      it { is_expected.to be true }
+    end
+
+    context "when the promotion has a path" do
+      before { promotion.path = "foo" }
+
+      it { is_expected.to be false }
+    end
+
+    context "when the promotion has neither a path nor a code" do
+      it { is_expected.to be true }
+    end
+
+    context "when the promotion applies automatically" do
+      before { promotion.apply_automatically = true }
+
+      it { is_expected.to be false }
+    end
+  end
 end