From 16cae2dbcc5c17af2df41514dcdd7987d24f4f82 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 27 Nov 2024 11:56:49 +0500 Subject: [PATCH 1/3] 12973: fix error raised in variant creation - if the sheet doesn't have the units present, then the variant is not saved due to model validation - After that, while assigning on_hand value, error is raised that the variant is not created first - Now this commit makes sure that the variant is created before implementing above logic --- app/models/product_import/entry_validator.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index f720af4aa5..bc1c4153ce 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -79,10 +79,9 @@ module ProductImport if entry.attributes['on_hand'].present? new_variant.on_hand = entry.attributes['on_hand'] end + check_on_hand_nil(entry, new_variant) end - check_on_hand_nil(entry, new_variant) - if new_variant.valid? entry.product_object = new_variant entry.validates_as = 'new_variant' unless entry.errors? From f3a30f94db934772667ea4f14bd6578130e1e4ad Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 27 Nov 2024 12:00:11 +0500 Subject: [PATCH 2/3] 12973: fix error rendering on UI - The caught errors do not get rendered to the UI because of incorrect rendering methods --- app/controllers/admin/product_import_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/product_import_controller.rb b/app/controllers/admin/product_import_controller.rb index 9331023aad..7cbb8f64b5 100644 --- a/app/controllers/admin/product_import_controller.rb +++ b/app/controllers/admin/product_import_controller.rb @@ -30,13 +30,13 @@ module Admin def validate_data return unless process_data('validate') - render json: @importer.import_results, response: 200 + render json: @importer.import_results end def save_data return unless process_data('save') - render json: @importer.save_results, response: 200 + render json: @importer.save_results end def reset_absent_products @@ -76,7 +76,7 @@ module Admin begin @importer.public_send("#{method}_entries") rescue StandardError => e - render json: e.message, response: 500 + render plain: e.message, status: :internal_server_error return false end From 93a313085194be9f69a55af42c03c2bc70917ef4 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 29 Nov 2024 04:17:57 +0500 Subject: [PATCH 3/3] 12973: add specs --- spec/models/product_importer_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index a2a2917f20..a2a5b96b25 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -1012,6 +1012,30 @@ RSpec.describe ProductImport::ProductImporter do expect(lettuce.count_on_hand).to eq 96 # In different enterprise; unchanged end end + + # Fix https://github.com/openfoodfoundation/openfoodnetwork/issues/12973 + describe 'update existing product with units and on_hand/on_demand is empty' do + let(:csv_data) do + CSV.generate do |csv| + csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", + "shipping_category", "on_demand"] + csv << ["Beetroot", enterprise3.name, "Vegetables", "", "6.50", "", "g", + shipping_category.name, "1"] + end + end + let(:importer) { import_data csv_data } + + it "returns the units error due to invalid data" do + importer.validate_entries + entries = JSON.parse(importer.entries_json) + + expect(entries.dig('2', 'errors')).to eq( + { + "units" => "Units can't be blank", + } + ) + end + end end private