From 09869714739ec364e4f76f497fc03f42886834cc Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 12 Jul 2024 17:00:42 +0500 Subject: [PATCH 1/4] 12570 - fix variant's display_as field being empty issue - New variant unit_value is empty, so +VariantUnits::OptionValueNamer.new(variant).name+ returns "" - Now we are making sure that new variant unit_value won't be empty --- app/helpers/admin/products_helper.rb | 18 ++++++++++++++++++ .../products_v3/_product_variant_row.html.haml | 2 +- .../admin/products_v3/_variant_row.html.haml | 4 +--- spec/system/admin/products_v3/update_spec.rb | 1 - 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index e29b378342..20318ecc14 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -9,5 +9,23 @@ module Admin new_admin_product_image_path(product.id) end end + + def prepare_new_variant(product) + product.variants.build do |variant| + variant.unit_value = 1.0 * product.variant_unit_scale + variant.unit_presentation = VariantUnits::OptionValueNamer.new(variant).name + end + end + + def unit_value_with_description(variant) + scaled_unit_value = variant.unit_value / (variant.product.variant_unit_scale || 1) + precised_unit_value = number_with_precision( + scaled_unit_value, + precision: nil, + strip_insignificant_zeros: true + ) + + [precised_unit_value, variant.unit_description].compact_blank.join(" ") + end end end diff --git a/app/views/admin/products_v3/_product_variant_row.html.haml b/app/views/admin/products_v3/_product_variant_row.html.haml index 5c2765ab2d..b41e7c56b5 100644 --- a/app/views/admin/products_v3/_product_variant_row.html.haml +++ b/app/views/admin/products_v3/_product_variant_row.html.haml @@ -11,7 +11,7 @@ %tr.condensed{ id: dom_id(variant), 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': variant.new_record? ? "true" : false } = render partial: 'variant_row', locals: { variant:, f: variant_form, category_options:, tax_category_options:, producer_options: } - = form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", product.variants.build) do |new_variant_form| + = form.fields_for("products][#{product_index}][variants_attributes][NEW_RECORD", prepare_new_variant(product)) do |new_variant_form| %template{ 'data-nested-form-target': "template" } %tr.condensed{ 'data-controller': "variant", 'class': "nested-form-wrapper", 'data-new-record': "true" } = render partial: 'variant_row', locals: { variant: new_variant_form.object, f: new_variant_form, category_options:, tax_category_options:, producer_options: } diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index b137c25de0..a1244e1d47 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -17,10 +17,8 @@ -# Show a composite field for unit_value and unit_description = f.hidden_field :unit_value = f.hidden_field :unit_description - -# todo: create a method for value_with_description = f.text_field :unit_value_with_description, - value: [number_with_precision((variant.unit_value || 1) / (variant.product.variant_unit_scale || 1), precision: nil, strip_insignificant_zeros: true), variant.unit_description].compact_blank.join(" "), - 'aria-label': t('admin.products_page.columns.unit_value'), required: true + value: unit_value_with_description(variant), 'aria-label': t('admin.products_page.columns.unit_value'), required: true .field = f.label :display_as, t('admin.products_page.columns.display_as') = f.text_field :display_as, placeholder: VariantUnits::OptionValueNamer.new(variant).name diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index 9495838ff3..bce91c3429 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -517,7 +517,6 @@ RSpec.describe 'As an enterprise user, I can update my products', feature: :admi expect(page).to have_field "SKU", with: "n" * 256 expect(page).to have_content "is too long" expect(page.find_button("Unit")).to have_text "" # have_button selector don't work here - expect(page).to have_content "can't be blank" expect(page).to have_field "Price", with: "10.25" # other updated value is retained end From d8354298f5a5a30c34e10d16ba1d81b91d7a7b1a Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 12 Jul 2024 18:02:57 +0500 Subject: [PATCH 2/4] 12570 - add specs --- .../admin/products_v3/_variant_row.html.haml | 2 +- spec/system/admin/products_v3/create_spec.rb | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index a1244e1d47..b117e80579 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -21,7 +21,7 @@ value: unit_value_with_description(variant), 'aria-label': t('admin.products_page.columns.unit_value'), required: true .field = f.label :display_as, t('admin.products_page.columns.display_as') - = f.text_field :display_as, placeholder: VariantUnits::OptionValueNamer.new(variant).name + = f.text_field :display_as, 'aria-label': t('admin.products_page.columns.display_as'), placeholder: VariantUnits::OptionValueNamer.new(variant).name = error_message_on variant, :unit_value %td.col-price.field.naked_inputs = f.text_field :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 diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index 2a007a8f43..84edef1b5e 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -45,6 +45,18 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi expect(page).to have_content "New variant" end + it "has the 1 unit value for the new variant display_as by default" do + new_variant_button.click + + within new_variant_row do + unit_button = find('button[aria-label="Unit"]') + expect(unit_button.text.strip).to eq('1kg') + + unit_button.click + find('input[aria-label="Display unit as"][placeholder="1kg"]') + end + end + shared_examples "creating a new variant (bulk)" do |stock| it "handles the #{stock} behaviour" do # the product and the default variant is displayed @@ -120,4 +132,12 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi login_as_admin visit spree.admin_products_path end + + def new_variant_button + find("button.secondary.condensed.naked.icon-plus") + end + + def new_variant_row + 'tr[data-new-record="true"]' + end end From c52c2ebfe1c8815ec953c669894636ff937609cd Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Fri, 12 Jul 2024 18:20:19 +0500 Subject: [PATCH 3/4] 12570 - fix specs --- app/helpers/admin/products_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index 20318ecc14..d0d5611d80 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -12,7 +12,7 @@ module Admin def prepare_new_variant(product) product.variants.build do |variant| - variant.unit_value = 1.0 * product.variant_unit_scale + variant.unit_value = 1.0 * (product.variant_unit_scale || 1) variant.unit_presentation = VariantUnits::OptionValueNamer.new(variant).name end end From 5b7fbc875ab4175e7a0e5434ad97108e12e3c399 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Mon, 15 Jul 2024 22:48:20 +0500 Subject: [PATCH 4/4] 12570 - address PR comments --- app/views/admin/products_v3/_variant_row.html.haml | 2 +- spec/system/admin/products_v3/create_spec.rb | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index b117e80579..a1244e1d47 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -21,7 +21,7 @@ value: unit_value_with_description(variant), 'aria-label': t('admin.products_page.columns.unit_value'), required: true .field = f.label :display_as, t('admin.products_page.columns.display_as') - = f.text_field :display_as, 'aria-label': t('admin.products_page.columns.display_as'), placeholder: VariantUnits::OptionValueNamer.new(variant).name + = f.text_field :display_as, placeholder: VariantUnits::OptionValueNamer.new(variant).name = error_message_on variant, :unit_value %td.col-price.field.naked_inputs = f.text_field :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 diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index 84edef1b5e..97dd6e69e9 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -39,9 +39,9 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi before { visit_products_page_as_admin } it "hovering over the New variant button displays the text" do - page.find('button[aria-label="New variant"]', text: "New variant", visible: false) + new_variant_button find("button.secondary.condensed.naked.icon-plus").hover - page.find('button[aria-label="New variant"]', text: "New variant", visible: true) + new_variant_button(visible: true) expect(page).to have_content "New variant" end @@ -53,7 +53,7 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi expect(unit_button.text.strip).to eq('1kg') unit_button.click - find('input[aria-label="Display unit as"][placeholder="1kg"]') + expect(page).to have_field "Display unit as", placeholder: "1kg" end end @@ -133,8 +133,8 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi visit spree.admin_products_path end - def new_variant_button - find("button.secondary.condensed.naked.icon-plus") + def new_variant_button(visible: false) + page.find('button[aria-label="New variant"]', text: "New variant", visible:) end def new_variant_row