Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Product Refactor] Primary Taxon #11369

Merged
merged 35 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2ef2663
Move primary taxon to variant
Matt-Yorkley Aug 7, 2023
c805486
Update attribute translations
Matt-Yorkley Aug 7, 2023
b309444
Update admin forms
Matt-Yorkley Aug 7, 2023
2707c31
Apply taxon to new variant when creating a new product
Matt-Yorkley Aug 7, 2023
3a38eeb
Remove products taxon null constraint
Matt-Yorkley Aug 7, 2023
65731f6
Update serializers and views for admin products index
Matt-Yorkley Aug 7, 2023
0dbbd5e
Migrate primary taxon id from products to variants
Matt-Yorkley Aug 7, 2023
3b71587
Update taxon association
Matt-Yorkley Aug 7, 2023
cd60131
Fix factory issues
Matt-Yorkley Aug 7, 2023
6b3e33f
Update Taxon associations and joins
Matt-Yorkley Aug 8, 2023
861f2ae
Update product import spec
Matt-Yorkley Aug 8, 2023
269d3ce
Update enterprise caching spec
Matt-Yorkley Aug 8, 2023
b22c426
Update taxon querying in reports
Matt-Yorkley Aug 8, 2023
d9899e8
Update more specs
Matt-Yorkley Aug 8, 2023
d281e9d
Adjust factory
Matt-Yorkley Aug 8, 2023
6e7b978
Update DFC product importer
Matt-Yorkley Aug 8, 2023
c086440
Update bulk products JS spec
Matt-Yorkley Aug 8, 2023
d4dd3cc
Update products v3 table
Matt-Yorkley Aug 8, 2023
c5ec7e3
Update product import
Matt-Yorkley Aug 8, 2023
6025491
Update product serializer
Matt-Yorkley Aug 8, 2023
78495d0
Drop unnecessary params filtering
Matt-Yorkley Aug 8, 2023
fb09a7f
Fix product filtering
Matt-Yorkley Aug 8, 2023
02abe5c
Remove dead code related to multiple product taxons
Matt-Yorkley Aug 9, 2023
3f2a578
Apply taxon from sibling variant if none is provided on variant creation
Matt-Yorkley Aug 9, 2023
2743a81
Disable sorting by user-defined product category order
Matt-Yorkley Aug 9, 2023
a55931c
Reinstate sorting by arbitrary list of product categories
Matt-Yorkley Aug 10, 2023
d959ee2
Fix rubocop warnings
Matt-Yorkley Sep 5, 2023
8a364a5
Fix product touch spec
Matt-Yorkley Sep 5, 2023
ac4ec36
Update ProductScopeQuery spec
Matt-Yorkley Sep 5, 2023
d3e62c4
Add test for persisting taxon on variant during product creation
Matt-Yorkley Sep 5, 2023
6d1249e
Update DFC supplied product
rioug Feb 20, 2024
8c05838
Move the category to the variant row
rioug Feb 21, 2024
678fa37
Fix duplication issue
rioug Feb 21, 2024
100239c
Fix #bulk_product duplicate
rioug Mar 12, 2024
484c037
Fix duplication coming from a rebase error
rioug Apr 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions app/assets/javascripts/admin/bulk_product_update.js.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)->
Expand Down
17 changes: 1 addition & 16 deletions app/controllers/api/v0/order_cycles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
dacook marked this conversation as resolved.
Show resolved Hide resolved
end

def distributor
Expand Down
5 changes: 2 additions & 3 deletions app/models/product_import/entry_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)

Expand Down
16 changes: 8 additions & 8 deletions app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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

Expand Down
7 changes: 5 additions & 2 deletions app/models/spree/taxon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
10 changes: 8 additions & 2 deletions app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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?

Expand Down
2 changes: 1 addition & 1 deletion app/queries/product_scope_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def bulk_products

product_query.
ransack(query_params_with_defaults).
result
result(distinct: true)
end

def find_product
Expand Down
4 changes: 2 additions & 2 deletions app/reflexes/products_reflex.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
1 change: 0 additions & 1 deletion app/serializers/api/admin/product_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions app/serializers/api/admin/variant_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
2 changes: 0 additions & 2 deletions app/serializers/api/product_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 16 additions & 0 deletions app/services/order_cycles/distributed_products_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
2 changes: 1 addition & 1 deletion app/services/permitted_attributes/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions app/services/products_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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(
dacook marked this conversation as resolved.
Show resolved Hide resolved
results,
page: args[:page] || 1,
items: args[:per_page] || DEFAULT_PER_PAGE
Expand All @@ -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"
Expand Down
6 changes: 0 additions & 6 deletions app/services/sets/product_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/products_v3/_product_row.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/products_v3/_variant_row.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions app/views/spree/admin/products/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/views/spree/admin/products/_primary_taxon_form.html.haml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions app/views/spree/admin/variants/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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