From 3c9c169f47cf7e74cbf3a402643436c1625e767f Mon Sep 17 00:00:00 2001 From: Pierre Theo Klein Date: Mon, 3 Dec 2018 19:48:25 -0500 Subject: [PATCH 01/25] Fix mention of docs to reflect new docs address --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1ed30901..7e873a2c 100755 --- a/README.md +++ b/README.md @@ -14,4 +14,4 @@ API for registration, live-site ## How to use and contribute -See documentation here: +See documentation here: From 2c548721f3c424a117a4e2242b537e99274317bc Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 12 Dec 2018 18:10:54 -0500 Subject: [PATCH 02/25] version 1.1.0 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index e4c0d46e..1cc5f657 100755 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.3 \ No newline at end of file +1.1.0 \ No newline at end of file From a91b88209f0b932da20910ef3ac4ce4fe661ecef Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 12 Dec 2018 18:47:05 -0500 Subject: [PATCH 03/25] version 1.1.1 --- VERSION | 2 +- app.js | 2 +- deployment.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index 1cc5f657..8cfbc905 100755 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.1.0 \ No newline at end of file +1.1.1 \ No newline at end of file diff --git a/app.js b/app.js index 842e6e37..7167e691 100755 --- a/app.js +++ b/app.js @@ -42,7 +42,7 @@ if (!Services.env.isProduction()) { } else { // TODO: change this when necessary corsOptions = { - origin: [`https://${process.env.FRONTEND_ADDRESS_DEPLOY}`, `https://hackerapi.mchacks.ca`], + origin: [`https://${process.env.FRONTEND_ADDRESS_DEPLOY}`, `https://docs.mchacks.ca`], credentials: true }; } diff --git a/deployment.yaml b/deployment.yaml index dbb0bdb9..d9adb40b 100644 --- a/deployment.yaml +++ b/deployment.yaml @@ -18,7 +18,7 @@ spec: secretName: hackboard-secret containers: - name: hackboard - image: gcr.io/mchacks-api/hackboard:1.0.2 + image: gcr.io/mchacks-api/hackboard:latest ports: # - containerPort: 443 - containerPort: 8080 \ No newline at end of file From 76ec217cdcd4d3694aee9cbd58513933ace74cf5 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sat, 15 Dec 2018 17:27:16 -0500 Subject: [PATCH 04/25] allow booleanValidator to take comparator value for codeOfConduct --- middlewares/validators/hacker.validator.js | 2 +- middlewares/validators/validator.helper.js | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/middlewares/validators/hacker.validator.js b/middlewares/validators/hacker.validator.js index cbe2b984..bf90c54f 100644 --- a/middlewares/validators/hacker.validator.js +++ b/middlewares/validators/hacker.validator.js @@ -13,7 +13,7 @@ module.exports = { VALIDATOR.alphaArrayValidator("body", "ethnicity", false), VALIDATOR.nameValidator("body", "major", false), VALIDATOR.integerValidator("body", "graduationYear", false, 2019, 2030), - VALIDATOR.booleanValidator("body", "codeOfConduct", false), + VALIDATOR.booleanValidator("body", "codeOfConduct", false, true), ], updateConfirmationValidator: [ diff --git a/middlewares/validators/validator.helper.js b/middlewares/validators/validator.helper.js index 1d5dcbfa..4237d645 100644 --- a/middlewares/validators/validator.helper.js +++ b/middlewares/validators/validator.helper.js @@ -103,19 +103,29 @@ function mongoIdArrayValidator(fieldLocation, fieldname, optional = true) { } /** - * Validates that field must be boolean. + * Validates that field must be boolean. Optionally checks for desired boolean * @param {"query" | "body" | "header" | "param"} fieldLocation the location where the field should be found * @param {string} fieldname name of the field that needs to be validated. * @param {boolean} optional whether the field is optional or not. */ -function booleanValidator(fieldLocation, fieldname, optional = true) { +function booleanValidator(fieldLocation, fieldname, optional = true, desire = null) { const booleanField = setProperValidationChainBuilder(fieldLocation, fieldname, "invalid boolean"); if (optional) { // do not use check falsy option as a 'false' boolean will be skipped - return booleanField.optional().isBoolean().withMessage("must be boolean"); + return booleanField.optional().isBoolean().withMessage("must be boolean").custom((val) => { + if (desire !== null) { + return desire === val; + } + return true; + }).withMessage(`Must be equal to ${desire}`); } else { - return booleanField.exists().isBoolean().withMessage("must be boolean"); + return booleanField.exists().isBoolean().withMessage("must be boolean").custom((val) => { + if (desire !== null) { + return desire === val; + } + return true; + }).withMessage(`Must be equal to ${desire}`); } } From 6080057d0b051578152ac4c5e8ae628056b82834 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sat, 15 Dec 2018 17:27:30 -0500 Subject: [PATCH 05/25] Create tests for false codeOfConduct in hacker --- tests/hacker.test.js | 27 +++++++++++++++++++++++++++ tests/util/hacker.test.util.js | 27 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/tests/hacker.test.js b/tests/hacker.test.js index 8b18025d..5cefc53a 100644 --- a/tests/hacker.test.js +++ b/tests/hacker.test.js @@ -36,6 +36,9 @@ const storedAccount2 = util.account.Account2; const storedHacker2 = util.hacker.HackerB; const newHacker1 = util.hacker.newHacker1; +// badConductHacker1 is the same as newHacker1, even linking to the same account +// the difference is that badConductHacker1 does not accept the code of conducts +const badConductHacker1 = util.hacker.badCodeOfConductHacker1; const newHackerAccount1 = util.account.allAccounts[13]; const newHacker2 = util.hacker.newHacker2; @@ -288,6 +291,30 @@ describe("POST create hacker", function () { }); }); + // should fail due to 'false' on code of conduct + it("should FAIL if the new hacker does not accept code of conduct", function (done) { + util.auth.login(agent, newHackerAccount1, (error) => { + if (error) { + agent.close(); + return done(error); + } + return agent + .post(`/api/hacker/`) + .type("application/json") + .send(badConductHacker1) + .end(function (err, res) { + res.should.have.status(422); + res.should.be.json; + res.body.should.have.property("message"); + res.body.message.should.equal("Validation failed"); + res.body.should.have.property("data"); + res.body.data.should.have.property("codeOfConduct"); + res.body.data.codeOfConduct.msg.should.equal("Must be equal to true"); + done(); + }); + }); + }); + // fail on unconfirmed account, using admin it("should FAIL to create a new hacker if the account hasn't been confirmed", function (done) { util.auth.login(agent, Admin1, (error) => { diff --git a/tests/util/hacker.test.util.js b/tests/util/hacker.test.util.js index 8f32ff44..360a80e2 100644 --- a/tests/util/hacker.test.util.js +++ b/tests/util/hacker.test.util.js @@ -27,6 +27,32 @@ const invalidHacker1 = { "codeOfConduct": true, }; +// duplicate of newHack1, but with false for code of conduct +const badCodeOfConductHacker1 = { + "accountId": Util.Account.generatedAccounts[6]._id, + "school": "University of ASDF", + "degree": "Masters", + "gender": "Female", + "needsBus": true, + "application": { + "portfolioURL": { + //gcloud bucket link + "resume": "www.gcloud.com/myResume100", + "github": "www.github.com/Person1", + "dropler": undefined, + "personal": "www.person1.com", + "linkedIn": "www.linkedin.com/in/Person1", + "other": undefined + }, + "jobInterest": "Full-time", + "skills": ["CSS", "HTML", "JS"], + }, + "ethnicity": ["Caucasian"], + "major": "EE", + "graduationYear": 2019, + "codeOfConduct": false, +}; + const duplicateAccountLinkHacker1 = { "_id": mongoose.Types.ObjectId(), "accountId": Util.Account.Account1._id, @@ -195,6 +221,7 @@ module.exports = { invalidHacker1: invalidHacker1, newHacker1: newHacker1, newHacker2: newHacker2, + badCodeOfConductHacker1: badCodeOfConductHacker1, HackerA: HackerA, HackerB: HackerB, HackerC: HackerC, From edc4eabf25d9bc45550305dda2f1606a2c82c297 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sat, 15 Dec 2018 23:45:48 -0500 Subject: [PATCH 06/25] WIP role create route --- app.js | 1 + controllers/role.controller.js | 11 ++++++ middlewares/role.middleware.js | 13 ++++++ middlewares/validators/role.validator.js | 13 ++++++ middlewares/validators/validator.helper.js | 46 +++++++++++++++++++++- routes/api/role.js | 35 ++++++++++++++++ 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 controllers/role.controller.js create mode 100644 middlewares/role.middleware.js create mode 100644 middlewares/validators/role.validator.js create mode 100644 routes/api/role.js diff --git a/app.js b/app.js index 7167e691..86de759c 100755 --- a/app.js +++ b/app.js @@ -28,6 +28,7 @@ const teamRouter = require("./routes/api/team"); const sponsorRouter = require("./routes/api/sponsor"); const searchRouter = require("./routes/api/search"); const volunteerRouter = require("./routes/api/volunteer"); +const roleRouter = require("./routes/api/role"); const app = express(); Services.db.connect(app); diff --git a/controllers/role.controller.js b/controllers/role.controller.js new file mode 100644 index 00000000..06eb4c32 --- /dev/null +++ b/controllers/role.controller.js @@ -0,0 +1,11 @@ +"use strict"; + +function createdRole(req, res) { + return res.status(200).json({ + message: "TEMP" + }); +} + +module.exports = { + +} \ No newline at end of file diff --git a/middlewares/role.middleware.js b/middlewares/role.middleware.js new file mode 100644 index 00000000..a1a1a858 --- /dev/null +++ b/middlewares/role.middleware.js @@ -0,0 +1,13 @@ +"use strict"; + +function parseRole(req, res, next) { + return next(); +} + +function createRole(req, res, next) { + return next(); +} + +modeul.exports = { + parseRole: parseRole, +}; \ No newline at end of file diff --git a/middlewares/validators/role.validator.js b/middlewares/validators/role.validator.js new file mode 100644 index 00000000..5f550e5b --- /dev/null +++ b/middlewares/validators/role.validator.js @@ -0,0 +1,13 @@ +"use strict"; +const VALIDATOR = require("./validator.helper"); +const Constants = require("../../constants/general.constant"); + +module.exports = { + newRoleValidator: [ + VALIDATOR.alphaValidator("body", "name", false), + VALIDATOR.enumValidator("body", "requestType", Constants.REQUEST_TYPES, false), + ], + + + +}; \ No newline at end of file diff --git a/middlewares/validators/validator.helper.js b/middlewares/validators/validator.helper.js index 4237d645..e08c976e 100644 --- a/middlewares/validators/validator.helper.js +++ b/middlewares/validators/validator.helper.js @@ -607,6 +607,49 @@ function accountTypeValidator(fieldLocation, fieldname, optional = true) { } } +/** + * Validates that field must be a value within the enum passed in through parameter 'enums' + * @param {"query" | "body" | "header" | "param"} fieldLocation The location where the field should be found. + * @param {string} fieldname The name of the field that needs to be validated. + * @param {Object} enums The enum object that the field must be part of. + * @param {boolean} optional Whether the field is optional or not. + */ +function enumValidator(fieldLocation, fieldname, enums, optional = true) { + const enumValue = setProperValidationChainBuilder(fieldLocation, fieldname, "Invalid enums"); + + if (optional) { + return enumValue + .optional({ + checkFalsy: true + }) + .custom((val) => { + return checkEnum(val, enums); + }).withMessage("The value must be part of the enum"); + } else { + return enumValue + .exists() + .withMessage("The value being checked agains the enums must exist.") + .custom((val) => { + return checkEnum(val, enums); + }).withMessage("The value must be part of the enum"); + } +} + +/** + * Checks that 'value' is part of 'enums'. 'enums' should be an enum dict. + * @param {*} value Should be of the same type as the values of the enum + * @param {Object} enums An object that represents an enum. They keys are the keys of the enum, and the values are the enum values. + * @return {boolean} Returns true if the value is part of the enum, false otherwise. + */ +function checkEnum(value, enums) { + for (var enumKey in enums) { + if (value === enumKey) { + return true; + } + } + return false; +} + /** * * @param {"query" | "body" | "header" | "param"} fieldLocation the location where the field should be found @@ -659,5 +702,6 @@ module.exports = { phoneNumberValidator: phoneNumberValidator, dateValidator: dateValidator, hackerCheckInStatusValidator: hackerCheckInStatusValidator, - accountTypeValidator: accountTypeValidator + accountTypeValidator: accountTypeValidator, + enumValidator: enumValidator, }; \ No newline at end of file diff --git a/routes/api/role.js b/routes/api/role.js new file mode 100644 index 00000000..283df638 --- /dev/null +++ b/routes/api/role.js @@ -0,0 +1,35 @@ +"use strict"; + +const express = require("express"); +const Controllers = { + +}; +const Middleware = { + Auth: require("../../middlewares/auth.middleware"), + Validator: { + Role: require("../../middlewares/validators/role.validator"), + }, + parseBody: require("../../middlewares/parse-body.middleware"), + Role: require("../../middlewares/role.middleware"), +}; +const Services = { + +}; + +module.exports = { + activate: function (apiRouter) { + const roleRouter = express.Router(); + + roleRouter.route("/").post( + Middleware.Auth.ensureAuthenticated(), + Middleware.Auth.ensureAuthorized(), + + Middleware.Validator.Role.newRoleValidator, + Middleware.parseBody.middleware, + Middleware.Role.parseBody + + ); + + apiRouter.use("/role", roleRouter); + } +}; \ No newline at end of file From adc8e6db8bd0c2cb15d27196b8c89b891cdbf98d Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:08:27 -0500 Subject: [PATCH 07/25] Change role service async calls to be in line with other services --- services/role.service.js | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/services/role.service.js b/services/role.service.js index 76bab7f4..5ab01954 100644 --- a/services/role.service.js +++ b/services/role.service.js @@ -9,38 +9,40 @@ const logger = require("./logger.service"); * @description * Returns the role defined by the role name */ -async function getRole(roleName) { +function getRole(roleName) { const TAG = "[Role Service # getRole]:"; - const query = {name: roleName}; + const query = { + name: roleName + }; //get the roleBinding for account //Populate roles for roleBinding - return await Role.findOne(query, logger.queryCallbackFactory(TAG, "role", query)); + return Role.findOne(query, logger.queryCallbackFactory(TAG, "role", query)); } /** - * @async * @function getById * @param {ObjectId} id The role id * @description * Returns the role specified by the id. */ -async function getById(id) { +function getById(id) { const TAG = "[Role Service # getById]:"; - const query = {_id: id}; + const query = { + _id: id + }; //get the roleBinding for account //Populate roles for roleBinding - return await Role.findById(query, logger.queryCallbackFactory(TAG, "role", query)); + return Role.findById(query, logger.queryCallbackFactory(TAG, "role", query)); } /** - * @async * @function getAll * @description * Returns all the roles in the database */ -async function getAll(){ +function getAll() { const TAG = "[Role Service # getAll]:"; - return await Role.find({},logger.queryCallbackFactory(TAG, "role", {})); + return Role.find({}, logger.queryCallbackFactory(TAG, "role", {})); } module.exports = { From 8fa1fa6fd1820145cfb053feac1dc80c8109a803 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:48:21 -0500 Subject: [PATCH 08/25] Add role to vs code debugger --- .vscode/launch.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.vscode/launch.json b/.vscode/launch.json index 510a5878..10ed6eb2 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -179,6 +179,22 @@ "${workspaceFolder}/tests/auth.test.js" ], "internalConsoleOptions": "openOnSessionStart" + }, + { + "type": "node", + "request": "launch", + "name": "Mocha Tests - Role", + "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", + "args": [ + "-u", + "tdd", + "--timeout", + "999999", + "--colors", + "${workspaceFolder}/tests/setup.spec.js", + "${workspaceFolder}/tests/role.test.js" + ], + "internalConsoleOptions": "openOnSessionStart" } ] } \ No newline at end of file From d251be65d30f2fb95a0e19402b03bbf7b44c6e2d Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:48:43 -0500 Subject: [PATCH 09/25] Add role services --- services/role.service.js | 60 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/services/role.service.js b/services/role.service.js index 5ab01954..c1bf5282 100644 --- a/services/role.service.js +++ b/services/role.service.js @@ -2,8 +2,59 @@ const Role = require("../models/role.model"); const logger = require("./logger.service"); +function createRole(roleDetails) { + const role = new Role(roleDetails); + + return role.save(); +} + +function isDuplicate(roleDetails) { + const roles = getAll(); + for (let roleName in roles) { + // skip loop if roleName is from prototype + if (!roles.hasOwnProperty(roleName)) { + continue; + } + + let existRole = roles[roleName]; + + let existRoutes = existRole.routes; + + if (routesEquals(existRoutes, roleDetails.routes)) { + return true; + } + } + return false; +} + +function routeEquals(route1, route2) { + return route1.uri === route2.uri && route1.requestType === route2.requestType; +} + +function routesEquals(routes1, routes2) { + if (routes1.length !== routes2.length) { + return false; + } + + for (let i = 0; i < routes1.length; i++) { + if (!routeEquals(routes1[i], routes2[i])) { + return false; + } + } + + return true; +} + +function getByRoutes(routes) { + const TAG = "[Role Service # getByRoutes]:"; + const query = { + routes: routes + }; + + return Role.findOne(query, logger.queryCallbackFactory(TAG, "role", query)); +} + /** - * @async * @function getRole * @param {string} roleName The name of the role that you're looking for. * @description @@ -46,7 +97,10 @@ function getAll() { } module.exports = { + getByRoutes: getByRoutes, getRole: getRole, getById: getById, - getAll: getAll -} \ No newline at end of file + getAll: getAll, + createRole: createRole, + isDuplicate: isDuplicate, +}; \ No newline at end of file From 6285a34b1c2d8d046e16360f9c1a91a4c8c41a19 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:48:58 -0500 Subject: [PATCH 10/25] Fix false async --- controllers/account.controller.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/controllers/account.controller.js b/controllers/account.controller.js index ec82f020..ce703a63 100644 --- a/controllers/account.controller.js +++ b/controllers/account.controller.js @@ -59,14 +59,13 @@ async function getUserById(req, res) { } /** - * @async * @function addUser * @param {{body: {accountDetails: {_id: ObjectId, firstName: string, lastName: string, email: string, password: string, dietaryRestrictions: string, shirtSize: string}}}} req * @param {*} res * @return {JSON} Success or error status * @description Adds a user from information in req.body.accountDetails */ -async function addUser(req, res) { +function addUser(req, res) { const acc = req.body.account; return res.status(200).json({ message: "Account creation successful", @@ -105,7 +104,7 @@ module.exports = { getUserById: Util.asyncMiddleware(getUserById), // assumes all information in req.body - addUser: Util.asyncMiddleware(addUser), + addUser: addUser, updatedAccount: updatedAccount, invitedAccount: invitedAccount }; \ No newline at end of file From 5c77e2be2a565be6e56700b0a20e3c9ce8e7f954 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:49:17 -0500 Subject: [PATCH 11/25] create role middleware --- middlewares/role.middleware.js | 52 ++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/middlewares/role.middleware.js b/middlewares/role.middleware.js index a1a1a858..ceb9a883 100644 --- a/middlewares/role.middleware.js +++ b/middlewares/role.middleware.js @@ -1,13 +1,59 @@ "use strict"; +const mongoose = require("mongoose"); +const Services = { + Role: require("../services/role.service"), +}; +const Constants = { + Error: require("../constants/error.constant"), +}; function parseRole(req, res, next) { + const roleDetails = { + _id: mongoose.Types.ObjectId(), + name: req.body.name, + routes: req.body.routes, + }; + + delete req.body.name; + delete req.body.routes; + + req.body.roleDetails = roleDetails; + return next(); } -function createRole(req, res, next) { - return next(); +async function createRole(req, res, next) { + const roleDetails = req.body.roleDetails; + + // check for duplicate roles + if (Services.Role.isDuplicate(roleDetails)) { + // get by the route + const existingRole = await Services.Role.getByRoutes(roleDetails.routes); + + return res.status(422).json({ + message: Constants.Error.ROLE_DUPLICATE_422_MESSAGE, + data: { + role: existingRole, + } + }); + } + + const role = await Services.Role.createRole(roleDetails); + + if (!!role) { + delete req.body.roleDetails; + req.body.role = role; + return next(); + } else { + return next({ + status: 500, + message: Constants.Error.HACKER_CREATE_500_MESSAGE, + data: {} + }); + } } -modeul.exports = { +module.exports = { parseRole: parseRole, + createRole: createRole, }; \ No newline at end of file From 0cff7efbfc0899c73cb1e184bd0c112748c7d61b Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:49:30 -0500 Subject: [PATCH 12/25] role error constants and add to app.js --- app.js | 2 ++ constants/error.constant.js | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/app.js b/app.js index 86de759c..23093ee7 100755 --- a/app.js +++ b/app.js @@ -84,6 +84,8 @@ volunteerRouter.activate(apiRouter); Services.log.info("Volunteer router activated"); searchRouter.activate(apiRouter); Services.log.info("Search router activated"); +roleRouter.activate(apiRouter); +Services.log.info("Role router activated"); apiRouter.use("/", indexRouter); app.use("/", indexRouter); diff --git a/constants/error.constant.js b/constants/error.constant.js index 92fe72c5..b802ad1e 100644 --- a/constants/error.constant.js +++ b/constants/error.constant.js @@ -16,6 +16,7 @@ const HACKER_STATUS_409_MESSAGE = "Conflict with hacker status"; const TEAM_MEMBER_422_MESSAGE = "Duplicate team member in input"; const VALIDATION_422_MESSAGE = "Validation failed"; const ACCOUNT_DUPLICATE_422_MESSAGE = "Account already exists"; +const ROLE_DUPLICATE_422_MESSAGE = "Role already exists"; const ACCOUNT_TOKEN_401_MESSAGE = "Invalid token for account"; const AUTH_401_MESSAGE = "Invalid Authentication"; @@ -32,6 +33,7 @@ const VOLUNTEER_CREATE_500_MESSAGE = "Error while creating volunteer"; const EMAIL_500_MESSAGE = "Error while generating email"; const GENERIC_500_MESSAGE = "Internal error"; const LOGIN_500_MESSAGE = "Error while logging in"; +const ROLE_CREATE_500_MESSAGE = "Error while creating role"; module.exports = { ACCOUNT_404_MESSAGE: ACCOUNT_404_MESSAGE, @@ -61,4 +63,6 @@ module.exports = { ACCOUNT_DUPLICATE_422_MESSAGE: ACCOUNT_DUPLICATE_422_MESSAGE, LOGIN_500_MESSAGE: LOGIN_500_MESSAGE, HACKER_STATUS_409_MESSAGE: HACKER_STATUS_409_MESSAGE, + ROLE_DUPLICATE_422_MESSAGE: ROLE_DUPLICATE_422_MESSAGE, + ROLE_CREATE_500_MESSAGE: ROLE_CREATE_500_MESSAGE, }; \ No newline at end of file From f7e3234dfdb9a5bcae7670bfa6832e72ef454803 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:49:37 -0500 Subject: [PATCH 13/25] create role controller --- controllers/role.controller.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/controllers/role.controller.js b/controllers/role.controller.js index 06eb4c32..5308d40f 100644 --- a/controllers/role.controller.js +++ b/controllers/role.controller.js @@ -2,10 +2,11 @@ function createdRole(req, res) { return res.status(200).json({ - message: "TEMP" + message: "Create sponsor successful", + data: req.body.sponsor.toJSON(), }); } module.exports = { - -} \ No newline at end of file + createdRole: createdRole, +}; \ No newline at end of file From 8d0338930f0a767713365ef8265edac6d83b1c18 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Sun, 16 Dec 2018 22:49:53 -0500 Subject: [PATCH 14/25] WIP role route and role route tests --- routes/api/role.js | 6 ++-- tests/role.test.js | 67 ++++++++++++++++++++++++++++++++++++ tests/util/role.test.util.js | 9 +++++ 3 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 tests/role.test.js diff --git a/routes/api/role.js b/routes/api/role.js index 283df638..638fb125 100644 --- a/routes/api/role.js +++ b/routes/api/role.js @@ -2,7 +2,7 @@ const express = require("express"); const Controllers = { - + Role: require("../../controllers/role.controller") }; const Middleware = { Auth: require("../../middlewares/auth.middleware"), @@ -26,8 +26,10 @@ module.exports = { Middleware.Validator.Role.newRoleValidator, Middleware.parseBody.middleware, - Middleware.Role.parseBody + Middleware.Role.parseRole, + Middleware.Role.createRole, + Controllers.Role.createdRole ); apiRouter.use("/role", roleRouter); diff --git a/tests/role.test.js b/tests/role.test.js new file mode 100644 index 00000000..d412bb2e --- /dev/null +++ b/tests/role.test.js @@ -0,0 +1,67 @@ +"use strict"; +const chai = require("chai"); +const chaiHttp = require("chai-http"); +chai.use(chaiHttp); +const server = require("../app"); +const Role = require("../models/role.model"); +const agent = chai.request.agent(server.app); +const should = chai.should(); + +const util = { + role: require("./util/role.test.util"), + account: require("./util/account.test.util"), + auth: require("./util/auth.test.util"), +}; + +const Constants = { + Error: require("../constants/error.constant"), +}; + +describe("POST create role", function () { + it("should Fail to create a role because staff is not logged in", function (done) { + chai.request(server.app) + .post(`/api/role/`) + .type("application/json") + .send(util.role.newRole1) + .end(function (err, res) { + res.should.have.status(401); + res.should.be.json; + res.body.should.have.property("message"); + res.body.message.should.equal(Constants.Error.AUTH_401_MESSAGE); + + done() + }); + }); + + // should succeed on logged in admin + it("should SUCCEED and update the user's hacker info", function (done) { + util.auth.login(agent, util.account.Admin1, (error) => { + if (error) { + agent.close(); + return done(error); + } + return agent + .post(`/api/role/`) + .type("application/json") + .send(util.role.newRole1) + .end(function (err, res) { + res.should.have.status(200); + res.should.be.json; + res.body.should.have.property("message"); + res.body.message.should.equal("Create role successful."); + + res.body.should.have.property("data"); + + // deleting _id because that was generated, and not part of original data + delete res.body.data._id; + chai.assert.equal(JSON.stringify(res.body.data), JSON.stringify(util.role.newRole1)); + done(); + }); + }); + }); + + // should fail due to lack of authorization + + // should fail due to duplicate routes + +}); \ No newline at end of file diff --git a/tests/util/role.test.util.js b/tests/util/role.test.util.js index f802ed91..1d90894c 100644 --- a/tests/util/role.test.util.js +++ b/tests/util/role.test.util.js @@ -4,6 +4,14 @@ const Constants = require("../../constants/general.constant"); const mongoose = require("mongoose"); const logger = require("../../services/logger.service"); +const newRole1 = { + name: "newRole", + routes: [{ + uri: "/api/fake/uri", + requestType: Constants.REQUEST_TYPES.GET, + }] +}; + function storeAll(attributes) { const roleDocs = []; const roleNames = []; @@ -28,6 +36,7 @@ async function dropAll() { } module.exports = { + newRole1: newRole1, storeAll: storeAll, dropAll: dropAll, }; \ No newline at end of file From 9e1b19f6c59f2af59e0f65d3182f08b6154a36b8 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 19 Dec 2018 22:13:45 -0800 Subject: [PATCH 15/25] create /api/role/ route --- constants/routes.constant.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/constants/routes.constant.js b/constants/routes.constant.js index 30504705..0a929361 100644 --- a/constants/routes.constant.js +++ b/constants/routes.constant.js @@ -159,6 +159,13 @@ const volunteerRoutes = { }, }; +const roleRoutes = { + "post": { + requestType: Constants.REQUEST_TYPES.POST, + uri: "/api/role/", + } +}; + const allRoutes = { "Auth": authRoutes, "Account": accountRoutes, @@ -166,6 +173,7 @@ const allRoutes = { "Sponsor": sponsorRoutes, "Team": teamRoutes, "Volunteer": volunteerRoutes, + "Role": roleRoutes, }; /** @@ -201,6 +209,7 @@ module.exports = { sponsorRoutes: sponsorRoutes, teamRoutes: teamRoutes, volunteerRoutes: volunteerRoutes, + roleRoutes: roleRoutes, allRoutes: allRoutes, listAllRoutes: listAllRoutes, }; \ No newline at end of file From 0af506283c4553a564b4b82bc60ec4ce9f4681c0 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 19 Dec 2018 22:14:25 -0800 Subject: [PATCH 16/25] Add success constants --- constants/success.constants.js | 7 +++++++ controllers/role.controller.js | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 constants/success.constants.js diff --git a/constants/success.constants.js b/constants/success.constants.js new file mode 100644 index 00000000..2c563528 --- /dev/null +++ b/constants/success.constants.js @@ -0,0 +1,7 @@ +"use strict"; + +const ROLE_CREATE = "Role creation successful."; + +module.exports = { + ROLE_CREATE: ROLE_CREATE, +}; \ No newline at end of file diff --git a/controllers/role.controller.js b/controllers/role.controller.js index 5308d40f..84894dc8 100644 --- a/controllers/role.controller.js +++ b/controllers/role.controller.js @@ -1,9 +1,11 @@ "use strict"; +const Success = require("../constants/success.constants"); + function createdRole(req, res) { return res.status(200).json({ - message: "Create sponsor successful", - data: req.body.sponsor.toJSON(), + message: Success.ROLE_CREATE, + data: req.body.role.toJSON(), }); } From 480326af3be7045d52b929e2c805205d6c6a7199 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 19 Dec 2018 22:14:32 -0800 Subject: [PATCH 17/25] Add tests --- tests/role.test.js | 58 ++++++++++++++++++++++++++++++++---- tests/util/role.test.util.js | 21 ++++++++++--- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/tests/role.test.js b/tests/role.test.js index d412bb2e..3011ba0b 100644 --- a/tests/role.test.js +++ b/tests/role.test.js @@ -15,6 +15,7 @@ const util = { const Constants = { Error: require("../constants/error.constant"), + Success: require("../constants/success.constants"), }; describe("POST create role", function () { @@ -29,12 +30,12 @@ describe("POST create role", function () { res.body.should.have.property("message"); res.body.message.should.equal(Constants.Error.AUTH_401_MESSAGE); - done() + done(); }); }); // should succeed on logged in admin - it("should SUCCEED and update the user's hacker info", function (done) { + it("should SUCCEED and add new role", function (done) { util.auth.login(agent, util.account.Admin1, (error) => { if (error) { agent.close(); @@ -48,20 +49,65 @@ describe("POST create role", function () { res.should.have.status(200); res.should.be.json; res.body.should.have.property("message"); - res.body.message.should.equal("Create role successful."); + res.body.message.should.equal(Constants.Success.ROLE_CREATE); res.body.should.have.property("data"); - // deleting _id because that was generated, and not part of original data - delete res.body.data._id; - chai.assert.equal(JSON.stringify(res.body.data), JSON.stringify(util.role.newRole1)); + // create JSON version of model + // delete id as they will be different between model objects + // delete ids of route objects in 'routes' + const role = (new Role(util.role.newRole1)).toJSON(); + delete res.body.data.id; + delete res.body.data.routes[0]._id; + delete role.id; + delete role.routes[0]._id; + + chai.assert.equal(JSON.stringify(res.body.data), JSON.stringify(role)); done(); }); }); }); // should fail due to lack of authorization + it("should Fail to add new role due to lack of authorization", function (done) { + util.auth.login(agent, util.account.Account1, (error) => { + if (error) { + agent.close(); + return done(error); + } + return agent + .post(`/api/role/`) + .type("application/json") + .send(util.role.newRole1) + .end(function (err, res) { + res.should.have.status(403); + res.should.be.json; + res.body.should.have.property("message"); + res.body.message.should.equal(Constants.Error.AUTH_403_MESSAGE); + done(); + }); + }); + }); // should fail due to duplicate routes + it("should Fail to add new role due to duplicate role", function (done) { + util.auth.login(agent, util.account.Admin1, (error) => { + if (error) { + agent.close(); + return done(error); + } + return agent + .post(`/api/role/`) + .type("application/json") + .send(util.role.duplicateRole1) + .end(function (err, res) { + res.should.have.status(422); + res.should.be.json; + res.body.should.have.property("message"); + res.body.message.should.equal(Constants.Error.ROLE_DUPLICATE_422_MESSAGE); + done(); + }); + }); + }); }); \ No newline at end of file diff --git a/tests/util/role.test.util.js b/tests/util/role.test.util.js index 1d90894c..42ea747e 100644 --- a/tests/util/role.test.util.js +++ b/tests/util/role.test.util.js @@ -1,15 +1,27 @@ "use strict"; const Role = require("../../models/role.model"); const Constants = require("../../constants/general.constant"); +const Routes = require("../../constants/routes.constant"); const mongoose = require("mongoose"); const logger = require("../../services/logger.service"); +const newRoute1 = { + uri: "/api/fake/uri", + requestType: Constants.REQUEST_TYPES.GET, +}; + const newRole1 = { name: "newRole", - routes: [{ - uri: "/api/fake/uri", - requestType: Constants.REQUEST_TYPES.GET, - }] + routes: [ + newRoute1, + ] +}; + +const duplicateRole1 = { + name: "duplicateRole", + routes: [ + Routes.hackerRoutes.getAnyById, + ] }; function storeAll(attributes) { @@ -37,6 +49,7 @@ async function dropAll() { module.exports = { newRole1: newRole1, + duplicateRole1: duplicateRole1, storeAll: storeAll, dropAll: dropAll, }; \ No newline at end of file From 6bed46fcc580693ad66f93966c89c73ae6d52893 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 19 Dec 2018 22:14:46 -0800 Subject: [PATCH 18/25] Fix validator logic --- middlewares/validators/role.validator.js | 2 +- middlewares/validators/validator.helper.js | 39 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/middlewares/validators/role.validator.js b/middlewares/validators/role.validator.js index 5f550e5b..622685c8 100644 --- a/middlewares/validators/role.validator.js +++ b/middlewares/validators/role.validator.js @@ -5,7 +5,7 @@ const Constants = require("../../constants/general.constant"); module.exports = { newRoleValidator: [ VALIDATOR.alphaValidator("body", "name", false), - VALIDATOR.enumValidator("body", "requestType", Constants.REQUEST_TYPES, false), + VALIDATOR.routesValidator("body", "routes", false), ], diff --git a/middlewares/validators/validator.helper.js b/middlewares/validators/validator.helper.js index e08c976e..780dee31 100644 --- a/middlewares/validators/validator.helper.js +++ b/middlewares/validators/validator.helper.js @@ -607,6 +607,42 @@ function accountTypeValidator(fieldLocation, fieldname, optional = true) { } } +function routesValidator(fieldLocation, fieldname, optional = true) { + const routes = setProperValidationChainBuilder(fieldLocation, fieldname, "Invalid routes"); + + if (optional) { + return routes + .optional({ + checkFalsy: true + }) + .custom(routesArrayValidationHelper).withMessage("The value must be a route"); + } else { + return routes + .exists() + .withMessage("The value being checked agains the enums must exist.") + .custom(routesArrayValidationHelper).withMessage("The value must be a route"); + } +} + +/** + * Returns true if value an array of routes + * @param {*} routes value to check against + */ +function routesArrayValidationHelper(routes) { + if (!Array.isArray(routes)) { + return false; + } + for (const route of routes) { + if (route.uri === null || typeof route.uri !== "string") { + return false; + } + if (route.requestType === null || !checkEnum(route.requestType, Constants.REQUEST_TYPES)) { + return false; + } + } + return true; +} + /** * Validates that field must be a value within the enum passed in through parameter 'enums' * @param {"query" | "body" | "header" | "param"} fieldLocation The location where the field should be found. @@ -643,7 +679,7 @@ function enumValidator(fieldLocation, fieldname, enums, optional = true) { */ function checkEnum(value, enums) { for (var enumKey in enums) { - if (value === enumKey) { + if (value === enums[enumKey]) { return true; } } @@ -704,4 +740,5 @@ module.exports = { hackerCheckInStatusValidator: hackerCheckInStatusValidator, accountTypeValidator: accountTypeValidator, enumValidator: enumValidator, + routesValidator: routesValidator, }; \ No newline at end of file From e5a2564a18e9d66b1a0df9d812fe8aea7b89cd5d Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 19 Dec 2018 22:15:12 -0800 Subject: [PATCH 19/25] fix bugs with services with missing await --- middlewares/role.middleware.js | 2 +- models/role.model.js | 2 +- services/role.service.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/middlewares/role.middleware.js b/middlewares/role.middleware.js index ceb9a883..ce0bcb3a 100644 --- a/middlewares/role.middleware.js +++ b/middlewares/role.middleware.js @@ -26,7 +26,7 @@ async function createRole(req, res, next) { const roleDetails = req.body.roleDetails; // check for duplicate roles - if (Services.Role.isDuplicate(roleDetails)) { + if (await Services.Role.isDuplicate(roleDetails)) { // get by the route const existingRole = await Services.Role.getByRoutes(roleDetails.routes); diff --git a/models/role.model.js b/models/role.model.js index 6abf3fa9..c9af26b1 100644 --- a/models/role.model.js +++ b/models/role.model.js @@ -21,7 +21,7 @@ const RoleSchema = new mongoose.Schema({ }, requestType: { type: String, - enum: Object.entries(Constants.REQUEST_TYPES) + enum: Object.values(Constants.REQUEST_TYPES) }, }] }); diff --git a/services/role.service.js b/services/role.service.js index c1bf5282..ef16ac62 100644 --- a/services/role.service.js +++ b/services/role.service.js @@ -8,8 +8,8 @@ function createRole(roleDetails) { return role.save(); } -function isDuplicate(roleDetails) { - const roles = getAll(); +async function isDuplicate(roleDetails) { + const roles = await getAll(); for (let roleName in roles) { // skip loop if roleName is from prototype if (!roles.hasOwnProperty(roleName)) { From 925f465b898267fb1f52a14fc76ae9091cae36d7 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 19 Dec 2018 22:26:20 -0800 Subject: [PATCH 20/25] comenting and documentation --- controllers/role.controller.js | 7 ++ docs/api/api_data.js | 85 ++++++++++++++++++++++ docs/api/api_data.json | 85 ++++++++++++++++++++++ docs/api/api_project.js | 2 +- docs/api/api_project.json | 2 +- middlewares/role.middleware.js | 19 +++++ middlewares/validators/validator.helper.js | 6 ++ routes/api/role.js | 43 ++++++++++- 8 files changed, 244 insertions(+), 5 deletions(-) diff --git a/controllers/role.controller.js b/controllers/role.controller.js index 84894dc8..17fe40c3 100644 --- a/controllers/role.controller.js +++ b/controllers/role.controller.js @@ -2,6 +2,13 @@ const Success = require("../constants/success.constants"); +/** + * @function createdRole + * @param {{body: {role: Object}}} req + * @param {*} res + * @return {JSON} Success status and role object + * @description Returns the JSON of role object located in req.body.role + */ function createdRole(req, res) { return res.status(200).json({ message: Success.ROLE_CREATE, diff --git a/docs/api/api_data.js b/docs/api/api_data.js index 4f5597f3..8e93cfd4 100644 --- a/docs/api/api_data.js +++ b/docs/api/api_data.js @@ -1697,6 +1697,91 @@ define({ "url": "https://api.mchacks.ca/api/" }] }, + { + "type": "post", + "url": "/api/role/", + "title": "create a new hacker", + "name": "createRole", + "group": "Role", + "version": "1.1.1", + "parameter": { + "fields": { + "body": [{ + "group": "body", + "type": "String", + "optional": false, + "field": "name", + "description": "

Name of the route

" + }, + { + "group": "body", + "type": "Route[]", + "optional": false, + "field": "routes", + "description": "

The routes that this role gives access to

" + } + ] + }, + "examples": [{ + "title": "application: ", + "content": "{\n \"name\": \"routename\",\n \"routes\": [\n {\n uri: \"/api/hacker/\"\n requestType: \"POST\"\n }\n ]\n}", + "type": "Json" + }] + }, + "success": { + "fields": { + "Success 200": [{ + "group": "Success 200", + "type": "string", + "optional": false, + "field": "message", + "description": "

Success message

" + }, + { + "group": "Success 200", + "type": "object", + "optional": false, + "field": "data", + "description": "

Role object

" + } + ] + }, + "examples": [{ + "title": "Success-Response: ", + "content": "{\n \"message\": \"Role creation successful\", \n \"data\": {\n \"name\": \"routename\",\n \"routes\": [\n {\n uri: \"/api/hacker/\"\n requestType: \"POST\"\n }\n ]\n }\n}", + "type": "object" + }] + }, + "error": { + "fields": { + "Error 4xx": [{ + "group": "Error 4xx", + "type": "string", + "optional": false, + "field": "message", + "description": "

Error message

" + }, + { + "group": "Error 4xx", + "type": "object", + "optional": false, + "field": "data", + "description": "

empty

" + } + ] + }, + "examples": [{ + "title": "Error-Response: ", + "content": "{\"message\": \"Error while creating role\", \"data\": {}}", + "type": "object" + }] + }, + "filename": "routes/api/role.js", + "groupTitle": "Role", + "sampleRequest": [{ + "url": "https://api.mchacks.ca/api/api/role/" + }] + }, { "type": "get", "url": "/search/:model", diff --git a/docs/api/api_data.json b/docs/api/api_data.json index 933b1514..449e189e 100644 --- a/docs/api/api_data.json +++ b/docs/api/api_data.json @@ -1696,6 +1696,91 @@ "url": "https://api.mchacks.ca/api/" }] }, + { + "type": "post", + "url": "/api/role/", + "title": "create a new hacker", + "name": "createRole", + "group": "Role", + "version": "1.1.1", + "parameter": { + "fields": { + "body": [{ + "group": "body", + "type": "String", + "optional": false, + "field": "name", + "description": "

Name of the route

" + }, + { + "group": "body", + "type": "Route[]", + "optional": false, + "field": "routes", + "description": "

The routes that this role gives access to

" + } + ] + }, + "examples": [{ + "title": "application: ", + "content": "{\n \"name\": \"routename\",\n \"routes\": [\n {\n uri: \"/api/hacker/\"\n requestType: \"POST\"\n }\n ]\n}", + "type": "Json" + }] + }, + "success": { + "fields": { + "Success 200": [{ + "group": "Success 200", + "type": "string", + "optional": false, + "field": "message", + "description": "

Success message

" + }, + { + "group": "Success 200", + "type": "object", + "optional": false, + "field": "data", + "description": "

Role object

" + } + ] + }, + "examples": [{ + "title": "Success-Response: ", + "content": "{\n \"message\": \"Role creation successful\", \n \"data\": {\n \"name\": \"routename\",\n \"routes\": [\n {\n uri: \"/api/hacker/\"\n requestType: \"POST\"\n }\n ]\n }\n}", + "type": "object" + }] + }, + "error": { + "fields": { + "Error 4xx": [{ + "group": "Error 4xx", + "type": "string", + "optional": false, + "field": "message", + "description": "

Error message

" + }, + { + "group": "Error 4xx", + "type": "object", + "optional": false, + "field": "data", + "description": "

empty

" + } + ] + }, + "examples": [{ + "title": "Error-Response: ", + "content": "{\"message\": \"Error while creating role\", \"data\": {}}", + "type": "object" + }] + }, + "filename": "routes/api/role.js", + "groupTitle": "Role", + "sampleRequest": [{ + "url": "https://api.mchacks.ca/api/api/role/" + }] + }, { "type": "get", "url": "/search/:model", diff --git a/docs/api/api_project.js b/docs/api/api_project.js index 62fd88fb..01be3871 100644 --- a/docs/api/api_project.js +++ b/docs/api/api_project.js @@ -9,7 +9,7 @@ define({ "apidoc": "0.3.0", "generator": { "name": "apidoc", - "time": "2018-12-11T06:22:15.249Z", + "time": "2018-12-20T06:25:49.908Z", "url": "http://apidocjs.com", "version": "0.17.6" } diff --git a/docs/api/api_project.json b/docs/api/api_project.json index 8ae30989..f4735333 100644 --- a/docs/api/api_project.json +++ b/docs/api/api_project.json @@ -9,7 +9,7 @@ "apidoc": "0.3.0", "generator": { "name": "apidoc", - "time": "2018-12-11T06:22:15.249Z", + "time": "2018-12-20T06:25:49.908Z", "url": "http://apidocjs.com", "version": "0.17.6" } diff --git a/middlewares/role.middleware.js b/middlewares/role.middleware.js index ce0bcb3a..469af01e 100644 --- a/middlewares/role.middleware.js +++ b/middlewares/role.middleware.js @@ -7,6 +7,16 @@ const Constants = { Error: require("../constants/error.constant"), }; +/** + * @function parseRole + * @param {{body: name: String, routes: route[]}}} req + * @param {*} res + * @param {(err?)=>void} next + * @return {void} + * @description + * Moves name and routes from req.body to req.body.roleDetails. + * Adds _id to roleDetails. + */ function parseRole(req, res, next) { const roleDetails = { _id: mongoose.Types.ObjectId(), @@ -22,6 +32,15 @@ function parseRole(req, res, next) { return next(); } +/** + * @function createRole + * @param {{body: {roleDetails: object}}} req + * @param {*} res + * @param {(err?)=>void} next + * @return {void} + * @description + * Creates role document after making sure there is no other role with the same routes + */ async function createRole(req, res, next) { const roleDetails = req.body.roleDetails; diff --git a/middlewares/validators/validator.helper.js b/middlewares/validators/validator.helper.js index 780dee31..38b1f638 100644 --- a/middlewares/validators/validator.helper.js +++ b/middlewares/validators/validator.helper.js @@ -607,6 +607,12 @@ function accountTypeValidator(fieldLocation, fieldname, optional = true) { } } +/** + * Validates that field must be an array of routes + * @param {"query" | "body" | "header" | "param"} fieldLocation the location where the field should be found + * @param {string} fieldname name of the field that needs to be validated. + * @param {boolean} optional whether the field is optional or not. + */ function routesValidator(fieldLocation, fieldname, optional = true) { const routes = setProperValidationChainBuilder(fieldLocation, fieldname, "Invalid routes"); diff --git a/routes/api/role.js b/routes/api/role.js index 638fb125..eb428f8f 100644 --- a/routes/api/role.js +++ b/routes/api/role.js @@ -12,14 +12,51 @@ const Middleware = { parseBody: require("../../middlewares/parse-body.middleware"), Role: require("../../middlewares/role.middleware"), }; -const Services = { - -}; module.exports = { activate: function (apiRouter) { const roleRouter = express.Router(); + /** + * @api {post} /api/role/ create a new hacker + * @apiName createRole + * @apiGroup Role + * @apiVersion 1.1.1 + * + * @apiParam (body) {String} name Name of the route + * @apiParam (body) {Route[]} routes The routes that this role gives access to + * @apiParamExample {Json} application: + * { + "name": "routename", + "routes": [ + { + uri: "/api/hacker/" + requestType: "POST" + } + ] + * } + * + * @apiSuccess {string} message Success message + * @apiSuccess {object} data Role object + * @apiSuccessExample {object} Success-Response: + * { + * "message": "Role creation successful", + * "data": { + "name": "routename", + "routes": [ + { + uri: "/api/hacker/" + requestType: "POST" + } + ] + * } + * } + + * @apiError {string} message Error message + * @apiError {object} data empty + * @apiErrorExample {object} Error-Response: + * {"message": "Error while creating role", "data": {}} + */ roleRouter.route("/").post( Middleware.Auth.ensureAuthenticated(), Middleware.Auth.ensureAuthorized(), From 1cc002e3346049b168ed5892e80d97ab1e434a80 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Thu, 20 Dec 2018 11:15:19 -0800 Subject: [PATCH 21/25] Add comments, change tests a little --- package-lock.json | 4 +-- services/role.service.js | 56 +++++++++++++++++++----------------- tests/role.test.js | 8 ++++-- tests/util/role.test.util.js | 8 ++---- 4 files changed, 40 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9ac919e8..562f3113 100755 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "dependencies": { "@google-cloud/common": { "version": "0.17.0", - "resolved": "http://registry.npmjs.org/@google-cloud/common/-/common-0.17.0.tgz", + "resolved": "https://registry.npmjs.org/@google-cloud/common/-/common-0.17.0.tgz", "integrity": "sha512-HRZLSU762E6HaKoGfJGa8W95yRjb9rY7LePhjaHK9ILAnFacMuUGVamDbTHu1csZomm1g3tZTtXfX/aAhtie/Q==", "requires": { "array-uniq": "^1.0.3", @@ -3429,7 +3429,7 @@ }, "gcp-metadata": { "version": "0.6.3", - "resolved": "http://registry.npmjs.org/gcp-metadata/-/gcp-metadata-0.6.3.tgz", + "resolved": "https://registry.npmjs.org/gcp-metadata/-/gcp-metadata-0.6.3.tgz", "integrity": "sha512-MSmczZctbz91AxCvqp9GHBoZOSbJKAICV7Ow/AIWSJZRrRchUd5NL1b2P4OfP+4m490BEUPhhARfpHdqCxuCvg==", "requires": { "axios": "^0.18.0", diff --git a/services/role.service.js b/services/role.service.js index ef16ac62..1bfa4256 100644 --- a/services/role.service.js +++ b/services/role.service.js @@ -2,49 +2,53 @@ const Role = require("../models/role.model"); const logger = require("./logger.service"); +/** + * @function createRole + * @param {{_id: ObjectId, name: String, routes: route[]}} roleDetails + * @return {Promise} The promise will resolve to a role object if save was successful. + * @description Adds a new role to database. + */ function createRole(roleDetails) { const role = new Role(roleDetails); return role.save(); } +/** + * @function isDuplicate + * @param {{_id: ObjectId, name: String, routes: route[]}} roleDetails + * @return {boolean} True or false depending on whether the routes are duplicated + * @description Check that the routes contained in routeDetails aren't already specified + */ async function isDuplicate(roleDetails) { - const roles = await getAll(); - for (let roleName in roles) { - // skip loop if roleName is from prototype - if (!roles.hasOwnProperty(roleName)) { - continue; - } + const existRole = await getByRoutes(roleDetails.routes); - let existRole = roles[roleName]; + const all = await getAll(); - let existRoutes = existRole.routes; - - if (routesEquals(existRoutes, roleDetails.routes)) { - return true; - } + if (!!existRole) { + return true; } + return false; } +/** + * @function routeEquals + * @param {{uri: String, requestType: Enum}} route1 + * @param {{uri: String, requestType: Enum}} route2 + * @return {boolean} Returns whether route1 is the same as route2 + * @description Checks that route1 and route2 are the same + */ function routeEquals(route1, route2) { return route1.uri === route2.uri && route1.requestType === route2.requestType; } -function routesEquals(routes1, routes2) { - if (routes1.length !== routes2.length) { - return false; - } - - for (let i = 0; i < routes1.length; i++) { - if (!routeEquals(routes1[i], routes2[i])) { - return false; - } - } - - return true; -} - +/** + * @function getByRoutes + * @param {route[]} routes + * @returns {Promise} The promise will resolve to a role object if successful. + * @description finds a role object by the routes + */ function getByRoutes(routes) { const TAG = "[Role Service # getByRoutes]:"; const query = { diff --git a/tests/role.test.js b/tests/role.test.js index 3011ba0b..cf582187 100644 --- a/tests/role.test.js +++ b/tests/role.test.js @@ -58,9 +58,13 @@ describe("POST create role", function () { // delete ids of route objects in 'routes' const role = (new Role(util.role.newRole1)).toJSON(); delete res.body.data.id; - delete res.body.data.routes[0]._id; + for (var route of res.body.data.routes) { + delete route._id; + } delete role.id; - delete role.routes[0]._id; + for (route of role.routes) { + delete route._id; + } chai.assert.equal(JSON.stringify(res.body.data), JSON.stringify(role)); done(); diff --git a/tests/util/role.test.util.js b/tests/util/role.test.util.js index 42ea747e..142b47c5 100644 --- a/tests/util/role.test.util.js +++ b/tests/util/role.test.util.js @@ -5,15 +5,11 @@ const Routes = require("../../constants/routes.constant"); const mongoose = require("mongoose"); const logger = require("../../services/logger.service"); -const newRoute1 = { - uri: "/api/fake/uri", - requestType: Constants.REQUEST_TYPES.GET, -}; - const newRole1 = { name: "newRole", routes: [ - newRoute1, + Routes.hackerRoutes.getSelf, + Routes.hackerRoutes.getSelfById, ] }; From 07128366b3136ff1395993a8a170b61ceec102ef Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Thu, 20 Dec 2018 12:25:28 -0800 Subject: [PATCH 22/25] Remove duplicate check --- constants/routes.constant.js | 2 +- middlewares/role.middleware.js | 15 +---------- services/role.service.js | 47 +--------------------------------- tests/role.test.js | 8 +++--- 4 files changed, 7 insertions(+), 65 deletions(-) diff --git a/constants/routes.constant.js b/constants/routes.constant.js index 34d3ece3..4c77e2b6 100644 --- a/constants/routes.constant.js +++ b/constants/routes.constant.js @@ -62,7 +62,7 @@ const accountRoutes = { const hackerRoutes = { "getSelf": { requestType: Constants.REQUEST_TYPES.GET, - uri: "/api/hacker/self/" + uri: "/api/hacker/self/", }, "getSelfById": { requestType: Constants.REQUEST_TYPES.GET, diff --git a/middlewares/role.middleware.js b/middlewares/role.middleware.js index 469af01e..08e6ee1b 100644 --- a/middlewares/role.middleware.js +++ b/middlewares/role.middleware.js @@ -39,24 +39,11 @@ function parseRole(req, res, next) { * @param {(err?)=>void} next * @return {void} * @description - * Creates role document after making sure there is no other role with the same routes + * Creates role document */ async function createRole(req, res, next) { const roleDetails = req.body.roleDetails; - // check for duplicate roles - if (await Services.Role.isDuplicate(roleDetails)) { - // get by the route - const existingRole = await Services.Role.getByRoutes(roleDetails.routes); - - return res.status(422).json({ - message: Constants.Error.ROLE_DUPLICATE_422_MESSAGE, - data: { - role: existingRole, - } - }); - } - const role = await Services.Role.createRole(roleDetails); if (!!role) { diff --git a/services/role.service.js b/services/role.service.js index 1bfa4256..79d05969 100644 --- a/services/role.service.js +++ b/services/role.service.js @@ -1,6 +1,7 @@ "use strict"; const Role = require("../models/role.model"); const logger = require("./logger.service"); +const mongoose = require("mongoose"); /** * @function createRole @@ -14,50 +15,6 @@ function createRole(roleDetails) { return role.save(); } -/** - * @function isDuplicate - * @param {{_id: ObjectId, name: String, routes: route[]}} roleDetails - * @return {boolean} True or false depending on whether the routes are duplicated - * @description Check that the routes contained in routeDetails aren't already specified - */ -async function isDuplicate(roleDetails) { - const existRole = await getByRoutes(roleDetails.routes); - - const all = await getAll(); - - if (!!existRole) { - return true; - } - - return false; -} - -/** - * @function routeEquals - * @param {{uri: String, requestType: Enum}} route1 - * @param {{uri: String, requestType: Enum}} route2 - * @return {boolean} Returns whether route1 is the same as route2 - * @description Checks that route1 and route2 are the same - */ -function routeEquals(route1, route2) { - return route1.uri === route2.uri && route1.requestType === route2.requestType; -} - -/** - * @function getByRoutes - * @param {route[]} routes - * @returns {Promise} The promise will resolve to a role object if successful. - * @description finds a role object by the routes - */ -function getByRoutes(routes) { - const TAG = "[Role Service # getByRoutes]:"; - const query = { - routes: routes - }; - - return Role.findOne(query, logger.queryCallbackFactory(TAG, "role", query)); -} - /** * @function getRole * @param {string} roleName The name of the role that you're looking for. @@ -101,10 +58,8 @@ function getAll() { } module.exports = { - getByRoutes: getByRoutes, getRole: getRole, getById: getById, getAll: getAll, createRole: createRole, - isDuplicate: isDuplicate, }; \ No newline at end of file diff --git a/tests/role.test.js b/tests/role.test.js index cf582187..1be51782 100644 --- a/tests/role.test.js +++ b/tests/role.test.js @@ -93,8 +93,8 @@ describe("POST create role", function () { }); }); - // should fail due to duplicate routes - it("should Fail to add new role due to duplicate role", function (done) { + // should succeed despite duplicate routes + it("should Suceed to add new role despite to duplicate routes", function (done) { util.auth.login(agent, util.account.Admin1, (error) => { if (error) { agent.close(); @@ -105,10 +105,10 @@ describe("POST create role", function () { .type("application/json") .send(util.role.duplicateRole1) .end(function (err, res) { - res.should.have.status(422); + res.should.have.status(200); res.should.be.json; res.body.should.have.property("message"); - res.body.message.should.equal(Constants.Error.ROLE_DUPLICATE_422_MESSAGE); + res.body.message.should.equal(Constants.Success.ROLE_CREATE); done(); }); }); From 295a157420fb69c20a45bea975622072ff14f526 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Thu, 27 Dec 2018 14:14:11 -0800 Subject: [PATCH 23/25] Fix typos and merge constants --- constants/success.constant.js | 5 ++++- constants/success.constants.js | 7 ------- middlewares/role.middleware.js | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) delete mode 100644 constants/success.constants.js diff --git a/constants/success.constant.js b/constants/success.constant.js index 0e9a335b..c7831939 100644 --- a/constants/success.constant.js +++ b/constants/success.constant.js @@ -23,10 +23,11 @@ const HACKER_UPDATE = "Hacker update successful."; const RESUME_UPLOAD = "Resume upload successful."; const RESUME_DOWNLOAD = "Resume download successful."; +const ROLE_CREATE = "Role creation successful."; + const SEARCH_QUERY = "Query search successful. Returning results."; const SEARCH_NO_RESULTS = "Query search successful. No results found."; - const SPONSOR_GET_BY_ID = "Sponsor found by id."; const SPONSOR_CREATE = "Sponsor creation successful."; @@ -59,6 +60,8 @@ module.exports = { RESUME_UPLOAD: RESUME_UPLOAD, RESUME_DOWNLOAD: RESUME_DOWNLOAD, + ROLE_CREATE: ROLE_CREATE, + SEARCH_QUERY: SEARCH_QUERY, SEARCH_NO_RESULTS: SEARCH_NO_RESULTS, diff --git a/constants/success.constants.js b/constants/success.constants.js deleted file mode 100644 index 2c563528..00000000 --- a/constants/success.constants.js +++ /dev/null @@ -1,7 +0,0 @@ -"use strict"; - -const ROLE_CREATE = "Role creation successful."; - -module.exports = { - ROLE_CREATE: ROLE_CREATE, -}; \ No newline at end of file diff --git a/middlewares/role.middleware.js b/middlewares/role.middleware.js index 08e6ee1b..8418e1d0 100644 --- a/middlewares/role.middleware.js +++ b/middlewares/role.middleware.js @@ -53,7 +53,7 @@ async function createRole(req, res, next) { } else { return next({ status: 500, - message: Constants.Error.HACKER_CREATE_500_MESSAGE, + message: Constants.Error.ROLE_CREATE_500_MESSAGE, data: {} }); } From 37d9bc61b73a8e26ac285de6fef80d0535c04d9c Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Thu, 27 Dec 2018 14:27:39 -0800 Subject: [PATCH 24/25] switch names to success.constant --- controllers/role.controller.js | 2 +- tests/role.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/role.controller.js b/controllers/role.controller.js index 17fe40c3..ac410feb 100644 --- a/controllers/role.controller.js +++ b/controllers/role.controller.js @@ -1,6 +1,6 @@ "use strict"; -const Success = require("../constants/success.constants"); +const Success = require("../constants/success.constant"); /** * @function createdRole diff --git a/tests/role.test.js b/tests/role.test.js index 1be51782..dc811e59 100644 --- a/tests/role.test.js +++ b/tests/role.test.js @@ -15,7 +15,7 @@ const util = { const Constants = { Error: require("../constants/error.constant"), - Success: require("../constants/success.constants"), + Success: require("../constants/success.constant"), }; describe("POST create role", function () { From 487b94c0b84f1f3b38a357b6f45caffe081b9dd0 Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Fri, 28 Dec 2018 20:58:24 -0500 Subject: [PATCH 25/25] Fix typo --- middlewares/validators/validator.helper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/middlewares/validators/validator.helper.js b/middlewares/validators/validator.helper.js index bc9c6afd..86fd6d84 100644 --- a/middlewares/validators/validator.helper.js +++ b/middlewares/validators/validator.helper.js @@ -625,7 +625,7 @@ function routesValidator(fieldLocation, fieldname, optional = true) { } else { return routes .exists() - .withMessage("The value being checked agains the enums must exist.") + .withMessage("The route must exist.") .custom(routesArrayValidationHelper).withMessage("The value must be a route"); } }