From 24afa218850bf9dbfe8917426438d4bacbee1210 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 22 Sep 2019 16:30:52 +0100 Subject: [PATCH 1/4] Revert "Change products controller to clear variants unit description if variant_unit is items" This reverts commit 1a4e83d63388409e9748656bca443528f6454beb. --- .../spree/admin/products_controller_decorator.rb | 8 -------- .../spree/admin/products_controller_spec.rb | 13 ------------- 2 files changed, 21 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index e05ba14792..1554ab08a0 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -44,8 +44,6 @@ Spree::Admin::ProductsController.class_eval do delete_stock_params_and_set_after do super end - - clear_variants_unit_description if @object.variant_unit == 'items' end def bulk_update @@ -190,10 +188,4 @@ Spree::Admin::ProductsController.class_eval do def set_product_master_variant_price_to_zero @product.price = 0 if @product.price.nil? end - - def clear_variants_unit_description - @object.variants.each do |variant| - variant.update_attribute :unit_description, '' - end - end end diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index 0466ec6564..7a73954f42 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -194,19 +194,6 @@ describe Spree::Admin::ProductsController, type: :controller do end end - describe "product variant unit is items" do - it "clears unit description of all variants of the product" do - product.variants.first.update_attribute :unit_description, "grams" - spree_put :update, - id: product, - product: { - variant_unit: "items", - variant_unit_name: "bag" - } - expect(product.reload.variants.first.unit_description).to be_empty - end - end - describe "product properties" do context "as an enterprise user" do let!(:property) { create(:property, name: "A nice name") } From 591efecde6f5b1456c30ef4998ee42c5b6deae11 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 22 Sep 2019 16:42:03 +0100 Subject: [PATCH 2/4] Make unit description field visible in the variant edit page even for products which variant_unit is items --- app/views/spree/admin/variants/_form.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/spree/admin/variants/_form.html.haml b/app/views/spree/admin/variants/_form.html.haml index 00c4b01bfe..9213a1e31b 100644 --- a/app/views/spree/admin/variants/_form.html.haml +++ b/app/views/spree/admin/variants/_form.html.haml @@ -14,9 +14,9 @@ = text_field_tag :unit_value_human, nil, {class: "fullwidth", 'ng-model' => 'unit_value_human', 'ng-change' => 'updateValue()'} = f.text_field :unit_value, {hidden: true, 'ng-value' => 'unit_value'} - .field - = f.label :unit_description, t(:spree_admin_unit_description) - = f.text_field :unit_description, class: "fullwidth", placeholder: t('admin.products.unit_name_placeholder') + .field + = f.label :unit_description, t(:spree_admin_unit_description) + = f.text_field :unit_description, class: "fullwidth", placeholder: t('admin.products.unit_name_placeholder') %div - @product.option_types.each do |option_type| From f32454b404f3ae5f299d54873d733ce0ad6314a8 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 22 Sep 2019 21:02:32 +0100 Subject: [PATCH 3/4] Add feature spec to validate unit_description is editable for products with unit items, regression for #3649 --- spec/features/admin/variants_spec.rb | 69 ++++++++++++++++++---------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb index 56f9cb8430..731afc7ab4 100644 --- a/spec/features/admin/variants_spec.rb +++ b/spec/features/admin/variants_spec.rb @@ -24,37 +24,56 @@ feature ' expect(page).to have_content "Variant \"#{p.name}\" has been successfully created!" end - scenario "editing unit value and description for a variant", js: true do - # Given a product with unit-related option types, with a variant - p = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") - v = p.variants.first - v.update_attributes( unit_value: 1, unit_description: 'foo' ) + describe "editing unit value and description for a variant", js: true do + scenario "when variant_unit is weight" do + # Given a product with unit-related option types, with a variant + p = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") + v = p.variants.first + v.update_attributes( unit_value: 1, unit_description: 'foo' ) - # And the product has option types for the unit-related and non-unit-related option values - p.option_types << v.option_values.first.option_type + # And the product has option types for the unit-related and non-unit-related option values + p.option_types << v.option_values.first.option_type - # When I view the variant - login_to_admin_section - visit spree.admin_product_variants_path p - page.find('table.index .icon-edit').click + # When I view the variant + login_to_admin_section + visit spree.admin_product_variants_path p + page.find('table.index .icon-edit').click - # Then I should not see a traditional option value field for the unit-related option value - expect(page).to have_no_selector "div[data-hook='presentation'] input" + # Then I should not see a traditional option value field for the unit-related option value + expect(page).to have_no_selector "div[data-hook='presentation'] input" - # And I should see unit value and description fields for the unit-related option value - expect(page).to have_field "unit_value_human", with: "1" - expect(page).to have_field "variant_unit_description", with: "foo" + # And I should see unit value and description fields for the unit-related option value + expect(page).to have_field "unit_value_human", with: "1" + expect(page).to have_field "variant_unit_description", with: "foo" - # When I update the fields and save the variant - fill_in "unit_value_human", with: "123" - fill_in "variant_unit_description", with: "bar" - click_button 'Update' - expect(page).to have_content %(Variant "#{p.name}" has been successfully updated!) + # When I update the fields and save the variant + fill_in "unit_value_human", with: "123" + fill_in "variant_unit_description", with: "bar" + click_button 'Update' + expect(page).to have_content %(Variant "#{p.name}" has been successfully updated!) - # Then the unit value and description should have been saved - v.reload - expect(v.unit_value).to eq(123) - expect(v.unit_description).to eq('bar') + # Then the unit value and description should have been saved + v.reload + expect(v.unit_value).to eq(123) + expect(v.unit_description).to eq('bar') + end + + scenario "can update unit_description when variant_unit is items" do + product = create(:simple_product, variant_unit: "items", variant_unit_name: "bunches") + variant = product.variants.first + variant.update_attributes(unit_description: 'foo') + + login_to_admin_section + visit spree.edit_admin_product_variant_path(product, variant) + + expect(page).to_not have_field "unit_value_human" + expect(page).to have_field "variant_unit_description", with: "foo" + + fill_in "variant_unit_description", with: "bar" + click_button 'Update' + expect(page).to have_content %(Variant "#{product.name}" has been successfully updated!) + expect(variant.reload.unit_description).to eq('bar') + end end describe "editing on hand and on demand values", js: true do From eb85dccac1642b360bc942f344e63cd8a4cbc649 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sun, 22 Sep 2019 21:07:32 +0100 Subject: [PATCH 4/4] Remove single letter variable names --- spec/features/admin/variants_spec.rb | 50 +++++++++++++--------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/spec/features/admin/variants_spec.rb b/spec/features/admin/variants_spec.rb index 731afc7ab4..450fb96621 100644 --- a/spec/features/admin/variants_spec.rb +++ b/spec/features/admin/variants_spec.rb @@ -9,11 +9,11 @@ feature ' scenario "creating a new variant" do # Given a product with a unit-related option type - p = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") + product = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") # When I create a variant on the product login_to_admin_section - visit spree.admin_product_variants_path p + visit spree.admin_product_variants_path product click_link 'New Variant' fill_in 'unit_value_human', with: '1' @@ -21,22 +21,22 @@ feature ' click_button 'Create' # Then the variant should have been created - expect(page).to have_content "Variant \"#{p.name}\" has been successfully created!" + expect(page).to have_content "Variant \"#{product.name}\" has been successfully created!" end describe "editing unit value and description for a variant", js: true do scenario "when variant_unit is weight" do # Given a product with unit-related option types, with a variant - p = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") - v = p.variants.first - v.update_attributes( unit_value: 1, unit_description: 'foo' ) + product = create(:simple_product, variant_unit: "weight", variant_unit_scale: "1") + variant = product.variants.first + variant.update_attributes( unit_value: 1, unit_description: 'foo' ) # And the product has option types for the unit-related and non-unit-related option values - p.option_types << v.option_values.first.option_type + product.option_types << variant.option_values.first.option_type # When I view the variant login_to_admin_section - visit spree.admin_product_variants_path p + visit spree.admin_product_variants_path product page.find('table.index .icon-edit').click # Then I should not see a traditional option value field for the unit-related option value @@ -50,12 +50,11 @@ feature ' fill_in "unit_value_human", with: "123" fill_in "variant_unit_description", with: "bar" click_button 'Update' - expect(page).to have_content %(Variant "#{p.name}" has been successfully updated!) + expect(page).to have_content %(Variant "#{product.name}" has been successfully updated!) # Then the unit value and description should have been saved - v.reload - expect(v.unit_value).to eq(123) - expect(v.unit_description).to eq('bar') + expect(variant.reload.unit_value).to eq(123) + expect(variant.unit_description).to eq('bar') end scenario "can update unit_description when variant_unit is items" do @@ -118,31 +117,29 @@ feature ' end it "soft-deletes variants", js: true do - p = create(:simple_product) - v = create(:variant, product: p) + product = create(:simple_product) + variant = create(:variant, product: product) login_to_admin_section - visit spree.admin_product_variants_path p + visit spree.admin_product_variants_path product - within "tr#spree_variant_#{v.id}" do + within "tr#spree_variant_#{variant.id}" do accept_alert do page.find('a.delete-resource').click end end - expect(page).not_to have_selector "tr#spree_variant_#{v.id}" - - v.reload - expect(v.deleted_at).not_to be_nil + expect(page).not_to have_selector "tr#spree_variant_#{variant.id}" + expect(variant.reload.deleted_at).not_to be_nil end scenario "editing display name for a variant", js: true do - p = create(:simple_product) - v = p.variants.first + product = create(:simple_product) + variant = product.variants.first # When I view the variant login_to_admin_section - visit spree.admin_product_variants_path p + visit spree.admin_product_variants_path product page.find('table.index .icon-edit').click # It should allow the display name to be changed @@ -153,11 +150,10 @@ feature ' fill_in "variant_display_name", with: "Display Name" fill_in "variant_display_as", with: "Display As This" click_button 'Update' - expect(page).to have_content %(Variant "#{p.name}" has been successfully updated!) + expect(page).to have_content %(Variant "#{product.name}" has been successfully updated!) # Then the displayed values should have been saved - v.reload - expect(v.display_name).to eq("Display Name") - expect(v.display_as).to eq("Display As This") + expect(variant.reload.display_name).to eq("Display Name") + expect(variant.display_as).to eq("Display As This") end end