Merge pull request #11372 from Matt-Yorkley/remove-variant-position

Remove unused variant :position attribute

app/models/spree/product.rb

@@ -39,12 +39,10 @@ module Spree
 
     has_many :product_properties, dependent: :destroy
     has_many :properties, through: :product_properties
-    has_many :variants, -> { order("spree_variants.position ASC") }, class_name: 'Spree::Variant',
-                                                                     dependent: :destroy
+    has_many :variants, -> { order("spree_variants.id ASC") }, class_name: 'Spree::Variant',
+                                                               dependent: :destroy
 
-    has_many :prices, -> {
-      order('spree_variants.position, spree_variants.id, currency')
-    }, through: :variants
+    has_many :prices, -> { order('spree_variants.id, currency') }, through: :variants
 
     has_many :stock_items, through: :variants
     has_many :supplier_properties, through: :supplier, source: :properties

app/models/spree/variant.rb

@@ -86,7 +86,6 @@ module Spree
     }
 
     after_create :create_stock_items
-    after_create :set_position
     around_destroy :destruction
     after_save :save_default_price
 
@@ -231,10 +230,6 @@ module Spree
       end
     end
 
-    def set_position
-      update_column(:position, product.variants.maximum(:position).to_i + 1)
-    end
-
     def update_weight_from_unit_value
       return unless product.variant_unit == 'weight' && unit_value.present?
 

db/migrate/20230809172206_remove_variant_position.rb

@@ -0,0 +1,5 @@
+class RemoveVariantPosition < ActiveRecord::Migration[7.0]
+  def up
+    remove_column :spree_variants, :position
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_08_07_145022) do
+ActiveRecord::Schema[7.0].define(version: 2023_08_09_172206) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
   enable_extension "plpgsql"
@@ -1035,7 +1035,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_08_07_145022) do
     t.datetime "deleted_at", precision: nil
     t.boolean "is_master", default: false
     t.integer "product_id"
-    t.integer "position"
     t.string "cost_currency", limit: 255
     t.float "unit_value"
     t.string "unit_description", limit: 255, default: ""

spec/models/spree/product_spec.rb

@@ -32,10 +32,8 @@ module Spree
       end
 
       describe 'Variants sorting' do
-        context 'without master variant' do
-          it 'sorts variants by position' do
-            expect(product.variants.to_sql).to match(/ORDER BY spree_variants.position ASC/)
-          end
+        it 'sorts variants by id' do
+          expect(product.variants.to_sql).to match(/ORDER BY spree_variants.id ASC/)
         end
       end
     end

spec/models/spree/variant_spec.rb

@@ -168,14 +168,6 @@ describe Spree::Variant do
     end
   end
 
-  # Regression test for #2744
-  describe "set_position" do
-    it "sets variant position after creation" do
-      variant = create(:variant)
-      expect(variant.position).to_not be_nil
-    end
-  end
-
   describe '#in_stock?' do
     # Stock data can only be stored against a persisted variant.
     subject(:variant) { create(:variant) }