Require variant category and supplier when creating new product variants [OFN-12666]

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 = ->

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?
 

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,

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

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)", ->

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) }

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
         )

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"

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 {
@@ -409,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
@@ -516,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
@@ -541,6 +548,9 @@
 
             click_on "Unit" # activate popout
             fill_in "Unit value", with: "200"
+
+            select producer.name, from: 'Producer'
+            select taxon.name, from: 'Category'
           end
 
           expect {