diff --git a/app/models/product_import/product_importer.rb b/app/models/product_import/product_importer.rb index 80a9744493..f83f2102a2 100644 --- a/app/models/product_import/product_importer.rb +++ b/app/models/product_import/product_importer.rb @@ -237,6 +237,15 @@ module ProductImport error_message: e.message)) end [] + rescue CSV::MalformedCSVError => e + # NOTE the init_product_importer method calls build_entries and + # buils_all_entries resulting in the error being raised twice + unless errors.added?(:importer, I18n.t('admin.product_import.model.malformed_csv', + error_message: e.message)) + errors.add(:importer, I18n.t('admin.product_import.model.malformed_csv', + error_message: e.message)) + end + [] end def build_entries_in_range diff --git a/config/locales/en.yml b/config/locales/en.yml index e2a70ff412..b30cc24c02 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -528,6 +528,7 @@ en: line_number: "Line %{number}:" encoding_error: "Please check the language setting of your source file and ensure it is saved with UTF-8 encoding" unexpected_error: "Product Import encountered an unexpected error whilst opening the file: %{error_message}" + malformed_csv: "Product Import encountered a malformed CSV: %{error_message}" index: notice: "Notice" beta_notice: "This feature is still in beta: you may experience some errors while using it. Please don't hesitate to contact support." diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index b14122ce5d..fc1624980d 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -377,6 +377,24 @@ feature "Product Import", js: true do expect(page).to have_no_selector 'input[type=submit][value="Save"]' File.delete('/tmp/test.csv') end + + it "handles cases where files contains malformed data" do + csv_data = "name,producer,category,on_hand,price,units,unit_type,shipping_category\n" + csv_data += "Malformed \rBrocolli,#{enterprise.name},Vegetables,8,2.50,200,g,#{shipping_category.name}\n" + + File.write('/tmp/test.csv', csv_data) + + visit main_app.admin_product_import_path + attach_file 'file', '/tmp/test.csv' + click_button 'Upload' + + expect(page).to have_no_selector '.create-count' + expect(page).to have_no_selector '.update-count' + expect(page).to have_no_selector 'input[type=submit][value="Save"]' + expect(flash_message).to match(I18n.t('admin.product_import.model.malformed_csv', error_message: "")) + + File.delete('/tmp/test.csv') + end end describe "handling enterprise permissions" do diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index 705f99d41e..6f4903ba86 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -50,11 +50,11 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "variant_unit_name", "on_demand", "shipping_category"] - csv << ["Carrots", "User Enterprise", "Vegetables", "5", "3.20", "500", "g", "", "", shipping_category.name] - csv << ["Potatoes", "User Enterprise", "Vegetables", "6", "6.50", "2", "kg", "", "", shipping_category.name] - csv << ["Pea Soup", "User Enterprise", "Vegetables", "8", "5.50", "750", "ml", "", "0", shipping_category.name] - csv << ["Salad", "User Enterprise", "Vegetables", "7", "4.50", "1", "", "bags", "", shipping_category.name] - csv << ["Hot Cross Buns", "User Enterprise", "Cake", "7", "3.50", "1", "", "buns", "1", shipping_category.name] + csv << ["Carrots", enterprise.name, "Vegetables", "5", "3.20", "500", "g", "", "", shipping_category.name] + csv << ["Potatoes", enterprise.name, "Vegetables", "6", "6.50", "2", "kg", "", "", shipping_category.name] + csv << ["Pea Soup", enterprise.name, "Vegetables", "8", "5.50", "750", "ml", "", "0", shipping_category.name] + csv << ["Salad", enterprise.name, "Vegetables", "7", "4.50", "1", "", "bags", "", shipping_category.name] + csv << ["Hot Cross Buns", enterprise.name, "Cake", "7", "3.50", "1", "", "buns", "1", shipping_category.name] end } let(:importer) { import_data csv_data } @@ -136,7 +136,7 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "shipping_category"] - csv << ["Good Carrots", "User Enterprise", "Vegetables", "5", "3.20", "500", "g", shipping_category.name] + csv << ["Good Carrots", enterprise.name, "Vegetables", "5", "3.20", "500", "g", shipping_category.name] csv << ["Bad Potatoes", "", "Vegetables", "6", "6.50", "1", "", shipping_category.name] end } @@ -169,11 +169,28 @@ describe ProductImport::ProductImporter do end end + describe "when uploading a spreadsheet with some malformed data" do + # Use a simple string as CVS.generate will do some escaping + let(:csv_data) { + csv = "name,producer,category,on_hand,price,units,unit_type,shipping_category\n" + csv += "Good Carrots,#{enterprise.name},Vegetables,5,3.20,500,g,#{shipping_category.name}\n" + csv += "Malformed \rBrocolli,#{enterprise.name},Vegetables,8,2.50,200,g,#{shipping_category.name}\n" + } + let(:importer) { import_data csv_data } + + # NOTE an unquoted \n will create a non valid line which will fail entry validation hence why we are only testing with \r + it "should raise an unquoted field error if data include unquoted filed with \r character" do + expect(importer.errors.messages.values).to include( + [I18n.t('admin.product_import.model.malformed_csv', error_message: "Unquoted fields do not allow \\r or \\n (line 3).")] + ) + end + end + describe "when shipping category is missing" do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "variant_unit_name", "on_demand", "shipping_category"] - csv << ["Shipping Test", "User Enterprise", "Vegetables", "5", "3.20", "500", "g", "", nil, nil] + csv << ["Shipping Test", enterprise.name, "Vegetables", "5", "3.20", "500", "g", "", nil, nil] end } let(:importer) { import_data csv_data } @@ -191,7 +208,7 @@ describe ProductImport::ProductImporter do CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type"] csv << ["Product 1", "Non-existent Enterprise", "Vegetables", "5", "5.50", "500", "g"] - csv << ["Product 2", "Non-Producer", "Vegetables", "5", "5.50", "500", "g"] + csv << ["Product 2", enterprise4.name, "Vegetables", "5", "5.50", "500", "g"] end } let(:importer) { import_data csv_data } @@ -209,8 +226,8 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] - csv << ["Hypothetical Cake", "Another Enterprise", "Cake", "5", "5.50", "500", "g", "Preexisting Banana", shipping_category.name] - csv << ["Hypothetical Cake", "Another Enterprise", "Cake", "6", "3.50", "500", "g", "Emergent Coffee", shipping_category.name] + csv << ["Hypothetical Cake", enterprise2.name, "Cake", "5", "5.50", "500", "g", "Preexisting Banana", shipping_category.name] + csv << ["Hypothetical Cake", enterprise2.name, "Cake", "6", "3.50", "500", "g", "Emergent Coffee", shipping_category.name] end } let(:importer) { import_data csv_data } @@ -251,7 +268,7 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "description", "category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] - csv << ["Hypothetical Cake", "Another Enterprise", "New Description", "Cake", "5", "5.50", "500", "g", "Preexisting Banana", shipping_category.name] + csv << ["Hypothetical Cake", enterprise2.name, "New Description", "Cake", "5", "5.50", "500", "g", "Preexisting Banana", shipping_category.name] end } let(:importer) { import_data csv_data } @@ -270,11 +287,11 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] - csv << ["Potatoes", "User Enterprise", "Vegetables", "5", "3.50", "500", "g", "Small Bag", shipping_category.name] - csv << ["Chives", "User Enterprise", "Vegetables", "6", "4.50", "500", "g", "Bunch", shipping_category.name] - csv << ["Potatoes", "User Enterprise", "Vegetables", "6", "5.50", "2", "kg", "Big Bag", shipping_category.name] - csv << ["Potatoes", "User Enterprise", "Vegetables", "6", "22.00", "10000", "g", "Small Sack", shipping_category.name] - csv << ["Potatoes", "User Enterprise", "Vegetables", "6", "60.00", "30000", "", "Big Sack", shipping_category.name] + csv << ["Potatoes", enterprise.name, "Vegetables", "5", "3.50", "500", "g", "Small Bag", shipping_category.name] + csv << ["Chives", enterprise.name, "Vegetables", "6", "4.50", "500", "g", "Bunch", shipping_category.name] + csv << ["Potatoes", enterprise.name, "Vegetables", "6", "5.50", "2", "kg", "Big Bag", shipping_category.name] + csv << ["Potatoes", enterprise.name, "Vegetables", "6", "22.00", "10000", "g", "Small Sack", shipping_category.name] + csv << ["Potatoes", enterprise.name, "Vegetables", "6", "60.00", "30000", "", "Big Sack", shipping_category.name] end } let(:importer) { import_data csv_data } @@ -318,8 +335,8 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "on_demand", "sku", "shipping_category"] - csv << ["Beetroot", "And Another Enterprise", "Vegetables", "5", "3.50", "500", "g", "0", nil, shipping_category.name] - csv << ["Tomato", "And Another Enterprise", "Vegetables", "6", "5.50", "500", "g", "1", "TOMS", shipping_category.name] + csv << ["Beetroot", enterprise3.name, "Vegetables", "5", "3.50", "500", "g", "0", nil, shipping_category.name] + csv << ["Tomato", enterprise3.name, "Vegetables", "6", "5.50", "500", "g", "1", "TOMS", shipping_category.name] end } let(:importer) { import_data csv_data } @@ -356,8 +373,8 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type"] - csv << ["Beetroot", "And Another Enterprise", "Meat", "5", "3.50", "500", "g"] - csv << ["Tomato", "And Another Enterprise", "Vegetables", "6", "5.50", "500", "Kg"] + csv << ["Beetroot", enterprise3.name, "Meat", "5", "3.50", "500", "g"] + csv << ["Tomato", enterprise3.name, "Vegetables", "6", "5.50", "500", "Kg"] end } let(:importer) { import_data csv_data } @@ -379,11 +396,11 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "description", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] - csv << ["Oats", "User Enterprise", "Cereal", "", "50", "3.50", "500", "g", "Rolled Oats", shipping_category.name] # Update - csv << ["Oats", "User Enterprise", "Cereal", "", "80", "3.75", "500", "g", "Flaked Oats", shipping_category.name] # Update - csv << ["Oats", "User Enterprise", "Cereal", "", "60", "5.50", "500", "g", "Magic Oats", shipping_category.name] # Add - csv << ["Oats", "User Enterprise", "Cereal", "", "70", "8.50", "500", "g", "French Oats", shipping_category.name] # Add - csv << ["Oats", "User Enterprise", "Cereal", "", "70", "8.50", "500", "g", "Scottish Oats", shipping_category.name] # Add + csv << ["Oats", enterprise.name, "Cereal", "", "50", "3.50", "500", "g", "Rolled Oats", shipping_category.name] # Update + csv << ["Oats", enterprise.name, "Cereal", "", "80", "3.75", "500", "g", "Flaked Oats", shipping_category.name] # Update + csv << ["Oats", enterprise.name, "Cereal", "", "60", "5.50", "500", "g", "Magic Oats", shipping_category.name] # Add + csv << ["Oats", enterprise.name, "Cereal", "", "70", "8.50", "500", "g", "French Oats", shipping_category.name] # Add + csv << ["Oats", enterprise.name, "Cereal", "", "70", "8.50", "500", "g", "Scottish Oats", shipping_category.name] # Add end } let(:importer) { import_data csv_data } @@ -415,11 +432,11 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] - csv << ["Bag of Oats", "User Enterprise", "Cereal", "60", "5.50", "500", "g", "Magic Oats", shipping_category.name] # Add - csv << ["Bag of Oats", "User Enterprise", "Cereal", "70", "8.50", "500", "g", "French Oats", shipping_category.name] # Add - csv << ["Bag of Oats", "User Enterprise", "Cereal", "80", "9.50", "500", "g", "Organic Oats", shipping_category.name] # Add - csv << ["Bag of Oats", "User Enterprise", "Cereal", "90", "7.50", "500", "g", "Scottish Oats", shipping_category.name] # Add - csv << ["Bag of Oats", "User Enterprise", "Cereal", "30", "6.50", "500", "g", "Breakfast Oats", shipping_category.name] # Add + csv << ["Bag of Oats", enterprise.name, "Cereal", "60", "5.50", "500", "g", "Magic Oats", shipping_category.name] # Add + csv << ["Bag of Oats", enterprise.name, "Cereal", "70", "8.50", "500", "g", "French Oats", shipping_category.name] # Add + csv << ["Bag of Oats", enterprise.name, "Cereal", "80", "9.50", "500", "g", "Organic Oats", shipping_category.name] # Add + csv << ["Bag of Oats", enterprise.name, "Cereal", "90", "7.50", "500", "g", "Scottish Oats", shipping_category.name] # Add + csv << ["Bag of Oats", enterprise.name, "Cereal", "30", "6.50", "500", "g", "Breakfast Oats", shipping_category.name] # Add end } @@ -481,9 +498,9 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "distributor", "producer", "on_hand", "price", "units", "unit_type", "variant_unit_name"] - csv << ["Beans", "Another Enterprise", "User Enterprise", "5", "3.20", "500", "g", ""] - csv << ["Sprouts", "Another Enterprise", "User Enterprise", "6", "6.50", "500", "g", ""] - csv << ["Cabbage", "Another Enterprise", "User Enterprise", "2001", "1.50", "1", "", "Whole"] + csv << ["Beans", enterprise2.name, enterprise.name, "5", "3.20", "500", "g", ""] + csv << ["Sprouts", enterprise2.name, enterprise.name, "6", "6.50", "500", "g", ""] + csv << ["Cabbage", enterprise2.name, enterprise.name, "2001", "1.50", "1", "", "Whole"] end } let(:importer) { import_data csv_data, import_into: 'inventories' } @@ -525,7 +542,7 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "display_name", "distributor", "producer", "on_hand", "price", "units"] - csv << ["Oats", "Porridge Oats", "Another Enterprise", "User Enterprise", "900", "", "500"] + csv << ["Oats", "Porridge Oats", enterprise2.name, enterprise.name, "900", "", "500"] end } let(:importer) { import_data csv_data, import_into: 'inventories' } @@ -548,7 +565,7 @@ describe ProductImport::ProductImporter do let(:csv_data) { CSV.generate do |csv| csv << ["name", "distributor", "producer", "on_hand", "price", "units", "variant_unit_name"] - csv << ["Cabbage", "Another Enterprise", "User Enterprise", "900", "", "1", "Whole"] + csv << ["Cabbage", enterprise2.name, enterprise.name, "900", "", "1", "Whole"] end } let(:importer) { import_data csv_data, import_into: 'inventories' } @@ -571,8 +588,8 @@ describe ProductImport::ProductImporter do it "only allows product import into enterprises the user is permitted to manage" do csv_data = CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "shipping_category"] - csv << ["My Carrots", "User Enterprise", "Vegetables", "5", "3.20", "500", "g", shipping_category.name] - csv << ["Your Potatoes", "Another Enterprise", "Vegetables", "6", "6.50", "1", "kg", shipping_category.name] + csv << ["My Carrots", enterprise.name, "Vegetables", "5", "3.20", "500", "g", shipping_category.name] + csv << ["Your Potatoes", enterprise2.name, "Vegetables", "6", "6.50", "1", "kg", shipping_category.name] end importer = import_data csv_data, import_user: user @@ -596,7 +613,7 @@ describe ProductImport::ProductImporter do it "allows creating inventories for producers that a user's hub has permission for" do csv_data = CSV.generate do |csv| csv << ["name", "producer", "distributor", "on_hand", "price", "units", "unit_type"] - csv << ["Beans", "User Enterprise", "Another Enterprise", "777", "3.20", "500", "g"] + csv << ["Beans", enterprise.name, enterprise2.name, "777", "3.20", "500", "g"] end importer = import_data csv_data, import_into: 'inventories' @@ -620,8 +637,8 @@ describe ProductImport::ProductImporter do it "does not allow creating inventories for producers that a user's hubs don't have permission for" do csv_data = CSV.generate do |csv| csv << ["name", "producer", "on_hand", "price", "units", "unit_type"] - csv << ["Beans", "User Enterprise", "5", "3.20", "500", "g"] - csv << ["Sprouts", "User Enterprise", "6", "6.50", "500", "g"] + csv << ["Beans", enterprise.name, "5", "3.20", "500", "g"] + csv << ["Sprouts", enterprise.name, "6", "6.50", "500", "g"] end importer = import_data csv_data, import_into: 'inventories' @@ -644,8 +661,8 @@ describe ProductImport::ProductImporter do it "can reset all products for an enterprise that are not present in the uploaded file to zero stock" do csv_data = CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "shipping_category"] - csv << ["Carrots", "User Enterprise", "Vegetables", "5", "3.20", "500", "g", shipping_category.name] - csv << ["Beans", "User Enterprise", "Vegetables", "6", "6.50", "500", "g", shipping_category.name] + csv << ["Carrots", enterprise.name, "Vegetables", "5", "3.20", "500", "g", shipping_category.name] + csv << ["Beans", enterprise.name, "Vegetables", "6", "6.50", "500", "g", shipping_category.name] end importer = import_data csv_data, reset_all_absent: true @@ -681,8 +698,8 @@ describe ProductImport::ProductImporter do it "can reset all inventory items for an enterprise that are not present in the uploaded file to zero stock" do csv_data = CSV.generate do |csv| csv << ["name", "distributor", "producer", "on_hand", "price", "units", "unit_type"] - csv << ["Beans", "Another Enterprise", "User Enterprise", "6", "3.20", "500", "g"] - csv << ["Sprouts", "Another Enterprise", "User Enterprise", "7", "6.50", "500", "g"] + csv << ["Beans", enterprise2.name, enterprise.name, "6", "3.20", "500", "g"] + csv << ["Sprouts", enterprise2.name, enterprise.name, "7", "6.50", "500", "g"] end importer = import_data csv_data, import_into: 'inventories', reset_all_absent: true