From 701896be95fcf4c48b1e1dc16845b6ef20c39552 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 13 Feb 2014 11:52:44 +1100 Subject: [PATCH] BPE: Add variant and save it to server. Edit the saved variant. --- .../admin/bulk_product_update.js.coffee | 56 +++++++++++++------ app/models/spree/product_set.rb | 6 +- .../admin/bulk_product_update_spec.rb | 21 +++++-- .../unit/bulk_product_update_spec.js.coffee | 53 +++++++++++++++--- 4 files changed, 106 insertions(+), 30 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index eb809d084f..a9735aca4e 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/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 diff --git a/app/models/spree/product_set.rb b/app/models/spree/product_set.rb index db3095187e..e79408d8c5 100644 --- a/app/models/spree/product_set.rb +++ b/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 diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index 0ff07ca0ab..4644aae8c1 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/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 diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee index f87c13a21a..e0d3f33660 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee +++ b/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: []}