Merge pull request openfoodfoundation#11468 from dacook/buu-editing-p…

…art2-11059

[BUU] Editing - part 2 (product and variant text fields) 🚧

app/models/spree/price.rb

@@ -44,7 +44,7 @@ def parse_price(price)
                                       :'number.currency.format.delimiter'])
       non_price_characters = /[^0-9\-#{separator}]/
       # Strip everything else first
-      price.gsub!(non_price_characters, '')
+      price = price.gsub(non_price_characters, '')
       # Then replace the locale-specific decimal separator with the standard separator if necessary
       price.gsub!(separator, '.') unless separator == '.'
 

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

@@ -32,15 +32,15 @@
         %th.align-left= t('admin.products_page.columns.category')
         %th.align-left= t('admin.products_page.columns.tax_category')
         %th.align-left= t('admin.products_page.columns.inherits_properties')
-    - products.each do |product, i|
-      = form.fields_for(product) do |product_form|
+    - products.each do |product|
+      = form.fields_for("products", product, index: nil) do |product_form|
         %tbody.relaxed
           %tr
             %td.align-left.header
-              = product_form.hidden_field :id, name: "[products][#{i}][id]" #todo: can we remove #{i} and implicitly pop?
-              .line-clamp-1= product_form.text_field :name, name: "[products][#{i}][name]", id: "_product_name_#{product.id}", 'aria-label': t('admin.products_page.columns.name')
+              = product_form.hidden_field :id
+              = product_form.text_field :name, 'aria-label': t('admin.products_page.columns.name')
             %td.align-right
-              .line-clamp-1= product.sku
+              = product_form.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
             %td.align-right
               .line-clamp-1
                 = product.variant_unit.upcase_first
@@ -59,22 +59,25 @@
             %td.align-left
               .line-clamp-1= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)
           - product.variants.each do |variant|
-            %tr.condensed
-              %td.align-left
-                .line-clamp-1= variant.display_name
-              %td.align-right
-                .line-clamp-1= variant.sku
-              %td.align-right
-                .line-clamp-1= variant.unit_to_display
-              %td.align-right
-                .line-clamp-1= number_to_currency(variant.price)
-              %td.align-right
-                .line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements.
-              %td.align-left
-                .line-clamp-1= variant.product.supplier.name # same as product
-              %td.align-left
-                -# empty
-              %td.align-left
-                .line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
-              %td.align-left
-                -# empty
+            - prefix = "[products][][variants_attributes][]" # Couldn't convince the formbuilder to generate this for me, so for now we manually add the prefix
+            = form.fields_for(variant) do |variant_form|
+              %tr.condensed
+                %td.align-left
+                  = variant_form.hidden_field :id, name: "#{prefix}[id]"
+                  = variant_form.text_field :display_name, name: "#{prefix}[display_name]", 'aria-label': t('admin.products_page.columns.name'), placeholder: product.name
+                %td.align-right
+                  = variant_form.text_field :sku, name: "#{prefix}[sku]", 'aria-label': t('admin.products_page.columns.sku')
+                %td.align-right
+                  .line-clamp-1= variant.unit_to_display
+                %td.align-right
+                  = variant_form.text_field :price, name: "#{prefix}[price]", 'aria-label': t('admin.products_page.columns.price'), value: number_to_currency(variant.price, unit: '')&.strip # TODO: add a spec to prove that this formatting is necessary. If so, it should be in a shared form helper for currency inputs
+                %td.align-right
+                  .line-clamp-1= variant.on_hand || 0 #TODO: spec for this according to requirements.
+                %td.align-left
+                  .line-clamp-1= variant.product.supplier.name # same as product
+                %td.align-left
+                  -# empty
+                %td.align-left
+                  .line-clamp-1= variant.tax_category&.name || "None" # TODO: convert to dropdown, else translate hardcoded string.
+                %td.align-left
+                  -# empty

spec/models/spree/price_spec.rb

@@ -25,5 +25,14 @@ module Spree
         expect(expensive_variant.persisted?).to be true
       end
     end
+
+    context "with string" do
+      it "parses the price" do
+        variant.price = " 10.25 eur"
+        variant.save
+
+        expect(variant.reload.price).to eq 10.25
+      end
+    end
   end
 end

spec/reflexes/products_reflex_spec.rb

@@ -34,24 +34,63 @@
   end
 
   describe '#bulk_update' do
-    let!(:product_b) { create(:simple_product, name: "Bananas") }
-    let!(:product_a) { create(:simple_product, name: "Apples") }
+    let!(:variant_a1) {
+      create(:variant,
+             product: product_a,
+             display_name: "Medium box",
+             sku: "APL-01",
+             price: 5.25)
+    }
+    let!(:product_b) { create(:simple_product, name: "Bananas", sku: "BAN-00") }
+    let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") }
 
     it "saves valid changes" do
       params = {
-        # '[products][<i>][name]'
+        # '[products][][name]'
         "products" => [
           {
             "id" => product_a.id.to_s,
             "name" => "Pommes",
+            "sku" => "POM-00",
           }
         ]
       }
 
       expect{
         run_reflex(:bulk_update, params:)
         product_a.reload
-      }.to change(product_a, :name).to("Pommes")
+      }.to change{ product_a.name }.to("Pommes")
+        .and change{ product_a.sku }.to("POM-00")
+    end
+
+    it "saves valid changes to products and nested variants" do
+      params = {
+        # '[products][][name]'
+        # '[products][][variants_attributes][][display_name]'
+        "products" => [
+          {
+            "id" => product_a.id.to_s,
+            "name" => "Pommes",
+            "variants_attributes" => [
+              {
+                "id" => variant_a1.id.to_s,
+                "display_name" => "Large box",
+                "sku" => "POM-01",
+                "price" => "10.25",
+              }
+            ],
+          }
+        ]
+      }
+
+      expect{
+        run_reflex(:bulk_update, params:)
+        product_a.reload
+        variant_a1.reload
+      }.to change{ product_a.name }.to("Pommes")
+        .and change{ variant_a1.display_name }.to("Large box")
+        .and change{ variant_a1.sku }.to("POM-01")
+        .and change{ variant_a1.price }.to(10.25)
     end
 
     describe "sorting" do

spec/system/admin/products_v3/products_spec.rb

@@ -158,21 +158,50 @@
   end
 
   describe "updating" do
+    let!(:variant_a1) {
+      create(:variant,
+             product: product_a,
+             display_name: "Medium box",
+             sku: "APL-01",
+             price: 5.25)
+    }
+    let!(:product_a) { create(:simple_product, name: "Apples", sku: "APL-00") }
+
     before do
       visit admin_products_v3_index_url
     end
 
-    it "can update product fields" do
-      fill_in id: "_product_name_#{product_1.id}", with: "An updated name"
+    it "can update product and variant fields" do
+      within row_containing_name("Apples") do
+        fill_in "Name", with: "Pommes"
+        fill_in "SKU", with: "POM-00"
+      end
+      within row_containing_name("Medium box") do
+        fill_in "Name", with: "Large box"
+        fill_in "SKU", with: "POM-01"
+        fill_in "Price", with: "10.25"
+      end
 
       expect {
         click_button "Save changes"
-        product_1.reload
-      }.to(
-        change { product_1.name }.to("An updated name")
-      )
+        product_a.reload
+        variant_a1.reload
+      }.to change { product_a.name }.to("Pommes")
+        .and change{ product_a.sku }.to("POM-00")
+        .and change{ variant_a1.display_name }.to("Large box")
+        .and change{ variant_a1.sku }.to("POM-01")
+        .and change{ variant_a1.price }.to(10.25)
+
+      within row_containing_name("Pommes") do
+        expect(page).to have_field "Name", with: "Pommes"
+        expect(page).to have_field "SKU", with: "POM-00"
+      end
+      within row_containing_name("Large box") do
+        expect(page).to have_field "Name", with: "Large box"
+        expect(page).to have_field "SKU", with: "POM-01"
+        expect(page).to have_field "Price", with: "10.25"
+      end
 
-      expect(page).to have_field id: "_product_name_#{product_1.id}", with: "An updated name"
       pending
       expect(page).to have_content "Changes saved"
     end
@@ -205,4 +234,11 @@ def search_by_category(category)
     select category, from: "category_id"
     click_button "Search"
   end
+
+  # Selector for table row that has an input with this value.
+  # Because there are no visible labels, the user has to assume which product it is, based on the
+  # visible name.
+  def row_containing_name(value)
+    "tr:has(input[aria-label=Name][value='#{value}'])"
+  end
 end