Merge pull request #4298 from luisramos0/variant_edit

Make unit description visible in the variant edit page even for products which variant_unit is items

app/controllers/spree/admin/products_controller_decorator.rb

@@ -49,8 +49,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
@@ -195,10 +193,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

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|

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") }

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,40 +21,58 @@ 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
 
-  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
+      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
+      # And the product has option types for the unit-related and non-unit-related option values
+      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
-    page.find('table.index .icon-edit').click
+      # When I view the variant
+      login_to_admin_section
+      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
-    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 "#{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')
+      # Then the unit value and description should have been saved
+      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
+      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
@@ -99,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
@@ -134,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