From a0dba001bcd45ba50acf4ff8b332faa0d8a46b46 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 2 Aug 2023 10:25:44 +1000 Subject: [PATCH] Attempt to save all records in bulk update Before, it would abort after the first invalid record, and it doesn't tell you about the others. This way you find out about all at once. This affects the existing Bulk Edit Products screen, and can result in longer error messages than before. But I would argue that's a good thing. I think this is technically optional for BUU at this point, but a helpful improvement. --- app/services/sets/product_set.rb | 5 +++-- spec/services/sets/product_set_spec.rb | 27 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb index 6222a96c51..06f3ac4f4f 100644 --- a/app/services/sets/product_set.rb +++ b/app/services/sets/product_set.rb @@ -13,9 +13,10 @@ module Sets end def save - @collection_hash.each_value.all? do |product_attributes| + # Attempt to save all records, collecting model errors. + @collection_hash.each_value.map do |product_attributes| update_product_attributes(product_attributes) - end + end.all? end def collection_attributes=(attributes) diff --git a/spec/services/sets/product_set_spec.rb b/spec/services/sets/product_set_spec.rb index 9b6fd5d2f8..e68601f887 100644 --- a/spec/services/sets/product_set_spec.rb +++ b/spec/services/sets/product_set_spec.rb @@ -229,6 +229,33 @@ describe Sets::ProductSet do expect(product_set.collection[0]).to eq product_a.reload expect(product_set.collection[1]).to eq product_b.reload end + + context 'first product has an error' do + let(:collection_hash) do + { + 0 => { + id: product_a.id, + name: "", # Product Name can't be blank + }, + 1 => { + id: product_b.id, + name: "Bananes", + }, + } + end + + it 'continues to update subsequent products' do + product_set.save + + # Errors are logged on the model + first_item = product_set.collection[0] + expect(first_item.errors.full_messages.to_sentence).to eq "Product Name can't be blank" + expect(first_item.name).to eq "" + + # Subsequent product was updated + expect(product_b.reload.name).to eq "Bananes" + end + end end end end