- Refactor tests and models for improved clarity and functionality

- Updated test descriptions in agents, home, and login controllers to English.
- Enhanced User model to include structured login records with timestamps and IP addresses.
- Added method to User model for adding login records.
- Improved comments for better understanding in test files.

Author: pablo-sch

nodeapp/controllers/agents.controller.test.js

@@ -2,37 +2,40 @@ import request from "supertest";
 import app from "../app.js";
 import { guard, middleware } from "../lib/sessionManager";
 
-// Podemos mockear funciones internas
+//We can mock internal functions+++++++++++++++++++++++++++++++
 jest.mock("../lib/sessionManager", () => ({
   guard: jest.fn((req, res, next) => {
-    // Simular que el usuario no esta autenticado
+    // Simulate that the user is not authenticated
     res.redirect(`/login?redir=${req.url}`);
 
-    // Todos los middlewares llaman a next();
+    // All middlewares call next();
     next();
   }),
   middleware: jest.fn((req, res, next) => next()),
   useSessionInViews: jest.fn((req, res, next) => next()),
 }));
 
+//#######################################################################################################
 describe("AgentsController", () => {
-  it("Debería redirigir a la vista de login si no existe una sesión", async () => {
+  //=========================================================================
+  it("should redirect to the login view if no session exists", async () => {
     expect.assertions(2);
-    // Hacer una request a nuestra app
+    // Make a request to our app
     const response = await request(app)
-      // en la ruta /agents/new
+      // on the /agents/new route
       .get("/agents/new");
 
-    // Esperamos una redirección a login
+    // We expect a redirection to login
     expect(response.status).toBe(302);
     expect(response.headers.location).toBe("/login?redir=/agents/new");
   });
 
-  it("debería llamar al guard antes de permitir el acceso a la ruta", async () => {
+  //=========================================================================
+  it("should call guard before allowing access to the route", async () => {
     expect.assertions(1);
     const response = await request(app).get("/agents/new");
 
-    // El middleware de guard se debe llamar
+    // The guard middleware should be called
     expect(guard).toHaveBeenCalled();
   });
 });

nodeapp/controllers/homeController.test.js

@@ -1,3 +1,5 @@
+//#######################################################################################################
 describe("homeController", () => {
-  it.todo("debe devolver un 200 al visitar la home");
+  //=========================================================================
+  it.todo("should return a 200 when visiting the home page");
 });

nodeapp/controllers/loginController.test.js

@@ -1,18 +1,19 @@
 import request from "supertest";
-import app from "../app";
+import app from "../app.js";
 import User from "../models/User";
 
+//+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 jest.mock("../models/User.js");
 
-// Mockeamos connectMongoose para evitar conexiones reales
+//We mock connectMongoose to avoid real connections++++++++++++
 jest.mock("../lib/connectMongoose.js", () => {
   return jest.fn().mockResolvedValue({
     name: "mockConnection",
     close: jest.fn(),
   });
 });
 
-// Mockeamos session para evitar problemas de sesión
+//We mock session to avoid session issues++++++++++++++++++++++
 jest.mock("express-session", () => {
   return () => (req, res, next) => {
     req.session = {
@@ -23,7 +24,7 @@ jest.mock("express-session", () => {
   };
 });
 
-// Mockeamos sessionManager para simplificar los tests
+//We mock sessionManager to simplify the tests+++++++++++++++++
 jest.mock("../lib/sessionManager", () => ({
   middleware: (req, res, next) => {
     req.session = {
@@ -39,27 +40,31 @@ jest.mock("../lib/sessionManager", () => ({
   guard: (req, res, next) => next(),
 }));
 
+//#######################################################################################################
 describe("loginController", () => {
-  // Limpiamos mocks antes de cada test
+  // Clear mocks before each test
   beforeEach(() => {
     jest.clearAllMocks();
   });
 
-  it("debe devolver un 200 al visitar la página de login", async () => {
+  //=========================================================================
+  it("should return 200 when visiting the login page", async () => {
     const response = await request(app).get("/login");
 
     expect(response.status).toBe(200);
   });
 
-  it("debe mostrar formulario de login sin errores inicialmente", async () => {
+  //=========================================================================
+  it("should show login form without errors initially", async () => {
     const response = await request(app).get("/login");
 
     expect(response.text).toContain("form");
     expect(response.text).not.toContain("Invalid credentials");
   });
 
-  it("debe redirigir a home tras login exitoso", async () => {
-    // Mock de usuario existente con contraseña correcta
+  //=========================================================================
+  it("should redirect to home after successful login", async () => {
+    // Mock an existing user with correct password
     const mockUser = {
       id: "user123",
       email: "test@example.com",
@@ -81,8 +86,9 @@ describe("loginController", () => {
     expect(mockUser.comparePassword).toHaveBeenCalledWith("password123");
   });
 
-  it("debe mostrar error con credenciales inválidas", async () => {
-    // Mock de usuario existente con contraseña incorrecta
+  //=========================================================================
+  it("should show error with invalid credentials", async () => {
+    // Mock an existing user with incorrect password
     const mockUser = {
       email: "test@example.com",
       comparePassword: jest.fn().mockResolvedValue(false),
@@ -99,8 +105,9 @@ describe("loginController", () => {
     expect(response.text).toContain("Invalid credentials");
   });
 
-  it("debe mostrar error cuando el usuario no existe", async () => {
-    // Mock de usuario no existente
+  //=========================================================================
+  it("should show error when user does not exist", async () => {
+    // Mock non-existent user
     User.findOne = jest.fn().mockResolvedValue(null);
 
     const response = await request(app).post("/login").type("form").send({
@@ -112,8 +119,9 @@ describe("loginController", () => {
     expect(response.text).toContain("Invalid credentials");
   });
 
-  it("debe redirigir a la URL especificada después del login exitoso", async () => {
-    // Mock de usuario existente con contraseña correcta
+  //=========================================================================
+  it("should redirect to specified URL after successful login", async () => {
+    // Mock an existing user with correct password
     const mockUser = {
       id: "user123",
       email: "test@example.com",
@@ -132,10 +140,35 @@ describe("loginController", () => {
     expect(response.status).toBe(302);
     expect(response.headers.location).toBe("/agents/new");
   });
-  it("debe cerrar sesión y redirigir a home al hacer logout", async () => {
+
+  //=========================================================================
+  it("should logout and redirect to home", async () => {
     const response = await request(app).get("/logout");
 
     expect(response.status).toBe(302);
     expect(response.headers.location).toBe("/");
   });
 });
+
+//#######################################################################################################
+describe.only("TDD Exercise", () => {
+  //=========================================================================
+  it("should record a new login upon successful login", async () => {
+    const mockUser = {
+      id: "user123",
+      email: "test@example.com",
+      comparePassword: jest.fn().mockResolvedValue(true),
+      addLoginRecord: jest.fn(),
+      save: jest.fn().mockResolvedValue(true),
+    };
+    User.findOne = jest.fn().mockResolvedValue(mockUser);
+
+    await request(app).post("/login").type("form").send({
+      email: "test@example.com",
+      password: "password123",
+    });
+
+    expect(mockUser.addLoginRecord).toHaveBeenCalled();
+    expect(mockUser.save).toHaveBeenCalled();
+  });
+});

nodeapp/models/Agent.test.js

@@ -2,26 +2,26 @@ import request from "supertest";
 import app from "../app.js";
 import Agent from "../models/Agent.js";
 
-// Mockeamos el modelo Agent
+//We mock the Agent model++++++++++++++++++++++++++++++++++++++
 jest.mock("../models/Agent.js");
 
-// Mockeamos connectMongoose para evitar conexiones reales
+//We mock connectMongoose to avoid real connections++++++++++++
 jest.mock("../lib/connectMongoose.js", () => {
   return jest.fn().mockResolvedValue({
     name: "mockConnection",
     close: jest.fn(),
   });
 });
 
-// Mockeamos session para evitar problemas de sesión
+//We mock session to avoid session issues++++++++++++++++++++++
 jest.mock("express-session", () => {
   return () => (req, res, next) => {
     req.session = { userId: null };
     next();
   };
 });
 
-// Mockeamos sessionManager para simplificar los tests
+//We mock sessionManager to simplify tests+++++++++++++++++++++
 jest.mock("../lib/sessionManager", () => ({
   middleware: (req, res, next) => {
     req.session = { userId: null };
@@ -34,37 +34,41 @@ jest.mock("../lib/sessionManager", () => ({
   guard: (req, res, next) => next(),
 }));
 
+//#######################################################################################################
 describe("homeController", () => {
-  // Test de integración usando supertest
-  describe("Pruebas de integración", () => {
-    // Configuramos los mocks antes de cada test
+  // Integration test using supertest
+  //#######################################################################################################
+  describe("Integration tests", () => {
+    // We setup the mocks before each test
     beforeEach(() => {
-      // Configuramos el mock de Agent.find para que devuelva una lista de agentes de prueba
+      // We setup the Agent.find mock to return a list of test agents
       Agent.find = jest.fn().mockResolvedValue([
         { _id: "agent1", name: "Smith", age: 45 },
         { _id: "agent2", name: "Brown", age: 33 },
       ]);
     });
 
-    it("debe devolver un 200 al visitar la home", async () => {
+    //=========================================================================
+    it("should return 200 when visiting the home page", async () => {
       expect.assertions(1);
       let response;
       try {
         response = await request(app).get("/");
       } catch (ex) {
-        console.log("Error al hacer la petición");
+        console.log("Error making the request");
         console.log(ex);
       }
 
       expect(response.status).toBe(200);
-    }, 15000); // Aumentamos el timeout a 15 segundos
+    }, 15000); // We increase the timeout to 15 seconds
 
-    it("debe llamar a Agent.find con los parámetros correctos", async () => {
-      // Realizamos la petición a la ruta /
+    //=========================================================================
+    it("should call Agent.find with the correct parameters", async () => {
+      // We make the request to the / route
       expect.assertions(1);
       await request(app).get("/");
 
-      // Verificamos que se llamó a Agent.find
+      // We verify that Agent.find was called
       expect(Agent.find).toHaveBeenCalled();
     });
   });

nodeapp/models/User.js

@@ -12,9 +12,19 @@ const loginRecordSchema = new Schema(
 const userSchema = new Schema({
   email: { type: String, unique: true },
   password: String,
-  loginRecords: [loginRecordSchema],
+  loginRecords: [
+    {
+      timestamp: { type: Date, required: true, default: Date.now },
+      ip: { type: String, required: true },
+    },
+  ],
 });
 
+userSchema.methods.addLoginRecord = function (ip) {
+  this.loginRecords.push({ timestamp: new Date(), ip });
+  this.loginRecords.sort((a, b) => a.timestamp - b.timestamp);
+};
+
 // método del modelo
 userSchema.statics.hashPassword = (clearPassword) => {
   return bcrypt.hash(clearPassword, 7);

nodeapp/models/User.test.js

@@ -1,28 +1,32 @@
-import User from "./User";
+import User from "./User.js";
 
+//#######################################################################################################
 describe("User model & schema", () => {
-  it("Debería hashear correctamente la contraseña", async () => {
-    // Crear un password plano
+  //=========================================================================
+  it("Should correctly hash the password", async () => {
+    // Create a plain password
     const clearPassword = "supersegura";
 
-    // Crear su hash
+    // Create its hash
     const hash = await User.hashPassword(clearPassword);
 
-    // Verificar que son diferentes
+    // Verify that they are different
     expect(hash).not.toBe(clearPassword);
     expect(hash.length).toBeGreaterThan(1);
     expect(hash).not.toHaveLength(0);
   });
 
-  it.todo("Debería generar el hash utilizando la utilidad bcrypt");
+  //=========================================================================
+  it.todo("Should generate the hash using the bcrypt utility");
 
-  it("Debería comparar correctamente la contraseña", async () => {
+  //=========================================================================
+  it("Should correctly compare the password", async () => {
     expect.assertions(2);
-    // Al crear un usuario, crear su hash y compararlo con el password plano, deberia ser true.
-    // Crear un password plano
+    // When creating a user, generate its hash and compare it with the plain password, it should be true.
+    // Create a plain password
     const clearPassword = "supersegura";
 
-    // Crear su hash
+    // Create its hash
     const hash = await User.hashPassword(clearPassword);
 
     const user = new User({
@@ -37,3 +41,21 @@ describe("User model & schema", () => {
     expect(notMatchPassword).toBeFalse();
   });
 });
+
+//#######################################################################################################
+describe.only("TDD Exercise", () => {
+  //=========================================================================
+  it("Should have a loginRecords field that is an array", () => {
+    const user = new User({ email: "test@example.com", password: "hash" });
+    expect(Array.isArray(user.loginRecords)).toBeTrue();
+    expect(user.loginRecords).toHaveLength(0);
+  });
+
+  //=========================================================================
+  it("Each login record should have timestamp and ip", () => {
+    const user = new User({ email: "test@example.com", password: "hash" });
+    user.loginRecords.push({ ip: "127.0.0.1" });
+    expect(user.loginRecords[0]).toHaveProperty("timestamp");
+    expect(user.loginRecords[0]).toHaveProperty("ip");
+  });
+});