From ecbf6ce802d29db47aef76050529fade4c0fdc0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20L=C3=A1szl=C3=B3?= <rlacko99@gmail.com>
Date: Mon, 21 Dec 2020 18:21:10 +0100
Subject: [PATCH] refactor User middlewares and add isLoggedIn middleware

---
 src/middlewares/auth/isLoggedIn.ts         | 16 ++++
 src/middlewares/files/card/getCardImage.ts |  2 +-
 src/middlewares/user/addUser.ts            | 87 ++++++++--------------
 src/middlewares/user/deleteUser.ts         | 21 +++---
 src/middlewares/user/getOwnUser.ts         | 28 ++-----
 src/middlewares/user/getUser.ts            | 31 +++-----
 src/middlewares/user/getUsersList.ts       | 13 +---
 src/middlewares/user/responseUser.ts       |  3 +
 src/middlewares/user/responseUserList.ts   |  3 +
 src/middlewares/user/updateUser.ts         | 38 +++++-----
 src/models/ProfileSchema.ts                |  1 -
 11 files changed, 106 insertions(+), 137 deletions(-)
 create mode 100644 src/middlewares/auth/isLoggedIn.ts

diff --git a/src/middlewares/auth/isLoggedIn.ts b/src/middlewares/auth/isLoggedIn.ts
new file mode 100644
index 00000000..713f192c
--- /dev/null
+++ b/src/middlewares/auth/isLoggedIn.ts
@@ -0,0 +1,16 @@
+import { NextFunction, Request, Response } from "express";
+
+/**
+ * Middleware to check if the user
+ * is logged in.
+ */
+const isLoggedIn = () => (req: Request, res: Response, next: NextFunction) => {
+  if (req.session!.user) {
+    next();
+  } else {
+    res.status(401);
+    res.json({ message: "You have to login to see this page!" });
+  }
+};
+
+export default isLoggedIn;
diff --git a/src/middlewares/files/card/getCardImage.ts b/src/middlewares/files/card/getCardImage.ts
index d103871d..39a7cd00 100644
--- a/src/middlewares/files/card/getCardImage.ts
+++ b/src/middlewares/files/card/getCardImage.ts
@@ -3,7 +3,7 @@ import { NextFunction, Request, Response } from "express";
 import CardImage from "../../../models/CardImageSchema";
 
 /**
- * Get the Entry card background image
+ * Middleware to get the Entry card background image
  * and set res.data.cardImage
  */
 const getCardImage = () => async (
diff --git a/src/middlewares/user/addUser.ts b/src/middlewares/user/addUser.ts
index 3639a4c2..61d29352 100644
--- a/src/middlewares/user/addUser.ts
+++ b/src/middlewares/user/addUser.ts
@@ -1,74 +1,51 @@
 import { NextFunction, Request, Response } from "express";
-import Profile, { Role } from "../../models/ProfileSchema";
+import Profile, { IProfile, Role } from "../../models/ProfileSchema";
 
 import { ValidationError } from "../utils/ValidationError";
-
-// After login the user can register itself
+import { validateFields } from "../utils/validateFields";
+
+const fields = [
+  { name: "studentCardNumber", required: true },
+  { name: "roomNumber", required: true },
+  { name: "picture", required: false },
+];
+
+/**
+ * Middleware to create own User if doesn't exist
+ * and set res.data.profile to it
+ */
 const addUser = () => async (
   req: Request,
   res: Response,
   next: NextFunction
 ) => {
-  if (!req.session!.user) {
-    return res.status(401).json({ message: "You have to login to register!" });
-  }
-
   // Already registered
   if (req.session?.user?.id) {
-    await Profile.findOne(
-      { external_id: req.session.user.id },
-      (error, profile) => {
-        if (error) {
-          console.warn(error);
-          return res.status(400);
-        } else {
-          res.status(200);
-
-          res.data.profile = profile;
-        }
-      }
-    );
+    res.data.profile = await Profile.findById(req.session.user.id)
+      .lean()
+      .exec();
     return next();
   }
+
   // Register
-  try {
-    const profile = new Profile();
+  const newProfile = new Profile();
 
-    // Check required fields
-    const fields = [
-      { name: "studentCardNumber", required: true },
-      { name: "roomNumber", required: true },
-      { name: "picture", required: false },
-    ];
-    fields.forEach((field) => {
-      const value = req.body[field.name];
-      if (field.required && !value) {
-        res.status(400);
-        throw new ValidationError(400, `Field: {${field.name}} is required!`);
-      }
-      if (value) profile.set(field.name, value);
-    });
+  // Validate and set fields from request body
+  validateFields({ fields, reqBody: req.body });
+  fields.forEach((field) => {
+    const value = req.body[field.name];
+    if (value) newProfile.set(field.name, req.body[field.name]);
+  });
 
-    profile.external_id = req.session!.user!.external_id;
-    profile.email = String(req.session?.user?.email);
-    profile.name = String(req.session?.user?.name);
+  newProfile.external_id = req.session!.user!.external_id;
+  newProfile.email = String(req.session?.user?.email);
+  newProfile.name = String(req.session?.user?.name);
 
-    profile.save((err) => {
-      if (err) {
-        res.status(400);
-      } else {
-        res.status(201);
-        res.data.profile = profile;
-        req.session!.user!.id = profile.id;
-      }
-      next();
-    });
-  } catch (error) {
-    if (error instanceof ValidationError) {
-      const { code, message } = error;
-      return res.status(code).send({ message });
-    }
-  }
+  await newProfile.save();
+
+  res.data.profile = newProfile;
+  req.session!.user!.id = newProfile.id;
+  next();
 };
 
 export default addUser;
diff --git a/src/middlewares/user/deleteUser.ts b/src/middlewares/user/deleteUser.ts
index ac37f253..f3625298 100644
--- a/src/middlewares/user/deleteUser.ts
+++ b/src/middlewares/user/deleteUser.ts
@@ -2,15 +2,16 @@ import { NextFunction, Request, Response } from "express";
 
 import Profile from "../../models/ProfileSchema";
 
-const deletUser = () => (req: Request, res: Response, next: NextFunction) => {
-  Profile.findByIdAndDelete(req.params.id, (error) => {
-    if (error) {
-      res.status(400);
-    } else {
-      res.status(204);
-    }
-    next();
-  });
+/**
+ * Middleware to delete User where id = req.params.id
+ */
+const deleteUser = () => async (
+  req: Request,
+  res: Response,
+  next: NextFunction
+) => {
+  await Profile.findByIdAndDelete(req.params.id);
+  next();
 };
 
-export default deletUser;
+export default deleteUser;
diff --git a/src/middlewares/user/getOwnUser.ts b/src/middlewares/user/getOwnUser.ts
index c9b76035..29835218 100644
--- a/src/middlewares/user/getOwnUser.ts
+++ b/src/middlewares/user/getOwnUser.ts
@@ -4,28 +4,12 @@ import Profile, { IProfile } from "../../models/ProfileSchema";
 import { IWarning } from "../../models/WarningSchema";
 import Warning from "../../models/WarningSchema";
 
+/**
+ * Middleware to set req.params.id to the current users id.
+ * getUser() middleware should be called after this.
+ */
 const getOwnUser = () => (req: Request, res: Response, next: NextFunction) => {
-  Profile.findOne(
-    { external_id: req.session!.user!.id },
-    async (error, profile) => {
-      if (error) {
-        console.warn(error);
-        res.status(400);
-      } else {
-        res.status(200);
-
-        let objProfile = profile?.toObject();
-        if (!!objProfile) {
-          objProfile.warnings = await Warning.find({
-            _id: { $in: profile?.warningIds },
-          }).exec();
-        }
-
-        delete objProfile.warningIds;
-        res.data.profile = objProfile;
-      }
-      next();
-    }
-  );
+  req.params.id = req.session.user!.id!;
+  next();
 };
 export default getOwnUser;
diff --git a/src/middlewares/user/getUser.ts b/src/middlewares/user/getUser.ts
index 4494e684..ad8460a1 100644
--- a/src/middlewares/user/getUser.ts
+++ b/src/middlewares/user/getUser.ts
@@ -4,26 +4,17 @@ import { IWarning } from "../../models/WarningSchema";
 import Profile from "../../models/ProfileSchema";
 import Warning from "../../models/WarningSchema";
 
-const getUser = () => (req: Request, res: Response, next: NextFunction) => {
-  Profile.findById(req.params.id, async (error, profile) => {
-    if (error) {
-      console.warn(error);
-      res.status(400);
-    } else {
-      res.status(200);
-
-      let objProfile = profile?.toObject();
-      if (!!objProfile) {
-        objProfile.warnings = await Warning.find({
-          _id: { $in: profile?.warningIds },
-        }).exec();
-      }
-
-      delete objProfile.warningIds;
-      res.data.profile = objProfile;
-    }
-    next();
-  });
+/**
+ * Middleware to get a User by req.params.id
+ * and set res.data.profile
+ */
+const getUser = () => async (
+  req: Request,
+  res: Response,
+  next: NextFunction
+) => {
+  res.data.profile = await Profile.findById(req.params.id).lean().exec();
+  next();
 };
 
 export default getUser;
diff --git a/src/middlewares/user/getUsersList.ts b/src/middlewares/user/getUsersList.ts
index 1749beb9..e3a8f69e 100644
--- a/src/middlewares/user/getUsersList.ts
+++ b/src/middlewares/user/getUsersList.ts
@@ -2,20 +2,13 @@ import { NextFunction, Request, Response } from "express";
 
 import Profile from "../../models/ProfileSchema";
 
-const getUsersList = () => (
+const getUsersList = () => async (
   req: Request,
   res: Response,
   next: NextFunction
 ) => {
-  Profile.find({}, (err, profiles) => {
-    if (err) {
-      res.status(400);
-    } else {
-      res.status(200);
-      res.data.profiles = profiles;
-    }
-    next();
-  });
+  res.data.profiles = await Profile.find();
+  next();
 };
 
 export default getUsersList;
diff --git a/src/middlewares/user/responseUser.ts b/src/middlewares/user/responseUser.ts
index ce5af1bb..7df3e826 100644
--- a/src/middlewares/user/responseUser.ts
+++ b/src/middlewares/user/responseUser.ts
@@ -1,5 +1,8 @@
 import { NextFunction, Request, Response, response } from "express";
 
+/**
+ * Return the found user from res.data.profile
+ */
 const responseUser = () => (req: Request, res: Response) => {
   if (!res.data.profile) {
     res.status(404).json({ message: "User not found!" });
diff --git a/src/middlewares/user/responseUserList.ts b/src/middlewares/user/responseUserList.ts
index fda9d94b..cdab625e 100644
--- a/src/middlewares/user/responseUserList.ts
+++ b/src/middlewares/user/responseUserList.ts
@@ -1,5 +1,8 @@
 import { NextFunction, Request, Response, response } from "express";
 
+/**
+ * Return the found users from res.data.profiles
+ */
 const responseUserList = () => (req: Request, res: Response) => {
   res.json(res.data.profiles);
 };
diff --git a/src/middlewares/user/updateUser.ts b/src/middlewares/user/updateUser.ts
index b3fd6cc9..29ef9816 100644
--- a/src/middlewares/user/updateUser.ts
+++ b/src/middlewares/user/updateUser.ts
@@ -5,24 +5,26 @@ import Profile from "../../models/ProfileSchema";
 // Valid fields to update
 const validFields = ["studentCardNumber", "roomNumber", "picture"];
 
-// Update user
-const updateUser = () => (req: Request, res: Response, next: NextFunction) => {
-  Profile.findOne({ _id: req.params.id }, (error, profile) => {
-    res.status(200);
-    if (error) {
-      res.status(400);
-    } else {
-      if (profile) {
-        validFields.forEach((field) => {
-          const value = req.body[field];
-          if (value) profile.set(field, value);
-        });
-        profile.save();
-        res.data.profile = profile;
-      }
-    }
-    next();
-  });
+/**
+ * Update a user
+ */
+const updateUser = () => async (
+  req: Request,
+  res: Response,
+  next: NextFunction
+) => {
+  const profile = await Profile.findById(req.params.id).exec();
+
+  if (profile) {
+    validFields.forEach((field) => {
+      const value = req.body[field];
+      if (value) profile.set(field, value);
+    });
+    profile.save();
+  }
+  res.data.profile = profile;
+
+  next();
 };
 
 export default updateUser;
diff --git a/src/models/ProfileSchema.ts b/src/models/ProfileSchema.ts
index 74831886..e01b02d0 100644
--- a/src/models/ProfileSchema.ts
+++ b/src/models/ProfileSchema.ts
@@ -18,7 +18,6 @@ export interface IProfile extends Document {
   email: string;
   name: string;
   warningIds: string[] | [];
-  warnings: IWarning[] | [];
 }
 
 const ProfileSchema = new Schema({
-- 
GitLab