diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 3ae92a38bd..2755f1b36d 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -46,7 +46,7 @@ module Spree # Re-fill the form with deleted params on product @on_hand = request.params[:product][:on_hand] @on_demand = request.params[:product][:on_demand] - render :new + render :new, status: :unprocessable_entity end end end diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index 8ce73489a9..6c7b71eff9 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -149,6 +149,7 @@ module ProductImport end end + # rubocop:disable Metrics/AbcSize def save_new_product(entry) @already_created ||= {} # If we've already added a new product with these attributes @@ -162,7 +163,7 @@ module ProductImport return end - product = Spree::Product.new + product = Spree::Product.new(supplier_id: entry.enterprise_id) product.assign_attributes( entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name') ) @@ -177,6 +178,7 @@ module ProductImport @already_created.deep_merge! entry.enterprise_id => { entry.name => product.id } end + # rubocop:enable Metrics/AbcSize def save_variant(entry) variant = entry.product_object diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 2705ff1126..4db5a60d2c 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -377,7 +377,7 @@ module ProductImport end def mark_as_new_product(entry) - new_product = Spree::Product.new + new_product = Spree::Product.new(supplier_id: entry.enterprise_id) new_product.assign_attributes( entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name') ) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 3703cc1864..d484b6d123 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -69,6 +69,7 @@ module Spree attr_accessor :price, :display_as, :unit_value, :unit_description, :tax_category_id, :shipping_category_id, :primary_taxon_id, :supplier_id + after_validation :validate_variant_attrs, on: :create after_create :ensure_standard_variant after_update :touch_supplier, if: :saved_change_to_primary_taxon_id? around_destroy :destruction @@ -289,6 +290,21 @@ module Spree private + def validate_variant_attrs + # Avoid running validation when we can't set variant attrs + # eg clone product. Will raise error if clonning a product with no variant + return if variants.first&.valid? + + unless Spree::Taxon.find_by(id: primary_taxon_id) + errors.add(:primary_taxon_id, + I18n.t('activerecord.errors.models.spree/product.must_exist')) + end + return if Enterprise.find_by(id: supplier_id) + + errors.add(:supplier_id, + I18n.t('activerecord.errors.models.spree/product.must_exist')) + end + def update_units return unless saved_change_to_variant_unit? || saved_change_to_variant_unit_name? diff --git a/app/views/spree/admin/products/new.html.haml b/app/views/spree/admin/products/new.html.haml index db7968a1da..70d620ef79 100644 --- a/app/views/spree/admin/products/new.html.haml +++ b/app/views/spree/admin/products/new.html.haml @@ -12,7 +12,7 @@ = f.label :supplier, t(".supplier") %span.required * = f.select :supplier_id, options_from_collection_for_select(@producers, :id, :name, @product.supplier_id), { include_blank: t("spree.admin.products.new.supplier_select_placeholder") }, { "data-controller": "tom-select", class: "primary" } - = f.error_message_on :supplier + = f.error_message_on :supplier_id .eight.columns.omega = f.field_container :name do = f.label :name, t(".product_name") diff --git a/config/locales/en.yml b/config/locales/en.yml index f2f0ed92ab..ddf29f38eb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -68,7 +68,7 @@ en: spree/product: name: "Product Name" price: "Price" - primary_taxon: "Product Category" + primary_taxon_id: "Product Category" shipping_category_id: "Shipping Category" variant_unit: "Variant Unit" variant_unit_name: "Variant Unit Name" @@ -120,6 +120,8 @@ en: attributes: base: card_expired: "has expired" + spree/product: + must_exist: 'must exist' order_cycle: attributes: orders_close_at: diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 9c1e83e8f5..ee64cccafe 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -35,9 +35,7 @@ class SuppliedProductBuilder < DfcBuilder apply(supplied_product, variant) end else - product = import_product(supplied_product) - product.ensure_standard_variant - product.variants.first.supplier = supplier + product = import_product(supplied_product, supplier) product.variants.first end.tap do |variant| link = supplied_product.semanticId @@ -62,15 +60,16 @@ class SuppliedProductBuilder < DfcBuilder end end - def self.import_product(supplied_product) + def self.import_product(supplied_product, supplier) Spree::Product.new( name: supplied_product.name, description: supplied_product.description, - price: 0 # will be in DFC Offer + price: 0, # will be in DFC Offer + supplier_id: supplier.id, + primary_taxon_id: taxon(supplied_product).id ).tap do |product| QuantitativeValueBuilder.apply(supplied_product.quantity, product) product.ensure_standard_variant - product.variants.first.primary_taxon = taxon(supplied_product) 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 c39aab031e..4b3fa4c445 100644 --- a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb @@ -107,7 +107,7 @@ RSpec.describe SuppliedProductBuilder do } it "creates a new Spree::Product" do - product = builder.import_product(supplied_product) + product = builder.import_product(supplied_product, supplier) expect(product).to be_a(Spree::Product) expect(product.name).to eq("Tomato") @@ -117,7 +117,7 @@ RSpec.describe SuppliedProductBuilder do describe "taxon" do it "assigns the taxon matching the DFC product type" do - product = builder.import_product(supplied_product) + product = builder.import_product(supplied_product, supplier) expect(product.variants.first.primary_taxon).to eq(taxon) end diff --git a/spec/controllers/api/v0/products_controller_spec.rb b/spec/controllers/api/v0/products_controller_spec.rb index df999c4ab4..3633dab6ea 100644 --- a/spec/controllers/api/v0/products_controller_spec.rb +++ b/spec/controllers/api/v0/products_controller_spec.rb @@ -120,7 +120,10 @@ RSpec.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", "variant_unit", "price"]) + expect(errors.keys).to match_array([ + "name", "variant_unit", "price", + "primary_taxon_id", "supplier_id" + ]) end it "can update a product" do diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index f41c4adfcd..6752239a8c 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -168,7 +168,17 @@ RSpec.describe Spree::Admin::ProductsController, type: :controller do spree_put :create, product: product_attrs_with_image - expect(response.status).to eq 200 + expect(response.status).to eq 422 + end + end + + describe "when variant attributes are missing" do + it 'renders 422 error' do + spree_post :create, product: product_attrs.merge!( + { supplier_id: nil, primary_taxon_id: nil } + ), + button: 'create' + expect(response.status).to eq 422 end end end diff --git a/spec/system/admin/products_spec.rb b/spec/system/admin/products_spec.rb index abe75464db..bd19bdcbfd 100644 --- a/spec/system/admin/products_spec.rb +++ b/spec/system/admin/products_spec.rb @@ -170,14 +170,7 @@ RSpec.describe ' expect(page).to have_content "Unit value is not a number" end - it "creating product with empty product category" do - pending("#12591") - - login_as_admin - visit spree.admin_products_path - - click_link 'New Product' - + it "creating product with empty product category fails" do fill_in 'product_name', with: 'Hot Cakes' select 'New supplier', from: 'product_supplier_id' select "Weight (kg)", from: 'product_variant_unit_with_scale'