Merge pull request #11369 from Matt-Yorkley/product-taxon

[Product Refactor] Primary Taxon

app/assets/javascripts/admin/bulk_product_update.js.coffee

@@ -48,7 +48,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
     params = {
       'q[name_cont]': $scope.q.query,
       'q[supplier_id_eq]': $scope.q.producerFilter,
-      'q[primary_taxon_id_eq]': $scope.q.categoryFilter,
+      'q[variants_primary_taxon_id_eq]': $scope.q.categoryFilter,
       'q[s]': $scope.sorting,
       import_date: $scope.q.importDateFilter,
       page: $scope.page,
@@ -136,6 +136,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
       on_hand: null
       price: null
       tax_category_id: null
+      category_id: null
     DisplayProperties.setShowVariants product.id, true
 
 
@@ -217,7 +218,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
         filters:
           'q[name_cont]': $scope.q.query
           'q[supplier_id_eq]': $scope.q.producerFilter
-          'q[primary_taxon_id_eq]': $scope.q.categoryFilter
+          'q[variants_primary_taxon_id_eq]': $scope.q.categoryFilter
           'q[s]': $scope.sorting
           import_date: $scope.q.importDateFilter
         page: $scope.page
@@ -332,9 +333,6 @@ filterSubmitProducts = (productsToFilter) ->
         if product.hasOwnProperty("on_demand") and filteredVariants.length == 0 #only update if no variants present
           filteredProduct.on_demand = product.on_demand
           hasUpdatableProperty = true
-        if product.hasOwnProperty("category_id")
-          filteredProduct.primary_taxon_id = product.category_id
-          hasUpdatableProperty = true
         if product.hasOwnProperty("inherits_properties")
           filteredProduct.inherits_properties = product.inherits_properties
           hasUpdatableProperty = true
@@ -375,6 +373,9 @@ filterSubmitVariant = (variant) ->
     if variant.hasOwnProperty("tax_category_id")
       filteredVariant.tax_category_id = variant.tax_category_id
       hasUpdatableProperty = true
+    if variant.hasOwnProperty("category_id")
+      filteredVariant.primary_taxon_id = variant.category_id
+      hasUpdatableProperty = true
     if variant.hasOwnProperty("display_as")
       filteredVariant.display_as = variant.display_as
       hasUpdatableProperty = true

app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee

@@ -68,7 +68,7 @@ angular.module('Darkswarm').controller "ProductsCtrl", ($scope, $sce, $filter, $
       per_page: $scope.per_page,
       'q[name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont]': $scope.query,
       'q[with_properties][]': $scope.activeProperties,
-      'q[primary_taxon_id_in_any][]': $scope.activeTaxons
+      'q[variants_primary_taxon_id_in_any][]': $scope.activeTaxons
     }
 
   $scope.searchKeypress = (e)->

app/controllers/api/v0/order_cycles_controller.rb

@@ -70,22 +70,7 @@ def producer_properties
       end
 
       def search_params
-        permitted_search_params = params.slice :q, :page, :per_page
-
-        if permitted_search_params.key? :q
-          permitted_search_params[:q].slice!(*permitted_ransack_params)
-        end
-
-        permitted_search_params
-      end
-
-      def permitted_ransack_params
-        [
-          "#{[:name, :meta_keywords, :variants_display_as,
-              :variants_display_name, :supplier_name]
-          .join('_or_')}_cont",
-          :with_properties, :primary_taxon_id_in_any
-        ]
+        params.slice :q, :page, :per_page
       end
 
       def distributor

app/models/product_import/entry_validator.rb

@@ -24,7 +24,6 @@ def initialize(current_user, import_time, spreadsheet_data, editable_enterprises
 
     def self.non_updatable_fields
       {
-        category: :primary_taxon_id,
         description: :description,
         unit_type: :variant_unit_scale,
         variant_unit_name: :variant_unit_name,
@@ -69,7 +68,7 @@ def enterprise_field
     def mark_as_new_variant(entry, product_id)
       variant_attributes = entry.assignable_attributes.except(
         'id', 'product_id', 'on_hand', 'on_demand', 'variant_unit', 'variant_unit_name',
-        'variant_unit_scale', 'primary_taxon_id'
+        'variant_unit_scale'
       )
       # Variant needs a product. Product needs to be assigned first in order for
       # delegate to work. name= will fail otherwise.
@@ -398,7 +397,7 @@ def mark_as_new_product(entry)
     def mark_as_existing_variant(entry, existing_variant)
       existing_variant.assign_attributes(
         entry.assignable_attributes.except('id', 'product_id', 'variant_unit', 'variant_unit_name',
-                                           'variant_unit_scale', 'primary_taxon_id')
+                                           'variant_unit_scale')
       )
       check_on_hand_nil(entry, existing_variant)
 

app/models/spree/product.rb

@@ -28,16 +28,11 @@ class Product < ApplicationRecord
 
     acts_as_paranoid
 
-    after_create :ensure_standard_variant
-    around_destroy :destruction
-    after_save :update_units
-
-    searchable_attributes :supplier_id, :primary_taxon_id, :meta_keywords, :sku
-    searchable_associations :supplier, :properties, :primary_taxon, :variants
+    searchable_attributes :supplier_id, :meta_keywords, :sku
+    searchable_associations :supplier, :properties, :variants
     searchable_scopes :active, :with_properties
 
     belongs_to :supplier, class_name: 'Enterprise', optional: false, touch: true
-    belongs_to :primary_taxon, class_name: 'Spree::Taxon', optional: false, touch: true
 
     has_one :image, class_name: "Spree::Image", as: :viewable, dependent: :destroy
 
@@ -77,7 +72,11 @@ class Product < ApplicationRecord
     # 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, :tax_category_id,
-                  :shipping_category_id
+                  :shipping_category_id, :primary_taxon_id
+
+    after_create :ensure_standard_variant
+    around_destroy :destruction
+    after_save :update_units
 
     scope :with_properties, ->(*property_ids) {
       left_outer_joins(:product_properties).
@@ -284,6 +283,7 @@ def ensure_standard_variant
       variant.unit_description = unit_description
       variant.tax_category_id = tax_category_id
       variant.shipping_category_id = shipping_category_id
+      variant.primary_taxon_id = primary_taxon_id
       variants << variant
     end
 

app/models/spree/taxon.rb

@@ -7,9 +7,12 @@ class Taxon < ApplicationRecord
     acts_as_nested_set dependent: :destroy
 
     belongs_to :taxonomy, class_name: 'Spree::Taxonomy', touch: true
-    has_many :products, class_name: "Spree::Product", foreign_key: "primary_taxon_id",
+
+    has_many :variants, class_name: "Spree::Variant", foreign_key: "primary_taxon_id",
                         inverse_of: :primary_taxon, dependent: :restrict_with_error
 
+    has_many :products, through: :variants, dependent: nil
+
     before_create :set_permalink
 
     validates :name, presence: true
@@ -77,7 +80,7 @@ def self.distributed_taxons(which_taxons = :all)
 
       taxons = Spree::Taxon
         .select("DISTINCT spree_taxons.id, ents_and_vars.enterprise_id")
-        .joins(products: :variants)
+        .joins(: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,8 +13,8 @@ class Variant < ApplicationRecord
 
     acts_as_paranoid
 
-    searchable_attributes :sku, :display_as, :display_name
-    searchable_associations :product, :default_price
+    searchable_attributes :sku, :display_as, :display_name, :primary_taxon_id
+    searchable_associations :product, :default_price, :primary_taxon
     searchable_scopes :active, :deleted
 
     NAME_FIELDS = ["display_name", "display_as", "weight", "unit_value", "unit_description"].freeze
@@ -28,6 +28,7 @@ class Variant < ApplicationRecord
     belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product'
     belongs_to :tax_category, class_name: 'Spree::TaxCategory'
     belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', optional: false
+    belongs_to :primary_taxon, class_name: 'Spree::Taxon', touch: true, optional: false
 
     delegate :name, :name=, :description, :description=, :meta_keywords, to: :product
 
@@ -82,6 +83,7 @@ class Variant < ApplicationRecord
     before_validation :ensure_unit_value
     before_validation :update_weight_from_unit_value, if: ->(v) { v.product.present? }
     before_validation :convert_variant_weight_to_decimal
+    before_validation :assign_related_taxon, if: ->(v) { v.primary_taxon.blank? }
 
     before_save :assign_units, if: ->(variant) {
       variant.new_record? || variant.changed_attributes.keys.intersection(NAME_FIELDS).any?
@@ -208,6 +210,10 @@ def total_on_hand
 
     private
 
+    def assign_related_taxon
+      self.primary_taxon ||= product.variants.last&.primary_taxon
+    end
+
     def check_currency
       return unless currency.nil?
 

app/queries/product_scope_query.rb

@@ -20,7 +20,7 @@ def bulk_products
 
     product_query.
       ransack(query_params_with_defaults).
-      result
+      result(distinct: true)
   end
 
   def find_product

app/reflexes/products_reflex.rb

@@ -152,7 +152,7 @@ def categories
 
   def fetch_products
     product_query = OpenFoodNetwork::Permissions.new(current_user)
-      .editable_products.merge(product_scope).ransack(ransack_query).result
+      .editable_products.merge(product_scope).ransack(ransack_query).result(distinct: true)
     @pagy, @products = pagy(product_query.order(:name), items: @per_page, page: @page,
                                                         size: [1, 2, 2, 1])
   end
@@ -173,7 +173,7 @@ def ransack_query
     if @search_term.present?
       query.merge!(Spree::Variant::SEARCH_KEY => @search_term)
     end
-    query.merge!(primary_taxon_id_in: @category_id) if @category_id.present?
+    query.merge!(variants_primary_taxon_id_in: @category_id) if @category_id.present?
     query
   end
 

app/serializers/api/admin/product_serializer.rb

@@ -8,7 +8,6 @@ class ProductSerializer < ActiveModel::Serializer
                  :thumb_url, :variants
 
       has_one :supplier, key: :producer_id, embed: :id
-      has_one :primary_taxon, key: :category_id, embed: :id
 
       def variants
         ActiveModel::ArraySerializer.new(

app/serializers/api/admin/variant_serializer.rb

@@ -8,6 +8,8 @@ class VariantSerializer < ActiveModel::Serializer
                  :display_as, :display_name, :name_to_display, :variant_overrides_count,
                  :price, :on_demand, :on_hand, :in_stock, :stock_location_id, :stock_location_name
 
+      has_one :primary_taxon, key: :category_id, embed: :id
+
       def name
         if object.full_name.present?
           "#{object.name} - #{object.full_name}"

app/serializers/api/product_serializer.rb

@@ -9,8 +9,6 @@ class Api::ProductSerializer < ActiveModel::Serializer
 
   has_many :variants, serializer: Api::VariantSerializer
 
-  has_one :primary_taxon, serializer: Api::TaxonSerializer
-
   has_one :image, serializer: Api::ImageSerializer
   has_one :supplier, serializer: Api::IdSerializer
 

app/services/order_cycles/distributed_products_service.rb

@@ -15,6 +15,22 @@ def products_relation
       Spree::Product.where(id: stocked_products).group("spree_products.id")
     end
 
+    # Joins on the first product variant to allow us to filter product by taxon. This is so
+    # enterprise can display product sorted by category in a custom order on their shopfront.
+    #
+    # Caveat, the category sorting won't work properly if there are multiple variant with different
+    # category for a given product.
+    #
+    def products_taxons_relation
+      Spree::Product.where(id: stocked_products).
+        joins("LEFT JOIN (
+                 SELECT DISTINCT ON(product_id) id, product_id, primary_taxon_id
+                 FROM spree_variants WHERE deleted_at IS NULL
+               ) first_variant ON spree_products.id = first_variant.product_id").
+        select("spree_products.*, first_variant.primary_taxon_id").
+        group("spree_products.id, first_variant.primary_taxon_id")
+    end
+
     def variants_relation
       order_cycle.
         variants_distributed_by(distributor).

app/services/permitted_attributes/variant.rb

@@ -7,7 +7,7 @@ def self.attributes
         :id, :sku, :on_hand, :on_demand, :shipping_category_id,
         :price, :unit_value, :unit_description,
         :display_name, :display_as, :tax_category_id,
-        :weight, :height, :width, :depth
+        :weight, :height, :width, :depth, :taxon_ids, :primary_taxon_id
       ]
     end
   end

app/services/products_renderer.rb

@@ -35,7 +35,7 @@ def products
 
     @products ||= begin
       results = distributed_products.
-        products_relation.
+        products_taxons_relation.
         order(Arel.sql(products_order))
 
       filter_and_paginate(results).
@@ -54,7 +54,7 @@ def enterprise_fee_calculator
   def filter_and_paginate(query)
     results = query.ransack(args[:q]).result
 
-    _pagy, paginated_results = pagy(
+    _pagy, paginated_results = pagy_arel(
       results,
       page: args[:page] || 1,
       items: args[:per_page] || DEFAULT_PER_PAGE
@@ -78,7 +78,7 @@ def products_order
           distributor.preferred_shopfront_taxon_order.present?
       distributor
         .preferred_shopfront_taxon_order
-        .split(",").map { |id| "spree_products.primary_taxon_id=#{id} DESC" }
+        .split(",").map { |id| "first_variant.primary_taxon_id=#{id} DESC" }
         .join(", ") + ", spree_products.name ASC, spree_products.id ASC"
     else
       "spree_products.name ASC, spree_products.id"

app/services/sets/product_set.rb

@@ -46,18 +46,12 @@ def collection_attributes=(attributes)
     #   variant.update( { price: xx.x } )
     #
     def update_product_attributes(attributes)
-      split_taxon_ids!(attributes)
-
       product = find_model(@collection, attributes[:id])
       return if product.nil?
 
       update_product(product, attributes)
     end
 
-    def split_taxon_ids!(attributes)
-      attributes[:taxon_ids] = attributes[:taxon_ids].split(',') if attributes[:taxon_ids].present?
-    end
-
     def update_product(product, attributes)
       return false unless update_product_only_attributes(product, attributes)
 

app/views/admin/products_v3/_product_row.html.haml

@@ -30,7 +30,7 @@
 %td.align-left
   .content= product.supplier&.name
 %td.align-left
-  .content= product.primary_taxon&.name
+  -# empty
 %td.align-left
 %td.align-left
   .content= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)

app/views/admin/products_v3/_variant_row.html.haml

@@ -42,7 +42,7 @@
 %td.align-left
   .content= variant.product.supplier&.name # same as product
 %td.align-left
-  -# empty
+  .content= variant.primary_taxon&.name
 %td.align-left
   .content= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
 %td.align-left

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

@@ -27,8 +27,6 @@
           = f.text_field :variant_unit_name, {placeholder: t('admin.products.unit_name_placeholder')}
           = f.error_message_on :variant_unit_name
 
-    = render 'spree/admin/products/primary_taxon_form', f: f
-
     = f.field_container :supplier do
       = f.label :supplier, t(:spree_admin_supplier)
       %br

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

@@ -1,6 +1,6 @@
 = f.field_container :primary_taxon do
-  = f.label :primary_taxon, t('.product_category')
+  = f.label :primary_taxon_id, t('.product_category')
   %span.required *
   %br
   = f.collection_select(:primary_taxon_id, Spree::Taxon.order(:name), :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"})
-  = f.error_message_on :primary_taxon
+  = f.error_message_on :primary_taxon_id

app/views/spree/admin/products/index/_products_product.html.haml

@@ -24,7 +24,6 @@
   %td.on_demand{ 'ng-show' => 'columns.on_demand.visible' }
     %input.field{ 'ng-model' => 'product.on_demand', :name => 'on_demand', 'ofn-track-product' => 'on_demand', :type => 'checkbox', 'ng-hide' => 'hasVariants(product)' }
   %td.category{ 'ng-if' => 'columns.category.visible' }
-    %input.fullwidth{ :type => 'text', id: "p{{product.id}}_category_id", 'ng-model' => 'product.category_id', 'ofn-taxon-autocomplete' => '', 'ofn-track-product' => 'category_id', 'multiple-selection' => 'false', placeholder: 'Category' }
   %td.tax_category{ 'ng-if' => 'columns.tax_category.visible' }
   %td.inherits_properties{ 'ng-show' => 'columns.inherits_properties.visible' }
     %input{ 'ng-model' => 'product.inherits_properties', :name => 'inherits_properties', 'ofn-track-product' => 'inherits_properties', type: "checkbox" }

app/views/spree/admin/products/index/_products_variant.html.haml

@@ -21,6 +21,7 @@
   %td{ 'ng-show' => 'columns.on_demand.visible' }
     %input.field{ 'ng-model' => 'variant.on_demand', :name => 'variant_on_demand', 'ofn-track-variant' => 'on_demand', :type => 'checkbox' }
   %td{ 'ng-show' => 'columns.category.visible' }
+    %input.fullwidth{ type: 'text', id: "p{{product.id}}_category_id", 'ng-model' => 'variant.category_id', 'ofn-taxon-autocomplete' => '', 'ofn-track-variant' => 'category_id', 'multiple-selection' => 'false', placeholder: 'Category' }
   %td{ 'ng-show' => 'columns.tax_category.visible' }
     %select.select2{ name: 'variant_tax_category_id', "ofn-track-variant": 'tax_category_id', "ng-model": 'variant.tax_category_id', "ng-options": 'tax_category.id as tax_category.name for tax_category in tax_categories' }
       %option{ value: '' }= t(:none)

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

@@ -72,4 +72,8 @@
     = f.label :shipping_category_id, t(:shipping_categories)
     = f.collection_select(:shipping_category_id, @shipping_categories, :id, :name, {}, { class: 'select2 fullwidth' })
 
+  .field
+    = f.label :primary_taxon, t('spree.admin.products.primary_taxon_form.product_category')
+    = f.collection_select(:primary_taxon_id, Spree::Taxon.order(:name), :id, :name, { include_blank: true }, { class: "select2 fullwidth" })
+
 .clear