Merge pull request #11510 from rioug/11421-deactivate-voucher

[Vouchers] add activate/deactivate feature

app/controllers/admin/enterprises_controller.rb

@@ -63,6 +63,7 @@ module Admin
       tag_rules_attributes = params[object_name].delete :tag_rules_attributes
       update_tag_rules(tag_rules_attributes) if tag_rules_attributes.present?
       update_enterprise_notifications
+      update_vouchers
 
       delete_custom_tab if params[:custom_tab] == 'false'
 
@@ -268,6 +269,18 @@ module Admin
       @enterprise.update_contact(user_id)
     end
 
+    def update_vouchers
+      params_voucher_ids = params[:enterprise][:voucher_ids].to_a.map(&:to_i)
+      voucher_ids = @enterprise.vouchers.map(&:id)
+      deleted_voucher_ids = @enterprise.vouchers.only_deleted.map(&:id)
+
+      vouchers_to_destroy = voucher_ids - params_voucher_ids
+      Voucher.where(id: vouchers_to_destroy).destroy_all if vouchers_to_destroy.present?
+
+      vouchers_to_restore = deleted_voucher_ids.intersection(params_voucher_ids)
+      Voucher.restore(vouchers_to_restore) if vouchers_to_restore.present?
+    end
+
     def create_calculator_for(rule, attrs)
       return unless attrs[:calculator_type].present? && attrs[:calculator_attributes].present?
 

app/controllers/admin/vouchers_controller.rb

@@ -22,6 +22,11 @@ module Admin
     rescue ActiveRecord::SubclassNotFound
       @voucher.errors.add(:type)
       render_error
+    rescue ActiveRecord::RecordNotUnique
+      # Rails unique validation doesn't work with soft deleted object, so we rescue the database
+      # exception  to display a nice message to the user
+      @voucher.errors.add(:code, :taken)
+      render_error
     end
 
     private

app/models/voucher.rb

@@ -7,10 +7,12 @@ class Voucher < ApplicationRecord
 
   belongs_to :enterprise, optional: false
 
+  # We want to keep the association with the adjustment when a voucher is "destroyed" as we use
+  # the soft delete functionality to activate/deactivate vouchers
   has_many :adjustments,
            as: :originator,
            class_name: 'Spree::Adjustment',
-           dependent: :nullify
+           dependent: nil
 
   validates :code, presence: true, uniqueness: { scope: :enterprise_id }
 

app/views/admin/enterprises/form/_vouchers.html.haml

@@ -3,12 +3,13 @@
     = t('.add_new')
 %br
 
-- if @enterprise.vouchers.present?
+- if @enterprise.vouchers.with_deleted.present?
   %table
     %thead
       %tr
         %th= t('.voucher_code')
         %th= t('.rate')
+        %th= t('.active')
         /%th= t('.label')
         /%th= t('.purpose')
         /%th= t('.expiry')
@@ -16,17 +17,16 @@
         /%th= t('.customers')
         /%th= t('.net_value')
     %tbody
-      - @enterprise.vouchers.each do |voucher|
+      - @enterprise.vouchers.with_deleted.order(deleted_at: :desc, code: :asc).each do |voucher|
         %tr
           %td= voucher.code
           %td= voucher.display_value  
+          %td= f.check_box :voucher_ids, { :multiple => true }, voucher.id, nil
           /%td
           /%td
           /%td
           /%td
           /%td
-          /%td
-
 - else
   %p.text-center
     = t('.no_voucher_yet')

config/locales/en.yml

@@ -1239,6 +1239,7 @@ en:
           use_limit: Use/Limit
           customers: Customer
           net_value: Net Value
+          active: Active? 
           add_new: Add New
           no_voucher_yet: No Vouchers yet
         white_label:

spec/controllers/admin/enterprises_controller_spec.rb

@@ -281,12 +281,72 @@ describe Admin::EnterprisesController, type: :controller do
                           }
                         }
                       }
-            expect(tag_rule.reload).to be
+
             new_tag_rule = TagRule::FilterOrderCycles.last
             expect(new_tag_rule.preferred_exchange_tags).to eq "tags,are,awesome"
           end
         end
       end
+
+      describe "vouchers" do
+        let(:enterprise) { create(:distributor_enterprise) }
+        let!(:voucher_a) { create(:voucher, enterprise: enterprise, code: "voucher 1") }
+        let(:voucher_b) { create(:voucher, enterprise: enterprise, code: "voucher 2") }
+
+        before do
+          controller_login_as_enterprise_user [enterprise]
+        end
+
+        it "activates checked vouchers" do
+          voucher_b.destroy
+
+          spree_put :update,
+                    id: enterprise,
+                    enterprise: {
+                      voucher_ids: [voucher_a.id.to_s, voucher_b.id.to_s]
+                    }
+
+          expect(voucher_b.reload.deleted?).to be(false)
+        end
+
+        it "deactivates unchecked vouchers" do
+          spree_put :update,
+                    id: enterprise,
+                    enterprise: {
+                      voucher_ids: [voucher_b.id.to_s]
+                    }
+
+          expect(voucher_a.reload.deleted?).to be(true)
+        end
+
+        context "when deactivating the only existing voucher" do
+          it "deactivates the voucher" do
+            # Rails won't populate vouchers_id params if no voucher selected
+            spree_put :update,
+                      id: enterprise,
+                      enterprise: {}
+
+            expect(voucher_a.reload.deleted?).to be(true)
+          end
+        end
+
+        context "when activating and deactivating voucher at the same time" do
+          it "deactivates and activates accordingly" do
+            voucher_b
+            voucher_c = create(:voucher, enterprise: enterprise, code: "voucher 3")
+            voucher_c.destroy
+
+            spree_put :update,
+                      id: enterprise,
+                      enterprise: {
+                        voucher_ids: [voucher_a.id.to_s, voucher_c.id.to_s]
+                      }
+
+            expect(enterprise.vouchers.reload).to include(voucher_c)
+            expect(enterprise.vouchers.reload.only_deleted).to include(voucher_b)
+          end
+        end
+      end
     end
 
     context "as owner" do

spec/models/voucher_spec.rb

@@ -12,7 +12,7 @@ describe Voucher do
 
   describe 'associations' do
     it { is_expected.to belong_to(:enterprise).required }
-    it { is_expected.to have_many(:adjustments) }
+    it { is_expected.to have_many(:adjustments).dependent(nil) }
   end
 
   describe '#code=' do

spec/requests/admin/vouchers_spec.rb

@@ -81,6 +81,20 @@ describe "/admin/enterprises/:enterprise_id/vouchers", type: :request do
       end
     end
 
+    context "with a code used by a deactivated voucher" do
+      before do
+        voucher = create(:voucher, code:, enterprise:)
+        voucher.destroy
+      end
+
+      it "render the new page with a code taken error" do
+        create_voucher
+
+        expect(response).to render_template("admin/vouchers/new")
+        expect(flash[:error]).to eq("Code has already been taken")
+      end
+    end
+
     it "redirects to admin enterprise setting page, voucher panel" do
       create_voucher
 

spec/system/consumer/split_checkout_spec.rb

@@ -1139,16 +1139,29 @@ describe "As a consumer, I want to checkout my order" do
 
         before do
           add_voucher_to_order(voucher, order)
-
-          visit checkout_step_path(:summary)
         end
 
         it "shows the applied voucher" do
+          visit checkout_step_path(:summary)
+
           within ".summary-right" do
             expect(page).to have_content "some_code"
             expect(page).to have_content "-6"
           end
         end
+
+        context "with voucher deactivated after being added to an order" do
+          it "completes the order" do
+            visit checkout_step_path(:summary)
+
+            # Deactivate voucher
+            voucher.destroy
+
+            place_order
+
+            expect(order.reload.state).to eq "complete"
+          end
+        end
       end
     end