From 613a738c1387f67333240db1427c5dc8dc17eb41 Mon Sep 17 00:00:00 2001 From: Karthik Viswanathan Date: Mon, 13 Aug 2012 15:47:57 -0700 Subject: [PATCH] Add validation for unique project and screen titles. Fix tests. --- lib/components.js | 26 ++++++++++++++--------- lib/elements.js | 45 +++++++++++++++++++++++++-------------- lib/projects.js | 33 +++++++++++++++++++++++------ lib/scaffold.js | 47 +++++++++++++++++++++-------------------- lib/screens.js | 38 +++++++++++++++++++++++++-------- test/test.components.js | 43 +++++++++++++++++-------------------- 6 files changed, 143 insertions(+), 89 deletions(-) diff --git a/lib/components.js b/lib/components.js index 02cfc7f..23fec46 100644 --- a/lib/components.js +++ b/lib/components.js @@ -23,12 +23,14 @@ function getDefaultComponent(req) { /* Validate component * Requires: * req - web request, - * beingCreated - true if this component is being created for the first time - * and false otherwise - * Returns: true if the request is valid to create/update a component or an - * error message otherwise + * beingCreated - true if this component is being created for the first time and + * false otherwise, + * db - db connection + * Calls: callback([error]): + * error - if there was a validation issue, this is an error message that + * explains it; otherwise, this argument is not provided. */ -function validateComponent(req, beingCreated) { +function validateComponent(req, beingCreated, db, callback) { var body = req.body; var type = body.type; var row = body.row; @@ -36,11 +38,14 @@ function validateComponent(req, beingCreated) { var action = body.action; if (typeof row !== 'number' || typeof col !== 'number') { - return 'Component must have a numeric row and column.'; + callback('Component must have a numeric row and column.'); + return; } else if (typeof type !== 'string') { - return 'Component must have a type.'; + callback('Component must have a type.'); + return; } else if (type.length < 1 || type.length > 20) { - return 'Component must have a type between 1 and 20 characters long.'; + callback('Component must have a type between 1 and 20 characters long.'); + return; } if (action !== undefined) { @@ -49,11 +54,12 @@ function validateComponent(req, beingCreated) { } if (typeof action !== 'number') { - return "Component's action must be a numeric id"; + callback("Component's action must be a numeric id"); + return; } } - return true; + callback(); } exports = module.exports = scaffold.generate(['projects', 'screens'], diff --git a/lib/elements.js b/lib/elements.js index 9a7133c..0cd347d 100644 --- a/lib/elements.js +++ b/lib/elements.js @@ -36,15 +36,18 @@ function getDefaultElement(req) { }; } + /* Validate element * Requires: * req - web request, - * beingCreated - true if this element is being created for the first time - * and false otherwise - * Returns: true if the request is valid to create/update a element or an - * error message otherwise + * beingCreated - true if this element is being created for the first time and + * false otherwise, + * db - db connection + * Calls: callback([error]): + * error - if there was a validation issue, this is an error message that + * explains it; otherwise, this argument is not provided. */ -function validateElement(req, beingCreated) { +function validateElement(req, beingCreated, db, callback) { var body = req.body; var type = body.type; var nextId = body.nextId; @@ -57,11 +60,14 @@ function validateElement(req, beingCreated) { // TODO: Should these be on both the client and server side? If so, how // should they be kept in sync? if (typeof type !== 'string') { - return 'Element must have a type.'; + callback('Element must have a type.'); + return; } else if (type.length < 1 || type.length > 20) { - return "Element must have a type between 1 and 20 characters long."; + callback("Element must have a type between 1 and 20 characters long."); + return; } else if (nextId && typeof nextId !== 'number') { - return "Element's nextId must be an integer."; + callback("Element's nextId must be an integer."); + return; } if (required !== undefined) { @@ -72,23 +78,28 @@ function validateElement(req, beingCreated) { } if (typeof required !== 'boolean') { - return "Element's required attribute must be a boolean."; + callback("Element's required attribute must be a boolean."); + return; } } if (name !== undefined) { if (typeof name !== 'string') { - return "Element's name must be a string."; + callback("Element's name must be a string."); + return; } else if (name.length < 1 || name.length > 50) { - return "Element's name must be between 1 and 50 characters long."; + callback("Element's name must be between 1 and 50 characters long."); + return; } } if (text !== undefined) { if (typeof text !== 'string') { - return "Element's text must be a string."; + callback("Element's text must be a string."); + return; } else if (text.length < 1 || text.length > 500) { - return "Element's text must be between 1 and 500 characters long."; + callback("Element's text must be between 1 and 500 characters long."); + return; } } @@ -98,13 +109,15 @@ function validateElement(req, beingCreated) { } if (typeof level !== 'number') { - return "Element's level must be a number."; + callback("Element's level must be a number."); + return; } else if (level < 1 || level > 6) { - return "Element's level must be between 1 and 6."; + callback("Element's level must be between 1 and 6."); + return; } } - return true; + callback(); } exports = module.exports = scaffold.generate(['projects', 'screens', diff --git a/lib/projects.js b/lib/projects.js index 6437ab0..09fb85e 100644 --- a/lib/projects.js +++ b/lib/projects.js @@ -22,19 +22,38 @@ function getDefaultProject(req) { * Requires: * req - web request, * beingCreated - true if this project is being created for the first time and - * false otherwise - * Returns: true if the request is valid to create/update a project - * or an error message otherwise + * false otherwise, + * db - db connection + * Calls: callback([error]): + * error - if there was a validation issue, this is an error message that + * explains it; otherwise, this argument is not provided. */ -function validateProject(req, beingCreated) { +function validateProject(req, beingCreated, db, callback) { var title = req.body.title; if (!title) { - return 'Project must have a title.'; + callback('Project must have a title.'); + return; } else if (title.length < 1 || title.length > 20) { - return 'Project must have a title between 1 and 20 characters long.'; + callback('Project must have a title between 1 and 20 characters long.'); + return; } - return true; + + exports.list(req, db, function(err, projectList) { + if (err) { + throw err; + } else { + for (var i = 0; i < projectList.length; i++) { + if (projectList[i].title.toLowerCase() === title.toLowerCase()) { + callback('Project must have a unique case-insensitive title.'); + return; + } + } + + // everything has been validated; there are no errors + callback(); + } + }); } /* diff --git a/lib/scaffold.js b/lib/scaffold.js index 8e4a58a..26507b5 100644 --- a/lib/scaffold.js +++ b/lib/scaffold.js @@ -15,11 +15,12 @@ var users = require('./users'); * a request * allowedFields - fields that are allowed to be modified in the form of * a map - * validateRequest - function(req, beingCreated) which determines whether the - * POST values in the given request are valid for creating or updating this - * object. beingCreated will be true if the object is being created or - * false if it is being updated. validateRequest should return true if the - * request is valid or return an error message otherwise. + * validateRequest - function(req, beingCreated, db, callback) which determines + * whether the POST values in the given request are valid for creating or + * updating this object. beingCreated will be true if the object is being + * created or false if it is being updated. validateRequest should call + * the given callback when finished with an error message as the first + * parameter if there were validation issues or no parameters otherwise * * Returns: the scaffold in the form of an object with the following methods: * list(req, db, callback) - get a list of the scaffolded objects @@ -254,34 +255,34 @@ exports.generate = function(parents, children, name, getDefault, // POST should create app.post(baseRoute, middleware, function(req, res) { - var result = validateRequest(req); - if (result !== true) { - res.send(result, 400); - return; - } - - that.add(req, db, function(err, object) { + var result = validateRequest(req, true, db, function(err) { if (err) { - throw err; + res.send(err, 400); } else { - res.send(object); + that.add(req, db, function(err, object) { + if (err) { + throw err; + } else { + res.send(object); + } + }); } }); }); // PUT should update app.put(baseRoute + '/:id', middleware, function(req, res) { - var result = validateRequest(req); - if (result !== true) { - res.send(result, 400); - return; - } - - that.update(req, db, req.params.id, function(err, object) { + var result = validateRequest(req, false, db, function(err) { if (err) { - throw err; + res.send(err, 400); } else { - res.send(object); + that.update(req, db, req.params.id, function(err, object) { + if (err) { + throw err; + } else { + res.send(object); + } + }); } }); }); diff --git a/lib/screens.js b/lib/screens.js index 6b8b316..483ecd0 100644 --- a/lib/screens.js +++ b/lib/screens.js @@ -25,11 +25,13 @@ function getDefaultScreen(req) { * Requires: * req - web request, * beingCreated - true if this screen is being created for the first time and - * false otherwise - * Returns: true if the request is valid to create/update a screen - * or an error message otherwise + * false otherwise, + * db - db connection + * Calls: callback([error]): + * error - if there was a validation issue, this is an error message that + * explains it; otherwise, this argument is not provided. */ -function validateScreen(req, beingCreated) { +function validateScreen(req, beingCreated, db, callback) { var body = req.body; var title = body.title; var isStart = body.isStart; @@ -37,9 +39,11 @@ function validateScreen(req, beingCreated) { // TODO: validate isStart and layout if (!title) { - return 'Screen must have a title.'; + callback('Screen must have a title.'); + return; } else if (title.length < 1 || title.length > 20) { - return 'Screen must have a title between 1 than 20 characters long.'; + callback('Screen must have a title between 1 than 20 characters long.'); + return; } if (isStart !== undefined) { @@ -50,7 +54,8 @@ function validateScreen(req, beingCreated) { } if (typeof isStart !== 'boolean') { - return 'Screen must have an isStart boolean.'; + callback('Screen must have an isStart boolean.'); + return; } } @@ -62,11 +67,26 @@ function validateScreen(req, beingCreated) { } if (typeof secure !== 'boolean') { - return 'Screen must have a secure boolean.'; + callback('Screen must have a secure boolean.'); + return; } } - return true; + exports.list(req, db, function(err, screenList) { + if (err) { + throw err; + } else { + for (var i = 0; i < screenList.length; i++) { + if (screenList[i].title.toLowerCase() === title.toLowerCase()) { + callback('Screen must have a unique case-insensitive title.'); + return; + } + } + + // everything has been validated; there are no errors + callback(); + } + }); } exports = module.exports = scaffold.generate(['projects'], ['components'], diff --git a/test/test.components.js b/test/test.components.js index 9e63323..3d43adc 100644 --- a/test/test.components.js +++ b/test/test.components.js @@ -45,10 +45,8 @@ var componentReq = { }, body: { type: 'form', - layout: { - row: 1, - col: 0 - }, + row: 1, + col: 0, action: '/' }, params: { @@ -63,10 +61,8 @@ var otherComponentReq = { }, body: { type: 'authentication', - layout: { - row: 1, - col: 1 - }, + row: 1, + col: 1, action: '/auth' }, params: { @@ -81,7 +77,7 @@ var elementReq = { }, body: { type: 'input_text', - layout: 'row1', + order: 1, required: true, src: '' }, @@ -113,7 +109,8 @@ describe('component', function() { components.add(req, db, function(err, component) { component.type.should.equal(req.body.type); - component.layout.should.eql(req.body.layout); + component.row.should.eql(req.body.row); + component.col.should.eql(req.body.col); component.action.should.equal(req.body.action); done(); }); @@ -127,7 +124,8 @@ describe('component', function() { setTimeout(function() { components.get(req, db, 2, function(err, component) { component.type.should.equal(req.body.type); - component.layout.should.eql(req.body.layout); + component.row.should.eql(req.body.row); + component.col.should.eql(req.body.col); component.action.should.equal(req.body.action); done(); }); @@ -142,12 +140,14 @@ describe('component', function() { components.list(req, db, function(errList, componentList) { componentList[0].type.should.equal(req.body.type); - componentList[0].layout.should.eql(req.body.layout); + componentList[0].row.should.eql(req.body.row); + componentList[0].col.should.eql(req.body.col); componentList[0].action.should.equal(req.body.action); req = otherComponentReq; componentList[1].type.should.equal(req.body.type); - componentList[1].layout.should.eql(req.body.layout); + componentList[1].row.should.eql(req.body.row); + componentList[1].col.should.eql(req.body.col); componentList[1].action.should.equal(req.body.action); done(); }); @@ -161,7 +161,8 @@ describe('component', function() { it('returns a specific component', function(done) { components.get(req, db, 1, function(err, component) { component.type.should.equal(req.body.type); - component.layout.should.eql(req.body.layout); + component.row.should.eql(req.body.row); + component.col.should.eql(req.body.col); component.action.should.equal(req.body.action); done(); }); @@ -180,27 +181,21 @@ describe('component', function() { var req = componentReq; it('updates a component', function(done) { - req.body.layout = { - row: 2, - col: 1, - }; + req.body.row = 2; components.update(req, db, 1, function(err, component) { - component.layout.should.eql(req.body.layout); + component.row.should.eql(req.body.row); done(); }); }); it('accepts an empty callback', function(done) { - req.body.layout = { - row: 2, - col: 2 - }; + req.body.col = 4; components.update(req, db, 1); // wait 10ms for db transaction to complete setTimeout(function() { components.get(req, db, 1, function(err, component) { - component.layout.should.eql(req.body.layout); + component.col.should.eql(req.body.col); done(); }); }, 10);