Enable master variants with associated order cycles

There's only 5 in UK prod. keeping them is easier than figuring out if it's safe to delete.

(cherry picked from commit 4bf65e330b)

app/assets/javascripts/admin/order_cycles/services/enterprise.js.coffee

@@ -33,6 +33,4 @@ angular.module('admin.orderCycles').factory('Enterprise', ($resource) ->
     variantsOf: (product) ->
       if product.variants.length > 0
         variant.id for variant in product.variants
-      else
-        [product.master_id]
   })

app/assets/javascripts/admin/spree/images/index.js.coffee

@@ -1,15 +0,0 @@
-$ ->
-  ($ '#new_image_link').click (event) ->
-    event.preventDefault()
-
-    ($ '.no-objects-found').hide()
-
-    ($ this).hide()
-    $.ajax
-      type: 'GET'
-      url: @href
-      data: (
-        authenticity_token: AUTH_TOKEN
-      )
-      success: (r) ->
-        ($ '#images').html r

app/assets/javascripts/admin/spree/images/new.js.coffee

@@ -1,7 +0,0 @@
-($ '#cancel_link').click (event) ->
-  event.preventDefault()
-
-  ($ '.no-objects-found').show()
-
-  ($ '#new_image_link').show()
-  ($ '#images').html('')

app/assets/javascripts/darkswarm/services/products.js.coffee

@@ -33,9 +33,9 @@ angular.module('Darkswarm').factory 'Products', (OrderCycleResource, OrderCycle,
           prices = (v.price for v in product.variants)
           product.price = Math.min.apply(null, prices)
         product.hasVariants = product.variants?.length > 0
-        product.primaryImage = product.images[0]?.small_url if product.images
+        product.primaryImage = product.image?.small_url if product.image
         product.primaryImageOrMissing = product.primaryImage || "/noimage/small.png"
-        product.largeImage = product.images[0]?.large_url if product.images
+        product.largeImage = product.image?.large_url if product.image
 
     dereference: ->
       for product in @fetched_products

app/components/product_component.rb

@@ -6,7 +6,7 @@ class ProductComponent < ViewComponentReflex::Component
   def initialize(product:, columns:)
     super
     @product = product
-    @image = @product.images[0] if product.images.any?
+    @image = @product.image if product.image.present?
     @columns = columns.map do |c|
       {
         id: c[:value],
@@ -28,7 +28,7 @@ class ProductComponent < ViewComponentReflex::Component
     when 'price'
       @product.price
     when 'unit'
-      "#{@product.unit_value} #{@product.variant_unit}"
+      "#{@product.variants.first.unit_value} #{@product.variant_unit}"
     when 'producer'
       @product.supplier.name
     when 'category'

app/components/products_table_component.rb

@@ -168,7 +168,7 @@ class ProductsTableComponent < ViewComponentReflex::Component
 
   def product_query_includes
     [
-      master: [:images],
+      :image,
       variants: [
         :default_price,
         :stock_locations,

app/controllers/admin/enterprises_controller.rb

@@ -188,7 +188,7 @@ module Admin
         if enterprises.present?
           enterprises.includes(
             supplied_products:
-              [:supplier, :variants, { master: [:images] }]
+              [:supplier, :variants, :image]
           )
         end
       when :index

app/controllers/api/v0/product_images_controller.rb

@@ -9,9 +9,9 @@ module Api
         product = Spree::Product.find(params[:product_id])
         authorize! :update, product
 
-        image = product.images.first || Spree::Image.new(
+        image = product.image || Spree::Image.new(
-          viewable_id: product.master.id,
+          viewable_id: product.id,
-          viewable_type: 'Spree::Variant'
+          viewable_type: 'Spree::Product'
         )
 
         success_status = image.persisted? ? :ok : :created

app/controllers/api/v0/products_controller.rb

@@ -116,7 +116,7 @@ module Api
 
       def product_query_includes
         [
-          master: { images: { attachment_attachment: :blob } },
+          image: { attachment_attachment: :blob },
           variants: [:default_price, :stock_locations, :stock_items, :variant_overrides]
         ]
       end

app/controllers/api/v0/variants_controller.rb

@@ -56,9 +56,9 @@ module Api
       def scope
         if @product
           variants = if current_api_user.has_spree_role?("admin") || params[:show_deleted]
-                       @product.variants_including_master.with_deleted
+                       @product.variants.with_deleted
                      else
-                       @product.variants_including_master
+                       @product.variants
                      end
         else
           variants = Spree::Variant.where(nil)

app/controllers/spree/admin/images_controller.rb

@@ -25,6 +25,7 @@ module Spree
         set_viewable
 
         @object.attributes = permitted_resource_params
+
         if @object.save
           flash[:success] = flash_message_for(@object, :successfully_created)
           redirect_to spree.admin_product_images_url(params[:product_id], @url_filters)
@@ -62,20 +63,28 @@ module Spree
 
       private
 
+      def collection
+        parent.image
+      end
+
+      def find_resource
+        parent.image
+      end
+
+      def build_resource
+        Spree::Image.new(viewable: parent)
+      end
+
       def location_after_save
         spree.admin_product_images_url(@product)
       end
 
       def load_data
         @product = Product.find_by(permalink: params[:product_id])
-        @variants = @product.variants.collect do |variant|
-          [variant.options_text, variant.id]
-        end
-        @variants.insert(0, [Spree.t(:all), @product.master.id])
       end
 
       def set_viewable
-        @image.viewable_type = 'Spree::Variant'
+        @image.viewable_type = 'Spree::Product'
         @image.viewable_id = params[:image][:viewable_id]
       end
 

app/controllers/spree/admin/products_controller.rb

@@ -121,8 +121,7 @@ module Spree
       end
 
       def product_includes
-        [{ variants: [:images] },
+        [:image, { variants: [:images] }]
-         { master: [:images, :default_price] }]
       end
 
       def collection_actions
@@ -182,7 +181,6 @@ module Spree
           joins(:product).
           where('spree_products.supplier_id IN (?)', editable_enterprises.collect(&:id)).
           where('spree_variants.import_date IS NOT NULL').
-          where(spree_variants: { is_master: false }).
           where(spree_variants: { deleted_at: nil }).
           order('spree_variants.import_date DESC')
       end
@@ -208,7 +206,7 @@ module Spree
       end
 
       def set_stock_levels(product, on_hand, on_demand)
-        variant = product_variant(product)
+        variant = product.variants.first
 
         begin
           variant.on_demand = on_demand if on_demand.present?
@@ -228,14 +226,6 @@ module Spree
         end
       end
 
-      def product_variant(product)
-        if product.variants.any?
-          product.variants.first
-        else
-          product.master
-        end
-      end
-
       def set_product_master_variant_price_to_zero
         @product.price = 0 if @product.price.nil?
       end

app/controllers/spree/admin/variants_controller.rb

@@ -7,8 +7,6 @@ module Spree
     class VariantsController < ::Admin::ResourceController
       belongs_to 'spree/product', find_by: :permalink
 
-      before_action :assign_default_attributes, only: :new
-
       def index
         @url_filters = ::ProductFilters.new.extract(request.query_parameters)
       end
@@ -45,6 +43,7 @@ module Spree
           flash[:success] = flash_message_for(@object, :successfully_created)
           redirect_to spree.admin_product_variants_url(params[:product_id], @url_filters)
         else
+          flash[:error] = @object.errors.full_messages.to_sentence if @object.errors.any?
           redirect_to spree.new_admin_product_variant_url(params[:product_id], @url_filters)
         end
 
@@ -83,13 +82,6 @@ module Spree
         @object.save
       end
 
-      def assign_default_attributes
-        @object.attributes = @object.product.master.
-          attributes.except('id', 'created_at', 'deleted_at', 'sku', 'is_master')
-        # Shallow Clone of the default price to populate the price field.
-        @object.default_price = @object.product.master.default_price.clone
-      end
-
       def collection
         @deleted = params.key?(:deleted) && params[:deleted] == "on" ? "checked" : ""
 

app/helpers/spree/admin/images_helper.rb

@@ -5,11 +5,7 @@ module Spree
     module ImagesHelper
       def options_text_for(image)
         if image.viewable.is_a?(Spree::Variant)
-          if image.viewable.is_master?
+          image.viewable.options_text
-            I18n.t(:all)
-          else
-            image.viewable.options_text
-          end
         else
           I18n.t(:all)
         end

app/models/enterprise.rb

@@ -160,7 +160,7 @@ class Enterprise < ApplicationRecord
   scope :is_distributor, -> { where('sells != ?', 'none') }
   scope :is_hub, -> { where(sells: 'any') }
   scope :supplying_variant_in, lambda { |variants|
-    joins(supplied_products: :variants_including_master).
+    joins(supplied_products: :variants).
       where('spree_variants.id IN (?)', variants).
       select('DISTINCT enterprises.*')
   }
@@ -389,7 +389,7 @@ class Enterprise < ApplicationRecord
   def current_distributed_taxons
     Spree::Taxon
       .select("DISTINCT spree_taxons.*")
-      .joins(products: :variants_including_master)
+      .joins(products: :variants)
       .joins("INNER JOIN (#{current_exchange_variants.to_sql}) \
         AS exchange_variants ON spree_variants.id = exchange_variants.variant_id")
   end

app/models/exchange.rb

@@ -55,7 +55,7 @@ class Exchange < ApplicationRecord
   }
   scope :with_product, lambda { |product|
     joins(:exchange_variants).
-      where('exchange_variants.variant_id IN (?)', product.variants_including_master.select(&:id))
+      where('exchange_variants.variant_id IN (?)', product.variants.select(&:id))
   }
   scope :by_enterprise_name, -> {
     joins('INNER JOIN enterprises AS sender   ON (sender.id   = exchanges.sender_id)').

app/models/product_import/entry_processor.rb

@@ -55,7 +55,6 @@ module ProductImport
             VariantOverride.for_hubs([enterprise_id]).count
           else
             Spree::Variant.
-              not_master.
               joins(:product).
               where('spree_products.supplier_id IN (?)', enterprise_id).
               count

app/models/spree/product.rb

@@ -28,42 +28,73 @@ module Spree
 
     acts_as_paranoid
 
-    searchable_attributes :supplier_id, :primary_taxon_id, :meta_keywords
+    searchable_attributes :supplier_id, :primary_taxon_id, :meta_keywords, :sku
-    searchable_associations :supplier, :properties, :primary_taxon, :variants, :master
+    searchable_associations :supplier, :properties, :primary_taxon, :variants
     searchable_scopes :active, :with_properties
 
-    has_many :product_properties, dependent: :destroy
-    has_many :properties, through: :product_properties
-
-    has_many :classifications, dependent: :delete_all
-    has_many :taxons, through: :classifications
-
     belongs_to :tax_category, class_name: 'Spree::TaxCategory'
     belongs_to :shipping_category, class_name: 'Spree::ShippingCategory'
     belongs_to :supplier, class_name: 'Enterprise', touch: true
     belongs_to :primary_taxon, class_name: 'Spree::Taxon', touch: true
 
-    has_one :master,
+    has_one :image, class_name: "Spree::Image", as: :viewable, dependent: :destroy
-            -> { where is_master: true },
-            class_name: 'Spree::Variant',
-            dependent: :destroy
 
-    has_many :variants, -> {
+    has_many :product_properties, dependent: :destroy
-      where(is_master: false).order("spree_variants.position ASC")
+    has_many :properties, through: :product_properties
-    }, class_name: 'Spree::Variant'
+    has_many :classifications, dependent: :delete_all
-
+    has_many :taxons, through: :classifications
-    has_many :variants_including_master,
+    has_many :variants, -> { order("spree_variants.position ASC") }, class_name: 'Spree::Variant',
-             -> { order("spree_variants.position ASC") },
+                                                                     dependent: :destroy
-             class_name: 'Spree::Variant',
-             dependent: :destroy
 
     has_many :prices, -> {
       order('spree_variants.position, spree_variants.id, currency')
     }, through: :variants
 
     has_many :stock_items, through: :variants
-
     has_many :supplier_properties, through: :supplier, source: :properties
+    has_many :variant_images, -> { order(:position) }, source: :images,
+                                                       through: :variants
+
+    validates :name, presence: true
+    validates :permalink, presence: true
+    validates :shipping_category, presence: true
+
+    validates :supplier, presence: true
+    validates :primary_taxon, presence: true
+    validates :tax_category, presence: true,
+                             if: proc { Spree::Config[:products_require_tax_category] }
+
+    validates :variant_unit, presence: true
+    validates :unit_value, presence:
+      { if: ->(p) { %w(weight volume).include?(p.variant_unit) && new_record? } }
+    validates :variant_unit_scale,
+              presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } }
+    validates :variant_unit_name,
+              presence: { if: ->(p) { p.variant_unit == 'items' } }
+    validate :validate_image
+
+    accepts_nested_attributes_for :variants, allow_destroy: true
+    accepts_nested_attributes_for :image
+    accepts_nested_attributes_for :product_properties,
+                                  allow_destroy: true,
+                                  reject_if: lambda { |pp| pp[:property_name].blank? }
+
+    # Transient attributes used temporarily when creating a new product,
+    # these values are persisted on the product's variant
+    attr_accessor :price, :display_as, :unit_value, :unit_description
+
+    make_permalink order: :name
+
+    after_initialize :set_available_on_to_now, if: :new_record?
+
+    before_validation :sanitize_permalink
+
+    before_save :add_primary_taxon_to_taxons
+    before_destroy :punch_permalink
+
+    after_save :remove_previous_primary_taxon_from_taxons
+    after_create :ensure_standard_variant
+    after_save :update_units
 
     scope :with_properties, ->(*property_ids) {
       left_outer_joins(:product_properties).
@@ -75,58 +106,6 @@ module Spree
         )
     }
 
-    delegate_belongs_to :master, :sku, :price, :currency, :display_amount, :display_price, :weight,
-                        :height, :width, :depth, :is_master, :cost_currency,
-                        :price_in, :amount_in, :unit_value, :unit_description
-    delegate :images_attributes=, :display_as=, to: :master
-
-    after_create :set_master_variant_defaults
-    after_save :save_master
-
-    delegate :images, to: :master, prefix: true
-    alias_method :images, :master_images
-
-    has_many :variant_images, -> { order(:position) }, source: :images,
-                                                       through: :variants_including_master
-
-    accepts_nested_attributes_for :variants, allow_destroy: true
-
-    validates :name, presence: true
-    validates :permalink, presence: true
-    validates :price, presence: true, if: proc { Spree::Config[:require_master_price] }
-    validates :shipping_category, presence: true
-
-    validates :supplier, presence: true
-    validates :primary_taxon, presence: true
-    validates :tax_category, presence: true,
-                             if: proc { Spree::Config[:products_require_tax_category] }
-
-    validates :variant_unit, presence: true
-    validates :unit_value, presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } }
-    validates :variant_unit_scale,
-              presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } }
-    validates :variant_unit_name,
-              presence: { if: ->(p) { p.variant_unit == 'items' } }
-    validate :validate_image_for_master
-
-    accepts_nested_attributes_for :product_properties,
-                                  allow_destroy: true,
-                                  reject_if: lambda { |pp| pp[:property_name].blank? }
-
-    make_permalink order: :name
-
-    after_initialize :ensure_master
-    after_initialize :set_available_on_to_now, if: :new_record?
-
-    before_validation :sanitize_permalink
-    before_save :add_primary_taxon_to_taxons
-    after_save :remove_previous_primary_taxon_from_taxons
-    after_save :ensure_standard_variant
-    after_save :update_units
-
-    before_destroy :punch_permalink
-
-    # -- Joins
     scope :with_order_cycles_outer, -> {
       joins("
         LEFT OUTER JOIN spree_variants AS o_spree_variants
@@ -150,7 +129,7 @@ module Spree
     }
 
     scope :with_order_cycles_inner, -> {
-      joins(variants_including_master: { exchanges: :order_cycle })
+      joins(variants: { exchanges: :order_cycle })
     }
 
     scope :visible_for, lambda { |enterprise|
@@ -283,13 +262,6 @@ module Spree
       stock_items.sum(&:count_on_hand)
     end
 
-    # Master variant may be deleted (i.e. when the product is deleted)
-    # which would make AR's default finder return nil.
-    # This is a stopgap for that little problem.
-    def master
-      super || variants_including_master.with_deleted.find_by(is_master: true)
-    end
-
     def properties_including_inherited
       # Product properties override producer properties
       ps = product_properties.all
@@ -325,7 +297,7 @@ module Spree
         touch_distributors
 
         ExchangeVariant.
-          where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted.
+          where('exchange_variants.variant_id IN (?)', variants.with_deleted.
           select(:id)).destroy_all
 
         super
@@ -334,39 +306,6 @@ module Spree
 
     private
 
-    # ensures the master variant is flagged as such
-    def set_master_variant_defaults
-      master.is_master = true
-    end
-
-    # Here we rescue errors when saving master variants (without the need for a
-    #   validates_associated on master) and we get more specific data about the errors
-    def save_master
-      if master && (
-          master.changed? || master.new_record? || (
-            master.default_price && (
-              master.default_price.changed? || master.default_price.new_record?
-            )
-          )
-        )
-        master.save!
-      end
-
-      # If the master cannot be saved, the Product object will get its errors
-      # and will be destroyed
-    rescue ActiveRecord::RecordInvalid
-      master.errors.each do |error|
-        errors.add error.attribute, error.message
-      end
-      raise
-    end
-
-    def ensure_master
-      return unless new_record?
-
-      self.master ||= Variant.new
-    end
-
     def punch_permalink
       # Punch permalink with date prefix
       update_attribute :permalink, "#{Time.now.to_i}_#{permalink}"
@@ -379,7 +318,7 @@ module Spree
     def update_units
       return unless saved_change_to_variant_unit? || saved_change_to_variant_unit_name?
 
-      variants_including_master.each(&:update_units)
+      variants.each(&:update_units)
     end
 
     def touch_distributors
@@ -397,11 +336,14 @@ module Spree
     end
 
     def ensure_standard_variant
-      return unless master.valid? && variants.empty?
+      return unless variants.empty?
 
-      variant = master.dup
+      variant = Spree::Variant.new
       variant.product = self
-      variant.is_master = false
+      variant.price = price
+      variant.display_as = display_as
+      variant.unit_value = unit_value
+      variant.unit_description = unit_description
       variants << variant
     end
 
@@ -413,8 +355,8 @@ module Spree
       self.permalink = create_unique_permalink(requested.parameterize)
     end
 
-    def validate_image_for_master
+    def validate_image
-      return if master.images.all?(&:valid?)
+      return if image.blank? || image.valid?
 
       errors.add(:base, I18n.t('spree.admin.products.image_not_processable'))
     end

app/models/spree/taxon.rb

@@ -79,7 +79,7 @@ module Spree
 
       taxons = Spree::Taxon
         .select("DISTINCT spree_taxons.id, ents_and_vars.enterprise_id")
-        .joins(products: :variants_including_master)
+        .joins(products: :variants)
         .joins("
           INNER JOIN (#{ents_and_vars.to_sql}) AS ents_and_vars
           ON spree_variants.id = ents_and_vars.variant_id")

app/models/spree/variant.rb

@@ -13,6 +13,8 @@ module Spree
 
     acts_as_paranoid
 
+    self.ignored_columns = [:is_master]
+
     searchable_attributes :sku, :display_as, :display_name
     searchable_associations :product, :default_price
     searchable_scopes :active, :deleted
@@ -53,10 +55,9 @@ module Spree
 
     localize_number :price, :weight
 
-    validate :check_price
+    validate :check_currency
     validates :price, numericality: { greater_than_or_equal_to: 0 },
-                      presence: true,
+                      presence: true
-                      if: proc { Spree::Config[:require_master_price] }
 
     validates :unit_value, presence: true, if: ->(variant) {
       %w(weight volume).include?(variant.product&.variant_unit)
@@ -89,7 +90,6 @@ module Spree
 
     scope :with_order_cycles_inner, -> { joins(exchanges: :order_cycle) }
 
-    scope :not_master, -> { where(is_master: false) }
     scope :in_order_cycle, lambda { |order_cycle|
       with_order_cycles_inner.
         merge(Exchange.outgoing).
@@ -198,15 +198,7 @@ module Spree
 
     private
 
-    # Ensures a new variant takes the product master price when price is not supplied
+    def check_currency
-    def check_price
-      if price.nil? && Spree::Config[:require_master_price]
-        raise 'No master variant found to infer price' unless product&.master
-        raise 'Must supply price for variant or master.price for product.' if self == product.master
-
-        self.price = product.master.price
-      end
-
       return unless currency.nil?
 
       self.currency = Spree::Config[:currency]

app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb

@@ -4,18 +4,14 @@ module Api
   module Admin
     module ForOrderCycle
       class SuppliedProductSerializer < ActiveModel::Serializer
-        attributes :name, :supplier_name, :image_url, :master_id, :variants
+        attributes :name, :supplier_name, :image_url, :variants
 
         def supplier_name
           object.supplier&.name
         end
 
         def image_url
-          object.images.first&.url(:mini)
+          object.image&.url(:mini)
-        end
-
-        def master_id
-          object.master.id
         end
 
         def variants

app/serializers/api/admin/product_serializer.rb

@@ -5,7 +5,7 @@ module Api
     class ProductSerializer < ActiveModel::Serializer
       attributes :id, :name, :sku, :variant_unit, :variant_unit_scale, :variant_unit_name,
                  :inherits_properties, :on_hand, :price, :available_on, :permalink_live,
-                 :tax_category_id, :import_date, :image_url, :thumb_url, :variants, :master
+                 :tax_category_id, :import_date, :image_url, :thumb_url, :variants
 
       has_one :supplier, key: :producer_id, embed: :id
       has_one :primary_taxon, key: :category_id, embed: :id
@@ -19,19 +19,12 @@ module Api
         )
       end
 
-      def master
-        Api::Admin::VariantSerializer.new(
-          object.master,
-          image: thumb_url
-        )
-      end
-
       def image_url
-        object.images.first&.url(:product) || Spree::Image.default_image_url(:product)
+        object.image&.url(:product) || Spree::Image.default_image_url(:product)
       end
 
       def thumb_url
-        object.images.first&.url(:mini) || Spree::Image.default_image_url(:mini)
+        object.image&.url(:mini) || Spree::Image.default_image_url(:mini)
       end
 
       def on_hand

app/serializers/api/admin/variant_serializer.rb

@@ -33,7 +33,7 @@ module Api
       end
 
       def image
-        options[:image] || object.product.images.first&.url(:mini)
+        options[:image] || object.product.image&.url(:mini)
       end
 
       def in_stock

app/serializers/api/product_serializer.rb

@@ -12,7 +12,7 @@ class Api::ProductSerializer < ActiveModel::Serializer
   has_one :primary_taxon, serializer: Api::TaxonSerializer
   has_many :taxons, serializer: Api::IdSerializer
 
-  has_many :images, serializer: Api::ImageSerializer
+  has_one :image, serializer: Api::ImageSerializer
   has_one :supplier, serializer: Api::IdSerializer
 
   # return an unformatted descripton

app/serializers/api/variant_serializer.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Api::VariantSerializer < ActiveModel::Serializer
-  attributes :id, :is_master, :product_name, :sku,
+  attributes :id, :product_name, :sku,
              :options_text, :unit_value, :unit_description, :unit_to_display,
              :display_as, :display_name, :name_to_display,
              :price, :on_demand, :on_hand,
@@ -40,7 +40,7 @@ class Api::VariantSerializer < ActiveModel::Serializer
   end
 
   def thumb_url
-    object.product.images.first&.url(:mini) || Spree::Image.default_image_url(:mini)
+    object.product.image&.url(:mini) || Spree::Image.default_image_url(:mini)
   end
 
   def unit_price_price

app/services/cache_service.rb

@@ -19,7 +19,7 @@ class CacheService
   # Gets the :updated_at value of the most recently updated record for a given class, and returns
   # it as a timestamp, eg: `1583836069`.
   def self.latest_timestamp_by_class(cached_class)
-    cached_class.maximum(:updated_at).to_i
+    cached_class.maximum(:updated_at).to_f
   end
 
   def self.home_stats(statistic, &block)

app/services/exchange_products_renderer.rb

@@ -18,7 +18,6 @@ class ExchangeProductsRenderer
 
   def exchange_variants(incoming, enterprise)
     variants_relation = Spree::Variant.
-      not_master.
       where(product_id: exchange_products(incoming, enterprise).select(&:id))
 
     filter_visible(variants_relation)
@@ -96,7 +95,7 @@ class ExchangeProductsRenderer
     return enterprises if enterprises.empty?
 
     enterprises.includes(
-      supplied_products: [:supplier, :variants, { master: [:images] }]
+      supplied_products: [:supplier, :variants, :image]
     )
   end
 end

app/services/image_importer.rb

@@ -8,8 +8,8 @@ class ImageImporter
 
     Spree::Image.create(
       attachment: { io: file, filename: filename },
-      viewable_id: product.master.id,
+      viewable_id: product.id,
-      viewable_type: Spree::Variant,
+      viewable_type: Spree::Product,
     )
   end
 end

app/services/permitted_attributes/product.rb

@@ -11,7 +11,7 @@ module PermittedAttributes
         :meta_keywords, :notes, :inherits_properties,
         { product_properties_attributes: [:id, :property_name, :value],
           variants_attributes: [PermittedAttributes::Variant.attributes],
-          images_attributes: [:attachment] }
+          image_attributes: [:attachment] }
       ]
     end
   end

app/services/products_renderer.rb

@@ -97,7 +97,7 @@ class ProductsRenderer
   end
 
   def variants_for_shop_by_id
-    index_by_product_id variants_for_shop.reject(&:is_master)
+    index_by_product_id variants_for_shop
   end
 
   def index_by_product_id(variants)

app/services/sets/product_set.rb

@@ -100,7 +100,7 @@ module Sets
     end
 
     def create_or_update_variant(product, variant_attributes)
-      variant = find_model(product.variants_including_master, variant_attributes[:id])
+      variant = find_model(product.variants, variant_attributes[:id])
       if variant.present?
         variant.update(variant_attributes.except(:id))
       else

app/views/spree/admin/images/_form.html.haml

@@ -1,13 +1,11 @@
 %div
+  = f.hidden_field :viewable_id, value: @product.id
+
   .four.columns.alpha
     .field
       = f.label t('spree.filename')
       %br/
       = f.file_field :attachment
-    .field
-      = f.label Spree::Variant.model_name.human
-      %br/
-      = f.select :viewable_id, @variants, {}, {class: 'select2 fullwidth'}
   .field.omega.five.columns
     = f.label t('spree.alt_text')
     %br/

app/views/spree/admin/images/index.html.haml

@@ -6,7 +6,7 @@
 
 #images
 
-- unless @product.images.any? || @product.variant_images.any?
+- unless @product.image.present?
   .no-objects-found
     = t('spree.no_images_found')
     \.
@@ -25,16 +25,14 @@
         %th= t('spree.alt_text')
         %th.actions
     %tbody
-      - (@product.variant_images).each do |image|
+      - image = @product.image
-        - tr_class = cycle('odd', 'even')
+      %tr{id: spree_dom_id(image) }
-        - tr_id = spree_dom_id(image)
+        %td.no-border
-        %tr{class: tr_class, id: tr_id }
+          %span.handle
-          %td.no-border
+        %td
-            %span.handle
+          = link_to image_tag(image.url(:mini)), image.url(:product)
-          %td
+        %td= options_text_for(image)
-            = link_to image_tag(image.url(:mini)), image.url(:product)
+        %td= image.alt
-          %td= options_text_for(image)
+        %td.actions
-          %td= image.alt
+          = link_to_with_icon 'icon-edit', t('spree.edit'), edit_admin_product_image_url(@product, image, @url_filters), no_text: true, data: { action: 'edit'}
-          %td.actions
+          = link_to_delete image, { url: admin_product_image_url(@product, image, @url_filters), no_text: true }
-            = link_to_with_icon 'icon-edit', t('spree.edit'), edit_admin_product_image_url(@product, image, @url_filters), no_text: true, data: { action: 'edit'}
-            = link_to_delete image, { url: admin_product_image_url(@product, image, @url_filters), no_text: true }

app/views/spree/admin/images/new.html.haml

@@ -7,5 +7,3 @@
     .form-buttons.filter-actions.actions
       = button t('actions.create'), 'icon-ok'
       = link_to_with_icon 'icon-remove', t('spree.actions.cancel'), admin_product_images_url(@product, @url_filters), id: 'cancel_link', class: 'button'
-
-= javascript_include_tag 'admin/spree/images/new.js'

app/views/spree/admin/products/_display_as.html.haml

@@ -1,4 +1,4 @@
 .six.columns.omega{ "ng-if" => "product.variant_unit_with_scale != 'items'" }
   = f.field_container :display_as do
     = f.label :product_display_as, t('.display_as')
-    %input#product_display_as.fullwidth{name: "product[display_as]", placeholder: "{{ placeholder_text }}", type: "text", value: @product.master.display_as}
+    %input#product_display_as.fullwidth{name: "product[display_as]", placeholder: "{{ placeholder_text }}", type: "text", value: @product.display_as}

app/views/spree/admin/products/new.html.haml

@@ -35,8 +35,8 @@
             = f.label :unit_value, t(".value"), 'ng-disabled' => "!hasUnit(product)"
             %span.required *
             %input.fullwidth{ id: 'product_unit_value', 'ng-model' => 'product.master.unit_value_with_description', :type => 'text', placeholder: "eg. 2", 'ng-disabled' => "!hasUnit(product)" }
-            %input{ type: 'hidden', 'ng-value': 'product.master.unit_value', "ng-init": "product.master.unit_value='#{@product.master.unit_value}'", name: 'product[unit_value]' }
+            %input{ type: 'hidden', 'ng-value': 'product.master.unit_value', "ng-init": "product.master.unit_value='#{@product.unit_value}'", name: 'product[unit_value]' }
-            %input{ type: 'hidden', 'ng-value': 'product.master.unit_description', "ng-init": "product.master.unit_description='#{@product.master.unit_description}'", name: 'product[unit_description]' }
+            %input{ type: 'hidden', 'ng-value': 'product.master.unit_description', "ng-init": "product.master.unit_description='#{@product.unit_description}'", name: 'product[unit_description]' }
             = f.error_message_on :unit_value
         = render 'display_as', f: f
         .six.columns.omega{ 'ng-show' => "product.variant_unit_with_scale == 'items'" }
@@ -101,7 +101,7 @@
       .row
         = image_tag Spree::Image.default_image_url(:product), class: "four columns alpha"
       .row
-        = f.fields_for 'images_attributes[]', f.object.images.build do |image_fields|
+        = f.fields_for 'image_attributes', f.object.build_image do |image_fields|
           = image_fields.file_field :attachment
   .sixteen.columns.alpha
     .form-buttons.filter-actions.actions

app/views/spree/shared/_variant_thumbnail.html.haml

@@ -1 +1 @@
-= image_tag(variant.product.images.first&.url(:mini) || Spree::Image.default_image_url(:mini))
+= image_tag(variant.product.image&.url(:mini) || Spree::Image.default_image_url(:mini))

db/migrate/20230522090252_add_sku_to_product.rb

@@ -0,0 +1,24 @@
+class AddSkuToProduct < ActiveRecord::Migration[7.0]
+  def up
+    add_column :spree_products, :sku, :string, limit: 255, default: "", null: false
+
+    migrate_master_sku
+  end
+
+  def down
+    remove_column :spree_products, :sku
+  end
+
+  private
+
+  def migrate_master_sku
+    ActiveRecord::Base.connection.execute(<<-SQL
+      UPDATE spree_products
+      SET sku = spree_variants.sku
+      FROM spree_variants
+      WHERE spree_variants.product_id = spree_products.id
+        AND spree_variants.is_master = true
+    SQL
+    )
+  end
+end

db/migrate/20230603181837_migrate_master_image_to_product.rb

@@ -0,0 +1,46 @@
+class MigrateMasterImageToProduct < ActiveRecord::Migration[7.0]
+  def up
+    # Multiple images can be present per variant, ordered by the `position` column.
+    # In some cases if the image was deleted and another was added, the numbering can be off,
+    # so the positions of the images might be 3,4,5 or 2,3 and the "first" image would be position 2
+    # or position 3 in those cases (instead of 1).
+    #
+    # This finds the image for each variant with the lowest `position` out of the current images and
+    # sets it's `position` to 1 (if it's not already 1) to make it easier to operate on the "first" image.
+    renumber_first_image
+
+    # Switches the association of the first image for each master variant to it's product
+    migrate_master_images
+  end
+
+  def renumber_first_image
+    ActiveRecord::Base.connection.execute(<<-SQL
+      UPDATE spree_assets
+      SET position = '1'
+      FROM (
+        SELECT DISTINCT ON (viewable_id) id, viewable_id, position
+        FROM spree_assets
+        WHERE spree_assets.viewable_type = 'Spree::Variant'
+        ORDER BY viewable_id, position ASC
+      ) variant_first_image
+      WHERE spree_assets.id = variant_first_image.id
+      AND spree_assets.position != '1'
+    SQL
+    )
+  end
+
+  def migrate_master_images
+    ActiveRecord::Base.connection.execute(<<-SQL
+      UPDATE spree_assets
+      SET viewable_type = 'Spree::Product',
+          viewable_id = spree_variants.product_id
+      FROM spree_variants
+      WHERE spree_variants.id = spree_assets.viewable_id
+        AND spree_variants.is_master = true
+        AND spree_variants.deleted_at IS NULL
+        AND spree_assets.viewable_type = 'Spree::Variant'
+        AND spree_assets.position = '1'
+    SQL
+    )
+  end
+end

db/migrate/20230605133804_remove_master_variants.rb

@@ -0,0 +1,86 @@
+class RemoveMasterVariants < ActiveRecord::Migration[7.0]
+  def change
+    if ActiveRecord::Base.connection.table_exists? "spree_option_values_variants"
+      delete_master_option_values
+    end
+
+    handle_master_line_items
+    handle_master_exchange_variants
+    delete_master_inventory_units
+    delete_master_variant_prices
+    delete_master_stock_items
+    delete_master_variants
+  end
+
+  private
+
+  def handle_master_line_items
+    ActiveRecord::Base.connection.execute(<<-SQL
+      UPDATE spree_variants
+      SET is_master = false
+      FROM spree_line_items
+      WHERE spree_variants.is_master = true
+        AND spree_variants.id = spree_line_items.variant_id
+    SQL
+    )
+  end
+
+  def handle_master_exchange_variants
+    ActiveRecord::Base.connection.execute(<<-SQL
+      UPDATE spree_variants
+      SET is_master = false
+      FROM exchange_variants
+      WHERE spree_variants.is_master = true
+        AND spree_variants.id = exchange_variants.variant_id
+    SQL
+    )
+  end
+
+  def delete_master_inventory_units
+    ActiveRecord::Base.connection.execute(<<-SQL
+      DELETE FROM spree_inventory_units
+      USING spree_variants
+      WHERE spree_variants.is_master = true
+        AND spree_variants.id = spree_inventory_units.variant_id
+    SQL
+    )
+  end
+
+  def delete_master_option_values
+    ActiveRecord::Base.connection.execute(<<-SQL
+      DELETE FROM spree_option_values_variants
+      USING spree_variants
+      WHERE spree_variants.is_master = true
+        AND spree_variants.id = spree_option_values_variants.variant_id
+    SQL
+    )
+  end
+
+  def delete_master_variant_prices
+    ActiveRecord::Base.connection.execute(<<-SQL
+      DELETE FROM spree_prices
+      USING spree_variants
+      WHERE spree_variants.is_master = true
+        AND spree_variants.id = spree_prices.variant_id
+    SQL
+    )
+  end
+
+  def delete_master_stock_items
+    ActiveRecord::Base.connection.execute(<<-SQL
+      DELETE FROM spree_stock_items
+      USING spree_variants
+      WHERE spree_variants.is_master = true
+        AND spree_variants.id = spree_stock_items.variant_id
+    SQL
+    )
+  end
+
+  def delete_master_variants
+    ActiveRecord::Base.connection.execute(<<-SQL
+      DELETE FROM spree_variants
+      WHERE is_master = true
+    SQL
+    )
+  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_05_22_120633) do
+ActiveRecord::Schema[7.0].define(version: 2023_06_05_133804) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
   enable_extension "plpgsql"
@@ -745,6 +745,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_22_120633) do
     t.text "notes"
     t.integer "primary_taxon_id", null: false
     t.boolean "inherits_properties", default: true, null: false
+    t.string "sku", limit: 255, default: "", null: false
     t.index ["available_on"], name: "index_products_on_available_on"
     t.index ["deleted_at"], name: "index_products_on_deleted_at"
     t.index ["name"], name: "index_products_on_name"

engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb

@@ -24,7 +24,7 @@ module Catalog
           .joins(:product)
           .where(
             spree_products: { supplier_id: enterprise_ids },
-            spree_variants: { is_master: false, deleted_at: nil }
+            spree_variants: { deleted_at: nil }
           )
 
         return relation if excluded_items_ids.blank?

engines/dfc_provider/app/services/variant_fetcher.rb

@@ -9,7 +9,7 @@ class VariantFetcher
   end
 
   def scope
-    Spree::Variant.not_master.
+    Spree::Variant.
       joins(:product).
       where(spree_products: { supplier: @enterprise })
   end

engines/dfc_provider/spec/requests/enterprises_spec.rb

@@ -13,7 +13,7 @@ describe "Enterprises", type: :request do
 
       expect(response).to have_http_status :ok
       expect(response.body).to include(product.name)
-      expect(response.body).to include(product.sku)
+      expect(response.body).to include(product.variants.first.sku)
       expect(response.body).to include("offers/#{product.variants.first.id}")
     end
 

engines/order_management/app/services/order_management/subscriptions/variants_list.rb

@@ -16,7 +16,7 @@ module OrderManagement
           variant_conditions << exchange_variant_ids
         end
 
-        Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions)
+        Spree::Variant.joins(:product).where(*variant_conditions)
       end
 
       def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant)

lib/open_food_network/order_cycle_permissions.rb

@@ -98,7 +98,7 @@ module OpenFoodNetwork
               @order_cycle
             ).pluck(:id).uniq
 
-          product_ids = Spree::Product.joins(:variants_including_master).
+          product_ids = Spree::Product.joins(:variants).
             where("spree_variants.id IN (?)", variant_ids).pluck(:id).uniq
 
           producers_active_ids = Enterprise.joins(:supplied_products).
@@ -304,7 +304,7 @@ module OpenFoodNetwork
               hubs.select("enterprises.id"),
               @order_cycle).pluck(:id).uniq
 
-      product_ids = Spree::Product.joins(:variants_including_master).
+      product_ids = Spree::Product.joins(:variants).
         where(spree_variants: { id: variant_ids }).pluck(:id).uniq
 
       producer_ids = Enterprise.joins(:supplied_products).

lib/open_food_network/scope_variants_for_search.rb

@@ -29,11 +29,11 @@ module OpenFoodNetwork
     attr_reader :params
 
     def search_params
-      { product_name_cont: params[:q], sku_cont: params[:q] }
+      { product_name_cont: params[:q], sku_cont: params[:q], product_sku_cont: params[:q] }
     end
 
     def query_scope
-      Spree::Variant.where(is_master: false).
+      Spree::Variant.
         ransack(search_params.merge(m: 'or')).
         result.
         order("spree_products.name, display_name, display_as, spree_products.variant_unit_name").

lib/reporting/reports/products_and_inventory/base.rb

@@ -33,7 +33,6 @@ module Reporting
 
         def child_variants
           Spree::Variant.
-            where(is_master: false).
             joins(:product).
             merge(visible_products).
             order('spree_products.name')

lib/spree/core/product_duplicator.rb

@@ -21,20 +21,17 @@ module Spree
         product.dup.tap do |new_product|
           new_product.name = "COPY OF #{product.name}"
           new_product.taxons = product.taxons
+          new_product.sku = ""
           new_product.created_at = nil
           new_product.deleted_at = nil
           new_product.updated_at = nil
+          new_product.unit_value = true
           new_product.product_properties = reset_properties
-          new_product.master = duplicate_master
+          new_product.image = duplicate_image(product.image) if product.image
           new_product.variants = duplicate_variants
         end
       end
 
-      def duplicate_master
-        master = product.master
-        duplicate_variant(master)
-      end
-
       def duplicate_variants
         product.variants.map do |variant|
           duplicate_variant(variant)

lib/tasks/data.rake

@@ -46,7 +46,7 @@ namespace :ofn do
           end
 
           # For each variant in the exchange
-          products = Spree::Product.joins(:variants_including_master).where(
+          products = Spree::Product.joins(:variants).where(
             'spree_variants.id IN (?)', exchange.variants
           ).pluck(:id).uniq
           producers = Enterprise.joins(:supplied_products).where("spree_products.id IN (?)",

lib/tasks/import_product_images.rake

@@ -25,7 +25,7 @@ namespace :ofn do
           next
         end
 
-        if product.images.first.nil?
+        if product.image.nil?
           ImageImporter.new.import(entry[:image_url], product)
           puts " image added."
         else

spec/controllers/admin/reports_controller_spec.rb

@@ -24,13 +24,13 @@ describe Admin::ReportsController, type: :controller do
     create(:simple_order_cycle, coordinator: coordinator1,
                                 distributors: [distributor1, distributor2],
                                 suppliers: [supplier1, supplier2, supplier3],
-                                variants: [product1.master, product3.master])
+                                variants: [product1.variants.first, product3.variants.first])
   }
   let(:ocB) {
     create(:simple_order_cycle, coordinator: coordinator2,
                                 distributors: [distributor1, distributor2],
                                 suppliers: [supplier1, supplier2, supplier3],
-                                variants: [product2.master])
+                                variants: [product2.variants.first])
   }
 
   # orderA1 can only be accessed by supplier1, supplier3 and distributor1
@@ -38,8 +38,8 @@ describe Admin::ReportsController, type: :controller do
     order = create(:order, distributor: distributor1, bill_address: bill_address,
                            ship_address: ship_address, special_instructions: instructions,
                            order_cycle: ocA)
-    order.line_items << create(:line_item, variant: product1.master)
+    order.line_items << create(:line_item, variant: product1.variants.first)
-    order.line_items << create(:line_item, variant: product3.master)
+    order.line_items << create(:line_item, variant: product3.variants.first)
     order.finalize!
     order.save
     order
@@ -49,7 +49,7 @@ describe Admin::ReportsController, type: :controller do
     order = create(:order, distributor: distributor2, bill_address: bill_address,
                            ship_address: ship_address, special_instructions: instructions,
                            order_cycle: ocA)
-    order.line_items << create(:line_item, variant: product2.master)
+    order.line_items << create(:line_item, variant: product2.variants.first)
     order.finalize!
     order.save
     order
@@ -59,8 +59,8 @@ describe Admin::ReportsController, type: :controller do
     order = create(:order, distributor: distributor1, bill_address: bill_address,
                            ship_address: ship_address, special_instructions: instructions,
                            order_cycle: ocB)
-    order.line_items << create(:line_item, variant: product1.master)
+    order.line_items << create(:line_item, variant: product1.variants.first)
-    order.line_items << create(:line_item, variant: product3.master)
+    order.line_items << create(:line_item, variant: product3.variants.first)
     order.finalize!
     order.save
     order
@@ -70,7 +70,7 @@ describe Admin::ReportsController, type: :controller do
     order = create(:order, distributor: distributor2, bill_address: bill_address,
                            ship_address: ship_address, special_instructions: instructions,
                            order_cycle: ocB)
-    order.line_items << create(:line_item, variant: product2.master)
+    order.line_items << create(:line_item, variant: product2.variants.first)
     order.finalize!
     order.save
     order

spec/controllers/api/v0/product_images_controller_spec.rb

@@ -25,7 +25,7 @@ describe Api::V0::ProductImagesController, type: :controller do
       }
 
       expect(response.status).to eq 201
-      expect(product_without_image.reload.images.first.id).to eq json_response['id']
+      expect(product_without_image.reload.image.id).to eq json_response['id']
     end
 
     it "updates an existing product image" do
@@ -34,7 +34,7 @@ describe Api::V0::ProductImagesController, type: :controller do
       }
 
       expect(response.status).to eq 200
-      expect(product_with_image.reload.images.first.id).to eq json_response['id']
+      expect(product_with_image.reload.image.id).to eq json_response['id']
     end
 
     it "reports errors when saving fails" do
@@ -43,7 +43,7 @@ describe Api::V0::ProductImagesController, type: :controller do
       }
 
       expect(response.status).to eq 422
-      expect(product_without_image.images.count).to eq 0
+      expect(product_without_image.image).to be_nil
       expect(json_response["id"]).to eq nil
       expect(json_response["errors"]).to include "Attachment has an invalid content type"
     end

spec/controllers/api/v0/products_controller_spec.rb

@@ -14,10 +14,7 @@ describe Api::V0::ProductsController, type: :controller do
   }
   let(:product_other_supplier) { create(:product, supplier: supplier2) }
   let(:product_with_image) { create(:product_with_image, supplier: supplier) }
-  let(:attributes) {
+  let(:all_attributes) { ["id", "name", "available_on", "variants"] }
-    ["id", "name", "supplier", "price", "on_hand", "available_on", "permalink_live"]
-  }
-  let(:all_attributes) { ["id", "name", "price", "available_on", "variants"] }
   let(:variants_attributes) {
     ["id", "options_text", "unit_value", "unit_description", "unit_to_display", "on_demand",
      "display_as", "display_name", "name_to_display", "sku", "on_hand", "price"]
@@ -36,8 +33,8 @@ describe Api::V0::ProductsController, type: :controller do
     end
 
     it "gets a single product" do
-      product.master.images.create!(attachment: image("thinking-cat.jpg"))
+      product.create_image!(attachment: image("thinking-cat.jpg"))
-      product.variants.create!(unit_value: "1", unit_description: "thing")
+      product.variants.create!(unit_value: "1", unit_description: "thing", price: 1)
       product.variants.first.images.create!(attachment: image("thinking-cat.jpg"))
       product.set_property("spree", "rocks")
       api_get :show, id: product.to_param
@@ -115,7 +112,7 @@ describe Api::V0::ProductsController, type: :controller do
 
     it "can create a new product" do
       api_post :create, product: { name: "The Other Product",
-                                   price: 19.99,
+                                   price: 123.45,
                                    shipping_category_id: create(:shipping_category).id,
                                    supplier_id: supplier.id,
                                    primary_taxon_id: FactoryBot.create(:taxon).id,
@@ -125,6 +122,7 @@ describe Api::V0::ProductsController, type: :controller do
 
       expect(all_attributes.all?{ |attr| json_response.keys.include? attr }).to eq(true)
       expect(response.status).to eq(201)
+      expect(Spree::Product.last.variants.first.price).to eq 123.45
     end
 
     it "cannot create a new product with invalid attributes" do
@@ -133,7 +131,7 @@ describe Api::V0::ProductsController, type: :controller do
       expect(response.status).to eq(422)
       expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.")
       errors = json_response["errors"]
-      expect(errors.keys).to match_array(["name", "price", "primary_taxon", "shipping_category",
+      expect(errors.keys).to match_array(["name", "primary_taxon", "shipping_category",
                                           "supplier", "variant_unit"])
     end
 
@@ -210,13 +208,6 @@ describe Api::V0::ProductsController, type: :controller do
         spree_post :clone, product_id: product.id, format: :json
         expect(Spree::Product.second.variants.count).not_to eq Spree::Product.first.variants.count
       end
-
-      # price info: it does not consider price changes; it considers the price set upon product creation
-      it '(does not) clone price which was updated' do
-        product.update_attribute(:price, 2.22)
-        spree_post :clone, product_id: product.id, format: :json
-        expect(json_response['price']).not_to eq(2.22)
-      end
     end
 
     context 'as an administrator' do

spec/controllers/api/v0/variants_controller_spec.rb

@@ -5,10 +5,10 @@ require 'spec_helper'
 describe Api::V0::VariantsController, type: :controller do
   render_views
 
-  let(:supplier) { FactoryBot.create(:supplier_enterprise) }
+  let(:supplier) { create(:supplier_enterprise) }
-  let!(:variant1) { FactoryBot.create(:variant) }
+  let!(:variant1) { create(:variant) }
-  let!(:variant2) { FactoryBot.create(:variant) }
+  let!(:variant2) { create(:variant) }
-  let!(:variant3) { FactoryBot.create(:variant) }
+  let!(:variant3) { create(:variant) }
   let(:attributes) {
     [:id, :options_text, :price, :on_hand, :unit_value, :unit_description, :on_demand, :display_as,
      :display_name]
@@ -22,10 +22,7 @@ describe Api::V0::VariantsController, type: :controller do
     let(:current_api_user) { build(:user) }
 
     let!(:product) { create(:product) }
-    let!(:variant) do
+    let!(:variant) { product.variants.first }
-      variant = product.master
-      variant
-    end
 
     it "retrieves a list of variants with appropriate attributes" do
       get :index, format: :json
@@ -45,18 +42,18 @@ describe Api::V0::VariantsController, type: :controller do
     # Regression test for spree#2141
     context "a deleted variant" do
       before do
-        expect(Spree::Variant.count).to eq 11
+        expect(Spree::Variant.count).to eq 7
         variant.update_column(:deleted_at, Time.zone.now)
       end
 
       it "is not returned in the results" do
         api_get :index
-        expect(json_response.count).to eq(10)
+        expect(json_response.count).to eq(6)
       end
 
       it "is not returned even when show_deleted is passed" do
         api_get :index, show_deleted: true
-        expect(json_response.count).to eq(10)
+        expect(json_response.count).to eq(6)
       end
     end
 
@@ -91,25 +88,37 @@ describe Api::V0::VariantsController, type: :controller do
   context "as an enterprise user" do
     let(:current_api_user) { create(:user, enterprises: [supplier]) }
     let(:supplier_other) { create(:supplier_enterprise) }
-    let(:product) { create(:product, supplier: supplier) }
+    let!(:product) { create(:product, supplier: supplier) }
-    let(:variant) { product.master }
+    let(:variant) { product.variants.first }
     let(:product_other) { create(:product, supplier: supplier_other) }
-    let(:variant_other) { product_other.master }
+    let(:variant_other) { product_other.variants.first }
 
-    it "deletes a variant" do
+    context "with a single remaining variant" do
-      api_delete :destroy, id: variant.to_param
+      it "does not delete the variant" do
+        api_delete :destroy, id: variant.id
 
-      expect(response.status).to eq(204)
+        expect(variant.reload.deleted_at).to be_nil
-      expect { variant.reload }.not_to raise_error
+      end
-      expect(variant.deleted_at).to be_present
     end
 
-    it "is denied access to soft deleting another enterprises' variant" do
+    context "with more than one variants" do
-      api_delete :destroy, id: variant_other.to_param
+      let(:variant_to_delete) { create(:variant, product: product) }
 
-      assert_unauthorized!
+      it "deletes a variant" do
-      expect { variant_other.reload }.not_to raise_error
+        api_delete :destroy, id: variant_to_delete.id
-      expect(variant_other.deleted_at).to be_nil
+
+        expect(response.status).to eq(204)
+        expect { variant_to_delete.reload }.not_to raise_error
+        expect(variant_to_delete.deleted_at).to be_present
+      end
+
+      it "is denied access to soft deleting another enterprises' variant" do
+        api_delete :destroy, id: variant_other.to_param
+
+        assert_unauthorized!
+        expect { variant_other.reload }.not_to raise_error
+        expect(variant_other.deleted_at).to be_nil
+      end
     end
   end
 
@@ -117,7 +126,8 @@ describe Api::V0::VariantsController, type: :controller do
     let(:current_api_user) { create(:admin_user) }
 
     let(:product) { create(:product) }
-    let(:variant) { product.master }
+    let(:variant) { product.variants.first }
+    let!(:variant2) { create(:variant, product: product) }
 
     context "deleted variants" do
       before do
@@ -133,7 +143,8 @@ describe Api::V0::VariantsController, type: :controller do
 
     it "can create a new variant" do
       original_number_of_variants = variant.product.variants.count
-      api_post :create, variant: { sku: "12345", unit_value: "1", unit_description: "L" },
+      api_post :create, variant: { sku: "12345", unit_value: "1",
+                                   unit_description: "L", price: "1" },
                         product_id: variant.product.to_param
 
       expect(attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true)

spec/controllers/spree/admin/products_controller_spec.rb

@@ -158,8 +158,8 @@ describe Spree::Admin::ProductsController, type: :controller do
           tempfile: Tempfile.new('unsupported_image_format.exr')
         )
         product_attrs_with_image = product_attrs.merge(
-          images_attributes: {
+          image_attributes: {
-            '0' => { attachment: product_image }
+            attachment: product_image
           }
         )
 

spec/controllers/spree/admin/variants_controller_spec.rb

@@ -11,7 +11,7 @@ module Spree
         describe "deleted variants" do
           let(:product) { create(:product, name: 'Product A') }
           let(:deleted_variant) do
-            deleted_variant = product.variants.create(unit_value: "2")
+            deleted_variant = product.variants.create(unit_value: "2", price: 1)
             deleted_variant.delete
             deleted_variant
           end

spec/factories/asset_factory.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :asset, class: Spree::Asset do
+    viewable { nil }
+    position { 1 }
+    type { "Spree::Image" }
+  end
+end

spec/factories/order_cycle_factory.rb

@@ -32,8 +32,8 @@ FactoryBot.define do
       proxy.exchanges.incoming.each do |exchange|
         product = create(:product, supplier: exchange.sender)
         Spree::Image.create(
-          viewable_id: product.master.id,
+          viewable_id: product.id,
-          viewable_type: 'Spree::Variant',
+          viewable_type: 'Spree::Product',
           alt: "position 1",
           attachment: Rack::Test::UploadedFile.new(white_logo_path),
           position: 1

spec/factories/product_factory.rb

@@ -32,7 +32,6 @@ FactoryBot.define do
       tax_category { |r| Spree::TaxCategory.first || r.association(:tax_category) }
 
       after(:create) do |product, evaluator|
-        product.master.on_hand = evaluator.on_hand
         product.variants.first.on_hand = evaluator.on_hand
         product.reload
       end
@@ -42,8 +41,8 @@ FactoryBot.define do
   factory :product_with_image, parent: :product do
     after(:create) do |product|
       Spree::Image.create(attachment: white_logo_file,
-                          viewable_id: product.master.id,
+                          viewable_id: product.id,
-                          viewable_type: 'Spree::Variant')
+                          viewable_type: 'Spree::Product')
     end
   end
 
@@ -53,8 +52,6 @@ FactoryBot.define do
       on_hand { 5 }
     end
     after(:create) do |product, evaluator|
-      product.master.on_demand = evaluator.on_demand
-      product.master.on_hand = evaluator.on_hand
       product.variants.first.on_demand = evaluator.on_demand
       product.variants.first.on_hand = evaluator.on_hand
       product.reload

spec/factories/stock_location_factory.rb

@@ -27,8 +27,8 @@ FactoryBot.define do
         product_1 = create(:product)
         product_2 = create(:product)
 
-        stock_location.stock_items.where(variant_id: product_1.master.id).first.adjust_count_on_hand(10)
+        stock_location.stock_items.where(variant_id: product_1.variants.first.id).first.adjust_count_on_hand(10)
-        stock_location.stock_items.where(variant_id: product_2.master.id).first.adjust_count_on_hand(20)
+        stock_location.stock_items.where(variant_id: product_2.variants.first.id).first.adjust_count_on_hand(20)
       end
     end
   end

spec/javascripts/unit/admin/order_cycles/services/enterprise_spec.js.coffee

@@ -45,14 +45,7 @@ describe 'Enterprise service', ->
     expect(Enterprise.suppliedVariants(1)).toEqual [10, 10]
 
   describe "finding the variants of a product", ->
-    it "returns the master for products without variants", ->
-      p =
-        master_id: 1
-        variants: []
-      expect(Enterprise.variantsOf(p)).toEqual [1]
-
     it "returns the variant ids for products with variants", ->
       p =
-        master_id: 1
         variants: [{id: 2}, {id: 3}]
       expect(Enterprise.variantsOf(p)).toEqual [2, 3]

spec/javascripts/unit/darkswarm/services/products_spec.js.coffee

@@ -29,9 +29,9 @@ describe 'Products service', ->
         id: 9
       master: {}
       variants: []
-      images: [
+      image: {
         large_url: 'foo.png'
-      ]
+      }
     currentOrder =
       line_items: []
     currentHub =

spec/lib/open_food_network/enterprise_fee_calculator_spec.rb

@@ -25,32 +25,32 @@ module OpenFoodNetwork
           describe "supplier fees" do
             let!(:exchange1) {
               create(:exchange, order_cycle: order_cycle, sender: supplier1, receiver: coordinator, incoming: true,
-                                enterprise_fees: [enterprise_fee1], variants: [product1.master])
+                                enterprise_fees: [enterprise_fee1], variants: [product1.variants.first])
             }
             let!(:exchange2) {
               create(:exchange, order_cycle: order_cycle, sender: supplier2, receiver: coordinator, incoming: true,
-                                enterprise_fees: [enterprise_fee2], variants: [product2.master])
+                                enterprise_fees: [enterprise_fee2], variants: [product2.variants.first])
             }
 
             it "calculates via regular computation" do
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).fees_for(product1.master)).to eq(20)
+                                                 order_cycle).fees_for(product1.variants.first)).to eq(20)
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).fees_for(product2.master)).to eq(3)
+                                                 order_cycle).fees_for(product2.variants.first)).to eq(3)
             end
 
             it "calculates via indexed computation" do
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).indexed_fees_for(product1.master)).to eq(20)
+                                                 order_cycle).indexed_fees_for(product1.variants.first)).to eq(20)
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).indexed_fees_for(product2.master)).to eq(3)
+                                                 order_cycle).indexed_fees_for(product2.variants.first)).to eq(3)
             end
           end
 
           describe "coordinator fees" do
             let!(:exchange) {
               create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false,
-                                enterprise_fees: [], variants: [product1.master])
+                                enterprise_fees: [], variants: [product1.variants.first])
             }
 
             before do
@@ -59,29 +59,29 @@ module OpenFoodNetwork
 
             it "sums via regular computation" do
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).fees_for(product1.master)).to eq(23)
+                                                 order_cycle).fees_for(product1.variants.first)).to eq(23)
             end
 
             it "sums via indexed computation" do
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).indexed_fees_for(product1.master)).to eq(23)
+                                                 order_cycle).indexed_fees_for(product1.variants.first)).to eq(23)
             end
           end
 
           describe "distributor fees" do
             let!(:exchange) {
               create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false,
-                                enterprise_fees: [enterprise_fee1, enterprise_fee2, enterprise_fee3], variants: [product1.master])
+                                enterprise_fees: [enterprise_fee1, enterprise_fee2, enterprise_fee3], variants: [product1.variants.first])
             }
 
             it "sums via regular computation" do
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).fees_for(product1.master)).to eq(23)
+                                                 order_cycle).fees_for(product1.variants.first)).to eq(23)
             end
 
             it "sums via indexed computation" do
               expect(EnterpriseFeeCalculator.new(distributor,
-                                                 order_cycle).indexed_fees_for(product1.master)).to eq(23)
+                                                 order_cycle).indexed_fees_for(product1.variants.first)).to eq(23)
             end
           end
         end
@@ -93,17 +93,17 @@ module OpenFoodNetwork
           }
           let!(:exchange) {
             create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor, incoming: false,
-                              enterprise_fees: [enterprise_fee1], variants: [product1.master])
+                              enterprise_fees: [enterprise_fee1], variants: [product1.variants.first])
           }
 
           it "sums via regular computation" do
             expect(EnterpriseFeeCalculator.new(distributor,
-                                               order_cycle).fees_for(product1.master)).to eq(2.00)
+                                               order_cycle).fees_for(product1.variants.first)).to eq(2.00)
           end
 
           it "sums via indexed computation" do
             expect(EnterpriseFeeCalculator.new(distributor,
-                                               order_cycle).indexed_fees_for(product1.master)).to eq(2.00)
+                                               order_cycle).indexed_fees_for(product1.variants.first)).to eq(2.00)
           end
         end
       end
@@ -123,27 +123,27 @@ module OpenFoodNetwork
                             enterprise_fees: [
                               ef_admin, ef_sales, ef_packing, ef_transport, ef_fundraising
                             ],
-                            variants: [product1.master])
+                            variants: [product1.variants.first])
         }
 
         describe "regular computation" do
           it "returns the fees names" do
             expect(EnterpriseFeeCalculator
-              .new(distributor, order_cycle).fees_name_by_type_for(product1.master))
+              .new(distributor, order_cycle).fees_name_by_type_for(product1.variants.first))
               .to eq({ admin: "Admin", fundraising: "Fundraising", packing: "Packing",
                        sales: "Sales", transport: "Transport" })
           end
 
           it "returns a breakdown of fees" do
             expect(EnterpriseFeeCalculator.new(distributor,
-                                               order_cycle).fees_by_type_for(product1.master)).to eq(admin: 1.23, sales: 4.56, packing: 7.89,
+                                               order_cycle).fees_by_type_for(product1.variants.first)).to eq(admin: 1.23, sales: 4.56, packing: 7.89,
                                                                                                      transport: 0.12, fundraising: 3.45)
           end
 
           it "filters out zero fees" do
             ef_admin.calculator.update_attribute :preferred_amount, 0
             expect(EnterpriseFeeCalculator.new(distributor,
-                                               order_cycle).fees_by_type_for(product1.master)).to eq(sales: 4.56, packing: 7.89, transport: 0.12,
+                                               order_cycle).fees_by_type_for(product1.variants.first)).to eq(sales: 4.56, packing: 7.89, transport: 0.12,
                                                                                                      fundraising: 3.45)
           end
         end
@@ -151,14 +151,14 @@ module OpenFoodNetwork
         describe "indexed computation" do
           it "returns a breakdown of fees" do
             expect(EnterpriseFeeCalculator.new(distributor,
-                                               order_cycle).indexed_fees_by_type_for(product1.master)).to eq(admin: 1.23, sales: 4.56,
+                                               order_cycle).indexed_fees_by_type_for(product1.variants.first)).to eq(admin: 1.23, sales: 4.56,
                                                                                                              packing: 7.89, transport: 0.12, fundraising: 3.45)
           end
 
           it "filters out zero fees" do
             ef_admin.calculator.update_attribute :preferred_amount, 0
             expect(EnterpriseFeeCalculator.new(distributor,
-                                               order_cycle).indexed_fees_by_type_for(product1.master)).to eq(sales: 4.56, packing: 7.89,
+                                               order_cycle).indexed_fees_by_type_for(product1.variants.first)).to eq(sales: 4.56, packing: 7.89,
                                                                                                              transport: 0.12, fundraising: 3.45)
           end
         end
@@ -166,14 +166,14 @@ module OpenFoodNetwork
 
       describe "creating adjustments" do
         let(:order) { create(:order, distributor: distributor, order_cycle: order_cycle) }
-        let!(:line_item) { create(:line_item, order: order, variant: product1.master) }
+        let!(:line_item) { create(:line_item, order: order, variant: product1.variants.first) }
         let(:enterprise_fee_line_item) { create(:enterprise_fee) }
         let(:enterprise_fee_order) {
           create(:enterprise_fee, calculator: Calculator::FlatRate.new(preferred_amount: 2))
         }
         let!(:exchange) {
           create(:exchange, order_cycle: order_cycle, sender: coordinator, receiver: distributor,
-                            incoming: false, variants: [product1.master])
+                            incoming: false, variants: [product1.variants.first])
         }
 
         before { order.reload }

spec/lib/open_food_network/scope_variants_to_search_spec.rb

@@ -33,6 +33,15 @@ describe OpenFoodNetwork::ScopeVariantsForSearch do
         expect(result).to include v1, v2
         expect(result).to_not include v3, v4
       end
+
+      context "matching both product SKUs and variant SKUs" do
+        let!(:v5) { create(:variant, sku: "Product 1b") }
+
+        it "returns all variants whose SKU or product's SKU match the query" do
+          expect(result).to include v1, v2, v5
+          expect(result).to_not include v3, v4
+        end
+      end
     end
 
     context "when a schedule_id is specified" do

spec/lib/reports/enterprise_fee_summary/enterprise_fee_summary_report_spec.rb

@@ -718,7 +718,7 @@ describe Reporting::Reports::EnterpriseFeeSummary::Base do
   end
 
   def default_variant_options
-    { product: product, producer: producer, is_master: false, coordinator: coordinator,
+    { product: product, producer: producer, coordinator: coordinator,
       distributor: distributor, order_cycle: order_cycle }
   end
 

spec/lib/reports/products_and_inventory_report_spec.rb

@@ -105,7 +105,7 @@ module Reporting
           end
 
           describe "Filtering variants" do
-            let(:variants) { Spree::Variant.where(nil).joins(:product).where(is_master: false) }
+            let(:variants) { Spree::Variant.where(nil).joins(:product) }
 
             describe "based on report type" do
               it "returns only variants on hand" do

spec/lib/spree/core/product_duplicator_spec.rb

@@ -8,8 +8,8 @@ describe Spree::Core::ProductDuplicator do
            name: "foo",
            taxons: [],
            product_properties: [property],
-           master: master_variant,
+           variants: [variant],
-           variants: [variant]
+           image: image
   end
 
   let(:new_product) do
@@ -25,14 +25,6 @@ describe Spree::Core::ProductDuplicator do
     double 'New Property'
   end
 
-  let(:master_variant) do
-    double 'Variant',
-           sku: "12345",
-           price: 19.99,
-           currency: "AUD",
-           images: [image]
-  end
-
   let(:variant) do
     double 'Variant 1',
            sku: "67890",
@@ -41,11 +33,6 @@ describe Spree::Core::ProductDuplicator do
            images: [image_variant]
   end
 
-  let(:new_master_variant) do
-    double 'New Variant',
-           sku: "12345"
-  end
-
   let(:new_variant) do
     double 'New Variant 1',
            sku: "67890"
@@ -72,7 +59,6 @@ describe Spree::Core::ProductDuplicator do
 
   before do
     expect(product).to receive(:dup).and_return(new_product)
-    expect(master_variant).to receive(:dup).and_return(new_master_variant)
     expect(variant).to receive(:dup).and_return(new_variant)
     expect(image).to receive(:dup).and_return(new_image)
     expect(image_variant).to receive(:dup).and_return(new_image_variant)
@@ -83,18 +69,14 @@ describe Spree::Core::ProductDuplicator do
     duplicator = Spree::Core::ProductDuplicator.new(product)
     expect(new_product).to receive(:name=).with("COPY OF foo")
     expect(new_product).to receive(:taxons=).with([])
+    expect(new_product).to receive(:sku=).with("")
     expect(new_product).to receive(:product_properties=).with([new_property])
     expect(new_product).to receive(:created_at=).with(nil)
+    expect(new_product).to receive(:unit_value=).with(true)
     expect(new_product).to receive(:updated_at=).with(nil)
     expect(new_product).to receive(:deleted_at=).with(nil)
-    expect(new_product).to receive(:master=).with(new_master_variant)
     expect(new_product).to receive(:variants=).with([new_variant])
-
+    expect(new_product).to receive(:image=).with(new_image)
-    expect(new_master_variant).to receive(:sku=).with("")
-    expect(new_master_variant).to receive(:deleted_at=).with(nil)
-    expect(new_master_variant).to receive(:images=).with([new_image])
-    expect(new_master_variant).to receive(:price=).with(master_variant.price)
-    expect(new_master_variant).to receive(:currency=).with(master_variant.currency)
 
     expect(new_variant).to receive(:sku=).with("")
     expect(new_variant).to receive(:deleted_at=).with(nil)

spec/models/concerns/product_stock_spec.rb

@@ -5,25 +5,6 @@ require "spec_helper"
 describe ProductStock do
   let(:product) { create(:simple_product) }
 
-  context "when product has no variants" do
-    before do
-      product.variants.destroy
-      product.variants.reload
-    end
-
-    describe "product.on_demand" do
-      it "is master.on_demand" do
-        expect(product.on_demand).to eq(product.master.on_demand)
-      end
-    end
-
-    describe "product.on_hand" do
-      it "is master.on_hand" do
-        expect(product.on_hand).to eq(product.master.on_hand)
-      end
-    end
-  end
-
   context "when product has one variant" do
     describe "product.on_demand" do
       it "is the products first variant on_demand" do

spec/models/enterprise_spec.rb

@@ -487,7 +487,7 @@ describe Enterprise do
         s = create(:supplier_enterprise)
         d = create(:distributor_enterprise)
         p = create(:product)
-        create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.master])
+        create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.variants.first])
         expect(Enterprise.distributors_with_active_order_cycles).to eq([d])
       end
 
@@ -496,19 +496,12 @@ describe Enterprise do
         d = create(:distributor_enterprise)
         p = create(:product)
         create(:simple_order_cycle, orders_open_at: 10.days.from_now,
-                                    orders_close_at: 17.days.from_now, suppliers: [s], distributors: [d], variants: [p.master])
+                                    orders_close_at: 17.days.from_now, suppliers: [s], distributors: [d], variants: [p.variants.first])
         expect(Enterprise.distributors_with_active_order_cycles).not_to include d
       end
     end
 
     describe "supplying_variant_in" do
-      it "finds producers by supply of master variant" do
-        s = create(:supplier_enterprise)
-        p = create(:simple_product, supplier: s)
-
-        expect(Enterprise.supplying_variant_in([p.master])).to eq([s])
-      end
-
       it "finds producers by supply of variant" do
         s = create(:supplier_enterprise)
         p = create(:simple_product, supplier: s)
@@ -523,7 +516,7 @@ describe Enterprise do
         p1 = create(:simple_product, supplier: s1)
         p2 = create(:simple_product, supplier: s2)
 
-        expect(Enterprise.supplying_variant_in([p1.master, p2.master])).to match_array [s1, s2]
+        expect(Enterprise.supplying_variant_in([p1.variants.first, p2.variants.first])).to match_array [s1, s2]
       end
 
       it "does not return duplicates" do
@@ -531,7 +524,7 @@ describe Enterprise do
         p1 = create(:simple_product, supplier: s)
         p2 = create(:simple_product, supplier: s)
 
-        expect(Enterprise.supplying_variant_in([p1.master, p2.master])).to eq([s])
+        expect(Enterprise.supplying_variant_in([p1.variants.first, p2.variants.first])).to eq([s])
       end
     end
 
@@ -541,14 +534,14 @@ describe Enterprise do
 
       it "returns enterprises distributing via an order cycle" do
         order_cycle = create(:simple_order_cycle, distributors: [distributor],
-                                                  variants: [product.master])
+                                                  variants: [product.variants.first])
         expect(Enterprise.distributing_products(product.id)).to eq([distributor])
       end
 
       it "does not return duplicate enterprises" do
         another_product = create(:product)
         order_cycle = create(:simple_order_cycle, distributors: [distributor],
-                                                  variants: [product.master, another_product.master])
+                                                  variants: [product.variants.first, another_product.variants.first])
         expect(Enterprise.distributing_products([product.id,
                                                  another_product.id])).to eq([distributor])
       end
@@ -660,13 +653,13 @@ describe Enterprise do
   end
 
   describe "finding variants distributed by the enterprise" do
-    it "finds variants, including master, distributed by order cycle" do
+    it "finds variants distributed by order cycle" do
       distributor = create(:distributor_enterprise)
       product = create(:product)
       variant = product.variants.first
       create(:simple_order_cycle, distributors: [distributor], variants: [variant])
 
-      expect(distributor.distributed_variants).to match_array [product.master, variant]
+      expect(distributor.distributed_variants).to match_array [variant]
     end
   end
 
@@ -858,7 +851,7 @@ describe Enterprise do
         :simple_order_cycle,
         suppliers: [supplier],
         distributors: [distributor],
-        variants: [product.master]
+        variants: [product.variants.first]
       )
       expect(distributor.plus_parents_and_order_cycle_producers(order_cycle)).to eq([supplier])
     end
@@ -872,7 +865,7 @@ describe Enterprise do
         :simple_order_cycle,
         distributors: [distributor],
         suppliers: [supplier],
-        variants: [product.master]
+        variants: [product.variants.first]
       )
       create(:enterprise_relationship, parent: distributor,
                                        child: supplier, permissions: [permission])
@@ -890,7 +883,7 @@ describe Enterprise do
         :simple_order_cycle,
         suppliers: [supplier],
         distributors: [distributor],
-        variants: [product.master]
+        variants: [product.variants.first]
       )
       expected = supplier.plus_parents_and_order_cycle_producers(order_cycle)
       expect(expected).not_to include(distributor)
@@ -908,7 +901,7 @@ permissions: [permission])
         :simple_order_cycle,
         suppliers: [sender],
         distributors: [distributor],
-        variants: [product.master]
+        variants: [product.variants.first]
       )
       expected = supplier.plus_parents_and_order_cycle_producers(order_cycle)
       expect(expected).to include(sender)

spec/models/exchange_spec.rb

@@ -38,7 +38,7 @@ describe Exchange do
     e = create(:exchange)
     p = create(:product)
 
-    e.exchange_variants.create(variant: p.master)
+    e.exchange_variants.create(variant: p.variants.first)
     expect(e.variants.count).to eq(1)
   end
 
@@ -243,16 +243,7 @@ describe Exchange do
       expect(Exchange.with_any_variant([v1.id, v2.id, v3.id])).to eq([ex])
     end
 
-    it "finds exchanges with a particular product's master variant" do
+    it "finds exchanges with a particular product's variant" do
-      p = create(:simple_product)
-      ex = create(:exchange)
-      ex.variants << p.master
-      p.reload
-
-      expect(Exchange.with_product(p)).to eq([ex])
-    end
-
-    it "finds exchanges with a particular product's non-master variant" do
       p = create(:simple_product)
       v = create(:variant, product: p)
       ex = create(:exchange)

spec/models/order_cycle_spec.rb

@@ -153,10 +153,10 @@ describe OrderCycle do
   it "checks for variants" do
     p1 = create(:simple_product)
     p2 = create(:simple_product)
-    oc = create(:simple_order_cycle, suppliers: [p1.supplier], variants: [p1.master])
+    oc = create(:simple_order_cycle, suppliers: [p1.supplier], variants: [p1.variants.first])
 
-    expect(oc).to have_variant(p1.master)
+    expect(oc).to have_variant(p1.variants.first)
-    expect(oc).not_to have_variant(p2.master)
+    expect(oc).not_to have_variant(p2.variants.first)
   end
 
   describe "product exchanges" do
@@ -193,18 +193,18 @@ describe OrderCycle do
       p1_v_deleted.deleted_at = Time.zone.now
       p1_v_deleted.save
 
-      e0.variants << p0.master
+      e0.variants << p0.variants.first
-      e1.variants << p1.master
+      e1.variants << p1.variants.first
-      e1.variants << p2.master
+      e1.variants << p2.variants.first
       e1.variants << p2_v
-      e2.variants << p1.master
+      e2.variants << p1.variants.first
       e2.variants << p1_v_deleted
       e2.variants << p1_v_visible
       e2.variants << p1_v_hidden
     end
 
     it "reports on the variants exchanged" do
-      expect(oc.variants).to match_array [p0.master, p1.master, p2.master, p2_v, p1_v_visible,
+      expect(oc.variants).to match_array [p0.variants.first, p1.variants.first, p2.variants.first, p2_v, p1_v_visible,
                                           p1_v_hidden]
     end
 
@@ -213,11 +213,11 @@ describe OrderCycle do
     end
 
     it "reports on the variants supplied" do
-      expect(oc.supplied_variants).to match_array [p0.master]
+      expect(oc.supplied_variants).to match_array [p0.variants.first]
     end
 
     it "reports on the variants distributed" do
-      expect(oc.distributed_variants).to match_array [p1.master, p2.master, p2_v, p1_v_visible,
+      expect(oc.distributed_variants).to match_array [p1.variants.first, p2.variants.first, p2_v, p1_v_visible,
                                                       p1_v_hidden]
     end
 
@@ -236,7 +236,7 @@ describe OrderCycle do
         end
 
         it "returns all variants in the outgoing exchange for the distributor provided" do
-          expect(oc.variants_distributed_by(d2)).to include p1.master, p1_v_visible
+          expect(oc.variants_distributed_by(d2)).to include p1.variants.first, p1_v_visible
           expect(oc.variants_distributed_by(d2)).not_to include p1_v_hidden, p1_v_deleted
           expect(oc.variants_distributed_by(d1)).to include p2_v
         end

spec/models/product_importer_spec.rb

@@ -151,51 +151,51 @@ describe ProductImport::ProductImporter do
       carrots = Spree::Product.find_by(name: 'Carrots')
       expect(carrots.supplier).to eq enterprise
       expect(carrots.on_hand).to eq 5
-      expect(carrots.price).to eq 3.20
+      expect(carrots.variants.first.price).to eq 3.20
-      expect(carrots.unit_value).to eq 500
+      expect(carrots.variants.first.unit_value).to eq 500
       expect(carrots.variant_unit).to eq 'weight'
       expect(carrots.variant_unit_scale).to eq 1
-      expect(carrots.on_demand).to_not eq true
+      expect(carrots.variants.first.on_demand).to_not eq true
       expect(carrots.variants.first.import_date).to be_within(1.minute).of Time.zone.now
 
       potatoes = Spree::Product.find_by(name: 'Potatoes')
       expect(potatoes.supplier).to eq enterprise
       expect(potatoes.on_hand).to eq 6
-      expect(potatoes.price).to eq 6.50
+      expect(potatoes.variants.first.price).to eq 6.50
-      expect(potatoes.unit_value).to eq 2000
+      expect(potatoes.variants.first.unit_value).to eq 2000
       expect(potatoes.variant_unit).to eq 'weight'
       expect(potatoes.variant_unit_scale).to eq 1000
-      expect(potatoes.on_demand).to_not eq true
+      expect(potatoes.variants.first.on_demand).to_not eq true
       expect(potatoes.variants.first.import_date).to be_within(1.minute).of Time.zone.now
 
       pea_soup = Spree::Product.find_by(name: 'Pea Soup')
       expect(pea_soup.supplier).to eq enterprise
       expect(pea_soup.on_hand).to eq 8
-      expect(pea_soup.price).to eq 5.50
+      expect(pea_soup.variants.first.price).to eq 5.50
-      expect(pea_soup.unit_value).to eq 0.75
+      expect(pea_soup.variants.first.unit_value).to eq 0.75
       expect(pea_soup.variant_unit).to eq 'volume'
       expect(pea_soup.variant_unit_scale).to eq 0.001
-      expect(pea_soup.on_demand).to_not eq true
+      expect(pea_soup.variants.first.on_demand).to_not eq true
       expect(pea_soup.variants.first.import_date).to be_within(1.minute).of Time.zone.now
 
       salad = Spree::Product.find_by(name: 'Salad')
       expect(salad.supplier).to eq enterprise
       expect(salad.on_hand).to eq 7
-      expect(salad.price).to eq 4.50
+      expect(salad.variants.first.price).to eq 4.50
-      expect(salad.unit_value).to eq 1
+      expect(salad.variants.first.unit_value).to eq 1
       expect(salad.variant_unit).to eq 'items'
       expect(salad.variant_unit_scale).to eq nil
-      expect(salad.on_demand).to_not eq true
+      expect(salad.variants.first.on_demand).to_not eq true
       expect(salad.variants.first.import_date).to be_within(1.minute).of Time.zone.now
 
       buns = Spree::Product.find_by(name: 'Hot Cross Buns')
       expect(buns.supplier).to eq enterprise
       expect(buns.on_hand).to eq 7
-      expect(buns.price).to eq 3.50
+      expect(buns.variants.first.price).to eq 3.50
-      expect(buns.unit_value).to eq 1
+      expect(buns.variants.first.unit_value).to eq 1
       expect(buns.variant_unit).to eq 'items'
       expect(buns.variant_unit_scale).to eq nil
-      expect(buns.on_demand).to eq true
+      expect(buns.variants.first.on_demand).to eq true
       expect(buns.variants.first.import_date).to be_within(1.minute).of Time.zone.now
     end
   end
@@ -232,7 +232,7 @@ describe ProductImport::ProductImporter do
       carrots = Spree::Product.find_by(name: 'Good Carrots')
       expect(carrots.supplier).to eq enterprise
       expect(carrots.on_hand).to eq 5
-      expect(carrots.price).to eq 3.20
+      expect(carrots.variants.first.price).to eq 3.20
       expect(carrots.variants.first.import_date).to be_within(1.minute).of Time.zone.now
 
       expect(Spree::Product.find_by(name: 'Bad Potatoes')).to eq nil
@@ -275,7 +275,7 @@ describe ProductImport::ProductImporter do
 
       carrots = Spree::Product.find_by(name: 'Good Carrots')
       expect(carrots.on_hand).to eq 5
-      expect(carrots.price).to eq 3.20
+      expect(carrots.variants.first.price).to eq 3.20
       expect(carrots.primary_taxon.name).to eq "Vegetables"
       expect(carrots.shipping_category).to eq shipping_category
       expect(carrots.supplier).to eq enterprise

spec/models/spree/ability_spec.rb

@@ -337,7 +337,7 @@ describe Spree::Ability do
         is_expected.to have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy],
                                     for: p1)
         is_expected.to have_ability(
-          [:admin, :index, :read, :edit, :update, :search, :destroy, :delete], for: p1.master
+          [:admin, :index, :read, :edit, :update, :search, :destroy, :delete], for: p1.variants.first
         )
       end
 
@@ -346,7 +346,7 @@ describe Spree::Ability do
         is_expected.to have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy],
                                     for: p_related)
         is_expected.to have_ability(
-          [:admin, :index, :read, :edit, :update, :search, :destroy, :delete], for: p_related.master
+          [:admin, :index, :read, :edit, :update, :search, :destroy, :delete], for: p_related.variants.first
         )
       end
 
@@ -354,7 +354,7 @@ describe Spree::Ability do
         is_expected.not_to have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy],
                                         for: p2)
         is_expected.not_to have_ability([:admin, :index, :read, :edit, :update, :search, :destroy],
-                                        for: p2.master)
+                                        for: p2.variants.first)
       end
 
       it "should not be able to access admin actions on orders" do
@@ -369,13 +369,13 @@ describe Spree::Ability do
         is_expected.to have_ability([:create], for: Spree::Variant)
         is_expected.to have_ability(
           [:admin, :index, :read, :create, :edit, :search, :update, :destroy,
-           :delete], for: p1.master
+           :delete], for: p1.variants.first
         )
       end
 
       it "should not be able to read/write other enterprises' product variants" do
         is_expected.not_to have_ability(
-          [:admin, :index, :read, :create, :edit, :search, :update, :destroy], for: p2.master
+          [:admin, :index, :read, :create, :edit, :search, :update, :destroy], for: p2.variants.first
         )
       end
 
@@ -554,10 +554,10 @@ describe Spree::Ability do
       end
 
       describe "variant overrides" do
-        let(:vo1) { create(:variant_override, hub: d1, variant: p1.master) }
+        let(:vo1) { create(:variant_override, hub: d1, variant: p1.variants.first) }
-        let(:vo2) { create(:variant_override, hub: d1, variant: p2.master) }
+        let(:vo2) { create(:variant_override, hub: d1, variant: p2.variants.first) }
-        let(:vo3) { create(:variant_override, hub: d2, variant: p1.master) }
+        let(:vo3) { create(:variant_override, hub: d2, variant: p1.variants.first) }
-        let(:vo4) { create(:variant_override, hub: d2, variant: p2.master) }
+        let(:vo4) { create(:variant_override, hub: d2, variant: p2.variants.first) }
 
         let!(:er1) {
           create(:enterprise_relationship, parent: s1, child: d1,

spec/models/spree/asset_spec.rb

@@ -6,7 +6,7 @@ describe Spree::Asset do
   describe "#viewable" do
     it "touches association" do
       product = create(:product)
-      asset = Spree::Asset.create! { |a| a.viewable = product.master }
+      asset = Spree::Asset.create! { |a| a.viewable = product }
 
       product.update_column(:updated_at, 1.day.ago)
 

spec/models/spree/image_spec.rb

@@ -9,7 +9,7 @@ module Spree
     subject {
       Spree::Image.create!(
         attachment: black_logo_file,
-        viewable: product.master,
+        viewable: product,
       )
     }
     let(:product) { create(:product) }

spec/models/spree/line_item_spec.rb

@@ -477,7 +477,7 @@ module Spree
       describe "inheriting units" do
         let!(:p) {
           create(:product, variant_unit: "weight", variant_unit_scale: 1,
-                           master: create(:variant, unit_value: 1000 ))
+                           unit_value: 1000 )
         }
         let!(:v) { p.variants.first }
         let!(:o) { create(:order) }

spec/models/spree/product_spec.rb

@@ -16,71 +16,21 @@ module Spree
         it 'duplicates product' do
           clone = product.duplicate
           expect(clone.name).to eq 'COPY OF ' + product.name
-          expect(clone.master.sku).to eq ''
+          expect(clone.sku).to eq ""
-          expect(clone.images.size).to eq product.images.size
+          expect(clone.image).to eq product.image
-        end
-      end
-
-      context "product has no variants" do
-        context "#destroy" do
-          it "should set deleted_at value" do
-            product.destroy
-            expect(product.deleted_at).to_not be_nil
-            expect(product.master.deleted_at).to_not be_nil
-          end
         end
       end
 
       context "product has variants" do
         before do
-          create(:variant, product: product)
+          product.reload.variants << create(:variant, product: product)
         end
 
         context "#destroy" do
           it "should set deleted_at value" do
             product.destroy
             expect(product.deleted_at).to_not be_nil
-            expect(product.variants_including_master.all? { |v| !v.deleted_at.nil? }).to be_truthy
+            expect(product.variants.all? { |v| !v.deleted_at.nil? }).to be_truthy
-          end
-        end
-      end
-
-      context "#price" do
-        # Regression test for Spree #1173
-        it 'strips non-price characters' do
-          product.price = "$10"
-          expect(product.price).to eq 10.0
-        end
-      end
-
-      context "#display_price" do
-        before { product.price = 10.55 }
-
-        context "with display_currency set to true" do
-          before { Spree::Config[:display_currency] = true }
-
-          it "shows the currency" do
-            expect(product.display_price.to_s).to eq "$10.55 #{Spree::Config[:currency]}"
-          end
-        end
-
-        context "with display_currency set to false" do
-          before { Spree::Config[:display_currency] = false }
-
-          it "does not include the currency" do
-            expect(product.display_price.to_s).to eq "$10.55"
-          end
-        end
-
-        context "with currency set to JPY" do
-          before do
-            product.master.default_price.currency = 'JPY'
-            product.master.default_price.save!
-            Spree::Config[:currency] = 'JPY'
-          end
-
-          it "displays the currency in yen" do
-            expect(product.display_price.to_s).to eq "¥11"
           end
         end
       end
@@ -91,12 +41,6 @@ module Spree
             expect(product.variants.to_sql).to match(/ORDER BY spree_variants.position ASC/)
           end
         end
-
-        context 'with master variant' do
-          it 'sorts variants by position' do
-            expect(product.variants_including_master.to_sql).to match(/ORDER BY spree_variants.position ASC/)
-          end
-        end
       end
     end
 
@@ -281,7 +225,7 @@ module Spree
 
     context "has stock movements" do
       let(:product) { create(:product) }
-      let(:variant) { product.master }
+      let(:variant) { product.variants.first }
       let(:stock_item) { variant.stock_items.first }
 
       it "doesnt raise ReadOnlyRecord error" do
@@ -312,14 +256,6 @@ module Spree
         expect(build(:simple_product, supplier: nil)).not_to be_valid
       end
 
-      it "does not save when master is invalid" do
-        product = build(:product)
-        product.variant_unit = 'weight'
-        product.master.unit_value = nil
-
-        expect(product.save).to eq(false)
-      end
-
       it "defaults available_on to now" do
         Timecop.freeze do
           product = Product.new
@@ -411,16 +347,10 @@ module Spree
             product.save!
           end
 
-          it "copies the properties on master variant to the first standard variant" do
+          it "copies properties to the first standard variant" do
             expect(product.variants.reload.length).to eq 1
             standard_variant = product.variants.reload.first
-            expect(standard_variant.price).to eq product.master.price
+            expect(standard_variant.price).to eq 4.27
-          end
-
-          it "only duplicates master with after_save when no standard variants exist" do
-            expect(product).to receive :ensure_standard_variant
-            product.name = "Something else"
-            expect{ product.save! }.to_not change{ product.variants.count }
           end
         end
 
@@ -461,11 +391,11 @@ module Spree
         end
       end
 
-      describe "#validate_image_for_master" do
+      describe "#validate_image" do
-        let(:product) { build_stubbed(:simple_product) }
+        let(:product) { create(:product_with_image) }
 
-        context "when the image attached to the master variant is invalid" do
+        context "when the image is invalid" do
-          before { product.master.images.new.errors.add(:image_not_processable, "invalid") }
+          before { expect(product.image).to receive(:valid?).and_return(false) }
 
           it "adds an error message to the base object" do
             expect(product).not_to be_valid
@@ -565,8 +495,8 @@ module Spree
           d2 = create(:distributor_enterprise)
           p1 = create(:product)
           p2 = create(:product)
-          create(:simple_order_cycle, suppliers: [s], distributors: [d1], variants: [p1.master])
+          create(:simple_order_cycle, suppliers: [s], distributors: [d1], variants: [p1.variants.first])
-          create(:simple_order_cycle, suppliers: [s], distributors: [d2], variants: [p2.master])
+          create(:simple_order_cycle, suppliers: [s], distributors: [d2], variants: [p2.variants.first])
           expect(Product.in_distributor(d1)).to eq([p1])
         end
 
@@ -590,7 +520,7 @@ module Spree
           p = create(:product)
           oc = create(:simple_order_cycle, coordinator: c, suppliers: [s], distributors: [d])
           ex = oc.exchanges.incoming.first
-          ex.variants << p.master
+          ex.variants << p.variants.first
 
           expect(Product.in_distributor(d)).to be_empty
         end
@@ -642,8 +572,8 @@ module Spree
           d2 = create(:distributor_enterprise)
           p1 = create(:product)
           p2 = create(:product)
-          create(:simple_order_cycle, suppliers: [s], distributors: [d1], variants: [p1.master])
+          create(:simple_order_cycle, suppliers: [s], distributors: [d1], variants: [p1.variants.first])
-          create(:simple_order_cycle, suppliers: [s], distributors: [d2], variants: [p2.master])
+          create(:simple_order_cycle, suppliers: [s], distributors: [d2], variants: [p2.variants.first])
           expect(Product.in_supplier_or_distributor(d1)).to eq([p1])
         end
 
@@ -651,7 +581,7 @@ module Spree
           s = create(:supplier_enterprise)
           d = create(:distributor_enterprise)
           p = create(:product, supplier: s)
-          create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.master])
+          create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.variants.first])
           [s, d].each { |e| expect(Product.in_supplier_or_distributor(e)).to eq([p]) }
         end
       end
@@ -664,9 +594,9 @@ module Spree
           p1 = create(:product)
           p2 = create(:product)
           oc1 = create(:simple_order_cycle, suppliers: [s], distributors: [d1],
-                                            variants: [p1.master])
+                                            variants: [p1.variants.first])
           oc2 = create(:simple_order_cycle, suppliers: [s], distributors: [d2],
-                                            variants: [p2.master])
+                                            variants: [p2.variants.first])
           expect(Product.in_order_cycle(oc1)).to eq([p1])
         end
       end
@@ -680,9 +610,9 @@ module Spree
           p2 = create(:product)
           p3 = create(:product)
           oc2 = create(:simple_order_cycle, suppliers: [s], distributors: [d2],
-                                            variants: [p2.master], orders_open_at: 8.days.ago, orders_close_at: 1.day.ago)
+                                            variants: [p2.variants.first], orders_open_at: 8.days.ago, orders_close_at: 1.day.ago)
           oc2 = create(:simple_order_cycle, suppliers: [s], distributors: [d3],
-                                            variants: [p3.master], orders_close_at: Date.tomorrow)
+                                            variants: [p3.variants.first], orders_close_at: Date.tomorrow)
           expect(Product.in_an_active_order_cycle).to eq([p3])
         end
       end
@@ -860,8 +790,8 @@ module Spree
         d2 = create(:distributor_enterprise)
         p1 = create(:product)
         p2 = create(:product)
-        oc1 = create(:simple_order_cycle, distributors: [d1], variants: [p1.master])
+        oc1 = create(:simple_order_cycle, distributors: [d1], variants: [p1.variants.first])
-        oc2 = create(:simple_order_cycle, distributors: [d2], variants: [p2.master])
+        oc2 = create(:simple_order_cycle, distributors: [d2], variants: [p2.variants.first])
 
         expect(p1).to be_in_distributor d1
         expect(p1).not_to be_in_distributor d2
@@ -872,8 +802,8 @@ module Spree
         d2 = create(:distributor_enterprise)
         p1 = create(:product)
         p2 = create(:product)
-        oc1 = create(:simple_order_cycle, distributors: [d1], variants: [p1.master])
+        oc1 = create(:simple_order_cycle, distributors: [d1], variants: [p1.variants.first])
-        oc2 = create(:simple_order_cycle, distributors: [d2], variants: [p2.master])
+        oc2 = create(:simple_order_cycle, distributors: [d2], variants: [p2.variants.first])
 
         expect(p1).to be_in_order_cycle oc1
         expect(p1).not_to be_in_order_cycle oc2
@@ -899,18 +829,6 @@ module Spree
 
           expect(v.reload.unit_presentation).to eq "1L"
         end
-
-        it "updates its master variant's unit values" do
-          p.master.update!(unit_value: 1)
-          p.reload
-
-          expect(p.master.unit_presentation).to eq "1g"
-
-          p.update!(variant_unit: 'volume', variant_unit_scale: 0.001)
-          p.reload
-
-          expect(p.master.unit_presentation).to eq "1L"
-        end
       end
     end
 
@@ -933,13 +851,7 @@ module Spree
         create(:exchange, order_cycle: oc, incoming: true, sender: s, receiver: oc.coordinator)
       }
 
-      it "removes the master variant from all order cycles" do
+      it "removes all variants from order cycles" do
-        e.variants << p.master
-        p.destroy
-        expect(e.variants.reload).to be_empty
-      end
-
-      it "removes all other variants from order cycles" do
         e.variants << v
         p.destroy
         expect(e.variants.reload).to be_empty

spec/models/spree/variant_spec.rb

@@ -232,16 +232,16 @@ describe Spree::Variant do
       let!(:d2) { create(:distributor_enterprise) }
       let!(:p1) { create(:simple_product) }
       let!(:p2) { create(:simple_product) }
-      let!(:oc1) { create(:simple_order_cycle, distributors: [d1], variants: [p1.master]) }
+      let!(:oc1) { create(:simple_order_cycle, distributors: [d1], variants: [p1.variants.first]) }
-      let!(:oc2) { create(:simple_order_cycle, distributors: [d2], variants: [p2.master]) }
+      let!(:oc2) { create(:simple_order_cycle, distributors: [d2], variants: [p2.variants.first]) }
 
       it "shows variants in an order cycle distribution" do
-        expect(Spree::Variant.in_distributor(d1)).to eq([p1.master])
+        expect(Spree::Variant.in_distributor(d1)).to eq([p1.variants.first])
       end
 
       it "doesn't show duplicates" do
-        oc_dup = create(:simple_order_cycle, distributors: [d1], variants: [p1.master])
+        oc_dup = create(:simple_order_cycle, distributors: [d1], variants: [p1.variants.first])
-        expect(Spree::Variant.in_distributor(d1)).to eq([p1.master])
+        expect(Spree::Variant.in_distributor(d1)).to eq([p1.variants.first])
       end
     end
 
@@ -250,18 +250,18 @@ describe Spree::Variant do
       let!(:d2) { create(:distributor_enterprise) }
       let!(:p1) { create(:product) }
       let!(:p2) { create(:product) }
-      let!(:oc1) { create(:simple_order_cycle, distributors: [d1], variants: [p1.master]) }
+      let!(:oc1) { create(:simple_order_cycle, distributors: [d1], variants: [p1.variants.first]) }
-      let!(:oc2) { create(:simple_order_cycle, distributors: [d2], variants: [p2.master]) }
+      let!(:oc2) { create(:simple_order_cycle, distributors: [d2], variants: [p2.variants.first]) }
 
       it "shows variants in an order cycle" do
-        expect(Spree::Variant.in_order_cycle(oc1)).to eq([p1.master])
+        expect(Spree::Variant.in_order_cycle(oc1)).to eq([p1.variants.first])
       end
 
       it "doesn't show duplicates" do
         ex = create(:exchange, order_cycle: oc1, sender: oc1.coordinator, receiver: d2)
-        ex.variants << p1.master
+        ex.variants << p1.variants.first
 
-        expect(Spree::Variant.in_order_cycle(oc1)).to eq([p1.master])
+        expect(Spree::Variant.in_order_cycle(oc1)).to eq([p1.variants.first])
       end
     end
 
@@ -520,10 +520,6 @@ describe Spree::Variant do
           variant.unit_value = nil
           expect(variant).not_to be_valid
         end
-
-        it "has a valid master variant" do
-          expect(product.master).to be_valid
-        end
       end
     end
 
@@ -552,10 +548,6 @@ describe Spree::Variant do
         expect(variant).to be_valid
         expect(variant.unit_value).to eq 1.0
       end
-
-      it "has a valid master variant" do
-        expect(product.master).to be_valid
-      end
     end
 
     context "when the product's unit is non-weight" do

spec/serializers/api/product_serializer_spec.rb

@@ -28,7 +28,7 @@ describe Api::ProductSerializer do
   it "serializes various attributes" do
     expect(serializer.serializable_hash.keys).to eq [
       :id, :name, :permalink, :meta_keywords, :group_buy, :notes, :description, :description_html,
-      :properties_with_values, :variants, :primary_taxon, :taxons, :images, :supplier
+      :properties_with_values, :variants, :primary_taxon, :taxons, :image, :supplier
     ]
   end
 

spec/serializers/api/variant_serializer_spec.rb

@@ -11,7 +11,6 @@ describe Api::VariantSerializer do
       to include(
         :id,
         :name_to_display,
-        :is_master,
         :on_hand,
         :name_to_display,
         :unit_to_display,

spec/services/cache_service_spec.rb

@@ -20,7 +20,7 @@ describe CacheService do
   end
 
   describe "#cached_data_by_class" do
-    let(:timestamp) { Time.now.to_i }
+    let(:timestamp) { Time.now.to_f }
 
     before do
       allow(rails_cache).to receive(:fetch)
@@ -42,10 +42,12 @@ describe CacheService do
 
     it "gets the :updated_at value of the last record for a given class and returns a timestamp" do
       taxon1.touch
-      expect(CacheService.latest_timestamp_by_class(Spree::Taxon)).to eq taxon1.updated_at.to_i
+      expect(CacheService.latest_timestamp_by_class(Spree::Taxon)).
+        to eq taxon1.reload.updated_at.to_f
 
       taxon2.touch
-      expect(CacheService.latest_timestamp_by_class(Spree::Taxon)).to eq taxon2.updated_at.to_i
+      expect(CacheService.latest_timestamp_by_class(Spree::Taxon)).
+        to eq taxon2.reload.updated_at.to_f
     end
   end
 end

spec/services/image_importer_spec.rb

@@ -14,8 +14,8 @@ describe ImageImporter do
         Spree::Image.count
       }.by(1)
 
-      expect(product.images.count).to eq 1
+      expect(product.image).to_not be_nil
-      expect(product.reload.images.first.attachment_blob.byte_size).to eq 6274
+      expect(product.reload.image.attachment_blob.byte_size).to eq 6274
     end
   end
 end

spec/system/admin/bulk_product_update_spec.rb

@@ -72,8 +72,8 @@ describe '
 
     it "displays 'on demand' for any variant that is available on demand" do
       p1 = FactoryBot.create(:product)
-      v1 = FactoryBot.create(:variant, product: p1, is_master: false, on_hand: 4)
+      v1 = FactoryBot.create(:variant, product: p1, on_hand: 4)
-      v2 = FactoryBot.create(:variant, product: p1, is_master: false, on_hand: 0, on_demand: true)
+      v2 = FactoryBot.create(:variant, product: p1, on_hand: 0, on_demand: true)
 
       visit spree.admin_products_path
       expect(page).to have_selector "a.view-variants", count: 1
@@ -128,10 +128,10 @@ describe '
       p1 = FactoryBot.create(:product)
       v0 = p1.variants.first
       v0.update_attribute(:on_demand, false)
-      v1 = FactoryBot.create(:variant, product: p1, is_master: false, on_hand: 15)
+      v1 = FactoryBot.create(:variant, product: p1, on_hand: 15)
       v1.update_attribute(:on_demand, false)
       p1.variants << v1
-      v2 = FactoryBot.create(:variant, product: p1, is_master: false, on_hand: 6)
+      v2 = FactoryBot.create(:variant, product: p1, on_hand: 6)
       v2.update_attribute(:on_demand, false)
       p1.variants << v2
 
@@ -146,15 +146,14 @@ describe '
     end
 
     it "displays a price input (for each variant) for each product" do
-      p1 = FactoryBot.create(:product, price: 2.0)
+      p1 = create(:product, price: 2.0)
-      v1 = FactoryBot.create(:variant, product: p1, is_master: false, price: 12.75)
+      v1 = create(:variant, product: p1, price: 12.75)
-      v2 = FactoryBot.create(:variant, product: p1, is_master: false, price: 2.50)
+      v2 = create(:variant, product: p1, price: 2.50)
 
       visit spree.admin_products_path
       expect(page).to have_selector "a.view-variants", count: 1
       all("a.view-variants").each(&:click)
 
-      expect(page).to have_field "price", with: "2.0", visible: false
       expect(page).to have_field "variant_price", with: "12.75"
       expect(page).to have_field "variant_price", with: "2.5"
     end
@@ -162,9 +161,9 @@ describe '
     it "displays a unit value field (for each variant) for each product" do
       p1 = FactoryBot.create(:product, price: 2.0, variant_unit: "weight",
                                        variant_unit_scale: "1000")
-      v1 = FactoryBot.create(:variant, product: p1, is_master: false, price: 12.75,
+      v1 = FactoryBot.create(:variant, product: p1, price: 12.75,
                                        unit_value: 1200, unit_description: "(small bag)", display_as: "bag")
-      v2 = FactoryBot.create(:variant, product: p1, is_master: false, price: 2.50,
+      v2 = FactoryBot.create(:variant, product: p1, price: 2.50,
                                        unit_value: 4800, unit_description: "(large bag)", display_as: "bin")
 
       visit spree.admin_products_path
@@ -229,9 +228,9 @@ describe '
   end
 
   context "creating new variants" do
+    let!(:product) { create(:product, variant_unit: 'weight', variant_unit_scale: 1000) }
+
     before do
-      # Given a product without variants or a unit
-      p = FactoryBot.create(:product, variant_unit: 'weight', variant_unit_scale: 1000)
       login_as_admin
       visit spree.admin_products_path
 
@@ -279,18 +278,19 @@ describe '
     end
 
     context "handle the 'on_demand' variant case creation" do
+      let(:v1) { create(:variant, product: product, on_hand: 4) }
+      let(:v2) { create(:variant, product: product, on_demand: true) }
+
       before do
-        p = Spree::Product.first
+        product.variants << v1
-        p.master.update_attribute(:on_hand, 5)
+        product.variants << v2
-        p.save
+
-        v1 = FactoryBot.create(:variant, product: p, is_master: false, on_hand: 4)
+        visit spree.admin_products_path
-        v2 = FactoryBot.create(:variant, product: p, is_master: false, on_demand: true)
+        page.find('a.view-variants').click
-        p.variants << v1
-        p.variants << v2
       end
 
       it "when variant unit value is: '120'" do
-        within "tr#v_#{Spree::Variant.second.id}" do
+        within "tr#v_#{v2.id}" do
           page.find(".add-variant").click
         end
 
@@ -304,7 +304,7 @@ describe '
       end
 
       it "creating a variant with unit value is: '120g' and 'on_hand' filled" do
-        within "tr#v_#{Spree::Variant.second.id}" do
+        within "tr#v_#{v2.id}" do
           page.find(".add-variant").click
         end
 
@@ -319,7 +319,7 @@ describe '
       end
 
       it "creating a variant with unit value is: '120g' and 'on_demand' checked" do
-        within "tr#v_#{Spree::Variant.second.id}" do
+        within "tr#v_#{v2.id}" do
           page.find(".add-variant").trigger("click")
         end
 
@@ -401,10 +401,10 @@ describe '
   end
 
   it "updating a product with variants" do
-    s1 = FactoryBot.create(:supplier_enterprise)
+    s1 = create(:supplier_enterprise)
-    s2 = FactoryBot.create(:supplier_enterprise)
+    s2 = create(:supplier_enterprise)
-    p = FactoryBot.create(:product, supplier: s1, available_on: Date.current, variant_unit: 'volume', variant_unit_scale: 0.001,
+    p = create(:product, supplier: s1, available_on: Date.current, variant_unit: 'volume', variant_unit_scale: 0.001,
-                                    price: 3.0, unit_value: 0.25, unit_description: '(bottle)' )
+                         price: 3.0, unit_value: 0.25, unit_description: '(bottle)' )
     v = p.variants.first
     v.update_attribute(:sku, "VARIANTSKU")
     v.update_attribute(:on_demand, false)

spec/system/admin/product_import_spec.rb

@@ -94,7 +94,7 @@ describe "Product Import" do
       potatoes = Spree::Product.find_by(name: 'Potatoes')
       expect(potatoes.supplier).to eq enterprise
       expect(potatoes.on_hand).to eq 6
-      expect(potatoes.price).to eq 6.50
+      expect(potatoes.variants.first.price).to eq 6.50
       expect(potatoes.variants.first.import_date).to be_within(1.minute).of Time.zone.now
 
       wait_until { page.find("a.button.view").present? }

spec/system/admin/products_spec.rb

@@ -103,17 +103,17 @@ describe '
       expect(product.supplier).to eq(@supplier)
       expect(product.variant_unit).to eq('weight')
       expect(product.variant_unit_scale).to eq(1000)
-      expect(product.unit_value).to eq(5000)
+      expect(product.variants.first.unit_value).to eq(5000)
-      expect(product.unit_description).to eq("")
+      expect(product.variants.first.unit_description).to eq("")
       expect(product.variant_unit_name).to eq("")
       expect(product.primary_taxon_id).to eq(taxon.id)
-      expect(product.price.to_s).to eq('19.99')
+      expect(product.variants.first.price.to_s).to eq('19.99')
       expect(product.on_hand).to eq(5)
       expect(product.tax_category_id).to eq(tax_category.id)
       expect(product.shipping_category).to eq(shipping_category)
       expect(product.description).to eq("<p>A description...</p>")
       expect(product.group_buy).to be_falsey
-      expect(product.master.unit_presentation).to eq("5kg")
+      expect(product.variants.first.unit_presentation).to eq("5kg")
     end
 
     it "creating an on-demand product" do
@@ -527,10 +527,6 @@ describe '
 
       page.find('a#new_image_link').click
 
-      uri = URI.parse(current_url)
-      # we stay on the same url as the new image content is loaded via an ajax call
-      expect("#{uri.path}?#{uri.query}").to eq spree.admin_product_images_path(product, filter)
-
       expected_cancel_link = Regexp.new(Regexp.escape(spree.admin_product_images_path(product,
                                                                                       filter)))
       expect(page).to have_link('Cancel', href: expected_cancel_link)
@@ -565,8 +561,8 @@ describe '
     it "loading edit product image page including url filter" do
       product = create(:simple_product, supplier: @supplier2)
       image = white_logo_file
-      image_object = Spree::Image.create(viewable_id: product.master.id,
+      image_object = Spree::Image.create(viewable_id: product.id,
-                                         viewable_type: 'Spree::Variant', alt: "position 1",
+                                         viewable_type: 'Spree::Product', alt: "position 1",
                                          attachment: image, position: 1)
 
       visit spree.admin_product_images_path(product, filter)
@@ -586,8 +582,8 @@ describe '
     it "updating a product image including url filter" do
       product = create(:simple_product, supplier: @supplier2)
       image = white_logo_file
-      image_object = Spree::Image.create(viewable_id: product.master.id,
+      image_object = Spree::Image.create(viewable_id: product.id,
-                                         viewable_type: 'Spree::Variant', alt: "position 1",
+                                         viewable_type: 'Spree::Product', alt: "position 1",
                                          attachment: image, position: 1)
 
       file_path = Rails.root + "spec/support/fixtures/thinking-cat.jpg"
@@ -608,7 +604,7 @@ describe '
       product = create(:simple_product, supplier: @supplier2)
 
       image = white_logo_file
-      Spree::Image.create(viewable_id: product.master.id, viewable_type: 'Spree::Variant',
+      Spree::Image.create(viewable_id: product.id, viewable_type: 'Spree::Product',
                           alt: "position 1", attachment: image, position: 1)
 
       visit spree.admin_product_images_path(product)
@@ -623,25 +619,25 @@ describe '
     it "deleting product images" do
       product = create(:simple_product, supplier: @supplier2)
       image = white_logo_file
-      Spree::Image.create(viewable_id: product.master.id, viewable_type: 'Spree::Variant',
+      Spree::Image.create(viewable_id: product.id, viewable_type: 'Spree::Product',
                           alt: "position 1", attachment: image, position: 1)
 
       visit spree.admin_product_images_path(product)
       expect(page).to have_selector "table.index td img"
-      expect(product.reload.images.count).to eq 1
+      expect(product.reload.image).to_not be_nil
 
       accept_alert do
         page.find('a.delete-resource').click
       end
 
       expect(page).to_not have_selector "table.index td img"
-      expect(product.reload.images.count).to eq 0
+      expect(product.reload.image).to be_nil
     end
 
     it "deleting product image including url filter" do
       product = create(:simple_product, supplier: @supplier2)
       image = white_logo_file
-      Spree::Image.create(viewable_id: product.master.id, viewable_type: 'Spree::Variant',
+      Spree::Image.create(viewable_id: product.id, viewable_type: 'Spree::Product',
                           alt: "position 1", attachment: image, position: 1)
 
       visit spree.admin_product_images_path(product, filter)

spec/system/admin/reports/packing_report_spec.rb

@@ -45,7 +45,7 @@ describe "Packing Reports" do
 
       create(:line_item_with_shipment, variant: variant1, quantity: 1, order: order1)
       create(:line_item_with_shipment, variant: variant2, quantity: 3, order: order1)
-      create(:line_item_with_shipment, variant: product2.master, quantity: 3, order: order2)
+      create(:line_item_with_shipment, variant: product2.variants.first, quantity: 3, order: order2)
     end
 
     describe "Pack By Customer" do

spec/system/admin/reports_spec.rb

@@ -568,7 +568,7 @@ describe '
     let(:order_cycle) {
       create(:simple_order_cycle, coordinator: distributor1,
                                   coordinator_fees: [enterprise_fee1, enterprise_fee2],
-                                  distributors: [distributor1], variants: [product1.master])
+                                  distributors: [distributor1], variants: [product1.variants.first])
     }
 
     let!(:zone) { create(:zone_with_member) }
@@ -752,7 +752,7 @@ describe '
 
     def xero_invoice_li_row(line_item, opts = {})
       tax_type = line_item.has_tax? ? 'GST on Income' : 'GST Free Income'
-      xero_invoice_row line_item.product.sku, line_item.product_and_full_name,
+      xero_invoice_row line_item.variant.sku, line_item.product_and_full_name,
                        line_item.price.to_s, line_item.quantity.to_s, tax_type, opts
     end
 

spec/system/admin/variants_spec.rb

@@ -60,11 +60,24 @@ describe '
 
       # Expect variant_weight to accept 3 decimal places
       fill_in 'variant_weight', with: '1.234'
+      fill_in 'unit_value_human', with: 1
       click_button 'Create'
 
       # Then the variant should have been created
       expect(page).to have_content "Variant \"#{product.name}\" has been successfully created!"
     end
+
+    it "show validation errors if present" do
+      product = create(:simple_product, variant_unit: "volume", variant_unit_scale: "1")
+      login_as_admin
+      visit spree.admin_product_variants_path product
+      click_link 'New Variant'
+
+      fill_in 'unit_value_human', with: 0
+      click_button 'Create'
+
+      expect(page).to have_content "Unit value must be greater than 0"
+    end
   end
 
   describe "viewing product variant" do

spec/system/consumer/caching/darkswarm_caching_spec.rb

@@ -57,9 +57,6 @@ describe "Darkswarm data caching", caching: true do
         expect(page).to have_content property.presentation
       end
 
-      # Update rows which should also update the timestamp.
-      # The timestamp represents seconds, so waiting one second is enough.
-      sleep 1
       taxon.update!(name: "Changed Taxon")
       property.update!(presentation: "Changed Property")
 

spec/views/spree/orders/show.html.haml_spec.rb

@@ -33,7 +33,7 @@ describe "spree/orders/show.html.haml" do
   end
 
   it "shows product images" do
-    order.line_items.first.variant.product.images << Spree::Image.new(
+    order.line_items.first.variant.product.image = Spree::Image.new(
       attachment: fixture_file_upload("logo.png", "image/png")
     )
 
@@ -43,7 +43,7 @@ describe "spree/orders/show.html.haml" do
   end
 
   it "handles broken images" do
-    image, = order.line_items.first.variant.product.images << Spree::Image.new(
+    image, = order.line_items.first.variant.product.image = Spree::Image.new(
       attachment: fixture_file_upload("logo.png", "image/png")
     )
     # This image is not "variable" and can't be resized: