From 9aac2d183222b3b76cc2ff1e6768c9cf8f20bd45 Mon Sep 17 00:00:00 2001 From: Eric Chang <55172722+echang594@users.noreply.github.com> Date: Thu, 29 Feb 2024 20:47:51 -0800 Subject: [PATCH] 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 --- package.json | 2 +- services/EventService.ts | 4 ++- tests/admin.test.ts | 58 +++++++++++++++++++--------------------- tests/event.test.ts | 52 ++++++++++++++++++++++++++++++++++- 4 files changed, 82 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index 9d4a142a1..42b144b69 100644 --- 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 index bdcdc352e..9994e080e 100644 --- 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 index 03faaddcc..8c697a580 100644 --- 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 index e6d38a242..ef1d64dd1 100644 --- 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); + }); +});