BPE: Add a variant. Do not show edit on unsaved variants. Delete unsaved variants.

app/assets/javascripts/admin/bulk_product_update.js.coffee

@@ -285,6 +285,17 @@ productEditModule.controller "AdminProductEditCtrl", [
         window.location = "/admin/products/" + product.permalink_live + ((if variant then "/variants/" + variant.id else "")) + "/edit"
 
 
+    $scope.addVariant = (product) ->
+      product.variants.push {id: $scope.nextVariantId()}
+      $scope.displayProperties[product.id].showVariants = true
+
+
+    $scope.nextVariantId = ->
+      $scope.variantIdCounter = 0 unless $scope.variantIdCounter?
+      $scope.variantIdCounter -= 1
+      $scope.variantIdCounter
+
+
     $scope.deleteProduct = (product) ->
       if confirm("Are you sure?")
         $http(
@@ -297,14 +308,20 @@ productEditModule.controller "AdminProductEditCtrl", [
 
 
     $scope.deleteVariant = (product, variant) ->
-      if confirm("Are you sure?")
-        $http(
-          method: "DELETE"
-          url: "/api/products/" + product.id + "/variants/" + variant.id
-        ).success (data) ->
-          product.variants.splice product.variants.indexOf(variant), 1
-          delete $scope.dirtyProducts[product.id].variants[variant.id]  if $scope.dirtyProducts.hasOwnProperty(product.id) and $scope.dirtyProducts[product.id].hasOwnProperty("variants") and $scope.dirtyProducts[product.id].variants.hasOwnProperty(variant.id)
-          $scope.displayDirtyProducts()
+      if !$scope.variantSaved(variant)
+        $scope.removeVariant(product, variant)
+      else
+        if confirm("Are you sure?")
+          $http(
+            method: "DELETE"
+            url: "/api/products/" + product.id + "/variants/" + variant.id
+          ).success (data) ->
+            $scope.removeVariant(product, variant)
+
+    $scope.removeVariant = (product, variant) ->
+      product.variants.splice product.variants.indexOf(variant), 1
+      delete $scope.dirtyProducts[product.id].variants[variant.id]  if $scope.dirtyProducts.hasOwnProperty(product.id) and $scope.dirtyProducts[product.id].hasOwnProperty("variants") and $scope.dirtyProducts[product.id].variants.hasOwnProperty(variant.id)
+      $scope.displayDirtyProducts()
 
 
     $scope.cloneProduct = (product) ->
@@ -325,6 +342,14 @@ productEditModule.controller "AdminProductEditCtrl", [
       Object.keys(product.variants).length > 0
 
 
+    $scope.hasUnit = (product) ->
+      product.variant_unit_with_scale?
+
+
+    $scope.variantSaved = (variant) ->
+      variant.hasOwnProperty('id') && variant.id > 0
+
+
     $scope.hasOnDemandVariants = (product) ->
       (variant for id, variant of product.variants when variant.on_demand).length > 0
 

app/views/spree/admin/products/bulk_edit.html.haml

@@ -111,6 +111,7 @@
         %tr.product
           %td.left-actions
             %a{ 'ofn-toggle-variants' => 'true', :class => "view-variants icon-chevron-right", 'ng-show' => 'hasVariants(product)' }
+            %a{ :class => "add-variant icon-plus-sign", 'ng-click' => "addVariant(product)", 'ng-show' => "!hasVariants(product) && hasUnit(product)" }
           %td.supplier{ 'ng-show' => 'columns.supplier.visible' }
             %select.select2{ 'ng-model' => 'product.supplier', :name => 'supplier', 'ofn-track-product' => 'supplier', 'ng-options' => 's.name for s in suppliers' }
           %td{ 'ng-show' => 'columns.name.visible' }
@@ -118,7 +119,7 @@
           %td.unit{ 'ng-show' => 'columns.unit.visible' }
             %select.select2{ 'ng-model' => 'product.variant_unit_with_scale', :name => 'variant_unit_with_scale', 'ofn-track-product' => 'variant_unit_with_scale', 'ng-options' => 'unit[1] as unit[0] for unit in variant_unit_options' }
               %option{'value' => '', 'ng-hide' => "hasVariants(product)"}
-            %input{ 'ng-model' => 'product.master.unit_value_with_description', :name => 'master_unit_value_with_description', 'ofn-track-product' => 'master.unit_value_with_description', :type => 'text', :placeholder => 'value', 'ng-show' => "product.variant_unit_with_scale != undefined && !hasVariants(product)" }
+            %input{ 'ng-model' => 'product.master.unit_value_with_description', :name => 'master_unit_value_with_description', 'ofn-track-product' => 'master.unit_value_with_description', :type => 'text', :placeholder => 'value', 'ng-show' => "!hasVariants(product) && hasUnit(product)" }
             %input{ 'ng-model' => 'product.variant_unit_name', :name => 'variant_unit_name', 'ofn-track-product' => 'variant_unit_name', :placeholder => 'unit', 'ng-show' => "product.variant_unit_with_scale == 'items'", :type => 'text' }
           %td{ 'ng-show' => 'columns.price.visible' }
             %input{ 'ng-model' => 'product.price', 'ofn-decimal' => :true, :name => 'price', 'ofn-track-product' => 'price', :type => 'text' }
@@ -135,7 +136,8 @@
             %a{ 'ng-click' => 'deleteProduct(product)', :class => "delete-product icon-trash no-text" }
         %tr.variant{ 'ng-repeat' => 'variant in product.variants', 'ng-show' => 'displayProperties[product.id].showVariants', 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'" }
           %td.left-actions
-            %a{ :class => "variant-item icon-caret-right" }
+            %a{ :class => "variant-item icon-caret-right", 'ng-hide' => "$last" }
+            %a{ :class => "add-variant icon-plus-sign", 'ng-click' => "addVariant(product)", 'ng-show' => "$last" }
           %td{ 'ng-show' => 'columns.supplier.visible' }
           %td{ 'ng-show' => 'columns.name.visible' }
             {{ variant.options_text }}
@@ -148,7 +150,7 @@
             %span{ 'ng-bind' => 'variant.on_hand', :name => 'variant_on_hand', 'ng-show' => 'variant.on_demand' }
           %td{ 'ng-show' => 'columns.available_on.visible' }
           %td.actions
-            %a{ 'ng-click' => 'editWarn(product,variant)', :class => "edit-variant icon-edit no-text" }
+            %a{ 'ng-click' => 'editWarn(product,variant)', :class => "edit-variant icon-edit no-text", 'ng-show' => "variantSaved(variant)" }
           %td.actions
           %td.actions
             %a{ 'ng-click' => 'deleteVariant(product,variant)', :class => "delete-variant icon-trash no-text" }

spec/features/admin/bulk_product_update_spec.rb

@@ -229,7 +229,8 @@ feature %q{
     end
   end
 
-  scenario "create a new product" do
+
+  scenario "creating a new product" do
     s = FactoryGirl.create(:supplier_enterprise)
     d = FactoryGirl.create(:distributor_enterprise)
 
@@ -253,6 +254,44 @@ feature %q{
     page.should have_field "product_name", with: 'Big Bag Of Apples'
   end
 
+
+  scenario "creating new variants" do
+    # Given a product without variants or a unit
+    p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil)
+    login_to_admin_section
+    visit '/admin/products/bulk_edit'
+
+    # I should not see an add variant button
+    page.should_not have_selector 'a.add-variant', visible: true
+
+    # When I set the unit
+    select "Weight (kg)", from: "variant_unit_with_scale"
+
+    # I should see an add variant button
+    page.should have_selector 'a.add-variant', visible: true
+
+    # When I add three variants
+    page.find('a.add-variant', visible: true).click
+    page.find('a.add-variant', visible: true).click
+    page.find('a.add-variant', visible: true).click
+
+    # They should be added, and should see no edit buttons
+    page.all("tr.variant").count.should == 3
+    page.should_not have_selector "a.edit-variant", visible: true
+
+    # When I remove one, it should be removed
+    page.all('a.delete-variant').first.click
+    page.all("tr.variant").count.should == 2
+
+    # When I fill out variant details and hit update
+
+    # Then the variants should be saved
+
+    # And I should see edit buttons for the new variants
+
+  end
+
+
   scenario "updating a product with no variants (except master)" do
     s1 = FactoryGirl.create(:supplier_enterprise)
     s2 = FactoryGirl.create(:supplier_enterprise)
@@ -340,6 +379,7 @@ feature %q{
     page.should have_field "variant_unit_value_with_description", with: "123 abc"
   end
 
+
   describe "setting the master unit value for a product without variants" do
     it "sets the master unit value" do
       p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil)

spec/javascripts/unit/bulk_product_update_spec.js.coffee

@@ -603,6 +603,43 @@ describe "AdminProductEditCtrl", ->
       expect(scope.hasOnDemandVariants(product)).toBe(false)
 
 
+  describe "determining whether a product has variants", ->
+    it "returns true when it does", ->
+      product =
+        variants: [{id: 1}, {id: 2}]
+      expect(scope.hasVariants(product)).toBe(true)
+
+    it "returns false when it does not", ->
+      product =
+        variants: []
+      expect(scope.hasVariants(product)).toBe(false)
+
+
+  describe "determining whether a product has a unit", ->
+    it "returns true when it does", ->
+      product =
+        variant_unit_with_scale: 'weight_1000'
+      expect(scope.hasUnit(product)).toBe(true)
+
+    it "returns false when its unit is undefined", ->
+      product = {}
+      expect(scope.hasUnit(product)).toBe(false)
+
+
+  describe "determining whether a variant has been saved", ->
+    it "returns true when it has a positive id", ->
+      variant = {id: 1}
+      expect(scope.variantSaved(variant)).toBe(true)
+
+    it "returns false when it has no id", ->
+      variant = {}
+      expect(scope.variantSaved(variant)).toBe(false)
+
+    it "returns false when it has a negative id", ->
+      variant = {id: -1}
+      expect(scope.variantSaved(variant)).toBe(false)
+
+
   describe "submitting products to be updated", ->
     describe "packing products", ->
       it "extracts variant_unit_with_scale into variant_unit and variant_unit_scale", ->
@@ -889,6 +926,31 @@ describe "AdminProductEditCtrl", ->
       expect(scope.findProduct(123)).toBeNull()
 
 
+  describe "adding variants", ->
+    beforeEach ->
+      scope.displayProperties ||= {123: {}}
+
+    it "adds the first variant", ->
+      product = {id: 123, variants: []}
+      scope.addVariant(product)
+      expect(product).toEqual
+        id: 123
+        variants: [{id: -1}]
+
+    it "adds subsequent variants", ->
+      product = {id: 123, variants: []}
+      scope.addVariant(product)
+      scope.addVariant(product)
+      expect(product).toEqual
+        id: 123
+        variants: [{id: -1}, {id: -2}]
+
+    it "shows the variant(s)", ->
+      product = {id: 123, variants: []}
+      scope.addVariant(product)
+      expect(scope.displayProperties[123].showVariants).toBe(true)
+
+
   describe "deleting products", ->
     it "deletes products with a http delete request to /api/products/id", ->
       spyOn(window, "confirm").andReturn true
@@ -942,83 +1004,100 @@ describe "AdminProductEditCtrl", ->
 
 
   describe "deleting variants", ->
-    it "deletes variants with a http delete request to /api/products/product_id/variants/(variant_id)", ->
-      spyOn(window, "confirm").andReturn true
-      scope.products = [
-        {
-          id: 9
-          permalink_live: "apples"
-          variants: [
-            id: 3
-            price: 12
-          ]
-        }
-        {
-          id: 13
-          permalink_live: "oranges"
-        }
-      ]
-      scope.dirtyProducts = {}
-      httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data"
-      scope.deleteVariant scope.products[0], scope.products[0].variants[0]
-      httpBackend.flush()
+    describe "when the variant has not been saved", ->
+      it "removes the variant from products and dirtyProducts", ->
+        spyOn(window, "confirm").andReturn true
+        scope.products = [
+          {id: 1, variants: [{id: -1}]}
+        ]
+        scope.dirtyProducts =
+          1: {id: 1, variants: {'-1': {id: -1}}}
+        scope.deleteVariant scope.products[0], scope.products[0].variants[0]
+        expect(scope.products).toEqual([
+          {id: 1, variants: []}
+        ])
+        expect(scope.dirtyProducts).toEqual
+          1: {id: 1, variants: {}}
 
-    it "removes the specified variant from both the variants object and scope.dirtyProducts (if it exists there)", ->
-      spyOn(window, "confirm").andReturn true
-      scope.products = [
-        {
-          id: 9
-          permalink_live: "apples"
-          variants: [
-            {
+  
+    describe "when the variant has been saved", ->
+      it "deletes variants with a http delete request to /api/products/product_id/variants/(variant_id)", ->
+        spyOn(window, "confirm").andReturn true
+        scope.products = [
+          {
+            id: 9
+            permalink_live: "apples"
+            variants: [
               id: 3
-              price: 12.0
-            }
-            {
-              id: 4
-              price: 6.0
-            }
-          ]
-        }
-        {
-          id: 13
-          permalink_live: "oranges"
-        }
-      ]
-      scope.dirtyProducts =
-        9:
-          id: 9
-          variants:
-            3:
-              id: 3
-              price: 12.0
+              price: 12
+            ]
+          }
+          {
+            id: 13
+            permalink_live: "oranges"
+          }
+        ]
+        scope.dirtyProducts = {}
+        httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data"
+        scope.deleteVariant scope.products[0], scope.products[0].variants[0]
+        httpBackend.flush()
 
-            4:
-              id: 4
-              price: 6.0
+      it "removes the specified variant from both the variants object and scope.dirtyProducts (if it exists there)", ->
+        spyOn(window, "confirm").andReturn true
+        scope.products = [
+          {
+            id: 9
+            permalink_live: "apples"
+            variants: [
+              {
+                id: 3
+                price: 12.0
+              }
+              {
+                id: 4
+                price: 6.0
+              }
+            ]
+          }
+          {
+            id: 13
+            permalink_live: "oranges"
+          }
+        ]
+        scope.dirtyProducts =
+          9:
+            id: 9
+            variants:
+              3:
+                id: 3
+                price: 12.0
+  
+              4:
+                id: 4
+                price: 6.0
+  
+          13:
+            id: 13
+            name: "P1"
 
-        13:
-          id: 13
-          name: "P1"
+        httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data"
+        scope.deleteVariant scope.products[0], scope.products[0].variants[0]
+        httpBackend.flush()
+        expect(scope.products[0].variants).toEqual [
+          id: 4
+          price: 6.0
+        ]
+        expect(scope.dirtyProducts).toEqual
+          9:
+            id: 9
+            variants:
+              4:
+                id: 4
+                price: 6.0
 
-      httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data"
-      scope.deleteVariant scope.products[0], scope.products[0].variants[0]
-      httpBackend.flush()
-      expect(scope.products[0].variants).toEqual [
-        id: 4
-        price: 6.0
-      ]
-      expect(scope.dirtyProducts).toEqual
-        9:
-          id: 9
-          variants:
-            4:
-              id: 4
-              price: 6.0
-
-        13:
-          id: 13
-          name: "P1"
+          13:
+            id: 13
+            name: "P1"