Show error message summary at top of form

app/reflexes/products_reflex.rb

@@ -41,8 +41,7 @@ class ProductsReflex < ApplicationReflex
       # flash[:success] = with_locale { I18n.t('.success') }
       # morph_admin_flashes  # ERROR: selector morph type has already been set
     elsif product_set.errors.present?
-      # @error_msg = with_locale{ I18n.t('.products_have_error', count: product_set.invalid.count) }
+      @error_counts = { saved: product_set.saved_count, invalid: product_set.invalid.count }
-      @error_msg = "#{product_set.invalid.count} products have errors."
     end
 
     render_products_form
@@ -87,10 +86,12 @@ class ProductsReflex < ApplicationReflex
   end
 
   def render_products_form
+    locals = { products: @products }
+    locals[:error_counts] = @error_counts if @error_counts.present?
+
     cable_ready.replace(
       selector: "#products-form",
-      html: render(partial: "admin/products_v3/table",
+      html: render(partial: "admin/products_v3/table", locals:)
-                   locals: { products: @products, error_msg: @error_msg })
     ).broadcast
     morph :nothing
 

app/views/admin/products_v3/_table.html.haml

@@ -1,15 +1,16 @@
 = form_with url: bulk_update_admin_products_path, method: :patch, id: "products-form",
             builder: BulkFormBuilder,
             html: {'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#bulk_update',
-                   'data-controller': "bulk-form", 'data-bulk-form-disable-selector-value': "#sort,#filters"} do |form|
+                   'data-controller': "bulk-form", 'data-bulk-form-disable-selector-value': "#sort,#filters" } do |form|
-  %fieldset.form-actions.hidden{ 'data-bulk-form-target': "actions" }
+  %fieldset.form-actions{ class: ("hidden" unless defined?(error_counts)), 'data-bulk-form-target': "actions" }
     .container
-      .status.ten.columns
+      .status.eleven.columns
-        .changed_summary{ 'data-bulk-form-target': "changedSummary", 'data-translation-key': 'admin.products_v3.table.changed_summary'}
+        .modified_summary{ 'data-bulk-form-target': "changedSummary", 'data-translation-key': 'admin.products_v3.table.changed_summary'}
-        - if defined?(error_msg) && error_msg.present?
+        - if defined?(error_counts)
-          .error
+          .error_summary
-            = error_msg
+            -# X products were saved correctly, but Y products could not be saved correctly. Please review errors and try again
-      .form-buttons.six.columns
+            = t('.error_summary.saved', count: error_counts[:saved]) + t('.error_summary.invalid', count: error_counts[:invalid])
+      .form-buttons.five.columns
         = form.submit t('.reset'), type: :reset, class: "medium", 'data-reflex': 'click->products#fetch'
         = form.submit t('.save'), class: "medium"
   %table.products

app/webpacker/css/admin_v3/shared/forms.scss

@@ -282,6 +282,20 @@ fieldset {
   text-align: center;
 }
 
+.modified_summary {
+  font-size: inherit;
+}
+
+.error_summary {
+  font-size: inherit;
+
+  @extend .icon-remove-sign;
+  &:before {
+    font-family: FontAwesome;
+    color: $color-error;
+  }
+}
+
 select {
   @extend input, [type="text"];
   background-color: white;

config/locales/en.yml

@@ -837,13 +837,18 @@ en:
           zero: ""
           one: "%{count} product modified."
           other: "%{count} products modified."
+        error_summary:
+          saved:
+            zero: ""
+            one: "%{count} product was saved correctly, but "
+            other: "%{count} products were saved correctly, but "
+          invalid:
+            one: "%{count} product could not be saved. Please review the errors and try again."
+            other: "%{count} products could not be saved. Please review the errors and try again."
         save: Save changes
         reset: Discard changes
       bulk_update: # TODO: fix these
         success: "Products successfully updated" #TODO: add count
-        products_have_error:
-          one: "%{count} product has an error."
-          other: "%{count} products have an error."
     product_import:
       title: Product Import
       file_not_found: File not found or could not be opened

spec/reflexes/products_reflex_spec.rb

@@ -13,7 +13,7 @@ describe ProductsReflex, type: :reflex do
     Flipper.enable(:admin_style_v3)
   end
 
-  describe 'fetch' do
+  describe '#fetch' do
     subject{ build_reflex(method_name: :fetch, **context) }
 
     describe "sorting" do
@@ -41,6 +41,7 @@ describe ProductsReflex, type: :reflex do
              sku: "APL-01",
              price: 5.25)
     }
+    let!(:product_c) { create(:simple_product, name: "Carrots", sku: "CAR-00") }
     let!(:product_b) { create(:simple_product, name: "Bananas", sku: "BAN-00") }
     let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") }
 
@@ -123,17 +124,21 @@ describe ProductsReflex, type: :reflex do
           "products" => [
             {
               "id" => product_a.id.to_s,
-              "name" => "",
+              "name" => "Pommes",
             },
             {
               "id" => product_b.id.to_s,
-              "name" => "",
+              "name" => "", # Name can't be blank
+            },
+            {
+              "id" => product_c.id.to_s,
+              "name" => "", # Name can't be blank
             },
           ]
         }
 
         reflex = run_reflex(:bulk_update, params:)
-        expect(reflex.get(:error_msg)).to include "2 products have errors"
+        expect(reflex.get(:error_counts)).to eq({ saved: 1, invalid: 2 })
 
         # # WTF
         # expect{ reflex(:bulk_update, params:) }.to broadcast(

spec/system/admin/products_v3/products_spec.rb

@@ -270,37 +270,54 @@ describe 'As an admin, I can see the new product page' do
       end
     end
 
-    it "shows errors for both product and variant fields" do
+    context "with invalid data" do
-      within row_containing_name("Apples") do
+      let!(:product_b) { create(:simple_product, name: "Bananas") }
-        fill_in "Name", with: ""
+
-        fill_in "SKU", with: "A" * 256
+      before do
-      end
+        within row_containing_name("Apples") do
-      within row_containing_name("Medium box") do
+          fill_in "Name", with: ""
-        fill_in "Name", with: "L" * 256
+          fill_in "SKU", with: "A" * 256
-        fill_in "SKU", with: "1" * 256
+        end
+        within row_containing_name("Medium box") do
+          fill_in "Name", with: "L" * 256
+          fill_in "SKU", with: "1" * 256
+          fill_in "Price", with: "10.25"
+        end
       end
 
-      expect {
+      it "shows errors for both product and variant fields" do
-        click_button "Save changes"
+        # Also update another product with valid data
-        product_a.reload
+        within row_containing_name("Bananas") do
-      }.to_not change { product_a.name }
+          fill_in "Name", with: "Bananes"
+        end
 
-      # (there's no identifier displayed, so the user must remember which product it is..)
+        expect {
-      within row_containing_name("") do
+          click_button "Save changes"
-        expect(page).to have_field "Name", with: ""
+          product_a.reload
-        expect(page).to have_content "can't be blank"
+        }.to_not change { product_a.name }
-        expect(page).to have_field "SKU", with: "A" * 256
-        expect(page).to have_content "is too long"
-      end
-      pending
-      within row_containing_name("L" * 256) do
-        expect(page).to have_field "Name", with: "L" * 256
-        expect(page).to have_field "SKU", with: "1" * 256
-        expect(page).to have_content "is too long"
-        expect(page).to have_field "Price", with: "10.25"
-      end
 
-      expect(page).to have_content "Please review the errors and try again"
+        # pending("unchanged rows are being saved") # TODO: don't report unchanged rows
+        # expect(page).to_not have_content("rows were saved correctly")
+        # Both the product and variant couldn't be saved.
+        expect(page).to have_content("1 product could not be saved")
+        expect(page).to have_content "Please review the errors and try again"
+
+        # (there's no identifier displayed, so the user must remember which product it is..)
+        within row_containing_name("") do
+          expect(page).to have_field "Name", with: ""
+          expect(page).to have_content "can't be blank"
+          expect(page).to have_field "SKU", with: "A" * 256
+          expect(page).to have_content "is too long"
+        end
+
+        pending "bug #11748"
+        within row_containing_name("L" * 256) do
+          expect(page).to have_field "Name", with: "L" * 256
+          expect(page).to have_field "SKU", with: "1" * 256
+          expect(page).to have_content "is too long"
+          expect(page).to have_field "Price", with: "10.25" # other updated value is retained
+        end
+      end
     end
   end