BPE: Add variant and save it to server. Edit the saved variant.

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

@@ -286,7 +286,13 @@ productEditModule.controller "AdminProductEditCtrl", [
 
 
     $scope.addVariant = (product) ->
-      product.variants.push {id: $scope.nextVariantId()}
+      product.variants.push
+        id: $scope.nextVariantId()
+        price: null
+        unit_value: null
+        unit_description: null
+        on_demand: false
+        on_hand: null
       $scope.displayProperties[product.id].showVariants = true
 
 
@@ -354,6 +360,19 @@ productEditModule.controller "AdminProductEditCtrl", [
       (variant for id, variant of product.variants when variant.on_demand).length > 0
 
 
+    $scope.submitProducts = ->
+      # Pack pack $scope.products, so they will match the list returned from the server,
+      # then pack $scope.dirtyProducts, ensuring that the correct product info is sent to the server.
+      $scope.packProduct product for id, product of $scope.products
+      $scope.packProduct product for id, product of $scope.dirtyProducts
+
+      productsToSubmit = filterSubmitProducts($scope.dirtyProducts)
+      if productsToSubmit.length > 0
+        $scope.updateProducts productsToSubmit # Don't submit an empty list
+      else
+        $scope.setMessage $scope.updateStatusMessage, "No changes to update.", color: "grey", 3000
+
+
     $scope.updateProducts = (productsToSubmit) ->
       $scope.displayUpdating()
       $http(
@@ -369,7 +388,7 @@ productEditModule.controller "AdminProductEditCtrl", [
         #       doing things. TODO: Review together and decide on strategy here. -- Rohan, 14-1-2014
         #if subset($scope.productsWithoutDerivedAttributes(), data)
 
-        if angular.toJson($scope.productsWithoutDerivedAttributes($scope.products)) == angular.toJson($scope.productsWithoutDerivedAttributes(data))
+        if $scope.productListsMatch $scope.products, data
           $scope.resetProducts data
           $timeout -> $scope.displaySuccess()
         else
@@ -382,19 +401,6 @@ productEditModule.controller "AdminProductEditCtrl", [
         $scope.displayFailure "Server returned with error status: " + status
 
 
-    $scope.submitProducts = ->
-      # Pack pack $scope.products, so they will match the list returned from the server,
-      # then pack $scope.dirtyProducts, ensuring that the correct product info is sent to the server.
-      $scope.packProduct product for id, product of $scope.products
-      $scope.packProduct product for id, product of $scope.dirtyProducts
-
-      productsToSubmit = filterSubmitProducts($scope.dirtyProducts)
-      if productsToSubmit.length > 0
-        $scope.updateProducts productsToSubmit # Don't submit an empty list
-      else
-        $scope.setMessage $scope.updateStatusMessage, "No changes to update.", color: "grey", 3000
-
-
     $scope.packProduct = (product) ->
       if product.variant_unit_with_scale
         match = product.variant_unit_with_scale.match(/^([^_]+)_([\d\.]+)$/)
@@ -420,6 +426,24 @@ productEditModule.controller "AdminProductEditCtrl", [
           variant.unit_description = match[3]
 
 
+    $scope.productListsMatch = (clientProducts, serverProducts) ->
+      $scope.copyNewVariantIds clientProducts, serverProducts
+      angular.toJson($scope.productsWithoutDerivedAttributes(clientProducts)) == angular.toJson($scope.productsWithoutDerivedAttributes(serverProducts))
+
+
+    # When variants are created clientside, they are given a negative id. The server
+    # responds with a real id, which would cause the productListsMatch() check to fail.
+    # To avoid that false negative, we copy the server variant id to the client for any
+    # negative ids.
+    $scope.copyNewVariantIds = (clientProducts, serverProducts) ->
+      if clientProducts?
+        for product, i in clientProducts
+          if product.variants?
+            for variant, j in product.variants
+              if variant.id < 0
+                variant.id = serverProducts[i].variants[j].id
+
+
     $scope.productsWithoutDerivedAttributes = (products) ->
       products_filtered = []
       if products
@@ -552,7 +576,7 @@ filterSubmitVariant = (variant) ->
   hasUpdatableProperty = false
   filteredVariant = {}
   if not variant.deleted_at? and variant.hasOwnProperty("id")
-    filteredVariant.id = variant.id
+    filteredVariant.id = variant.id unless variant.id <= 0
     if variant.hasOwnProperty("on_hand")
       filteredVariant.on_hand = variant.on_hand
       hasUpdatableProperty = true

app/models/spree/product_set.rb

@@ -20,7 +20,11 @@ class Spree::ProductSet < ModelSet
   def update_variants_attributes(product, variants_attributes)
     variants_attributes.each do |attributes|
       e = product.variants.detect { |e| e.id.to_s == attributes[:id].to_s && !e.id.nil? }
-      e.update_attributes(attributes.except(:id)) if e.present?
+      if e.present?
+        e.update_attributes(attributes.except(:id))
+      else
+        product.variants.create attributes
+      end
     end
   end
 

spec/features/admin/bulk_product_update_spec.rb

@@ -279,16 +279,29 @@ feature %q{
     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
+    # When I remove two, they should be removed
     page.all('a.delete-variant').first.click
-    page.all("tr.variant").count.should == 2
+    page.all('a.delete-variant').first.click
+    page.all("tr.variant").count.should == 1
 
     # When I fill out variant details and hit update
+    fill_in "variant_unit_value_with_description", with: "4000 (12x250 mL bottles)"
+    fill_in "variant_price", with: "4.0"
+    fill_in "variant_on_hand", with: "10"
+    click_button 'Update'
+    page.find("span#update-status-message").should have_content "Update complete"
 
-    # Then the variants should be saved
+    # Then I should see edit buttons for the new variant
+    page.should have_selector "a.edit-variant", visible: true
 
-    # And I should see edit buttons for the new variants
+    # And the variants should be saved
+    visit '/admin/products/bulk_edit'
+    page.should have_selector "a.view-variants"
+    first("a.view-variants").click
 
+    page.should have_field "variant_unit_value_with_description", with: "4000 (12x250 mL bottles)"
+    page.should have_field "variant_price", with: "4.0"
+    page.should have_field "variant_on_hand", with: "10"
   end
 
 

spec/javascripts/unit/bulk_product_update_spec.js.coffee

@@ -121,6 +121,27 @@ describe "filtering products for submission to database", ->
       ]
     ]
 
+  it "returns variants with a negative id without that id", ->
+    testProduct =
+      id: 1
+      variants: [
+        id: -1
+        on_hand: 5
+        price: 12.0
+        unit_value: 250
+        unit_description: "(bottle)"
+      ]
+
+    expect(filterSubmitProducts([testProduct])).toEqual [
+      id: 1
+      variants_attributes: [
+        on_hand: 5
+        price: 12.0
+        unit_value: 250
+        unit_description: "(bottle)"
+      ]
+    ]
+
   it "does not return variants_attributes property if variants is an empty array", ->
     testProduct =
       id: 1
@@ -862,6 +883,24 @@ describe "AdminProductEditCtrl", ->
         expect(scope.displayFailure).toHaveBeenCalled()
 
 
+  describe "copying new variant ids from server to client", ->
+    it "copies server ids to the client where the client id is negative", ->
+      clientProducts = [
+        {
+          id: 123
+          variants: [{id: 1}, {id: -2}, {id: -3}]
+        }
+      ]
+      serverProducts = [
+        {
+          id: 123
+          variants: [{id: 1}, {id: 4534}, {id: 3453}]
+        }
+      ]
+      scope.copyNewVariantIds(clientProducts, serverProducts)
+      expect(clientProducts).toEqual(serverProducts)
+
+
   describe "fetching products without derived attributes", ->
     it "returns products without the variant_unit_with_scale field", ->
       scope.products = [{id: 123, variant_unit_with_scale: 'weight_1000'}]
@@ -930,20 +969,16 @@ describe "AdminProductEditCtrl", ->
     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", ->
+    it "adds first and subsequent variants", ->
       product = {id: 123, variants: []}
       scope.addVariant(product)
       scope.addVariant(product)
       expect(product).toEqual
         id: 123
-        variants: [{id: -1}, {id: -2}]
+        variants: [
+          {id: -1, price: null, unit_value: null, unit_description: null, on_demand: false, on_hand: null}
+          {id: -2, price: null, unit_value: null, unit_description: null, on_demand: false, on_hand: null}
+        ]
 
     it "shows the variant(s)", ->
       product = {id: 123, variants: []}