From e808c7fb2bbde2c3e927971ae45298e5919354f3 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Thu, 25 Jul 2024 23:42:03 +0500 Subject: [PATCH 1/4] 12705 - set 1 as default for variant's unit_value --- app/helpers/admin/products_helper.rb | 2 +- spec/helpers/admin/products_helper_spec.rb | 48 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 spec/helpers/admin/products_helper_spec.rb diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index d0d5611d80..74fe24a0b1 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -18,7 +18,7 @@ module Admin end def unit_value_with_description(variant) - scaled_unit_value = variant.unit_value / (variant.product.variant_unit_scale || 1) + scaled_unit_value = (variant.unit_value || 1) / (variant.product.variant_unit_scale || 1) precised_unit_value = number_with_precision( scaled_unit_value, precision: nil, diff --git a/spec/helpers/admin/products_helper_spec.rb b/spec/helpers/admin/products_helper_spec.rb new file mode 100644 index 0000000000..8a06f12c4b --- /dev/null +++ b/spec/helpers/admin/products_helper_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Admin::ProductsHelper do + describe '#unit_value_with_description' do + let(:product) { create(:product, variant_unit_scale: 1000.0) } + let(:variant) { create(:variant, product:, unit_value: 2000.0, unit_description: 'kg') } + + context 'when unit_value and unit_description are present' do + it 'returns the scaled unit value with the description' do + expect(helper.unit_value_with_description(variant)).to eq('2 kg') + end + end + + context 'when unit_value is nil' do + before { variant.update_column(:unit_value, nil) } + + it 'defaults to 1 and returns the scaled unit value with the description' do + expect(helper.unit_value_with_description(variant)).to eq('0.001 kg') + end + end + + context 'when unit_description is nil' do + before { variant.update_column(:unit_description, nil) } + + it 'returns only the scaled unit value' do + expect(helper.unit_value_with_description(variant)).to eq('2') + end + end + + context 'when variant_unit_scale is nil' do + before { product.update_column(:variant_unit_scale, nil) } + + it 'uses default scale of 1 and returns the unscaled unit value with the description' do + expect(helper.unit_value_with_description(variant)).to eq('2000 kg') + end + end + + context 'when both unit_value and unit_description are nil' do + before { variant.update_columns(unit_description: nil, unit_value: nil) } + + it 'returns the default unit value without description' do + expect(helper.unit_value_with_description(variant)).to eq('0.001') + end + end + end +end From 1014a50aff403cc83347bd12aa9b0ee7bda73590 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 4 Aug 2024 17:47:02 +0500 Subject: [PATCH 2/4] 12705 - incorporate old UI behavior - if unit_value is not present and unit_description then display unit_description only. - if both are not present then display empty fields --- app/helpers/admin/products_helper.rb | 21 ++++++++++--------- .../admin/products_v3/_variant_row.html.haml | 2 +- spec/helpers/admin/products_helper_spec.rb | 8 +++---- spec/system/admin/products_v3/update_spec.rb | 1 + 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/app/helpers/admin/products_helper.rb b/app/helpers/admin/products_helper.rb index 74fe24a0b1..5c13c4a0d3 100644 --- a/app/helpers/admin/products_helper.rb +++ b/app/helpers/admin/products_helper.rb @@ -11,19 +11,20 @@ module Admin end def prepare_new_variant(product) - product.variants.build do |variant| - variant.unit_value = 1.0 * (product.variant_unit_scale || 1) - variant.unit_presentation = VariantUnits::OptionValueNamer.new(variant).name - end + product.variants.build end def unit_value_with_description(variant) - scaled_unit_value = (variant.unit_value || 1) / (variant.product.variant_unit_scale || 1) - precised_unit_value = number_with_precision( - scaled_unit_value, - precision: nil, - strip_insignificant_zeros: true - ) + precised_unit_value = nil + + if variant.unit_value + 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 + ) + end [precised_unit_value, variant.unit_description].compact_blank.join(" ") end diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml index a1244e1d47..ad29b77108 100644 --- a/app/views/admin/products_v3/_variant_row.html.haml +++ b/app/views/admin/products_v3/_variant_row.html.haml @@ -18,7 +18,7 @@ = f.hidden_field :unit_value = f.hidden_field :unit_description = f.text_field :unit_value_with_description, - value: unit_value_with_description(variant), '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') .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/helpers/admin/products_helper_spec.rb b/spec/helpers/admin/products_helper_spec.rb index 8a06f12c4b..cdd4df9e2a 100644 --- a/spec/helpers/admin/products_helper_spec.rb +++ b/spec/helpers/admin/products_helper_spec.rb @@ -16,8 +16,8 @@ RSpec.describe Admin::ProductsHelper do context 'when unit_value is nil' do before { variant.update_column(:unit_value, nil) } - it 'defaults to 1 and returns the scaled unit value with the description' do - expect(helper.unit_value_with_description(variant)).to eq('0.001 kg') + it 'returns the description' do + expect(helper.unit_value_with_description(variant)).to eq('kg') end end @@ -40,8 +40,8 @@ RSpec.describe Admin::ProductsHelper do context 'when both unit_value and unit_description are nil' do before { variant.update_columns(unit_description: nil, unit_value: nil) } - it 'returns the default unit value without description' do - expect(helper.unit_value_with_description(variant)).to eq('0.001') + it 'returns empty string' do + expect(helper.unit_value_with_description(variant)).to eq('') end end end diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index 0b184f4353..ea0bfcb6bf 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -517,6 +517,7 @@ RSpec.describe 'As an enterprise user, I can update my products' do 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 d4e0b2ab51d36bd71b5dce4800221efecb679468 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 4 Aug 2024 18:20:15 +0500 Subject: [PATCH 3/4] 12705 - fix specs --- spec/system/admin/products_v3/create_spec.rb | 6 +++--- spec/system/admin/products_v3/update_spec.rb | 3 --- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/spec/system/admin/products_v3/create_spec.rb b/spec/system/admin/products_v3/create_spec.rb index e30dfba18c..43e0370ed4 100644 --- a/spec/system/admin/products_v3/create_spec.rb +++ b/spec/system/admin/products_v3/create_spec.rb @@ -45,15 +45,15 @@ RSpec.describe 'As an enterprise user, I can manage my products' do expect(page).to have_content "New variant" end - it "has the 1 unit value for the new variant display_as by default" do + it "has the empty 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') + expect(unit_button.text.strip).to eq('') unit_button.click - expect(page).to have_field "Display unit as", placeholder: "1kg" + expect(page).to have_field "Display unit as", placeholder: "" end end diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index ea0bfcb6bf..76b18aec69 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -58,9 +58,6 @@ RSpec.describe 'As an enterprise user, I can update my products' do end # Unit popout - fill_in "Unit value", with: "" - click_button "Save changes" # attempt to save or close the popout - expect(page).to have_field "Unit value", with: "" # popout is still open fill_in "Unit value", with: "500.1" within row_containing_name("Medium box") do From 647a384561138ce831ead2ac9ce62165d091322b Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Wed, 7 Aug 2024 16:59:10 +0500 Subject: [PATCH 4/4] 12705 - add specs for updating invalid unit_value --- spec/system/admin/products_v3/update_spec.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/system/admin/products_v3/update_spec.rb b/spec/system/admin/products_v3/update_spec.rb index 76b18aec69..d2eccfc238 100644 --- a/spec/system/admin/products_v3/update_spec.rb +++ b/spec/system/admin/products_v3/update_spec.rb @@ -50,17 +50,20 @@ RSpec.describe 'As an enterprise user, I can update my products' do fill_in "SKU", with: "POM-00" tomselect_select "Volume (mL)", from: "Unit scale" end - within row_containing_name("Medium box") do - fill_in "Name", with: "Large box" - fill_in "SKU", with: "POM-01" - - click_on "Unit" # activate popout - end # Unit popout + click_on "Unit" # activate popout + # have to use below method to trigger the +change+ event, + # +fill_in "Unit value", with: ""+ does not trigger +change+ event + find_field('Unit value').send_keys(:control, 'a', :backspace) # empty the field + click_button "Save changes" # attempt to save and should fail with below error + expect(page).to have_content "must be greater than 0" + click_on "Unit" # activate popout fill_in "Unit value", with: "500.1" 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" click_on "On Hand" # activate popout