From ce931cf97bfc85da824919e253b4889719c4b962 Mon Sep 17 00:00:00 2001 From: Sergio Neves Barros Date: Sat, 18 Mar 2023 13:22:08 +0000 Subject: [PATCH] Adding returns back in (#63) * adding returns back in. Two more API's to handle. * final bits on handling errors * fixed create execution test and cleanup which was removing all executions * disabling test to check pipeline. --- app/controllers/angles.controller.js | 8 +- app/controllers/baseline.controller.js | 44 +++---- app/controllers/build.controller.js | 90 ++++++------- app/controllers/environment.controller.js | 50 +++----- app/controllers/execution.controller.js | 87 +++++-------- app/controllers/metrics.controller.js | 10 +- app/controllers/phase.controller.js | 52 +++----- app/controllers/screenshot.controller.js | 149 ++++++++-------------- app/controllers/team.controller.js | 64 ++++------ app/exceptions/errors.js | 9 +- test/execution.tests.js | 4 +- 11 files changed, 220 insertions(+), 347 deletions(-) diff --git a/app/controllers/angles.controller.js b/app/controllers/angles.controller.js index 53cf132..13b7c6b 100644 --- a/app/controllers/angles.controller.js +++ b/app/controllers/angles.controller.js @@ -1,15 +1,17 @@ const { validationResult } = require('express-validator'); const debug = require('debug'); const mongoose = require('mongoose'); +// eslint-disable-next-line import/extensions const { version } = require('../../package.json'); const { handleError } = require('../exceptions/errors.js'); +// eslint-disable-next-line no-unused-vars const log = debug('angles:controller'); exports.versions = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } return mongoose.connection.db.command({ buildInfo: 1 }) .then((info) => { @@ -18,8 +20,8 @@ exports.versions = (req, res) => { mongo: info.version, angles: version, }; - res.status(200).send(response); + return res.status(200).send(response); }).catch((err) => { - handleError(err, res); + return handleError(err, res); }); }; diff --git a/app/controllers/baseline.controller.js b/app/controllers/baseline.controller.js index 7e6ada6..502b1d6 100644 --- a/app/controllers/baseline.controller.js +++ b/app/controllers/baseline.controller.js @@ -18,7 +18,7 @@ exports.create = (req, res) => { // check the request is valid const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { screenshotId, view: requestView, ignoreBoxes } = req.body; @@ -27,7 +27,7 @@ exports.create = (req, res) => { Screenshot.findById(screenshotId).exec(), Baseline.find({ view: requestView }).exec(), ]; - Promise.all(promises) + return Promise.all(promises) .then((results) => { const screenshot = results[0]; const baselinesFound = results[1]; @@ -64,16 +64,14 @@ exports.create = (req, res) => { }) .then((savedBaseline) => { log(`Created baseline with id "${savedBaseline._id}" for view "${savedBaseline.view}" and platorm "${savedBaseline.platformName}"`); - res.status(201).send(savedBaseline); - }).catch((err) => { - handleError(err, res); - }); + return res.status(201).send(savedBaseline); + }).catch((err) => handleError(err, res)); }; exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { view, @@ -91,37 +89,35 @@ exports.findAll = (req, res) => { if (browserName) baseLineQuery['platform.browserName'] = browserName; if (screenHeight) baseLineQuery.screenHeight = screenHeight; if (screenWidth) baseLineQuery.screenWidth = screenWidth; - Baseline.find(baseLineQuery) + return Baseline.find(baseLineQuery) .populate('screenshot') .then((baselines) => { - res.status(200).send(baselines); + return res.status(200).send(baselines); }) .catch((err) => { - handleError(err, res); + return handleError(err, res); }); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { baselineId } = req.query; - Baseline.findById(baselineId) + return Baseline.findById(baselineId) .then((baseline) => { if (!baseline) { throw new NotFoundError(`Baseline not found with id ${baselineId}`); } - res.status(200).send(baseline); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(baseline); + }).catch((err) => handleError(err, res)); }; exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { screenshotId, ignoreBoxes } = req.body; let screenshotPromise = new Promise((resolve) => { resolve(); }); @@ -134,7 +130,7 @@ exports.update = (req, res) => { screenshotPromise, Baseline.findById(baselineId).exec(), ]; - Promise.all(promises) + return Promise.all(promises) .then((results) => { const screenshot = results[0]; const baselineFound = results[1]; @@ -158,26 +154,26 @@ exports.update = (req, res) => { }) .then((savedBaseline) => savedBaseline.populate('screenshot')) .then((savedBaselineWithScreenshot) => { - res.status(200).send(savedBaselineWithScreenshot); + return res.status(200).send(savedBaselineWithScreenshot); }) .catch((err) => { - handleError(err, res); + return handleError(err, res); }); }; exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { baselineId } = req.params; - Baseline.findByIdAndRemove(baselineId) + return Baseline.findByIdAndRemove(baselineId) .then((baseline) => { if (!baseline) { throw new NotFoundError(`Baseline not found with id ${baselineId}`); } - res.status(200).send({ message: 'Baseline deleted successfully!' }); + return res.status(200).send({ message: 'Baseline deleted successfully!' }); }).catch((err) => { - handleError(err, res); + return handleError(err, res); }); }; diff --git a/app/controllers/build.controller.js b/app/controllers/build.controller.js index 9ddad62..b4c2ec4 100644 --- a/app/controllers/build.controller.js +++ b/app/controllers/build.controller.js @@ -20,7 +20,7 @@ const log = debug('build:controller'); exports.create = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { team, @@ -39,7 +39,7 @@ exports.create = (req, res) => { phasePromise, ]; - Promise.all(promises) + return Promise.all(promises) .then((results) => { const teamFound = results[0]; const environmentFound = results[1]; @@ -92,17 +92,15 @@ exports.create = (req, res) => { .exec()) .then((savedBuild) => { log(`Created build "${savedBuild.name}" for team "${savedBuild.team.name}" with id: ${savedBuild._id}`); - res.status(201).send(savedBuild); + return res.status(201).send(savedBuild); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { teamId, @@ -116,7 +114,7 @@ exports.findAll = (req, res) => { const limit = parseInt(req.query.limit, 10) || 10; const skip = parseInt(req.query.skip, 10) || 0; let query = {}; - Team.findById({ _id: teamId }) + return Team.findById({ _id: teamId }) .then((teamFound) => { if (!teamFound) { throw new NotFoundError(`No team found with name ${req.body.team}`); @@ -170,20 +168,18 @@ exports.findAll = (req, res) => { const builds = results[0]; const count = results[1]; const response = { builds, count }; - res.status(200).send(response); + return res.status(200).send(response); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId } = req.params; - Build.findById(buildId) + return Build.findById(buildId) .populate('team') .populate('environment') .populate('phase') @@ -192,21 +188,19 @@ exports.findOne = (req, res) => { if (!build) { throw new NotFoundError(`No build found with id ${buildId}`); } - res.status(200).send(build); + return res.status(200).send(build); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.getReport = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } let build; const { buildId } = req.params; - Build.findById(req.params.buildId) + return Build.findById(req.params.buildId) .populate('team') .populate('environment') .populate('phase') @@ -222,22 +216,20 @@ exports.getReport = (req, res) => { }) .then((screenshots) => { // eslint-disable-next-line global-require - res.render('index', { build, screenshots, moment: require('moment') }); + return res.render('index', { build, screenshots, moment: require('moment') }); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; // TODO: We should be able to update more than just team and/or environment. exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId } = req.params; const { team, environment } = req.body; - Build.findByIdAndUpdate(buildId, { + return Build.findByIdAndUpdate(buildId, { team, environment, }, { new: true }) @@ -245,39 +237,35 @@ exports.update = (req, res) => { if (!build) { throw new NotFoundError(`No build found with id ${buildId}`); } - res.status(200).send(build); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(build); + }).catch((err) => handleError(err, res)); }; exports.setKeep = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId } = req.params; const { keep } = req.body; - Build.findByIdAndUpdate(buildId, { + return Build.findByIdAndUpdate(buildId, { keep, }, { new: true }) .then((build) => { if (!build) { throw new NotFoundError(`No build found with id ${buildId}`); } - res.status(200).send(build); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(build); + }).catch((err) => handleError(err, res)); }; exports.setArtifacts = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId } = req.params; - Build.findByIdAndUpdate(buildId, { + return Build.findByIdAndUpdate(buildId, { artifacts: req.body.artifacts, }, { new: true }) .populate('team') @@ -287,11 +275,9 @@ exports.setArtifacts = (req, res) => { if (!build) { throw new NotFoundError(`No build found with id ${buildId}`); } - res.status(200).send(build); + return res.status(200).send(build); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; /* @@ -303,30 +289,28 @@ exports.setArtifacts = (req, res) => { exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId } = req.params; - Build.findByIdAndRemove(buildId) + return Build.findByIdAndRemove(buildId) .then((build) => { if (!build) { throw new NotFoundError(`No build found with id ${buildId}`); } - res.status(200).send({ message: 'Build deleted successfully!' }); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send({ message: 'Build deleted successfully!' }); + }).catch((err) => handleError(err, res)); }; exports.deleteMany = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } let allBuildsToDelete; let reportingMetrics = {}; const { teamId, ageInDays } = req.query; // delete by team and age (or default age 2 months) unless keep flag - Team.findById({ _id: teamId }) + return Team.findById({ _id: teamId }) .then((teamFound) => { if (!teamFound) { throw new NotFoundError(`No team found with id ${teamId}`); @@ -373,9 +357,7 @@ exports.deleteMany = (req, res) => { }) .then((results) => { log(results); - res.status(200).send({ message: `Deleted [${reportingMetrics.buildsToDeleteLength}] for team with id ${teamId} and age ${ageInDays}. Unable to delete ${reportingMetrics.uniqueBaselineBuildIdsLength} builds as they have baselines set.` }); + return res.status(200).send({ message: `Deleted [${reportingMetrics.buildsToDeleteLength}] for team with id ${teamId} and age ${ageInDays}. Unable to delete ${reportingMetrics.uniqueBaselineBuildIdsLength} builds as they have baselines set.` }); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; diff --git a/app/controllers/environment.controller.js b/app/controllers/environment.controller.js index 2977d94..b1fa084 100644 --- a/app/controllers/environment.controller.js +++ b/app/controllers/environment.controller.js @@ -9,11 +9,11 @@ const log = debug('environment:controller'); exports.create = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { name } = req.body; - Environment.findOne({ name }).exec() + return Environment.findOne({ name }).exec() .then((existingEnvironment) => { if (existingEnvironment) { throw new ConflictError(`Environment with name ${name} already exists.`); @@ -25,52 +25,46 @@ exports.create = (req, res) => { }) .then((data) => { log(`Created environment "${data.name}" with id: "${data._id}".`); - res.status(201).send(data); + return res.status(201).send(data); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } - Environment.find() + return Environment.find() .then((environments) => { - res.status(200).send(environments); + return res.status(200).send(environments); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { environmentId } = req.params; - Environment.findById(environmentId) + return Environment.findById(environmentId) .then((environment) => { if (!environment) { throw new NotFoundError(`Environment not found with id ${environmentId}`); } - res.send(environment); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(environment); + }).catch((err) => handleError(err, res)); }; exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { name } = req.body; const { environmentId } = req.params; - Environment.findOne({ name }) + return Environment.findOne({ name }) .then((existingEnvironment) => { if (existingEnvironment) { throw new ConflictError(`Environment with name ${name} already exists.`); @@ -83,26 +77,22 @@ exports.update = (req, res) => { if (!environment) { throw new NotFoundError(`Environment not found with id ${environmentId}`); } - res.status(200).send(environment); + return res.status(200).send(environment); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { environmentId } = req.params; - Environment.findByIdAndRemove(environmentId) + return Environment.findByIdAndRemove(environmentId) .then((environment) => { if (!environment) { throw new NotFoundError(`Environment not found with id ${environmentId}`); } - res.status(200).send({ message: 'Environment deleted successfully!' }); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send({ message: 'Environment deleted successfully!' }); + }).catch((err) => handleError(err, res)); }; diff --git a/app/controllers/execution.controller.js b/app/controllers/execution.controller.js index aa0077e..96accb2 100644 --- a/app/controllers/execution.controller.js +++ b/app/controllers/execution.controller.js @@ -12,11 +12,11 @@ exports.create = (req, res) => { // check the request is valid const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } let testExecution; const { build: buildId } = req.body; - Build.findById(buildId) + return Build.findById(buildId) .populate('suites.executions') .then((buildFound) => { if (!buildFound) { @@ -32,21 +32,19 @@ exports.create = (req, res) => { .then((savedBuild) => { testExecution.build = savedBuild._id; log(`Created test "${testExecution.title}", suite "${testExecution.suite}" build "${testExecution.build}", with id: "${testExecution._id}"`); - res.status(201).send(testExecution); + return res.status(201).send(testExecution); }) - .catch((error) => { - handleError(error, res); - }); + .catch((error) => handleError(error, res)); }; exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId, executionIds } = req.query; if (buildId) { - Build.findById(buildId) + return Build.findById(buildId) // .populate('suites.executions') .then((buildFound) => { if (!buildFound) { @@ -59,49 +57,38 @@ exports.findAll = (req, res) => { } return TestExecution.find(query); }) - .then((executionsFound) => { - res.status(200).send(executionsFound); - }) - .catch((err) => { - handleError(err, res); - }); - } else { - // if no buildId provided - const executionIdArray = executionIds.split(','); - TestExecution.find({ _id: { $in: executionIdArray } }) - .then((testExecutions) => { - res.status(200).send(testExecutions); - }) - .catch((err) => { - handleError(err, res); - }); + .then((executionsFound) => res.status(200).send(executionsFound)) + .catch((err) => handleError(err, res)); } + // if no buildId provided + const executionIdArray = executionIds.split(','); + return TestExecution.find({ _id: { $in: executionIdArray } }) + .then((testExecutions) => res.status(200).send(testExecutions)) + .catch((err) => handleError(err, res)); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { executionId } = req.params; - TestExecution.findById(executionId) + return TestExecution.findById(executionId) .then((testExecution) => { if (!testExecution) { throw new NotFoundError(`Execution not found with id ${executionId}`); } - res.status(200).send(testExecution); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(testExecution); + }).catch((err) => handleError(err, res)); }; exports.findHistory = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { executionId } = req.params; - TestExecution.findById(executionId) + return TestExecution.findById(executionId) .populate('build') .then((testExecution) => { if (!testExecution) { @@ -131,66 +118,58 @@ exports.findHistory = (req, res) => { const executions = results[0]; const count = results[1]; const response = { executions, count }; - res.status(200).send(response); + return res.status(200).send(response); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { executionId } = req.params; const { name } = req.body; - TestExecution.findByIdAndUpdate(executionId, { + return TestExecution.findByIdAndUpdate(executionId, { name, }, { new: true }) .then((testExecution) => { if (!testExecution) { throw new NotFoundError(`Execution not found with id ${executionId}`); } - res.status(200).send(testExecution); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(testExecution); + }).catch((err) => handleError(err, res)); }; exports.setPlatform = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { executionId } = req.params; const { platforms } = req.body; - TestExecution.findByIdAndUpdate(executionId, { + return TestExecution.findByIdAndUpdate(executionId, { platforms, }, { new: true }) .then((execution) => { if (!execution) { throw new NotFoundError(`Execution not found with id ${executionId}`); } - res.status(200).send(execution); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(execution); + }).catch((err) => handleError(err, res)); }; exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { executionId } = req.params; - TestExecution.findByIdAndRemove(executionId) + return TestExecution.findByIdAndRemove(executionId) .then((testExecution) => { if (!testExecution) { throw new NotFoundError(`Execution not found with id ${executionId}`); } - res.status(200).send({ message: 'Test execution deleted successfully!' }); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send({ message: 'Test execution deleted successfully!' }); + }).catch((err) => handleError(err, res)); }; diff --git a/app/controllers/metrics.controller.js b/app/controllers/metrics.controller.js index f267573..8a3e8c7 100644 --- a/app/controllers/metrics.controller.js +++ b/app/controllers/metrics.controller.js @@ -51,7 +51,7 @@ const log = debug('metrics:controller'); exports.retrieveMetricsPerPhase = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const metrics = {}; const { @@ -62,7 +62,7 @@ exports.retrieveMetricsPerPhase = (req, res) => { groupingPeriod, } = req.query; let query = {}; - Team.findById({ _id: teamId }) + return Team.findById({ _id: teamId }) .then((teamFound) => { if (!teamFound) { throw new NotFoundError(`No team found with id ${teamId}`); @@ -224,11 +224,9 @@ exports.retrieveMetricsPerPhase = (req, res) => { delete period.items; delete period.buildIds; }); - res.status(200).send(metrics); + return res.status(200).send(metrics); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; /** diff --git a/app/controllers/phase.controller.js b/app/controllers/phase.controller.js index 1b3bf1d..2680b2f 100644 --- a/app/controllers/phase.controller.js +++ b/app/controllers/phase.controller.js @@ -8,10 +8,10 @@ const log = debug('phase:controller'); exports.create = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { name, orderNumber } = req.body; - Phase + return Phase .findOne({ name }) .then((existingPhase) => { if (existingPhase) { @@ -27,48 +27,40 @@ exports.create = (req, res) => { }) .then((data) => { log(`Created phase "${name}" with id: "${data._id}"`); - res.status(201).send(data); - }).catch((err) => { - handleError(err, res); - }); + return res.status(201).send(data); + }).catch((err) => handleError(err, res)); }; exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } - Phase.find({}) + return Phase.find({}) .sort({ orderNumber: 1 }) - .then((phases) => { - res.status(200).send(phases); - }) - .catch((err) => { - handleError(err, res); - }); + .then((phases) => res.status(200).send(phases)) + .catch((err) => handleError(err, res)); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { phaseId } = req.params; - Phase.findById(phaseId) + return Phase.findById(phaseId) .then((phase) => { if (!phase) { throw new NotFoundError(`Phase not found with id ${phaseId}`); } - res.status(200).send(phase); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(phase); + }).catch((err) => handleError(err, res)); }; exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { name, orderNumber } = req.body; const { phaseId } = req.params; @@ -86,7 +78,7 @@ exports.update = (req, res) => { } else { promise = Promise.resolve(true); } - promise + return promise .then(() => { if (orderNumber) updateRequest.orderNumber = orderNumber; return Phase.findByIdAndUpdate(phaseId, updateRequest, { new: true }); @@ -95,26 +87,22 @@ exports.update = (req, res) => { if (!phase) { throw new NotFoundError(`Phase not found with id ${req.params.phaseId}`); } - res.status(200).send(phase); + return res.status(200).send(phase); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { phaseId } = req.params; - Phase.findByIdAndRemove(phaseId) + return Phase.findByIdAndRemove(phaseId) .then((phase) => { if (!phase) { throw new NotFoundError(`Phase not found with id ${phaseId}`); } - res.status(200).send({ message: 'Phase deleted successfully!' }); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send({ message: 'Phase deleted successfully!' }); + }).catch((err) => handleError(err, res)); }; diff --git a/app/controllers/screenshot.controller.js b/app/controllers/screenshot.controller.js index b21811e..504a220 100644 --- a/app/controllers/screenshot.controller.js +++ b/app/controllers/screenshot.controller.js @@ -46,7 +46,7 @@ exports.create = (req, res) => { // run validation here. const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } let build; const { @@ -55,7 +55,7 @@ exports.create = (req, res) => { view, tags, } = req.body; - Build.findById(buildId) + return Build.findById(buildId) .then((foundBuild) => { if (!foundBuild) { throw new NotFoundError(`No build found with id ${buildId}`); @@ -97,21 +97,18 @@ exports.create = (req, res) => { }) .then((savedScreenshot) => { log(`Created screenshot "${savedScreenshot.path}", view "${savedScreenshot.view}" build "${savedScreenshot.build}", with id: "${savedScreenshot._id}"`); - res.status(201).send(savedScreenshot); + return res.status(201).send(savedScreenshot); }) - .catch((error) => { - handleError(error, res); - }); + .catch((error) => handleError(error, res)); }; -exports.createFail = (error, req, res) => { - res.status(400).send({ error: error.message }); -}; +exports.createFail = (error, req, res) => res.status(400).send({ error: error.message }); +// TODO: check query exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { buildId, @@ -152,25 +149,21 @@ exports.findAll = (req, res) => { findScreenshotsPromise = Screenshot.find(query, null, options); } - findScreenshotsPromise - .then((screenshots) => { - res.status(200).send(screenshots); - }) - .catch((err) => { - handleError(err, res); - }); + return findScreenshotsPromise + .then((screenshots) => res.status(200).send(screenshots)) + .catch((err) => handleError(err, res)); }; /* This method will find the latest image for a specific view on every unique platform */ exports.findLatestForViewGroupedByPlatform = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { view, numberOfDays } = req.query; const searchDate = new Date(); searchDate.setDate(searchDate.getDate() - numberOfDays); - Screenshot.aggregate([ + return Screenshot.aggregate([ { $match: { view, createdAt: { $gt: searchDate } } }, { $sort: { _id: 1 } }, { $group: { _id: { view: '$view', platformId: '$platformId' }, lastId: { $last: '$_id' } } }, @@ -180,23 +173,19 @@ exports.findLatestForViewGroupedByPlatform = (req, res) => { const latestScreenshotIds = screenshotsIdsArray.map(({ _id }) => _id); return Screenshot.find({ _id: { $in: latestScreenshotIds } }); }) - .then((screenshots) => { - res.status(200).send(screenshots); - }) - .catch((err) => { - handleError(err, res); - }); + .then((screenshots) => res.status(200).send(screenshots)) + .catch((err) => handleError(err, res)); }; exports.findLatestForTagGroupedByView = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { tag, numberOfDays } = req.query; const searchDate = new Date(); searchDate.setDate(searchDate.getDate() - numberOfDays); - Screenshot.aggregate([ + return Screenshot.aggregate([ { $match: { tags: { $in: [tag] }, createdAt: { $gt: searchDate } } }, { $sort: { view: 1, _id: 1 } }, { $group: { _id: { view: '$view', platformId: '$platformId' }, lastId: { $last: '$_id' } } }, @@ -206,53 +195,45 @@ exports.findLatestForTagGroupedByView = (req, res) => { const latestScreenshotIds = screenshotsIdsArray.map(({ _id }) => _id); return Screenshot.find({ _id: { $in: latestScreenshotIds } }); }) - .then((screenshots) => { - res.status(200).send(screenshots); - }) - .catch((err) => { - handleError(err, res); - }); + .then((screenshots) => res.status(200).send(screenshots)) + .catch((err) => handleError(err, res)); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { screenshotId } = req.params; - Screenshot.findById(screenshotId) + return Screenshot.findById(screenshotId) .then((screenshot) => { if (!screenshot) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); } - res.status(200).send(screenshot); + return res.status(200).send(screenshot); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.findOneImage = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { screenshotId } = req.params; - Screenshot.findById(screenshotId) + return Screenshot.findById(screenshotId) .then((screenshot) => { if (!screenshot) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); } - res.sendFile(path.resolve(`${screenshot.path}`)); - }).catch((err) => { - handleError(err, res); - }); + return res.sendFile(path.resolve(`${screenshot.path}`)); + }).catch((err) => handleError(err, res)); }; exports.compareImages = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } // find both images const { screenshotId, screenshotCompareId } = req.params; @@ -260,7 +241,7 @@ exports.compareImages = (req, res) => { Screenshot.findById(screenshotId).exec(), Screenshot.findById(screenshotCompareId).exec(), ]; - Promise.all(promises).then((results) => { + return Promise.all(promises).then((results) => { const screenshot = results[0]; const screenshotCompare = results[1]; const options = { @@ -273,17 +254,15 @@ exports.compareImages = (req, res) => { if (err) { throw new ServerError(`Something went wrong comparing the images ${err}`); } - res.status(200).send(data); + return res.status(200).send(data); }); - }).catch((err) => { - handleError(err, res); - }); + }).catch((err) => handleError(err, res)); }; exports.compareImagesAndReturnImage = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } // find both images const promises = [ @@ -293,7 +272,7 @@ exports.compareImagesAndReturnImage = (req, res) => { const { useCache } = req.query; - Promise.all(promises) + return Promise.all(promises) .then((results) => { const screenshot = results[0]; const screenshotToCompare = results[1]; @@ -302,18 +281,14 @@ exports.compareImagesAndReturnImage = (req, res) => { } return imageUtils.compareImages(screenshot, screenshotToCompare, undefined, useCache); }) - .then((tempFileName) => { - res.sendFile(path.resolve(tempFileName)); - }) - .catch((err) => { - handleError(err, res); - }); + .then((tempFileName) => res.sendFile(path.resolve(tempFileName))) + .catch((err) => handleError(err, res)); }; exports.generateDynamicBaselineImage = async (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } // 1. request should contain image, you want to generate baseline @@ -326,7 +301,7 @@ exports.generateDynamicBaselineImage = async (req, res) => { const { numberOfImagesToCompare } = req.query; const queryHistory = numberOfImagesToCompare || 5; let originalScreenshot; - Screenshot.findById(screenshotId) + return Screenshot.findById(screenshotId) .then((screenshot) => { if (!screenshot) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); @@ -356,22 +331,18 @@ exports.generateDynamicBaselineImage = async (req, res) => { return imageUtils.generateDynamicBaseline(originalScreenshot, screenshots); }) .then((baselineScreenshot) => baselineScreenshot.save()) - .then((savedBaselineScreenshot) => { - res.status(201).send(savedBaselineScreenshot); - }) - .catch((err) => { - handleError(err, res); - }); + .then((savedBaselineScreenshot) => res.status(201).send(savedBaselineScreenshot)) + .catch((err) => handleError(err, res)); }; exports.compareImageAgainstBaseline = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } let screenshot; const { screenshotId } = req.params; - Screenshot.findById(screenshotId) + return Screenshot.findById(screenshotId) .then((screenshotFound) => { if (!screenshotFound) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); @@ -432,24 +403,22 @@ exports.compareImageAgainstBaseline = (req, res) => { if (err) { throw new ServerError(`Unable to compare images due to [${err}]`); } - res.status(200).send(data); + return res.status(200).send(data); }, ); }) - .catch((error) => { - handleError(error, res); - }); + .catch((error) => handleError(error, res)); }; exports.compareImageAgainstBaselineAndReturnImage = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { useCache } = req.query; const { screenshotId } = req.params; let screenshotToCompare; - Screenshot.findById(screenshotId) + return Screenshot.findById(screenshotId) .then((screenshot) => { if (!screenshot) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); @@ -493,22 +462,18 @@ exports.compareImageAgainstBaselineAndReturnImage = (req, res) => { }); return imageUtils.compareImages(screenshot, screenshotToCompare, ignoredBoxes, useCache); }) - .then((tempFileName) => { - res.sendFile(tempFileName); - }) - .catch((err) => { - handleError(err, res); - }); + .then((tempFileName) => res.sendFile(tempFileName)) + .catch((err) => handleError(err, res)); }; exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { platform, tags } = req.body; const { screenshotId } = req.params; - Screenshot.findById(screenshotId) + return Screenshot.findById(screenshotId) .then((screenshot) => { if (!screenshot) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); @@ -522,28 +487,22 @@ exports.update = (req, res) => { .generatePlatformId(platformToStore, screenshot); } return screenshotToModify.save(); - }).then((savedScreenshot) => { - res.status(200).send(savedScreenshot); - }) - .catch((err) => { - handleError(err, res); - }); + }).then((savedScreenshot) => res.status(200).send(savedScreenshot)) + .catch((err) => handleError(err, res)); }; exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { screenshotId } = req.params; - Screenshot.findByIdAndRemove(screenshotId) + return Screenshot.findByIdAndRemove(screenshotId) .then((screenshot) => { if (!screenshot) { throw new NotFoundError(`No screenshot found with id ${screenshotId}`); } fs.unlinkSync(screenshot.path); - res.status(200).send({ message: 'Screenshot deleted successfully!' }); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send({ message: 'Screenshot deleted successfully!' }); + }).catch((err) => handleError(err, res)); }; diff --git a/app/controllers/team.controller.js b/app/controllers/team.controller.js index 45608b1..68bda4e 100644 --- a/app/controllers/team.controller.js +++ b/app/controllers/team.controller.js @@ -8,10 +8,10 @@ const log = debug('team:controller'); exports.create = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { name, components } = req.body; - Team.findOne({ name }).exec() + return Team.findOne({ name }).exec() .then((foundTeam) => { if (foundTeam) { throw new ConflictError(`Team with name ${name} already exists.`); @@ -23,51 +23,43 @@ exports.create = (req, res) => { return team.save(); }).then((savedTeam) => { log(`Created team "${savedTeam.name}" with id: "${savedTeam._id}"`); - res.status(201).send(savedTeam); + return res.status(201).send(savedTeam); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.findAll = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } - Team.find() - .then((teams) => { - res.status(200).send(teams); - }) - .catch((err) => { - handleError(err, res); - }); + return Team.find() + .then((teams) => res.status(200).send(teams)) + .catch((err) => handleError(err, res)); }; exports.findOne = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { teamId } = req.params; - Team.findById(teamId) + return Team.findById(teamId) .then((team) => { if (!team) { throw new NotFoundError(`Team not found with id ${teamId}`); } - res.status(200).send(team); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send(team); + }).catch((err) => handleError(err, res)); }; exports.update = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { name, teamId } = req.body; - Team.where({ name }).findOne() + return Team.where({ name }).findOne() .then((foundTeam) => { if (foundTeam) { throw new ConflictError(`Team with name ${name} already exists.`); @@ -79,21 +71,19 @@ exports.update = (req, res) => { if (!team) { throw new NotFoundError(`Team not found with id ${teamId}`); } - res.status(200).send(team); + return res.status(200).send(team); }) - .catch((err) => { - handleError(err, res); - }); + .catch((err) => handleError(err, res)); }; exports.addComponents = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { teamId } = req.params; const { components } = req.body; - Team.findById(teamId) + return Team.findById(teamId) .then((team) => { if (!team) { throw new NotFoundError(`Team not found with id ${teamId}`); @@ -103,27 +93,21 @@ exports.addComponents = (req, res) => { }); return team.save(); }) - .then((savedTeam) => { - res.status(200).send(savedTeam); - }) - .catch((err) => { - handleError(err, res); - }); + .then((savedTeam) => res.status(200).send(savedTeam)) + .catch((err) => handleError(err, res)); }; exports.delete = (req, res) => { const errors = validationResult(req); if (!errors.isEmpty()) { - res.status(422).json({ errors: errors.array() }); + return res.status(422).json({ errors: errors.array() }); } const { teamId } = req.params; - Team.findByIdAndRemove(teamId) + return Team.findByIdAndRemove(teamId) .then((team) => { if (!team) { throw new NotFoundError(`Team not found with id ${teamId}`); } - res.status(200).send({ message: 'Team deleted successfully!' }); - }).catch((err) => { - handleError(err, res); - }); + return res.status(200).send({ message: 'Team deleted successfully!' }); + }).catch((err) => handleError(err, res)); }; diff --git a/app/exceptions/errors.js b/app/exceptions/errors.js index a23e5ab..6077acb 100644 --- a/app/exceptions/errors.js +++ b/app/exceptions/errors.js @@ -39,14 +39,9 @@ const handleError = (error, res) => { if (error.statusCode) { const { statusCode, message } = error; res.status(statusCode).send({ message }); - } else { - const message = error.message || 'Server Error'; - if (!res.headersSent) { - res.status(500).send({ message }); - } else { - console.log(`${message}, headersSent: ${res.headersSent}`); - } } + const message = error.message || 'Server Error'; + return res.status(500).send({ message }); }; module.exports = { diff --git a/test/execution.tests.js b/test/execution.tests.js index 678f984..a40063e 100644 --- a/test/execution.tests.js +++ b/test/execution.tests.js @@ -18,9 +18,9 @@ let testbuild = null; describe('Execution API Tests', () => { before((done) => { // do the setup required for all tests - Execution.deleteMany({ name: /^unit-testing/ }, (err) => { + Execution.deleteMany({ title: { $regex: /^unit-testing/ } }, (err) => { if (err) { - logger.error('Error occured whilst clearing test executions', err); + logger.error('Error occurred whilst clearing test executions', err); } else { logger.info('Cleared any lingering test executions'); }