Split dirtyProducts out into its own service

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

@@ -1,6 +1,6 @@
 Admin.controller "AdminProductEditCtrl", [
-  "$scope", "$timeout", "$http", "dataFetcher"
-  ($scope, $timeout, $http, dataFetcher) ->
+  "$scope", "$timeout", "$http", "dataFetcher", "DirtyProducts"
+  ($scope, $timeout, $http, dataFetcher, DirtyProducts) ->
     $scope.updateStatusMessage =
       text: ""
       style: {}
@@ -85,7 +85,7 @@ Admin.controller "AdminProductEditCtrl", [
 
     $scope.resetProducts = (data) ->
       $scope.products = data
-      $scope.dirtyProducts = {}
+      DirtyProducts.clear()
       $scope.setMessage $scope.updateStatusMessage, "", {}, false
       $scope.displayProperties ||= {}
       angular.forEach $scope.products, (product) ->
@@ -164,7 +164,7 @@ Admin.controller "AdminProductEditCtrl", [
     $scope.addFilter = (filter) ->
       existingfilterIndex = $scope.indexOfFilter filter
       if $scope.filterableColumns.indexOf(filter.property) >= 0 && $scope.filterTypes.indexOf(filter.predicate) >= 0 && filter.value != "" && filter.value != undefined
-        if ($scope.dirtyProductCount() > 0 and confirm("Unsaved changes will be lost. Continue anyway?")) or ($scope.dirtyProductCount() == 0)
+        if (DirtyProducts.count() > 0 and confirm("Unsaved changes will be lost. Continue anyway?")) or (DirtyProducts.count() == 0)
           if existingfilterIndex == -1
             $scope.currentFilters.push filter
             $scope.fetchProducts()
@@ -186,7 +186,7 @@ Admin.controller "AdminProductEditCtrl", [
       return -1
 
     $scope.editWarn = (product, variant) ->
-      if ($scope.dirtyProductCount() > 0 and confirm("Unsaved changes will be lost. Continue anyway?")) or ($scope.dirtyProductCount() == 0)
+      if (DirtyProducts.count() > 0 and confirm("Unsaved changes will be lost. Continue anyway?")) or (DirtyProducts.count() == 0)
         window.location = "/admin/products/" + product.permalink_live + ((if variant then "/variants/" + variant.id else "")) + "/edit"
 
 
@@ -214,7 +214,7 @@ Admin.controller "AdminProductEditCtrl", [
           url: "/api/products/" + product.id
         ).success (data) ->
           $scope.products.splice $scope.products.indexOf(product), 1
-          delete $scope.dirtyProducts[product.id]  if $scope.dirtyProducts.hasOwnProperty(product.id)
+          DirtyProducts.deleteProduct product.id
           $scope.displayDirtyProducts()
 
 
@@ -231,7 +231,7 @@ Admin.controller "AdminProductEditCtrl", [
 
     $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)
+      DirtyProducts.deleteVariant product.id, variant.id
       $scope.displayDirtyProducts()
 
 
@@ -269,9 +269,9 @@ Admin.controller "AdminProductEditCtrl", [
       # 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
+      $scope.packProduct product for id, product of DirtyProducts.all()
 
-      productsToSubmit = filterSubmitProducts($scope.dirtyProducts)
+      productsToSubmit = filterSubmitProducts(DirtyProducts.all())
       if productsToSubmit.length > 0
         $scope.updateProducts productsToSubmit # Don't submit an empty list
       else
@@ -410,16 +410,12 @@ Admin.controller "AdminProductEditCtrl", [
 
 
     $scope.displayDirtyProducts = ->
-      if $scope.dirtyProductCount() > 0
-        $scope.setMessage $scope.updateStatusMessage, "Changes to " + $scope.dirtyProductCount() + " products remain unsaved.",
+      if DirtyProducts.count() > 0
+        $scope.setMessage $scope.updateStatusMessage, "Changes to " + DirtyProducts.count() + " products remain unsaved.",
           color: "gray"
         , false
       else
         $scope.setMessage $scope.updateStatusMessage, "", {}, false
-
-
-    $scope.dirtyProductCount = ->
-      Object.keys($scope.dirtyProducts).length
 ]
 
 filterSubmitProducts = (productsToFilter) ->
@@ -498,18 +494,6 @@ filterSubmitVariant = (variant) ->
   {filteredVariant: filteredVariant, hasUpdatableProperty: hasUpdatableProperty}
 
 
-addDirtyProperty = (dirtyObjects, objectID, parsedPropertyName, propertyValue) ->
-  if !dirtyObjects.hasOwnProperty(objectID)
-    dirtyObjects[objectID] = {}
-    dirtyObjects[objectID]["id"] = objectID
-  parsedPropertyName.assign(dirtyObjects[objectID], propertyValue)
-
-
-removeCleanProperty = (dirtyObjects, objectID, propertyName) ->
-  delete dirtyObjects[objectID][propertyName]  if dirtyObjects.hasOwnProperty(objectID) and dirtyObjects[objectID].hasOwnProperty(propertyName)
-  delete dirtyObjects[objectID]  if dirtyObjects.hasOwnProperty(objectID) and Object.keys(dirtyObjects[objectID]).length <= 1
-
-
 toObjectWithIDKeys = (array) ->
   object = {}
   

app/assets/javascripts/admin/directives/track_product.js.coffee

@@ -1,10 +1,9 @@
-Admin.directive "ofnTrackProduct", ['$parse', ($parse) ->
+Admin.directive "ofnTrackProduct", ["DirtyProducts", (DirtyProducts) ->
   require: "ngModel"
   link: (scope, element, attrs, ngModel) ->
     ngModel.$parsers.push (viewValue) ->
       if ngModel.$dirty
-        parsedPropertyName = $parse(attrs.ofnTrackProduct)
-        addDirtyProperty scope.dirtyProducts, scope.product.id, parsedPropertyName, viewValue
+        DirtyProducts.addProductProperty scope.product.id, attrs.ofnTrackProduct, viewValue
         scope.displayDirtyProducts()
       viewValue
   ]

app/assets/javascripts/admin/directives/track_variant.js.coffee

@@ -1,13 +1,9 @@
-Admin.directive "ofnTrackVariant", ['$parse', ($parse) ->
+Admin.directive "ofnTrackVariant", ["DirtyProducts", (DirtyProducts) ->
   require: "ngModel"
   link: (scope, element, attrs, ngModel) ->
     ngModel.$parsers.push (viewValue) ->
-      dirtyVariants = {}
-      dirtyVariants = scope.dirtyProducts[scope.product.id].variants  if scope.dirtyProducts.hasOwnProperty(scope.product.id) and scope.dirtyProducts[scope.product.id].hasOwnProperty("variants")
       if ngModel.$dirty
-        parsedPropertyName = $parse(attrs.ofnTrackVariant)
-        addDirtyProperty dirtyVariants, scope.variant.id, parsedPropertyName, viewValue
-        addDirtyProperty scope.dirtyProducts, scope.product.id, $parse("variants"), dirtyVariants
+        DirtyProducts.addVariantProperty scope.product.id, scope.variant.id, attrs.ofnTrackVariant, viewValue
         scope.displayDirtyProducts()
       viewValue
   ]

app/assets/javascripts/admin/services/dirty_products.js.coffee

@@ -0,0 +1,50 @@
+Admin.factory "DirtyProducts", ($parse) ->
+  # Temporary service to track changes in products on admin bulk product edit
+  dirtyProducts = {}
+  
+  addDirtyProperty = (dirtyObjects, objectID, propertyName, propertyValue) ->
+    dirtyObjects[objectID] = { id: objectID } if !dirtyObjects.hasOwnProperty(objectID)
+    $parse(propertyName).assign(dirtyObjects[objectID], propertyValue)
+    
+  return {
+    all: ->
+      dirtyProducts
+
+    addProductProperty: (productID, propertyName, propertyValue) ->
+      addDirtyProperty dirtyProducts, productID, propertyName, propertyValue
+    
+    addVariantProperty: (productID, variantID, propertyName, propertyValue) ->
+      if !dirtyProducts.hasOwnProperty(productID) or !dirtyProducts[productID].hasOwnProperty("variants")
+        addDirtyProperty dirtyProducts, productID, "variants", {}
+      addDirtyProperty dirtyProducts[productID]["variants"], variantID, propertyName, propertyValue
+
+    removeProductProperty: (productID, propertyName) ->
+      if dirtyProducts.hasOwnProperty("#{productID}") and
+        dirtyProducts["#{productID}"].hasOwnProperty("#{propertyName}") 
+          delete dirtyProducts["#{productID}"]["#{propertyName}"]
+          @deleteProduct productID  if Object.keys(dirtyProducts["#{productID}"]).length == 1 # ID
+
+    removeVariantProperty: (productID, variantID, propertyName) ->
+      if dirtyProducts.hasOwnProperty("#{productID}") and
+        dirtyProducts["#{productID}"].hasOwnProperty("variants") and
+        dirtyProducts["#{productID}"]["variants"].hasOwnProperty(variantID) and
+        dirtyProducts["#{productID}"]["variants"]["#{variantID}"].hasOwnProperty("#{propertyName}")
+          delete dirtyProducts["#{productID}"]["variants"]["#{variantID}"]["#{propertyName}"]
+          @deleteVariant productID, variantID  if Object.keys(dirtyProducts["#{productID}"]["variants"]["#{variantID}"]).length == 1 # ID
+    
+    deleteProduct: (productID) ->
+      delete dirtyProducts[productID]  if dirtyProducts.hasOwnProperty(productID)
+
+    deleteVariant: (productID, variantID) ->
+      if dirtyProducts.hasOwnProperty(productID) and
+        dirtyProducts[productID].hasOwnProperty("variants") and
+        dirtyProducts[productID].variants.hasOwnProperty(variantID)
+          delete dirtyProducts["#{productID}"]["variants"]["#{variantID}"] 
+          @removeProductProperty productID, "variants"  if Object.keys(dirtyProducts["#{productID}"]["variants"]).length < 1
+
+    count: ->
+      Object.keys(dirtyProducts).length
+
+    clear: ->
+      dirtyProducts = {}
+  }

spec/javascripts/unit/admin/services/dirty_products_spec.js.coffee

@@ -0,0 +1,111 @@
+describe "Maintaining a live record of dirty products and properties", ->
+  DirtyProducts = null
+  
+  beforeEach ->
+    module "ofn.admin"
+
+  beforeEach inject (_DirtyProducts_) ->
+    DirtyProducts = _DirtyProducts_
+
+  describe "adding a new change", ->
+    it "adds a new object with key of id if it does not already exist", ->
+      expect(DirtyProducts.all()).toEqual {}
+      expect(DirtyProducts.all()["1"]).not.toBeDefined()
+      DirtyProducts.addProductProperty 1, "propertyName", { a: 1 }
+      expect(DirtyProducts.all()["1"]).toBeDefined()
+
+    it "adds an id attribute to newly created objects in dirtyProducts", ->
+      expect(DirtyProducts.all()).toEqual {}
+      DirtyProducts.addProductProperty 1, "propertyName", "val1"
+      expect(DirtyProducts.all()["1"]).toBeDefined()
+      expect(DirtyProducts.all()["1"]["id"]).toBeDefined()
+      expect(DirtyProducts.all()["1"]["id"]).toBe 1
+
+    it "adds a new object with key of the altered attribute name if it does not already exist", ->
+      DirtyProducts.addProductProperty 1, "propertyName", { a: 1 }
+      expect(DirtyProducts.all()["1"]).toBeDefined()
+      expect(DirtyProducts.all()["1"]["propertyName"]).toEqual { a: 1 }
+
+    it "replaces the existing object when adding a change to an attribute which already exists", ->
+      DirtyProducts.addProductProperty 1, "propertyName", "val1"
+      expect(DirtyProducts.all()["1"]).toBeDefined()
+      expect(DirtyProducts.all()["1"]["propertyName"]).toBe "val1"
+      DirtyProducts.addProductProperty 1, "propertyName", "val2"
+      expect(DirtyProducts.all()["1"]["propertyName"]).toBe "val2"
+
+   it "adds an attribute to key to a line item object when one already exists", ->
+      DirtyProducts.addProductProperty 1, "propertyName1", "val1"
+      DirtyProducts.addProductProperty 1, "propertyName2", "val2"
+      expect(DirtyProducts.all()["1"]).toBeDefined()
+      expect(DirtyProducts.all()["1"].hasOwnProperty "propertyName1").toBe true
+      expect(DirtyProducts.all()["1"]["propertyName1"]).toBe "val1"
+      expect(DirtyProducts.all()["1"].hasOwnProperty "propertyName2").toBe true
+      expect(DirtyProducts.all()["1"]["propertyName2"]).toBe "val2"
+
+  describe "clearing all existing changes", ->
+    it "resets pendingChanges object", ->
+      DirtyProducts.addProductProperty 1, "PropertyName1", "val1"
+      DirtyProducts.addProductProperty 1, "PropertyName2", "val2"
+      expect(DirtyProducts.all()["1"]["PropertyName1"]).toBeDefined()
+      expect(DirtyProducts.all()["1"]["PropertyName2"]).toBeDefined()
+      DirtyProducts.clear()
+      expect(DirtyProducts.all()["1"]).not.toBeDefined()
+      expect(DirtyProducts.all()).toEqual {}
+
+  describe "removing an existing dirty product", ->
+    it "deletes a change if it exists", ->
+      DirtyProducts.addProductProperty 1, "PropertyName1", "val1"
+      DirtyProducts.addProductProperty 2, "PropertyName2", "val2"
+      expect(DirtyProducts.all()["1"]["PropertyName1"]).toBeDefined()
+      DirtyProducts.deleteProduct 1
+      expect(DirtyProducts.all()["1"]).not.toBeDefined()
+      expect(DirtyProducts.all()["2"]).toBeDefined()
+      
+    it "does nothing if id key does not exist", ->
+      DirtyProducts.addProductProperty 1, "PropertyName1", "val1"
+      expect(DirtyProducts.all()["1"]["PropertyName1"]).toBeDefined()
+      DirtyProducts.deleteProduct 3
+      expect(DirtyProducts.all()["1"]["PropertyName1"]).toEqual "val1"
+
+  describe "removing an attribute of an existing dirty product", ->
+    it "removes the attribute", ->
+      DirtyProducts.addProductProperty 1, "PropertyName1", "val1"
+      DirtyProducts.addProductProperty 1, "PropertyName2", "val2"
+      DirtyProducts.removeProductProperty 1, "PropertyName1"
+      expect(DirtyProducts.all()["1"]["PropertyName1"]).not.toBeDefined()
+      expect(DirtyProducts.all()["1"]["PropertyName2"]).toBeDefined()
+    
+    it "calls deleteProduct on the productID if no other properties are defined on it", ->
+      spyOn(DirtyProducts, "deleteProduct")
+      DirtyProducts.addProductProperty 1, "PropertyName1", "val1"
+      DirtyProducts.removeProductProperty 1, "PropertyName1"
+      expect(DirtyProducts.deleteProduct).toHaveBeenCalledWith 1
+
+  describe "removing an existing dirty variant", ->
+    it "removes the variant from the variants object", ->
+      DirtyProducts.addVariantProperty 1, 3, "PropertyName1", "val1"
+      DirtyProducts.addProductProperty 1, "PropertyName2", "val2"
+      DirtyProducts.deleteVariant 1, 3
+      expect(DirtyProducts.all()["1"]["variants"]).not.toBeDefined()
+
+    it "calls removeProductProperty on the products if the variants list becomes empty", ->
+      spyOn(DirtyProducts, "removeProductProperty")
+      DirtyProducts.addVariantProperty 1, 3, "PropertyName1", "val1"
+      DirtyProducts.addProductProperty 1, "PropertyName2", "val2"
+      DirtyProducts.deleteVariant 1, 3
+      expect(DirtyProducts.removeProductProperty).toHaveBeenCalledWith 1, "variants"
+
+  describe "removing an attribute of an existing dirty variant", ->
+    it "removes the attribute from the variant object", ->
+      DirtyProducts.addVariantProperty 1, 3, "PropertyName1", "val1"
+      DirtyProducts.addVariantProperty 1, 3, "PropertyName2", "val2"
+      DirtyProducts.removeVariantProperty 1, 3, "PropertyName1"
+      expect(DirtyProducts.all()["1"]["variants"]["3"]["PropertyName1"]).not.toBeDefined()
+      expect(DirtyProducts.all()["1"]["variants"]["3"]["PropertyName2"]).toBeDefined()
+    
+    it "calls deleteVariant on the variantID if no other properties are defined on it", ->
+      spyOn(DirtyProducts, "deleteVariant")
+      DirtyProducts.addVariantProperty 1, 3, "PropertyName1", "val1"
+      DirtyProducts.addProductProperty 1, "PropertyName1", "val2"
+      DirtyProducts.removeVariantProperty 1, 3, "PropertyName1"
+      expect(DirtyProducts.deleteVariant).toHaveBeenCalledWith 1, 3