BPUR: can update master price. Test refactor and added angular ng-decimal directive

This commit is contained in:
Rob H
2013-05-17 14:11:17 +05:30
committed by Rohan Mitchell
parent 44ad7584f0
commit fbb3657bdc
8 changed files with 209 additions and 115 deletions

View File

@@ -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;
}
}

View File

@@ -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

View File

@@ -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()'}

View File

@@ -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

View File

@@ -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*'
];

View File

@@ -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] }]

View File

@@ -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

View File

@@ -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("<input type='text' ng-demical='true' ng-model='testValue'>");
compiler(field)(scope);
scope.$apply();
expect(field.text()).toBe("123");
});
});*/
});