diff --git a/app/assets/javascripts/admin/bulk_product_update.js b/app/assets/javascripts/admin/bulk_product_update.js index 024aa462ef..62c71abeb0 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js +++ b/app/assets/javascripts/admin/bulk_product_update.js @@ -4,7 +4,39 @@ productsApp.config(["$httpProvider", function(provider) { provider.defaults.headers.common['X-CSRF-Token'] = $('meta[name=csrf-token]').attr('content'); }]); -function AdminBulkProductsCtrl($scope, $timeout, $http) { +productsApp.directive('ngDecimal', function () { + return { + require: 'ngModel', + link: function(scope, element, attrs, ngModel) { + var numRegExp = /^\d+(\.\d+)?$/; + + element.bind('blur', function() { + scope.$apply(ngModel.$setViewValue(ngModel.$modelValue)); + ngModel.$render(); + }); + + ngModel.$setValidity('notADecimalError', function(){ + if (angular.isString(ngModel.$modelValue) && numRegExp.test(ngModel.$modelValue)){ + return true; + } + else{ + return false; + } + }); + + ngModel.$parsers.push(function(viewValue){ + if (angular.isString(viewValue) && numRegExp.test(viewValue)){ + if (viewValue.indexOf(".") == -1){ + return viewValue+".0"; + } + } + return viewValue; + }); + } + } +}); + +productsApp.controller('AdminBulkProductsCtrl', function($scope, $timeout, $http) { $scope.refreshSuppliers = function(){ $http.get('/enterprises/suppliers.json').success(function(data) { $scope.suppliers = data; @@ -13,8 +45,8 @@ function AdminBulkProductsCtrl($scope, $timeout, $http) { $scope.refreshProducts = function(){ $http({ method: 'GET', url:'/admin/products/bulk_index.json' }).success(function(data) { - $scope.products = clone(data); - $scope.cleanProducts = clone(data); + $scope.products = angular.copy(data); + $scope.cleanProducts = angular.copy(data); }); } @@ -70,7 +102,7 @@ function AdminBulkProductsCtrl($scope, $timeout, $http) { $scope.displayFailure = function(failMessage){ $scope.setMessage($scope.updateStatusMessage,"Updating failed. "+failMessage,{ color: "red" },10000); } -} +}); function sortByID(array){ var sortedArray = []; @@ -114,16 +146,25 @@ function getMatchedObjects(testList, cleanList){ return matchedObjects; } -function getDirtyProperties(testObject, cleanObject){ +function getDirtyPropertiesOfObject(testObject, cleanObject){ var dirtyProperties = {}; for (var key in testObject){ if (testObject.hasOwnProperty(key) && cleanObject.hasOwnProperty(key)){ if (testObject[key] != cleanObject[key]){ if (testObject[key] instanceof Array){ - dirtyProperties[key] = getDirtyObjects(testObject[key],cleanObject[key]); //only works for objects with id + if (key == "variants"){ + dirtyProperties[key] = getDirtyObjects(testObject[key],cleanObject[key]); + } + else{ + dirtyProperties[key] = testObject[key]; + } } else if(testObject[key] instanceof Object){ - dirtyProperties[key] = getDirtyObjects([testObject[key]],[cleanObject[key]]); //only works for objects with id + var tempObject = getDirtyPropertiesOfObject(testObject[key],cleanObject[key]); + if ( !isEmpty(tempObject) ){ + if (testObject[key].hasOwnProperty("id")) { tempObject["id"] = testObject[key].id; } + dirtyProperties[key] = tempObject; + } } else{ dirtyProperties[key] = testObject[key]; @@ -140,7 +181,7 @@ function getDirtyObjects(testObjects, cleanObjects){ testObjects = sortByID(testObjects); for (var i in testObjects){ - var dirtyObject = getDirtyProperties(testObjects[i], matchedCleanObjects[i]) + var dirtyObject = getDirtyPropertiesOfObject(testObjects[i], matchedCleanObjects[i]) if ( !isEmpty(dirtyObject) ){ dirtyObject["id"] = testObjects[i].id; dirtyObjects.push(dirtyObject); @@ -187,35 +228,4 @@ function isEmpty(object){ } } return true; -} - -// A. Levy http://stackoverflow.com/questions/728360/most-elegant-way-to-clone-a-javascript-object -function clone(obj) { - // Handle the 3 simple types, and null or undefined - if (null == obj || "object" != typeof obj) return obj; - - // Handle Date - if (obj instanceof Date) { - var copy = new Date(); - copy.setTime(obj.getTime()); - return copy; - } - - // Handle Array - if (obj instanceof Array) { - var copy = []; - for (var i = 0, len = obj.length; i < len; i++) { - copy[i] = clone(obj[i]); - } - return copy; - } - - // Handle Object - if (obj instanceof Object) { - var copy = {}; - for (var attr in obj) { - if (obj.hasOwnProperty(attr)) copy[attr] = clone(obj[attr]); - } - return copy; - } } \ No newline at end of file diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index cd809d880e..80464ce86d 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -5,8 +5,9 @@ Spree::Product.class_eval do has_many :distributors, :through => :product_distributions accepts_nested_attributes_for :product_distributions, :allow_destroy => true + accepts_nested_attributes_for :master, :allow_destroy => false - attr_accessible :supplier_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size + attr_accessible :supplier_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size, :master_attributes validates_presence_of :supplier diff --git a/app/views/spree/admin/products/bulk_index.html.haml b/app/views/spree/admin/products/bulk_index.html.haml index 3dd72c92f8..3c5bd8f6a8 100644 --- a/app/views/spree/admin/products/bulk_index.html.haml +++ b/app/views/spree/admin/products/bulk_index.html.haml @@ -17,6 +17,7 @@ %tr %th Name %th Supplier + %th Price %th Av. On %tbody{ 'ng-repeat' => 'product in products | filter:query' } %tr @@ -24,6 +25,8 @@ %input{ 'ng-model' => "product.name", :name => 'product_name', :type => 'text' } %td %select.select2{ :name => 'supplier_id', 'ng-model' => 'product.supplier_id', 'ng-options' => 's.id as s.name for s in suppliers' } + %td + %input{ 'ng-model' => 'product.master.price', 'ng-decimal' => :true, :name => 'master_price', :type => 'text' } %td %input{ 'ng-model' => 'product.available_on', :name => 'available_on', :type => 'text' } %input{:type => 'button', :value => 'Update', 'ng-click' => 'prepareProductsForSubmit()'} diff --git a/app/views/spree/admin/products/bulk_index.rep b/app/views/spree/admin/products/bulk_index.rep index bf430bc73d..f095c2ab01 100644 --- a/app/views/spree/admin/products/bulk_index.rep +++ b/app/views/spree/admin/products/bulk_index.rep @@ -3,4 +3,8 @@ r.list_of :products, @collection do r.element :name r.element :supplier_id r.element :available_on, Proc.new{ |product| product.available_on.strftime("%F %T") } + r.element :master do + r.element :id + r.element :price + end end \ No newline at end of file diff --git a/config-angular/testacular.conf.js b/config-angular/testacular.conf.js index 9147464ce3..8c48c93db7 100644 --- a/config-angular/testacular.conf.js +++ b/config-angular/testacular.conf.js @@ -8,6 +8,7 @@ files = [ 'app/assets/javascripts/shared/angular-*.js', 'app/assets/javascripts/admin/order_cycle.js.erb.coffee', + 'app/assets/javascripts/admin/bulk_product_update.js', 'spec/javascripts/unit/**/*.js*' ]; diff --git a/spec/controllers/product_controller_spec.rb b/spec/controllers/product_controller_spec.rb index 44825f4377..077179618c 100644 --- a/spec/controllers/product_controller_spec.rb +++ b/spec/controllers/product_controller_spec.rb @@ -28,13 +28,21 @@ describe Spree::Admin::ProductsController do "id" => p1.id, "name" => p1.name, "supplier_id" => p1.supplier_id, - "available_on" => p1.available_on.strftime("%F %T") + "available_on" => p1.available_on.strftime("%F %T"), + "master" => { + "id" => p1.master.id, + "price" => p1.master.price.to_s + } } p2r = { "id" => p2.id, "name" => p2.name, "supplier_id" => p2.supplier_id, - "available_on" => p2.available_on.strftime("%F %T") + "available_on" => p2.available_on.strftime("%F %T"), + "master" => { + "id" => p2.master.id, + "price" => p2.master.price.to_s + } } json_response = JSON.parse(response.body) #json_response = Hash[json_response.map{ |k, v| [k.to_sym, v] }] diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index 579f61c372..ef0b0e0eca 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -43,7 +43,7 @@ feature %q{ page.should have_select "supplier_id", with_options: [s1.name,s2.name,s3.name], selected: s2.name page.should have_select "supplier_id", with_options: [s1.name,s2.name,s3.name], selected: s3.name end - + it "displays a date input for available_on for each product, formatted to yyyy-mm-dd hh:mm:ss" do p1 = FactoryGirl.create(:product, available_on: Date.today) p2 = FactoryGirl.create(:product, available_on: Date.today-1) @@ -53,8 +53,22 @@ feature %q{ page.should have_field "available_on", with: p1.available_on.strftime("%F %T") page.should have_field "available_on", with: p2.available_on.strftime("%F %T") end + + it "displays a price input (for master variant) for each product" do + p1 = FactoryGirl.create(:product) + p2 = FactoryGirl.create(:product) + p1.master.price = 22.00 + p2.master.price = 44.00 + p1.save! + p2.save! + + visit '/admin/products/bulk_index' + + page.should have_field "master_price", with: "22.0" + page.should have_field "master_price", with: "44.0" + end end - + scenario "create a new product" do s = FactoryGirl.create(:supplier_enterprise) d = FactoryGirl.create(:distributor_enterprise) @@ -79,13 +93,14 @@ feature %q{ flash_message.should == 'Product "Big Bag Of Apples" has been successfully created!' page.should have_field "product_name", with: 'Big Bag Of Apples' end - - + scenario "updating a product" do s1 = FactoryGirl.create(:supplier_enterprise) s2 = FactoryGirl.create(:supplier_enterprise) p = FactoryGirl.create(:product, supplier: s1, available_on: Date.today) - + p.master.price = 10.0 + p.save! + login_to_admin_section visit '/admin/products/bulk_index' @@ -93,10 +108,12 @@ feature %q{ page.should have_field "product_name", with: p.name page.should have_select "supplier_id", selected: s1.name page.should have_field "available_on", with: p.available_on.strftime("%F %T") + page.should have_field "master_price", with: "10.0" fill_in "product_name", with: "Big Bag Of Potatoes" select(s2.name, :from => 'supplier_id') fill_in "available_on", with: (Date.today-3).strftime("%F %T") + fill_in "master_price", with: "20" click_button 'Update' page.find("span#update-status-message").should have_content "Update complete" @@ -106,5 +123,6 @@ feature %q{ page.should have_field "product_name", with: "Big Bag Of Potatoes" page.should have_select "supplier_id", selected: s2.name page.should have_field "available_on", with: (Date.today-3).strftime("%F %T") + page.should have_field "master_price", with: "20.0" end end \ No newline at end of file diff --git a/spec/javascripts/unit/bulk_product_update_spec.js b/spec/javascripts/unit/bulk_product_update_spec.js index ad10ac8a93..9500649074 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js +++ b/spec/javascripts/unit/bulk_product_update_spec.js @@ -66,18 +66,25 @@ describe("Auxillary functions", function(){ }); it("returns only differing properties when object do not match", function() { - expect( getDirtyProperties(a, b) ).toEqual( { "2": 5 } ); - expect( getDirtyProperties(b, b) ).toEqual( {} ); + expect( getDirtyPropertiesOfObject(a, b) ).toEqual( { "2": 5 } ); + expect( getDirtyPropertiesOfObject(b, b) ).toEqual( {} ); }); it("ignores properties which are not possessed by both objects", function() { - expect( getDirtyProperties(b, c) ).toEqual( {} ); - expect( getDirtyProperties(c, b) ).toEqual( {} ); + expect( getDirtyPropertiesOfObject(b, c) ).toEqual( {} ); + expect( getDirtyPropertiesOfObject(c, b) ).toEqual( {} ); }); - it("sends and properties which are objects back to getDirtyObjects",function(){ + it("sends properties which are objects back to getDirtyPropertiesOfObject",function(){ + spyOn(window, "getDirtyPropertiesOfObject").andCallThrough(); + getDirtyPropertiesOfObject( { id: 1, num: 3, object: { id: 1, value: "something" } }, { id: 1, num: 2, object: { id: 1, value: "somethingelse" } } ); + expect(getDirtyPropertiesOfObject.calls.length).toEqual(2); + expect(getDirtyPropertiesOfObject).toHaveBeenCalledWith( { id: 1, value: "something" }, { id: 1, value: "somethingelse" } ); + }) + + it("sends properties which are arrays with a key of 'variants' back to getDirtyObjects, ignores other arrays",function(){ spyOn(window, "getDirtyObjects"); - getDirtyProperties( { id: 1, num: 3, object: { id: 1, value: "something" } }, { id: 1, num: 2, object: { id: 1, value: "somethingelse" } } ); + getDirtyPropertiesOfObject( { id: 1, num: 3, array: [ 1, 2, 3 ], variants: [ { id: 1, value: "something" } ] }, { id: 1, num: 2, array: [ 2, 3, 4], variants: [ { id: 1, value: "somethingelse" } ] } ); expect(getDirtyObjects.calls.length).toEqual(1); expect(getDirtyObjects).toHaveBeenCalledWith( [ { id: 1, value: "something" } ], [ { id: 1, value: "somethingelse" } ] ); }) @@ -94,7 +101,7 @@ describe("Auxillary functions", function(){ it("calls getMatchedObjects() once for each call to getDirtyItems", function(){ spyOn(window, "getMatchedObjects").andReturn(b); - spyOn(window, "getDirtyProperties"); + spyOn(window, "getDirtyPropertiesOfObject"); var dirtyObjects = getDirtyObjects(a, b); expect(getMatchedObjects.calls.length).toEqual(1); @@ -104,14 +111,14 @@ describe("Auxillary functions", function(){ it("calls sortByID once for the test Array", function(){ spyOn(window, "getMatchedObjects").andReturn(b); spyOn(window, "sortByID"); - spyOn(window, "getDirtyProperties"); + spyOn(window, "getDirtyPropertiesOfObject"); var dirtyObjects = getDirtyObjects(a, b); expect(sortByID.calls.length).toEqual(1); expect(sortByID).toHaveBeenCalledWith(a); }); - it("returns only valid (non-empty) objects returned by getDirtyProperties", function(){ + it("returns only valid (non-empty) objects returned by getDirtyPropertiesOfObject", function(){ expect( getDirtyObjects(a, b) ).not.toContain( {} ); }); @@ -119,60 +126,20 @@ describe("Auxillary functions", function(){ expect( getDirtyObjects(a, b) ).toEqual( [ { id: 2, value: 20 } ] ); }); - it("calls getDirtyProperties() once for each pair of objects", function(){ + it("calls getDirtyPropertiesOfObject() once for each pair of objects", function(){ spyOn(window, "getMatchedObjects").andReturn(b); - spyOn(window, "getDirtyProperties").andCallThrough(); + spyOn(window, "getDirtyPropertiesOfObject").andCallThrough(); var dirtyObjects = getDirtyObjects(a, b); - expect(getDirtyProperties.calls.length).toEqual(2); - expect(getDirtyProperties).toHaveBeenCalledWith(a[0],b[0]); - expect(getDirtyProperties).toHaveBeenCalledWith(a[1],b[1]); + expect(getDirtyPropertiesOfObject.calls.length).toEqual(2); + expect(getDirtyPropertiesOfObject).toHaveBeenCalledWith(a[0],b[0]); + expect(getDirtyPropertiesOfObject).toHaveBeenCalledWith(a[1],b[1]); }); it("returns an array of objects identified by their ids, and any additional differing properties", function(){ expect( getDirtyObjects(c, d) ).toEqual( [ { id: 2, value: 12 }, { id: 3, value2: 15 } ] ); }); }); -}); - - -describe("AdminBulkProductsCtrl", function(){ - ctrl = null; - scope = null; - timeout = null; - httpBackend = null; - supplierController = null; - - beforeEach(inject(function($controller,$rootScope,$timeout,$httpBackend) { - scope = $rootScope.$new(); - timeout = $timeout; - ctrl = $controller; - httpBackend = $httpBackend; - })); - - describe("loading data upon initialisation", function(){ - it("gets a list of suppliers, a list of products and stores a clean list of products", function(){ - httpBackend.expectGET('/enterprises/suppliers.json').respond("list of suppliers"); - httpBackend.expectGET('/admin/products/bulk_index.json').respond("list of products"); - ctrl('AdminBulkProductsCtrl', { $scope: scope } ); - httpBackend.flush(); - expect(scope.suppliers).toEqual("list of suppliers"); - expect(scope.products).toEqual("list of products"); - expect(scope.cleanProducts).toEqual("list of products"); - }); - - it("does not affect clean products list when products list is altered", function(){ - httpBackend.expectGET('/enterprises/suppliers.json').respond("list of suppliers"); - httpBackend.expectGET('/admin/products/bulk_index.json').respond( [1,2,3,4,5] ); - ctrl('AdminBulkProductsCtrl', { $scope: scope } ); - httpBackend.flush(); - expect(scope.products).toEqual( [1,2,3,4,5] ); - expect(scope.cleanProducts).toEqual( [1,2,3,4,5] ); - scope.products.push(6); - expect(scope.products).toEqual( [1,2,3,4,5,6] ); - expect(scope.cleanProducts).toEqual( [1,2,3,4,5] ); - }); - }); describe("filtering products", function(){ it("only accepts and returns an array", function(){ @@ -183,11 +150,11 @@ describe("AdminBulkProductsCtrl", function(){ expect( filterSubmitProducts( "2" ) ).toEqual([]); expect( filterSubmitProducts( null ) ).toEqual([]); }); - + it("only returns products which have an id property", function(){ expect( filterSubmitProducts( [ { id: 1, name: "p1" }, { notanid: 2, name: "p2"} ] ) ).toEqual( [ { id: 1, name: "p1" } ]); }); - + it("returns variants as variants_attributes", function(){ var testProduct = { id: 1, @@ -206,7 +173,7 @@ describe("AdminBulkProductsCtrl", function(){ } ] } ] ); }); - + it("returns master as master_attributes", function(){ var testProduct = { id: 1, @@ -225,7 +192,7 @@ describe("AdminBulkProductsCtrl", function(){ } ] } ] ); }); - + it("ignores variants without an id, and those for which deleted_at is not null", function(){ var testProduct = { id: 1, @@ -254,7 +221,7 @@ describe("AdminBulkProductsCtrl", function(){ } ] } ] ); }); - + // TODO Not an exhaustive test, is there a better way to do this? it("only returns properties the properties of products which ought to be updated", function(){ var testProduct = { @@ -297,8 +264,65 @@ describe("AdminBulkProductsCtrl", function(){ ); }); }); - +}); + + +describe("AdminBulkProductsCtrl", function(){ + describe("loading data upon initialisation", function(){ + ctrl = null; + scope = null; + httpBackend = null; + + beforeEach(function(){ + module('bulk_product_update'); + }); + + beforeEach(inject(function($controller,$rootScope,$httpBackend) { + scope = $rootScope.$new(); + ctrl = $controller; + httpBackend = $httpBackend; + })); + + it("gets a list of suppliers, a list of products and stores a clean list of products", function(){ + httpBackend.expectGET('/enterprises/suppliers.json').respond("list of suppliers"); + httpBackend.expectGET('/admin/products/bulk_index.json').respond("list of products"); + ctrl('AdminBulkProductsCtrl', { $scope: scope } ); + httpBackend.flush(); + expect(scope.suppliers).toEqual("list of suppliers"); + expect(scope.products).toEqual("list of products"); + expect(scope.cleanProducts).toEqual("list of products"); + }); + + it("does not affect clean products list when products list is altered", function(){ + httpBackend.expectGET('/enterprises/suppliers.json').respond("list of suppliers"); + httpBackend.expectGET('/admin/products/bulk_index.json').respond( [1,2,3,4,5] ); + ctrl('AdminBulkProductsCtrl', { $scope: scope } ); + httpBackend.flush(); + expect(scope.products).toEqual( [1,2,3,4,5] ); + expect(scope.cleanProducts).toEqual( [1,2,3,4,5] ); + scope.products.push(6); + expect(scope.products).toEqual( [1,2,3,4,5,6] ); + expect(scope.cleanProducts).toEqual( [1,2,3,4,5] ); + }); + }); + describe("submitting products to be updated", function(){ + ctrl = null; + scope = null; + timeout = null; + httpBackend = null; + + beforeEach(function(){ + module('bulk_product_update'); + }); + + beforeEach(inject(function($controller,$rootScope,$timeout,$httpBackend) { + scope = $rootScope.$new(); + ctrl = $controller; + timeout = $timeout; + httpBackend = $httpBackend; + })); + describe("preparing products for submit",function(){ beforeEach(function(){ httpBackend.expectGET('/enterprises/suppliers.json').respond("list of suppliers"); @@ -310,15 +334,15 @@ describe("AdminBulkProductsCtrl", function(){ spyOn(scope, "updateProducts"); scope.prepareProductsForSubmit(); }); - + it("fetches dirty products required for submitting", function(){ expect(getDirtyObjects).toHaveBeenCalledWith([1,2,3,4,5],[1,2,3,4,5]); }); - + it("filters returned dirty products", function(){ expect(filterSubmitProducts).toHaveBeenCalledWith( [ { id: 1, value: 1 }, { id:2, value: 2 } ] ); }); - + it("sends dirty and filtered objects to submitProducts()", function(){ expect(scope.updateProducts).toHaveBeenCalledWith( [ { id: 1, value: 3 }, { id:2, value: 4 } ] ); }); @@ -331,13 +355,13 @@ describe("AdminBulkProductsCtrl", function(){ ctrl('AdminBulkProductsCtrl', { $scope: scope, $timeout: timeout } ); httpBackend.flush(); }); - + it("submits products to be updated with a http post request to /admin/products/bulk_update", function(){ httpBackend.expectPOST('/admin/products/bulk_update').respond("list of products"); scope.updateProducts("list of products"); httpBackend.flush(); }); - + it("runs displaySuccess() when post returns success",function(){ spyOn(scope, "displaySuccess"); scope.products = "updated list of products"; @@ -346,7 +370,7 @@ describe("AdminBulkProductsCtrl", function(){ httpBackend.flush(); expect(scope.displaySuccess).toHaveBeenCalled(); }); - + it("runs displayFailure() when post return data does not match $scope.products",function(){ spyOn(scope, "displayFailure"); scope.products = "current list of products"; @@ -355,7 +379,7 @@ describe("AdminBulkProductsCtrl", function(){ httpBackend.flush(); expect(scope.displayFailure).toHaveBeenCalled(); }); - + it("runs displayFailure() when post returns error",function(){ spyOn(scope, "displayFailure"); scope.products = "updated list of products"; @@ -365,5 +389,30 @@ describe("AdminBulkProductsCtrl", function(){ expect(scope.displayFailure).toHaveBeenCalled(); }); }); - }); + }); + + /*describe("directives",function(){ + scope = null; + compiler = null; + + beforeEach(function(){ + module('bulk_product_update'); + }); + + beforeEach(inject(function($rootScope,$compile) { + compiler = $compile; + scope = $rootScope; + })); + + it("should format numeric strings in ngDecimal fields as decimals in the associated model",function(){ + scope.$apply(function() { scope.testValue = "123"; }); + + var field = angular.element(""); + compiler(field)(scope); + + scope.$apply(); + + expect(field.text()).toBe("123"); + }); + });*/ }); \ No newline at end of file