Better error message for event deletion when event has attendances (#410

) * Throw error when deleting event with attendance * Fixed tests that assumed repository query results are in order * Test events with no attendances can be deleted * Bump version number
acmucsd · Mar 1, 2024 · 9aac2d1 · 9aac2d1
1 parent 60cba34
commit 9aac2d1
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 34 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@acmucsd/membership-portal",
-  "version": "3.4.0",
+  "version": "3.4.1",
   "description": "REST API for ACM UCSD's membership portal.",
   "main": "index.d.ts",
   "files": [

diff --git a/services/EventService.ts b/services/EventService.ts
@@ -1,6 +1,6 @@
 import { Service } from 'typedi';
 import { InjectManager } from 'typeorm-typedi-extensions';
-import { NotFoundError } from 'routing-controllers';
+import { ForbiddenError, NotFoundError } from 'routing-controllers';
 import { EntityManager } from 'typeorm';
 import { EventModel } from '../models/EventModel';
 import { Uuid, PublicEvent, Event, EventSearchOptions } from '../types';
@@ -81,6 +81,8 @@ export default class EventService {
       const eventRepository = Repositories.event(txn);
       const event = await eventRepository.findByUuid(uuid);
       if (!event) throw new NotFoundError('Event not found');
+      const attendances = await Repositories.attendance(txn).getAttendancesForEvent(uuid);
+      if (attendances.length > 0) throw new ForbiddenError('Cannot delete event that has attendances');
       await eventRepository.deleteEvent(event);
     });
   }

diff --git a/tests/admin.test.ts b/tests/admin.test.ts
@@ -200,37 +200,31 @@ describe('updating user access level', () => {
       .createUsers(admin, staffUser, standardUser, marketingUser, merchStoreDistributorUser)
       .write();
 
+    // Note that PortalState.createUsers changes the user emails to be lowercase so this array is created afterwards
+    const accessUpdates = [
+      { user: staffUser.email, accessType: UserAccessType.MERCH_STORE_MANAGER },
+      { user: standardUser.email, accessType: UserAccessType.MARKETING },
+      { user: marketingUser.email, accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR },
+      { user: merchStoreDistributorUser.email, accessType: UserAccessType.STAFF },
+    ].sort((a, b) => a.user.localeCompare(b.user));
+
     const adminController = ControllerFactory.admin(conn);
 
-    const accessLevelResponse = await adminController.updateUserAccessLevel({
-      accessUpdates: [
-        { user: staffUser.email, accessType: UserAccessType.MERCH_STORE_MANAGER },
-        { user: standardUser.email, accessType: UserAccessType.MARKETING },
-        { user: marketingUser.email, accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR },
-        { user: merchStoreDistributorUser.email, accessType: UserAccessType.STAFF },
-      ],
-    }, admin);
+    const accessLevelResponse = await adminController.updateUserAccessLevel({ accessUpdates }, admin);
+    accessLevelResponse.updatedUsers.sort((a, b) => a.email.localeCompare(b.email));
 
     const repository = conn.getRepository(UserModel);
     const updatedUsers = await repository.find({
       email: In([staffUser.email, standardUser.email, marketingUser.email, merchStoreDistributorUser.email]),
     });
+    // sorting to guarantee order
+    updatedUsers.sort((a, b) => a.email.localeCompare(b.email));
 
-    expect(updatedUsers[0].email).toEqual(staffUser.email);
-    expect(updatedUsers[0].accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER);
-    expect(accessLevelResponse.updatedUsers[0].accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER);
-
-    expect(updatedUsers[1].email).toEqual(standardUser.email);
-    expect(updatedUsers[1].accessType).toEqual(UserAccessType.MARKETING);
-    expect(accessLevelResponse.updatedUsers[1].accessType).toEqual(UserAccessType.MARKETING);
-
-    expect(updatedUsers[2].email).toEqual(marketingUser.email);
-    expect(updatedUsers[2].accessType).toEqual(UserAccessType.MERCH_STORE_DISTRIBUTOR);
-    expect(accessLevelResponse.updatedUsers[2].accessType).toEqual(UserAccessType.MERCH_STORE_DISTRIBUTOR);
-
-    expect(updatedUsers[3].email).toEqual(merchStoreDistributorUser.email);
-    expect(updatedUsers[3].accessType).toEqual(UserAccessType.STAFF);
-    expect(accessLevelResponse.updatedUsers[3].accessType).toEqual(UserAccessType.STAFF);
+    accessUpdates.forEach((accessUpdate, index) => {
+      expect(updatedUsers[index].email).toEqual(accessUpdate.user);
+      expect(updatedUsers[index].accessType).toEqual(accessUpdate.accessType);
+      expect(accessLevelResponse.updatedUsers[index].accessType).toEqual(accessUpdate.accessType);
+    });
   });
 
   test('attempt to update when user is not an admin', async () => {
@@ -246,6 +240,10 @@ describe('updating user access level', () => {
       .createUsers(staffUser, standardUser, marketingUser, merchStoreDistributorUser, standard)
       .write();
 
+    // Note that PortalState.createUsers changes the user emails to be lowercase so this array is created afterwards
+    const users = [staffUser, standardUser, marketingUser, merchStoreDistributorUser]
+      .sort((a, b) => a.email.localeCompare(b.email));
+
     const adminController = ControllerFactory.admin(conn);
 
     await expect(async () => {
@@ -263,15 +261,13 @@ describe('updating user access level', () => {
     const updatedUsers = await repository.find({
       email: In([staffUser.email, standardUser.email, marketingUser.email, merchStoreDistributorUser.email]),
     });
+    // sorting to guarantee order
+    updatedUsers.sort((a, b) => a.email.localeCompare(b.email));
 
-    expect(updatedUsers[0].email).toEqual(staffUser.email);
-    expect(updatedUsers[0].accessType).toEqual(UserAccessType.STAFF);
-    expect(updatedUsers[1].email).toEqual(standardUser.email);
-    expect(updatedUsers[1].accessType).toEqual(UserAccessType.STANDARD);
-    expect(updatedUsers[2].email).toEqual(marketingUser.email);
-    expect(updatedUsers[2].accessType).toEqual(UserAccessType.MARKETING);
-    expect(updatedUsers[3].email).toEqual(merchStoreDistributorUser.email);
-    expect(updatedUsers[3].accessType).toEqual(UserAccessType.MERCH_STORE_DISTRIBUTOR);
+    users.forEach((user, index) => {
+      expect(updatedUsers[index].email).toEqual(user.email);
+      expect(updatedUsers[index].accessType).toEqual(user.accessType);
+    });
   });
 
   test('attempt to update duplicate users', async () => {

diff --git a/tests/event.test.ts b/tests/event.test.ts
@@ -2,8 +2,9 @@ import * as moment from 'moment';
 import { ForbiddenError } from 'routing-controllers';
 import { UserAccessType } from '../types';
 import { ControllerFactory } from './controllers';
-import { DatabaseConnection, PortalState, UserFactory } from './data';
+import { DatabaseConnection, EventFactory, PortalState, UserFactory } from './data';
 import { CreateEventRequest } from '../api/validators/EventControllerRequests';
+import { EventModel } from '../models/EventModel';
 
 beforeAll(async () => {
   await DatabaseConnection.connect();
@@ -126,3 +127,52 @@ describe('event creation', () => {
       .rejects.toThrow('Start date after end date');
   });
 });
+
+describe('event deletion', () => {
+  test('should delete event that has no attendances', async () => {
+    // setting up inputs
+    const conn = await DatabaseConnection.get();
+    const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN });
+    const event = EventFactory.fake();
+
+    await new PortalState()
+      .createUsers(admin)
+      .createEvents(event)
+      .write();
+
+    // deleting the event
+    const eventController = ControllerFactory.event(conn);
+    await eventController.deleteEvent({ uuid: event.uuid }, admin);
+
+    // verifying event deleted
+    const repository = conn.getRepository(EventModel);
+    const repositoryEvent = await repository.find({ uuid: event.uuid });
+
+    expect(repositoryEvent).toHaveLength(0);
+  });
+
+  test('should not delete event that has attendances', async () => {
+    // setting up inputs
+    const conn = await DatabaseConnection.get();
+    const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN });
+    const user = UserFactory.fake();
+    const event = EventFactory.fake();
+
+    await new PortalState()
+      .createUsers(admin, user)
+      .createEvents(event)
+      .attendEvents([user], [event])
+      .write();
+
+    // verifying correct error thrown
+    const eventController = ControllerFactory.event(conn);
+    await expect(eventController.deleteEvent({ uuid: event.uuid }, admin))
+      .rejects.toThrow('Cannot delete event that has attendances');
+
+    // verifying event not deleted
+    const repository = conn.getRepository(EventModel);
+    const repositoryEvent = await repository.findOne({ uuid: event.uuid });
+
+    expect(repositoryEvent).toEqual(event);
+  });
+});