From 2709479bf2188083aa7989b79eb601273cbec03d Mon Sep 17 00:00:00 2001 From: wandji20 Date: Sun, 21 Jul 2024 21:05:05 +0100 Subject: [PATCH 1/3] Require varian t category when creating new product variant [OFN-12666] --- .../javascripts/admin/bulk_product_update.js.coffee | 8 ++++++-- app/models/spree/variant.rb | 5 ----- app/views/admin/products_v3/_variant_row.html.haml | 10 +++++++--- config/locales/en.yml | 2 ++ spec/system/admin/products_v3/update_spec.rb | 7 +++++++ 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 7df51e7711a..d32977b8c05 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -126,8 +126,11 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout DisplayProperties.setShowVariants 0, showVariants $scope.addVariant = (product) -> + # Set new variant category to same as last product variant category to keep compactibility with deleted variant callback to set new variant category + newVariantId = $scope.nextVariantId(); + newVariantCategoryId = product.variants[product.variants.length - 1]?.category_id product.variants.push - id: $scope.nextVariantId() + id: newVariantId unit_value: null unit_description: null on_demand: false @@ -136,8 +139,9 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout on_hand: null price: null tax_category_id: null - category_id: null + category_id: newVariantCategoryId DisplayProperties.setShowVariants product.id, true + DirtyProducts.addVariantProperty(product.id, newVariantId, 'category_id', newVariantCategoryId) $scope.nextVariantId = -> diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index 00340f49d7f..5950886104f 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -87,7 +87,6 @@ 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? @@ -217,10 +216,6 @@ 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? diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index a1244e1d47b..5fc7777cb69 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -37,20 +37,24 @@ = f.label :on_demand do = f.check_box :on_demand, 'data-action': 'change->toggle-control#disableIfPresent change->popout#closeIfChecked' = t(:on_demand) -%td.col-producer.naked_inputs +%td.col-producer.field.naked_inputs = render(SearchableDropdownComponent.new(form: f, name: :supplier_id, aria_label: t('.producer_field_name'), options: producer_options, selected_option: variant.supplier_id, - placeholder_value: t('admin.products_v3.filters.search_for_producers'))) + include_blank: t('admin.products_v3.filters.select_producer'), + placeholder_value: t('admin.products_v3.filters.select_producer'))) + = error_message_on variant, :supplier %td.col-category.field.naked_inputs = render(SearchableDropdownComponent.new(form: f, name: :primary_taxon_id, options: category_options, selected_option: variant.primary_taxon_id, aria_label: t('.category_field_name'), - placeholder_value: t('admin.products_v3.filters.search_for_categories'))) + include_blank: t('admin.products_v3.filters.select_category'), + placeholder_value: t('admin.products_v3.filters.select_category'))) + = error_message_on variant, :primary_taxon %td.col-tax_category.field.naked_inputs = render(SearchableDropdownComponent.new(form: f, name: :tax_category_id, diff --git a/config/locales/en.yml b/config/locales/en.yml index d0764b7a91f..2528e78bc98 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -868,8 +868,10 @@ en: filters: search_products: Search for products search_for_producers: Search for producers + select_producer: Select producer all_producers: All producers search_for_categories: Search for categories + select_category: Select category all_categories: All categories producers: label: Producers diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index 0b184f43536..33232f7a93c 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -20,6 +20,7 @@ let(:tax_categories_search_selector) { 'input[placeholder="Search for tax categories"]' } describe "updating" do + let!(:taxon) { create(:taxon) } let!(:variant_a1) { product_a.variants.first.tap{ |v| v.update! display_name: "Medium box", sku: "APL-01", price: 5.25, on_hand: 5, @@ -349,6 +350,9 @@ click_on "On Hand" # activate popout fill_in "On Hand", with: "3" + + select producer.name, from: 'Producer' + select taxon.name, from: 'Category' end expect { @@ -541,6 +545,9 @@ click_on "Unit" # activate popout fill_in "Unit value", with: "200" + + select producer.name, from: 'Producer' + select taxon.name, from: 'Category' end expect { From 0376c04ad5a4c6ae9ba63b9e471c74457398614f Mon Sep 17 00:00:00 2001 From: wandji20 Date: Tue, 23 Jul 2024 12:52:54 +0100 Subject: [PATCH 2/3] Fix failing specs [OFN-12666] --- .../unit/admin/bulk_product_update_spec.js.coffee | 12 +++++++++--- spec/services/sets/product_set_spec.rb | 5 ++++- spec/system/admin/products_v3/actions_spec.rb | 6 ++++-- spec/system/admin/products_v3/create_spec.rb | 9 +++++++-- 4 files changed, 24 insertions(+), 8 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 77802a3df29..70fe6dac589 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -797,14 +797,20 @@ describe "AdminProductEditCtrl", -> spyOn DisplayProperties, 'setShowVariants' it "adds first and subsequent variants", -> - product = {id: 123, variants: []} + product = {id: 123, variants: [ + { + id: 1, price: 10.0, unit_value: 1, tax_category_id: null, unit_description: '1kg', + on_demand: false, on_hand: null, display_as: null, display_name: null, category_id: 2 + } + ]} $scope.addVariant(product) $scope.addVariant(product) 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, 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} + {id: 1, price: 10.0, unit_value: 1, tax_category_id: null, unit_description: '1kg', on_demand: false, on_hand: null, display_as: null, display_name: null, category_id: 2} + {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: 2} + {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: 2} ] it "shows the variant(s)", -> diff --git a/spec/services/sets/product_set_spec.rb b/spec/services/sets/product_set_spec.rb index 068b49a9007..c04dd4bdb5d 100644 --- a/spec/services/sets/product_set_spec.rb +++ b/spec/services/sets/product_set_spec.rb @@ -304,7 +304,10 @@ [ { id: product.variants.first.id.to_s }, # default variant unchanged # omit ID for new variant - { sku: "new sku", price: "5.00", unit_value: "5", supplier_id: supplier.id }, + { + sku: "new sku", price: "5.00", unit_value: "5", + supplier_id: supplier.id, primary_taxon_id: create(:taxon).id + }, ] } let(:supplier) { create(:supplier_enterprise) } diff --git a/spec/system/admin/products_v3/actions_spec.rb b/spec/system/admin/products_v3/actions_spec.rb index 12c021b0a6f..e864218c1d7 100644 --- a/spec/system/admin/products_v3/actions_spec.rb +++ b/spec/system/admin/products_v3/actions_spec.rb @@ -15,8 +15,8 @@ login_as user end - let(:producer_search_selector) { 'input[placeholder="Search for producers"]' } - let(:categories_search_selector) { 'input[placeholder="Search for categories"]' } + let(:producer_search_selector) { 'input[placeholder="Select producer"]' } + let(:categories_search_selector) { 'input[placeholder="Select category"]' } let(:tax_categories_search_selector) { 'input[placeholder="Search for tax categories"]' } describe "with no products" do @@ -520,6 +520,7 @@ def expect_other_columns_visible expect(page).to have_select( '_products_0_variants_attributes_0_supplier_id', options: [ + 'Select producer', supplier_managed1.name, supplier_managed2.name, supplier_permitted.name ], selected: supplier_managed1.name ) @@ -529,6 +530,7 @@ def expect_other_columns_visible expect(page).to have_select( '_products_1_variants_attributes_0_supplier_id', options: [ + 'Select producer', supplier_managed1.name, supplier_managed2.name, supplier_permitted.name ], selected: supplier_permitted.name ) diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index e30dfba18c1..b7e99a217de 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -6,12 +6,13 @@ include AuthenticationHelper include WebHelper + let!(:supplier) { create(:supplier_enterprise) } + let!(:taxon) { create(:taxon) } + describe "creating a new product" do let!(:stock_location) { create(:stock_location, backorderable_default: false) } - let!(:supplier) { create(:supplier_enterprise) } let!(:distributor) { create(:distributor_enterprise) } let!(:shipping_category) { create(:shipping_category) } - let!(:taxon) { create(:taxon) } before { visit_products_page_as_admin } @@ -81,6 +82,10 @@ find('input[id$="_display_as"]').fill_in with: "2 grams" find('button[aria-label="On Hand"]').click find('input[id$="_price"]').fill_in with: "11.1" + + select supplier.name, from: 'Producer' + select taxon.name, from: 'Category' + if stock == "on_hand" find('input[id$="_on_hand"]').fill_in with: "66" elsif stock == "on_demand" From 55df9416ccd9940a974fe844ba8241a0067c5155 Mon Sep 17 00:00:00 2001 From: wandji20 Date: Tue, 30 Jul 2024 12:09:59 +0100 Subject: [PATCH 3/3] Add test to check when new variant category and producer is empty [OFN-12666] --- spec/system/admin/products_v3/update_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index 33232f7a93c..cc0d3bb6bba 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -413,7 +413,8 @@ end click_on "New variant" - second_new_variant_row = find_field("Name", placeholder: "Apples", with: "").ancestor("tr") + second_new_variant_row = find_field("Name", placeholder: "Apples", + with: "").ancestor("tr") within second_new_variant_row do fill_in "Name", with: "Huge box" end @@ -520,6 +521,8 @@ expect(page).to have_field "Name", with: "N" * 256 expect(page).to have_field "SKU", with: "n" * 256 expect(page).to have_content "is too long" + expect(page.find('.col-producer')).to have_content('must exist') + expect(page.find('.col-category')).to have_content('must exist') expect(page.find_button("Unit")).to have_text "" # have_button selector don't work here expect(page).to have_field "Price", with: "10.25" # other updated value is retained end