Remove soft delete from prices

backend/app/controllers/spree/admin/prices_controller.rb

@@ -8,7 +8,7 @@ class PricesController < ResourceController
       def index
         params[:q] ||= {}
 
-        @search = @product.prices.kept.accessible_by(current_ability, :index).ransack(params[:q])
+        @search = @product.prices.accessible_by(current_ability, :index).ransack(params[:q])
         @master_prices = @search.result
           .currently_valid
           .for_master

backend/app/views/spree/admin/prices/_master_variant_table.html.erb

@@ -20,17 +20,17 @@
       </tr>
     </thead>
     <% master_prices.each do |price| %>
-      <tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.discarded? %>">
+      <tr id="<%= spree_dom_id price %>" data-hook="prices_row">
         <td><%= price.display_country %></td>
         <td><%= price.currency %></td>
         <td><%= price.money.to_html %></td>
         <td class="actions">
           <% if can?(:edit, price) %>
-            <%= link_to_edit(price, no_text: true) unless price.discarded? %>
+            <%= link_to_edit(price, no_text: true) %>
           <% end %>
           <% if can?(:destroy, price) %>
             &nbsp;
-            <%= link_to_delete(price, no_text: true) unless price.discarded? %>
+            <%= link_to_delete(price, no_text: true) %>
           <% end %>
         </td>
       </tr>

backend/app/views/spree/admin/prices/_table.html.erb

@@ -14,18 +14,18 @@
     </thead>
     <tbody>
       <% variant_prices.each do |price| %>
-        <tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.discarded? %>">
+        <tr id="<%= spree_dom_id price %>" data-hook="prices_row">
           <td><%= price.variant.descriptive_name %></td>
           <td><%= price.display_country %></td>
           <td><%= price.currency %></td>
           <td><%= price.money.to_html %></td>
           <td class="actions">
             <% if can?(:edit, price) %>
-              <%= link_to_edit(price, no_text: true) unless price.discarded? %>
+              <%= link_to_edit(price, no_text: true) %>
             <% end %>
             <% if can?(:destroy, price) %>
               &nbsp;
-              <%= link_to_delete(price, no_text: true) unless price.discarded? %>
+              <%= link_to_delete(price, no_text: true) %>
             <% end %>
           </td>
         </tr>

backend/spec/controllers/spree/admin/prices_controller_spec.rb

@@ -11,7 +11,7 @@
     context "when only given a product" do
       let(:product) { create(:product) }
 
-      let!(:deleted_master_price) { create(:price, variant: product.master).tap(&:discard!) }
+      let!(:deleted_master_price) { create(:price, variant: product.master).tap(&:destroy!) }
 
       subject { get :index, params: { product_id: product.slug } }
 
@@ -21,7 +21,7 @@
         subject
         expect(assigns(:search)).to be_a(Ransack::Search)
         expect(assigns(:variant_prices)).to be_empty
-        expect(assigns(:master_prices)).to eq(product.prices.kept.for_master)
+        expect(assigns(:master_prices)).to eq(product.prices.for_master)
         expect(assigns(:master_prices)).to_not include(deleted_master_price)
         expect(assigns(:product)).to eq(product)
       end
@@ -31,7 +31,7 @@
       let(:variant) { create(:variant) }
       let(:product) { variant.product }
 
-      let!(:deleted_variant_price) { create(:price, variant:).tap(&:discard!) }
+      let!(:deleted_variant_price) { create(:price, variant:).tap(&:destroy!) }
 
       subject { get :index, params: { product_id: product.slug, variant_id: variant.id } }
 
@@ -40,7 +40,7 @@
       it 'assigns usable instance variables' do
         subject
         expect(assigns(:search)).to be_a(Ransack::Search)
-        expect(assigns(:variant_prices)).to eq(product.prices.kept.for_variant)
+        expect(assigns(:variant_prices)).to eq(product.prices.for_variant)
         expect(assigns(:master_prices)).to eq(product.prices.for_master)
         expect(assigns(:variant_prices)).to include(variant.default_price)
         expect(assigns(:variant_prices)).to_not include(deleted_variant_price)

backend/spec/features/admin/products/edit/products_spec.rb

@@ -61,6 +61,8 @@
 
       click_link 'Prices'
 
+      # The deprecation warning will disappear in Solidus 5
+      expect(Spree.deprecator).to receive(:warn)
       within "#spree_price_#{product.master.default_price.id}" do
         accept_alert do
           click_icon :trash

backend/spec/features/admin/products/pricing_spec.rb

@@ -168,6 +168,9 @@
 
       it "will delete the non-default price" do
         subject
+
+        # The deprecation warning will disappear in Solidus 5
+        expect(Spree.deprecator).to receive(:warn)
         within "#spree_price_#{other_price.id}" do
           accept_alert do
             click_icon :trash
@@ -178,12 +181,16 @@
 
       it "does not break when default price is deleted" do
         subject
+
+        # The deprecation warning will disappear in Solidus 5
+        expect(Spree.deprecator).to receive(:warn)
         within "#spree_price_#{variant.default_price.id}" do
           accept_alert do
             click_icon :trash
           end
         end
         expect(page).to have_content("Price has been successfully removed")
+
         visit spree.admin_products_path
         expect(page).to have_selector("#spree_product_#{product.id}")
       end

core/app/models/concerns/spree/default_price.rb

@@ -41,7 +41,7 @@ def default_price
     end
 
     def has_default_price?
-      default_price.present? && !default_price.discarded?
+      default_price.present?
     end
   end
 end

core/app/models/concerns/spree/hard_deletable.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module Spree
+  # Implements all of the methods that Discard implements, but without a deleted_at column.
+  # Deprecates all usages of Discard methods.
+  module HardDeletable
+    extend ActiveSupport::Concern
+
+    def discarded? = false
+    def undiscarded? = true
+    def kept? = true
+    def deleted_at = nil
+
+    included do
+      def self.kept
+        all
+      end
+
+      def self.discarded
+        none
+      end
+
+      def self.discard_all
+        destroy_all
+      end
+
+      def self.with_discarded
+        all
+      end
+      class << self
+        deprecate :kept, :discarded, :with_discarded, :discard_all, deprecator: Spree.deprecator
+      end
+
+      alias_method :discard, :destroy
+      alias_method :discard!, :destroy
+
+      deprecate :discard, :discard!, deprecator: Spree.deprecator
+      deprecate :discarded?, :undiscarded?, :kept?, :deleted_at, deprecator: Spree.deprecator
+    end
+  end
+end

core/app/models/spree/price.rb

@@ -2,7 +2,7 @@
 
 module Spree
   class Price < Spree::Base
-    include Spree::SoftDeletable
+    include Spree::HardDeletable
 
     MAXIMUM_AMOUNT = BigDecimal('99_999_999.99')
 

core/app/models/spree/variant.rb

@@ -59,7 +59,6 @@ class Variant < Spree::Base
     has_many :images, -> { order(:position) }, as: :viewable, dependent: :destroy, class_name: "Spree::Image"
 
     has_many :prices,
-      -> { with_discarded },
       class_name: 'Spree::Price',
       dependent: :destroy,
       inverse_of: :variant,

core/app/models/spree/variant/price_selector.rb

@@ -39,9 +39,7 @@ def price_for_options(price_options)
       #
       # @return [Array<Spree::Price>]
       def sorted_prices_for(variant)
-        variant.prices.select do |price|
-          variant.discarded? || price.kept?
-        end.sort_by do |price|
+        variant.prices.sort_by do |price|
           [
             price.country_iso.nil? ? 0 : 1,
             price.updated_at || Time.zone.now,

core/db/migrate/20251223220550_remove_soft_deleted_prices.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class RemoveSoftDeletedPrices < ActiveRecord::Migration[7.0]
+  def up
+    discarded_prices = Spree::Price.where.not(deleted_at: nil)
+    affected_variant_ids = discarded_prices.map(&:variant_id).uniq
+    discarded_prices.delete_all
+    Spree::Variant.where(id: affected_variant_ids).find_each(&:touch)
+    remove_column :spree_prices, :deleted_at
+  end
+
+  def down
+    add_column :spree_prices, :deleted_at, :timestamp, null: true
+  end
+end

core/spec/models/spree/hard_deletable_spec.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Spree::HardDeletable do
+  let(:hard_deletable_migration) do
+    Class.new(ActiveRecord::Migration[5.1]) do
+      def change
+        create_table(:hard_deletable_items)
+      end
+    end
+  end
+
+  let(:hard_deletable_item_class) do
+    Class.new(Spree::Base) do
+      include Spree::HardDeletable
+
+      def self.name
+        "HardDeletableItem"
+      end
+    end
+  end
+
+  let(:hard_deletable_item) { hard_deletable_item_class.new }
+
+  around do |example|
+    hard_deletable_migration.migrate(:up)
+    example.run
+    hard_deletable_migration.migrate(:down)
+  end
+
+  before do
+    expect(Spree.deprecator).to receive(:warn)
+  end
+
+  describe ".with_discarded" do
+    it "is deprecated and returns all" do
+      expect(hard_deletable_item_class.with_discarded.to_sql).to eq(hard_deletable_item_class.all.to_sql)
+    end
+  end
+
+  describe ".kept" do
+    it "is deprecated and returns all" do
+      expect(hard_deletable_item_class.kept.to_sql).to eq(hard_deletable_item_class.all.to_sql)
+    end
+  end
+
+  describe ".discarded" do
+    it "is deprecated and returns none" do
+      expect(hard_deletable_item_class.discarded.to_sql).to eq(hard_deletable_item_class.none.to_sql)
+    end
+  end
+
+  describe ".discard_all" do
+    it "is deprecated and calls #destroy_all" do
+      expect(hard_deletable_item_class).to receive(:destroy_all)
+      hard_deletable_item_class.discard_all
+    end
+  end
+
+  describe "#deleted_at" do
+    subject { hard_deletable_item.deleted_at }
+    it { is_expected.to be nil }
+  end
+
+  describe "#discarded?" do
+    subject { hard_deletable_item.discarded? }
+    it { is_expected.to be false }
+  end
+
+  describe "#undiscarded?" do
+    subject { hard_deletable_item.undiscarded? }
+    it { is_expected.to be true }
+  end
+
+  describe "#kept?" do
+    subject { hard_deletable_item.kept? }
+    it { is_expected.to be true }
+  end
+end

core/spec/models/spree/product/scopes_spec.rb

@@ -166,14 +166,6 @@
         end
       end
 
-      context "with soft-deleted master price" do
-        before { product.master.prices.discard_all }
-
-        it "doesn't include the product" do
-          expect(Spree::Product.available).to match_array([])
-        end
-      end
-
       context "with multiple prices" do
         let!(:second_price) { create(:price, variant: product.master) }
 

core/spec/models/spree/variant_spec.rb

@@ -350,18 +350,17 @@
       expect(variant.default_price.attributes).to eq(price.attributes)
     end
 
-    context "when the variant and the price are both soft-deleted" do
+    context "when the variant is soft-deleted" do
       it "will use a deleted price as the default price" do
         variant = create(:variant, deleted_at: 1.day.ago)
-        variant.prices.each { |price| price.discard }
-        expect(variant.reload.price).to be_present
+        expect(variant.price).to be_present
       end
     end
 
     context "when the variant is not soft-deleted, but its price is" do
       it "will not use a deleted price as the default price" do
         variant = create(:variant)
-        variant.prices.each { |price| price.discard }
+        variant.prices.each { |price| price.destroy }
         expect(variant.reload.price).not_to be_present
       end
     end
@@ -397,13 +396,13 @@
       end
     end
 
-    context 'when default price is discarded' do
+    context 'when default price is destroyed' do
       it 'returns false' do
         variant = create(:variant, price: 20)
 
-        variant.default_price.discard
+        variant.default_price.destroy
 
-        expect(variant.has_default_price?).to be(false)
+        expect(variant.reload.has_default_price?).to be(false)
       end
     end
   end
@@ -873,7 +872,7 @@
       it "should not discard default_price" do
         variant.discard
         variant.reload
-        expect(previous_variant_price.reload).not_to be_discarded
+        expect(previous_variant_price.reload).to be_present
       end
 
       it "should keep its price if deleted" do