Merge pull request #12991 from murjax/remove-sku-11973

Remove product SKU from product pages and report

app/assets/javascripts/admin/bulk_product_update.js.coffee

@@ -307,9 +307,6 @@ filterSubmitProducts = (productsToFilter) ->
             variantHasUpdatableProperty = result.hasUpdatableProperty
             filteredVariants.push filteredVariant  if variantHasUpdatableProperty
 
-        if product.hasOwnProperty("sku")
-          filteredProduct.sku = product.sku
-          hasUpdatableProperty = true
         if product.hasOwnProperty("name")
           filteredProduct.name = product.name
           hasUpdatableProperty = true

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

@@ -5,8 +5,6 @@
   = f.text_field :name, 'aria-label': t('admin.products_page.columns.name')
   = error_message_on product, :name
 %td.col-sku.field.naked_inputs
-  = f.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
-  = error_message_on product, :sku
 %td.col-unit_scale.align-right
   -# empty
 %td.col-unit.align-right

lib/reporting/reports/orders_and_distributors/base.rb

@@ -13,7 +13,7 @@ def columns
             customer_email: proc { |line_item| line_item.order.email },
             customer_phone: proc { |line_item| line_item.order.bill_address.phone },
             customer_city: proc { |line_item| line_item.order.bill_address.city },
-            sku: proc { |line_item| line_item.product.sku },
+            sku: proc { |line_item| line_item.variant.sku },
             product: proc { |line_item| line_item.product.name },
             variant: proc { |line_item| line_item.full_name },
             quantity: proc { |line_item| line_item.quantity },

spec/lib/reports/orders_and_distributors_report_spec.rb

@@ -59,7 +59,7 @@
                                    order.email,
                                    bill_address.phone,
                                    bill_address.city,
-                                   line_item.product.sku,
+                                   line_item.variant.sku,
                                    line_item.product.name,
                                    "1g",
                                    line_item.quantity,

spec/system/admin/products_v3/update_spec.rb

@@ -47,7 +47,6 @@
     it "updates 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
@@ -83,7 +82,6 @@
         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.unit_value }.to(0.5001) # volumes are stored in litres
@@ -94,7 +92,6 @@
 
       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"
@@ -230,8 +227,8 @@
         expect(page).to have_field "Name", with: "Pommes" # Changed value wasn't lost
       end
 
-      # Meanwhile, the SKU was updated
-      product_a.update! sku: "APL-10"
+      # Meanwhile, the price was updated
+      variant_a1.update!(price: 10.25)
 
       expect {
         accept_confirm do
@@ -242,19 +239,22 @@
 
       within row_containing_name("Apples") do
         expect(page).to have_field "Name", with: "Apples" # Changed value wasn't saved
-        expect(page).to have_field "SKU", with: "APL-10" # Updated value shown
+      end
+
+      within row_containing_name("Medium box") do
+        expect(page).to have_field "Price", with: "10.25" # Updated value shown
       end
     end
 
     context "with invalid data" do
       let!(:product_b) { create(:simple_product, name: "Bananas") }
+      let(:invalid_product_name) { "A" * 256 }
 
       before do
         visit admin_products_url
 
         within row_containing_name("Apples") do
-          fill_in "Name", with: ""
-          fill_in "SKU", with: "A" * 256
+          fill_in "Name", with: invalid_product_name
         end
       end
 
@@ -280,10 +280,8 @@
         }.not_to change { product_a.name }
 
         # (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
+        within row_containing_name(invalid_product_name) do
+          expect(page).to have_field "Name", with: invalid_product_name
           expect(page).to have_content "is too long"
         end
 
@@ -304,9 +302,8 @@
           product_a.reload
         }.not_to change { product_a.name }
 
-        within row_containing_name("") do
+        within row_containing_name(invalid_product_name) do
           fill_in "Name", with: "Pommes"
-          fill_in "SKU", with: "POM-00"
         end
 
         expect {
@@ -316,7 +313,6 @@
           product_a.reload
           variant_a1.reload
         }.to change { product_a.name }.to("Pommes")
-          .and change{ product_a.sku }.to("POM-00")
       end
     end
 
@@ -638,7 +634,6 @@
 
         within row_containing_name("Apples") do
           fill_in "Name", with: ""
-          fill_in "SKU", with: "A" * 256
         end
       end
 

spec/system/admin/reports/orders_and_distributors_spec.rb

@@ -23,6 +23,7 @@
     let!(:order2) {
       create(:order_ready_to_ship, distributor_id: distributor2.id, completed_at:)
     }
+    let(:variant) { order.variants.first }
 
     context "as an enterprise user" do
       let(:header) {
@@ -34,27 +35,27 @@
       }
       let(:line_item1) {
         [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon",
-         "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
+         variant.sku, Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
          "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ")
       }
       let(:line_item2) {
         [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon",
-         "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
+         variant.sku, Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
          "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ")
       }
       let(:line_item3) {
         [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon",
-         "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
+         variant.sku, Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
          "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ")
       }
       let(:line_item4) {
         [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon",
-         "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
+         variant.sku, Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
          "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ")
       }
       let(:line_item5) {
         [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon",
-         "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
+         variant.sku, Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check",
          "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ")
       }