diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 8fc088fc97..e0d67d1747 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -42,6 +42,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ $scope.setPage = (page) -> $scope.currentPage = page $scope.minPage = -> Math.max(1,Math.min($scope.totalPages()-4,$scope.currentPage-2)) $scope.maxPage = -> Math.min($scope.totalPages(),Math.max(5,$scope.currentPage+2)) + $scope.productsWithUnsavedVariants = [] $scope.$watch -> $scope.totalPages() @@ -108,13 +109,14 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ else null - if product.variants - for variant in product.variants - $scope.loadVariantVariantUnit product, variant - $scope.loadVariantVariantUnit product, product.master if product.master + $scope.loadVariantUnitValues product if product.variants + $scope.loadVariantUnitValue product, product.master if product.master + $scope.loadVariantUnitValues = (product) -> + for variant in product.variants + $scope.loadVariantUnitValue product, variant - $scope.loadVariantVariantUnit = (product, variant) -> + $scope.loadVariantUnitValue = (product, variant) -> unit_value = $scope.variantUnitValue product, variant unit_value = if unit_value? then unit_value else '' variant.unit_value_with_description = "#{unit_value} #{variant.unit_description || ''}".trim() @@ -192,6 +194,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ display_name: null on_hand: null price: null + $scope.productsWithUnsavedVariants.push product $scope.displayProperties[product.id].showVariants = true @@ -200,6 +203,11 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ $scope.variantIdCounter -= 1 $scope.variantIdCounter + $scope.updateVariantLists = (server_products) -> + for product in $scope.productsWithUnsavedVariants + server_product = $scope.findProduct(product.id, server_products) + product.variants = server_product.variants + $scope.loadVariantUnitValues product $scope.deleteProduct = (product) -> if confirm("Are you sure?") @@ -281,20 +289,9 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ products: productsToSubmit filters: $scope.currentFilters ).success((data) -> - # TODO: remove this check altogether, need to write controller tests if we want to test this behaviour properly - # Note: Rob implemented subset(), which is a simpler alternative to productsWithoutDerivedAttributes(). However, it - # conflicted with some changes I made before merging my work, so for now I've reverted to the old way of - # doing things. TODO: Review together and decide on strategy here. -- Rohan, 14-1-2014 - #if subset($scope.productsWithoutDerivedAttributes(), data) - if $scope.productListsMatch $scope.products, data - $scope.resetProducts data - $timeout -> $scope.displaySuccess() - else - # console.log angular.toJson($scope.productsWithoutDerivedAttributes($scope.products)) - # console.log "---" - # console.log angular.toJson($scope.productsWithoutDerivedAttributes(data)) - # console.log "---" - $scope.displayFailure "Product lists do not match." + DirtyProducts.clear() + #$scope.updateVariantLists(data) + $timeout -> $scope.displaySuccess() ).error (data, status) -> $scope.displayFailure "Server returned with error status: " + status @@ -322,56 +319,14 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ if variant.hasOwnProperty("unit_value_with_description") match = variant.unit_value_with_description.match(/^([\d\.]+(?= |$)|)( |)(.*)$/) if match - product = $scope.findProduct(product.id) + product = $scope.findProduct(product.id, $scope.products) variant.unit_value = parseFloat(match[1]) variant.unit_value = null if isNaN(variant.unit_value) variant.unit_value *= product.variant_unit_scale if variant.unit_value && product.variant_unit_scale 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 - products_filtered = $scope.deepCopyProducts products - for product in products_filtered - delete product.variant_unit_with_scale - if product.variants - for variant in product.variants - delete variant.unit_value_with_description - # If we end up live-updating this field, we might want to reinstate its verification here - delete variant.options_text - delete product.master - products_filtered - - - $scope.deepCopyProducts = (products) -> - copied_products = (angular.extend {}, product for product in products) - for product in copied_products - if product.variants - product.variants = (angular.extend {}, variant for variant in product.variants) - copied_products - - - $scope.findProduct = (id) -> - products = (product for product in $scope.products when product.id == id) + $scope.findProduct = (id, product_list) -> + products = (product for product in product_list when product.id == id) if products.length == 0 then null else products[0] diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee index 44466b5d10..88da46d0e2 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee @@ -397,8 +397,9 @@ describe "AdminProductEditCtrl", -> $scope.loadVariantUnit product expect(product.variant_unit_with_scale).toEqual "items" - it "loads data for variants (inc. master)", -> - spyOn $scope, "loadVariantVariantUnit" + it "loads data for variants (incl. master)", -> + spyOn $scope, "loadVariantUnitValues" + spyOn $scope, "loadVariantUnitValue" product = variant_unit_scale: 1.0 @@ -406,15 +407,27 @@ describe "AdminProductEditCtrl", -> variants: [{id: 2, unit_value: 2, unit_description: '(two)'}] $scope.loadVariantUnit product - expect($scope.loadVariantVariantUnit).toHaveBeenCalledWith product, product.variants[0] - expect($scope.loadVariantVariantUnit).toHaveBeenCalledWith product, product.master + expect($scope.loadVariantUnitValues).toHaveBeenCalledWith product + expect($scope.loadVariantUnitValue).toHaveBeenCalledWith product, product.master + + it "loads data for variants (excl. master)", -> + spyOn $scope, "loadVariantUnitValue" + + product = + variant_unit_scale: 1.0 + master: {id: 1, unit_value: 1, unit_description: '(one)'} + variants: [{id: 2, unit_value: 2, unit_description: '(two)'}] + $scope.loadVariantUnitValues product + + expect($scope.loadVariantUnitValue).toHaveBeenCalledWith product, product.variants[0] + expect($scope.loadVariantUnitValue).not.toHaveBeenCalledWith product, product.master describe "setting variant unit_value_with_description", -> it "sets by combining unit_value and unit_description", -> product = variant_unit_scale: 1.0 variants: [{id: 1, unit_value: 1, unit_description: '(bottle)'}] - $scope.loadVariantVariantUnit product, product.variants[0] + $scope.loadVariantUnitValues product, product.variants[0] expect(product.variants[0]).toEqual id: 1 unit_value: 1 @@ -425,28 +438,28 @@ describe "AdminProductEditCtrl", -> product = variant_unit_scale: 1.0 variants: [{id: 1, unit_value: 1}] - $scope.loadVariantVariantUnit product, product.variants[0] + $scope.loadVariantUnitValues product, product.variants[0] expect(product.variants[0].unit_value_with_description).toEqual '1' it "uses unit_description when value is missing", -> product = variant_unit_scale: 1.0 variants: [{id: 1, unit_description: 'Small'}] - $scope.loadVariantVariantUnit product, product.variants[0] + $scope.loadVariantUnitValues product, product.variants[0] expect(product.variants[0].unit_value_with_description).toEqual 'Small' it "converts values from base value to chosen unit", -> product = variant_unit_scale: 1000.0 variants: [{id: 1, unit_value: 2500}] - $scope.loadVariantVariantUnit product, product.variants[0] + $scope.loadVariantUnitValues product, product.variants[0] expect(product.variants[0].unit_value_with_description).toEqual '2.5' it "displays a unit_value of zero", -> product = variant_unit_scale: 1.0 variants: [{id: 1, unit_value: 0}] - $scope.loadVariantVariantUnit product, product.variants[0] + $scope.loadVariantUnitValues product, product.variants[0] expect(product.variants[0].unit_value_with_description).toEqual '0' @@ -827,6 +840,8 @@ describe "AdminProductEditCtrl", -> it "runs displaySuccess() when post returns success", -> spyOn $scope, "displaySuccess" + spyOn $scope, "updateVariantLists" + spyOn DirtyProducts, "clear" $scope.products = [ { id: 1 @@ -851,14 +866,8 @@ describe "AdminProductEditCtrl", -> $httpBackend.flush() $timeout.flush() expect($scope.displaySuccess).toHaveBeenCalled() - - it "runs displayFailure() when post return data does not match $scope.products", -> - spyOn $scope, "displayFailure" - $scope.products = "current list of products" - $httpBackend.expectPOST("/admin/products/bulk_update").respond 200, "returned list of products" - $scope.updateProducts "updated list of products" - $httpBackend.flush() - expect($scope.displayFailure).toHaveBeenCalled() + expect(DirtyProducts.clear).toHaveBeenCalled() + expect($scope.updateVariantLists).toHaveBeenCalled() it "runs displayFailure() when post returns error", -> spyOn $scope, "displayFailure" @@ -868,87 +877,15 @@ describe "AdminProductEditCtrl", -> $httpBackend.flush() 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'}] - expect($scope.productsWithoutDerivedAttributes($scope.products)).toEqual([{id: 123}]) - - it "returns an empty array when products are undefined", -> - expect($scope.productsWithoutDerivedAttributes($scope.products)).toEqual([]) - - it "does not alter original products", -> - $scope.products = [{ - id: 123 - variant_unit_with_scale: 'weight_1000' - variants: [{options_text: 'foo'}] - }] - $scope.productsWithoutDerivedAttributes($scope.products) - expect($scope.products).toEqual [{ - id: 123 - variant_unit_with_scale: 'weight_1000' - variants: [{options_text: 'foo'}] - }] - - describe "updating variants", -> - it "returns variants without the unit_value_with_description field", -> - $scope.products = [{id: 123, variants: [{id: 234, unit_value_with_description: 'foo'}]}] - expect($scope.productsWithoutDerivedAttributes($scope.products)).toEqual [ - { - id: 123 - variants: [{id: 234}] - } - ] - - it "removes the master variant", -> - $scope.products = [{id: 123, master: {id: 234, unit_value_with_description: 'foo'}}] - expect($scope.productsWithoutDerivedAttributes($scope.products)).toEqual [ - { - id: 123 - } - ] - - - describe "deep copying products", -> - it "copies products", -> - product = {id: 123} - copiedProducts = $scope.deepCopyProducts [product] - expect(copiedProducts[0]).not.toBe(product) - - it "copies variants", -> - variant = {id: 1} - product = {id: 123, variants: [variant]} - copiedProducts = $scope.deepCopyProducts [product] - expect(copiedProducts[0].variants[0]).not.toBe(variant) - - describe "fetching a product by id", -> it "returns the product when it is present", -> product = {id: 123} $scope.products = [product] - expect($scope.findProduct(123)).toEqual product + expect($scope.findProduct(123, $scope.products)).toEqual product it "returns null when the product is not present", -> $scope.products = [] - expect($scope.findProduct(123)).toBeNull() + expect($scope.findProduct(123, $scope.products)).toBeNull() describe "adding variants", ->