Fix/be/refactor monitor controller

Client/src/Pages/Monitors/Details/index.jsx

@@ -95,6 +95,7 @@ const DetailsPage = ({ isAdmin }) => {
 					setCertificateExpiry(formatDateWithTz(date, dateFormat, uiTimezone) ?? "N/A");
 				}
 			} catch (error) {
+				setCertificateExpiry("N/A");
 				console.error(error);
 			}
 		};

Server/controllers/controllerUtils.js

@@ -17,6 +17,10 @@ const fetchMonitorCertificate = async (sslChecker, monitor) => {
 		const monitorUrl = new URL(monitor.url);
 		const hostname = monitorUrl.hostname;
 		const cert = await sslChecker(hostname);
+		// Throw an error if no cert or if cert.validTo is not present
+		if (cert?.validTo === null || cert?.validTo === undefined) {
+			throw new Error("Certificate not found");
+		}
 		return cert;
 	} catch (error) {
 		throw error;

Server/controllers/monitorController.js

@@ -3,7 +3,7 @@ import {
 	getMonitorByIdQueryValidation,
 	getMonitorsByTeamIdValidation,
 	createMonitorBodyValidation,
-  getMonitorURLByQueryValidation,
+	getMonitorURLByQueryValidation,
 	editMonitorBodyValidation,
 	getMonitorsAndSummaryByTeamIdParamValidation,
 	getMonitorsAndSummaryByTeamIdQueryValidation,
@@ -86,21 +86,14 @@ const getMonitorCertificate = async (req, res, next, fetchMonitorCertificate) =>
 		const { monitorId } = req.params;
 		const monitor = await req.db.getMonitorById(monitorId);
 		const certificate = await fetchMonitorCertificate(sslChecker, monitor);
-		if (certificate && certificate.validTo) {
-			return res.status(200).json({
-				success: true,
-				msg: successMessages.MONITOR_CERTIFICATE,
-				data: {
-					certificateDate: new Date(certificate.validTo),
-				},
-			});
-		} else {
-			return res.status(200).json({
-				success: true,
-				msg: successMessages.MONITOR_CERTIFICATE,
-				data: { certificateDate: "N/A" },
-			});
-		}
+
+		return res.status(200).json({
+			success: true,
+			msg: successMessages.MONITOR_CERTIFICATE,
+			data: {
+				certificateDate: new Date(certificate.validTo),
+			},
+		});
 	} catch (error) {
 		next(handleError(error, SERVICE_NAME, "getMonitorCertificate"));
 	}
@@ -128,12 +121,6 @@ const getMonitorById = async (req, res, next) => {
 
 	try {
 		const monitor = await req.db.getMonitorById(req.params.monitorId);
-		if (!monitor) {
-			const error = new Error(errorMessages.MONITOR_GET_BY_ID);
-			error.status = 404;
-			throw error;
-		}
-
 		return res.status(200).json({
 			success: true,
 			msg: successMessages.MONITOR_GET_BY_ID,
@@ -238,15 +225,15 @@ const createMonitor = async (req, res, next) => {
 		const notifications = req.body.notifications;
 		const monitor = await req.db.createMonitor(req, res);
 
-		if (notifications && notifications.length !== 0) {
-			monitor.notifications = await Promise.all(
+		monitor.notifications =
+			notifications &&
+			(await Promise.all(
 				notifications.map(async (notification) => {
 					notification.monitorId = monitor._id;
 					await req.db.createNotification(notification);
 				})
-			);
-			await monitor.save();
-		}
+			));
+		await monitor.save();
 		// Add monitor to job queue
 		req.jobQueue.addJob(monitor._id, monitor);
 		return res.status(201).json({
@@ -270,32 +257,32 @@ const createMonitor = async (req, res, next) => {
  * @throws {Error} If there is an error during the process, especially if there is a validation error (422).
  */
 const checkEndpointResolution = async (req, res, next) => {
-  try {
+	try {
 		await getMonitorURLByQueryValidation.validateAsync(req.query);
 	} catch (error) {
 		next(handleValidationError(error, SERVICE_NAME));
 		return;
 	}
 
-  try {
-    let { monitorURL } = req.query;
-    monitorURL = new URL(monitorURL);
-    await new Promise((resolve, reject) => {
-      dns.resolve(monitorURL.hostname, (error) => {
-        if (error) {
-          reject(error);
-        }
-        resolve();
-      });
-    });
-    return res.status(200).json({
-      success: true,
-      msg: `URL resolved successfully`,
-    });
-  } catch (error) {
-    next(handleError(error, SERVICE_NAME, "checkEndpointResolution"));
-  }
-}
+	try {
+		let { monitorURL } = req.query;
+		monitorURL = new URL(monitorURL);
+		await new Promise((resolve, reject) => {
+			dns.resolve(monitorURL.hostname, (error) => {
+				if (error) {
+					reject(error);
+				}
+				resolve();
+			});
+		});
+		return res.status(200).json({
+			success: true,
+			msg: `URL resolved successfully`,
+		});
+	} catch (error) {
+		next(handleError(error, SERVICE_NAME, "checkEndpointResolution"));
+	}
+};
 
 /**
  * Deletes a monitor by its ID and also deletes associated checks, alerts, and notifications.
@@ -415,14 +402,13 @@ const editMonitor = async (req, res, next) => {
 
 		await req.db.deleteNotificationsByMonitorId(editedMonitor._id);
 
-		if (notifications && notifications.length !== 0) {
-			await Promise.all(
+		await Promise.all(
+			notifications &&
 				notifications.map(async (notification) => {
 					notification.monitorId = editedMonitor._id;
 					await req.db.createNotification(notification);
 				})
-			);
-		}
+		);
 
 		// Delete the old job(editedMonitor has the same ID as the old monitor)
 		await req.jobQueue.deleteJob(monitorBeforeEdit);
@@ -458,11 +444,10 @@ const pauseMonitor = async (req, res, next) => {
 
 	try {
 		const monitor = await req.db.getMonitorById(req.params.monitorId);
-		if (monitor.isActive) {
-			await req.jobQueue.deleteJob(monitor);
-		} else {
-			await req.jobQueue.addJob(monitor._id, monitor);
-		}
+		monitor.isActive === true
+			? await req.jobQueue.deleteJob(monitor)
+			: await req.jobQueue.addJob(monitor._id, monitor);
+
 		monitor.isActive = !monitor.isActive;
 		monitor.status = undefined;
 		monitor.save();
@@ -517,7 +502,7 @@ export {
 	getMonitorsAndSummaryByTeamId,
 	getMonitorsByTeamId,
 	createMonitor,
-  checkEndpointResolution,
+	checkEndpointResolution,
 	deleteMonitor,
 	deleteAllMonitors,
 	editMonitor,

Server/db/mongo/modules/monitorModule.js

@@ -288,7 +288,9 @@ const getMonitorById = async (monitorId) => {
 	try {
 		const monitor = await Monitor.findById(monitorId);
 		if (monitor === null || monitor === undefined) {
-			throw new Error(errorMessages.DB_FIND_MONITOR_BY_ID(monitorId));
+			const error = new Error(errorMessages.DB_FIND_MONITOR_BY_ID(monitorId));
+			error.status = 404;
+			throw error;
 		}
 		// Get notifications
 		const notifications = await Notification.find({

Server/tests/controllers/authController.test.js

@@ -467,7 +467,7 @@ describe("Auth Controller - editUser", async () => {
 		});
 		await editUser(req, res, next);
 		expect(next.firstCall.args[0]).to.be.an("error");
-		expect(next.firstCall.args[0].status).to.equal(403);
+		expect(next.firstCall.args[0].status).to.equal(401);
 		expect(next.firstCall.args[0].message).to.equal(
 			errorMessages.AUTH_INCORRECT_PASSWORD
 		);
@@ -812,9 +812,9 @@ describe("Auth Controller - deleteUser", () => {
 	afterEach(() => {
 		sinon.restore();
 	});
-	it("should return 404 if user is not found", async () => {
+	it("should throw an error if user is not found", async () => {
 		jwt.decode.returns({ email: "[email protected]" });
-		req.db.getUserByEmail.resolves(null);
+		req.db.getUserByEmail.throws(new Error(errorMessages.DB_USER_NOT_FOUND));
 
 		await deleteUser(req, res, next);
 

Server/tests/controllers/controllerUtils.test.js

@@ -148,4 +148,11 @@ describe("controllerUtils - fetchMonitorCertificate", () => {
 		const result = await fetchMonitorCertificate(sslChecker, monitor);
 		expect(result).to.deep.equal({ validTo: "2022-01-01" });
 	});
+	it("should throw an error if a ssl-checker returns null", async () => {
+		sslChecker.returns(null);
+		await fetchMonitorCertificate(sslChecker, monitor).catch((error) => {
+			expect(error).to.be.an("error");
+			expect(error.message).to.equal("Certificate not found");
+		});
+	});
 });

Server/tests/controllers/monitorController.test.js

@@ -169,19 +169,6 @@ describe("Monitor Controller - getMonitorCertificate", () => {
 			})
 		).to.be.true;
 	});
-	it("should return success message and data if all operations succeed with an invalid cert", async () => {
-		req.db.getMonitorById.returns({ url: "https://www.google.com" });
-		fetchMonitorCertificate.returns({});
-		await getMonitorCertificate(req, res, next, fetchMonitorCertificate);
-		expect(res.status.firstCall.args[0]).to.equal(200);
-		expect(
-			res.json.calledOnceWith({
-				success: true,
-				msg: successMessages.MONITOR_CERTIFICATE,
-				data: { certificateDate: "N/A" },
-			})
-		).to.be.true;
-	});
 	it("should return an error if fetchMonitorCertificate fails", async () => {
 		req.db.getMonitorById.returns({ url: "https://www.google.com" });
 		fetchMonitorCertificate.throws(new Error("Certificate error"));
@@ -232,7 +219,9 @@ describe("Monitor Controller - getMonitorById", () => {
 		expect(next.firstCall.args[0].message).to.equal("DB error");
 	});
 	it("should return 404 if a monitor is not found", async () => {
-		req.db.getMonitorById.returns(null);
+		const error = new Error("Monitor not found");
+		error.status = 404;
+		req.db.getMonitorById.throws(error);
 		await getMonitorById(req, res, next);
 		expect(next.firstCall.args[0]).to.be.an("error");
 		expect(next.firstCall.args[0].status).to.equal(404);
@@ -430,19 +419,11 @@ describe("Monitor Controller - createMonitor", () => {
 		expect(next.firstCall.args[0]).to.be.an("error");
 		expect(next.firstCall.args[0].message).to.equal("Job error");
 	});
-	it("should return success message and data if all operations succeed with empty notifications", async () => {
+	it("should fail validation with empty notifications", async () => {
 		req.body.notifications = [];
-		const monitor = { _id: "123", save: sinon.stub() };
-		req.db.createMonitor.returns(monitor);
 		await createMonitor(req, res, next);
-		expect(res.status.firstCall.args[0]).to.equal(201);
-		expect(
-			res.json.calledOnceWith({
-				success: true,
-				msg: successMessages.MONITOR_CREATE,
-				data: monitor,
-			})
-		).to.be.true;
+		expect(next.firstCall.args[0]).to.be.an("error");
+		expect(next.firstCall.args[0].status).to.equal(422);
 	});
 	it("should return success message and data if all operations succeed", async () => {
 		const monitor = { _id: "123", save: sinon.stub() };
@@ -462,38 +443,40 @@ describe("Monitor Controller - createMonitor", () => {
 describe("Monitor Controllor - checkEndpointResolution", () => {
 	let req, res, next, dnsResolveStub;
 	beforeEach(() => {
-		req = { query: { monitorURL: 'https://example.com' } };
+		req = { query: { monitorURL: "https://example.com" } };
 		res = { status: sinon.stub().returnsThis(), json: sinon.stub() };
 		next = sinon.stub();
-		dnsResolveStub = sinon.stub(dns, 'resolve');
+		dnsResolveStub = sinon.stub(dns, "resolve");
 	});
 	afterEach(() => {
 		dnsResolveStub.restore();
 	});
-	it('should resolve the URL successfully', async () => {
+	it("should resolve the URL successfully", async () => {
 		dnsResolveStub.callsFake((hostname, callback) => callback(null));
 		await checkEndpointResolution(req, res, next);
 		expect(res.status.calledWith(200)).to.be.true;
-		expect(res.json.calledWith({
-			success: true,
-			msg: 'URL resolved successfully',
-		})).to.be.true;
+		expect(
+			res.json.calledWith({
+				success: true,
+				msg: "URL resolved successfully",
+			})
+		).to.be.true;
 		expect(next.called).to.be.false;
 	});
 	it("should return an error if DNS resolution fails", async () => {
 		const dnsError = new Error("DNS resolution failed");
-		dnsError.code = 'ENOTFOUND';
+		dnsError.code = "ENOTFOUND";
 		dnsResolveStub.callsFake((hostname, callback) => callback(dnsError));
 		await checkEndpointResolution(req, res, next);
 		expect(next.calledOnce).to.be.true;
-  		const errorPassedToNext = next.getCall(0).args[0];
-  		expect(errorPassedToNext).to.be.an.instanceOf(Error);
-		expect(errorPassedToNext.message).to.include('DNS resolution failed');
-		expect(errorPassedToNext.code).to.equal('ENOTFOUND');
+		const errorPassedToNext = next.getCall(0).args[0];
+		expect(errorPassedToNext).to.be.an.instanceOf(Error);
+		expect(errorPassedToNext.message).to.include("DNS resolution failed");
+		expect(errorPassedToNext.code).to.equal("ENOTFOUND");
 		expect(errorPassedToNext.status).to.equal(500);
 	});
-	it('should reject with an error if query validation fails', async () => {
-		req.query.monitorURL = 'invalid-url';
+	it("should reject with an error if query validation fails", async () => {
+		req.query.monitorURL = "invalid-url";
 		await checkEndpointResolution(req, res, next);
 		expect(next.calledOnce).to.be.true;
 		const error = next.getCall(0).args[0];
@@ -838,20 +821,11 @@ describe("Monitor Controller - editMonitor", () => {
 		expect(next.firstCall.args[0]).to.be.an("error");
 		expect(next.firstCall.args[0].message).to.equal("Add Job error");
 	});
-	it("should return success message with data if all operations succeed and empty notifications", async () => {
+	it("should fail validation with empty notifications", async () => {
 		req.body.notifications = [];
-		const monitor = { _id: "123" };
-		req.db.getMonitorById.returns({ teamId: "123" });
-		req.db.editMonitor.returns(monitor);
 		await editMonitor(req, res, next);
-		expect(res.status.firstCall.args[0]).to.equal(200);
-		expect(
-			res.json.calledOnceWith({
-				success: true,
-				msg: successMessages.MONITOR_EDIT,
-				data: monitor,
-			})
-		).to.be.true;
+		expect(next.firstCall.args[0]).to.be.an("error");
+		expect(next.firstCall.args[0].status).to.equal(422);
 	});
 	it("should return success message with data if all operations succeed", async () => {
 		const monitor = { _id: "123" };

Server/validation/joi.js

@@ -222,14 +222,14 @@ const createMonitorBodyValidation = joi.object({
 	url: joi.string().required(),
 	isActive: joi.boolean(),
 	interval: joi.number(),
-	notifications: joi.array().items(joi.object()),
+	notifications: joi.array().items(joi.object()).min(1),
 });
 
 const editMonitorBodyValidation = joi.object({
 	name: joi.string(),
 	description: joi.string(),
 	interval: joi.number(),
-	notifications: joi.array().items(joi.object()),
+	notifications: joi.array().items(joi.object()).min(1),
 });
 
 const pauseMonitorParamValidation = joi.object({