From 504ee50dd41218d9b4a9b07d553e5cb8ffe5730b Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Tue, 20 Jun 2023 14:03:00 +0100 Subject: [PATCH 1/6] Sets up a spec for #11085 --- spec/system/admin/variants_spec.rb | 77 +++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index d843f4c4ae..3489178474 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -179,6 +179,81 @@ describe ' expect(page).to have_content %(Variant "#{product.name}" has been successfully updated!) expect(variant.reload.unit_description).to eq('bar') end + + context "with ES as a locale" do + let(:product) { create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") } + let(:variant) { product.variants.first } + + before do + # sets the locale into ES + I18n.default_locale = 'es' + + variant.update( unit_value: 1, unit_description: 'foo' ) + + # When I view the variant + login_as_admin + visit spree.admin_product_variants_path product + end + + context "with localization disabled" do + before do + allow(Spree::Config).to receive(:enable_localized_number?).and_return false + + Spree::Config[:currency_decimal_mark] = "." + Spree::Config[:currency_thousands_separator] = "," + end + + it "when variant_unit is weight" do + expect(Spree::Price.second.amount).to eq(19.99) + + # Given a product with unit-related option types, with a variant + page.find('table.index .icon-edit').click + + # assert on the price field + expect(page).to have_field "variant_price", with: "19,99 " + + # When I update the fields and save the variant + fill_in "variant_price", with: "12,50 " + + click_button 'Actualizar' + expect(page).to have_content %(Variant "#{product.name}" ha sido actualizado exitosamente) + + # Then the variant price should have been updated + expect(Spree::Price.second.amount).to eq(12.50) + end + end + + context "with localization enabled" do + before do + allow(Spree::Config).to receive(:enable_localized_number?).and_return true + + Spree::Config[:currency_decimal_mark] = "," + Spree::Config[:currency_thousands_separator] = "." + + login_as_admin + visit spree.admin_product_variants_path product + end + + it "when variant_unit is weight" do + pending("#11085") + expect(Spree::Price.second.amount).to eq(19.99) + + # Given a product with unit-related option types, with a variant + page.find('table.index .icon-edit').click + + # assert on the Price field + expect(page).to have_field "variant_price", with: "19,99 " + + # When I update the fields and save the variant + fill_in "variant_price", with: "12,50 " + click_button 'Actualizar' + expect(page).to have_content %(Variant "#{product.name}" ha sido actualizado exitosamente) + + # Then the variant price should have been updated + expect(Spree::Price.second.amount).to eq(12.50) + end + end + end end describe "editing on hand and on demand values" do @@ -224,7 +299,7 @@ describe ' it "soft-deletes variants" do product = create(:simple_product) - variant = create(:variant, product: product) + variant = create(:variant, product:) login_as_admin visit spree.admin_product_variants_path product From 6b84fbf2b8a536ea633f2ee814b3398de89fbd72 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Tue, 20 Jun 2023 16:02:35 +0100 Subject: [PATCH 2/6] Adds after block, to remove configurations Sets pending shared examples --- spec/system/admin/variants_spec.rb | 89 +++++++++++++----------------- 1 file changed, 37 insertions(+), 52 deletions(-) diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index 3489178474..0a2d7fd39b 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -90,8 +90,6 @@ describe ' login_as_admin visit spree.admin_product_variants_path(product, filter) - visit spree.admin_product_variants_path(product, filter) - expected_new_url = Regexp.new( Regexp.escape(spree.new_admin_product_variant_path(product, filter)) ) @@ -195,63 +193,50 @@ describe ' visit spree.admin_product_variants_path product end - context "with localization disabled" do - before do - allow(Spree::Config).to receive(:enable_localized_number?).and_return false + shared_examples "with localization" do |localized, decimal_mark, thousands_separator| + context "set to #{localized}" do + before do + allow(Spree::Config).to receive(:enable_localized_number?).and_return localized + Spree::Config[:currency_decimal_mark] = decimal_mark + Spree::Config[:currency_thousands_separator] = thousands_separator + end - Spree::Config[:currency_decimal_mark] = "." - Spree::Config[:currency_thousands_separator] = "," + it "when variant_unit is weight" do + expect(Spree::Price.second.amount).to eq(19.99) + + # Given a product with unit-related option types, with a variant + page.find('table.index .icon-edit').click + + # assert on the price field + expect(page).to have_field "variant_price", with: "19,99 " + + # When I update the fields and save the variant + fill_in "variant_price", with: "12,50 " + click_button 'Actualizar' + expect(page).to have_content \ + %(Variant "#{product.name}" ha sido actualizado exitosamente) + + # Then the variant price should have been updated + expect(Spree::Price.second.amount).to eq(12.50) + end end - it "when variant_unit is weight" do - expect(Spree::Price.second.amount).to eq(19.99) + after do + # sets the locale back to EN + I18n.default_locale = 'en' - # Given a product with unit-related option types, with a variant - page.find('table.index .icon-edit').click - - # assert on the price field - expect(page).to have_field "variant_price", with: "19,99 " - - # When I update the fields and save the variant - fill_in "variant_price", with: "12,50 " - - click_button 'Actualizar' - expect(page).to have_content %(Variant "#{product.name}" ha sido actualizado exitosamente) - - # Then the variant price should have been updated - expect(Spree::Price.second.amount).to eq(12.50) + # disables localization to prevent leaking between specs + allow(Spree::Config).to receive(:enable_localized_number?).and_return false end end - context "with localization enabled" do - before do - allow(Spree::Config).to receive(:enable_localized_number?).and_return true - - Spree::Config[:currency_decimal_mark] = "," - Spree::Config[:currency_thousands_separator] = "." - - login_as_admin - visit spree.admin_product_variants_path product - end - - it "when variant_unit is weight" do - pending("#11085") - expect(Spree::Price.second.amount).to eq(19.99) - - # Given a product with unit-related option types, with a variant - page.find('table.index .icon-edit').click - - # assert on the Price field - expect(page).to have_field "variant_price", with: "19,99 " - - # When I update the fields and save the variant - fill_in "variant_price", with: "12,50 " - click_button 'Actualizar' - expect(page).to have_content %(Variant "#{product.name}" ha sido actualizado exitosamente) - - # Then the variant price should have been updated - expect(Spree::Price.second.amount).to eq(12.50) - end + it_behaves_like "with localization", false, ".", "," + it_behaves_like "with localization", true, ".", "," do + before { pending("#11085") } + end + it_behaves_like "with localization", false, ",", "." + it_behaves_like "with localization", true, ",", "." do + before { pending("#11085") } end end end From 2fcf706aa314fe2277e95220203e4d11bea71c42 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 22 Jun 2023 15:12:28 +0200 Subject: [PATCH 3/6] Remove unwanted space at the end of the price This `number_to_currency` method seems to display an extra space, not necessary when unit is `''` Strip it. Update specs as well. Thanks @filipefurtad0 for specs!!! --- app/views/spree/admin/variants/_form.html.haml | 4 ++-- spec/system/admin/variants_spec.rb | 12 ++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index 35a41f666d..c9cb54e573 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/app/views/spree/admin/variants/_form.html.haml @@ -23,7 +23,7 @@ = f.text_field :sku, class: 'fullwidth' .field = f.label :price, t('.price') - = f.text_field :price, class: 'fullwidth', "ng-model" => "variant.price", "ng-init" => "variant.price = '#{number_to_currency(@variant.price, unit: '')}'" + = f.text_field :price, class: 'fullwidth', "ng-model" => "variant.price", "ng-init" => "variant.price = '#{number_to_currency(@variant.price, unit: '').strip}'" .field = hidden_field_tag 'product_variant_unit', @product.variant_unit = hidden_field_tag 'product_variant_unit_name', @product.variant_unit_name @@ -64,4 +64,4 @@ - value = number_with_precision(@variant.send(field), precision: 2) = f.number_field field, value: value, class: 'fullwidth', step: 0.01 -.clear \ No newline at end of file +.clear diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index 0a2d7fd39b..2f7c165c2e 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -208,10 +208,10 @@ describe ' page.find('table.index .icon-edit').click # assert on the price field - expect(page).to have_field "variant_price", with: "19,99 " + expect(page).to have_field "variant_price", with: "19,99" # When I update the fields and save the variant - fill_in "variant_price", with: "12,50 " + fill_in "variant_price", with: "12,50" click_button 'Actualizar' expect(page).to have_content \ %(Variant "#{product.name}" ha sido actualizado exitosamente) @@ -231,13 +231,9 @@ describe ' end it_behaves_like "with localization", false, ".", "," - it_behaves_like "with localization", true, ".", "," do - before { pending("#11085") } - end + it_behaves_like "with localization", true, ".", "," it_behaves_like "with localization", false, ",", "." - it_behaves_like "with localization", true, ",", "." do - before { pending("#11085") } - end + it_behaves_like "with localization", true, ",", "." end end From 8b3f45263c8541559bc91d27a7cac86a663dd230 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 29 Jun 2023 10:35:01 +1000 Subject: [PATCH 4/6] Remove unnecessary spec clean-up And clarify locale setup. --- spec/system/admin/variants_spec.rb | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index 2f7c165c2e..a15917696f 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -182,10 +182,13 @@ describe ' let(:product) { create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") } let(:variant) { product.variants.first } - before do - # sets the locale into ES - I18n.default_locale = 'es' + around do |example| + I18n.default_locale = :es + example.run + I18n.default_locale = :en + end + before do variant.update( unit_value: 1, unit_description: 'foo' ) # When I view the variant @@ -220,14 +223,6 @@ describe ' expect(Spree::Price.second.amount).to eq(12.50) end end - - after do - # sets the locale back to EN - I18n.default_locale = 'en' - - # disables localization to prevent leaking between specs - allow(Spree::Config).to receive(:enable_localized_number?).and_return false - end end it_behaves_like "with localization", false, ".", "," From 3286094804c450be562c4fa319509af50d2e63b6 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Jun 2023 10:47:00 +0200 Subject: [PATCH 5/6] `number_to_currency` can be nil: use a safe operator --- app/views/spree/admin/variants/_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index c9cb54e573..4fcf2d2449 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/app/views/spree/admin/variants/_form.html.haml @@ -23,7 +23,7 @@ = f.text_field :sku, class: 'fullwidth' .field = f.label :price, t('.price') - = f.text_field :price, class: 'fullwidth', "ng-model" => "variant.price", "ng-init" => "variant.price = '#{number_to_currency(@variant.price, unit: '').strip}'" + = f.text_field :price, class: 'fullwidth', "ng-model" => "variant.price", "ng-init" => "variant.price = '#{number_to_currency(@variant.price, unit: '')&.strip}'" .field = hidden_field_tag 'product_variant_unit', @product.variant_unit = hidden_field_tag 'product_variant_unit_name', @product.variant_unit_name From 045435e9ccd1b3b5b9f93031a24f98689b7d52f1 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Jun 2023 11:02:56 +0200 Subject: [PATCH 6/6] use the variant price itself --- spec/system/admin/variants_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/system/admin/variants_spec.rb b/spec/system/admin/variants_spec.rb index a15917696f..f95bc91d95 100644 --- a/spec/system/admin/variants_spec.rb +++ b/spec/system/admin/variants_spec.rb @@ -205,7 +205,7 @@ describe ' end it "when variant_unit is weight" do - expect(Spree::Price.second.amount).to eq(19.99) + expect(variant.price).to eq(19.99) # Given a product with unit-related option types, with a variant page.find('table.index .icon-edit').click @@ -220,7 +220,7 @@ describe ' %(Variant "#{product.name}" ha sido actualizado exitosamente) # Then the variant price should have been updated - expect(Spree::Price.second.amount).to eq(12.50) + expect(variant.reload.price).to eq(12.50) end end end