Improve soft-deletion faculty

app/controllers/solidus_friendly_promotions/admin/promotion_actions_controller.rb

@@ -53,7 +53,7 @@ def update
 
       def destroy
         @promotion_action = @promotion.actions.find(params[:id])
-        if @promotion_action.discard
+        if @promotion_action.destroy
           flash[:success] =
             t("spree.successfully_removed", resource: SolidusFriendlyPromotions::PromotionAction.model_name.human)
         end

app/models/solidus_friendly_promotions/promotion.rb

@@ -2,11 +2,13 @@
 
 module SolidusFriendlyPromotions
   class Promotion < Spree::Base
+    include Spree::SoftDeletable
+
     belongs_to :category, class_name: "SolidusFriendlyPromotions::PromotionCategory",
       foreign_key: :promotion_category_id, optional: true
     belongs_to :original_promotion, class_name: "Spree::Promotion", optional: true
     has_many :rules, class_name: "SolidusFriendlyPromotions::PromotionRule", dependent: :destroy
-    has_many :actions, class_name: "SolidusFriendlyPromotions::PromotionAction", dependent: :nullify
+    has_many :actions, class_name: "SolidusFriendlyPromotions::PromotionAction", dependent: :destroy
     has_many :codes, class_name: "SolidusFriendlyPromotions::PromotionCode", dependent: :destroy
     has_many :code_batches, class_name: "SolidusFriendlyPromotions::PromotionCodeBatch", dependent: :destroy
     has_many :order_promotions, class_name: "SolidusFriendlyPromotions::OrderPromotion", dependent: :destroy
@@ -49,7 +51,7 @@ def self.ordered_lanes
 
     self.allowed_ransackable_associations = ["codes"]
     self.allowed_ransackable_attributes = %w[name customer_label path promotion_category_id lane updated_at]
-    self.allowed_ransackable_scopes = %i[active]
+    self.allowed_ransackable_scopes = %i[active with_discarded]
 
     # All orders that have been discounted using this promotion
     def discounted_orders

app/models/solidus_friendly_promotions/promotion_action.rb

@@ -9,9 +9,10 @@ module SolidusFriendlyPromotions
   # by an event and determined to be eligible.
   class PromotionAction < Spree::Base
     include Spree::Preferences::Persistable
-    include Spree::SoftDeletable
     include Spree::CalculatedAdjustments
     include Spree::AdjustmentSource
+    before_destroy :remove_adjustments_from_incomplete_orders
+    before_destroy :raise_for_adjustments_for_completed_orders
 
     belongs_to :promotion, inverse_of: :actions
     belongs_to :original_promotion_action, class_name: "Spree::PromotionAction", optional: true
@@ -69,5 +70,14 @@ def relevant_rules
     def available_calculators
       SolidusFriendlyPromotions.config.promotion_calculators[self.class] || (raise NotImplementedError)
     end
+
+    private
+
+    def raise_for_adjustments_for_completed_orders
+      if adjustments.joins(:order).merge(Spree::Order.complete).any?
+        errors.add(:base, :cannot_destroy_if_order_completed)
+        throw(:abort)
+      end
+    end
   end
 end

app/views/solidus_friendly_promotions/admin/promotions/_table.html.erb

@@ -14,7 +14,7 @@
   </thead>
   <tbody>
     <% promotions.each do |promotion| %>
-      <tr id="<%= spree_dom_id promotion %>">
+      <tr class="<%= 'deleted' if promotion.discarded? %>" id="<%= spree_dom_id promotion %>">
         <td><%= promotion.name %></td>
         <td>
           <%= (promotion.codes.size == 1) ? promotion.codes.pluck(:value).first : t('solidus_friendly_promotions.number_of_codes', count: promotion.codes.size) %>
@@ -43,10 +43,10 @@
           <%= l(promotion.updated_at, format: :short) %>
         </td>
         <td class="actions">
-          <% if can?(:edit, promotion) %>
+          <% if can?(:edit, promotion) && !promotion.discarded? %>
             <%= link_to_edit promotion, no_text: true %>
           <% end %>
-          <% if can?(:destroy, promotion) %>
+          <% if can?(:destroy, promotion) && !promotion.discarded? %>
             <%= link_to_delete promotion, no_text: true %>
           <% end %>
         </td>

app/views/solidus_friendly_promotions/admin/promotions/_table_filter.html.erb

@@ -43,6 +43,15 @@
           </div>
         </div>
 
+        <div class="col-2">
+          <div class="field checkbox">
+            <label>
+              <%= f.check_box :with_discarded, { checked: params[:q][:with_discarded] == 'true', class: 'js-with-discarded-input' }, 'true', 'false' %>
+              <%= t('spree.show_deleted') %>
+            </label>
+          </div>
+        </div>
+
         <div class="col-2">
           <div class="field">
             <%= label_tag :q_promotion_category_id_eq, SolidusFriendlyPromotions::PromotionCategory.model_name.human %><br>

bin/sandbox

@@ -65,6 +65,7 @@ unbundled bundle exec rake db:drop db:create
 
 unbundled bin/rails generate solidus:install \
   --auto-accept \
+  --admin_preview=false \
   $@
 
 unbundled bundle exec rails generate solidus:auth:install --auto-run-migrations

config/locales/en.yml

@@ -314,6 +314,10 @@ en:
 
     errors:
       models:
+        solidus_friendly_promotions/promotion_action:
+          attributes:
+            base:
+              cannot_destroy_if_order_completed: Action has been applied to complete orders. It cannot be destroyed.
         solidus_friendly_promotions/promotion_code:
           attributes:
             base:

db/migrate/20240124104855_add_deleted_at_to_promotions.rb

@@ -0,0 +1,6 @@
+class AddDeletedAtToPromotions < ActiveRecord::Migration[7.0]
+  def change
+    add_column :friendly_promotions, :deleted_at, :datetime
+    add_index :friendly_promotions, :deleted_at
+  end
+end

db/migrate/20240125102050_drop_deleted_at_from_promotion_actions.rb

@@ -0,0 +1,5 @@
+class DropDeletedAtFromPromotionActions < ActiveRecord::Migration[7.0]
+  def change
+    remove_column :friendly_promotion_actions, :deleted_at, :datetime, null: true
+  end
+end

spec/models/solidus_friendly_promotions/friendly_promotion_adjuster/load_promotions_spec.rb

@@ -43,6 +43,25 @@
         expect(subject).to eq([active_promotion])
       end
     end
+
+    context "discarded promotions" do
+      let!(:discarded_promotion) { create(:friendly_promotion, :with_adjustable_action, deleted_at: 1.hour.ago, apply_automatically: true) }
+
+      it "does not check discarded promotions" do
+        expect(subject).not_to include(discarded_promotion)
+      end
+
+      context "a discarded promo is connected to the order" do
+        before do
+          order.friendly_promotions << discarded_promotion
+        end
+
+        it "does not check connected discarded promotions" do
+          expect(subject).not_to include(discarded_promotion)
+          expect(subject).to eq([active_promotion])
+        end
+      end
+    end
   end
 
   context "promotions in the past" do

spec/models/solidus_friendly_promotions/promotion_action_spec.rb

@@ -17,6 +17,46 @@
     end
   end
 
+  describe "#destroy" do
+    subject { action.destroy }
+    let(:action) { promotion.actions.first }
+    let!(:promotion) { create(:friendly_promotion, :with_adjustable_action, apply_automatically: true) }
+
+    it "destroys the action" do
+      expect { subject }.to change { SolidusFriendlyPromotions::PromotionAction.count }.by(-1)
+    end
+
+    context "when the action has adjustments on an incomplete order" do
+      let(:order) { create(:order_with_line_items) }
+
+      before do
+        order.recalculate
+      end
+
+      it "destroys the action" do
+        expect { subject }.to change { SolidusFriendlyPromotions::PromotionAction.count }.by(-1)
+      end
+
+      it "destroys the adjustments" do
+        expect { subject }.to change { Spree::Adjustment.count }.by(-1)
+      end
+
+      context "when the action has adjustments on a complete order" do
+        let(:order) { create(:order_ready_to_complete) }
+
+        before do
+          order.recalculate
+          order.complete!
+        end
+
+        it "raises an error" do
+          expect { subject }.not_to change { SolidusFriendlyPromotions::PromotionAction.count }
+          expect(action.errors.full_messages).to include("Action has been applied to complete orders. It cannot be destroyed.")
+        end
+      end
+    end
+  end
+
   describe "#discount" do
     subject { action.discount(discountable) }
 

spec/models/solidus_friendly_promotions/promotion_spec.rb

@@ -20,14 +20,26 @@
   end
 
   describe "#destroy" do
-    let!(:promotion) { create(:friendly_promotion, :with_adjustable_action) }
+    let!(:promotion) { create(:friendly_promotion, :with_adjustable_action, apply_automatically: true) }
 
     subject { promotion.destroy! }
 
     it "destroys the promotion and nullifies the action" do
       expect { subject }.to change { SolidusFriendlyPromotions::Promotion.count }.by(-1)
-      expect(SolidusFriendlyPromotions::PromotionAction.count).to eq(1)
-      expect(SolidusFriendlyPromotions::PromotionAction.first.promotion_id).to be nil
+      expect(SolidusFriendlyPromotions::PromotionAction.count).to be_zero
+    end
+
+    context "when the promotion has been applied to a complete order" do
+      let(:order) { create(:order_ready_to_complete) }
+
+      before do
+        order.recalculate
+        order.complete!
+      end
+
+      it "raises an error" do
+        expect { subject }.to raise_exception(ActiveRecord::RecordNotDestroyed)
+      end
     end
   end