From 2ef266390d5d940c1cfeca86c6e021af8c4298c6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:03:15 +0100 Subject: [PATCH 01/35] Move primary taxon to variant --- app/models/spree/product.rb | 13 ++++++------- app/models/spree/variant.rb | 5 +++-- db/migrate/20230803191831_add_taxons_to_variants.rb | 5 +++++ db/schema.rb | 3 +++ 4 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20230803191831_add_taxons_to_variants.rb diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index fcc133aa8d..129edb189b 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -28,16 +28,11 @@ module Spree 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 @@ -79,6 +74,10 @@ module Spree attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id, :shipping_category_id + after_create :ensure_standard_variant + around_destroy :destruction + after_save :update_units + scope :with_properties, ->(*property_ids) { left_outer_joins(:product_properties). left_outer_joins(:supplier_properties). diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 8eb290a123..6fc36cc27c 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -13,8 +13,8 @@ module Spree 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 @@ module Spree 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 diff --git a/db/migrate/20230803191831_add_taxons_to_variants.rb b/db/migrate/20230803191831_add_taxons_to_variants.rb new file mode 100644 index 0000000000..c9193ac344 --- /dev/null +++ b/db/migrate/20230803191831_add_taxons_to_variants.rb @@ -0,0 +1,5 @@ +class AddTaxonsToVariants < ActiveRecord::Migration[7.0] + def change + add_reference :spree_variants, :primary_taxon, foreign_key: { to_table: :spree_taxons } + end +end diff --git a/db/schema.rb b/db/schema.rb index 5c9ef39bf5..092a5c9814 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -979,6 +979,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_13_044159) do t.datetime "updated_at", default: -> { "now()" }, null: false t.bigint "tax_category_id" t.bigint "shipping_category_id" + t.bigint "primary_taxon_id" + t.index ["primary_taxon_id"], name: "index_spree_variants_on_primary_taxon_id" t.index ["product_id"], name: "index_variants_on_product_id" t.index ["shipping_category_id"], name: "index_spree_variants_on_shipping_category_id" t.index ["sku"], name: "index_spree_variants_on_sku" @@ -1225,6 +1227,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_13_044159) do add_foreign_key "spree_variants", "spree_products", column: "product_id", name: "spree_variants_product_id_fk" add_foreign_key "spree_variants", "spree_shipping_categories", column: "shipping_category_id" add_foreign_key "spree_variants", "spree_tax_categories", column: "tax_category_id" + add_foreign_key "spree_variants", "spree_taxons", column: "primary_taxon_id" add_foreign_key "spree_zone_members", "spree_zones", column: "zone_id", name: "spree_zone_members_zone_id_fk" add_foreign_key "subscription_line_items", "spree_variants", column: "variant_id", name: "subscription_line_items_variant_id_fk" add_foreign_key "subscription_line_items", "subscriptions", name: "subscription_line_items_subscription_id_fk" From c805486f0a34d84102a898d9cb255fbb3cef027a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:46:05 +0100 Subject: [PATCH 02/35] Update attribute translations --- config/locales/en.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index c1cccb66f6..949d7af400 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -72,6 +72,9 @@ en: variant_unit: "Variant Unit" variant_unit_name: "Variant Unit Name" unit_value: "Unit value" + spree/variant: + primary_taxon: "Product Category" + shipping_category_id: "Shipping Category" spree/credit_card: base: "Credit Card" number: "Number" From b30944471dcc6a9705b71591feae1652c476285f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 12:24:20 +0100 Subject: [PATCH 03/35] Update admin forms --- app/services/permitted_attributes/variant.rb | 2 +- app/views/spree/admin/products/_form.html.haml | 2 -- app/views/spree/admin/products/_primary_taxon_form.html.haml | 4 ++-- app/views/spree/admin/variants/_form.html.haml | 4 ++++ 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/services/permitted_attributes/variant.rb b/app/services/permitted_attributes/variant.rb index 47e4481e5b..21f0f997b8 100644 --- a/app/services/permitted_attributes/variant.rb +++ b/app/services/permitted_attributes/variant.rb @@ -7,7 +7,7 @@ module PermittedAttributes :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 diff --git a/app/views/spree/admin/products/_form.html.haml b/app/views/spree/admin/products/_form.html.haml index 1bd362b7e7..1f3492c123 100644 --- a/app/views/spree/admin/products/_form.html.haml +++ b/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 diff --git a/app/views/spree/admin/products/_primary_taxon_form.html.haml b/app/views/spree/admin/products/_primary_taxon_form.html.haml index e24f84a6a2..fa4b0b1976 100644 --- a/app/views/spree/admin/products/_primary_taxon_form.html.haml +++ b/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 diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index 1e9aa79236..76a1ebfafc 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/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 From 2707c3137a8e671cd95579e49154b75ba6234e44 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 12:24:53 +0100 Subject: [PATCH 04/35] Apply taxon to new variant when creating a new product --- app/models/spree/product.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 129edb189b..496194c7ec 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -72,7 +72,7 @@ module Spree # 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 @@ -283,6 +283,7 @@ module Spree 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 From 3a38eeb248a2afe29663cda3b009eb0c8b2e5493 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 12:43:04 +0100 Subject: [PATCH 05/35] Remove products taxon null constraint --- db/migrate/20230807114014_remove_taxon_constraint.rb | 5 +++++ db/schema.rb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20230807114014_remove_taxon_constraint.rb diff --git a/db/migrate/20230807114014_remove_taxon_constraint.rb b/db/migrate/20230807114014_remove_taxon_constraint.rb new file mode 100644 index 0000000000..a2dabbbb35 --- /dev/null +++ b/db/migrate/20230807114014_remove_taxon_constraint.rb @@ -0,0 +1,5 @@ +class RemoveTaxonConstraint < ActiveRecord::Migration[7.0] + def change + change_column_null :spree_products, :primary_taxon_id, true + end +end diff --git a/db/schema.rb b/db/schema.rb index 092a5c9814..51491dce7f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -698,7 +698,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_13_044159) do t.float "variant_unit_scale" t.string "variant_unit_name", limit: 255 t.text "notes" - t.integer "primary_taxon_id", null: false + t.integer "primary_taxon_id" t.boolean "inherits_properties", default: true, null: false t.string "sku", limit: 255, default: "", null: false t.index ["deleted_at"], name: "index_products_on_deleted_at" From 65731f6c5290f78f54ffc9b94191c013987d09a6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 13:18:53 +0100 Subject: [PATCH 06/35] Update serializers and views for admin products index --- app/assets/javascripts/admin/bulk_product_update.js.coffee | 7 ++++--- app/serializers/api/admin/product_serializer.rb | 1 - app/serializers/api/admin/variant_serializer.rb | 2 ++ .../spree/admin/products/index/_products_product.html.haml | 1 - .../spree/admin/products/index/_products_variant.html.haml | 1 + 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 910fb39f8b..4598669203 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -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 @@ -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 diff --git a/app/serializers/api/admin/product_serializer.rb b/app/serializers/api/admin/product_serializer.rb index b3eb0225e6..356c0debdf 100644 --- a/app/serializers/api/admin/product_serializer.rb +++ b/app/serializers/api/admin/product_serializer.rb @@ -8,7 +8,6 @@ module Api :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( diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb index 23b5dbd1e3..06732aa7b5 100644 --- a/app/serializers/api/admin/variant_serializer.rb +++ b/app/serializers/api/admin/variant_serializer.rb @@ -8,6 +8,8 @@ module Api :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}" diff --git a/app/views/spree/admin/products/index/_products_product.html.haml b/app/views/spree/admin/products/index/_products_product.html.haml index 99ca44c24c..e399d64aa3 100644 --- a/app/views/spree/admin/products/index/_products_product.html.haml +++ b/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" } diff --git a/app/views/spree/admin/products/index/_products_variant.html.haml b/app/views/spree/admin/products/index/_products_variant.html.haml index e66499f7c9..48c732c155 100644 --- a/app/views/spree/admin/products/index/_products_variant.html.haml +++ b/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) From 0dbbd5ed3b159fab1dc7e881c833fe3e4ba8adbe Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 14:00:21 +0100 Subject: [PATCH 07/35] Migrate primary taxon id from products to variants --- .../20230807122052_migrate_product_taxons.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 db/migrate/20230807122052_migrate_product_taxons.rb diff --git a/db/migrate/20230807122052_migrate_product_taxons.rb b/db/migrate/20230807122052_migrate_product_taxons.rb new file mode 100644 index 0000000000..5bc982a805 --- /dev/null +++ b/db/migrate/20230807122052_migrate_product_taxons.rb @@ -0,0 +1,15 @@ +class MigrateProductTaxons < ActiveRecord::Migration[7.0] + def up + migrate_primary_taxon + end + + def migrate_primary_taxon + ActiveRecord::Base.connection.execute(<<-SQL + UPDATE spree_variants + SET primary_taxon_id = spree_products.primary_taxon_id + FROM spree_products + WHERE spree_variants.product_id = spree_products.id + SQL + ) + end +end From 3b715875adc85a30f9cf0c1db28a806c3e6de019 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 19:04:35 +0100 Subject: [PATCH 08/35] Update taxon association --- app/models/spree/taxon.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/taxon.rb b/app/models/spree/taxon.rb index 2ed96f4d57..7023959a26 100644 --- a/app/models/spree/taxon.rb +++ b/app/models/spree/taxon.rb @@ -7,7 +7,7 @@ module Spree 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 before_create :set_permalink From cd601319f3a1ce0bc49864c9bb590aec9cfaa781 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 7 Aug 2023 19:05:00 +0100 Subject: [PATCH 09/35] Fix factory issues --- spec/factories/product_factory.rb | 8 +++++++- spec/factories/variant_factory.rb | 5 +++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/spec/factories/product_factory.rb b/spec/factories/product_factory.rb index 320060563f..8e1d5e64c7 100644 --- a/spec/factories/product_factory.rb +++ b/spec/factories/product_factory.rb @@ -3,13 +3,18 @@ FactoryBot.define do factory :base_product, class: Spree::Product do sequence(:name) { |n| "Product ##{n} - #{Kernel.rand(9999)}" } + + transient do + primary_taxon { nil } + end + + primary_taxon_id { |p| (p.primary_taxon || Spree::Taxon.first || create(:taxon)).id } description { generate(:random_description) } price { 19.99 } sku { 'ABC' } deleted_at { nil } supplier { Enterprise.is_primary_producer.first || FactoryBot.create(:supplier_enterprise) } - primary_taxon { Spree::Taxon.first || FactoryBot.create(:taxon) } unit_value { 1 } unit_description { '' } @@ -48,6 +53,7 @@ FactoryBot.define do on_demand { false } on_hand { 5 } end + after(:create) do |product, evaluator| product.variants.first.on_demand = evaluator.on_demand product.variants.first.on_hand = evaluator.on_hand diff --git a/spec/factories/variant_factory.rb b/spec/factories/variant_factory.rb index cd4b322a02..ec2ebf8a4d 100644 --- a/spec/factories/variant_factory.rb +++ b/spec/factories/variant_factory.rb @@ -11,7 +11,8 @@ FactoryBot.define do width { generate(:random_float) } depth { generate(:random_float) } - product { |p| p.association(:base_product) } + primary_taxon { Spree::Taxon.first || FactoryBot.create(:taxon) } + product { |v| create(:product, primary_taxon_id: v.primary_taxon.id) } # ensure stock item will be created for this variant before(:create) { create(:stock_location) if Spree::StockLocation.count.zero? } @@ -22,7 +23,7 @@ FactoryBot.define do on_hand { 5 } end - product { |p| p.association(:product) } + product { |v| create(:product, primary_taxon_id: v.primary_taxon.id) } unit_value { 1 } unit_description { '' } From 6b3e33f60756c3bc684c4c1e613cc3f795d00570 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 11:47:41 +0100 Subject: [PATCH 10/35] Update Taxon associations and joins --- app/models/spree/taxon.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/spree/taxon.rb b/app/models/spree/taxon.rb index 7023959a26..0ccb9e6499 100644 --- a/app/models/spree/taxon.rb +++ b/app/models/spree/taxon.rb @@ -7,9 +7,12 @@ module Spree acts_as_nested_set dependent: :destroy belongs_to :taxonomy, class_name: 'Spree::Taxonomy', touch: true + 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 @@ module Spree 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") From 861f2aef018c608e64dc288418ce3b157487fb3a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 11:53:48 +0100 Subject: [PATCH 11/35] Update product import spec --- spec/models/product_importer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index c96e4b1870..593a0de53b 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -284,7 +284,7 @@ describe ProductImport::ProductImporter do carrots = Spree::Product.find_by(name: 'Good Carrots') expect(carrots.on_hand).to eq 5 expect(carrots.variants.first.price).to eq 3.20 - expect(carrots.primary_taxon.name).to eq "Vegetables" + expect(carrots.variants.first.primary_taxon.name).to eq "Vegetables" expect(carrots.variants.first.shipping_category).to eq shipping_category expect(carrots.supplier).to eq enterprise expect(carrots.variants.first.unit_presentation).to eq "500g" From 269d3ced0c954fcc6ed7e9abc538a88c26ef75ee Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 11:57:03 +0100 Subject: [PATCH 12/35] Update enterprise caching spec --- spec/models/enterprise_caching_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/models/enterprise_caching_spec.rb b/spec/models/enterprise_caching_spec.rb index 8fab0abb17..e66063abc1 100644 --- a/spec/models/enterprise_caching_spec.rb +++ b/spec/models/enterprise_caching_spec.rb @@ -12,6 +12,7 @@ describe Enterprise do describe "with a supplied product" do let(:product) { create(:simple_product, supplier: enterprise, primary_taxon_id: taxon.id) } + let(:variant) { product.variants.first } let(:property) { product.product_properties.last } let(:producer_property) { enterprise.producer_properties.last } @@ -20,9 +21,9 @@ describe Enterprise do enterprise.set_producer_property 'Biodynamic', 'ASDF 4321' end - it "touches enterprise when a taxon on that product changes" do + it "touches enterprise when a taxon on that variant changes" do expect { - later { product.update(primary_taxon_id: taxon2.id) } + later { variant.update(primary_taxon_id: taxon2.id) } }.to change { enterprise.reload.updated_at } end @@ -47,6 +48,7 @@ describe Enterprise do describe "with a distributed product" do let(:product) { create(:simple_product, primary_taxon_id: taxon.id) } + let(:variant) { product.variants.first } let(:oc) { create(:simple_order_cycle, distributors: [enterprise], variants: [product.variants.first]) @@ -63,9 +65,9 @@ describe Enterprise do context "with an order cycle" do before { oc } - it "touches enterprise when a taxon on that product changes" do + it "touches enterprise when a taxon on that variant changes" do expect { - later { product.update(primary_taxon_id: taxon2.id) } + later { variant.update(primary_taxon_id: taxon2.id) } }.to change { enterprise.reload.updated_at } end From b22c42613a56c3290137284a1d79c6e8b61d6542 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 12:34:08 +0100 Subject: [PATCH 13/35] Update taxon querying in reports --- lib/reporting/reports/products_and_inventory/base.rb | 2 +- lib/reporting/reports/products_and_inventory/lettuce_share.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/reporting/reports/products_and_inventory/base.rb b/lib/reporting/reports/products_and_inventory/base.rb index a65b113f2a..555e30cd68 100644 --- a/lib/reporting/reports/products_and_inventory/base.rb +++ b/lib/reporting/reports/products_and_inventory/base.rb @@ -17,7 +17,7 @@ module Reporting producer_suburb: proc { |variant| variant.product.supplier.address.city }, product: proc { |variant| variant.product.name }, product_properties: proc { |v| v.product.properties.map(&:name).join(", ") }, - taxons: proc { |variant| variant.product.primary_taxon.name }, + taxons: proc { |variant| variant.primary_taxon.name }, variant_value: proc { |variant| variant.full_name }, price: proc { |variant| variant.price }, group_buy_unit_quantity: proc { |variant| variant.product.group_buy_unit_size }, diff --git a/lib/reporting/reports/products_and_inventory/lettuce_share.rb b/lib/reporting/reports/products_and_inventory/lettuce_share.rb index 8311ff111d..366332fd55 100644 --- a/lib/reporting/reports/products_and_inventory/lettuce_share.rb +++ b/lib/reporting/reports/products_and_inventory/lettuce_share.rb @@ -32,7 +32,7 @@ module Reporting total: proc { |_variant| '' }, gst: proc { |variant| gst(variant) }, grower: proc { |variant| grower_and_method(variant) }, - taxon: proc { |variant| variant.product.primary_taxon.name } + taxon: proc { |variant| variant.primary_taxon.name } } end From d9899e8af61b6134b5e0b14a8ce280930bbfda22 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 12:10:05 +0100 Subject: [PATCH 14/35] Update more specs --- spec/controllers/api/v0/products_controller_spec.rb | 7 ++++--- spec/controllers/api/v0/variants_controller_spec.rb | 3 ++- .../spree/admin/products_controller_spec.rb | 4 +++- .../spree/admin/variants_controller_spec.rb | 4 +++- .../reports/products_and_inventory_report_spec.rb | 2 +- spec/models/spree/product_spec.rb | 7 +------ spec/models/spree/taxon_spec.rb | 6 +++--- spec/services/products_renderer_spec.rb | 6 ------ spec/system/admin/bulk_product_update_spec.rb | 3 --- spec/system/admin/products_spec.rb | 2 +- spec/system/admin/reports_spec.rb | 12 ++++++------ spec/system/admin/variants_spec.rb | 4 ++++ spec/system/consumer/caching/shops_caching_spec.rb | 5 +++-- 13 files changed, 31 insertions(+), 34 deletions(-) diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index af6dbfe032..96f22a9fe8 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -25,6 +25,7 @@ describe Api::V0::ProductsController, type: :controller do end context "as a normal user" do + let(:taxon) { create(:taxon) } let(:attachment) { fixture_file_upload("thinking-cat.jpg") } before do @@ -34,9 +35,10 @@ describe Api::V0::ProductsController, type: :controller do it "gets a single product" do product.create_image!(attachment:) - product.variants.create!(unit_value: "1", unit_description: "thing", price: 1) + product.variants.create!(unit_value: "1", unit_description: "thing", price: 1, primary_taxon: taxon) product.variants.first.images.create!(attachment:) product.set_property("spree", "rocks") + api_get :show, id: product.to_param expect(all_attributes.all?{ |attr| json_response.keys.include? attr }).to eq(true) @@ -117,8 +119,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", "primary_taxon", "supplier", "variant_unit", - "price"]) + expect(errors.keys).to match_array(["name", "supplier", "variant_unit", "price"]) end it "can update a product" do diff --git a/spec/controllers/api/v0/variants_controller_spec.rb b/spec/controllers/api/v0/variants_controller_spec.rb index 9272b0b522..1a90dd704a 100644 --- a/spec/controllers/api/v0/variants_controller_spec.rb +++ b/spec/controllers/api/v0/variants_controller_spec.rb @@ -127,6 +127,7 @@ describe Api::V0::VariantsController, type: :controller do let(:product) { create(:product) } let(:variant) { product.variants.first } + let(:taxon) { create(:taxon) } let!(:variant2) { create(:variant, product:) } context "deleted variants" do @@ -144,7 +145,7 @@ 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", price: "1" }, + unit_description: "L", price: "1", primary_taxon_id: taxon.id }, product_id: variant.product.id expect(attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true) diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index 2fcb53ae2a..9dbf146f53 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -93,6 +93,7 @@ describe Spree::Admin::ProductsController, type: :controller do variant_unit_name: nil ) end + let!(:taxon) { create(:taxon) } before { controller_login_as_enterprise_user([producer]) } @@ -111,7 +112,8 @@ describe Spree::Admin::ProductsController, type: :controller do "price" => "5.0", "unit_value" => 4, "unit_description" => "", - "display_name" => "name" + "display_name" => "name", + "primary_taxon_id" => taxon.id } ] } diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index b3b05d8afc..7bdfc7bcbe 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -11,7 +11,9 @@ 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", price: 1) + deleted_variant = product.variants.create( + unit_value: "2", price: 1, primary_taxon: create(:taxon) + ) deleted_variant.delete deleted_variant end diff --git a/spec/lib/reports/products_and_inventory_report_spec.rb b/spec/lib/reports/products_and_inventory_report_spec.rb index f2449f56f0..8096fa46b1 100644 --- a/spec/lib/reports/products_and_inventory_report_spec.rb +++ b/spec/lib/reports/products_and_inventory_report_spec.rb @@ -43,7 +43,7 @@ module Reporting allow(variant).to receive_message_chain(:product, :name).and_return("Product Name") allow(variant).to receive_message_chain(:product, :properties) .and_return [double(name: "property1"), double(name: "property2")] - allow(variant).to receive_message_chain(:product, :primary_taxon). + allow(variant).to receive_message_chain(:primary_taxon). and_return double(name: "taxon1") allow(variant).to receive_message_chain(:product, :group_buy_unit_size).and_return(21) allow(subject).to receive(:query_result).and_return [variant] diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 49fc233147..82bddc66db 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -155,7 +155,6 @@ module Spree describe "associations" do it { is_expected.to belong_to(:supplier).required } - it { is_expected.to belong_to(:primary_taxon).required } end describe "validations and defaults" do @@ -167,10 +166,6 @@ module Spree it { is_expected.to validate_length_of(:name).is_at_most(255) } it { is_expected.to validate_length_of(:sku).is_at_most(255) } - it "requires a primary taxon" do - expect(build(:simple_product, primary_taxon: nil)).not_to be_valid - end - context "unit value" do it "requires a unit value when variant unit is weight" do expect(build(:simple_product, variant_unit: 'weight', variant_unit_name: 'name', @@ -232,7 +227,7 @@ module Spree before do create(:stock_location) - product.primary_taxon = create(:taxon) + product.primary_taxon_id = create(:taxon).id product.supplier = create(:supplier_enterprise) product.name = "Product1" product.variant_unit = "weight" diff --git a/spec/models/spree/taxon_spec.rb b/spec/models/spree/taxon_spec.rb index edbc968585..7f481413df 100644 --- a/spec/models/spree/taxon_spec.rb +++ b/spec/models/spree/taxon_spec.rb @@ -41,11 +41,11 @@ module Spree let!(:taxon1) { create(:taxon) } let!(:taxon2) { create(:taxon) } let!(:product) { create(:simple_product, primary_taxon: taxon1) } + let(:variant) { product.variants.first } - it "is touched when assignment of primary_taxon on a product changes" do + it "is touched when assignment of primary_taxon on a variant changes" do expect do - product.primary_taxon = taxon2 - product.save + variant.update(primary_taxon: taxon2) end.to change { taxon2.reload.updated_at } end end diff --git a/spec/services/products_renderer_spec.rb b/spec/services/products_renderer_spec.rb index e14a6832d0..179233fe2d 100644 --- a/spec/services/products_renderer_spec.rb +++ b/spec/services/products_renderer_spec.rb @@ -162,12 +162,6 @@ describe ProductsRenderer do expect(products_renderer.products_json).to include "998.0" end - it "includes the primary taxon" do - taxon = create(:taxon) - allow_any_instance_of(Spree::Product).to receive(:primary_taxon).and_return taxon - expect(products_renderer.products_json).to include taxon.name - end - it "loads tag_list for variants" do VariantOverride.create(variant:, hub: distributor, tag_list: 'lalala') expect(products_renderer.products_json).to include "[\"lalala\"]" diff --git a/spec/system/admin/bulk_product_update_spec.rb b/spec/system/admin/bulk_product_update_spec.rb index 9475b3a9cb..1f954dc142 100644 --- a/spec/system/admin/bulk_product_update_spec.rb +++ b/spec/system/admin/bulk_product_update_spec.rb @@ -344,7 +344,6 @@ describe ' within "tr#p_#{p.id}" do expect(page).to have_field "product_name", with: p.name expect(page).to have_select "producer_id", selected: s1.name - expect(page).to have_select2 "p#{p.id}_category_id", selected: t2.name expect(page).to have_select "variant_unit_with_scale", selected: "Volume (L)" expect(page).to have_checked_field "inherits_properties" expect(page).to have_field "product_sku", with: p.sku @@ -352,7 +351,6 @@ describe ' fill_in "product_name", with: "Big Bag Of Potatoes" select s2.name, from: 'producer_id' select "Weight (kg)", from: "variant_unit_with_scale" - select2_select t1.name, from: "p#{p.id}_category_id" uncheck "inherits_properties" fill_in "product_sku", with: "NEW SKU" end @@ -365,7 +363,6 @@ describe ' expect(p.supplier).to eq s2 expect(p.variant_unit).to eq "weight" expect(p.variant_unit_scale).to eq 1000 # Kg - expect(p.primary_taxon.permalink).to eq t1.permalink expect(p.inherits_properties).to be false expect(p.sku).to eq "NEW SKU" end diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index 8f497f086f..55e15a3e41 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -143,7 +143,7 @@ describe ' expect(product.variants.first.unit_value).to eq(5000) 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.variants.first.primary_taxon_id).to eq(taxon.id) expect(product.variants.first.price.to_s).to eq('19.99') expect(product.on_hand).to eq(5) expect(product.variants.first.tax_category_id).to eq(tax_category.id) diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 472ce6a333..abe26647c7 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -358,15 +358,15 @@ describe ' let(:taxon) { create(:taxon, name: 'Taxon Name') } let(:product1) { create(:simple_product, name: "Product Name", price: 100, supplier:, - primary_taxon: taxon) + primary_taxon_id: taxon.id) } let(:product2) { create(:simple_product, name: "Product 2", price: 99.0, variant_unit: 'weight', variant_unit_scale: 1, unit_value: '100', supplier:, - primary_taxon: taxon, sku: "product_sku") + primary_taxon_id: taxon.id, sku: "product_sku") } let(:variant1) { product1.variants.first } - let(:variant2) { create(:variant, product: product1, price: 80.0) } + let(:variant2) { create(:variant, product: product1, price: 80.0, primary_taxon: taxon) } let(:variant3) { product2.variants.first } before do @@ -396,17 +396,17 @@ describe ' expect(page).to have_table_row [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.map(&:presentation).join(", "), - product1.primary_taxon.name, "1g", "100.0", + taxon.name, "1g", "100.0", "none", "", "sku1", "No", "10"] expect(page).to have_table_row [product1.supplier.name, product1.supplier.address.city, "Product Name", product1.properties.map(&:presentation).join(", "), - product1.primary_taxon.name, "1g", "80.0", + taxon.name, "1g", "80.0", "none", "", "sku2", "No", "20"] expect(page).to have_table_row [product2.supplier.name, product1.supplier.address.city, "Product 2", product1.properties.map(&:presentation).join(", "), - product2.primary_taxon.name, "100g", "99.0", + taxon.name, "100g", "99.0", "none", "", "product_sku", "No", "9"] end diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index e8487959c3..754670b542 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -9,6 +9,8 @@ describe ' include AuthenticationHelper include WebHelper + let!(:taxon) { create(:taxon) } + describe "new variant" do it "creating a new variant" do # Given a product with a unit-related option type @@ -21,6 +23,7 @@ describe ' fill_in 'unit_value_human', with: '1' fill_in 'variant_unit_description', with: 'foo' + select taxon.name, from: "variant_primary_taxon_id" click_button 'Create' # Then the variant should have been created @@ -61,6 +64,7 @@ describe ' # Expect variant_weight to accept 3 decimal places fill_in 'variant_weight', with: '1.234' fill_in 'unit_value_human', with: 1 + select taxon.name, from: "variant_primary_taxon_id" click_button 'Create' # Then the variant should have been created diff --git a/spec/system/consumer/caching/shops_caching_spec.rb b/spec/system/consumer/caching/shops_caching_spec.rb index 7c84c6c2f8..b44a8f77bc 100644 --- a/spec/system/consumer/caching/shops_caching_spec.rb +++ b/spec/system/consumer/caching/shops_caching_spec.rb @@ -53,8 +53,9 @@ describe "Shops caching", caching: true do let!(:property) { create(:property, presentation: "Cached Property") } let!(:property2) { create(:property, presentation: "New Property") } let!(:product) { - create(:product, primary_taxon: taxon, properties: [property]) + create(:product, primary_taxon_id: taxon.id, properties: [property]) } + let(:variant) { product.variants.first } let(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first } let(:test_domain) { @@ -92,7 +93,7 @@ describe "Shops caching", caching: true do expect(page).to have_content taxon.name expect(page).to have_content property.presentation - product.update_attribute(:primary_taxon, taxon2) + variant.update_attribute(:primary_taxon, taxon2) product.update_attribute(:properties, [property2]) visit enterprise_shop_path(distributor) From d281e9d1b5df1e1127878bbbfd9ab1b9c4671b67 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 13:03:00 +0100 Subject: [PATCH 15/35] Adjust factory --- spec/factories/variant_factory.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/factories/variant_factory.rb b/spec/factories/variant_factory.rb index ec2ebf8a4d..3cb44c6344 100644 --- a/spec/factories/variant_factory.rb +++ b/spec/factories/variant_factory.rb @@ -12,7 +12,7 @@ FactoryBot.define do depth { generate(:random_float) } primary_taxon { Spree::Taxon.first || FactoryBot.create(:taxon) } - product { |v| create(:product, primary_taxon_id: v.primary_taxon.id) } + product { |p| p.association(:product) } # ensure stock item will be created for this variant before(:create) { create(:stock_location) if Spree::StockLocation.count.zero? } @@ -23,7 +23,6 @@ FactoryBot.define do on_hand { 5 } end - product { |v| create(:product, primary_taxon_id: v.primary_taxon.id) } unit_value { 1 } unit_description { '' } From 6e7b97879bde4a92f6395cd5b1d1fc4c13b51558 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 14:18:03 +0100 Subject: [PATCH 16/35] Update DFC product importer --- engines/dfc_provider/app/services/supplied_product_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 31c343b524..83f2255812 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -66,7 +66,7 @@ class SuppliedProductBuilder < DfcBuilder name: supplied_product.name, description: supplied_product.description, price: 0, # will be in DFC Offer - primary_taxon: taxon(supplied_product) + primary_taxon_id: taxon(supplied_product).id ).tap do |product| QuantitativeValueBuilder.apply(supplied_product.quantity, product) end From c0864405a18d14c907c101fef1c146bdecc94ade Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 14:20:57 +0100 Subject: [PATCH 17/35] Update bulk products JS spec --- .../javascripts/unit/admin/bulk_product_update_spec.js.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee index aa56ccd595..57f6225110 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -803,8 +803,8 @@ describe "AdminProductEditCtrl", -> expect(product).toEqual id: 123 variants: [ - {id: -1, price: null, unit_value: null, tax_category_id: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null} - {id: -2, price: null, unit_value: null, tax_category_id: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null} + {id: -1, price: null, unit_value: null, tax_category_id: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null, category_id: null} + {id: -2, price: null, unit_value: null, tax_category_id: null, unit_description: null, on_demand: false, on_hand: null, display_as: null, display_name: null, category_id: null} ] it "shows the variant(s)", -> From d4dd3cc708eac3781c02bffdc4f07f4018b6d45f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 14:58:42 +0100 Subject: [PATCH 18/35] Update products v3 table --- app/views/admin/products_v3/_product_row.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml index 1526fb26cf..9ad27681d0 100644 --- a/app/views/admin/products_v3/_product_row.html.haml +++ b/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 + .content= product.variants.first.primary_taxon&.name %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) From c5ec7e361b22724cb65436c8051f2d62235f0855 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 15:09:29 +0100 Subject: [PATCH 19/35] Update product import --- app/models/product_import/entry_validator.rb | 5 ++--- spec/models/product_importer_spec.rb | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 9eaed6034a..34369fe44d 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -24,7 +24,6 @@ module ProductImport 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 @@ module ProductImport 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 @@ module ProductImport 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) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index 593a0de53b..ff6470d782 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -562,7 +562,7 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type"] - csv << ["Beetroot", enterprise3.name, "Meat", "5", "3.50", "500", "g"] + csv << ["Beetroot", enterprise3.name, "Vegetables", "5", "3.50", "500", "Kg"] csv << ["Tomato", enterprise3.name, "Vegetables", "6", "5.50", "500", "Kg"] end } From 6025491f94e8a06af21ef928a5e03ba6d5fa5a47 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 19:27:12 +0100 Subject: [PATCH 20/35] Update product serializer --- app/serializers/api/product_serializer.rb | 2 -- spec/serializers/api/product_serializer_spec.rb | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index 11f228c1e6..e25fdaf04a 100644 --- a/app/serializers/api/product_serializer.rb +++ b/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 diff --git a/spec/serializers/api/product_serializer_spec.rb b/spec/serializers/api/product_serializer_spec.rb index fc50f7c3c0..d44e964665 100644 --- a/spec/serializers/api/product_serializer_spec.rb +++ b/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, :meta_keywords, :group_buy, :notes, :description, :description_html, - :properties_with_values, :variants, :primary_taxon, :image, :supplier + :properties_with_values, :variants, :image, :supplier ] end From 78495d0ace571d272f8bdc454b808342af183373 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 19:40:41 +0100 Subject: [PATCH 21/35] Drop unnecessary params filtering --- .../api/v0/order_cycles_controller.rb | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/app/controllers/api/v0/order_cycles_controller.rb b/app/controllers/api/v0/order_cycles_controller.rb index 2c3c0b2318..7bb56ecc56 100644 --- a/app/controllers/api/v0/order_cycles_controller.rb +++ b/app/controllers/api/v0/order_cycles_controller.rb @@ -70,22 +70,7 @@ module Api 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 From fb09a7f1e62de5333fce89c960bcbccc9ee9cf33 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 8 Aug 2023 19:42:26 +0100 Subject: [PATCH 22/35] Fix product filtering --- app/assets/javascripts/admin/bulk_product_update.js.coffee | 4 ++-- .../darkswarm/controllers/products_controller.js.coffee | 2 +- app/reflexes/products_reflex.rb | 2 +- spec/controllers/api/v0/order_cycles_controller_spec.rb | 2 +- spec/controllers/api/v0/products_controller_spec.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 4598669203..5adb65c399 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/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, @@ -218,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 diff --git a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee index 521859f7ab..79f1db42ab 100644 --- a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee +++ b/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)-> diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index b312c74205..650ce2b23f 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -173,7 +173,7 @@ class ProductsReflex < ApplicationReflex 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 diff --git a/spec/controllers/api/v0/order_cycles_controller_spec.rb b/spec/controllers/api/v0/order_cycles_controller_spec.rb index 7a8aaa0a24..a8640fa581 100644 --- a/spec/controllers/api/v0/order_cycles_controller_spec.rb +++ b/spec/controllers/api/v0/order_cycles_controller_spec.rb @@ -126,7 +126,7 @@ module Api context "with taxon filters" do it "filters by taxon" do api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { primary_taxon_id_in_any: [taxon2.id] } + q: { variants_primary_taxon_id_in_any: [taxon2.id] } expect(product_ids).to include product2.id, product3.id expect(product_ids).not_to include product1.id, product4.id diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index 96f22a9fe8..90480a6f2e 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -267,7 +267,7 @@ describe Api::V0::ProductsController, type: :controller do end it "filters results by product category" do - api_get :bulk_products, { page: 1, per_page: 15, q: { primary_taxon_id_eq: taxon.id } }, + api_get :bulk_products, { page: 1, per_page: 15, q: { variants_primary_taxon_id_eq: taxon.id } }, format: :json expect(returned_product_ids).to eq [product3.id, product2.id] end From 02abe5cc063c5fc44ae604b6aeda9015b4d70284 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 9 Aug 2023 11:33:05 +0100 Subject: [PATCH 23/35] Remove dead code related to multiple product taxons --- app/services/sets/product_set.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb index 1adcdcb579..ba3fb40240 100644 --- a/app/services/sets/product_set.rb +++ b/app/services/sets/product_set.rb @@ -46,18 +46,12 @@ module Sets # 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) From 3f2a5786bd1f2bbb242d3f63a9cb0d557498c407 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 9 Aug 2023 11:57:12 +0100 Subject: [PATCH 24/35] Apply taxon from sibling variant if none is provided on variant creation --- app/models/spree/variant.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 6fc36cc27c..94e3bfb3c5 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -83,6 +83,7 @@ module Spree 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? @@ -209,6 +210,10 @@ module Spree private + def assign_related_taxon + self.primary_taxon ||= product.variants.last&.primary_taxon + end + def check_currency return unless currency.nil? From 2743a8183d82b4cedb36d205d61512716719e969 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 9 Aug 2023 13:11:32 +0100 Subject: [PATCH 25/35] Disable sorting by user-defined product category order --- app/services/products_renderer.rb | 6 ------ config/locales/en.yml | 1 + spec/controllers/api/v0/order_cycles_controller_spec.rb | 4 ++-- spec/services/products_renderer_spec.rb | 4 ++-- spec/system/admin/enterprises_spec.rb | 2 +- 5 files changed, 6 insertions(+), 11 deletions(-) diff --git a/app/services/products_renderer.rb b/app/services/products_renderer.rb index 5d0d794856..e7ab58f60b 100644 --- a/app/services/products_renderer.rb +++ b/app/services/products_renderer.rb @@ -74,12 +74,6 @@ class ProductsRenderer .preferred_shopfront_producer_order .split(",").map { |id| "spree_products.supplier_id=#{id} DESC" } .join(", ") + ", spree_products.name ASC, spree_products.id ASC" - elsif distributor.preferred_shopfront_product_sorting_method == "by_category" && - distributor.preferred_shopfront_taxon_order.present? - distributor - .preferred_shopfront_taxon_order - .split(",").map { |id| "spree_products.primary_taxon_id=#{id} DESC" } - .join(", ") + ", spree_products.name ASC, spree_products.id ASC" else "spree_products.name ASC, spree_products.id" end diff --git a/config/locales/en.yml b/config/locales/en.yml index 949d7af400..efb8d01f37 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1249,6 +1249,7 @@ en: open_date: "Open Date" close_date: "Close Date" display_ordering_in_shopfront: "Display ordering in shopfront:" + shopfront_sort_by_product: "By product" shopfront_sort_by_category: "By category" shopfront_sort_by_producer: "By producer" shopfront_sort_by_category_placeholder: "Category" diff --git a/spec/controllers/api/v0/order_cycles_controller_spec.rb b/spec/controllers/api/v0/order_cycles_controller_spec.rb index a8640fa581..9344bca3a8 100644 --- a/spec/controllers/api/v0/order_cycles_controller_spec.rb +++ b/spec/controllers/api/v0/order_cycles_controller_spec.rb @@ -280,13 +280,13 @@ module Api exchange.variants << product8.variants.first end - it "displays products in new order" do + xit "displays products in new order" do api_get :products, id: order_cycle.id, distributor: distributor.id expect(product_ids).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, product6.id, product1.id] end - it "displays products in correct order across multiple pages" do + xit "displays products in correct order across multiple pages" do api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3 expect(product_ids).to eq [product7.id, product8.id, product2.id] diff --git a/spec/services/products_renderer_spec.rb b/spec/services/products_renderer_spec.rb index 179233fe2d..ade810c08d 100644 --- a/spec/services/products_renderer_spec.rb +++ b/spec/services/products_renderer_spec.rb @@ -39,7 +39,7 @@ describe ProductsRenderer do end describe "sorting" do - it "sorts products by the distributor's preferred taxon list" do + xit "sorts products by the distributor's preferred taxon list" do allow(distributor) .to receive(:preferred_shopfront_taxon_order) { "#{cakes.id},#{fruits.id}" } products = products_renderer.send(:products) @@ -106,7 +106,7 @@ describe ProductsRenderer do expect(products).to eq([product_apples, product_cherries]) end - it "filters products with property when sorting is enabled" do + xit "filters products with property when sorting is enabled" do allow(distributor).to receive(:preferred_shopfront_taxon_order) { "#{fruits.id},#{cakes.id}" } diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 92fdf304d3..7e1cbcaec8 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/spec/system/admin/enterprises_spec.rb @@ -536,7 +536,7 @@ describe ' .to eq('Enterprise "First Distributor" has been successfully updated!') end - it "sets the preference correctly" do + xit "sets the preference correctly" do expect(distributor1.preferred_shopfront_product_sorting_method).to eql("by_category") expect(distributor1.preferred_shopfront_taxon_order).to eql(taxon.id.to_s) end From a55931c081207523578051fcf5fb7ac4ee649ed2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 10 Aug 2023 15:47:54 +0100 Subject: [PATCH 26/35] Reinstate sorting by arbitrary list of product categories --- .../order_cycles/distributed_products_service.rb | 16 ++++++++++++++++ app/services/products_renderer.rb | 10 ++++++++-- .../enterprises/form/_shop_preferences.html.haml | 8 ++++++++ config/initializers/pagy.rb | 7 +++++-- .../api/v0/order_cycles_controller_spec.rb | 4 ++-- spec/services/products_renderer_spec.rb | 4 ++-- spec/system/admin/enterprises_spec.rb | 2 +- 7 files changed, 42 insertions(+), 9 deletions(-) diff --git a/app/services/order_cycles/distributed_products_service.rb b/app/services/order_cycles/distributed_products_service.rb index 567478c7eb..4da5039e26 100644 --- a/app/services/order_cycles/distributed_products_service.rb +++ b/app/services/order_cycles/distributed_products_service.rb @@ -15,6 +15,22 @@ module OrderCycles 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). diff --git a/app/services/products_renderer.rb b/app/services/products_renderer.rb index e7ab58f60b..d7296e9fa1 100644 --- a/app/services/products_renderer.rb +++ b/app/services/products_renderer.rb @@ -35,7 +35,7 @@ class ProductsRenderer @products ||= begin results = distributed_products. - products_relation. + products_taxons_relation. order(Arel.sql(products_order)) filter_and_paginate(results). @@ -54,7 +54,7 @@ class ProductsRenderer 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 @@ -74,6 +74,12 @@ class ProductsRenderer .preferred_shopfront_producer_order .split(",").map { |id| "spree_products.supplier_id=#{id} DESC" } .join(", ") + ", spree_products.name ASC, spree_products.id ASC" + elsif distributor.preferred_shopfront_product_sorting_method == "by_category" && + distributor.preferred_shopfront_taxon_order.present? + distributor + .preferred_shopfront_taxon_order + .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" end diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index 616d631509..8c214e9d35 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml @@ -24,6 +24,14 @@ = label :enterprise, :preferred_shopfront_product_sorting_method_by_category, t('.shopfront_sort_by_category') .eight.columns.omega %textarea.fullwidth{ id: 'enterprise_preferred_shopfront_taxon_order', name: 'enterprise[preferred_shopfront_taxon_order]', rows: 6, "ofn-taxon-autocomplete": '', "multiple-selection": 'true', placeholder: t('.shopfront_sort_by_category_placeholder'), "ng-model": 'Enterprise.preferred_shopfront_taxon_order', "ng-readonly": "Enterprise.preferred_shopfront_product_sorting_method != 'by_category'" } +.row + .three.columns.alpha + = radio_button :enterprise, :preferred_shopfront_product_sorting_method, :by_category, { 'ng-model' => 'Enterprise.preferred_shopfront_product_sorting_method' } + = label :enterprise, :preferred_shopfront_product_sorting_method_by_category, t('.shopfront_sort_by_category') + .eight.columns.omega + %textarea.fullwidth{ id: 'enterprise_preferred_shopfront_taxon_order', name: 'enterprise[preferred_shopfront_taxon_order]', rows: 6, + 'ofn-taxon-autocomplete' => '', 'multiple-selection' => 'true', placeholder: t('.shopfront_sort_by_category_placeholder'), + ng: { model: 'Enterprise.preferred_shopfront_taxon_order', readonly: "Enterprise.preferred_shopfront_product_sorting_method != 'by_category'" }} .row .three.columns.alpha = radio_button :enterprise, :preferred_shopfront_product_sorting_method, :by_producer, { 'ng-model' => 'Enterprise.preferred_shopfront_product_sorting_method' } diff --git a/config/initializers/pagy.rb b/config/initializers/pagy.rb index ff9ed076e6..cd18f12b32 100644 --- a/config/initializers/pagy.rb +++ b/config/initializers/pagy.rb @@ -1,15 +1,18 @@ # frozen_string_literal: true +require 'pagy/extras/arel' +require 'pagy/extras/items' +require 'pagy/extras/overflow' + + # Pagy Variables # See https://ddnexus.github.io/pagy/api/pagy#variables Pagy::DEFAULT[:items] = 100 # Items extra: Allow the client to request a custom number of items per page with an optional selector UI # See https://ddnexus.github.io/pagy/extras/items -require 'pagy/extras/items' Pagy::DEFAULT[:items_param] = :per_page Pagy::DEFAULT[:max_items] = 100 # For handling requests for non-existant pages eg: page 35 when there are only 4 pages of results -require 'pagy/extras/overflow' Pagy::DEFAULT[:overflow] = :empty_page diff --git a/spec/controllers/api/v0/order_cycles_controller_spec.rb b/spec/controllers/api/v0/order_cycles_controller_spec.rb index 9344bca3a8..a8640fa581 100644 --- a/spec/controllers/api/v0/order_cycles_controller_spec.rb +++ b/spec/controllers/api/v0/order_cycles_controller_spec.rb @@ -280,13 +280,13 @@ module Api exchange.variants << product8.variants.first end - xit "displays products in new order" do + it "displays products in new order" do api_get :products, id: order_cycle.id, distributor: distributor.id expect(product_ids).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, product6.id, product1.id] end - xit "displays products in correct order across multiple pages" do + it "displays products in correct order across multiple pages" do api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3 expect(product_ids).to eq [product7.id, product8.id, product2.id] diff --git a/spec/services/products_renderer_spec.rb b/spec/services/products_renderer_spec.rb index ade810c08d..179233fe2d 100644 --- a/spec/services/products_renderer_spec.rb +++ b/spec/services/products_renderer_spec.rb @@ -39,7 +39,7 @@ describe ProductsRenderer do end describe "sorting" do - xit "sorts products by the distributor's preferred taxon list" do + it "sorts products by the distributor's preferred taxon list" do allow(distributor) .to receive(:preferred_shopfront_taxon_order) { "#{cakes.id},#{fruits.id}" } products = products_renderer.send(:products) @@ -106,7 +106,7 @@ describe ProductsRenderer do expect(products).to eq([product_apples, product_cherries]) end - xit "filters products with property when sorting is enabled" do + it "filters products with property when sorting is enabled" do allow(distributor).to receive(:preferred_shopfront_taxon_order) { "#{fruits.id},#{cakes.id}" } diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 7e1cbcaec8..92fdf304d3 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/spec/system/admin/enterprises_spec.rb @@ -536,7 +536,7 @@ describe ' .to eq('Enterprise "First Distributor" has been successfully updated!') end - xit "sets the preference correctly" do + it "sets the preference correctly" do expect(distributor1.preferred_shopfront_product_sorting_method).to eql("by_category") expect(distributor1.preferred_shopfront_taxon_order).to eql(taxon.id.to_s) end From d959ee2358bc56a69b2c4bf0728e356dad10413a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Sep 2023 15:37:08 +0100 Subject: [PATCH 27/35] Fix rubocop warnings --- spec/controllers/api/v0/products_controller_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index 90480a6f2e..4abba9f3bd 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -35,7 +35,8 @@ describe Api::V0::ProductsController, type: :controller do it "gets a single product" do product.create_image!(attachment:) - product.variants.create!(unit_value: "1", unit_description: "thing", price: 1, primary_taxon: taxon) + product.variants.create!(unit_value: "1", unit_description: "thing", price: 1, + primary_taxon: taxon) product.variants.first.images.create!(attachment:) product.set_property("spree", "rocks") @@ -267,7 +268,8 @@ describe Api::V0::ProductsController, type: :controller do end it "filters results by product category" do - api_get :bulk_products, { page: 1, per_page: 15, q: { variants_primary_taxon_id_eq: taxon.id } }, + api_get :bulk_products, + { page: 1, per_page: 15, q: { variants_primary_taxon_id_eq: taxon.id } }, format: :json expect(returned_product_ids).to eq [product3.id, product2.id] end From 8a364a5f48aec13b5d04c820d25009ec4d31cec8 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Sep 2023 16:00:01 +0100 Subject: [PATCH 28/35] Fix product touch spec --- spec/models/spree/product_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 82bddc66db..a0b15065fd 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -319,7 +319,7 @@ module Spree let(:product) { create(:simple_product) } describe "touching affected enterprises when the product is deleted" do - let(:product) { create(:simple_product) } + let(:product) { create(:simple_product, supplier: distributor) } let(:supplier) { product.supplier } let(:distributor) { create(:distributor_enterprise) } let!(:oc) { From ac4ec36b3b46ebf7911ee2b6c1e9a992954f9f5f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Sep 2023 16:04:47 +0100 Subject: [PATCH 29/35] Update ProductScopeQuery spec --- spec/queries/product_scope_query_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/queries/product_scope_query_spec.rb b/spec/queries/product_scope_query_spec.rb index f8bde2c8da..d1efd12d98 100755 --- a/spec/queries/product_scope_query_spec.rb +++ b/spec/queries/product_scope_query_spec.rb @@ -30,7 +30,8 @@ describe ProductScopeQuery do it "filters results by product category" do subject = ProductScopeQuery - .new(current_api_user, { q: { primary_taxon_id_eq: taxon.id } }).bulk_products + .new(current_api_user, { q: { variants_primary_taxon_id_eq: taxon.id } }) + .bulk_products expect(subject).to include(product, product2) expect(subject).not_to include(product3) From d3e62c439099c15ef935dd863c84e5d3d9144ed3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 Sep 2023 16:47:12 +0100 Subject: [PATCH 30/35] Add test for persisting taxon on variant during product creation --- spec/models/spree/product_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index a0b15065fd..05e5fdf52e 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -224,10 +224,11 @@ module Spree context "saving a new product" do let!(:product){ Spree::Product.new } let!(:shipping_category){ create(:shipping_category) } + let!(:taxon){ create(:taxon) } before do create(:stock_location) - product.primary_taxon_id = create(:taxon).id + product.primary_taxon_id = taxon.id product.supplier = create(:supplier_enterprise) product.name = "Product1" product.variant_unit = "weight" @@ -243,6 +244,7 @@ module Spree standard_variant = product.variants.reload.first expect(standard_variant.price).to eq 4.27 expect(standard_variant.shipping_category).to eq shipping_category + expect(standard_variant.primary_taxon).to eq taxon end end From 6d1249e7f9f2d958cd02209281abbc3af4564e12 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 20 Feb 2024 16:27:05 +1100 Subject: [PATCH 31/35] Update DFC supplied product --- .../app/services/supplied_product_builder.rb | 13 ++-- .../spec/requests/supplied_products_spec.rb | 9 ++- .../services/supplied_product_builder_spec.rb | 61 ++----------------- 3 files changed, 14 insertions(+), 69 deletions(-) diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 83f2255812..bdf069bdfb 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -65,26 +65,25 @@ class SuppliedProductBuilder < DfcBuilder Spree::Product.new( name: supplied_product.name, description: supplied_product.description, - price: 0, # will be in DFC Offer - primary_taxon_id: taxon(supplied_product).id + price: 0 # will be in DFC Offer ).tap do |product| QuantitativeValueBuilder.apply(supplied_product.quantity, product) + product.ensure_standard_variant + product.variants.first.primary_taxon = taxon(supplied_product) end end def self.apply(supplied_product, variant) - variant.product.assign_attributes( - description: supplied_product.description, - primary_taxon: taxon(supplied_product) - ) + variant.product.assign_attributes(description: supplied_product.description) variant.display_name = supplied_product.name + variant.primary_taxon = taxon(supplied_product) QuantitativeValueBuilder.apply(supplied_product.quantity, variant.product) variant.unit_value = variant.product.unit_value end def self.product_type(variant) - taxon_dfc_id = variant.product.primary_taxon&.dfc_id + taxon_dfc_id = variant.primary_taxon&.dfc_id DfcProductTypeFactory.for(taxon_dfc_id) end diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 6f22460e03..d57d45c0f0 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -10,11 +10,10 @@ describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml", rswag_auto :product_with_image, id: 90_000, supplier: enterprise, name: "Pesto", description: "Basil Pesto", - variants: [variant], - primary_taxon: taxon + variants: [variant] ) } - let(:variant) { build(:base_variant, id: 10_001, unit_value: 1) } + let(:variant) { build(:base_variant, id: 10_001, unit_value: 1, primary_taxon: taxon) } let(:taxon) { build( :taxon, @@ -114,7 +113,7 @@ describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml", rswag_auto product = Spree::Product.find(product_id) expect(product.name).to eq "Apple" expect(product.variants).to eq [variant] - expect(product.primary_taxon).to eq(non_local_vegetable) + expect(product.variants.first.primary_taxon).to eq(non_local_vegetable) # Creates a variant for existing product supplied_product[:'ofn:spree_product_id'] = product_id @@ -244,7 +243,7 @@ describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml", rswag_auto }.to change { variant.description }.to("DFC-Pesto updated") .and change { variant.display_name }.to("Pesto novo") .and change { variant.unit_value }.to(17) - .and change { variant.product.primary_taxon }.to(drink_taxon) + .and change { variant.primary_taxon }.to(drink_taxon) end end end diff --git a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb index 08db382860..12bc49f518 100644 --- a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb @@ -7,12 +7,10 @@ describe SuppliedProductBuilder do subject(:builder) { described_class } let(:variant) { - build(:variant, id: 5, product: spree_product) + build(:variant, id: 5, product: spree_product, primary_taxon: taxon) } let(:spree_product) { - create(:product, id: 6, supplier:).tap do |p| - p.primary_taxon = taxon - end + create(:product, id: 6, supplier:) } let(:supplier) { build(:supplier_enterprise, id: 7) @@ -121,7 +119,7 @@ describe SuppliedProductBuilder do it "assigns the taxon matching the DFC product type" do product = builder.import_product(supplied_product) - expect(product.primary_taxon).to eq(taxon) + expect(product.variants.first.primary_taxon).to eq(taxon) end end end @@ -191,58 +189,7 @@ describe SuppliedProductBuilder do imported_product = imported_variant.product expect(imported_product.id).to eq(spree_product.id) expect(imported_product.description).to eq("Better Awesome tomato") - expect(imported_product.primary_taxon).to eq(new_taxon) - end - - it "adds a new variant" do - expect(imported_variant.id).to be_nil - expect(imported_variant.product).to eq(spree_product) - expect(imported_variant.display_name).to eq("Tomato") - expect(imported_variant.unit_value).to eq(2000) - end - end - - context "with spree_product_uri supplied" do - let(:imported_variant) { builder.import_variant(supplied_product, supplier) } - let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.DRINK.SOFT_DRINK } - let!(:new_taxon) { - create( - :taxon, - name: "Soft Drink", - dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#soft-drink" - ) - } - - context "when spree_product_uri match the server host" do - let(:supplied_product) do - variant.save! # referenced in spree_product_id - - DfcProvider::SuppliedProduct.new( - "https://example.net/tomato", - name: "Tomato", - description: "Better Awesome tomato", - quantity: DataFoodConsortium::Connector::QuantitativeValue.new( - unit: DfcLoader.connector.MEASURES.KILOGRAM, - value: 2, - ), - productType: product_type, - spree_product_uri: "http://test.host/api/dfc/enterprises/7?spree_product_id=6" - ) - end - - it "update an existing Spree::Product" do - imported_product = imported_variant.product - expect(imported_product.id).to eq(spree_product.id) - expect(imported_product.description).to eq("Better Awesome tomato") - expect(imported_product.primary_taxon).to eq(new_taxon) - end - - it "adds a new variant" do - expect(imported_variant.id).to be_nil - expect(imported_variant.product).to eq(spree_product) - expect(imported_variant.display_name).to eq("Tomato") - expect(imported_variant.unit_value).to eq(2000) - end + expect(imported_variant.primary_taxon).to eq(new_taxon) end context "when spree_product_uri doesn't match the server host" do From 8c0583808069dcaacdcb771a470be3b58d422658 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 21 Feb 2024 16:10:28 +1100 Subject: [PATCH 32/35] Move the category to the variant row --- app/views/admin/products_v3/_product_row.html.haml | 2 +- app/views/admin/products_v3/_variant_row.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml index 9ad27681d0..4a8b0c8ce3 100644 --- a/app/views/admin/products_v3/_product_row.html.haml +++ b/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.variants.first.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) diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index 661776db96..77af4ee424 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/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 From 678fa37df9432d594aa002cde7fbb8c57bd1c69e Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 21 Feb 2024 16:12:33 +1100 Subject: [PATCH 33/35] Fix duplication issue When a product had mutiple variant assigned to the same category it should only return the product one --- app/reflexes/products_reflex.rb | 2 +- spec/reflexes/products_reflex_spec.rb | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 650ce2b23f..e8d7634b7c 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -152,7 +152,7 @@ class ProductsReflex < ApplicationReflex 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 diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb index 542199628e..cb8314b544 100644 --- a/spec/reflexes/products_reflex_spec.rb +++ b/spec/reflexes/products_reflex_spec.rb @@ -15,7 +15,7 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do end describe '#fetch' do - subject{ build_reflex(method_name: :fetch, **context) } + subject { build_reflex(method_name: :fetch, **context) } describe "sorting" do let!(:product_z) { create(:simple_product, name: "Zucchini") } @@ -34,6 +34,27 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do end end + describe '#filter' do + context "when filtering by category" do + let!(:product_a) { create(:simple_product, name: "Apples") } + let!(:product_z) do + create(:simple_product, name: "Zucchini").tap do |p| + p.variants.first.update(primary_taxon: category_c) + end + end + let(:category_c) { create(:taxon, name: "Category 1") } + + it "returns product with a variant matching the given category" do + # Add a second variant to test we are not returning duplicate product + product_z.variants << create(:variant, primary_taxon: category_c) + + reflex = run_reflex(:filter, params: { category_id: category_c.id } ) + + expect(reflex.get(:products).to_a).to eq([product_z]) + end + end + end + describe '#bulk_update' do let!(:variant_a1) { product_a.variants.first.tap{ |v| From 100239c4e6ff90a559a8b575622ca81f1a9487db Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 12 Mar 2024 15:26:49 +1100 Subject: [PATCH 34/35] Fix #bulk_product duplicate Remove duplicate when a product has mutiple variant in the same category (taxon) --- app/queries/product_scope_query.rb | 2 +- spec/queries/product_scope_query_spec.rb | 30 ++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/app/queries/product_scope_query.rb b/app/queries/product_scope_query.rb index bc8b6953b2..00f4a401e8 100644 --- a/app/queries/product_scope_query.rb +++ b/app/queries/product_scope_query.rb @@ -20,7 +20,7 @@ class ProductScopeQuery product_query. ransack(query_params_with_defaults). - result + result(distinct: true) end def find_product diff --git a/spec/queries/product_scope_query_spec.rb b/spec/queries/product_scope_query_spec.rb index d1efd12d98..9674a201f5 100755 --- a/spec/queries/product_scope_query_spec.rb +++ b/spec/queries/product_scope_query_spec.rb @@ -12,7 +12,7 @@ describe ProductScopeQuery do before { current_api_user.enterprise_roles.create(enterprise: supplier2) } - describe 'bulk update' do + describe '#bulk_products' do let!(:product3) { create(:product, supplier: supplier2) } it "returns a list of products" do @@ -28,13 +28,29 @@ describe ProductScopeQuery do expect(subject).not_to include(product2, product3) end - it "filters results by product category" do - subject = ProductScopeQuery - .new(current_api_user, { q: { variants_primary_taxon_id_eq: taxon.id } }) - .bulk_products + describe "by variant category" do + it "filters results by product category" do + create(:variant, product: product2, primary_taxon: taxon) - expect(subject).to include(product, product2) - expect(subject).not_to include(product3) + subject = ProductScopeQuery + .new(current_api_user, { q: { variants_primary_taxon_id_eq: taxon.id } }) + .bulk_products + + expect(subject).to match_array([product, product2]) + expect(subject).not_to include(product3) + end + + context "with mutiple variant in the same category" do + it "doesn't duplicate products" do + create(:variant, product: product2, primary_taxon: taxon) + + subject = ProductScopeQuery + .new(current_api_user, { q: { variants_primary_taxon_id_eq: taxon.id } }) + .bulk_products + + expect(subject).to match_array([product, product2]) + end + end end it "filters results by import_date" do From 484c037c38f5dfe663254ab5f6f4870effcc97f0 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 2 Apr 2024 10:18:47 +1100 Subject: [PATCH 35/35] Fix duplication coming from a rebase error --- .../admin/enterprises/form/_shop_preferences.html.haml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/views/admin/enterprises/form/_shop_preferences.html.haml b/app/views/admin/enterprises/form/_shop_preferences.html.haml index 8c214e9d35..616d631509 100644 --- a/app/views/admin/enterprises/form/_shop_preferences.html.haml +++ b/app/views/admin/enterprises/form/_shop_preferences.html.haml @@ -24,14 +24,6 @@ = label :enterprise, :preferred_shopfront_product_sorting_method_by_category, t('.shopfront_sort_by_category') .eight.columns.omega %textarea.fullwidth{ id: 'enterprise_preferred_shopfront_taxon_order', name: 'enterprise[preferred_shopfront_taxon_order]', rows: 6, "ofn-taxon-autocomplete": '', "multiple-selection": 'true', placeholder: t('.shopfront_sort_by_category_placeholder'), "ng-model": 'Enterprise.preferred_shopfront_taxon_order', "ng-readonly": "Enterprise.preferred_shopfront_product_sorting_method != 'by_category'" } -.row - .three.columns.alpha - = radio_button :enterprise, :preferred_shopfront_product_sorting_method, :by_category, { 'ng-model' => 'Enterprise.preferred_shopfront_product_sorting_method' } - = label :enterprise, :preferred_shopfront_product_sorting_method_by_category, t('.shopfront_sort_by_category') - .eight.columns.omega - %textarea.fullwidth{ id: 'enterprise_preferred_shopfront_taxon_order', name: 'enterprise[preferred_shopfront_taxon_order]', rows: 6, - 'ofn-taxon-autocomplete' => '', 'multiple-selection' => 'true', placeholder: t('.shopfront_sort_by_category_placeholder'), - ng: { model: 'Enterprise.preferred_shopfront_taxon_order', readonly: "Enterprise.preferred_shopfront_product_sorting_method != 'by_category'" }} .row .three.columns.alpha = radio_button :enterprise, :preferred_shopfront_product_sorting_method, :by_producer, { 'ng-model' => 'Enterprise.preferred_shopfront_product_sorting_method' }