Fixing all Unit Tests + adding back deleteUserByOtherUser functionality

Author: ashleynhs

src/api/controllers/UserController.ts

@@ -73,11 +73,11 @@ export class UserController {
   async deleteUser(@CurrentUser() user: UserModel): Promise<UserModel> {
     return await this.userService.deleteUser(user);
   }
-
-  // @Delete('id/:id/')
-  // async deleteUser(@Params() params: UuidParam, @CurrentUser() user: UserModel): Promise<GetUserResponse> {
-  //   return { user: await this.userService.deleteUser(user, params) };
-  // }
+  
+  @Delete('id/:id/')
+  async deleteUserByOtherUser(@Params() params: FirebaseUidParam, @CurrentUser() user: UserModel): Promise<GetUserResponse> {
+    return { user: await this.userService.deleteUserByOtherUser(user, params) };
+  }
 
   @Post('softDelete/id/:id/')
   async softDeleteUser(@Params() params: FirebaseUidParam): Promise<GetUserResponse> {

src/models/UserModel.ts

@@ -21,7 +21,7 @@ export class UserModel {
   firebaseUid: string;
 
   /** @deprecated This column will be removed after the AuthorizationRefactor migration */
-  @Column()
+  @Column({ nullable: true })
   id: string;
 
   @Column({ unique: true })

src/services/UserService.ts

@@ -173,6 +173,18 @@ export class UserService {
     });
   }
 
+  public async deleteUserByOtherUser(user: UserModel, params: FirebaseUidParam): Promise<UserModel> {
+    return this.transactions.readWrite(async (transactionalEntityManager) => {
+      const userRepository = Repositories.user(transactionalEntityManager);
+      const userToDelete = await userRepository.getUserById(params.id);
+      if (!userToDelete) throw new NotFoundError('User not found!');
+      if (user.firebaseUid !== userToDelete.firebaseUid && !user.admin) {
+        throw new UnauthorizedError('User does not have permission to delete other users');
+      }
+      return userRepository.deleteUser(userToDelete);
+    });
+  }
+
   public async softDeleteUser(params: FirebaseUidParam): Promise<UserModel> {
     return this.transactions.readWrite(async (transactionalEntityManager) => {
       const userRepository = Repositories.user(transactionalEntityManager);

src/tests/UserTest.test.ts

@@ -6,15 +6,21 @@ import { UuidParam } from '../api/validators/GenericRequests';
 import { UserModel } from '../models/UserModel';
 import { ControllerFactory } from './controllers';
 import { DatabaseConnection, DataFactory, PostFactory, UserFactory } from './data';
-import e from 'express';
-import exp from 'constants';
+import { CreateUserRequest } from '../types';
+
 
 let uuidParam: UuidParam;
 let expectedUser: UserModel;
 let conn: Connection;
 let userController: UserController;
 let postController: PostController;
 
+// Helper function to omit a property from an object. Used for testing without the deprecated id property in UserModel.
+function omit(obj: any, keyToOmit: string) {
+  return Object.fromEntries(
+    Object.entries(obj).filter(([key]) => key !== keyToOmit)
+  );
+}
 
 beforeAll(async () => {
   await DatabaseConnection.connect();
@@ -96,7 +102,8 @@ describe('user tests', () => {
     if (getUserResponse.user != undefined) {
       getUserResponse.user.stars = Number(getUserResponse.user.stars);
     }
-    expect(getUserResponse.user).toEqual(expectedUser);
+
+    expect(omit(getUserResponse.user, 'id')).toEqual(omit(expectedUser, 'id'));
   });
 
   test('get user by email', async () => {
@@ -110,7 +117,7 @@ describe('user tests', () => {
     if (getUserResponse.user != undefined) {
       getUserResponse.user.stars = Number(getUserResponse.user.stars);
     }
-    expect(getUserResponse.user).toEqual(expectedUser);
+    expect(omit(getUserResponse.user, 'id')).toEqual(omit(expectedUser, 'id'));
   });
 
   test('get user by google id', async () => {
@@ -124,7 +131,7 @@ describe('user tests', () => {
     if (getUserResponse.user != undefined) {
       getUserResponse.user.stars = Number(getUserResponse.user.stars);
     }
-    expect(getUserResponse.user).toEqual(expectedUser);
+    expect(omit(getUserResponse.user, 'id')).toEqual(omit(expectedUser, 'id'));
   });
 
   test('edit profile', async () => {
@@ -137,54 +144,75 @@ describe('user tests', () => {
     await userController.editProfile({
       photoUrlBase64: undefined,
       username: undefined,
-      venmoHandle: '@Shungo-Najima1',
+      venmoHandle: 'Shungo-Najima1',
       bio: 'Mateo Slay'
     }, user);
 
     expectedUser.bio = "Mateo Slay";
-    expectedUser.venmoHandle = "@Shungo-Najima1";
+    expectedUser.venmoHandle = "Shungo-Najima1";
 
     const getUserResponse = await userController.getUserByGoogleId('shungoGoogleID');
     if (getUserResponse.user != undefined) {
       getUserResponse.user.stars = Number(getUserResponse.user.stars);
     }
-    expect(getUserResponse.user).toEqual(expectedUser);
+    expect(omit(getUserResponse.user, 'id')).toEqual(omit(expectedUser, 'id'));
   });
 
   test('set super admin status', async () => {
-    const authController = ControllerFactory.auth(conn);
+    // Create super admin user first
+    const superAdmin = UserFactory.fake();
+    superAdmin.email = '[email protected]';
+    superAdmin.admin = true;
+
+    await new DataFactory()
+      .createUsers(superAdmin)
+      .write();
 
-    const createUserRequest = {
+    const createUserRequest: CreateUserRequest = {
       username: "admin",
       netid: "adm999",
       givenName: "administrator",
       familyName: "Weiner",
       photoUrl: "https://melmagazine.com/wp-content/uploads/2021/01/66f-1.jpg",
-      venmoHandle: "@admin-Weiner",
-      email: "[email protected]",
+      venmoHandle: "admin-Weiner",
+      email: "[email protected]",
       googleId: "mateoGoogleId",
       bio: "Personally, I would not stand for this.",
+      fcmToken: ""
     }
 
-    const createUserRequestResponse = await authController.createUser(createUserRequest);
+    // Create a new regular user
+    const newUser = await userController.createUser(superAdmin, createUserRequest);
+    expect(newUser.admin).toEqual(false); // New users should not be admin by default
 
-    expect(createUserRequestResponse.user?.admin).toEqual(true);
+    // Make the user an admin using setAdmin
+    const setAdminResponse = await userController.setAdmin({ 
+      email: createUserRequest.email, 
+      status: true 
+    }, superAdmin);
+    expect(setAdminResponse.user?.admin).toBe(true);
   });
 
   test('set admin status from super user', async () => {
-    const admin = UserFactory.fakeTemplate();
-    admin.email = '[email protected]';
-    let user = UserFactory.fake();
+    // Create super admin user first
+    const superAdmin = UserFactory.fake();
+    superAdmin.email = '[email protected]';
+    superAdmin.admin = true;
+
+    // Create a regular user
+    const regularUser = UserFactory.fake();
+    regularUser.email = regularUser.netid + '@cornell.edu';
 
     await new DataFactory()
-      .createUsers(admin, user)
+      .createUsers(superAdmin, regularUser)
       .write();
 
-    expectedUser.admin = true;
-
-    const getUserResponse = await userController.setAdmin({ email: user.email, status: true }, admin);
-
-    expect(getUserResponse.user?.admin).toBe(true);
+    // Make the regular user an admin
+    const setAdminResponse = await userController.setAdmin({ 
+      email: regularUser.email, 
+      status: true 
+    }, superAdmin);
+    expect(setAdminResponse.user?.admin).toBe(true);
   });
 
   test('block users', async () => {
@@ -438,9 +466,9 @@ describe('user tests', () => {
     if (getUserResponse.user != undefined) {
       getUserResponse.user.stars = Number(getUserResponse.user.stars);
     }
-    expect(getUserResponse.user).toEqual(expectedUser);
+    expect(omit(getUserResponse.user, 'id')).toEqual(omit(expectedUser, 'id'));
 
-    const deleteUserResponse = await userController.deleteUser(uuidParam, user);
+    const deleteUserResponse = await userController.deleteUserByOtherUser(uuidParam, user);
     if (deleteUserResponse.user != undefined) {
       deleteUserResponse.user.stars = Number(deleteUserResponse.user.stars);
     }
@@ -460,7 +488,7 @@ describe('user tests', () => {
     const preDeleteUserResponse = await userController.getUsers(admin);
     expect(preDeleteUserResponse.users).toHaveLength(2);
 
-    const deleteUserResponse = await userController.deleteUser(uuidParam, admin);
+    const deleteUserResponse = await userController.deleteUserByOtherUser(uuidParam, admin);
     if (deleteUserResponse.user != undefined) {
       deleteUserResponse.user.stars = Number(deleteUserResponse.user.stars);
     }
@@ -476,7 +504,7 @@ describe('user tests', () => {
       .write();
 
     try {
-      await userController.deleteUser({firebaseUid: user2.firebaseUid}, user1);
+      await userController.deleteUserByOtherUser({id: user2.firebaseUid}, user1);
     } catch (error) {
       expect(error.message).toBe('User does not have permission to delete other users');
     }

src/tests/data/DatabaseConnection.ts

@@ -33,6 +33,7 @@ export class DatabaseConnection {
         'Feedback',
         'notifications', // Add notifications table before User since it has a fk to User
         'Post',
+        'FCMToken',
         'Request',
         'UserReview',
         'User'

src/tests/data/UserFactory.ts

@@ -29,10 +29,13 @@ export class UserFactory {
         fakeUser.username = 'snajima';
         fakeUser.netid = 'sn999';
         fakeUser.admin = false;
+        fakeUser.stars = 0;
+        fakeUser.numReviews = 0;
         fakeUser.photoUrl = 'https://media-exp1.licdn.com/dms/image/C5603AQGmvQtdub6nAQ/profile-displayphoto-shrink_400_400/0/1635358826496?e=1668643200&v=beta&t=ncqjrFUqgqipctcmaSwPzSPrkj0RIQHiCINup_55NNs';
         fakeUser.email = fakeUser.netid + '@cornell.edu';
         fakeUser.googleId = 'shungoGoogleID';
         fakeUser.venmoHandle = "@Shungo-Najima";
+        fakeUser.bio = "";
         fakeUser.isActive = true;
 
         return fakeUser;
@@ -48,15 +51,19 @@ export class UserFactory {
          */
         const fakeUser = new UserModel();
         fakeUser.firebaseUid = 'c6f0a14a-48ae-4b1c-bd6f-5f3b7e8c2b99';
+        fakeUser.id = fakeUser.firebaseUid; // Add deprecated id during refactor
         fakeUser.givenName = 'Tony';
         fakeUser.familyName = 'Matchev';
         fakeUser.username = 'tmatchev';
         fakeUser.netid = 'tkm21';
         fakeUser.admin = false;
+        fakeUser.stars = 0;
+        fakeUser.numReviews = 0;
         fakeUser.photoUrl = 'https://media-exp1.licdn.com/dms/image/C5603AQGmvQtdub6nAQ/profile-displayphoto-shrink_400_400/0/1635358826496?e=1668643200&v=beta&t=ncqjrFUqgqipctcmaSwPzSPrkj0RIQHiCINup_55NNs';
         fakeUser.email = fakeUser.netid + '@cornell.edu';
         fakeUser.googleId = 'tonyGoogleID';
         fakeUser.venmoHandle = "@Tony-Matchev";
+        fakeUser.bio = "";
         fakeUser.isActive = true;
 
         return fakeUser;
@@ -73,14 +80,18 @@ export class UserFactory {
 
         const fakeUser = new UserModel();
         fakeUser.firebaseUid = faker.datatype.uuid();
+        fakeUser.id = fakeUser.firebaseUid; // Add deprecated id during refactor
         fakeUser.givenName = firstName;
         fakeUser.familyName = lastName;
         fakeUser.username = faker.internet.userName(firstName, lastName);
         fakeUser.admin = false;
+        fakeUser.stars = 0;
+        fakeUser.numReviews = 0;
         fakeUser.netid = FactoryUtils.getRandomLetter() + FactoryUtils.getRandomLetter() + FactoryUtils.getRandomNumber(0, 9999);
         fakeUser.photoUrl = faker.internet.url();
         fakeUser.email = fakeUser.netid + '@cornell.edu';
         fakeUser.googleId = faker.datatype.uuid();
+        fakeUser.bio = "";
         fakeUser.isActive = true;
 
         return fakeUser;