From e7a129043db8ab2f94e8d73c0c0b0c286964c26d Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Sat, 4 Nov 2023 20:12:22 -0700 Subject: [PATCH 01/30] Seed data for Resumes (#367) * seed data for resumes * linting :( * bruh * typo * unncessary file factory export * added pdf link to resumes --- tests/Seeds.ts | 14 +++++++++++++- tests/data/index.ts | 1 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/Seeds.ts b/tests/Seeds.ts index 8633deef..74ba5023 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -1,6 +1,6 @@ import * as moment from 'moment'; import { UserAccessType } from '../types'; -import { DatabaseConnection, EventFactory, MerchFactory, PortalState, UserFactory } from './data'; +import { DatabaseConnection, EventFactory, MerchFactory, PortalState, UserFactory, ResumeFactory } from './data'; function getGraduationYear(n: number) { return moment().year() + n; @@ -116,6 +116,14 @@ async function seed(): Promise { accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR, }); + const RESUME_URL = 'https://acmucsd-local.s3.us-west-1.amazonaws.com/resumeSeedingData/alexface.pdf'; + + const USER_VISIBLE_RESUME = UserFactory.fake(); + const RESUME_1 = ResumeFactory.fake({ user: USER_VISIBLE_RESUME, isResumeVisible: true, url: RESUME_URL }); + + const USER_HIDDEN_RESUME = UserFactory.fake(); + const RESUME_2 = ResumeFactory.fake({ user: USER_HIDDEN_RESUME, isResumeVisible: false, url: RESUME_URL }); + // create members in bulk for testing things like sliding leaderboard in a realistic manner const otherMembers = UserFactory.create(200); const highAttendanceMembers = otherMembers.slice(0, 50); @@ -549,6 +557,8 @@ async function seed(): Promise { USER_MARKETING, USER_MERCH_STORE_MANAGER, USER_MERCH_STORE_DISTRIBUTOR, + USER_VISIBLE_RESUME, + USER_HIDDEN_RESUME, ...otherMembers, ) .createEvents( @@ -642,6 +652,8 @@ async function seed(): Promise { .orderMerch(MEMBER_SOPHOMORE, [{ option: MERCH_ITEM_2_OPTION_2X2, quantity: 1 }], ONGOING_ORDER_PICKUP_EVENT) .orderMerch(MEMBER_JUNIOR, [{ option: MERCH_ITEM_2_OPTION_4X4, quantity: 2 }], ONGOING_ORDER_PICKUP_EVENT) .orderMerch(MEMBER_SENIOR, [{ option: MERCH_ITEM_2_OPTION_3X3, quantity: 1 }], ONGOING_ORDER_PICKUP_EVENT) + .createResumes(USER_VISIBLE_RESUME, RESUME_1) + .createResumes(USER_HIDDEN_RESUME, RESUME_2) .write(); } diff --git a/tests/data/index.ts b/tests/data/index.ts index e760ea5a..be09679f 100644 --- a/tests/data/index.ts +++ b/tests/data/index.ts @@ -3,5 +3,6 @@ export * from './DatabaseConnection'; export * from './UserFactory'; export * from './EventFactory'; export * from './MerchFactory'; +export * from './ResumeFactory'; export * from './PortalState'; From 89426fcb951ca2c92b0bc23bcf7259846e0f99eb Mon Sep 17 00:00:00 2001 From: yimmyj <69327109+yimmyj@users.noreply.github.com> Date: Tue, 7 Nov 2023 17:04:13 -0800 Subject: [PATCH 02/30] added 'await' in front of eventrepository call (#356) * added 'await' in front of eventrepository call * deleted yarn.lock * removed brewfile and extra line in package.json * added yarn.lock back lols * removed yarn.lock * removed yarn.lock file from gitignore, updated yarn.lock contents --- services/EventService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/EventService.ts b/services/EventService.ts index 6d4c7bc8..0778d424 100644 --- a/services/EventService.ts +++ b/services/EventService.ts @@ -24,7 +24,7 @@ export default class EventService { public async create(event: Event): Promise { const eventCreated = await this.transactions.readWrite(async (txn) => { const eventRepository = Repositories.event(txn); - const isUnusedAttendanceCode = eventRepository.isUnusedAttendanceCode(event.attendanceCode); + const isUnusedAttendanceCode = await eventRepository.isUnusedAttendanceCode(event.attendanceCode); if (!isUnusedAttendanceCode) throw new UserError('Attendance code has already been used'); if (event.start > event.end) throw new UserError('Start date after end date'); return eventRepository.upsertEvent(EventModel.create(event)); From a45a68833854068aa3a6cceee59700f84114c308 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Sun, 12 Nov 2023 23:42:36 -0800 Subject: [PATCH 03/30] added rule for unhandled promises (#372) --- .eslintrc.yml | 2 ++ services/EventService.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index 87fe33ac..3115f6a8 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -8,6 +8,8 @@ env: node: true jest: true rules: + "@typescript-eslint/no-misused-promises": + - "error" "@typescript-eslint/no-unused-vars": - "error" - argsIgnorePattern: "type|_*" diff --git a/services/EventService.ts b/services/EventService.ts index 0778d424..bdcdc352 100644 --- a/services/EventService.ts +++ b/services/EventService.ts @@ -68,7 +68,7 @@ export default class EventService { const currentEvent = await eventRepository.findByUuid(uuid); if (!currentEvent) throw new NotFoundError('Event not found'); if (changes.attendanceCode !== currentEvent.attendanceCode) { - const isUnusedAttendanceCode = eventRepository.isUnusedAttendanceCode(changes.attendanceCode); + const isUnusedAttendanceCode = await eventRepository.isUnusedAttendanceCode(changes.attendanceCode); if (!isUnusedAttendanceCode) throw new UserError('Attendance code has already been used'); } return eventRepository.upsertEvent(currentEvent, changes); From 048e9805f0819ae569190a451bf3f90152b3b55b Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Tue, 28 Nov 2023 18:34:46 -0800 Subject: [PATCH 04/30] User Social Media Link Seed Data (#365) * social media seed data * uh * done * cleanup * linting is the death of me --- tests/Seeds.ts | 67 +++++++++++++++++++++++++++++++++++++++++++-- tests/data/index.ts | 1 + 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/tests/Seeds.ts b/tests/Seeds.ts index 74ba5023..a15a8cdf 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -1,6 +1,7 @@ import * as moment from 'moment'; -import { UserAccessType } from '../types'; -import { DatabaseConnection, EventFactory, MerchFactory, PortalState, UserFactory, ResumeFactory } from './data'; +import { UserAccessType, SocialMediaType } from '../types'; +import { DatabaseConnection, EventFactory, MerchFactory, + PortalState, UserFactory, ResumeFactory, UserSocialMediaFactory } from './data'; function getGraduationYear(n: number) { return moment().year() + n; @@ -116,6 +117,56 @@ async function seed(): Promise { accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR, }); + // Used for testing various User Social Media + const USER_SOCIAL_MEDIA_1 = UserFactory.fake(); + const USER_SOCIAL_MEDIA_1_FACEBOOK = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_1, type: SocialMediaType.FACEBOOK }, + ); + + const USER_SOCIAL_MEDIA_2 = UserFactory.fake(); + const USER_SOCIAL_MEDIA_2_FACEBOOK = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_2, type: SocialMediaType.FACEBOOK }, + ); + const USER_SOCIAL_MEDIA_2_GITHUB = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_2, type: SocialMediaType.GITHUB }, + ); + + const USER_SOCIAL_MEDIA_3 = UserFactory.fake(); + const USER_SOCIAL_MEDIA_3_FACEBOOK = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_3, type: SocialMediaType.FACEBOOK }, + ); + const USER_SOCIAL_MEDIA_3_INSTAGRAM = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_3, type: SocialMediaType.INSTAGRAM }, + ); + const USER_SOCIAL_MEDIA_3_LINKEDIN = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_3, type: SocialMediaType.LINKEDIN }, + ); + + const USER_SOCIAL_MEDIA_ALL = UserFactory.fake(); + const USER_SOCIAL_MEDIA_ALL_FACEBOOK = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.FACEBOOK }, + ); + const USER_SOCIAL_MEDIA_ALL_GITHUB = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.GITHUB }, + ); + const USER_SOCIAL_MEDIA_ALL_INSTAGRAM = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.INSTAGRAM }, + ); + const USER_SOCIAL_MEDIA_ALL_LINKEDIN = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.LINKEDIN }, + ); + const USER_SOCIAL_MEDIA_ALL_DEVPOST = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.DEVPOST }, + ); + const USER_SOCIAL_MEDIA_ALL_TWITTER = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.TWITTER }, + ); + const USER_SOCIAL_MEDIA_ALL_PORTFOLIO = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.PORTFOLIO }, + ); + const USER_SOCIAL_MEDIA_ALL_EMAIL = UserSocialMediaFactory.fake( + { user: USER_SOCIAL_MEDIA_ALL, type: SocialMediaType.EMAIL }, + ); const RESUME_URL = 'https://acmucsd-local.s3.us-west-1.amazonaws.com/resumeSeedingData/alexface.pdf'; const USER_VISIBLE_RESUME = UserFactory.fake(); @@ -557,6 +608,10 @@ async function seed(): Promise { USER_MARKETING, USER_MERCH_STORE_MANAGER, USER_MERCH_STORE_DISTRIBUTOR, + USER_SOCIAL_MEDIA_1, + USER_SOCIAL_MEDIA_2, + USER_SOCIAL_MEDIA_3, + USER_SOCIAL_MEDIA_ALL, USER_VISIBLE_RESUME, USER_HIDDEN_RESUME, ...otherMembers, @@ -652,6 +707,14 @@ async function seed(): Promise { .orderMerch(MEMBER_SOPHOMORE, [{ option: MERCH_ITEM_2_OPTION_2X2, quantity: 1 }], ONGOING_ORDER_PICKUP_EVENT) .orderMerch(MEMBER_JUNIOR, [{ option: MERCH_ITEM_2_OPTION_4X4, quantity: 2 }], ONGOING_ORDER_PICKUP_EVENT) .orderMerch(MEMBER_SENIOR, [{ option: MERCH_ITEM_2_OPTION_3X3, quantity: 1 }], ONGOING_ORDER_PICKUP_EVENT) + .createUserSocialMedia(USER_SOCIAL_MEDIA_1, USER_SOCIAL_MEDIA_1_FACEBOOK) + .createUserSocialMedia(USER_SOCIAL_MEDIA_2, USER_SOCIAL_MEDIA_2_FACEBOOK, USER_SOCIAL_MEDIA_2_GITHUB) + .createUserSocialMedia(USER_SOCIAL_MEDIA_3, USER_SOCIAL_MEDIA_3_FACEBOOK, USER_SOCIAL_MEDIA_3_INSTAGRAM, + USER_SOCIAL_MEDIA_3_LINKEDIN) + .createUserSocialMedia(USER_SOCIAL_MEDIA_ALL, USER_SOCIAL_MEDIA_ALL_FACEBOOK, USER_SOCIAL_MEDIA_ALL_GITHUB, + USER_SOCIAL_MEDIA_ALL_INSTAGRAM, + USER_SOCIAL_MEDIA_ALL_LINKEDIN, USER_SOCIAL_MEDIA_ALL_DEVPOST, USER_SOCIAL_MEDIA_ALL_TWITTER, + USER_SOCIAL_MEDIA_ALL_PORTFOLIO, USER_SOCIAL_MEDIA_ALL_EMAIL) .createResumes(USER_VISIBLE_RESUME, RESUME_1) .createResumes(USER_HIDDEN_RESUME, RESUME_2) .write(); diff --git a/tests/data/index.ts b/tests/data/index.ts index be09679f..4d694557 100644 --- a/tests/data/index.ts +++ b/tests/data/index.ts @@ -3,6 +3,7 @@ export * from './DatabaseConnection'; export * from './UserFactory'; export * from './EventFactory'; export * from './MerchFactory'; +export * from './UserSocialMediaFactory'; export * from './ResumeFactory'; export * from './PortalState'; From 81d7ce5af04c75c826bdd603337f1b52028c6abd Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Sat, 2 Dec 2023 12:49:44 -0800 Subject: [PATCH 05/30] Modify Users' Access Types (#357) * requests/responses types, validator, controller * api request + half of service * core functionality done * fixme note for apiresponses * added admin restrictions * get route and query * fixed bug for email array being out of order * half of unit tests * more tests * validator edits * restruct. api request,checks for duplicate users * fixed tests and linting * renamed request and made duplicate email function * new user map and activity log * fixed tests and linting * fixed typing of initial request * remove unncessary comments and maybe a new type * removed check for demoting admin/updated tests * linting :( * fixed tests and allowed promotion to admin * marcelo is a data structures goat * cleaning up comments * linting * removed unnecessary async/await for checkDupEmails * marcelo's naming changes * linting :( * bumped semver --- api/controllers/AdminController.ts | 22 ++- api/validators/AdminControllerRequests.ts | 22 ++- package.json | 2 +- repositories/UserRepository.ts | 8 + services/PermissionsService.ts | 8 + services/UserAccountService.ts | 65 ++++++++ tests/admin.test.ts | 179 ++++++++++++++++++++++ types/ApiRequests.ts | 11 +- types/ApiResponses.ts | 8 + types/Enums.ts | 1 + 10 files changed, 322 insertions(+), 4 deletions(-) diff --git a/api/controllers/AdminController.ts b/api/controllers/AdminController.ts index 488c79cc..6ab76108 100644 --- a/api/controllers/AdminController.ts +++ b/api/controllers/AdminController.ts @@ -1,9 +1,10 @@ -import { JsonController, Post, UploadedFile, UseBefore, ForbiddenError, Body, Get } from 'routing-controllers'; +import { JsonController, Post, Patch, UploadedFile, UseBefore, ForbiddenError, Body, Get } from 'routing-controllers'; import { UserAuthentication } from '../middleware/UserAuthentication'; import { CreateBonusRequest, CreateMilestoneRequest, SubmitAttendanceForUsersRequest, + ModifyUserAccessLevelRequest, } from '../validators/AdminControllerRequests'; import { File, @@ -13,6 +14,8 @@ import { UploadBannerResponse, GetAllEmailsResponse, SubmitAttendanceForUsersResponse, + ModifyUserAccessLevelResponse, + GetAllUserAccessLevelsResponse, } from '../../types'; import { AuthenticatedUser } from '../decorators/AuthenticatedUser'; import UserAccountService from '../../services/UserAccountService'; @@ -79,4 +82,21 @@ export class AdminController { const attendances = await this.attendanceService.submitAttendanceForUsers(emails, event, asStaff, currentUser); return { error: null, attendances }; } + + @Patch('/access') + async updateUserAccessLevel(@Body() modifyUserAccessLevelRequest: ModifyUserAccessLevelRequest, + @AuthenticatedUser() currentUser: UserModel): Promise { + if (!PermissionsService.canModifyUserAccessLevel(currentUser)) throw new ForbiddenError(); + const { accessUpdates } = modifyUserAccessLevelRequest; + const emails = accessUpdates.map((e) => e.user.toLowerCase()); + const updatedUsers = await this.userAccountService.updateUserAccessLevels(accessUpdates, emails, currentUser); + return { error: null, updatedUsers }; + } + + @Get('/access') + async getAllUsersWithAccessLevels(@AuthenticatedUser() user: UserModel): Promise { + if (!PermissionsService.canSeeAllUserAccessLevels(user)) throw new ForbiddenError(); + const users = await this.userAccountService.getAllFullUserProfiles(); + return { error: null, users }; + } } diff --git a/api/validators/AdminControllerRequests.ts b/api/validators/AdminControllerRequests.ts index ca8ea91b..3739d4d2 100644 --- a/api/validators/AdminControllerRequests.ts +++ b/api/validators/AdminControllerRequests.ts @@ -1,13 +1,18 @@ -import { Allow, ArrayNotEmpty, IsDefined, IsNotEmpty, IsPositive, IsUUID, ValidateNested } from 'class-validator'; +import { Allow, ArrayNotEmpty, IsDefined, IsIn, IsNotEmpty, IsPositive, IsUUID, ValidateNested } from 'class-validator'; import { Type } from 'class-transformer'; import { CreateBonusRequest as ICreateBonusRequest, CreateMilestoneRequest as ICreateMilestoneRequest, SubmitAttendanceForUsersRequest as ISubmitAttendanceForUsersRequest, + ModifyUserAccessLevelRequest as IModifyUserAccessLevelRequest, Milestone as IMilestone, Bonus as IBonus, + UserAccessUpdates as IUserAccessUpdates, + UserAccessType, } from '../../types'; +const validUserAccessTypes = Object.values(UserAccessType); + export class Milestone implements IMilestone { @IsDefined() @IsNotEmpty() @@ -58,3 +63,18 @@ export class SubmitAttendanceForUsersRequest implements ISubmitAttendanceForUser @Allow() asStaff?: boolean; } + +export class UserAccessUpdates implements IUserAccessUpdates { + @IsDefined() + user: string; + + @IsDefined() + @IsIn(validUserAccessTypes) + accessType: UserAccessType; +} +export class ModifyUserAccessLevelRequest implements IModifyUserAccessLevelRequest { + @Type(() => UserAccessUpdates) + @IsDefined() + @ValidateNested() + accessUpdates: UserAccessUpdates[]; +} diff --git a/package.json b/package.json index bcab0b6d..c765afe9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "2.10.1", + "version": "2.11.1", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/repositories/UserRepository.ts b/repositories/UserRepository.ts index 6e738605..17f32eba 100644 --- a/repositories/UserRepository.ts +++ b/repositories/UserRepository.ts @@ -91,4 +91,12 @@ export class UserRepository extends BaseRepository { }) .execute(); } + + public async getUserInfoAndAccessTypes() { + const profiles = await this.repository + .createQueryBuilder() + .select(['uuid', 'handle', 'email', 'UserModel.firstName', 'UserModel.lastName', 'UserModel.accessType']) + .getRawMany(); + return profiles; + } } diff --git a/services/PermissionsService.ts b/services/PermissionsService.ts index d702e7cd..ed1b9421 100644 --- a/services/PermissionsService.ts +++ b/services/PermissionsService.ts @@ -81,4 +81,12 @@ export default class PermissionsService { public static canSeeAllVisibleResumes(user: UserModel) { return user.isAdmin(); } + + public static canModifyUserAccessLevel(user: UserModel): boolean { + return user.isAdmin(); + } + + public static canSeeAllUserAccessLevels(user: UserModel): boolean { + return user.isAdmin(); + } } diff --git a/services/UserAccountService.ts b/services/UserAccountService.ts index fc1a7e0f..3a17d175 100644 --- a/services/UserAccountService.ts +++ b/services/UserAccountService.ts @@ -4,6 +4,7 @@ import { InjectManager } from 'typeorm-typedi-extensions'; import { EntityManager } from 'typeorm'; import * as moment from 'moment'; import * as faker from 'faker'; +import { UserAccessUpdates } from 'api/validators/AdminControllerRequests'; import Repositories, { TransactionsManager } from '../repositories'; import { Uuid, @@ -192,4 +193,68 @@ export default class UserAccountService { return userProfile; }); } + + public checkDuplicateEmails(emails: string[]) { + const emailSet = emails.reduce((set, email) => { + set.add(email); + return set; + }, new Set()); + + if (emailSet.size !== emails.length) { + throw new BadRequestError('Duplicate emails found in request'); + } + } + + public async updateUserAccessLevels(accessUpdates: UserAccessUpdates[], emails: string[], + currentUser: UserModel): Promise { + return this.transactions.readWrite(async (txn) => { + this.checkDuplicateEmails(emails); + + // Strip out the user emails & validate that users exist + const userRepository = Repositories.user(txn); + const users = await userRepository.findByEmails(emails); + const emailsFound = users.map((user) => user.email); + const emailsNotFound = emails.filter((email) => !emailsFound.includes(email)); + + if (emailsNotFound.length > 0) { + throw new BadRequestError(`Couldn't find accounts matching these emails: ${emailsNotFound}`); + } + + const emailToUserMap = users.reduce((map, user) => { + map[user.email] = user; + return map; + }, {}); + + const updatedUsers = await Promise.all(accessUpdates.map(async (accessUpdate, index) => { + const { user: userEmail, accessType } = accessUpdate; + + const currUser = emailToUserMap[userEmail]; + const oldAccess = currUser.accessType; + + const updatedUser = await userRepository.upsertUser(currUser, { accessType }); + + const activity = { + user: currentUser, + type: ActivityType.ACCOUNT_ACCESS_LEVEL_UPDATE, + description: `${currentUser.email} changed ${updatedUser.email}'s + access level from ${oldAccess} to ${accessType}`, + }; + + await Repositories + .activity(txn) + .logActivity(activity); + + return updatedUser; + })); + + return updatedUsers; + }); + } + + public async getAllFullUserProfiles(): Promise { + const users = await this.transactions.readOnly(async (txn) => Repositories + .user(txn) + .findAll()); + return users.map((user) => user.getFullUserProfile()); + } } diff --git a/tests/admin.test.ts b/tests/admin.test.ts index 81d80e00..021f8c6d 100644 --- a/tests/admin.test.ts +++ b/tests/admin.test.ts @@ -1,6 +1,9 @@ +import { BadRequestError, ForbiddenError } from 'routing-controllers'; +import { In } from 'typeorm'; import { ActivityScope, ActivityType, SubmitAttendanceForUsersRequest, UserAccessType } from '../types'; import { ControllerFactory } from './controllers'; import { DatabaseConnection, EventFactory, PortalState, UserFactory } from './data'; +import { UserModel } from '../models/UserModel'; beforeAll(async () => { await DatabaseConnection.connect(); @@ -182,3 +185,179 @@ describe('bonus points submission', () => { expect(getNoBonusUserResponse.user.points).toEqual(0); }); }); + +describe('updating user access level', () => { + test('updates the access level of the user', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + + const staffUser = UserFactory.fake({ accessType: UserAccessType.STAFF }); + const standardUser = UserFactory.fake({ accessType: UserAccessType.STANDARD }); + const marketingUser = UserFactory.fake({ accessType: UserAccessType.MARKETING }); + const merchStoreDistributorUser = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR }); + + await new PortalState() + .createUsers(admin, staffUser, standardUser, marketingUser, merchStoreDistributorUser) + .write(); + + 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 repository = conn.getRepository(UserModel); + const updatedUsers = await repository.find({ + email: In([staffUser.email, standardUser.email, marketingUser.email, merchStoreDistributorUser.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); + }); + + test('attempt to update when user is not an admin', async () => { + const conn = await DatabaseConnection.get(); + const standard = UserFactory.fake({ accessType: UserAccessType.STANDARD }); + + const staffUser = UserFactory.fake({ accessType: UserAccessType.STAFF }); + const standardUser = UserFactory.fake({ accessType: UserAccessType.STANDARD }); + const marketingUser = UserFactory.fake({ accessType: UserAccessType.MARKETING }); + const merchStoreDistributorUser = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR }); + + await new PortalState() + .createUsers(staffUser, standardUser, marketingUser, merchStoreDistributorUser, standard) + .write(); + + const adminController = ControllerFactory.admin(conn); + + await expect(async () => { + 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 }, + ], + }, standard); + }).rejects.toThrow(ForbiddenError); + + const repository = conn.getRepository(UserModel); + const updatedUsers = await repository.find({ + email: In([staffUser.email, standardUser.email, marketingUser.email, merchStoreDistributorUser.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); + }); + + test('attempt to update duplicate users', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + + const userOne = UserFactory.fake({ accessType: UserAccessType.STAFF, email: 'smhariha@ucsd.edu' }); + + await new PortalState() + .createUsers(userOne, admin) + .write(); + + const adminController = ControllerFactory.admin(conn); + + await expect(async () => { + await adminController.updateUserAccessLevel({ + accessUpdates: [ + { user: userOne.email, accessType: UserAccessType.MERCH_STORE_MANAGER }, + { user: userOne.email, accessType: UserAccessType.STAFF }, + ], + }, admin); + }).rejects.toThrow(BadRequestError); + + const repository = conn.getRepository(UserModel); + const updatedUsers = await repository.findOne({ email: userOne.email }); + + expect(updatedUsers.email).toEqual(userOne.email); + expect(updatedUsers.accessType).toEqual(UserAccessType.STAFF); + }); + + test('admin ability to demote another admin', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + + const secondAdmin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + + await new PortalState() + .createUsers(admin, secondAdmin) + .write(); + + const adminController = ControllerFactory.admin(conn); + + const accessLevelResponse = await adminController.updateUserAccessLevel({ + accessUpdates: [ + { user: secondAdmin.email, accessType: UserAccessType.MERCH_STORE_MANAGER }, + ], + }, admin); + + const repository = conn.getRepository(UserModel); + const updatedUser = await repository.findOne({ email: secondAdmin.email }); + + expect(updatedUser.email).toEqual(secondAdmin.email); + expect(updatedUser.accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER); + expect(accessLevelResponse.updatedUsers[0].accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER); + }); + + test("ensure that the updating user's access level is not changed", async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + + const staffUser = UserFactory.fake({ accessType: UserAccessType.STAFF }); + const standardUser = UserFactory.fake({ accessType: UserAccessType.STANDARD }); + const marketingUser = UserFactory.fake({ accessType: UserAccessType.MARKETING }); + const merchStoreDistributorUser = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR }); + + await new PortalState() + .createUsers(staffUser, standardUser, marketingUser, merchStoreDistributorUser, admin) + .write(); + + const adminController = ControllerFactory.admin(conn); + + 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 repository = conn.getRepository(UserModel); + const existingAdmin = await repository.find({ + email: admin.email, + }); + + expect(existingAdmin[0].email).toEqual(admin.email); + expect(existingAdmin[0].accessType).toEqual(UserAccessType.ADMIN); + }); +}); diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index 34769b9e..2f5b50aa 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -1,4 +1,4 @@ -import { FeedbackStatus, FeedbackType, SocialMediaType } from './Enums'; +import { FeedbackStatus, FeedbackType, SocialMediaType, UserAccessType } from './Enums'; import { Uuid } from '.'; // REQUEST TYPES @@ -127,6 +127,15 @@ export interface SubmitAttendanceForUsersRequest { asStaff?: boolean; } +export interface UserAccessUpdates { + user: string; + accessType: UserAccessType; +} + +export interface ModifyUserAccessLevelRequest { + accessUpdates: UserAccessUpdates[]; +} + // EVENT export interface OptionalEventProperties { diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index 1e1057df..c508f7ff 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -39,6 +39,14 @@ export interface SubmitAttendanceForUsersResponse extends ApiResponse { attendances: PublicAttendance[]; } +export interface ModifyUserAccessLevelResponse extends ApiResponse { + updatedUsers: PrivateProfile[]; +} + +export interface GetAllUserAccessLevelsResponse extends ApiResponse { + users: PrivateProfile[]; +} + // ATTENDANCE export interface PublicAttendance { diff --git a/types/Enums.ts b/types/Enums.ts index d4a43288..5185b06b 100644 --- a/types/Enums.ts +++ b/types/Enums.ts @@ -29,6 +29,7 @@ export enum ActivityType { ACCOUNT_RESET_PASS = 'ACCOUNT_RESET_PASS', ACCOUNT_RESET_PASS_REQUEST = 'ACCOUNT_RESET_PASS_REQUEST', ACCOUNT_UPDATE_INFO = 'ACCOUNT_UPDATE_INFO', + ACCOUNT_ACCESS_LEVEL_UPDATE = 'ACCOUNT_ACCESS_LEVEL_UPDATE', ACCOUNT_LOGIN = 'ACCOUNT_LOGIN', ATTEND_EVENT = 'ATTEND_EVENT', ATTEND_EVENT_AS_STAFF = 'ATTEND_EVENT_AS_STAFF', From 0cf49febd6070b0bb8bba00b83ab903ef98e2dea Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Sun, 3 Dec 2023 11:07:20 -0800 Subject: [PATCH 06/30] Update Auto Comment on PRs GitHub Action + PR Template (#375) * updated yml file * crazy new pr template * edited checklist * made it sound nicer --- .github/PULL_REQUEST_TEMPLATE.md | 47 +++++++++++++++++++++++++++++- .github/workflows/auto-comment.yml | 4 +-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e5f86064..ec52bdd8 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1 +1,46 @@ -Closes #[ISSUE-NUM]. +# Info + +Closes **[ISSUE NUMBER]**. (If there is no issue for this pull request yet, please create one or +delete this line if the pull request is for a very minor tweak). + +# Description + +What changes did you make? List all distinct problems that this PR addresses. Explain any relevant +motivation or context. + +[description] + +## Changes + +- [Fill in here] + +# Type of Change + +- [ ] Patch (non-breaking change/bugfix) +- [ ] Minor (non-breaking change which adds functionality) +- [ ] Major (fix or feature that would cause existing functionality to not work as + expected) +- [ ] Documentation (A change to a README/description) +- [ ] Continuous Integration/DevOps Change (Related to deployment steps, continuous integration + workflows, linting, etc.) +- [ ] Other: (Fill In) + +If you've selected Patch, Minor, or Major as your change type, **make sure to bump the version before merging in `package.json`!** +# Testing + +I have tested that my changes fully resolve the linked issue ... + +- [ ] locally. +- [ ] on the testing API/testing database. +- [ ] with appropriate Postman routes. Screenshots are included below. + +# Checklist + +- [ ] I have performed a self-review of my own code. +- [ ] I have followed the style guidelines of this project. +- [ ] I have appropriately edited the API version in the `package.json` file. +- [ ] My changes produce no new warnings. + +# Screenshots + +Please include a screenshot of your Postman testing passing successfully. \ No newline at end of file diff --git a/.github/workflows/auto-comment.yml b/.github/workflows/auto-comment.yml index 8fd61929..87ea6f3d 100644 --- a/.github/workflows/auto-comment.yml +++ b/.github/workflows/auto-comment.yml @@ -4,10 +4,10 @@ jobs: run: runs-on: ubuntu-latest steps: - - uses: bubkoo/auto-comment@v1.0.7 + - uses: wow-actions/auto-comment@v1 with: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - pullRequestOpened: > + pullRequestOpened: | Thanks for contributing! If you've made changes to the API's functionality, please make sure to bump the package version—see [this guide to semantic versioning](https://semver.org/) for details—and From f0a5d4e66ad08453db3bc6309d18487bbc917606 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Fri, 8 Dec 2023 17:50:49 -0800 Subject: [PATCH 07/30] update codeowners (#377) --- .github/CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3e1b8778..c25a1a2c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @sumeet-bansal @shravanhariharan2 @dowhep +* @sumeet-bansal @shravanhariharan2 @dowhep @nik-dange \ No newline at end of file From 03a4c3b1fec6125ea188c43da227d31ed5ff9895 Mon Sep 17 00:00:00 2001 From: Max Weng <73797155+maxwn04@users.noreply.github.com> Date: Sun, 31 Dec 2023 10:29:04 -0800 Subject: [PATCH 08/30] Get another user's event attendance (#358) * attendences from user uuid * lint and bugfix * check same user * controller factory changes * lint fixes * unit test for get attendance by uuid * lint * add permision * add types * add everything else * rename migrtion * test when permision is off * lint * forgor to add * change permission name and fix logic a bit * rename permission, change patch user * lint fix * lint fix * oops * check user exists * lint * rename tests * public profile change * change user model * lint * tests * lint * updated api version --------- Co-authored-by: Nikhil Dange --- api/controllers/AttendanceController.ts | 12 ++++- api/validators/UserControllerRequests.ts | 3 ++ ...d-userAttendancePermission-to-userTable.ts | 17 ++++++ models/UserModel.ts | 5 ++ package.json | 2 +- services/AttendanceService.ts | 14 ++++- tests/attendance.test.ts | 54 +++++++++++++++++++ tests/auth.test.ts | 1 + tests/data/UserFactory.ts | 1 + types/ApiRequests.ts | 1 + types/ApiResponses.ts | 1 + 11 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 migrations/0037-add-userAttendancePermission-to-userTable.ts diff --git a/api/controllers/AttendanceController.ts b/api/controllers/AttendanceController.ts index 50e5b17c..5487c98a 100644 --- a/api/controllers/AttendanceController.ts +++ b/api/controllers/AttendanceController.ts @@ -27,7 +27,17 @@ export class AttendanceController { @Get() async getAttendancesForCurrentUser(@AuthenticatedUser() user: UserModel): Promise { - const attendances = await this.attendanceService.getAttendancesForUser(user); + const attendances = await this.attendanceService.getAttendancesForCurrentUser(user); + return { error: null, attendances }; + } + + @Get('/user/:uuid') + async getAttendancesForUser(@Params() params: UuidParam, + @AuthenticatedUser() currentUser: UserModel): Promise { + if (params.uuid === currentUser.uuid) { + return this.getAttendancesForCurrentUser(currentUser); + } + const attendances = await this.attendanceService.getAttendancesForUser(params.uuid); return { error: null, attendances }; } diff --git a/api/validators/UserControllerRequests.ts b/api/validators/UserControllerRequests.ts index e21267ca..f0b03f9b 100644 --- a/api/validators/UserControllerRequests.ts +++ b/api/validators/UserControllerRequests.ts @@ -44,6 +44,9 @@ export class UserPatches implements IUserPatches { @Allow() bio?: string; + @Allow() + isAttendancePublic?: boolean; + @Type(() => PasswordUpdate) @ValidateNested() @HasMatchingPasswords() diff --git a/migrations/0037-add-userAttendancePermission-to-userTable.ts b/migrations/0037-add-userAttendancePermission-to-userTable.ts new file mode 100644 index 00000000..dfd053fe --- /dev/null +++ b/migrations/0037-add-userAttendancePermission-to-userTable.ts @@ -0,0 +1,17 @@ +import { MigrationInterface, QueryRunner, TableColumn } from 'typeorm'; + +const TABLE_NAME = 'Users'; + +export class AddUserAttendancePermissionToUserTable1691286073346 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.addColumn(TABLE_NAME, new TableColumn({ + name: 'isAttendancePublic', + type: 'boolean', + default: true, + })); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropColumn(TABLE_NAME, 'isAttendancePublic'); + } +} diff --git a/models/UserModel.ts b/models/UserModel.ts index 334d5575..a2cd0114 100644 --- a/models/UserModel.ts +++ b/models/UserModel.ts @@ -55,6 +55,9 @@ export class UserModel extends BaseEntity { }) bio: string; + @Column('boolean', { default: true }) + isAttendancePublic: boolean; + @Column('integer', { default: 0 }) @Index('leaderboard_index') points: number; @@ -126,6 +129,7 @@ export class UserModel extends BaseEntity { major: this.major, bio: this.bio, points: this.points, + isAttendancePublic: this.isAttendancePublic, }; if (this.userSocialMedia) { publicProfile.userSocialMedia = this.userSocialMedia.map((sm) => sm.getPublicSocialMedia()); @@ -148,6 +152,7 @@ export class UserModel extends BaseEntity { bio: this.bio, points: this.points, credits: this.credits, + isAttendancePublic: this.isAttendancePublic, }; if (this.userSocialMedia) { fullUserProfile.userSocialMedia = this.userSocialMedia.map((sm) => sm.getPublicSocialMedia()); diff --git a/package.json b/package.json index c765afe9..542e7dab 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "2.11.1", + "version": "2.12.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/services/AttendanceService.ts b/services/AttendanceService.ts index 6f0e171b..332d1069 100644 --- a/services/AttendanceService.ts +++ b/services/AttendanceService.ts @@ -1,6 +1,6 @@ import { Service } from 'typedi'; import { InjectManager } from 'typeorm-typedi-extensions'; -import { BadRequestError, NotFoundError } from 'routing-controllers'; +import { BadRequestError, ForbiddenError, NotFoundError } from 'routing-controllers'; import { EntityManager } from 'typeorm'; import * as moment from 'moment'; import { ActivityType, PublicAttendance, Uuid } from '../types'; @@ -27,13 +27,23 @@ export default class AttendanceService { return attendances.map((attendance) => attendance.getPublicAttendance()); } - public async getAttendancesForUser(user: UserModel): Promise { + public async getAttendancesForCurrentUser(user: UserModel): Promise { const attendances = await this.transactions.readOnly(async (txn) => Repositories .attendance(txn) .getAttendancesForUser(user)); return attendances.map((attendance) => attendance.getPublicAttendance()); } + public async getAttendancesForUser(uuid: Uuid): Promise { + return this.transactions.readOnly(async (txn) => { + const user = await Repositories.user(txn).findByUuid(uuid); + if (!user) throw new NotFoundError('User does not exist'); + if (!user.isAttendancePublic) throw new ForbiddenError(); + const attendances = await Repositories.attendance(txn).getAttendancesForUser(user); + return attendances.map((attendance) => attendance.getPublicAttendance()); + }); + } + public async attendEvent(user: UserModel, attendanceCode: string, asStaff = false): Promise { return this.transactions.readWrite(async (txn) => { const event = await Repositories.event(txn).findByAttendanceCode(attendanceCode); diff --git a/tests/attendance.test.ts b/tests/attendance.test.ts index a8b8f583..62b1b55a 100644 --- a/tests/attendance.test.ts +++ b/tests/attendance.test.ts @@ -272,4 +272,58 @@ describe('attendance', () => { expect(attendance.user.uuid).toEqual(staff.uuid); expect(attendance.event.uuid).toEqual(event.uuid); }); + + test('get another user attendance by uuid', async () => { + const conn = await DatabaseConnection.get(); + const member1 = UserFactory.fake(); + const member2 = UserFactory.fake(); + const event1 = EventFactory.fake({ requiresStaff: true }); + const event2 = EventFactory.fake({ requiresStaff: true }); + + await new PortalState() + .createUsers(member1, member2) + .createEvents(event1, event2) + .attendEvents([member1], [event1, event2]) + .write(); + + const attendanceController = ControllerFactory.attendance(conn); + const params = { uuid: member1.uuid }; + + // returns all attendances for uuid + const getAttendancesForUserUuid = await attendanceController.getAttendancesForUser(params, member2); + const attendancesForEvent = getAttendancesForUserUuid.attendances.map((a) => ({ + user: a.user.uuid, + event: a.event.uuid, + asStaff: a.asStaff, + })); + const expectedAttendances = [ + { event: event1.uuid, user: member1.uuid, asStaff: false }, + { event: event2.uuid, user: member1.uuid, asStaff: false }, + ]; + expect(attendancesForEvent).toEqual(expect.arrayContaining(expectedAttendances)); + }); + + test('throws error when isAttendancePublic is false', async () => { + const conn = await DatabaseConnection.get(); + const member1 = UserFactory.fake(); + const member2 = UserFactory.fake(); + const event1 = EventFactory.fake({ requiresStaff: true }); + const event2 = EventFactory.fake({ requiresStaff: true }); + + await new PortalState() + .createUsers(member1, member2) + .createEvents(event1, event2) + .attendEvents([member1, member2], [event1, event2]) + .write(); + + const attendanceController = ControllerFactory.attendance(conn); + const userController = ControllerFactory.user(conn); + const params = { uuid: member1.uuid }; + + const changePublicAttendancePatch = { user: { isAttendancePublic: false } }; + await userController.patchCurrentUser(changePublicAttendancePatch, member1); + + await expect(attendanceController.getAttendancesForUser(params, member2)) + .rejects.toThrow(ForbiddenError); + }); }); diff --git a/tests/auth.test.ts b/tests/auth.test.ts index 3fb30542..79476dc6 100644 --- a/tests/auth.test.ts +++ b/tests/auth.test.ts @@ -59,6 +59,7 @@ describe('account registration', () => { uuid: registerResponse.user.uuid, profilePicture: null, userSocialMedia: [], + isAttendancePublic: true, }); // check that email verification is sent diff --git a/tests/data/UserFactory.ts b/tests/data/UserFactory.ts index 69c60bae..17017e01 100644 --- a/tests/data/UserFactory.ts +++ b/tests/data/UserFactory.ts @@ -43,6 +43,7 @@ export class UserFactory { points: 0, credits: 0, handle: UserAccountService.generateDefaultHandle(firstName, lastName), + isAttendancePublic: true, }); return UserModel.merge(fake, substitute); } diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index 2f5b50aa..ff271f3b 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -66,6 +66,7 @@ export interface UserPatches { major?: string; graduationYear?: number; bio?: string; + isAttendancePublic?: boolean; passwordChange?: PasswordUpdate; } diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index c508f7ff..97d25732 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -310,6 +310,7 @@ export interface PublicProfile { bio: string, points: number, userSocialMedia?: PublicUserSocialMedia[]; + isAttendancePublic: boolean, } export interface PrivateProfile extends PublicProfile { From 57f6c4ad4e3888651bd0afb65f99b59dd3226391 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Sun, 31 Dec 2023 10:47:39 -0800 Subject: [PATCH 09/30] staging deployment workflow (#381) --- .circleci/config.yml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4e1f2f10..5f06099b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -119,6 +119,17 @@ jobs: GIT_HASH=$(echo $CIRCLE_SHA1 | cut -c -7) docker run caprover/cli-caprover:v2.1.1 caprover deploy --caproverUrl https://captain.caprover.acmucsd.com --caproverPassword $CAPROVER_PASS --caproverApp membership-portal-api-testing --imageName acmucsd/membership-portal-api:$GIT_HASH docker run caprover/cli-caprover:v2.1.1 caprover deploy --caproverUrl https://captain.caprover.acmucsd.com --caproverPassword $CAPROVER_PASS --caproverApp membership-portal-api --imageName acmucsd/membership-portal-api:$GIT_HASH + deploy_staging: + environment: + GIT_HASH: $(echo $CIRCLE_SHA1 | cut -c -7) + machine: + enabled: true + steps: + - checkout + - run: + command: | + GIT_HASH=$(echo $CIRCLE_SHA1 | cut -c -7) + docker run caprover/cli-caprover:v2.1.1 caprover deploy --caproverUrl https://captain.caprover.acmucsd.com --caproverPassword $CAPROVER_PASS --caproverApp membership-portal-api-testing --imageName acmucsd/membership-portal-api:$GIT_HASH workflows: test_and_deploy: @@ -133,7 +144,9 @@ workflows: - test filters: branches: - only: master + only: + - master + - staging - deploy: requires: - build @@ -143,3 +156,12 @@ workflows: filters: branches: only: master + - deploy_staging: + requires: + - build + - lint + - test + - image + filters: + branches: + only: staging \ No newline at end of file From f658e57f7ca0c1085de6cc67464d59153922ebd8 Mon Sep 17 00:00:00 2001 From: "Caogang (Marcelo) Shen" Date: Sun, 31 Dec 2023 12:29:45 -0800 Subject: [PATCH 10/30] Create Multiple Gallery Store Images (#351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * started migration * almost completed the query * change MerchandiseItemModel picture to pictures * renamed Merchandise to MercahndiseItems * generated a new model for the pictures * fixed syntax error with migration * understand and fix casting error * edited requests and created some todos * return position == 0 instead of first picture in array * migration temp fix * this one line of code would attach pictures to the collection so that the frontend can display the first picture * edited some todos to implement a new idea * edited the service * created a repository for photos * Completed the photo create route * completed the photo deletion route * getting started with seeding * make sure the index is consistent * removed the current file name from url for security purpose * quick linting * edited seeding to ensure correctness * update MerchFactory item for photo support * refactor and renaming variables * wrote outline for test and rewrote a method * fix error * edits * removing some junk code * the error is playing hide and seek with me * im such a genius * removing partial debug msgs * edits * fixed the order item test * finished creating tests and pass all tests * fixed some error * I CHATGPTED THE SQL AND IT WORKED * fixed linting error * edit migration number order * clean up some unused variables * renamed picture to uploadedPhoto and photo to merchPhoto for clarity, added some documentation * remove magic number * slight seeding edit * removed position logic * clarify cascading quetsion * fixed cascade * clean up * clarify seeding data structure * link fix * change position in request to string because form data does not accept number * throw error if position is not a number * updated deletion logic to delete from s3 first * link fix * default url logic fix for positions no longer being 0 * Test automated migrations–get another user's attendance (#379) * attendences from user uuid * lint and bugfix * check same user * controller factory changes * lint fixes * unit test for get attendance by uuid * lint * add permision * add types * add everything else * rename migrtion * test when permision is off * lint * forgor to add * change permission name and fix logic a bit * rename permission, change patch user * lint fix * lint fix * oops * check user exists * lint * new ci --------- Co-authored-by: Max Weng * bumped my migration file number * bumped my migration file number v2 * remove local settings.json change * added edge case for migration up * lint * lint * such developer velocity * removed debugging --------- Co-authored-by: Nikhil Dange Co-authored-by: Max Weng Co-authored-by: Nikhil Dange --- api/controllers/MerchStoreController.ts | 45 ++++- api/validators/MerchStoreRequests.ts | 34 +++- migrations/0038-add-merch-item-image-table.ts | 88 +++++++++ models/MerchandiseItemModel.ts | 20 +- models/MerchandiseItemOptionModel.ts | 2 +- models/MerchandiseItemPhotoModel.ts | 34 ++++ models/index.ts | 2 + package.json | 2 +- repositories/MerchOrderRepository.ts | 11 +- repositories/MerchStoreRepository.ts | 34 +++- repositories/index.ts | 11 +- services/MerchStoreService.ts | 101 +++++++++- services/StorageService.ts | 14 ++ tests/Seeds.ts | 85 ++++++++- tests/data/MerchFactory.ts | 27 ++- tests/merchStore.test.ts | 175 +++++++++++++++++- types/ApiRequests.ts | 17 +- types/ApiResponses.ts | 17 +- 18 files changed, 676 insertions(+), 43 deletions(-) create mode 100644 migrations/0038-add-merch-item-image-table.ts create mode 100644 models/MerchandiseItemPhotoModel.ts diff --git a/api/controllers/MerchStoreController.ts b/api/controllers/MerchStoreController.ts index 5c9b4268..bfaca0b9 100644 --- a/api/controllers/MerchStoreController.ts +++ b/api/controllers/MerchStoreController.ts @@ -12,6 +12,7 @@ import { BadRequestError, UploadedFile, } from 'routing-controllers'; +import { v4 as uuid } from 'uuid'; import PermissionsService from '../../services/PermissionsService'; import { UserAuthentication } from '../middleware/UserAuthentication'; import { @@ -41,7 +42,8 @@ import { CancelAllPendingOrdersResponse, MediaType, File, - UpdateMerchPhotoResponse, + CreateMerchPhotoResponse, + DeleteMerchItemPhotoResponse, CompleteOrderPickupEventResponse, GetOrderPickupEventResponse, CancelOrderPickupEventResponse, @@ -60,6 +62,7 @@ import { FulfillMerchOrderRequest, RescheduleOrderPickupRequest, CreateMerchItemOptionRequest, + CreateMerchItemPhotoRequest, CreateOrderPickupEventRequest, EditOrderPickupEventRequest, GetCartRequest, @@ -158,14 +161,36 @@ export class MerchStoreController { @UseBefore(UserAuthentication) @Post('/item/picture/:uuid') - async updateMerchPhoto(@UploadedFile('image', + async createMerchItemPhoto(@UploadedFile('image', { options: StorageService.getFileOptions(MediaType.MERCH_PHOTO) }) file: File, @Params() params: UuidParam, - @AuthenticatedUser() user: UserModel): Promise { + @Body() createItemPhotoRequest: CreateMerchItemPhotoRequest, + @AuthenticatedUser() user: UserModel): Promise { if (!PermissionsService.canEditMerchStore(user)) throw new ForbiddenError(); - const picture = await this.storageService.upload(file, MediaType.MERCH_PHOTO, params.uuid); - const item = await this.merchStoreService.editItem(params.uuid, { picture }); - return { error: null, item }; + + // generate a random string for the uploaded photo url + const position = Number(createItemPhotoRequest.position); + if (Number.isNaN(position)) throw new BadRequestError('Position is not a number'); + const uniqueFileName = uuid(); + const uploadedPhoto = await this.storageService.uploadToFolder( + file, MediaType.MERCH_PHOTO, uniqueFileName, params.uuid, + ); + const merchPhoto = await this.merchStoreService.createItemPhoto( + params.uuid, { uploadedPhoto, position }, + ); + + return { error: null, merchPhoto }; + } + + @UseBefore(UserAuthentication) + @Delete('/item/picture/:uuid') + async deleteMerchItemPhoto(@Params() params: UuidParam, @AuthenticatedUser() user: UserModel): + Promise { + if (!PermissionsService.canEditMerchStore(user)) throw new ForbiddenError(); + const photoToDelete = await this.merchStoreService.getItemPhotoForDeletion(params.uuid); + await this.storageService.deleteAtUrl(photoToDelete.uploadedPhoto); + await this.merchStoreService.deleteItemPhoto(photoToDelete); + return { error: null }; } @Post('/option/:uuid') @@ -189,9 +214,11 @@ export class MerchStoreController { async getOneMerchOrder(@Params() params: UuidParam, @AuthenticatedUser() user: UserModel): Promise { if (!PermissionsService.canAccessMerchStore(user)) throw new ForbiddenError(); - const order = await this.merchStoreService.findOrderByUuid(params.uuid); - if (!PermissionsService.canSeeMerchOrder(user, order)) throw new NotFoundError(); - return { error: null, order: order.getPublicOrderWithItems() }; + // get "public" order bc canSeeMerchOrder need singular merchPhoto field + // default order has merchPhotos field, which cause incorrect casting + const publicOrder = (await this.merchStoreService.findOrderByUuid(params.uuid)).getPublicOrderWithItems(); + if (!PermissionsService.canSeeMerchOrder(user, publicOrder)) throw new NotFoundError(); + return { error: null, order: publicOrder }; } @Get('/orders/all') diff --git a/api/validators/MerchStoreRequests.ts b/api/validators/MerchStoreRequests.ts index 78fc6283..3d8f0ac4 100644 --- a/api/validators/MerchStoreRequests.ts +++ b/api/validators/MerchStoreRequests.ts @@ -10,6 +10,7 @@ import { IsDateString, ArrayNotEmpty, IsNumber, + IsNumberString, } from 'class-validator'; import { Type } from 'class-transformer'; import { @@ -18,6 +19,7 @@ import { CreateMerchItemRequest as ICreateMerchItemRequest, EditMerchItemRequest as IEditMerchItemRequest, CreateMerchItemOptionRequest as ICreateMerchItemOptionRequest, + CreateMerchItemPhotoRequest as ICreateMerchItemPhotoRequest, PlaceMerchOrderRequest as IPlaceMerchOrderRequest, VerifyMerchOrderRequest as IVerifyMerchOrderRequest, FulfillMerchOrderRequest as IFulfillMerchOrderRequest, @@ -34,6 +36,8 @@ import { MerchItemOption as IMerchItemOption, MerchItemOptionEdit as IMerchItemOptionEdit, MerchItemOptionMetadata as IMerchItemOptionMetadata, + MerchItemPhoto as IMerchItemPhoto, + MerchItemPhotoEdit as IMerchItemPhotoEdit, MerchOrderEdit as IMerchOrderEdit, OrderPickupEvent as IOrderPickupEvent, OrderPickupEventEdit as IOrderPickupEventEdit, @@ -129,6 +133,26 @@ export class MerchItemOptionEdit implements IMerchItemOptionEdit { metadata?: MerchItemOptionMetadata; } +export class MerchItemPhoto implements IMerchItemPhoto { + @Allow() + uploadedPhoto: string; + + @IsNumber() + position: number; +} + +export class MerchItemPhotoEdit implements IMerchItemPhotoEdit { + @IsDefined() + @IsUUID() + uuid: string; + + @Allow() + uploadedPhoto?: string; + + @IsNumber() + position?: number; +} + export class MerchItem implements IMerchItem { @IsDefined() @IsNotEmpty() @@ -142,7 +166,7 @@ export class MerchItem implements IMerchItem { description: string; @Allow() - picture?: string; + merchPhotos: MerchItemPhoto[]; @Min(0) quantity?: number; @@ -177,7 +201,7 @@ export class MerchItemEdit implements IMerchItemEdit { description?: string; @Allow() - picture?: string; + merchPhotos?: MerchItemPhotoEdit[]; @Allow() hidden?: boolean; @@ -293,6 +317,12 @@ export class CreateMerchItemOptionRequest implements ICreateMerchItemOptionReque option: MerchItemOption; } +export class CreateMerchItemPhotoRequest implements ICreateMerchItemPhotoRequest { + @IsDefined() + @IsNumberString() + position: string; +} + export class PlaceMerchOrderRequest implements IPlaceMerchOrderRequest { @Type(() => MerchItemOptionAndQuantity) @ValidateNested() diff --git a/migrations/0038-add-merch-item-image-table.ts b/migrations/0038-add-merch-item-image-table.ts new file mode 100644 index 00000000..5802a986 --- /dev/null +++ b/migrations/0038-add-merch-item-image-table.ts @@ -0,0 +1,88 @@ +import { MigrationInterface, QueryRunner, Table, TableColumn } from 'typeorm'; + +const TABLE_NAME = 'MerchandiseItemPhotos'; +const MERCH_TABLE_NAME = 'MerchandiseItems'; + +export class AddMerchItemImageTable1691286073347 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + // instantiates table with columns: uuid, merchItem, uploadedPhoto, uploadedAt, position + await queryRunner.createTable(new Table({ + name: TABLE_NAME, + columns: [ + { + name: 'uuid', + type: 'uuid', + isGenerated: true, + isPrimary: true, + generationStrategy: 'uuid', + }, + { + name: 'merchItem', + type: 'uuid', + }, + { + name: 'uploadedPhoto', + type: 'varchar(255)', + }, + { + name: 'uploadedAt', + type: 'timestamptz', + default: 'CURRENT_TIMESTAMP(6)', + }, + { + name: 'position', + type: 'integer', + }, + ], + // optimize searching + indices: [ + { + name: 'images_by_item_index', + columnNames: ['merchItem'], + }, + ], + // cascade delete + foreignKeys: [ + { + columnNames: ['merchItem'], + referencedTableName: MERCH_TABLE_NAME, + referencedColumnNames: ['uuid'], + onDelete: 'CASCADE', + }, + ], + })); + + // add images from each item of the merchandise table to the photo table + await queryRunner.query( + `INSERT INTO "${TABLE_NAME}" ("merchItem", "uploadedPhoto", position) ` + + `SELECT uuid, picture, 0 AS position FROM "${MERCH_TABLE_NAME}" ` + + 'WHERE picture IS NOT NULL', + ); + + // remove the column from the old table + await queryRunner.dropColumn(`${MERCH_TABLE_NAME}`, 'picture'); + } + + public async down(queryRunner: QueryRunner): Promise { + // create old column (copied from migration #7) + await queryRunner.addColumn(`${MERCH_TABLE_NAME}`, new TableColumn({ + name: 'picture', + type: 'varchar(255)', + isNullable: true, + })); + + // fill old column with the first image from the photo table + await queryRunner.query( + `UPDATE "${MERCH_TABLE_NAME}" m ` + + 'SET picture = (' + + 'SELECT "uploadedPhoto" ' + + `FROM "${TABLE_NAME}" p ` + + 'WHERE p."merchItem" = m.uuid ' + + 'ORDER BY p."uploadedAt" ' + + 'LIMIT 1' + + ')', + ); + + await queryRunner.dropTable(TABLE_NAME); + } +} diff --git a/models/MerchandiseItemModel.ts b/models/MerchandiseItemModel.ts index 8ba7fd48..e27971d3 100644 --- a/models/MerchandiseItemModel.ts +++ b/models/MerchandiseItemModel.ts @@ -4,6 +4,7 @@ import { import { Uuid, PublicMerchItem, PublicCartMerchItem } from '../types'; import { MerchandiseCollectionModel } from './MerchandiseCollectionModel'; import { MerchandiseItemOptionModel } from './MerchandiseItemOptionModel'; +import { MerchandiseItemPhotoModel } from './MerchandiseItemPhotoModel'; @Entity('MerchandiseItems') export class MerchandiseItemModel extends BaseEntity { @@ -20,9 +21,6 @@ export class MerchandiseItemModel extends BaseEntity { @JoinColumn({ name: 'collection' }) collection: MerchandiseCollectionModel; - @Column('varchar', { length: 255, nullable: true }) - picture: string; - @Column('text') description: string; @@ -38,6 +36,9 @@ export class MerchandiseItemModel extends BaseEntity { @Column('boolean', { default: false }) hasVariantsEnabled: boolean; + @OneToMany((type) => MerchandiseItemPhotoModel, (merchPhoto) => merchPhoto.merchItem, { cascade: true }) + merchPhotos: MerchandiseItemPhotoModel[]; + @OneToMany((type) => MerchandiseItemOptionModel, (option) => option.item, { cascade: true }) options: MerchandiseItemOptionModel[]; @@ -45,7 +46,7 @@ export class MerchandiseItemModel extends BaseEntity { const baseMerchItem: PublicMerchItem = { uuid: this.uuid, itemName: this.itemName, - picture: this.picture, + merchPhotos: this.merchPhotos.map((o) => o.getPublicMerchItemPhoto()).sort((a, b) => a.position - b.position), description: this.description, options: this.options.map((o) => o.getPublicMerchItemOption()), monthlyLimit: this.monthlyLimit, @@ -61,8 +62,17 @@ export class MerchandiseItemModel extends BaseEntity { return { uuid: this.uuid, itemName: this.itemName, - picture: this.picture, + uploadedPhoto: this.getDefaultPhotoUrl(), description: this.description, }; } + + // get the first index of photo if possible + public getDefaultPhotoUrl(): string { + if (this.merchPhotos.length === 0) return null; + return this.merchPhotos.reduce( + (min, current) => ((min.position < current.position) ? min : current), + this.merchPhotos[0], + ).uploadedPhoto; + } } diff --git a/models/MerchandiseItemOptionModel.ts b/models/MerchandiseItemOptionModel.ts index e2c8908f..4da70499 100644 --- a/models/MerchandiseItemOptionModel.ts +++ b/models/MerchandiseItemOptionModel.ts @@ -11,8 +11,8 @@ export class MerchandiseItemOptionModel extends BaseEntity { uuid: Uuid; @ManyToOne((type) => MerchandiseItemModel, (merchItem) => merchItem.options, { nullable: false, onDelete: 'CASCADE' }) - @Index('merch_item_options_index') @JoinColumn({ name: 'item' }) + @Index('merch_item_options_index') item: MerchandiseItemModel; @Column('integer', { default: 0 }) diff --git a/models/MerchandiseItemPhotoModel.ts b/models/MerchandiseItemPhotoModel.ts new file mode 100644 index 00000000..e20334e3 --- /dev/null +++ b/models/MerchandiseItemPhotoModel.ts @@ -0,0 +1,34 @@ +import { BaseEntity, Column, Entity, Index, JoinColumn, ManyToOne, PrimaryGeneratedColumn } from 'typeorm'; +import { PublicMerchItemPhoto, Uuid } from '../types'; +import { MerchandiseItemModel } from './MerchandiseItemModel'; + +@Entity('MerchandiseItemPhotos') +export class MerchandiseItemPhotoModel extends BaseEntity { + @PrimaryGeneratedColumn('uuid') + uuid: Uuid; + + @ManyToOne((type) => MerchandiseItemModel, + (merchItem) => merchItem.merchPhotos, + { nullable: false, onDelete: 'CASCADE' }) + @JoinColumn({ name: 'merchItem' }) + @Index('images_by_item_index') + merchItem: MerchandiseItemModel; + + @Column('varchar', { length: 255, nullable: false }) + uploadedPhoto: string; + + @Column('timestamptz', { default: () => 'CURRENT_TIMESTAMP(6)', nullable: false }) + uploadedAt: Date; + + @Column('integer') + position: number; + + public getPublicMerchItemPhoto(): PublicMerchItemPhoto { + return { + uuid: this.uuid, + uploadedPhoto: this.uploadedPhoto, + uploadedAt: this.uploadedAt, + position: this.position, + }; + } +} diff --git a/models/index.ts b/models/index.ts index 4df9c059..e571f4e2 100644 --- a/models/index.ts +++ b/models/index.ts @@ -4,6 +4,7 @@ import { EventModel } from './EventModel'; import { AttendanceModel } from './AttendanceModel'; import { MerchandiseCollectionModel } from './MerchandiseCollectionModel'; import { MerchandiseItemModel } from './MerchandiseItemModel'; +import { MerchandiseItemPhotoModel } from './MerchandiseItemPhotoModel'; import { OrderModel } from './OrderModel'; import { OrderItemModel } from './OrderItemModel'; import { MerchandiseItemOptionModel } from './MerchandiseItemOptionModel'; @@ -20,6 +21,7 @@ export const models = [ AttendanceModel, MerchandiseCollectionModel, MerchandiseItemModel, + MerchandiseItemPhotoModel, MerchandiseItemOptionModel, OrderModel, OrderItemModel, diff --git a/package.json b/package.json index 542e7dab..5c6ef264 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "2.12.0", + "version": "3.0.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/repositories/MerchOrderRepository.ts b/repositories/MerchOrderRepository.ts index 7cdd1843..0695a149 100644 --- a/repositories/MerchOrderRepository.ts +++ b/repositories/MerchOrderRepository.ts @@ -24,6 +24,7 @@ export class MerchOrderRepository extends BaseRepository { .leftJoinAndSelect('order.user', 'user') .leftJoinAndSelect('orderItem.option', 'option') .leftJoinAndSelect('option.item', 'merchItem') + .leftJoinAndSelect('merchItem.merchPhotos', 'merchPhotos') .where('order.uuid = :uuid', { uuid }) .getOne(); } @@ -52,7 +53,7 @@ export class MerchOrderRepository extends BaseRepository { /** * Gets all orders for a given user. Returns the order joined with its pickup event, user, - * merch item options, and merch items. + * merch item options, merch items, and merch item photos. */ public async getAllOrdersWithItemsForUser(user: UserModel): Promise { return this.repository @@ -62,6 +63,7 @@ export class MerchOrderRepository extends BaseRepository { .leftJoinAndSelect('order.user', 'user') .leftJoinAndSelect('orderItem.option', 'option') .leftJoinAndSelect('option.item', 'merchItem') + .leftJoinAndSelect('merchItem.merchPhotos', 'merchPhotos') .where('order.user = :uuid', { uuid: user.uuid }) .getMany(); } @@ -114,6 +116,7 @@ export class OrderItemRepository extends BaseRepository { .innerJoinAndSelect('oi.order', 'order') .innerJoinAndSelect('order.user', 'user') .innerJoinAndSelect('option.item', 'item') + .innerJoinAndSelect('item.merchPhotos', 'merchPhotos') .where('item.uuid = :itemUuid', { itemUuid: item.uuid }) .andWhere('user.uuid = :userUuid', { userUuid: user.uuid }) .getMany(); @@ -153,6 +156,9 @@ export class OrderPickupEventRepository extends BaseRepository): Promise { if (changes) pickupEvent = OrderPickupEventModel.merge(pickupEvent, changes); @@ -170,7 +176,8 @@ export class OrderPickupEventRepository extends BaseRepository { diff --git a/repositories/MerchStoreRepository.ts b/repositories/MerchStoreRepository.ts index 955eb532..7f5f7e47 100644 --- a/repositories/MerchStoreRepository.ts +++ b/repositories/MerchStoreRepository.ts @@ -1,5 +1,6 @@ import { EntityRepository, SelectQueryBuilder } from 'typeorm'; import { MerchandiseItemOptionModel } from '../models/MerchandiseItemOptionModel'; +import { MerchandiseItemPhotoModel } from '../models/MerchandiseItemPhotoModel'; import { MerchandiseCollectionModel } from '../models/MerchandiseCollectionModel'; import { MerchandiseItemModel } from '../models/MerchandiseItemModel'; import { Uuid } from '../types'; @@ -39,14 +40,15 @@ export class MerchCollectionRepository extends BaseRepository { return this.repository.createQueryBuilder('collection') .leftJoinAndSelect('collection.items', 'items') - .leftJoinAndSelect('items.options', 'options'); + .leftJoinAndSelect('items.options', 'options') + .leftJoinAndSelect('items.merchPhotos', 'merchPhotos'); } } @EntityRepository(MerchandiseItemModel) export class MerchItemRepository extends BaseRepository { public async findByUuid(uuid: Uuid): Promise { - return this.repository.findOne(uuid, { relations: ['collection', 'options'] }); + return this.repository.findOne(uuid, { relations: ['collection', 'options', 'merchPhotos'] }); } public async upsertMerchItem(item: MerchandiseItemModel, changes?: Partial): @@ -74,11 +76,11 @@ export class MerchItemRepository extends BaseRepository { @EntityRepository(MerchandiseItemOptionModel) export class MerchItemOptionRepository extends BaseRepository { public async findByUuid(uuid: Uuid): Promise { - return this.repository.findOne(uuid, { relations: ['item'] }); + return this.repository.findOne(uuid, { relations: ['item', 'item.merchPhotos'] }); } public async batchFindByUuid(uuids: Uuid[]): Promise> { - const options = await this.repository.findByIds(uuids, { relations: ['item'] }); + const options = await this.repository.findByIds(uuids, { relations: ['item', 'item.merchPhotos'] }); return new Map(options.map((o) => [o.uuid, o])); } @@ -106,3 +108,27 @@ export class MerchItemOptionRepository extends BaseRepository { + public async findByUuid(uuid: Uuid): Promise { + return this.repository.findOne(uuid, { relations: ['merchItem'] }); + } + + // for querying a group of photos together + public async batchFindByUuid(uuids: Uuid[]): Promise> { + const merchPhotos = await this.repository.findByIds(uuids, { relations: ['merchItem'] }); + return new Map(merchPhotos.map((o) => [o.uuid, o])); + } + + public async upsertMerchItemPhoto(merchPhoto: MerchandiseItemPhotoModel, + changes?: Partial): Promise { + if (changes) merchPhoto = MerchandiseItemPhotoModel.merge(merchPhoto, changes); + return this.repository.save(merchPhoto); + } + + public async deleteMerchItemPhoto(merchPhoto: MerchandiseItemPhotoModel): Promise { + await this.repository.remove(merchPhoto); + } +} diff --git a/repositories/index.ts b/repositories/index.ts index 2f14390f..02a50c92 100644 --- a/repositories/index.ts +++ b/repositories/index.ts @@ -4,7 +4,12 @@ import { FeedbackRepository } from './FeedbackRepository'; import { AttendanceRepository } from './AttendanceRepository'; import { EventRepository } from './EventRepository'; import { MerchOrderRepository, OrderItemRepository, OrderPickupEventRepository } from './MerchOrderRepository'; -import { MerchCollectionRepository, MerchItemRepository, MerchItemOptionRepository } from './MerchStoreRepository'; +import { + MerchCollectionRepository, + MerchItemRepository, + MerchItemOptionRepository, + MerchItemPhotoRepository, +} from './MerchStoreRepository'; import { ActivityRepository } from './ActivityRepository'; import { LeaderboardRepository } from './LeaderboardRepository'; import { ResumeRepository } from './ResumeRepository'; @@ -47,6 +52,10 @@ export default class Repositories { return transactionalEntityManager.getCustomRepository(MerchItemRepository); } + public static merchStoreItemPhoto(transactionalEntityManager: EntityManager): MerchItemPhotoRepository { + return transactionalEntityManager.getCustomRepository(MerchItemPhotoRepository); + } + public static merchStoreItemOption(transactionalEntityManager: EntityManager): MerchItemOptionRepository { return transactionalEntityManager.getCustomRepository(MerchItemOptionRepository); } diff --git a/services/MerchStoreService.ts b/services/MerchStoreService.ts index d083fd09..ee1b5f76 100644 --- a/services/MerchStoreService.ts +++ b/services/MerchStoreService.ts @@ -23,6 +23,8 @@ import { OrderPickupEventEdit, PublicMerchItemWithPurchaseLimits, OrderPickupEventStatus, + PublicMerchItemPhoto, + MerchItemPhoto, } from '../types'; import { MerchandiseItemModel } from '../models/MerchandiseItemModel'; import { OrderModel } from '../models/OrderModel'; @@ -33,9 +35,12 @@ import EmailService, { OrderInfo, OrderPickupEventInfo } from './EmailService'; import { UserError } from '../utils/Errors'; import { OrderItemModel } from '../models/OrderItemModel'; import { OrderPickupEventModel } from '../models/OrderPickupEventModel'; +import { MerchandiseItemPhotoModel } from '../models/MerchandiseItemPhotoModel'; @Service() export default class MerchStoreService { + private static readonly MAX_MERCH_PHOTO_COUNT = 5; + private emailService: EmailService; private transactions: TransactionsManager; @@ -188,10 +193,11 @@ export default class MerchStoreService { const merchItemRepository = Repositories.merchStoreItem(txn); const item = await merchItemRepository.findByUuid(uuid); if (!item) throw new NotFoundError(); + if (itemEdit.hidden === false && item.options.length === 0) { throw new UserError('Item cannot be set to visible if it has 0 options.'); } - const { options, collection: updatedCollection, ...changes } = itemEdit; + const { options, merchPhotos, collection: updatedCollection, ...changes } = itemEdit; if (options) { const optionUpdatesByUuid = new Map(options.map((option) => [option.uuid, option])); item.options.map((currentOption) => { @@ -210,6 +216,26 @@ export default class MerchStoreService { }); } + // this part only handles updating the positions of the pictures + if (merchPhotos) { + // error on duplicate photo uuids + const dupSet = new Set(); + merchPhotos.forEach((merchPhoto) => { + if (dupSet.has(merchPhoto.uuid)) { + throw new UserError(`Multiple edits is made to photo: ${merchPhoto.uuid}`); + } + dupSet.add(merchPhoto.uuid); + }); + + const photoUpdatesByUuid = new Map(merchPhotos.map((merchPhoto) => [merchPhoto.uuid, merchPhoto])); + + item.merchPhotos.map((currentPhoto) => { + if (!photoUpdatesByUuid.has(currentPhoto.uuid)) return; + const photoUpdate = photoUpdatesByUuid.get(currentPhoto.uuid); + return MerchandiseItemPhotoModel.merge(currentPhoto, photoUpdate); + }); + } + const updatedItem = MerchandiseItemModel.merge(item, changes); MerchStoreService.verifyItemHasValidOptions(updatedItem); @@ -281,6 +307,76 @@ export default class MerchStoreService { }); } + /** + * Verify that items have valid options. An item with variants disabled cannot have multiple + * options, and an item with variants enabled cannot have multiple option types. + */ + private static verifyItemHasValidPhotos(item: MerchItem | MerchandiseItemModel) { + if (item.merchPhotos.length > MerchStoreService.MAX_MERCH_PHOTO_COUNT) { + throw new UserError('Merch items cannot have more than 5 pictures'); + } + } + + /** + * Creates an item photo and assign it the corresponding picture url + * and append the photo to the photos list from merchItem + * @param item merch item uuid + * @param properties merch item photo picture url and position + * @returns created item photo + */ + public async createItemPhoto(item: Uuid, properties: MerchItemPhoto): Promise { + return this.transactions.readWrite(async (txn) => { + const merchItem = await Repositories.merchStoreItem(txn).findByUuid(item); + if (!merchItem) throw new NotFoundError('Merch item not found'); + + const createdPhoto = MerchandiseItemPhotoModel.create({ ...properties, merchItem }); + const merchStoreItemPhotoRepository = Repositories.merchStoreItemPhoto(txn); + + // verify the result photos array + merchItem.merchPhotos.push(createdPhoto); + MerchStoreService.verifyItemHasValidPhotos(merchItem); + + const upsertedPhoto = await merchStoreItemPhotoRepository.upsertMerchItemPhoto(createdPhoto); + return upsertedPhoto.getPublicMerchItemPhoto(); + }); + } + + /** + * Check if the photo is ready to be deleted. Fail if the merch item is visible + * and it was the only photo of the item. + * + * @param uuid the uuid of photo to be deleted + * @returns the photo object to be removed from database + */ + public async getItemPhotoForDeletion(uuid: Uuid): Promise { + return this.transactions.readWrite(async (txn) => { + const merchStoreItemPhotoRepository = Repositories.merchStoreItemPhoto(txn); + const merchPhoto = await merchStoreItemPhotoRepository.findByUuid(uuid); + if (!merchPhoto) throw new NotFoundError('Merch item photo not found'); + + const merchItem = await Repositories.merchStoreItem(txn).findByUuid(merchPhoto.merchItem.uuid); + if (merchItem.merchPhotos.length === 1 && !merchItem.hidden) { + throw new UserError('Cannot delete the only photo for a visible merch item'); + } + + return merchPhoto; + }); + } + + /** + * Deletes the given item photo. + * + * @param merchPhoto the photo object to be removed + * @returns the photo object removed from database + */ + public async deleteItemPhoto(merchPhoto: MerchandiseItemPhotoModel): Promise { + return this.transactions.readWrite(async (txn) => { + const merchStoreItemPhotoRepository = Repositories.merchStoreItemPhoto(txn); + await merchStoreItemPhotoRepository.deleteMerchItemPhoto(merchPhoto); + return merchPhoto; + }); + } + public async findOrderByUuid(uuid: Uuid): Promise { const order = await this.transactions.readOnly(async (txn) => Repositories .merchOrder(txn) @@ -386,6 +482,7 @@ export default class MerchStoreService { const { item } = option; return { ...item, + picture: item.getDefaultPhotoUrl(), quantityRequested: oi.quantity, salePrice: option.getPrice(), total: oi.quantity * option.getPrice(), @@ -577,6 +674,7 @@ export default class MerchStoreService { && MerchStoreService.isLessThanTwoDaysBeforePickupEvent(order.pickupEvent)) { throw new NotFoundError('Cannot cancel an order with a pickup date less than 2 days away'); } + const customer = order.user; await this.refundAndConfirmOrderCancellation(order, user, txn); const activityRepository = Repositories.activity(txn); @@ -683,6 +781,7 @@ export default class MerchStoreService { const { quantity, price } = optionPricesAndQuantities.get(option); return { ...item, + picture: item.getDefaultPhotoUrl(), quantityRequested: quantity, salePrice: price, total: quantity * price, diff --git a/services/StorageService.ts b/services/StorageService.ts index 911b1c18..647c3413 100644 --- a/services/StorageService.ts +++ b/services/StorageService.ts @@ -68,6 +68,20 @@ export default class StorageService { }; } + public static getRandomString(): string { + const chars = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-'; + const stringLength = 25; + // according to nanoID: ~611 trillion years needed, in order to have a 1% + // probability of at least one collision. + + let result = ''; + for (let i = 0; i < stringLength; i += 1) { + result += chars.charAt(Math.floor(Math.random() * chars.length)); + } + + return result; + } + private static getMediaConfig(type: MediaType): MediaTypeConfig { switch (type) { case MediaType.EVENT_COVER: { diff --git a/tests/Seeds.ts b/tests/Seeds.ts index a15a8cdf..717f19d1 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -377,7 +377,6 @@ async function seed(): Promise { const MERCH_ITEM_1 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_1, itemName: 'Unisex Hack School Anorak', - picture: 'https://i.imgur.com/jkTcUJO.jpg', description: 'San Diego has an average annual precipitation less than 12 inches,' + 'but that doesn\'t mean you don\'t need one of these.', monthlyLimit: 1, @@ -447,10 +446,30 @@ async function seed(): Promise { MERCH_ITEM_1_OPTION_L, MERCH_ITEM_1_OPTION_XL, ]; + // uploadedPhoto is 'https://www.fakepicture.com/' by default, test if this applies + const MERCH_ITEM_1_PHOTO_0 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_1, + position: 0, + }); + const MERCH_ITEM_1_PHOTO_1 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_1, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 1, + }); + const MERCH_ITEM_1_PHOTO_2 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_1, + uploadedPhoto: 'https://i.imgur.com/pSZ921P.png', + position: 2, + }); + MERCH_ITEM_1.merchPhotos = [ + MERCH_ITEM_1_PHOTO_0, + MERCH_ITEM_1_PHOTO_1, + MERCH_ITEM_1_PHOTO_2, + ]; + const MERCH_ITEM_2 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_1, itemName: 'Hack School Sticker Pack (4) - Cyan', - picture: 'https://i.imgur.com/pSZ921P.png', description: 'Make space on your laptop cover for these Cyan stickers. Pack of 4, size in inches.', monthlyLimit: 5, lifetimeLimit: 25, @@ -495,6 +514,20 @@ async function seed(): Promise { MERCH_ITEM_2_OPTION_3X3, MERCH_ITEM_2_OPTION_4X4, ]; + const MERCH_ITEM_2_PHOTO_0 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_2, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 0, + }); + const MERCH_ITEM_2_PHOTO_1 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_2, + uploadedPhoto: 'https://i.imgur.com/pSZ921P.png', + position: 1, + }); + MERCH_ITEM_2.merchPhotos = [ + MERCH_ITEM_2_PHOTO_0, + MERCH_ITEM_2_PHOTO_1, + ]; MERCH_COLLECTION_1.items = [MERCH_ITEM_1, MERCH_ITEM_2]; const MERCH_COLLECTION_2 = MerchFactory.fakeCollection({ @@ -504,7 +537,6 @@ async function seed(): Promise { const MERCH_ITEM_3 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_2, itemName: 'Camp Snoopy Snapback', - picture: 'https://i.imgur.com/QNdhfuO.png', description: 'Guaranteed 2x return on Grailed.', monthlyLimit: 2, lifetimeLimit: 5, @@ -518,10 +550,42 @@ async function seed(): Promise { discountPercentage: 5, }); MERCH_ITEM_3.options = [MERCH_ITEM_3_OPTION]; + const MERCH_ITEM_3_PHOTO_0 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_3, + uploadedPhoto: 'https://i.imgur.com/QNdhfuO.png', + position: 0, + }); + const MERCH_ITEM_3_PHOTO_1 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_3, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 1, + }); + const MERCH_ITEM_3_PHOTO_2 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_3, + uploadedPhoto: 'https://i.imgur.com/pSZ921P.png', + position: 2, + }); + const MERCH_ITEM_3_PHOTO_3 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_3, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 3, + }); + const MERCH_ITEM_3_PHOTO_4 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_3, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 4, + }); + MERCH_ITEM_3.merchPhotos = [ + MERCH_ITEM_3_PHOTO_0, + MERCH_ITEM_3_PHOTO_1, + MERCH_ITEM_3_PHOTO_2, + MERCH_ITEM_3_PHOTO_3, + MERCH_ITEM_3_PHOTO_4, + ]; + const MERCH_ITEM_4 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_2, itemName: 'Salt & Pepper (Canyon) Shakers', - picture: 'https://i.pinimg.com/originals/df/c5/72/dfc5729a0dea666c31c5f4daea851619.jpg', description: 'Salt and pepper not included.', monthlyLimit: 3, lifetimeLimit: 10, @@ -535,10 +599,15 @@ async function seed(): Promise { discountPercentage: 20, }); MERCH_ITEM_4.options = [MERCH_ITEM_4_OPTION]; + const MERCH_ITEM_4_PHOTO_0 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_4, + position: 0, + uploadedPhoto: 'https://i.pinimg.com/originals/df/c5/72/dfc5729a0dea666c31c5f4daea851619.jpg', + }); + MERCH_ITEM_4.merchPhotos = [MERCH_ITEM_4_PHOTO_0]; const MERCH_ITEM_5 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_2, itemName: 'Unisex Raccoon Print Shell Jacket', - picture: 'https://i.etsystatic.com/8812670/r/il/655afa/3440382093/il_340x270.3440382093_cbui.jpg', description: 'Self-explanatory.', monthlyLimit: 1, lifetimeLimit: 2, @@ -568,6 +637,12 @@ async function seed(): Promise { }, }); MERCH_ITEM_5.options = [MERCH_ITEM_5_MEDIUM, MERCH_ITEM_5_LARGE]; + const MERCH_ITEM_5_PHOTO_0 = MerchFactory.fakePhoto({ + merchItem: MERCH_ITEM_5, + position: 0, + uploadedPhoto: 'https://i.etsystatic.com/8812670/r/il/655afa/3440382093/il_340x270.3440382093_cbui.jpg', + }); + MERCH_ITEM_5.merchPhotos = [MERCH_ITEM_5_PHOTO_0]; MERCH_COLLECTION_2.items = [MERCH_ITEM_3, MERCH_ITEM_4, MERCH_ITEM_5]; const PAST_ORDER_PICKUP_EVENT = MerchFactory.fakeOrderPickupEvent({ diff --git a/tests/data/MerchFactory.ts b/tests/data/MerchFactory.ts index 37263bda..fa4b9b35 100644 --- a/tests/data/MerchFactory.ts +++ b/tests/data/MerchFactory.ts @@ -1,12 +1,13 @@ import * as faker from 'faker'; import * as moment from 'moment'; import { v4 as uuid } from 'uuid'; +import FactoryUtils from './FactoryUtils'; import { MerchItemOptionMetadata, OrderPickupEventStatus } from '../../types'; import { OrderPickupEventModel } from '../../models/OrderPickupEventModel'; import { MerchandiseCollectionModel } from '../../models/MerchandiseCollectionModel'; import { MerchandiseItemModel } from '../../models/MerchandiseItemModel'; import { MerchandiseItemOptionModel } from '../../models/MerchandiseItemOptionModel'; -import FactoryUtils from './FactoryUtils'; +import { MerchandiseItemPhotoModel } from '../../models/MerchandiseItemPhotoModel'; export class MerchFactory { public static fakeCollection(substitute?: Partial): MerchandiseCollectionModel { @@ -35,14 +36,12 @@ export class MerchFactory { const fake = MerchandiseItemModel.create({ uuid: uuid(), itemName: faker.datatype.hexaDecimal(10), - picture: faker.image.cats(), description: faker.lorem.sentences(2), hasVariantsEnabled, monthlyLimit: FactoryUtils.getRandomNumber(1, 5), lifetimeLimit: FactoryUtils.getRandomNumber(6, 10), hidden: false, }); - // merging arrays returns a union of fake.options and substitute.options so only create // fake.options if the substitute doesn't provide any if (!substitute?.options) { @@ -51,9 +50,25 @@ export class MerchFactory { .createOptions(numOptions) .map((option) => MerchandiseItemOptionModel.merge(option, { item: fake })); } + if (!substitute?.merchPhotos) { + const numPhotos = FactoryUtils.getRandomNumber(1, 5); + fake.merchPhotos = MerchFactory + .createPhotos(numPhotos) + .map((merchPhoto) => MerchandiseItemPhotoModel.merge(merchPhoto, { merchItem: fake })); + } return MerchandiseItemModel.merge(fake, substitute); } + public static fakePhoto(substitute?: Partial): MerchandiseItemPhotoModel { + const fake = MerchandiseItemPhotoModel.create({ + uuid: uuid(), + position: 0, + uploadedPhoto: 'https://www.fakepicture.com/', + uploadedAt: faker.date.recent(), + }); + return MerchandiseItemPhotoModel.merge(fake, substitute); + } + public static fakeOption(substitute?: Partial): MerchandiseItemOptionModel { const fake = MerchandiseItemOptionModel.create({ uuid: uuid(), @@ -132,6 +147,12 @@ export class MerchFactory { return FactoryUtils.create(n, () => MerchFactory.fakeOptionWithType(type)); } + private static createPhotos(n: number): MerchandiseItemPhotoModel[] { + return FactoryUtils + .create(n, () => MerchFactory.fakePhoto()) + .map((merchPhoto, i) => MerchandiseItemPhotoModel.merge(merchPhoto, { position: i })); + } + private static randomPrice(): number { // some multiple of 50, min 250 and max 50_000 return FactoryUtils.getRandomNumber(250, 50_000, 50); diff --git a/tests/merchStore.test.ts b/tests/merchStore.test.ts index 00e23646..c04a8e7c 100644 --- a/tests/merchStore.test.ts +++ b/tests/merchStore.test.ts @@ -1,13 +1,16 @@ import * as faker from 'faker'; -import { ForbiddenError } from 'routing-controllers'; +import { ForbiddenError, NotFoundError } from 'routing-controllers'; import { zip } from 'underscore'; -import { anything, instance, mock, when } from 'ts-mockito'; +import { anything, instance, verify, mock, when } from 'ts-mockito'; import { OrderModel } from '../models/OrderModel'; import { MerchandiseItemOptionModel } from '../models/MerchandiseItemOptionModel'; -import { MerchItemEdit, UserAccessType } from '../types'; +import { MediaType, MerchItemEdit, UserAccessType } from '../types'; import { ControllerFactory } from './controllers'; import { DatabaseConnection, MerchFactory, PortalState, UserFactory } from './data'; import EmailService from '../services/EmailService'; +import { FileFactory } from './data/FileFactory'; +import { Config } from '../config'; +import Mocks from './mocks/MockFactory'; beforeAll(async () => { await DatabaseConnection.connect(); @@ -753,6 +756,171 @@ describe('merch item options', () => { }); }); +describe('merch item photos', () => { + const folderLocation = 'https://s3.amazonaws.com/upload-photo/'; + + test('can create an item with up to 5 pictures', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const photo1 = MerchFactory.fakePhoto(); + const item = MerchFactory.fakeItem({ merchPhotos: [photo1] }); + + await new PortalState() + .createUsers(admin) + .createMerchItem(item) + .write(); + + const image2 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const image3 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const image4 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const image5 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const imageExtra = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const storageService = Mocks.storage(folderLocation); + + const merchStoreController = ControllerFactory.merchStore( + conn, + undefined, + instance(storageService), + ); + + const params = { uuid: item.uuid }; + + const response2 = await merchStoreController.createMerchItemPhoto(image2, params, { position: '1' }, admin); + const response3 = await merchStoreController.createMerchItemPhoto(image3, params, { position: '2' }, admin); + const response4 = await merchStoreController.createMerchItemPhoto(image4, params, { position: '3' }, admin); + const response5 = await merchStoreController.createMerchItemPhoto(image5, params, { position: '4' }, admin); + + // checking no error is thrown and storage is correctly modified + // enough to check first and last response + expect(response2.error).toBe(null); + expect(response5.error).toBe(null); + verify( + storageService.uploadToFolder( + image2, + MediaType.MERCH_PHOTO, + anything(), + anything(), + ), + ).called(); + verify( + storageService.uploadToFolder( + image5, + MediaType.MERCH_PHOTO, + anything(), + anything(), + ), + ).called(); + + const photo2 = response2.merchPhoto; + const photo3 = response3.merchPhoto; + const photo4 = response4.merchPhoto; + const photo5 = response5.merchPhoto; + + // 0 index + expect(photo2.position).toBe(1); + expect(photo3.position).toBe(2); + expect(photo4.position).toBe(3); + expect(photo5.position).toBe(4); + + const photos = [photo1, photo2, photo3, photo4, photo5]; + expect((await merchStoreController.getOneMerchItem(params, admin)).item.merchPhotos) + .toEqual(photos); + + expect(merchStoreController.createMerchItemPhoto(imageExtra, params, { position: '5' }, admin)) + .rejects.toThrow('Merch items cannot have more than 5 pictures'); + }); + + test('can remap the picture of an item to different orders', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const photo1 = MerchFactory.fakePhoto({ position: 0 }); + const photo2 = MerchFactory.fakePhoto({ position: 1 }); + const photo3 = MerchFactory.fakePhoto({ position: 2 }); + const photo4 = MerchFactory.fakePhoto({ position: 3 }); + const photo5 = MerchFactory.fakePhoto({ position: 4 }); + const merchPhotos = [photo1, photo2, photo3, photo4, photo5]; + const item = MerchFactory.fakeItem({ merchPhotos }); + + await new PortalState() + .createUsers(admin) + .createMerchItem(item) + .write(); + + const merchStoreController = ControllerFactory.merchStore(conn); + const params = { uuid: item.uuid }; + + // check before remap whether photos are correctly positioned + expect((await merchStoreController.getOneMerchItem(params, admin)).item.merchPhotos).toEqual(merchPhotos); + + // reversing the order of the photos + const editMerchItemRequest = { merchandise: { + merchPhotos: [ + { uuid: photo5.uuid, position: 0 }, + { uuid: photo4.uuid, position: 1 }, + { uuid: photo3.uuid, position: 2 }, + { uuid: photo2.uuid, position: 3 }, + { uuid: photo1.uuid, position: 4 }, + ], + } }; + + await merchStoreController.editMerchItem(params, editMerchItemRequest, admin); + + const newPhotos = (await merchStoreController.getOneMerchItem(params, admin)).item.merchPhotos; + const newPhotosUuids = newPhotos.map((photo) => photo.uuid); + const expectedPhotosUuids = [photo5.uuid, photo4.uuid, photo3.uuid, photo2.uuid, photo1.uuid]; + expect(newPhotosUuids).toStrictEqual(expectedPhotosUuids); + }); + + test('can delete photo until 1 photo left except merch item is deleted', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const photo1 = MerchFactory.fakePhoto({ position: 0 }); + const photo2 = MerchFactory.fakePhoto({ position: 1 }); + const merchPhotos = [photo1, photo2]; + const item = MerchFactory.fakeItem({ merchPhotos }); + + await new PortalState() + .createUsers(admin) + .createMerchItem(item) + .write(); + + const storageService = Mocks.storage(); + const merchStoreController = ControllerFactory.merchStore( + conn, + undefined, + instance(storageService), + ); + const params = { uuid: item.uuid }; + + // verify before deleting, the photos all exist + const itemInDatabase = (await merchStoreController.getOneMerchItem(params, admin)).item; + expect(itemInDatabase.merchPhotos).toEqual(merchPhotos); + + const deleteMerchItemPhotoParam1 = { uuid: photo1.uuid }; + const deleteMerchItemPhotoParam2 = { uuid: photo2.uuid }; + + // verify deletion delete correctly + await merchStoreController.deleteMerchItemPhoto(deleteMerchItemPhotoParam1, admin); + const expectedUrl = itemInDatabase.merchPhotos[0].uploadedPhoto; + verify(storageService.deleteAtUrl(expectedUrl)).called(); + + const newPhotos = (await merchStoreController.getOneMerchItem(params, admin)).item.merchPhotos; + + expect(newPhotos).toHaveLength(1); + expect(newPhotos[0].uuid).toEqual(photo2.uuid); + expect(newPhotos[0].position).toEqual(1); + + // verify visible item photo limitation + expect(merchStoreController.deleteMerchItemPhoto(deleteMerchItemPhotoParam2, admin)) + .rejects.toThrow('Cannot delete the only photo for a visible merch item'); + + // check cascade + await merchStoreController.deleteMerchItem(params, admin); + expect(merchStoreController.deleteMerchItemPhoto(deleteMerchItemPhotoParam2, admin)) + .rejects.toThrow(NotFoundError); + }); +}); + describe('checkout cart', () => { test('passing in valid item option uuids returns the full options and their items', async () => { const conn = await DatabaseConnection.get(); @@ -782,7 +950,6 @@ describe('checkout cart', () => { const params = { items: options.map((o) => o.uuid) }; const merchStoreController = ControllerFactory.merchStore(conn); const getCartResponse = await merchStoreController.getCartItems(params, member); - const { cart } = getCartResponse; expect(cart).toHaveLength(3); diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index ff271f3b..f8ef3c3e 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -205,6 +205,10 @@ export interface CreateMerchItemOptionRequest { option: MerchItemOption; } +export interface CreateMerchItemPhotoRequest { + position: string; +} + export interface PlaceMerchOrderRequest { order: MerchItemOptionAndQuantity[]; pickupEvent: Uuid; @@ -237,7 +241,6 @@ export interface CommonMerchItemProperties { itemName: string; collection: string; description: string; - picture?: string; hidden?: boolean; monthlyLimit?: number; lifetimeLimit?: number; @@ -250,6 +253,16 @@ export interface MerchItemOptionMetadata { position: number; } +export interface MerchItemPhoto { + uploadedPhoto: string; + position: number; +} + +export interface MerchItemPhotoEdit { + uuid: string; + position?: number; +} + export interface MerchItemOption { quantity: number; price: number; @@ -259,6 +272,7 @@ export interface MerchItemOption { export interface MerchItem extends CommonMerchItemProperties { options: MerchItemOption[]; + merchPhotos: MerchItemPhoto[]; } export interface MerchItemOptionEdit { @@ -271,6 +285,7 @@ export interface MerchItemOptionEdit { export interface MerchItemEdit extends Partial { options?: MerchItemOptionEdit[]; + merchPhotos?: MerchItemPhotoEdit[]; } export interface MerchOrderEdit { diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index 97d25732..9133d18a 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -164,12 +164,12 @@ export interface PublicMerchItem { uuid: Uuid; itemName: string; collection?: PublicMerchCollection; - picture: string; description: string; monthlyLimit: number; lifetimeLimit: number; hidden: boolean; hasVariantsEnabled: boolean; + merchPhotos: PublicMerchItemPhoto[]; options: PublicMerchItemOption[]; } @@ -181,7 +181,7 @@ export interface PublicMerchItemWithPurchaseLimits extends PublicMerchItem { export interface PublicCartMerchItem { uuid: Uuid; itemName: string; - picture: string; + uploadedPhoto: string; description: string; } @@ -193,6 +193,13 @@ export interface PublicMerchItemOption { metadata: MerchItemOptionMetadata; } +export interface PublicMerchItemPhoto { + uuid: Uuid; + uploadedPhoto: string; + position: number; + uploadedAt: Date; +} + export interface PublicOrderMerchItemOption { uuid: Uuid; price: number; @@ -256,10 +263,12 @@ export interface EditMerchItemResponse extends ApiResponse { export interface DeleteMerchItemResponse extends ApiResponse {} -export interface UpdateMerchPhotoResponse extends ApiResponse { - item: PublicMerchItem; +export interface CreateMerchPhotoResponse extends ApiResponse { + merchPhoto: PublicMerchItemPhoto; } +export interface DeleteMerchItemPhotoResponse extends ApiResponse {} + export interface CreateMerchItemOptionResponse extends ApiResponse { option: PublicMerchItemOption; } From cdb5e435972fe4ca0e9004f072aecc0e740afa3e Mon Sep 17 00:00:00 2001 From: Faris Ashai Date: Tue, 2 Jan 2024 12:14:46 -0800 Subject: [PATCH 11/30] Update UserModel.ts (#383) * Update UserModel.ts * lint * fixed get profile by handle route for social media --------- Co-authored-by: Nikhil Dange --- api/controllers/UserController.ts | 4 ++-- tests/user.handles.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/controllers/UserController.ts b/api/controllers/UserController.ts index b3afbd46..e80f4347 100644 --- a/api/controllers/UserController.ts +++ b/api/controllers/UserController.ts @@ -85,9 +85,9 @@ export class UserController { if (params.handle === currentUser.handle) { return this.getCurrentUser(currentUser); } - const user = await this.userAccountService.findByHandle(params.handle); - return { error: null, user: user.getPublicProfile() }; + const userProfile = await this.userAccountService.getPublicProfile(user); + return { error: null, user: userProfile }; } @Get() diff --git a/tests/user.handles.test.ts b/tests/user.handles.test.ts index f8929e9b..0d2cabaa 100644 --- a/tests/user.handles.test.ts +++ b/tests/user.handles.test.ts @@ -84,7 +84,7 @@ describe('Get User Handles', () => { loggedInUser, ); - expect(response.user).toEqual(otherUser.getPublicProfile()); + expect(response.user.uuid).toEqual(otherUser.getPublicProfile().uuid); }); }); From 4b95f0382e1062d0c7d1ee0862dff584d65ee1cb Mon Sep 17 00:00:00 2001 From: yimmyj <69327109+yimmyj@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:46:41 -0800 Subject: [PATCH 12/30] Create Sponsorship Manager Role with Resume Permissions (#374) * modified isUnusedAttendanceCode check if an event has already happened, and don't count it if so * a * modified isUnusedAttendanceCode check if an event has already happened, and don't count it if so * created new sponsorshib_member role for user model * reverted eventrepository * renamed sponsorship member to sponsorship manager * updated coments on resumetest * added seeding user with sponsorship manager role --------- Co-authored-by: Nikhil Dange --- models/UserModel.ts | 4 +++ services/PermissionsService.ts | 2 +- tests/Seeds.ts | 5 ++++ tests/resume.test.ts | 50 +++++++++++++++++++++++++++++++++- types/Enums.ts | 1 + 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/models/UserModel.ts b/models/UserModel.ts index a2cd0114..6f4e09cb 100644 --- a/models/UserModel.ts +++ b/models/UserModel.ts @@ -118,6 +118,10 @@ export class UserModel extends BaseEntity { return this.accessType === UserAccessType.MERCH_STORE_DISTRIBUTOR; } + public isSponsorshipManager(): boolean { + return this.accessType === UserAccessType.SPONSORSHIP_MANAGER; + } + public getPublicProfile(): PublicProfile { const publicProfile: PublicProfile = { uuid: this.uuid, diff --git a/services/PermissionsService.ts b/services/PermissionsService.ts index ed1b9421..c8e0e19f 100644 --- a/services/PermissionsService.ts +++ b/services/PermissionsService.ts @@ -79,7 +79,7 @@ export default class PermissionsService { } public static canSeeAllVisibleResumes(user: UserModel) { - return user.isAdmin(); + return user.isAdmin() || user.isSponsorshipManager(); } public static canModifyUserAccessLevel(user: UserModel): boolean { diff --git a/tests/Seeds.ts b/tests/Seeds.ts index 717f19d1..31c57a72 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -116,6 +116,10 @@ async function seed(): Promise { email: 'acm_store_distributor@ucsd.edu', accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR, }); + const USER_SPONSORSHIP_MANAGER = UserFactory.fake({ + email: 'acm_sponsorship_manager@ucsd.edu', + accessType: UserAccessType.SPONSORSHIP_MANAGER, + }); // Used for testing various User Social Media const USER_SOCIAL_MEDIA_1 = UserFactory.fake(); @@ -683,6 +687,7 @@ async function seed(): Promise { USER_MARKETING, USER_MERCH_STORE_MANAGER, USER_MERCH_STORE_DISTRIBUTOR, + USER_SPONSORSHIP_MANAGER, USER_SOCIAL_MEDIA_1, USER_SOCIAL_MEDIA_2, USER_SOCIAL_MEDIA_3, diff --git a/tests/resume.test.ts b/tests/resume.test.ts index 6e5f6995..34c2021f 100644 --- a/tests/resume.test.ts +++ b/tests/resume.test.ts @@ -24,7 +24,7 @@ afterAll(async () => { }); describe('resume fetching', () => { - test('only admins can get all visible resumes', async () => { + test('admins can get all visible resumes', async () => { const conn = await DatabaseConnection.get(); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); const member = UserFactory.fake(); @@ -55,6 +55,37 @@ describe('resume fetching', () => { }); }); + test('sponsorship managers can get all visible resumes', async () => { + const conn = await DatabaseConnection.get(); + const sponsorshipManager = UserFactory.fake({ accessType: UserAccessType.SPONSORSHIP_MANAGER }); + const member = UserFactory.fake(); + const resume = ResumeFactory.fake({ user: member, isResumeVisible: true }); + + await new PortalState() + .createUsers(sponsorshipManager, member) + .createResumes(member, resume) + .write(); + + const resumeController = ControllerFactory.resume(conn); + + // throws permissions error for member + await expect(resumeController.getAllVisibleResumes(member)).rejects.toThrow( + ForbiddenError, + ); + + // get response resumes + const response = await resumeController.getAllVisibleResumes(sponsorshipManager); + expect(response.error).toBeNull(); + expect(response.resumes).toHaveLength(1); + expect(response.resumes[0]).toStrictEqual({ + uuid: resume.uuid, + user: member.getPublicProfile(), + isResumeVisible: resume.isResumeVisible, + url: resume.url, + lastUpdated: resume.lastUpdated, + }); + }); + test('admins cannot get hidden resumes', async () => { const conn = await DatabaseConnection.get(); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); @@ -71,6 +102,23 @@ describe('resume fetching', () => { expect(response.error).toBeNull(); expect(response.resumes).toHaveLength(0); }); + + test('sponsorship managers cannot get hidden resumes', async () => { + const conn = await DatabaseConnection.get(); + const sponsorshipManager = UserFactory.fake({ accessType: UserAccessType.SPONSORSHIP_MANAGER }); + const member = UserFactory.fake(); + const resume = ResumeFactory.fake({ user: member, isResumeVisible: false }); + + await new PortalState() + .createUsers(sponsorshipManager, member) + .createResumes(member, resume) + .write(); + + const resumeController = ControllerFactory.resume(conn); + const response = await resumeController.getAllVisibleResumes(sponsorshipManager); + expect(response.error).toBeNull(); + expect(response.resumes).toHaveLength(0); + }); }); describe('upload resume', () => { diff --git a/types/Enums.ts b/types/Enums.ts index 5185b06b..67f3bd6c 100644 --- a/types/Enums.ts +++ b/types/Enums.ts @@ -6,6 +6,7 @@ export enum UserAccessType { MARKETING = 'MARKETING', MERCH_STORE_MANAGER = 'MERCH_STORE_MANAGER', MERCH_STORE_DISTRIBUTOR = 'MERCH_STORE_DISTRIBUTOR', + SPONSORSHIP_MANAGER = 'SPONSORSHIP_MANAGER', } export enum UserState { From 65cd0223eb25196c10a7791310ebf71830857f46 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Sat, 20 Jan 2024 13:28:54 -0800 Subject: [PATCH 13/30] Add linkedEvent column to OrderPickupEvent Table (#385) * added migration and relation * maybe a better relation idk * added types throughout request/response * nvm i think this is correct * a lot of random changes * added OrderPickupEventModel relation * linting * unit test * linting + unit test * added faker internet urls to eventfactory * bumped package version --- api/validators/MerchStoreRequests.ts | 9 ++++ ...dEvent-column-to-orderPickupEvent-table.ts | 18 ++++++++ models/OrderPickupEventModel.ts | 8 +++- package.json | 2 +- repositories/MerchOrderRepository.ts | 10 ++-- services/MerchStoreService.ts | 44 ++++++++++++++---- tests/data/EventFactory.ts | 5 ++ tests/data/MerchFactory.ts | 1 + tests/merchOrder.test.ts | 46 ++++++++++++++++++- types/ApiRequests.ts | 1 + types/ApiResponses.ts | 1 + 11 files changed, 127 insertions(+), 18 deletions(-) create mode 100644 migrations/0039-add-linkedEvent-column-to-orderPickupEvent-table.ts diff --git a/api/validators/MerchStoreRequests.ts b/api/validators/MerchStoreRequests.ts index 3d8f0ac4..379b740b 100644 --- a/api/validators/MerchStoreRequests.ts +++ b/api/validators/MerchStoreRequests.ts @@ -11,6 +11,7 @@ import { ArrayNotEmpty, IsNumber, IsNumberString, + IsOptional, } from 'class-validator'; import { Type } from 'class-transformer'; import { @@ -41,6 +42,7 @@ import { MerchOrderEdit as IMerchOrderEdit, OrderPickupEvent as IOrderPickupEvent, OrderPickupEventEdit as IOrderPickupEventEdit, + Uuid, } from '../../types'; export class MerchCollection implements IMerchCollection { @@ -258,6 +260,10 @@ export class OrderPickupEvent implements IOrderPickupEvent { @IsDefined() @Min(1) orderLimit: number; + + @IsOptional() + @IsUUID() + linkedEventUuid?: Uuid; } export class OrderPickupEventEdit implements IOrderPickupEventEdit { @@ -275,6 +281,9 @@ export class OrderPickupEventEdit implements IOrderPickupEventEdit { @Min(1) orderLimit?: number; + + @IsUUID() + linkedEventUuid?: Uuid; } export class MerchOrderEdit implements IMerchOrderEdit { diff --git a/migrations/0039-add-linkedEvent-column-to-orderPickupEvent-table.ts b/migrations/0039-add-linkedEvent-column-to-orderPickupEvent-table.ts new file mode 100644 index 00000000..54c16fea --- /dev/null +++ b/migrations/0039-add-linkedEvent-column-to-orderPickupEvent-table.ts @@ -0,0 +1,18 @@ +import { MigrationInterface, QueryRunner, TableColumn } from 'typeorm'; + +const TABLE_NAME = 'OrderPickupEvents'; +const COLUMN_NAME = 'linkedEvent'; + +export class AddLinkedEventColumnToOrderPickupEventTable1704352457840 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.addColumn(TABLE_NAME, new TableColumn({ + name: COLUMN_NAME, + type: 'uuid', + isNullable: true, + })); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropColumn(TABLE_NAME, COLUMN_NAME); + } +} diff --git a/models/OrderPickupEventModel.ts b/models/OrderPickupEventModel.ts index c829ea5c..928fe1da 100644 --- a/models/OrderPickupEventModel.ts +++ b/models/OrderPickupEventModel.ts @@ -1,6 +1,7 @@ -import { Entity, BaseEntity, Column, PrimaryGeneratedColumn, JoinColumn, OneToMany } from 'typeorm'; +import { Entity, BaseEntity, Column, PrimaryGeneratedColumn, JoinColumn, OneToMany, OneToOne } from 'typeorm'; import { Uuid, PublicOrderPickupEvent, OrderPickupEventStatus } from '../types'; import { OrderModel } from './OrderModel'; +import { EventModel } from './EventModel'; @Entity('OrderPickupEvents') export class OrderPickupEventModel extends BaseEntity { @@ -29,6 +30,10 @@ export class OrderPickupEventModel extends BaseEntity { @JoinColumn({ name: 'order' }) orders: OrderModel[]; + @OneToOne((type) => EventModel, { nullable: true }) + @JoinColumn({ name: 'linkedEvent' }) + linkedEvent: EventModel; + public getPublicOrderPickupEvent(canSeeOrders = false): PublicOrderPickupEvent { const pickupEvent: PublicOrderPickupEvent = { uuid: this.uuid, @@ -38,6 +43,7 @@ export class OrderPickupEventModel extends BaseEntity { description: this.description, orderLimit: this.orderLimit, status: this.status, + linkedEvent: this.linkedEvent ? this.linkedEvent.getPublicEvent() : null, }; if (canSeeOrders) pickupEvent.orders = this.orders.map((order) => order.getPublicOrderWithItems()); diff --git a/package.json b/package.json index 5c6ef264..39b41095 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.0.0", + "version": "3.1.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/repositories/MerchOrderRepository.ts b/repositories/MerchOrderRepository.ts index 0695a149..139677da 100644 --- a/repositories/MerchOrderRepository.ts +++ b/repositories/MerchOrderRepository.ts @@ -130,7 +130,7 @@ export class OrderPickupEventRepository extends BaseRepository { return this.getBaseFindManyQuery() - .where('"end" < :now') + .where('"orderPickupEvent"."end" < :now') .setParameter('now', new Date()) .getMany(); } @@ -140,7 +140,7 @@ export class OrderPickupEventRepository extends BaseRepository { return this.getBaseFindManyQuery() - .where('"end" >= :now') + .where('"orderPickupEvent"."end" >= :now') .setParameter('now', new Date()) .getMany(); } @@ -157,7 +157,7 @@ export class OrderPickupEventRepository extends BaseRepository): Promise { @@ -177,6 +177,7 @@ export class OrderPickupEventRepository extends BaseRepository { - if (pickupEvent.start >= pickupEvent.end) { - throw new UserError('Order pickup event start time must come before the end time'); - } - const pickupEventModel = OrderPickupEventModel.create(pickupEvent); - if (MerchStoreService.isLessThanTwoDaysBeforePickupEvent(pickupEventModel)) { - throw new UserError('Cannot create a pickup event that starts in less than 2 days'); - } - return this.transactions.readWrite(async (txn) => Repositories - .merchOrderPickupEvent(txn) - .upsertPickupEvent(pickupEventModel)); + return this.transactions.readWrite(async (txn) => { + const orderPickupEventRepository = Repositories.merchOrderPickupEvent(txn); + if (pickupEvent.start >= pickupEvent.end) { + throw new UserError('Order pickup event start time must come before the end time'); + } + + const pickupEventModel = OrderPickupEventModel.create(pickupEvent); + + if (pickupEvent.linkedEventUuid) { + const linkedRegularEvent = await this.getLinkedRegularEvent(pickupEvent.linkedEventUuid); + pickupEventModel.linkedEvent = linkedRegularEvent; + } + + if (MerchStoreService.isLessThanTwoDaysBeforePickupEvent(pickupEventModel)) { + throw new UserError('Cannot create a pickup event that starts in less than 2 days'); + } + + return orderPickupEventRepository.upsertPickupEvent(pickupEventModel); + }); } public async editPickupEvent(uuid: Uuid, changes: OrderPickupEventEdit): Promise { @@ -1050,6 +1060,12 @@ export default class MerchStoreService { const orderPickupEventRepository = Repositories.merchOrderPickupEvent(txn); const pickupEvent = await orderPickupEventRepository.findByUuid(uuid); const updatedPickupEvent = OrderPickupEventModel.merge(pickupEvent, changes); + + if (changes.linkedEventUuid) { + const linkedRegularEvent = await this.getLinkedRegularEvent(changes.linkedEventUuid); + updatedPickupEvent.linkedEvent = linkedRegularEvent; + } + if (updatedPickupEvent.start >= updatedPickupEvent.end) { throw new UserError('Order pickup event start time must come before the end time'); } @@ -1140,6 +1156,14 @@ export default class MerchStoreService { return pickupEvent.status === OrderPickupEventStatus.ACTIVE; } + private async getLinkedRegularEvent(uuid: Uuid): Promise { + return this.transactions.readOnly(async (txn) => { + const linkedEvent = await Repositories.event(txn).findByUuid(uuid); + if (!linkedEvent) throw new NotFoundError('Linked event not found!'); + return linkedEvent; + }); + } + private isUnfulfilledOrder(order: OrderModel): boolean { return order.status !== OrderStatus.FULFILLED && order.status !== OrderStatus.PARTIALLY_FULFILLED diff --git a/tests/data/EventFactory.ts b/tests/data/EventFactory.ts index 0142b17b..c6dccd2e 100644 --- a/tests/data/EventFactory.ts +++ b/tests/data/EventFactory.ts @@ -36,6 +36,11 @@ export class EventFactory { pointValue: EventFactory.randomPointValue(), requiresStaff: FactoryUtils.getRandomBoolean(), staffPointBonus: EventFactory.randomPointValue(), + committee: 'ACM', + cover: faker.internet.url(), + deleted: false, + eventLink: faker.internet.url(), + thumbnail: faker.internet.url(), }); return EventModel.merge(fake, substitute); } diff --git a/tests/data/MerchFactory.ts b/tests/data/MerchFactory.ts index fa4b9b35..a97ace12 100644 --- a/tests/data/MerchFactory.ts +++ b/tests/data/MerchFactory.ts @@ -110,6 +110,7 @@ export class MerchFactory { orderLimit: FactoryUtils.getRandomNumber(1, 5), status: OrderPickupEventStatus.ACTIVE, orders: [], + linkedEvent: null, }); return OrderPickupEventModel.merge(fake, substitute); } diff --git a/tests/merchOrder.test.ts b/tests/merchOrder.test.ts index 2d8e7da0..f4fe09e7 100644 --- a/tests/merchOrder.test.ts +++ b/tests/merchOrder.test.ts @@ -7,7 +7,7 @@ import { OrderModel } from '../models/OrderModel'; import { OrderPickupEventModel } from '../models/OrderPickupEventModel'; import { UserAccessType, OrderStatus, ActivityType, OrderPickupEventStatus } from '../types'; import { ControllerFactory } from './controllers'; -import { DatabaseConnection, MerchFactory, PortalState, UserFactory } from './data'; +import { DatabaseConnection, EventFactory, MerchFactory, PortalState, UserFactory } from './data'; import { MerchStoreControllerWrapper } from './controllers/MerchStoreControllerWrapper'; import { UserModel } from '../models/UserModel'; @@ -962,10 +962,52 @@ describe('merch order pickup events', () => { const merchController = ControllerFactory.merchStore(conn, instance(emailService)); await merchController.createPickupEvent({ pickupEvent }, merchDistributor); - const persistedPickupEvent = await conn.manager.findOne(OrderPickupEventModel, { relations: ['orders'] }); + const persistedPickupEvent = await conn.manager.findOne(OrderPickupEventModel, + { relations: ['orders', 'linkedEvent'] }); expect(persistedPickupEvent).toStrictEqual(pickupEvent); }); + test('pickup events can be linked to normal events & edited', async () => { + const conn = await DatabaseConnection.get(); + const merchDistributor = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR }); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const linkedEvent = EventFactory.fake(); + const eventController = ControllerFactory.event(conn); + await eventController.createEvent({ event: linkedEvent }, admin); + + const pickupEvent = MerchFactory.fakeFutureOrderPickupEvent({ linkedEvent }); + + await new PortalState() + .createUsers(merchDistributor) + .write(); + + const emailService = mock(EmailService); + when(emailService.sendOrderConfirmation(anything(), anything(), anything())) + .thenResolve(); + + const createPickupEventRequest = { pickupEvent: { ...pickupEvent, linkedEventUuid: linkedEvent.uuid } }; + const merchController = ControllerFactory.merchStore(conn, instance(emailService)); + await merchController.createPickupEvent(createPickupEventRequest, merchDistributor); + + const persistedPickupEvent = await conn.manager.findOne(OrderPickupEventModel, + { relations: ['orders', 'linkedEvent'] }); + expect(persistedPickupEvent).toStrictEqual(pickupEvent); + + // edit a linked event + + const newLinkedEvent = EventFactory.fake(); + await eventController.createEvent({ event: newLinkedEvent }, admin); + + const editPickupEventRequest = { pickupEvent: { linkedEventUuid: newLinkedEvent.uuid } }; + const params = { uuid: pickupEvent.uuid }; + await merchController.editPickupEvent(params, editPickupEventRequest, merchDistributor); + + const editedPersistedPickupEvent = await conn.manager.findOne(OrderPickupEventModel, + { relations: ['orders', 'linkedEvent'] }); + expect(editedPersistedPickupEvent.uuid).toEqual(pickupEvent.uuid); + expect(editedPersistedPickupEvent.linkedEvent.uuid).toEqual(editPickupEventRequest.pickupEvent.linkedEventUuid); + }); + test('pickup event creation fails if start date is later than end date', async () => { const conn = await DatabaseConnection.get(); const merchDistributor = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR }); diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index f8ef3c3e..d4807c6b 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -308,6 +308,7 @@ export interface OrderPickupEvent { end: Date; description: string; orderLimit: number; + linkedEventUuid?: Uuid; } export interface OrderPickupEventEdit extends Partial {} diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index 9133d18a..cb93f046 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -410,6 +410,7 @@ export interface PublicOrderPickupEvent { orders?: PublicOrderWithItems[]; orderLimit?: number; status: OrderPickupEventStatus; + linkedEvent?: PublicEvent; } export interface GetOrderPickupEventsResponse extends ApiResponse { From 98c266c33ecd505452a4b17a7bbd89f025019edb Mon Sep 17 00:00:00 2001 From: "Caogang (Marcelo) Shen" Date: Sat, 20 Jan 2024 14:06:34 -0800 Subject: [PATCH 14/30] Quick Example Env Variable Name Fix (#388) This should allow outside contributors to use the env files correctly --- .env.example | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.env.example b/.env.example index 19e79731..b05f429d 100644 --- a/.env.example +++ b/.env.example @@ -19,13 +19,13 @@ CLIENT=localhost:8000 MAX_EVENT_COVER_FILE_SIZE= MAX_PROFILE_PICTURE_FILE_SIZE= MAX_BANNER_FILE_SIZE= -MAX_MERCH_ITEM_PICTURE_FILE_SIZE= +MAX_MERCH_PHOTO_FILE_SIZE= MAX_RESUME_FILE_SIZE= PROFILE_PICTURE_UPLOAD_PATH= EVENT_COVER_UPLOAD_PATH= BANNER_UPLOAD_PATH= -MERCH_ITEM_PICTURE_UPLOAD_PATH= +MERCH_PHOTO_UPLOAD_PATH= RESUME_UPLOAD_PATH= BASE_UPLOAD_PATH= From 93d90adc34f9332683f7971b64d3a4f5d9daa130 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Wed, 7 Feb 2024 22:21:58 -0800 Subject: [PATCH 15/30] Enforce Stricter Validation on Access Type Edits (#392) * conditionals to disallow self demotion/admin edits * unit tests and linting * added ForbiddenError --- services/UserAccountService.ts | 20 ++++++--- tests/admin.test.ts | 78 ++++++++++++++++++++++++++++------ 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/services/UserAccountService.ts b/services/UserAccountService.ts index 3a17d175..e60bf78b 100644 --- a/services/UserAccountService.ts +++ b/services/UserAccountService.ts @@ -1,4 +1,4 @@ -import { BadRequestError, NotFoundError } from 'routing-controllers'; +import { BadRequestError, ForbiddenError, NotFoundError } from 'routing-controllers'; import { Service } from 'typedi'; import { InjectManager } from 'typeorm-typedi-extensions'; import { EntityManager } from 'typeorm'; @@ -225,13 +225,23 @@ export default class UserAccountService { return map; }, {}); - const updatedUsers = await Promise.all(accessUpdates.map(async (accessUpdate, index) => { + const updatedUsers = await Promise.all(accessUpdates.map(async (accessUpdate) => { const { user: userEmail, accessType } = accessUpdate; - const currUser = emailToUserMap[userEmail]; - const oldAccess = currUser.accessType; + // Prevent a user from demoting themselves + if (currentUser.email === userEmail) { + throw new ForbiddenError('Cannot alter own access level'); + } - const updatedUser = await userRepository.upsertUser(currUser, { accessType }); + const userToUpdate = emailToUserMap[userEmail]; + const oldAccess = userToUpdate.accessType; + + // Prevent users from promoting to admin or demoting from admin + if (oldAccess === 'ADMIN' || accessType === 'ADMIN') { + throw new ForbiddenError('Cannot alter access level of admin users'); + } + + const updatedUser = await userRepository.upsertUser(userToUpdate, { accessType }); const activity = { user: currentUser, diff --git a/tests/admin.test.ts b/tests/admin.test.ts index 021f8c6d..03faaddc 100644 --- a/tests/admin.test.ts +++ b/tests/admin.test.ts @@ -302,33 +302,50 @@ describe('updating user access level', () => { expect(updatedUsers.accessType).toEqual(UserAccessType.STAFF); }); - test('admin ability to demote another admin', async () => { + test('ensure that admins cannot demote/promote other admins', async () => { const conn = await DatabaseConnection.get(); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); const secondAdmin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const regularUser = UserFactory.fake({ accessType: UserAccessType.STANDARD }); await new PortalState() - .createUsers(admin, secondAdmin) + .createUsers(admin, secondAdmin, regularUser) .write(); const adminController = ControllerFactory.admin(conn); - const accessLevelResponse = await adminController.updateUserAccessLevel({ - accessUpdates: [ - { user: secondAdmin.email, accessType: UserAccessType.MERCH_STORE_MANAGER }, - ], - }, admin); + // attempt to demote an admin to merch store manager + await expect(async () => { + await adminController.updateUserAccessLevel({ + accessUpdates: [ + { user: secondAdmin.email, accessType: UserAccessType.MERCH_STORE_MANAGER }, + ], + }, admin); + }).rejects.toThrow(ForbiddenError); const repository = conn.getRepository(UserModel); - const updatedUser = await repository.findOne({ email: secondAdmin.email }); + const secondAdminFromDatabase = await repository.findOne({ email: secondAdmin.email }); - expect(updatedUser.email).toEqual(secondAdmin.email); - expect(updatedUser.accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER); - expect(accessLevelResponse.updatedUsers[0].accessType).toEqual(UserAccessType.MERCH_STORE_MANAGER); + expect(secondAdminFromDatabase.email).toEqual(secondAdmin.email); + expect(secondAdminFromDatabase.accessType).toEqual(UserAccessType.ADMIN); + + // attempt to promote a regular user to admin + await expect(async () => { + await adminController.updateUserAccessLevel({ + accessUpdates: [ + { user: regularUser.email, accessType: UserAccessType.ADMIN }, + ], + }, admin); + }).rejects.toThrow(ForbiddenError); + + const regularUserFromDatabase = await repository.findOne({ email: regularUser.email }); + + expect(regularUserFromDatabase.email).toEqual(regularUser.email); + expect(regularUserFromDatabase.accessType).toEqual(UserAccessType.STANDARD); }); - test("ensure that the updating user's access level is not changed", async () => { + test("ensure that the updating user's access level is not changed & cannot demote themselves", async () => { const conn = await DatabaseConnection.get(); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); @@ -360,4 +377,41 @@ describe('updating user access level', () => { expect(existingAdmin[0].email).toEqual(admin.email); expect(existingAdmin[0].accessType).toEqual(UserAccessType.ADMIN); }); + + test('ensure that a user cannot demote themselves', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + + const staffUser = UserFactory.fake({ accessType: UserAccessType.STAFF }); + const standardUser = UserFactory.fake({ accessType: UserAccessType.STANDARD }); + const marketingUser = UserFactory.fake({ accessType: UserAccessType.MARKETING }); + const merchStoreDistributorUser = UserFactory.fake({ accessType: UserAccessType.MERCH_STORE_DISTRIBUTOR }); + + await new PortalState() + .createUsers(staffUser, standardUser, marketingUser, merchStoreDistributorUser, admin) + .write(); + + const adminController = ControllerFactory.admin(conn); + + // attempt to demote oneself + await expect(async () => { + 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 }, + { user: admin.email, accessType: UserAccessType.STANDARD }, + ], + }, admin); + }).rejects.toThrow(ForbiddenError); + + const repository = conn.getRepository(UserModel); + const existingAdmin = await repository.find({ + email: admin.email, + }); + + expect(existingAdmin[0].email).toEqual(admin.email); + expect(existingAdmin[0].accessType).toEqual(UserAccessType.ADMIN); + }); }); From 55aac1175272a8806bcbd533333d54fcda4e3767 Mon Sep 17 00:00:00 2001 From: Max Weng <73797155+maxwn04@users.noreply.github.com> Date: Sat, 17 Feb 2024 20:42:38 -0800 Subject: [PATCH 16/30] Store multiple collection images (#360) * migration * Schema and response classes and interfaces * validators and naming * Merch Collection Photo Repository * repository and models stuff * api request bug * renaming and bugfix * renaming and stuff * types * add/delete collection photo, edit collection fix service * naming and documentation * migration name fix * delete controller * renaming and shifting types for consistency * factory and start seed * bugfixes for postman test * tests * weird tests stuff * deletion logic change, cascade delete * sort fix * ling * naming and some position logic * merge stuff * seed data change * test change * remove automatically end of the list * lint stuff * build and lint fix * lint * more lint * fix migration * update version and database connection * remove unused collection in test * lint and migration rename --- api/controllers/MerchStoreController.ts | 37 ++++ api/validators/MerchStoreRequests.ts | 32 ++++ config/index.ts | 1 + migrations/0039-add-collection-image-table.ts | 58 ++++++ ...Event-column-to-orderPickupEvent-table.ts} | 0 models/MerchCollectionPhotoModel.ts | 33 ++++ models/MerchandiseCollectionModel.ts | 5 + models/index.ts | 2 + package.json | 2 +- repositories/MerchStoreRepository.ts | 32 +++- repositories/index.ts | 5 + services/MerchStoreService.ts | 111 +++++++++++- tests/Seeds.ts | 30 ++++ tests/data/DatabaseConnection.ts | 1 + tests/data/MerchFactory.ts | 27 ++- tests/data/PortalState.ts | 1 + tests/merchStore.test.ts | 167 ++++++++++++++++++ types/ApiRequests.ts | 23 ++- types/ApiResponses.ts | 14 ++ 19 files changed, 571 insertions(+), 10 deletions(-) create mode 100644 migrations/0039-add-collection-image-table.ts rename migrations/{0039-add-linkedEvent-column-to-orderPickupEvent-table.ts => 0040-add-linkedEvent-column-to-orderPickupEvent-table.ts} (100%) create mode 100644 models/MerchCollectionPhotoModel.ts diff --git a/api/controllers/MerchStoreController.ts b/api/controllers/MerchStoreController.ts index bfaca0b9..4bcf915e 100644 --- a/api/controllers/MerchStoreController.ts +++ b/api/controllers/MerchStoreController.ts @@ -20,6 +20,8 @@ import { GetAllMerchCollectionsResponse, CreateMerchCollectionResponse, EditMerchCollectionResponse, + CreateCollectionPhotoResponse, + DeleteCollectionPhotoResponse, GetOneMerchItemResponse, DeleteMerchCollectionResponse, CreateMerchItemResponse, @@ -55,6 +57,7 @@ import MerchStoreService from '../../services/MerchStoreService'; import { CreateMerchCollectionRequest, EditMerchCollectionRequest, + CreateCollectionPhotoRequest, CreateMerchItemRequest, EditMerchItemRequest, PlaceMerchOrderRequest, @@ -124,6 +127,40 @@ export class MerchStoreController { return { error: null }; } + @UseBefore(UserAuthentication) + @Post('/collection/picture/:uuid') + async createMerchCollectionPhoto(@UploadedFile('image', + { options: StorageService.getFileOptions(MediaType.MERCH_PHOTO) }) file: File, + @Params() params: UuidParam, + @Body() createCollectionRequest: CreateCollectionPhotoRequest, + @AuthenticatedUser() user: UserModel): Promise { + if (!PermissionsService.canEditMerchStore(user)) throw new ForbiddenError(); + + // generate a random string for the uploaded photo url + const position = parseInt(createCollectionRequest.position, 10); + if (Number.isNaN(position)) throw new BadRequestError('Position must be a number'); + const uniqueFileName = uuid(); + const uploadedPhoto = await this.storageService.uploadToFolder( + file, MediaType.MERCH_PHOTO, uniqueFileName, params.uuid, + ); + const collectionPhoto = await this.merchStoreService.createCollectionPhoto( + params.uuid, { uploadedPhoto, position }, + ); + + return { error: null, collectionPhoto }; + } + + @UseBefore(UserAuthentication) + @Delete('/collection/picture/:uuid') + async deleteMerchCollectionPhoto(@Params() params: UuidParam, @AuthenticatedUser() user: UserModel): + Promise { + if (!PermissionsService.canEditMerchStore(user)) throw new ForbiddenError(); + const photoToDelete = await this.merchStoreService.getCollectionPhotoForDeletion(params.uuid); + await this.storageService.deleteAtUrl(photoToDelete.uploadedPhoto); + await this.merchStoreService.deleteCollectionPhoto(photoToDelete); + return { error: null }; + } + @Get('/item/:uuid') async getOneMerchItem(@Params() params: UuidParam, @AuthenticatedUser() user: UserModel): Promise { diff --git a/api/validators/MerchStoreRequests.ts b/api/validators/MerchStoreRequests.ts index 379b740b..5159025e 100644 --- a/api/validators/MerchStoreRequests.ts +++ b/api/validators/MerchStoreRequests.ts @@ -17,6 +17,7 @@ import { Type } from 'class-transformer'; import { CreateMerchCollectionRequest as ICreateMerchCollectionRequest, EditMerchCollectionRequest as IEditMerchCollectionRequest, + CreateCollectionPhotoRequest as ICreateCollectionPhotoRequest, CreateMerchItemRequest as ICreateMerchItemRequest, EditMerchItemRequest as IEditMerchItemRequest, CreateMerchItemOptionRequest as ICreateMerchItemOptionRequest, @@ -32,6 +33,8 @@ import { OrderItemFulfillmentUpdate as IOrderItemFulfillmentUpdate, MerchCollection as IMerchCollection, MerchCollectionEdit as IMerchCollectionEdit, + MerchCollectionPhoto as IMerchCollectionPhoto, + MerchCollectionPhotoEdit as IMerchCollectionPhotoEdit, MerchItem as IMerchItem, MerchItemEdit as IMerchItemEdit, MerchItemOption as IMerchItemOption, @@ -60,6 +63,9 @@ export class MerchCollection implements IMerchCollection { @Allow() archived?: boolean; + + @Allow() + collectionPhotos: MerchCollectionPhoto[]; } export class MerchCollectionEdit implements IMerchCollectionEdit { @@ -78,6 +84,26 @@ export class MerchCollectionEdit implements IMerchCollectionEdit { @Min(0) @Max(100) discountPercentage?: number; + + @Allow() + collectionPhotos?: MerchCollectionPhotoEdit[]; +} + +export class MerchCollectionPhoto implements IMerchCollectionPhoto { + @Allow() + uploadedPhoto: string; + + @Allow() + position: number; +} + +export class MerchCollectionPhotoEdit implements IMerchCollectionPhotoEdit { + @IsDefined() + @IsUUID() + uuid: string; + + @Allow() + position: number; } export class MerchItemOptionMetadata implements IMerchItemOptionMetadata { @@ -305,6 +331,12 @@ export class EditMerchCollectionRequest implements IEditMerchCollectionRequest { collection: MerchCollectionEdit; } +export class CreateCollectionPhotoRequest implements ICreateCollectionPhotoRequest { + @IsDefined() + @IsNumberString() + position: string; +} + export class CreateMerchItemRequest implements ICreateMerchItemRequest { @Type(() => MerchItem) @ValidateNested() diff --git a/config/index.ts b/config/index.ts index 58c321b7..b2a13d26 100644 --- a/config/index.ts +++ b/config/index.ts @@ -60,6 +60,7 @@ export const Config = { MAX_EVENT_COVER_FILE_SIZE: Number(process.env.MAX_EVENT_COVER_FILE_SIZE) * BYTES_PER_KILOBYTE, MAX_BANNER_FILE_SIZE: Number(process.env.MAX_BANNER_FILE_SIZE) * BYTES_PER_KILOBYTE, MAX_MERCH_PHOTO_FILE_SIZE: Number(process.env.MAX_MERCH_PHOTO_FILE_SIZE) * BYTES_PER_KILOBYTE, + MAX_COLLECTION_PHOTO_FILE_SIZE: Number(process.env.MAX_MERCH_PHOTO_FILE_SIZE) * BYTES_PER_KILOBYTE, MAX_RESUME_FILE_SIZE: Number(process.env.MAX_RESUME_FILE_SIZE) * BYTES_PER_KILOBYTE, PROFILE_PICTURE_UPLOAD_PATH: process.env.BASE_UPLOAD_PATH + process.env.PROFILE_PICTURE_UPLOAD_PATH, EVENT_COVER_UPLOAD_PATH: process.env.BASE_UPLOAD_PATH + process.env.EVENT_COVER_UPLOAD_PATH, diff --git a/migrations/0039-add-collection-image-table.ts b/migrations/0039-add-collection-image-table.ts new file mode 100644 index 00000000..eef968fd --- /dev/null +++ b/migrations/0039-add-collection-image-table.ts @@ -0,0 +1,58 @@ +import { MigrationInterface, QueryRunner, Table, TableIndex } from 'typeorm'; + +const TABLE_NAME = 'MerchCollectionPhotos'; +const COLLECTION_TABLE_NAME = 'MerchandiseCollections'; + +export class AddCollectionImageTable1696990832868 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.createTable(new Table({ + name: TABLE_NAME, + columns: [ + { + name: 'uuid', + type: 'uuid', + isGenerated: true, + isPrimary: true, + generationStrategy: 'uuid', + }, + { + name: 'merchCollection', + type: 'uuid', + }, + { + name: 'uploadedPhoto', + type: 'varchar(255)', + }, + { + name: 'uploadedAt', + type: 'timestamptz', + default: 'CURRENT_TIMESTAMP(6)', + }, + { + name: 'position', + type: 'integer', + }, + ], + // cascade delete + foreignKeys: [ + { + columnNames: ['merchCollection'], + referencedTableName: COLLECTION_TABLE_NAME, + referencedColumnNames: ['uuid'], + onDelete: 'CASCADE', + }, + ], + })); + + await queryRunner.createIndices(TABLE_NAME, [ + new TableIndex({ + name: 'images_by_collection_index', + columnNames: ['merchCollection'], + }), + ]); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropTable(TABLE_NAME); + } +} diff --git a/migrations/0039-add-linkedEvent-column-to-orderPickupEvent-table.ts b/migrations/0040-add-linkedEvent-column-to-orderPickupEvent-table.ts similarity index 100% rename from migrations/0039-add-linkedEvent-column-to-orderPickupEvent-table.ts rename to migrations/0040-add-linkedEvent-column-to-orderPickupEvent-table.ts diff --git a/models/MerchCollectionPhotoModel.ts b/models/MerchCollectionPhotoModel.ts new file mode 100644 index 00000000..e3e3bdbe --- /dev/null +++ b/models/MerchCollectionPhotoModel.ts @@ -0,0 +1,33 @@ +import { BaseEntity, Column, Entity, Index, JoinColumn, ManyToOne, PrimaryGeneratedColumn } from 'typeorm'; +import { PublicMerchCollectionPhoto, Uuid } from '../types'; +import { MerchandiseCollectionModel } from './MerchandiseCollectionModel'; + +@Entity('MerchCollectionPhotos') +export class MerchCollectionPhotoModel extends BaseEntity { + @PrimaryGeneratedColumn('uuid') + uuid: Uuid; + + @ManyToOne((type) => MerchandiseCollectionModel, + (merchCollection) => merchCollection.collectionPhotos, { nullable: false, onDelete: 'CASCADE' }) + @JoinColumn({ name: 'merchCollection' }) + @Index('images_by_collection_index') + merchCollection: MerchandiseCollectionModel; + + @Column('varchar', { length: 255, nullable: false }) + uploadedPhoto: string; + + @Column('timestamptz', { default: () => 'CURRENT_TIMESTAMP(6)', nullable: false }) + uploadedAt: Date; + + @Column('integer') + position: number; + + public getPublicMerchCollectionPhoto(): PublicMerchCollectionPhoto { + return { + uuid: this.uuid, + uploadedPhoto: this.uploadedPhoto, + uploadedAt: this.uploadedAt, + position: this.position, + }; + } +} diff --git a/models/MerchandiseCollectionModel.ts b/models/MerchandiseCollectionModel.ts index 94b30bcf..6c7d5da2 100644 --- a/models/MerchandiseCollectionModel.ts +++ b/models/MerchandiseCollectionModel.ts @@ -1,6 +1,7 @@ import { Entity, BaseEntity, Column, PrimaryGeneratedColumn, OneToMany, CreateDateColumn } from 'typeorm'; import { Uuid, PublicMerchCollection } from '../types'; import { MerchandiseItemModel } from './MerchandiseItemModel'; +import { MerchCollectionPhotoModel } from './MerchCollectionPhotoModel'; @Entity('MerchandiseCollections') export class MerchandiseCollectionModel extends BaseEntity { @@ -25,10 +26,14 @@ export class MerchandiseCollectionModel extends BaseEntity { @OneToMany((type) => MerchandiseItemModel, (item) => item.collection, { cascade: true }) items: MerchandiseItemModel[]; + @OneToMany((type) => MerchCollectionPhotoModel, (photo) => photo.merchCollection, { cascade: true }) + collectionPhotos: MerchCollectionPhotoModel[]; + public getPublicMerchCollection(canSeeHiddenItems = false): PublicMerchCollection { const baseMerchCollection: any = { uuid: this.uuid, title: this.title, + collectionPhotos: this.collectionPhotos, themeColorHex: this.themeColorHex, description: this.description, createdAt: this.createdAt, diff --git a/models/index.ts b/models/index.ts index e571f4e2..a077f5dc 100644 --- a/models/index.ts +++ b/models/index.ts @@ -3,6 +3,7 @@ import { ActivityModel } from './ActivityModel'; import { EventModel } from './EventModel'; import { AttendanceModel } from './AttendanceModel'; import { MerchandiseCollectionModel } from './MerchandiseCollectionModel'; +import { MerchCollectionPhotoModel } from './MerchCollectionPhotoModel'; import { MerchandiseItemModel } from './MerchandiseItemModel'; import { MerchandiseItemPhotoModel } from './MerchandiseItemPhotoModel'; import { OrderModel } from './OrderModel'; @@ -20,6 +21,7 @@ export const models = [ EventModel, AttendanceModel, MerchandiseCollectionModel, + MerchCollectionPhotoModel, MerchandiseItemModel, MerchandiseItemPhotoModel, MerchandiseItemOptionModel, diff --git a/package.json b/package.json index 39b41095..83243d05 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.1.0", + "version": "3.2.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/repositories/MerchStoreRepository.ts b/repositories/MerchStoreRepository.ts index 7f5f7e47..0eedb4b7 100644 --- a/repositories/MerchStoreRepository.ts +++ b/repositories/MerchStoreRepository.ts @@ -2,6 +2,7 @@ import { EntityRepository, SelectQueryBuilder } from 'typeorm'; import { MerchandiseItemOptionModel } from '../models/MerchandiseItemOptionModel'; import { MerchandiseItemPhotoModel } from '../models/MerchandiseItemPhotoModel'; import { MerchandiseCollectionModel } from '../models/MerchandiseCollectionModel'; +import { MerchCollectionPhotoModel } from '../models/MerchCollectionPhotoModel'; import { MerchandiseItemModel } from '../models/MerchandiseItemModel'; import { Uuid } from '../types'; import { BaseRepository } from './BaseRepository'; @@ -40,15 +41,44 @@ export class MerchCollectionRepository extends BaseRepository { return this.repository.createQueryBuilder('collection') .leftJoinAndSelect('collection.items', 'items') + .leftJoinAndSelect('collection.collectionPhotos', 'collectionPhotos') .leftJoinAndSelect('items.options', 'options') .leftJoinAndSelect('items.merchPhotos', 'merchPhotos'); } } +@EntityRepository(MerchCollectionPhotoModel) +export class MerchCollectionPhotoRepository extends BaseRepository { + public async findByUuid(uuid: Uuid): Promise { + return this.repository.findOne(uuid, { relations: ['merchCollection'] }); + } + + // for querying a group of pictures together + public async batchFindByUuid(uuids: Uuid[]): Promise> { + const photos = await this.repository.findByIds(uuids, { relations: ['merchCollection'] }); + return new Map(photos.map((o) => [o.uuid, o])); + } + + public async upsertCollectionPhoto(photo: MerchCollectionPhotoModel, + changes?: Partial): Promise { + if (changes) photo = MerchCollectionPhotoModel.merge(photo, changes); + return this.repository.save(photo); + } + + public async deleteCollectionPhoto(photo: MerchCollectionPhotoModel): Promise { + await this.repository.remove(photo); + } +} + @EntityRepository(MerchandiseItemModel) export class MerchItemRepository extends BaseRepository { public async findByUuid(uuid: Uuid): Promise { - return this.repository.findOne(uuid, { relations: ['collection', 'options', 'merchPhotos'] }); + return this.repository.findOne(uuid, { relations: [ + 'collection', + 'options', + 'merchPhotos', + 'collection.collectionPhotos', + ] }); } public async upsertMerchItem(item: MerchandiseItemModel, changes?: Partial): diff --git a/repositories/index.ts b/repositories/index.ts index 02a50c92..540ab69d 100644 --- a/repositories/index.ts +++ b/repositories/index.ts @@ -8,6 +8,7 @@ import { MerchCollectionRepository, MerchItemRepository, MerchItemOptionRepository, + MerchCollectionPhotoRepository, MerchItemPhotoRepository, } from './MerchStoreRepository'; import { ActivityRepository } from './ActivityRepository'; @@ -48,6 +49,10 @@ export default class Repositories { return transactionalEntityManager.getCustomRepository(MerchCollectionRepository); } + public static merchStoreCollectionPhoto(transactionalEntityManager: EntityManager): MerchCollectionPhotoRepository { + return transactionalEntityManager.getCustomRepository(MerchCollectionPhotoRepository); + } + public static merchStoreItem(transactionalEntityManager: EntityManager): MerchItemRepository { return transactionalEntityManager.getCustomRepository(MerchItemRepository); } diff --git a/services/MerchStoreService.ts b/services/MerchStoreService.ts index 9bdac82b..01618694 100644 --- a/services/MerchStoreService.ts +++ b/services/MerchStoreService.ts @@ -25,6 +25,8 @@ import { OrderPickupEventStatus, PublicMerchItemPhoto, MerchItemPhoto, + PublicMerchCollectionPhoto, + MerchCollectionPhoto, } from '../types'; import { MerchandiseItemModel } from '../models/MerchandiseItemModel'; import { OrderModel } from '../models/OrderModel'; @@ -32,6 +34,7 @@ import { UserModel } from '../models/UserModel'; import { EventModel } from '../models/EventModel'; import Repositories, { TransactionsManager } from '../repositories'; import { MerchandiseCollectionModel } from '../models/MerchandiseCollectionModel'; +import { MerchCollectionPhotoModel } from '../models/MerchCollectionPhotoModel'; import EmailService, { OrderInfo, OrderPickupEventInfo } from './EmailService'; import { UserError } from '../utils/Errors'; import { OrderItemModel } from '../models/OrderItemModel'; @@ -42,6 +45,8 @@ import { MerchandiseItemPhotoModel } from '../models/MerchandiseItemPhotoModel'; export default class MerchStoreService { private static readonly MAX_MERCH_PHOTO_COUNT = 5; + private static readonly MAX_COLLECTION_PHOTO_COUNT = 5; + private emailService: EmailService; private transactions: TransactionsManager; @@ -57,6 +62,7 @@ export default class MerchStoreService { .findByUuid(uuid)); if (!collection) throw new NotFoundError('Merch collection not found'); if (collection.archived && !canSeeInactiveCollections) throw new ForbiddenError(); + collection.collectionPhotos = collection.collectionPhotos.sort((a, b) => a.position - b.position); return canSeeInactiveCollections ? collection : collection.getPublicMerchCollection(); } @@ -78,14 +84,15 @@ export default class MerchStoreService { .upsertMerchCollection(MerchandiseCollectionModel.create(collection))); } - public async editCollection(uuid: Uuid, changes: MerchCollectionEdit): Promise { + public async editCollection(uuid: Uuid, collectionEdit: MerchCollectionEdit): Promise { return this.transactions.readWrite(async (txn) => { const merchCollectionRepository = Repositories.merchStoreCollection(txn); const currentCollection = await merchCollectionRepository.findByUuid(uuid); if (!currentCollection) throw new NotFoundError('Merch collection not found'); - let updatedCollection = await merchCollectionRepository.upsertMerchCollection(currentCollection, changes); - if (changes.discountPercentage !== undefined) { - const { discountPercentage } = changes; + + const { discountPercentage, collectionPhotos, ...changes } = collectionEdit; + + if (discountPercentage !== undefined) { await Repositories .merchStoreItemOption(txn) .updateMerchItemOptionsInCollection(uuid, discountPercentage); @@ -95,9 +102,33 @@ export default class MerchStoreService { .merchStoreItem(txn) .updateMerchItemsInCollection(uuid, { hidden: changes.archived }); } - if (changes.discountPercentage !== undefined || changes.archived !== undefined) { + + // this part only handles updating the positions of the pictures + if (collectionPhotos) { + // error on duplicate photo uuids + const dupSet = new Set(); + collectionPhotos.forEach((merchPhoto) => { + if (dupSet.has(merchPhoto.uuid)) { + throw new UserError(`Multiple edits is made to photo: ${merchPhoto.uuid}`); + } + dupSet.add(merchPhoto.uuid); + }); + + const photoUpdatesByUuid = new Map(collectionPhotos.map((merchPhoto) => [merchPhoto.uuid, merchPhoto])); + + currentCollection.collectionPhotos.map((currentPhoto) => { + if (!photoUpdatesByUuid.has(currentPhoto.uuid)) return; + const photoUpdate = photoUpdatesByUuid.get(currentPhoto.uuid); + return MerchCollectionPhotoModel.merge(currentPhoto, photoUpdate); + }); + } + + let updatedCollection = await merchCollectionRepository.upsertMerchCollection(currentCollection, changes); + + if (discountPercentage !== undefined || changes.archived !== undefined) { updatedCollection = await merchCollectionRepository.findByUuid(uuid); } + return updatedCollection; }); } @@ -115,6 +146,76 @@ export default class MerchStoreService { }); } + /** + * Verify that collections have valid photots. + */ + private static verifyCollectionHasValidPhotos(collection: MerchCollection | MerchandiseCollectionModel) { + if (collection.collectionPhotos.length > this.MAX_COLLECTION_PHOTO_COUNT) { + throw new UserError('Collections cannot have more than 5 pictures'); + } + } + + /** + * Creates a collection photo and assign it the corresponding picture url + * and append the photo to the photos list from merchItem + * @param collection merch collection uuid + * @param properties merch collection photo picture url and position + * @returns created collection photo + */ + public async createCollectionPhoto(collection: Uuid, properties: MerchCollectionPhoto): + Promise { + return this.transactions.readWrite(async (txn) => { + const merchCollection = await Repositories.merchStoreCollection(txn).findByUuid(collection); + if (!merchCollection) throw new NotFoundError('Collection not found'); + + const createdPhoto = MerchCollectionPhotoModel.create({ ...properties, merchCollection }); + const merchStoreCollectionPhotoRepository = Repositories.merchStoreCollectionPhoto(txn); + + // verify the result photos array + merchCollection.collectionPhotos.push(createdPhoto); + MerchStoreService.verifyCollectionHasValidPhotos(merchCollection); + + const upsertedPhoto = await merchStoreCollectionPhotoRepository.upsertCollectionPhoto(createdPhoto); + return upsertedPhoto.getPublicMerchCollectionPhoto(); + }); + } + + /** + * Check if the photo is ready to be deleted. Fail if the merch item is visible + * and it was the only photo of the item. + * + * @param uuid the uuid of photo to be deleted + * @returns the photo object to be removed from database + */ + public async getCollectionPhotoForDeletion(uuid: Uuid): Promise { + return this.transactions.readWrite(async (txn) => { + const merchCollectionPhotoRepository = Repositories.merchStoreCollectionPhoto(txn); + const collectionPhoto = await merchCollectionPhotoRepository.findByUuid(uuid); + if (!collectionPhoto) throw new NotFoundError('Merch collection photo not found'); + + const collection = await Repositories.merchStoreCollection(txn).findByUuid(collectionPhoto.merchCollection.uuid); + if (collection.collectionPhotos.length === 1) { + throw new UserError('Cannot delete the only photo for a collection'); + } + + return collectionPhoto; + }); + } + + /** + * Deletes the given item photo. + * + * @param merchPhoto the photo object to be removed + * @returns the photo object removed from database + */ + public async deleteCollectionPhoto(collectionPhoto: MerchCollectionPhotoModel): Promise { + return this.transactions.readWrite(async (txn) => { + const merchStoreItemPhotoRepository = Repositories.merchStoreCollectionPhoto(txn); + await merchStoreItemPhotoRepository.deleteCollectionPhoto(collectionPhoto); + return collectionPhoto; + }); + } + public async findItemByUuid(uuid: Uuid, user: UserModel): Promise { return this.transactions.readOnly(async (txn) => { const item = await Repositories.merchStoreItem(txn).findByUuid(uuid); diff --git a/tests/Seeds.ts b/tests/Seeds.ts index 31c57a72..7954f197 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -378,6 +378,28 @@ async function seed(): Promise { description: 'Do you like to code? Tell the world with this Hack School inspired collection.', themeColorHex: '#EB8C34', }); + + const MERCH_COLLECTION_1_PHOTO_1 = MerchFactory.fakeCollectionPhoto({ + merchCollection: MERCH_COLLECTION_1, + position: 0, + }); + const MERCH_COLLECTION_1_PHOTO_2 = MerchFactory.fakeCollectionPhoto({ + merchCollection: MERCH_COLLECTION_1, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 1, + }); + const MERCH_COLLECTION_1_PHOTO_3 = MerchFactory.fakeCollectionPhoto({ + merchCollection: MERCH_COLLECTION_1, + uploadedPhoto: 'https://i.imgur.com/pSZ921P.png', + position: 2, + }); + + MERCH_COLLECTION_1.collectionPhotos = [ + MERCH_COLLECTION_1_PHOTO_1, + MERCH_COLLECTION_1_PHOTO_2, + MERCH_COLLECTION_1_PHOTO_3, + ]; + const MERCH_ITEM_1 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_1, itemName: 'Unisex Hack School Anorak', @@ -538,6 +560,14 @@ async function seed(): Promise { title: 'Fall 2001', description: 'Celebrate the opening of Sixth College in style, featuring raccoon print jackets.', }); + + const MERCH_COLLECTION_2_PHOTO_1 = MerchFactory.fakeCollectionPhoto({ + merchCollection: MERCH_COLLECTION_2, + uploadedPhoto: 'https://www.fakepicture.com/', + position: 1, + }); + MERCH_COLLECTION_2.collectionPhotos = [MERCH_COLLECTION_2_PHOTO_1]; + const MERCH_ITEM_3 = MerchFactory.fakeItem({ collection: MERCH_COLLECTION_2, itemName: 'Camp Snoopy Snapback', diff --git a/tests/data/DatabaseConnection.ts b/tests/data/DatabaseConnection.ts index d3913b39..bbe166bb 100644 --- a/tests/data/DatabaseConnection.ts +++ b/tests/data/DatabaseConnection.ts @@ -44,6 +44,7 @@ export class DatabaseConnection { 'Feedback', 'Resumes', 'UserSocialMedia', + 'MerchCollectionPhotos', ]; await Promise.all(tableNames.map((t) => txn.query(`DELETE FROM "${t}"`))); }); diff --git a/tests/data/MerchFactory.ts b/tests/data/MerchFactory.ts index a97ace12..b2834932 100644 --- a/tests/data/MerchFactory.ts +++ b/tests/data/MerchFactory.ts @@ -1,13 +1,14 @@ import * as faker from 'faker'; import * as moment from 'moment'; import { v4 as uuid } from 'uuid'; -import FactoryUtils from './FactoryUtils'; import { MerchItemOptionMetadata, OrderPickupEventStatus } from '../../types'; import { OrderPickupEventModel } from '../../models/OrderPickupEventModel'; import { MerchandiseCollectionModel } from '../../models/MerchandiseCollectionModel'; +import { MerchCollectionPhotoModel } from '../../models/MerchCollectionPhotoModel'; import { MerchandiseItemModel } from '../../models/MerchandiseItemModel'; import { MerchandiseItemOptionModel } from '../../models/MerchandiseItemOptionModel'; import { MerchandiseItemPhotoModel } from '../../models/MerchandiseItemPhotoModel'; +import FactoryUtils from './FactoryUtils'; export class MerchFactory { public static fakeCollection(substitute?: Partial): MerchandiseCollectionModel { @@ -28,6 +29,14 @@ export class MerchFactory { hidden: substitute?.archived, })); } + + if (!substitute?.collectionPhotos) { + const numPhotos = FactoryUtils.getRandomNumber(1, 5); + fake.collectionPhotos = MerchFactory + .createCollectionPhotos(numPhotos) + .map((collectionPhoto) => MerchCollectionPhotoModel.merge(collectionPhoto, { merchCollection: fake })); + } + return MerchandiseCollectionModel.merge(fake, substitute); } @@ -69,6 +78,22 @@ export class MerchFactory { return MerchandiseItemPhotoModel.merge(fake, substitute); } + public static fakeCollectionPhoto(substitute?: Partial): MerchCollectionPhotoModel { + const fake = MerchCollectionPhotoModel.create({ + uuid: uuid(), + position: 0, + uploadedPhoto: 'https://www.fakepicture.com/', + uploadedAt: faker.date.recent(), + }); + return MerchCollectionPhotoModel.merge(fake, substitute); + } + + private static createCollectionPhotos(n: number): MerchCollectionPhotoModel[] { + return FactoryUtils + .create(n, () => MerchFactory.fakeCollectionPhoto()) + .map((collectionPhoto, i) => MerchCollectionPhotoModel.merge(collectionPhoto, { position: i })); + } + public static fakeOption(substitute?: Partial): MerchandiseItemOptionModel { const fake = MerchandiseItemOptionModel.create({ uuid: uuid(), diff --git a/tests/data/PortalState.ts b/tests/data/PortalState.ts index 2838b85a..bb2a5f6f 100644 --- a/tests/data/PortalState.ts +++ b/tests/data/PortalState.ts @@ -100,6 +100,7 @@ export class PortalState { public createMerchItem(item: MerchandiseItemModel): PortalState { const collectionWithItem = MerchFactory.fakeCollection({ items: [item] }); + // console.log(collectionWithItem) return this.createMerchCollections(collectionWithItem); } diff --git a/tests/merchStore.test.ts b/tests/merchStore.test.ts index c04a8e7c..cbaa4bce 100644 --- a/tests/merchStore.test.ts +++ b/tests/merchStore.test.ts @@ -161,6 +161,7 @@ describe('creating merch collections', () => { .toEqual(expectedCollectionOrder); }); }); + describe('editing merch collections', () => { test('only admins can edit merch collections', async () => { const conn = await DatabaseConnection.get(); @@ -187,6 +188,172 @@ describe('editing merch collections', () => { }); }); +describe('merch collection photos', () => { + const folderLocation = 'https://s3.amazonaws.com/upload-photo/'; + + test('can create a collection with up to 5 pictures', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const photo1 = MerchFactory.fakeCollectionPhoto(); + const collection = MerchFactory.fakeCollection({ collectionPhotos: [photo1] }); + + await new PortalState() + .createUsers(admin) + .createMerchCollections(collection) + .write(); + + const image2 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const image3 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const image4 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const image5 = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const imageExtra = FileFactory.image(Config.file.MAX_MERCH_PHOTO_FILE_SIZE / 2); + const storageService = Mocks.storage(folderLocation); + + const merchStoreController = ControllerFactory.merchStore( + conn, + undefined, + instance(storageService), + ); + + const params = { uuid: collection.uuid }; + + const response2 = await merchStoreController.createMerchCollectionPhoto(image2, params, { position: '1' }, admin); + const response3 = await merchStoreController.createMerchCollectionPhoto(image3, params, { position: '2' }, admin); + const response4 = await merchStoreController.createMerchCollectionPhoto(image4, params, { position: '3' }, admin); + const response5 = await merchStoreController.createMerchCollectionPhoto(image5, params, { position: '4' }, admin); + + // checking no error is thrown and storage is correctly modified + // enough to check first and last response + expect(response2.error).toBe(null); + expect(response5.error).toBe(null); + verify( + storageService.uploadToFolder( + image2, + MediaType.MERCH_PHOTO, + anything(), + anything(), + ), + ).called(); + verify( + storageService.uploadToFolder( + image5, + MediaType.MERCH_PHOTO, + anything(), + anything(), + ), + ).called(); + + const photo2 = response2.collectionPhoto; + const photo3 = response3.collectionPhoto; + const photo4 = response4.collectionPhoto; + const photo5 = response5.collectionPhoto; + + // 0 index + expect(photo2.position).toBe(1); + expect(photo3.position).toBe(2); + expect(photo4.position).toBe(3); + expect(photo5.position).toBe(4); + + const photos = [photo1, photo2, photo3, photo4, photo5]; + expect((await merchStoreController.getOneMerchCollection(params, admin)).collection.collectionPhotos) + .toEqual(photos); + + expect(merchStoreController.createMerchCollectionPhoto(imageExtra, params, { position: '5' }, admin)) + .rejects.toThrow('Collections cannot have more than 5 pictures'); + }); + + test('can remap the picture of a collection to different orders', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const photo1 = MerchFactory.fakeCollectionPhoto({ position: 0 }); + const photo2 = MerchFactory.fakeCollectionPhoto({ position: 1 }); + const photo3 = MerchFactory.fakeCollectionPhoto({ position: 2 }); + const photo4 = MerchFactory.fakeCollectionPhoto({ position: 3 }); + const photo5 = MerchFactory.fakeCollectionPhoto({ position: 4 }); + const collectionPhotos = [photo1, photo2, photo3, photo4, photo5]; + const collection = MerchFactory.fakeCollection({ collectionPhotos }); + + await new PortalState() + .createUsers(admin) + .createMerchCollections(collection) + .write(); + + const merchStoreController = ControllerFactory.merchStore(conn); + const params = { uuid: collection.uuid }; + + // check before remap whether photos are correctly positioned + expect((await merchStoreController.getOneMerchCollection(params, admin)) + .collection.collectionPhotos).toEqual(collectionPhotos); + + // reversing the order of the photos + const editMerchCollectionRequest = { collection: { + collectionPhotos: [ + { uuid: photo5.uuid, position: 0 }, + { uuid: photo4.uuid, position: 1 }, + { uuid: photo3.uuid, position: 2 }, + { uuid: photo2.uuid, position: 3 }, + { uuid: photo1.uuid, position: 4 }, + ], + } }; + + await merchStoreController.editMerchCollection(params, editMerchCollectionRequest, admin); + + const newPhotos = (await merchStoreController.getOneMerchCollection(params, admin)).collection.collectionPhotos; + const newPhotosUuids = newPhotos.map((photo) => photo.uuid); + const expectedPhotosUuids = [photo5.uuid, photo4.uuid, photo3.uuid, photo2.uuid, photo1.uuid]; + expect(newPhotosUuids).toStrictEqual(expectedPhotosUuids); + }); + + test('can delete photo until 1 photo left except merch collection is deleted', async () => { + const conn = await DatabaseConnection.get(); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const photo1 = MerchFactory.fakeCollectionPhoto({ position: 0 }); + const photo2 = MerchFactory.fakeCollectionPhoto({ position: 1 }); + const collectionPhotos = [photo1, photo2]; + const collection = MerchFactory.fakeCollection({ collectionPhotos }); + + await new PortalState() + .createUsers(admin) + .createMerchCollections(collection) + .write(); + + const storageService = Mocks.storage(); + const merchStoreController = ControllerFactory.merchStore( + conn, + undefined, + instance(storageService), + ); + const params = { uuid: collection.uuid }; + + // verify before deleting, the photos all exist + const collectionInDatabase = (await merchStoreController.getOneMerchCollection(params, admin)).collection; + expect(collectionInDatabase.collectionPhotos).toEqual(collectionPhotos); + + const deleteMerchCollectionPhotoParam1 = { uuid: photo1.uuid }; + const deleteMerchCollectionPhotoParam2 = { uuid: photo2.uuid }; + + // verify deletion delete correctly + await merchStoreController.deleteMerchCollectionPhoto(deleteMerchCollectionPhotoParam1, admin); + const expectedUrl = collectionInDatabase.collectionPhotos[0].uploadedPhoto; + verify(storageService.deleteAtUrl(expectedUrl)).called(); + + const newPhotos = (await merchStoreController.getOneMerchCollection(params, admin)).collection.collectionPhotos; + + expect(newPhotos).toHaveLength(1); + expect(newPhotos[0].uuid).toEqual(photo2.uuid); + expect(newPhotos[0].position).toEqual(1); + + // verify visible item photo limitation + expect(merchStoreController.deleteMerchCollectionPhoto(deleteMerchCollectionPhotoParam2, admin)) + .rejects.toThrow('Cannot delete the only photo for a collection'); + + // check cascade + await merchStoreController.deleteMerchCollection(params, admin); + expect(merchStoreController.deleteMerchCollectionPhoto(deleteMerchCollectionPhotoParam2, admin)) + .rejects.toThrow(NotFoundError); + }); +}); + describe('archived merch collections', () => { test('only admins can view archived collections', async () => { const conn = await DatabaseConnection.get(); diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index d4807c6b..0a733272 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -193,6 +193,10 @@ export interface EditMerchCollectionRequest { collection: MerchCollectionEdit; } +export interface CreateCollectionPhotoRequest { + position: string; +} + export interface CreateMerchItemRequest { merchandise: MerchItem; } @@ -226,15 +230,30 @@ export interface RescheduleOrderPickupRequest { pickupEvent: Uuid; } -export interface MerchCollection { +export interface CommonCollectionProperties { title: string; themeColorHex?: string; description: string; archived?: boolean; } -export interface MerchCollectionEdit extends Partial { +export interface MerchCollectionPhoto { + uploadedPhoto: string; + position: number; +} + +export interface MerchCollectionPhotoEdit { + uuid: string; + position?: number; +} + +export interface MerchCollection extends Partial { + collectionPhotos: MerchCollectionPhoto[] +} + +export interface MerchCollectionEdit extends Partial { discountPercentage?: number; + collectionPhotos?: MerchCollectionPhotoEdit[] } export interface CommonMerchItemProperties { diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index cb93f046..1884cdd4 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -157,6 +157,7 @@ export interface PublicMerchCollection { themeColorHex?: string; description: string; items: PublicMerchItem[]; + collectionPhotos: PublicMerchCollectionPhoto[] createdAt: Date; } @@ -200,6 +201,13 @@ export interface PublicMerchItemPhoto { uploadedAt: Date; } +export interface PublicMerchCollectionPhoto { + uuid: Uuid; + uploadedPhoto: string; + position: number; + uploadedAt: Date +} + export interface PublicOrderMerchItemOption { uuid: Uuid; price: number; @@ -249,6 +257,12 @@ export interface EditMerchCollectionResponse extends ApiResponse { export interface DeleteMerchCollectionResponse extends ApiResponse {} +export interface CreateCollectionPhotoResponse extends ApiResponse { + collectionPhoto: PublicMerchCollectionPhoto; +} + +export interface DeleteCollectionPhotoResponse extends ApiResponse {} + export interface GetOneMerchItemResponse extends ApiResponse { item: PublicMerchItemWithPurchaseLimits; } From 635f3d5d6b60e95fe56c18b2a164303c7f45b291 Mon Sep 17 00:00:00 2001 From: Shravan Hariharan <33337209+shravanhariharan2@users.noreply.github.com> Date: Sat, 24 Feb 2024 13:45:04 -0800 Subject: [PATCH 17/30] Implement Express Check-in (#344) * progress * Implement POST /attendance/expressCheckin * touchups * fix build error * finish tests for POST /attendance/expressCheckin * fix * finish registration logic and tests * remove redundant nullable * update email template * updated branch and renamed migration file --------- Co-authored-by: Nikhil Dange --- api/controllers/AttendanceController.ts | 21 ++- .../AttendanceControllerRequests.ts | 17 +- migrations/0041-add-expressCheckins-table.ts | 38 +++++ models/EventModel.ts | 4 + models/ExpressCheckinModel.ts | 28 ++++ models/index.ts | 2 + repositories/AttendanceRepository.ts | 12 ++ repositories/UserRepository.ts | 5 + repositories/index.ts | 6 +- services/AttendanceService.ts | 56 ++++++- services/EmailService.ts | 22 +++ services/UserAuthService.ts | 35 +++- templates/expressCheckinConfirmation.ejs | 155 ++++++++++++++++++ tests/auth.test.ts | 66 +++++++- tests/controllers/ControllerFactory.ts | 4 +- tests/data/DatabaseConnection.ts | 1 + tests/data/PortalState.ts | 14 ++ tests/expressCheckin.test.ts | 130 +++++++++++++++ types/ApiRequests.ts | 5 + types/ApiResponses.ts | 6 + types/internal/index.ts | 2 +- 21 files changed, 606 insertions(+), 23 deletions(-) create mode 100644 migrations/0041-add-expressCheckins-table.ts create mode 100644 models/ExpressCheckinModel.ts create mode 100644 templates/expressCheckinConfirmation.ejs create mode 100644 tests/expressCheckin.test.ts diff --git a/api/controllers/AttendanceController.ts b/api/controllers/AttendanceController.ts index 5487c98a..e47bd121 100644 --- a/api/controllers/AttendanceController.ts +++ b/api/controllers/AttendanceController.ts @@ -1,22 +1,26 @@ import { JsonController, Get, Post, UseBefore, Params, ForbiddenError, Body } from 'routing-controllers'; +import EmailService from '../../services/EmailService'; import { UserAuthentication } from '../middleware/UserAuthentication'; import { AuthenticatedUser } from '../decorators/AuthenticatedUser'; -import { AttendEventRequest } from '../validators/AttendanceControllerRequests'; +import { AttendEventRequest, AttendViaExpressCheckinRequest } from '../validators/AttendanceControllerRequests'; import { UserModel } from '../../models/UserModel'; import AttendanceService from '../../services/AttendanceService'; import PermissionsService from '../../services/PermissionsService'; import { GetAttendancesForEventResponse, GetAttendancesForUserResponse, AttendEventResponse } from '../../types'; import { UuidParam } from '../validators/GenericRequests'; -@UseBefore(UserAuthentication) @JsonController('/attendance') export class AttendanceController { private attendanceService: AttendanceService; - constructor(attendanceService: AttendanceService) { + private emailService: EmailService; + + constructor(attendanceService: AttendanceService, emailService: EmailService) { this.attendanceService = attendanceService; + this.emailService = emailService; } + @UseBefore(UserAuthentication) @Get('/:uuid') async getAttendancesForEvent(@Params() params: UuidParam, @AuthenticatedUser() user: UserModel): Promise { @@ -25,6 +29,7 @@ export class AttendanceController { return { error: null, attendances }; } + @UseBefore(UserAuthentication) @Get() async getAttendancesForCurrentUser(@AuthenticatedUser() user: UserModel): Promise { const attendances = await this.attendanceService.getAttendancesForCurrentUser(user); @@ -41,10 +46,20 @@ export class AttendanceController { return { error: null, attendances }; } + @UseBefore(UserAuthentication) @Post() async attendEvent(@Body() body: AttendEventRequest, @AuthenticatedUser() user: UserModel): Promise { const { event } = await this.attendanceService.attendEvent(user, body.attendanceCode, body.asStaff); return { error: null, event }; } + + @Post('/expressCheckin') + async attendViaExpressCheckin(@Body() body: AttendViaExpressCheckinRequest): Promise { + body.email = body.email.toLowerCase(); + const { email, attendanceCode } = body; + const { event } = await this.attendanceService.attendViaExpressCheckin(attendanceCode, email); + await this.emailService.sendExpressCheckinConfirmation(email, event.title, event.pointValue); + return { error: null, event }; + } } diff --git a/api/validators/AttendanceControllerRequests.ts b/api/validators/AttendanceControllerRequests.ts index fdd562b3..bc90ea65 100644 --- a/api/validators/AttendanceControllerRequests.ts +++ b/api/validators/AttendanceControllerRequests.ts @@ -1,5 +1,8 @@ -import { IsDefined, IsNotEmpty, Allow } from 'class-validator'; -import { AttendEventRequest as IAttendEventRequest } from '../../types'; +import { IsDefined, IsNotEmpty, Allow, IsEmail } from 'class-validator'; +import { + AttendEventRequest as IAttendEventRequest, + AttendViaExpressCheckinRequest as IAttendViaExpressCheckinRequest, +} from '../../types'; export class AttendEventRequest implements IAttendEventRequest { @IsDefined() @@ -9,3 +12,13 @@ export class AttendEventRequest implements IAttendEventRequest { @Allow() asStaff?: boolean; } + +export class AttendViaExpressCheckinRequest implements IAttendViaExpressCheckinRequest { + @IsDefined() + @IsNotEmpty() + attendanceCode: string; + + @IsDefined() + @IsEmail() + email: string; +} diff --git a/migrations/0041-add-expressCheckins-table.ts b/migrations/0041-add-expressCheckins-table.ts new file mode 100644 index 00000000..e137f554 --- /dev/null +++ b/migrations/0041-add-expressCheckins-table.ts @@ -0,0 +1,38 @@ +import { MigrationInterface, QueryRunner, Table } from 'typeorm'; + +const TABLE_NAME = 'ExpressCheckins'; + +export class AddExpressCheckinsTable1708807643314 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.createTable(new Table({ + name: TABLE_NAME, + columns: [ + { + name: 'uuid', + type: 'uuid', + isPrimary: true, + isGenerated: true, + generationStrategy: 'uuid', + }, + { + name: 'email', + type: 'varchar(255)', + isUnique: true, + }, + { + name: 'event', + type: 'uuid', + }, + { + name: 'timestamp', + type: 'timestamptz', + default: 'CURRENT_TIMESTAMP(6)', + }, + ], + })); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropTable(TABLE_NAME); + } +} diff --git a/models/EventModel.ts b/models/EventModel.ts index ca17ec71..99f2d20e 100644 --- a/models/EventModel.ts +++ b/models/EventModel.ts @@ -2,6 +2,7 @@ import * as moment from 'moment'; import { BaseEntity, Column, Entity, Index, PrimaryGeneratedColumn, OneToMany } from 'typeorm'; import { PublicEvent, Uuid } from '../types'; import { AttendanceModel } from './AttendanceModel'; +import { ExpressCheckinModel } from './ExpressCheckinModel'; @Entity('Events') @Index('event_start_end_index', ['start', 'end']) @@ -59,6 +60,9 @@ export class EventModel extends BaseEntity { @OneToMany((type) => AttendanceModel, (attendance) => attendance.event, { cascade: true }) attendances: AttendanceModel[]; + @OneToMany((type) => ExpressCheckinModel, (expressCheckin) => expressCheckin.event, { cascade: true }) + expressCheckins: ExpressCheckinModel[]; + public getPublicEvent(canSeeAttendanceCode = false): PublicEvent { const publicEvent: PublicEvent = { uuid: this.uuid, diff --git a/models/ExpressCheckinModel.ts b/models/ExpressCheckinModel.ts new file mode 100644 index 00000000..d9e39d84 --- /dev/null +++ b/models/ExpressCheckinModel.ts @@ -0,0 +1,28 @@ +import { BaseEntity, Column, Entity, Index, JoinColumn, ManyToOne, PrimaryGeneratedColumn } from 'typeorm'; +import { PublicExpressCheckin, Uuid } from '../types'; +import { EventModel } from './EventModel'; + +@Entity('ExpressCheckins') +export class ExpressCheckinModel extends BaseEntity { + @PrimaryGeneratedColumn('uuid') + uuid: Uuid; + + @Column() + @Index({ unique: true }) + email: string; + + @ManyToOne((type) => EventModel, (event) => event.expressCheckins, { nullable: false }) + @JoinColumn({ name: 'event' }) + event: EventModel; + + @Column('timestamptz', { default: () => 'CURRENT_TIMESTAMP(6)' }) + timestamp: Date; + + public getPublicExpressCheckin(): PublicExpressCheckin { + return { + email: this.email, + event: this.event.getPublicEvent(), + timestamp: this.timestamp, + }; + } +} diff --git a/models/index.ts b/models/index.ts index a077f5dc..0838f058 100644 --- a/models/index.ts +++ b/models/index.ts @@ -13,6 +13,7 @@ import { FeedbackModel } from './FeedbackModel'; import { OrderPickupEventModel } from './OrderPickupEventModel'; import { ResumeModel } from './ResumeModel'; import { UserSocialMediaModel } from './UserSocialMediaModel'; +import { ExpressCheckinModel } from './ExpressCheckinModel'; export const models = [ UserModel, @@ -30,4 +31,5 @@ export const models = [ OrderPickupEventModel, ResumeModel, UserSocialMediaModel, + ExpressCheckinModel, ]; diff --git a/repositories/AttendanceRepository.ts b/repositories/AttendanceRepository.ts index ed6572c9..aa16270e 100644 --- a/repositories/AttendanceRepository.ts +++ b/repositories/AttendanceRepository.ts @@ -1,4 +1,5 @@ import { EntityRepository } from 'typeorm'; +import { ExpressCheckinModel } from '../models/ExpressCheckinModel'; import { Uuid } from '../types'; import { AttendanceModel } from '../models/AttendanceModel'; import { UserModel } from '../models/UserModel'; @@ -48,3 +49,14 @@ export class AttendanceRepository extends BaseRepository { return this.repository.save(attendance); } } + +@EntityRepository(ExpressCheckinModel) +export class ExpressCheckinRepository extends BaseRepository { + public async getPastExpressCheckin(email: string): Promise { + return this.repository.findOne({ where: { email }, relations: ['event'] }); + } + + public async createExpressCheckin(email: string, event: EventModel): Promise { + return this.repository.save(ExpressCheckinModel.create({ email, event })); + } +} diff --git a/repositories/UserRepository.ts b/repositories/UserRepository.ts index 17f32eba..95eb5934 100644 --- a/repositories/UserRepository.ts +++ b/repositories/UserRepository.ts @@ -41,6 +41,11 @@ export class UserRepository extends BaseRepository { }); } + public async isEmailInUse(email: string): Promise { + const user = await this.findByEmail(email); + return user !== undefined; + } + public async findByAccessCode(accessCode: string): Promise { return this.repository.findOne({ accessCode }); } diff --git a/repositories/index.ts b/repositories/index.ts index 540ab69d..e08de61e 100644 --- a/repositories/index.ts +++ b/repositories/index.ts @@ -1,7 +1,7 @@ import { EntityManager } from 'typeorm'; import { UserRepository } from './UserRepository'; import { FeedbackRepository } from './FeedbackRepository'; -import { AttendanceRepository } from './AttendanceRepository'; +import { AttendanceRepository, ExpressCheckinRepository } from './AttendanceRepository'; import { EventRepository } from './EventRepository'; import { MerchOrderRepository, OrderItemRepository, OrderPickupEventRepository } from './MerchOrderRepository'; import { @@ -80,6 +80,10 @@ export default class Repositories { public static userSocialMedia(transactionalEntityManager: EntityManager): UserSocialMediaRepository { return transactionalEntityManager.getCustomRepository(UserSocialMediaRepository); } + + public static expressCheckin(transactionalEntityManager: EntityManager): ExpressCheckinRepository { + return transactionalEntityManager.getCustomRepository(ExpressCheckinRepository); + } } export class TransactionsManager { diff --git a/services/AttendanceService.ts b/services/AttendanceService.ts index 332d1069..166ef16f 100644 --- a/services/AttendanceService.ts +++ b/services/AttendanceService.ts @@ -3,7 +3,7 @@ import { InjectManager } from 'typeorm-typedi-extensions'; import { BadRequestError, ForbiddenError, NotFoundError } from 'routing-controllers'; import { EntityManager } from 'typeorm'; import * as moment from 'moment'; -import { ActivityType, PublicAttendance, Uuid } from '../types'; +import { ActivityType, PublicAttendance, PublicExpressCheckin, Uuid } from '../types'; import { Config } from '../config'; import { UserModel } from '../models/UserModel'; import { EventModel } from '../models/EventModel'; @@ -47,13 +47,8 @@ export default class AttendanceService { public async attendEvent(user: UserModel, attendanceCode: string, asStaff = false): Promise { return this.transactions.readWrite(async (txn) => { const event = await Repositories.event(txn).findByAttendanceCode(attendanceCode); - if (!event) throw new NotFoundError('Oh no! That code didn\'t work.'); - if (event.isTooEarlyToAttendEvent()) { - throw new UserError('This event hasn\'t started yet, please wait to check in.'); - } - if (event.isTooLateToAttendEvent()) { - throw new UserError('This event has ended and is no longer accepting attendances'); - } + + this.validateEventToAttend(event); const hasAlreadyAttended = await Repositories.attendance(txn).hasUserAttendedEvent(user, event); if (hasAlreadyAttended) throw new UserError('You have already attended this event'); @@ -63,6 +58,16 @@ export default class AttendanceService { }); } + private validateEventToAttend(event: EventModel) { + if (!event) throw new NotFoundError('Oh no! That code didn\'t work.'); + if (event.isTooEarlyToAttendEvent()) { + throw new UserError('This event hasn\'t started yet, please wait to check in.'); + } + if (event.isTooLateToAttendEvent()) { + throw new UserError('This event has ended and is no longer accepting attendances'); + } + } + private async writeEventAttendance(user: UserModel, event: EventModel, asStaff: boolean, txn: EntityManager): Promise { const attendedAsStaff = asStaff && user.isStaff() && event.requiresStaff; @@ -81,6 +86,41 @@ export default class AttendanceService { }); } + public async attendViaExpressCheckin(attendanceCode: string, email: string): Promise { + return this.transactions.readWrite(async (txn) => { + const event = await Repositories.event(txn).findByAttendanceCode(attendanceCode); + + this.validateEventToAttend(event); + + const expressCheckinRepository = Repositories.expressCheckin(txn); + + const isEmailInUse = await Repositories.user(txn).isEmailInUse(email); + if (isEmailInUse) { + throw new UserError( + 'This email already has an account registered to it. ' + + 'Please login to your account to check-in to this event!', + ); + } + + const pastExpressCheckin = await expressCheckinRepository.getPastExpressCheckin(email); + if (pastExpressCheckin) { + if (pastExpressCheckin.event.uuid === event.uuid) { + throw new UserError( + 'You have already successfully checked into this event!', + ); + } else { + throw new UserError( + 'You have already done an express check-in before for a previous event. ' + + 'Please complete your account registration to attend this event!', + ); + } + } + + const expressCheckin = await expressCheckinRepository.createExpressCheckin(email, event); + return expressCheckin.getPublicExpressCheckin(); + }); + } + public async submitAttendanceForUsers(emails: string[], eventUuid: Uuid, asStaff = false, proxyUser: UserModel): Promise { return this.transactions.readWrite(async (txn) => { diff --git a/services/EmailService.ts b/services/EmailService.ts index f36848c3..ac99929c 100644 --- a/services/EmailService.ts +++ b/services/EmailService.ts @@ -34,6 +34,9 @@ export default class EmailService { private static readonly orderPartiallyFulfilledTemplate = EmailService.readTemplate('orderPartiallyFulfilled.ejs'); + private static readonly expressCheckinConfirmationTemplate = EmailService + .readTemplate('expressCheckinConfirmation.ejs'); + constructor() { this.mailer.setApiKey(Config.email.apiKey); } @@ -225,6 +228,25 @@ export default class EmailService { } } + public async sendExpressCheckinConfirmation(email: string, eventName, pointValue) { + try { + const data = { + to: email, + from: Config.email.user, + subject: 'ACM UCSD Express Checkin - Complete Your Account Registration', + html: ejs.render(EmailService.expressCheckinConfirmationTemplate, { + eventName, + pointValue, + registerLink: `${Config.client}/register`, + storeLink: `${Config.client}/store`, + }), + }; + await this.sendEmail(data); + } catch (error) { + log.warn(`Failed to send express checkin confirmation to ${email}`, { error }); + } + } + private static readTemplate(filename: string): string { return fs.readFileSync(path.join(__dirname, `../templates/${filename}`), 'utf-8'); } diff --git a/services/UserAuthService.ts b/services/UserAuthService.ts index e7e0cdec..0565fa73 100644 --- a/services/UserAuthService.ts +++ b/services/UserAuthService.ts @@ -4,6 +4,7 @@ import * as crypto from 'crypto'; import * as jwt from 'jsonwebtoken'; import { InjectManager } from 'typeorm-typedi-extensions'; import { EntityManager } from 'typeorm'; +import { ExpressCheckinModel } from 'models/ExpressCheckinModel'; import { UserRepository } from '../repositories/UserRepository'; import { Uuid, ActivityType, UserState, UserRegistration } from '../types'; import { Config } from '../config'; @@ -27,29 +28,61 @@ export default class UserAuthService { public async registerUser(registration: UserRegistration): Promise { return this.transactions.readWrite(async (txn) => { const userRepository = Repositories.user(txn); - const emailAlreadyUsed = !!(await userRepository.findByEmail(registration.email)); + + const { email } = registration; + const emailAlreadyUsed = await userRepository.isEmailInUse(email); if (emailAlreadyUsed) throw new BadRequestError('Email already in use'); + if (registration.handle) { const userHandleTaken = await userRepository.isHandleTaken(registration.handle); if (userHandleTaken) throw new BadRequestError('This handle is already in use.'); } const userHandle = registration.handle ?? UserAccountService.generateDefaultHandle(registration.firstName, registration.lastName); + const user = await userRepository.upsertUser(UserModel.create({ ...registration, hash: await UserRepository.generateHash(registration.password), accessCode: UserAuthService.generateAccessCode(), handle: userHandle, })); + const activityRepository = Repositories.activity(txn); await activityRepository.logActivity({ user, type: ActivityType.ACCOUNT_CREATE, }); + + const expressCheckin = await Repositories.expressCheckin(txn).getPastExpressCheckin(email); + if (expressCheckin) { + await this.processExpressCheckin(user, expressCheckin, txn); + } + return user; }); } + /** + * Creates the attendance and activity records and awards user points + * given the express checkin details. + */ + private async processExpressCheckin(user: UserModel, + expressCheckin: ExpressCheckinModel, + txn: EntityManager): Promise { + const { event } = expressCheckin; + + await Repositories.attendance(txn).writeAttendance({ + user, + event, + asStaff: false, + }); + await Repositories.activity(txn).logActivity({ + user, + type: ActivityType.ATTEND_EVENT, + }); + await Repositories.user(txn).addPoints(user, event.pointValue); + } + public async modifyEmail(user: UserModel, proposedEmail: string): Promise { const updatedUser = await this.transactions.readWrite(async (txn) => { const userRepository = Repositories.user(txn); diff --git a/templates/expressCheckinConfirmation.ejs b/templates/expressCheckinConfirmation.ejs new file mode 100644 index 00000000..16b1063b --- /dev/null +++ b/templates/expressCheckinConfirmation.ejs @@ -0,0 +1,155 @@ + + + + + + Complete User Registration - Express Checkin + + + + + + + + + +
  +
+ + + + + + + + + + + +
+ + + + +
+

Hello,

+

Thanks for attending your first ACM event, <%= eventName %>! You've just earned your very first <%= pointValue %> membership points. Earn more points to climb the leaderboard, make amazing friends, and create memories here with our community! You can spend the points you earned and collect at future events at <%= storeLink %> on shirts, sweaters, and more swag.

+

In order to finish becoming a full member and access these benefits, please register for an account on our Membership Portal. You will not be able to check in for future events until registering and verifying your email.

+ + + + + + +
+ + + + + + +
Complete Registration
+
+

Visit <%= registerLink %> if the link above doesn't work.

+

Regards,
ACM UCSD

+
+
+ + +
+ + + + +
+ Please do not reply to this email. +
You can contact us at acm@ucsd.edu. +
+
+ + + +
+ 		 
+ + diff --git a/tests/auth.test.ts b/tests/auth.test.ts index 79476dc6..9a9fb06c 100644 --- a/tests/auth.test.ts +++ b/tests/auth.test.ts @@ -7,11 +7,13 @@ import { Config } from '../config'; import { UserModel } from '../models/UserModel'; import EmailService from '../services/EmailService'; import UserAuthService from '../services/UserAuthService'; -import { UserAccessType, UserState } from '../types'; +import { ActivityType, UserAccessType, UserState } from '../types'; import { ControllerFactory } from './controllers'; -import { DatabaseConnection, PortalState, UserFactory } from './data'; +import { DatabaseConnection, EventFactory, PortalState, UserFactory } from './data'; import FactoryUtils from './data/FactoryUtils'; import { UserRegistrationFactory } from './data/UserRegistrationFactory'; +import { AttendanceModel } from '../models/AttendanceModel'; +import { ActivityModel } from '../models/ActivityModel'; beforeAll(async () => { await DatabaseConnection.connect(); @@ -62,6 +64,13 @@ describe('account registration', () => { isAttendancePublic: true, }); + // check that no express checkin was processed + const attendances = await conn.manager.find(AttendanceModel); + expect(attendances).toHaveLength(0); + + const attendanceActivities = await conn.manager.find(ActivityModel, { where: { type: ActivityType.ATTEND_EVENT } }); + expect(attendanceActivities).toHaveLength(0); + // check that email verification is sent verify(emailService.sendEmailVerification(user.email, user.firstName, anyString())) .called(); @@ -91,7 +100,7 @@ describe('account registration', () => { .never(); }); - test('User cannot register with a handle that is already in use', async () => { + test('user cannot register with a handle that is already in use', async () => { const conn = await DatabaseConnection.get(); const existingMember = UserFactory.fake(); @@ -115,7 +124,7 @@ describe('account registration', () => { .never(); }); - test('User registration with a full name longer than 32 characters truncates handle to exactly 32 characters', + test('user registration with a full name longer than 32 characters truncates handle to exactly 32 characters', async () => { const conn = await DatabaseConnection.get(); const existingMember = UserFactory.fake(); @@ -140,7 +149,7 @@ describe('account registration', () => { expect(registerResponse.user.handle.length).toBe(32); }); - test('User can register with an optional handle to be set', async () => { + test('user can register with an optional handle to be set', async () => { const conn = await DatabaseConnection.get(); const existingMember = UserFactory.fake(); @@ -169,6 +178,53 @@ describe('account registration', () => { verify(emailService.sendEmailVerification(user.email, user.firstName, anyString())) .called(); }); + + test('user who registers after an express checkin has their attendance properly logged', async () => { + const conn = await DatabaseConnection.get(); + // email can have uppercases in it, so we should test that lowercase emails + // are used by SendGrid / written to the database when an uppercase email is sent + // in the request + const newUserEmail = faker.internet.email(); + const lowercasedEmail = newUserEmail.toLowerCase(); + const pastEvent = EventFactory.fake(EventFactory.daysBefore(5)); + + await new PortalState() + .createEvents(pastEvent) + .createExpressCheckin(lowercasedEmail, pastEvent) + .write(); + + const userToRegister = UserRegistrationFactory.fake({ + email: newUserEmail, + }); + + // register user + const emailService = mock(EmailService); + when(emailService.sendEmailVerification(lowercasedEmail, userToRegister.firstName, anyString())) + .thenResolve(); + const authController = ControllerFactory.auth(conn, instance(emailService)); + const registerRequest = { user: userToRegister }; + const registerResponse = await authController.register(registerRequest, FactoryUtils.randomHexString()); + const { user } = registerResponse; + + // check that points for event were logged and that attendance and activity objects were persisted + expect(user.points).toEqual(pastEvent.pointValue); + + const attendances = await conn.manager.find(AttendanceModel, { relations: ['event', 'user'] }); + expect(attendances).toHaveLength(1); + expect(attendances[0].event.uuid).toEqual(pastEvent.uuid); + expect(attendances[0].user.uuid).toEqual(user.uuid); + + const attendanceActivities = await conn.manager.find( + ActivityModel, + { where: { type: ActivityType.ATTEND_EVENT }, relations: ['user'] }, + ); + expect(attendanceActivities).toHaveLength(1); + expect(attendanceActivities[0].user.uuid).toEqual(user.uuid); + + // verify email was sent + verify(emailService.sendEmailVerification(lowercasedEmail, user.firstName, anyString())) + .called(); + }); }); describe('account login', () => { diff --git a/tests/controllers/ControllerFactory.ts b/tests/controllers/ControllerFactory.ts index 41b42b49..8d993284 100644 --- a/tests/controllers/ControllerFactory.ts +++ b/tests/controllers/ControllerFactory.ts @@ -43,9 +43,9 @@ export class ControllerFactory { return new AdminController(storageService, userAccountService, attendanceService); } - public static attendance(conn: Connection): AttendanceController { + public static attendance(conn: Connection, emailService = new EmailService()): AttendanceController { const attendanceService = new AttendanceService(conn.manager); - return new AttendanceController(attendanceService); + return new AttendanceController(attendanceService, emailService); } public static auth(conn: Connection, emailService: EmailService): AuthController { diff --git a/tests/data/DatabaseConnection.ts b/tests/data/DatabaseConnection.ts index bbe166bb..d8b7c6e3 100644 --- a/tests/data/DatabaseConnection.ts +++ b/tests/data/DatabaseConnection.ts @@ -44,6 +44,7 @@ export class DatabaseConnection { 'Feedback', 'Resumes', 'UserSocialMedia', + 'ExpressCheckins', 'MerchCollectionPhotos', ]; await Promise.all(tableNames.map((t) => txn.query(`DELETE FROM "${t}"`))); diff --git a/tests/data/PortalState.ts b/tests/data/PortalState.ts index bb2a5f6f..1bd1426f 100644 --- a/tests/data/PortalState.ts +++ b/tests/data/PortalState.ts @@ -17,6 +17,7 @@ import { UserSocialMediaModel } from '../../models/UserSocialMediaModel'; import { DatabaseConnection } from './DatabaseConnection'; import { MerchFactory } from '.'; import { ResumeModel } from '../../models/ResumeModel'; +import { ExpressCheckinModel } from '../../models/ExpressCheckinModel'; export class PortalState { users: UserModel[] = []; @@ -39,6 +40,8 @@ export class PortalState { userSocialMedia: UserSocialMediaModel[] = []; + expressCheckins: ExpressCheckinModel[] = []; + public from(state: PortalState): PortalState { // deep clones all around for immutable PortalStates this.users = rfdc()(state.users); @@ -51,6 +54,7 @@ export class PortalState { this.feedback = rfdc()(state.feedback); this.resumes = rfdc()(state.resumes); this.userSocialMedia = rfdc()(state.userSocialMedia); + this.expressCheckins = rfdc()(state.expressCheckins); return this; } @@ -67,6 +71,7 @@ export class PortalState { this.feedback = await txn.save(this.feedback); this.resumes = await txn.save(this.resumes); this.userSocialMedia = await txn.save(this.userSocialMedia); + this.expressCheckins = await txn.save(this.expressCheckins); }); } @@ -231,6 +236,15 @@ export class PortalState { } return this; } + + public createExpressCheckin(email: string, event: EventModel): PortalState { + this.expressCheckins.push(ExpressCheckinModel.create({ + email: email.toLowerCase(), + event, + })); + + return this; + } } export interface MerchItemOptionAndQuantity { diff --git a/tests/expressCheckin.test.ts b/tests/expressCheckin.test.ts new file mode 100644 index 00000000..5b33d18d --- /dev/null +++ b/tests/expressCheckin.test.ts @@ -0,0 +1,130 @@ +import * as faker from 'faker'; +import { } from '../types'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { ControllerFactory } from './controllers'; +import { DatabaseConnection, EventFactory, PortalState, UserFactory } from './data'; +import EmailService from '../services/EmailService'; +import { ExpressCheckinModel } from '../models/ExpressCheckinModel'; + +beforeAll(async () => { + await DatabaseConnection.connect(); +}); + +beforeEach(async () => { + await DatabaseConnection.clear(); +}); + +afterAll(async () => { + await DatabaseConnection.clear(); + await DatabaseConnection.close(); +}); + +describe('express check-in', () => { + test('succeeds for new accounts that have never used express check-in before', async () => { + const conn = await DatabaseConnection.get(); + const event = EventFactory.fake(EventFactory.ongoing()); + await new PortalState().createEvents(event).write(); + + // email can have uppercases in it, so we should test that lowercase emails + // are used by SendGrid / written to the database + const newUserEmail = faker.internet.email(); + const lowercasedEmail = newUserEmail.toLowerCase(); + + const emailService = mock(EmailService); + when(emailService.sendExpressCheckinConfirmation(lowercasedEmail, event.title, event.pointValue)).thenResolve(); + const attendanceController = ControllerFactory.attendance(conn, instance(emailService)); + + const request = { + email: newUserEmail, + attendanceCode: event.attendanceCode, + }; + const response = await attendanceController.attendViaExpressCheckin(request); + + expect(response.error).toBeNull(); + expect(response.event).toStrictEqual(event.getPublicEvent()); + + const expressCheckins = await conn.manager.find(ExpressCheckinModel, { relations: ['event'] }); + expect(expressCheckins).toHaveLength(1); + expect(expressCheckins[0].email).toEqual(newUserEmail.toLowerCase()); + expect(expressCheckins[0].event).toEqual(event); + + verify(emailService.sendExpressCheckinConfirmation(lowercasedEmail, event.title, event.pointValue)).called(); + }); + + test('throws proper error when user already has a registered account', async () => { + const conn = await DatabaseConnection.get(); + const registeredMember = UserFactory.fake(); + const event = EventFactory.fake(EventFactory.ongoing()); + await new PortalState() + .createUsers(registeredMember) + .createEvents(event) + .write(); + + const newUserEmail = registeredMember.email; + + const emailService = mock(EmailService); + const attendanceController = ControllerFactory.attendance(conn, instance(emailService)); + + const request = { + email: newUserEmail, + attendanceCode: event.attendanceCode, + }; + + await (expect(attendanceController.attendViaExpressCheckin(request)) + .rejects + .toThrow('This email already has an account registered to it. ' + + 'Please login to your account to check-in to this event!')); + + verify(emailService.sendExpressCheckinConfirmation(anything(), anything(), anything())).never(); + }); + + test('throws proper error when user already used express checkin for the same event', async () => { + const conn = await DatabaseConnection.get(); + const newUserEmail = faker.internet.email(); + const event = EventFactory.fake(EventFactory.ongoing()); + await new PortalState() + .createEvents(event) + .createExpressCheckin(newUserEmail, event) + .write(); + + const emailService = mock(EmailService); + const attendanceController = ControllerFactory.attendance(conn, instance(emailService)); + + const request = { + email: newUserEmail, + attendanceCode: event.attendanceCode, + }; + + await (expect(attendanceController.attendViaExpressCheckin(request)) + .rejects + .toThrow('You have already successfully checked into this event!')); + + verify(emailService.sendExpressCheckinConfirmation(anything(), anything(), anything())).never(); + }); + + test('throws proper error when user already used express checkin for any previous event', async () => { + const conn = await DatabaseConnection.get(); + const newUserEmail = faker.internet.email(); + const previousEvent = EventFactory.fake(EventFactory.daysBefore(5)); + const ongoingEvent = EventFactory.fake(EventFactory.ongoing()); + await new PortalState() + .createEvents(previousEvent, ongoingEvent) + .createExpressCheckin(newUserEmail, previousEvent) + .write(); + + const emailService = mock(EmailService); + const attendanceController = ControllerFactory.attendance(conn, instance(emailService)); + + const request = { + email: newUserEmail, + attendanceCode: ongoingEvent.attendanceCode, + }; + + await (expect(attendanceController.attendViaExpressCheckin(request)) + .rejects + .toThrow('You have already done an express check-in before for a previous event. ' + + 'Please complete your account registration to attend this event!')); + + verify(emailService.sendExpressCheckinConfirmation(anything(), anything(), anything())).never(); + }); +}); diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index 0a733272..44447fb9 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -172,6 +172,11 @@ export interface AttendEventRequest { asStaff?: boolean; } +export interface AttendViaExpressCheckinRequest { + attendanceCode: string; + email: string; +} + export interface SubmitEventFeedbackRequest { feedback: string[]; } diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index 1884cdd4..f449b2d5 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -69,6 +69,12 @@ export interface AttendEventResponse extends ApiResponse { event: PublicEvent; } +export interface PublicExpressCheckin { + email: string; + event: PublicEvent; + timestamp: Date; +} + // AUTH export interface RegistrationResponse extends ApiResponse { diff --git a/types/internal/index.ts b/types/internal/index.ts index 2ecaea25..a068e013 100644 --- a/types/internal/index.ts +++ b/types/internal/index.ts @@ -1,4 +1,4 @@ -import { MerchandiseItemModel } from 'models/MerchandiseItemModel'; +import { MerchandiseItemModel } from '../../models/MerchandiseItemModel'; import { EventModel } from '../../models/EventModel'; import { UserModel } from '../../models/UserModel'; import { ActivityScope, ActivityType } from '../Enums'; From 5cf2ad5b868f7f39a13d6303c6e671d13c236fbf Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Mon, 26 Feb 2024 20:49:15 -0800 Subject: [PATCH 18/30] added auth decorator to GET attendance/user/uuid (#409) --- api/controllers/AttendanceController.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/api/controllers/AttendanceController.ts b/api/controllers/AttendanceController.ts index e47bd121..ffc3c6c6 100644 --- a/api/controllers/AttendanceController.ts +++ b/api/controllers/AttendanceController.ts @@ -36,6 +36,7 @@ export class AttendanceController { return { error: null, attendances }; } + @UseBefore(UserAuthentication) @Get('/user/:uuid') async getAttendancesForUser(@Params() params: UuidParam, @AuthenticatedUser() currentUser: UserModel): Promise { From fa7532439e62b6e3fce9d911b19fa94b98963e50 Mon Sep 17 00:00:00 2001 From: Max Weng <73797155+maxwn04@users.noreply.github.com> Date: Wed, 28 Feb 2024 16:34:54 -0800 Subject: [PATCH 19/30] Bad words filter (#395) * basic bad words filter * bad words filter * lint * change error * remove unused package * bump version number --- package.json | 3 ++- services/UserAccountService.ts | 14 ++++++++++++++ yarn.lock | 5 +++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 83243d05..5c84392f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.2.0", + "version": "3.2.1", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ @@ -55,6 +55,7 @@ "moment-timezone": "^0.5.34", "morgan": "^1.10.0", "multer": "^1.4.2", + "obscenity": "^0.2.0", "pg": "^8.6.0", "reflect-metadata": "^0.1.13", "routing-controllers": "^0.9.0", diff --git a/services/UserAccountService.ts b/services/UserAccountService.ts index e60bf78b..d59e2d96 100644 --- a/services/UserAccountService.ts +++ b/services/UserAccountService.ts @@ -5,6 +5,11 @@ import { EntityManager } from 'typeorm'; import * as moment from 'moment'; import * as faker from 'faker'; import { UserAccessUpdates } from 'api/validators/AdminControllerRequests'; +import { + RegExpMatcher, + englishDataset, + englishRecommendedTransformers, +} from 'obscenity'; import Repositories, { TransactionsManager } from '../repositories'; import { Uuid, @@ -23,8 +28,14 @@ import { UserModel } from '../models/UserModel'; export default class UserAccountService { private transactions: TransactionsManager; + private matcher: RegExpMatcher; + constructor(@InjectManager() entityManager: EntityManager) { this.transactions = new TransactionsManager(entityManager); + this.matcher = new RegExpMatcher({ + ...englishDataset.build(), + ...englishRecommendedTransformers, + }); } public async findByUuid(uuid: Uuid): Promise { @@ -104,6 +115,9 @@ export default class UserAccountService { } changes.hash = await UserRepository.generateHash(newPassword); } + if (this.matcher.hasMatch(userPatches.handle)) { + throw new ForbiddenError('Please remove profanity from handle.'); + } return this.transactions.readWrite(async (txn) => { if (userPatches.handle) { const userRepository = Repositories.user(txn); diff --git a/yarn.lock b/yarn.lock index 400edfbf..f53205a5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4342,6 +4342,11 @@ object.values@^1.1.5: define-properties "^1.1.3" es-abstract "^1.19.1" +obscenity@^0.2.0: + version "0.2.0" + resolved "https://registry.yarnpkg.com/obscenity/-/obscenity-0.2.0.tgz#5a234c7af0aaebcd58fc762b29d2852104350fa2" + integrity sha512-eYe8r9hqJk5dEMZkLtWlGlwGxKYO6xA/yEOzd8MGSG3vzn8hAgo6MUZ9dirA3kPAfy/1d1jEOkCUpIU1nGI1EQ== + on-finished@^2.3.0: version "2.4.1" resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.4.1.tgz#58c8c44116e54845ad57f14ab10b03533184ac3f" From 0a6da54aadd4308e2486ed4901236d3b4952866c Mon Sep 17 00:00:00 2001 From: "Caogang (Marcelo) Shen" Date: Wed, 28 Feb 2024 16:49:52 -0800 Subject: [PATCH 20/30] Added the merch photo table name to the list of tables for testing (#406) * added the thing * bump version number for express checkin --- package.json | 2 +- tests/data/DatabaseConnection.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 5c84392f..a92c3ce5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.2.1", + "version": "3.3.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/tests/data/DatabaseConnection.ts b/tests/data/DatabaseConnection.ts index d8b7c6e3..857f32f3 100644 --- a/tests/data/DatabaseConnection.ts +++ b/tests/data/DatabaseConnection.ts @@ -36,6 +36,7 @@ export class DatabaseConnection { 'Orders', 'OrderPickupEvents', 'MerchandiseItemOptions', + 'MerchandiseItemPhotos', 'MerchandiseItems', 'MerchandiseCollections', 'Attendances', From 60cba34680fede5fc94864c6ceb56a881220d4ce Mon Sep 17 00:00:00 2001 From: "Caogang (Marcelo) Shen" Date: Wed, 28 Feb 2024 17:00:00 -0800 Subject: [PATCH 21/30] Bug Fix for Transaction Read/Write Error (#401) * added packages * sample implementation test * lint fix * testing base functionalities * lint fix * more testing * omfg i got it * clean up testing related variables and functions * lint fix * added comments * added concurrent tests * bumped version number --- package.json | 4 +- repositories/index.ts | 31 +++++- tests/Seeds.ts | 5 - tests/data/EventFactory.ts | 4 +- tests/data/FactoryUtils.ts | 4 + tests/data/MerchFactory.ts | 2 +- tests/userSocialMedia.test.ts | 189 +++++++++++++++++++++++++++++++++- yarn.lock | 24 +++++ 8 files changed, 248 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index a92c3ce5..9d4a142a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.3.0", + "version": "3.4.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ @@ -39,6 +39,7 @@ "dependencies": { "@sendgrid/mail": "^7.4.4", "amazon-s3-uri": "^0.1.1", + "async-retry": "^1.3.3", "aws-sdk": "^2.906.0", "bcrypt": "^5.0.0", "body-parser": "^1.19.0", @@ -71,6 +72,7 @@ "winston-daily-rotate-file": "^4.5.0" }, "devDependencies": { + "@types/async-retry": "^1.4.8", "@types/bcrypt": "^3.0.0", "@types/body-parser": "^1.19.0", "@types/datadog-metrics": "^0.6.1", diff --git a/repositories/index.ts b/repositories/index.ts index e08de61e..9ede2b5c 100644 --- a/repositories/index.ts +++ b/repositories/index.ts @@ -1,4 +1,5 @@ import { EntityManager } from 'typeorm'; +import AsyncRetry = require('async-retry'); import { UserRepository } from './UserRepository'; import { FeedbackRepository } from './FeedbackRepository'; import { AttendanceRepository, ExpressCheckinRepository } from './AttendanceRepository'; @@ -16,6 +17,9 @@ import { LeaderboardRepository } from './LeaderboardRepository'; import { ResumeRepository } from './ResumeRepository'; import { UserSocialMediaRepository } from './UserSocialMediaRepository'; +const HINT_FOR_RETRIABLE_TRANSACTIONS : string = 'The transaction might succeed if retried.'; +const AMOUNT_OF_RETRIES : number = 10; + export default class Repositories { public static activity(transactionalEntityManager: EntityManager): ActivityRepository { return transactionalEntityManager.getCustomRepository(ActivityRepository); @@ -93,11 +97,34 @@ export class TransactionsManager { this.transactionalEntityManager = transactionalEntityManager; } + // used async-retry library to handle automatic retries under transaction failure due to concurrent transactions public readOnly(fn: (transactionalEntityManager: EntityManager) => Promise): Promise { - return this.transactionalEntityManager.transaction('REPEATABLE READ', fn); + return AsyncRetry(async (bail, attemptNum) => { + try { + const res = await this.transactionalEntityManager.transaction('REPEATABLE READ', fn); + return res; + } catch (e) { + if (e.hint !== HINT_FOR_RETRIABLE_TRANSACTIONS) bail(e); + else throw e; + } + }, + { + retries: AMOUNT_OF_RETRIES, + }); } public readWrite(fn: (transactionalEntityManager: EntityManager) => Promise): Promise { - return this.transactionalEntityManager.transaction('SERIALIZABLE', fn); + return AsyncRetry(async (bail, attemptNum) => { + try { + const res = await this.transactionalEntityManager.transaction('SERIALIZABLE', fn); + return res; + } catch (e) { + if (e.hint !== HINT_FOR_RETRIABLE_TRANSACTIONS) bail(e); + else throw e; + } + }, + { + retries: AMOUNT_OF_RETRIES, + }); } } diff --git a/tests/Seeds.ts b/tests/Seeds.ts index 7954f197..a3626228 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -479,7 +479,6 @@ async function seed(): Promise { }); const MERCH_ITEM_1_PHOTO_1 = MerchFactory.fakePhoto({ merchItem: MERCH_ITEM_1, - uploadedPhoto: 'https://www.fakepicture.com/', position: 1, }); const MERCH_ITEM_1_PHOTO_2 = MerchFactory.fakePhoto({ @@ -542,7 +541,6 @@ async function seed(): Promise { ]; const MERCH_ITEM_2_PHOTO_0 = MerchFactory.fakePhoto({ merchItem: MERCH_ITEM_2, - uploadedPhoto: 'https://www.fakepicture.com/', position: 0, }); const MERCH_ITEM_2_PHOTO_1 = MerchFactory.fakePhoto({ @@ -591,7 +589,6 @@ async function seed(): Promise { }); const MERCH_ITEM_3_PHOTO_1 = MerchFactory.fakePhoto({ merchItem: MERCH_ITEM_3, - uploadedPhoto: 'https://www.fakepicture.com/', position: 1, }); const MERCH_ITEM_3_PHOTO_2 = MerchFactory.fakePhoto({ @@ -601,12 +598,10 @@ async function seed(): Promise { }); const MERCH_ITEM_3_PHOTO_3 = MerchFactory.fakePhoto({ merchItem: MERCH_ITEM_3, - uploadedPhoto: 'https://www.fakepicture.com/', position: 3, }); const MERCH_ITEM_3_PHOTO_4 = MerchFactory.fakePhoto({ merchItem: MERCH_ITEM_3, - uploadedPhoto: 'https://www.fakepicture.com/', position: 4, }); MERCH_ITEM_3.merchPhotos = [ diff --git a/tests/data/EventFactory.ts b/tests/data/EventFactory.ts index c6dccd2e..d07f523b 100644 --- a/tests/data/EventFactory.ts +++ b/tests/data/EventFactory.ts @@ -37,10 +37,10 @@ export class EventFactory { requiresStaff: FactoryUtils.getRandomBoolean(), staffPointBonus: EventFactory.randomPointValue(), committee: 'ACM', - cover: faker.internet.url(), + cover: FactoryUtils.getRandomImageUrl(), deleted: false, eventLink: faker.internet.url(), - thumbnail: faker.internet.url(), + thumbnail: FactoryUtils.getRandomImageUrl(), }); return EventModel.merge(fake, substitute); } diff --git a/tests/data/FactoryUtils.ts b/tests/data/FactoryUtils.ts index 9bd9f9cd..567990cf 100644 --- a/tests/data/FactoryUtils.ts +++ b/tests/data/FactoryUtils.ts @@ -40,4 +40,8 @@ export default class FactoryUtils { public static randomHexString(): string { return faker.datatype.hexaDecimal(10); } + + public static getRandomImageUrl(): string { + return `http://i.imgur.com/${faker.random.alphaNumeric(10)}.jpeg`; + } } diff --git a/tests/data/MerchFactory.ts b/tests/data/MerchFactory.ts index b2834932..5885fde0 100644 --- a/tests/data/MerchFactory.ts +++ b/tests/data/MerchFactory.ts @@ -72,7 +72,7 @@ export class MerchFactory { const fake = MerchandiseItemPhotoModel.create({ uuid: uuid(), position: 0, - uploadedPhoto: 'https://www.fakepicture.com/', + uploadedPhoto: FactoryUtils.getRandomImageUrl(), uploadedAt: faker.date.recent(), }); return MerchandiseItemPhotoModel.merge(fake, substitute); diff --git a/tests/userSocialMedia.test.ts b/tests/userSocialMedia.test.ts index 98938edc..f2b39778 100644 --- a/tests/userSocialMedia.test.ts +++ b/tests/userSocialMedia.test.ts @@ -1,24 +1,26 @@ import faker = require('faker'); +import { Connection } from 'typeorm'; import { UserModel } from '../models/UserModel'; import { SocialMediaType } from '../types'; import { ControllerFactory } from './controllers'; import { DatabaseConnection, PortalState, UserFactory } from './data'; import { UserSocialMediaFactory } from './data/UserSocialMediaFactory'; +import { UserController } from '../api/controllers/UserController'; beforeAll(async () => { await DatabaseConnection.connect(); }); -beforeEach(async () => { - await DatabaseConnection.clear(); -}); - afterAll(async () => { await DatabaseConnection.clear(); await DatabaseConnection.close(); }); describe('social media URL submission', () => { + beforeEach(async () => { + await DatabaseConnection.clear(); + }); + test('properly persists on successful submission', async () => { const conn = await DatabaseConnection.get(); let member = UserFactory.fake(); @@ -60,6 +62,10 @@ describe('social media URL submission', () => { }); describe('social media URL update', () => { + beforeEach(async () => { + await DatabaseConnection.clear(); + }); + test('is invalidated when updating social media URL of another user', async () => { const conn = await DatabaseConnection.get(); let member = UserFactory.fake(); @@ -100,6 +106,10 @@ describe('social media URL update', () => { }); describe('social media URL delete', () => { + beforeEach(async () => { + await DatabaseConnection.clear(); + }); + test('is invalidated when deleting social media URL of another user', async () => { const conn = await DatabaseConnection.get(); let member = UserFactory.fake(); @@ -119,3 +129,174 @@ describe('social media URL delete', () => { await expect(userController.deleteSocialMediaForUser(uuidParams, unauthorizedMember)).rejects.toThrow(errorMessage); }); }); + +describe('social media URL update', () => { + let conn : Connection; + let member : UserModel; + let userController : UserController; + + let flag : boolean = false; + function waitForFlag(interval = 500, timeout = 5000) { + let acc = 0; // time accumulation + return new Promise((resolve, reject) => { + const i = setInterval(() => { + acc += interval; + if (flag) { + clearInterval(i); + resolve(flag); + } + if (acc > timeout) { + clearInterval(i); + reject(); + } + }, interval); + }); + } + + // beforeAll does not behave properly when concurrent is being used: + // https://stackoverflow.com/questions/42633635/how-to-run-concurrent-tests-in-jest-with-multiple-tests-per-request + beforeAll(async () => { + await DatabaseConnection.clear(); + conn = await DatabaseConnection.get(); + member = UserFactory.fake(); + + const userSocialMedia0 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.FACEBOOK }); + const userSocialMedia1 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.GITHUB }); + const userSocialMedia2 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.DEVPOST }); + const userSocialMedia3 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.EMAIL }); + const userSocialMedia4 = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.INSTAGRAM }); + + await new PortalState() + .createUsers(member) + .createUserSocialMedia( + member, userSocialMedia0, userSocialMedia1, userSocialMedia2, userSocialMedia3, userSocialMedia4, + ).write(); + + userController = ControllerFactory.user(conn); + + // refreshes member to have the userSocialMedia field + member = await conn.manager.findOne(UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }); + + flag = true; + }); + + test.concurrent('concurrent updates properly persist on successful submission 0', async () => { + await waitForFlag(); + const index = 0; + const uuidParams = { uuid: member.userSocialMedia[index].uuid }; + const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; + await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + + const expectedUserSocialMedia0 = { + url: socialMediaParams.socialMedia.url, + type: member.userSocialMedia[index].type, + uuid: uuidParams.uuid, + }; + const updatedMember = await conn.manager.findOne( + UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, + ); + + expect(updatedMember.userSocialMedia).toHaveLength(5); + const targetUserSocialMedia = updatedMember.userSocialMedia.find( + (socialMedia) => socialMedia.uuid === expectedUserSocialMedia0.uuid, + ); + expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia0.url); + expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia0.type); + }); + + test.concurrent('concurrent updates properly persist on successful submission 1', async () => { + await waitForFlag(); + const index = 1; + const uuidParams = { uuid: member.userSocialMedia[index].uuid }; + const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; + await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + + const expectedUserSocialMedia1 = { + url: socialMediaParams.socialMedia.url, + type: member.userSocialMedia[index].type, + uuid: uuidParams.uuid, + }; + const updatedMember = await conn.manager.findOne( + UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, + ); + + expect(updatedMember.userSocialMedia).toHaveLength(5); + const targetUserSocialMedia = updatedMember.userSocialMedia.find( + (socialMedia) => socialMedia.uuid === expectedUserSocialMedia1.uuid, + ); + expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia1.url); + expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia1.type); + }); + + test.concurrent('concurrent updates properly persist on successful submission 2', async () => { + await waitForFlag(); + const index = 2; + const uuidParams = { uuid: member.userSocialMedia[index].uuid }; + const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; + await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + + const expectedUserSocialMedia2 = { + url: socialMediaParams.socialMedia.url, + type: member.userSocialMedia[index].type, + uuid: uuidParams.uuid, + }; + const updatedMember = await conn.manager.findOne( + UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, + ); + + expect(updatedMember.userSocialMedia).toHaveLength(5); + const targetUserSocialMedia = updatedMember.userSocialMedia.find( + (socialMedia) => socialMedia.uuid === expectedUserSocialMedia2.uuid, + ); + expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia2.url); + expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia2.type); + }); + + test.concurrent('concurrent updates properly persist on successful submission 3', async () => { + await waitForFlag(); + const index = 3; + const uuidParams = { uuid: member.userSocialMedia[index].uuid }; + const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; + await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + + const expectedUserSocialMedia3 = { + url: socialMediaParams.socialMedia.url, + type: member.userSocialMedia[index].type, + uuid: uuidParams.uuid, + }; + const updatedMember = await conn.manager.findOne( + UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, + ); + + expect(updatedMember.userSocialMedia).toHaveLength(5); + const targetUserSocialMedia = updatedMember.userSocialMedia.find( + (socialMedia) => socialMedia.uuid === expectedUserSocialMedia3.uuid, + ); + expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia3.url); + expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia3.type); + }); + + test.concurrent('concurrent updates properly persist on successful submission 4', async () => { + await waitForFlag(); + const index = 4; + const uuidParams = { uuid: member.userSocialMedia[index].uuid }; + const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; + await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + + const expectedUserSocialMedia4 = { + url: socialMediaParams.socialMedia.url, + type: member.userSocialMedia[index].type, + uuid: uuidParams.uuid, + }; + const updatedMember = await conn.manager.findOne( + UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, + ); + + expect(updatedMember.userSocialMedia).toHaveLength(5); + const targetUserSocialMedia = updatedMember.userSocialMedia.find( + (socialMedia) => socialMedia.uuid === expectedUserSocialMedia4.uuid, + ); + expect(targetUserSocialMedia.url).toEqual(expectedUserSocialMedia4.url); + expect(targetUserSocialMedia.type).toEqual(expectedUserSocialMedia4.type); + }); +}); diff --git a/yarn.lock b/yarn.lock index f53205a5..c8425572 100644 --- a/yarn.lock +++ b/yarn.lock @@ -641,6 +641,13 @@ resolved "https://registry.yarnpkg.com/@tootallnate/once/-/once-1.1.2.tgz#ccb91445360179a04e7fe6aff78c00ffc1eeaf82" integrity sha512-RbzJvlNzmRq5c3O09UipeuXno4tA1FE6ikOjxZK0tuxVv3412l64l5t1W5pj4+rJq9vpkm/kwiR07aZXnsKPxw== +"@types/async-retry@^1.4.8": + version "1.4.8" + resolved "https://registry.yarnpkg.com/@types/async-retry/-/async-retry-1.4.8.tgz#eb32df13aceb9ba1a8a80e7fe518ff4e3fe46bb3" + integrity sha512-Qup/B5PWLe86yI5I3av6ePGaeQrIHNKCwbsQotD6aHQ6YkHsMUxVZkZsmx/Ry3VZQ6uysHwTjQ7666+k6UjVJA== + dependencies: + "@types/retry" "*" + "@types/babel__core@^7.0.0", "@types/babel__core@^7.1.14": version "7.1.18" resolved "https://registry.yarnpkg.com/@types/babel__core/-/babel__core-7.1.18.tgz#1a29abcc411a9c05e2094c98f9a1b7da6cdf49f8" @@ -823,6 +830,11 @@ resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.4.tgz#cd667bcfdd025213aafb7ca5915a932590acdcdc" integrity sha512-EEhsLsD6UsDM1yFhAvy0Cjr6VwmpMWqFBCb9w07wVugF7w9nfajxLuVmngTIpgS6svCnm6Vaw+MZhoDCKnOfsw== +"@types/retry@*": + version "0.12.5" + resolved "https://registry.yarnpkg.com/@types/retry/-/retry-0.12.5.tgz#f090ff4bd8d2e5b940ff270ab39fd5ca1834a07e" + integrity sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw== + "@types/rfdc@^1.1.0": version "1.2.0" resolved "https://registry.yarnpkg.com/@types/rfdc/-/rfdc-1.2.0.tgz#ff0aedd2d33589442f3a1285d37bdbb54d18dfbc" @@ -1169,6 +1181,13 @@ astral-regex@^2.0.0: resolved "https://registry.yarnpkg.com/astral-regex/-/astral-regex-2.0.0.tgz#483143c567aeed4785759c0865786dc77d7d2e31" integrity sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ== +async-retry@^1.3.3: + version "1.3.3" + resolved "https://registry.yarnpkg.com/async-retry/-/async-retry-1.3.3.tgz#0e7f36c04d8478e7a58bdbed80cedf977785f280" + integrity sha512-wfr/jstw9xNi/0teMHrRW7dsz3Lt5ARhYNZ2ewpadnhaIp5mbALhOAP+EAdsC7t4Z6wqsDVv9+W6gm1Dk9mEyw== + dependencies: + retry "0.13.1" + async@0.9.x: version "0.9.2" resolved "https://registry.yarnpkg.com/async/-/async-0.9.2.tgz#aea74d5e61c1f899613bf64bda66d4c78f2fd17d" @@ -4918,6 +4937,11 @@ responselike@^1.0.2: dependencies: lowercase-keys "^1.0.0" +retry@0.13.1: + version "0.13.1" + resolved "https://registry.yarnpkg.com/retry/-/retry-0.13.1.tgz#185b1587acf67919d63b357349e03537b2484658" + integrity sha512-XQBQ3I8W1Cge0Seh+6gjj03LbmRFWuoszgK9ooCpwYIrhhoO80pfq4cUkU5DkknwfOfFteRwlZ56PYOGYyFWdg== + reusify@^1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.0.4.tgz#90da382b1e126efc02146e90845a88db12925d76" 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 22/30] 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 9d4a142a..42b144b6 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 bdcdc352..9994e080 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 03faaddc..8c697a58 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 e6d38a24..ef1d64dd 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); + }); +}); From db9d7dbc65768d9dcc58ccd64d2856a24de24420 Mon Sep 17 00:00:00 2001 From: Max Weng <73797155+maxwn04@users.noreply.github.com> Date: Wed, 6 Mar 2024 17:28:21 -0800 Subject: [PATCH 23/30] add lowercase emails to when in registration test (#408) * change email to lowercase in whens * update version number --- package.json | 2 +- tests/auth.test.ts | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 42b144b6..ffa2d67f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.4.1", + "version": "3.4.2", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/tests/auth.test.ts b/tests/auth.test.ts index 9a9fb06c..cb935ab5 100644 --- a/tests/auth.test.ts +++ b/tests/auth.test.ts @@ -41,7 +41,7 @@ describe('account registration', () => { // register member const emailService = mock(EmailService); - when(emailService.sendEmailVerification(user.email, user.firstName, anyString())) + when(emailService.sendEmailVerification(user.email.toLowerCase(), user.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); const registerRequest = { user }; @@ -89,7 +89,7 @@ describe('account registration', () => { }); const emailService = mock(EmailService); - when(emailService.sendEmailVerification(user.email, user.firstName, anyString())) + when(emailService.sendEmailVerification(user.email.toLowerCase(), user.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); const registerRequest = { user }; @@ -113,7 +113,7 @@ describe('account registration', () => { }); const emailService = mock(EmailService); - when(emailService.sendEmailVerification(user.email, user.firstName, anyString())) + when(emailService.sendEmailVerification(user.email.toLowerCase(), user.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); const registerRequest = { user }; @@ -140,7 +140,7 @@ describe('account registration', () => { // register member const emailService = mock(EmailService); - when(emailService.sendEmailVerification(user.email, user.firstName, anyString())) + when(emailService.sendEmailVerification(user.email.toLowerCase(), user.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); const registerRequest = { user }; @@ -163,7 +163,7 @@ describe('account registration', () => { // register member const emailService = mock(EmailService); - when(emailService.sendEmailVerification(user.email, user.firstName, anyString())) + when(emailService.sendEmailVerification(user.email.toLowerCase(), user.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); const registerRequest = { user }; @@ -283,7 +283,7 @@ describe('verifying email', () => { .write(); const emailService = mock(EmailService); - when(emailService.sendEmailVerification(member.email, member.firstName, anyString())) + when(emailService.sendEmailVerification(member.email.toLowerCase(), member.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); await authController.verifyEmail({ accessCode }); @@ -305,7 +305,7 @@ describe('verifying email', () => { .write(); const emailService = mock(EmailService); - when(emailService.sendEmailVerification(member.email, member.firstName, anyString())) + when(emailService.sendEmailVerification(member.email.toLowerCase(), member.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); await expect(authController.verifyEmail({ accessCode: FactoryUtils.randomHexString() })) @@ -326,7 +326,7 @@ describe('resending email verification', () => { .write(); const emailService = mock(EmailService); - when(emailService.sendEmailVerification(member.email, member.firstName, anyString())) + when(emailService.sendEmailVerification(member.email.toLowerCase(), member.firstName, anyString())) .thenResolve(); const authController = ControllerFactory.auth(conn, instance(emailService)); const params = { email: member.email }; @@ -478,7 +478,7 @@ describe('resending password reset email', () => { .write(); const emailService = mock(EmailService); - when(emailService.sendPasswordReset(member.email, member.firstName, anyString())); + when(emailService.sendPasswordReset(member.email.toLowerCase(), member.firstName, anyString())); const authController = ControllerFactory.auth(conn, instance(emailService)); const params = { email: member.email }; await authController.sendPasswordResetEmail(params, FactoryUtils.randomHexString()); @@ -495,7 +495,7 @@ describe('resending password reset email', () => { const member = UserFactory.fake(); const emailService = mock(EmailService); - when(emailService.sendPasswordReset(member.email, member.firstName, anyString())); + when(emailService.sendPasswordReset(member.email.toLowerCase(), member.firstName, anyString())); const authController = ControllerFactory.auth(conn, instance(emailService)); const params = { email: member.email }; await expect(authController.sendPasswordResetEmail(params, FactoryUtils.randomHexString())) From 5c1bd1b076ffda633c35cfbb97cfe25df4411949 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Wed, 13 Mar 2024 16:28:52 -0700 Subject: [PATCH 24/30] migration script (#403) --- Dockerfile | 2 +- package.json | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 093bb5a3..2f035856 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,4 +10,4 @@ FROM node:14.15.3-alpine WORKDIR /app COPY --from=build /app /app EXPOSE 3000 -CMD ["yarn", "start"] +CMD ["yarn", "release"] diff --git a/package.json b/package.json index ffa2d67f..dc8cdcc8 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "start": "node build/index.js", "test": "jest --config jest.config.json --runInBand", "dev": "nodemon -L -e ts -i node_modules -i .git -V index.ts", + "release": "ts-node ./node_modules/typeorm/cli.js migration:run && node build/index.js", "lint": "eslint ./ --ext .ts", "lint:fix": "eslint ./ --ext .ts --fix", "db:migrate:create": "ts-node ./node_modules/typeorm/cli.js migration:create -n", From 5849581efb842e84b6e26281381ae25073325ffb Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Mon, 25 Mar 2024 17:33:06 -0700 Subject: [PATCH 25/30] Fix null linkedEvent for some Order routes (#419) * added left join for getOrder and getAllOrders * fixed order queries with linkedEvent joins * bumped package version to 3.4.3 * removed unnecessary join * cleanup --- package.json | 2 +- repositories/MerchOrderRepository.ts | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index dc8cdcc8..188180e1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.4.2", + "version": "3.4.3", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/repositories/MerchOrderRepository.ts b/repositories/MerchOrderRepository.ts index 139677da..769ceefd 100644 --- a/repositories/MerchOrderRepository.ts +++ b/repositories/MerchOrderRepository.ts @@ -11,7 +11,7 @@ import { MerchandiseItemModel } from '../models/MerchandiseItemModel'; export class MerchOrderRepository extends BaseRepository { /** * Gets a single order. Returns the order joined with ordered items, - * user, pickup event, the ordered items' merch options, + * user, pickup event, the pickup event's linked event, the ordered items' merch options, * and those merch options' merch items. * * This is the same set of joins that gets executed for OrderPickupEventRepository::findByUuid() @@ -25,12 +25,13 @@ export class MerchOrderRepository extends BaseRepository { .leftJoinAndSelect('orderItem.option', 'option') .leftJoinAndSelect('option.item', 'merchItem') .leftJoinAndSelect('merchItem.merchPhotos', 'merchPhotos') + .leftJoinAndSelect('orderPickupEvent.linkedEvent', 'linkedEvent') .where('order.uuid = :uuid', { uuid }) .getOne(); } /** - * Gets all orders for all users. Returns the order joined with its pickup event. + * Gets all orders for all users. Returns the order joined with its pickup event and linked event. * Can optionally filter by order status. */ public async getAllOrdersForAllUsers(...statuses: OrderStatus[]): Promise { @@ -41,14 +42,25 @@ export class MerchOrderRepository extends BaseRepository { }, }); } - return this.repository.find(); + return this.repository + .createQueryBuilder('order') + .leftJoinAndSelect('order.pickupEvent', 'orderPickupEvent') + .leftJoinAndSelect('order.user', 'user') + .leftJoinAndSelect('orderPickupEvent.linkedEvent', 'linkedEvent') + .getMany(); } /** - * Gets all orders for a given user. Returns the order joined with its pickup event and user. + * Gets all orders for a given user. Returns the order joined with its pickup event, linked event, and user. */ public async getAllOrdersForUser(user: UserModel): Promise { - return this.repository.find({ user }); + return this.repository + .createQueryBuilder('order') + .leftJoinAndSelect('order.pickupEvent', 'orderPickupEvent') + .leftJoinAndSelect('order.user', 'user') + .leftJoinAndSelect('orderPickupEvent.linkedEvent', 'linkedEvent') + .where('order.user = :uuid', { uuid: user.uuid }) + .getMany(); } /** From ee1051cc45e7ead98e38a43433719725f46e3868 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Thu, 28 Mar 2024 21:50:52 -0700 Subject: [PATCH 26/30] Remove lastLogin column from Users table (#421) * migration and edited model * Update package.json * typo in migration fixed --- .../0042-remove-users-lastLogin-column.ts | 17 +++++++++++++++++ models/UserModel.ts | 3 --- package.json | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 migrations/0042-remove-users-lastLogin-column.ts diff --git a/migrations/0042-remove-users-lastLogin-column.ts b/migrations/0042-remove-users-lastLogin-column.ts new file mode 100644 index 00000000..71ffbfd4 --- /dev/null +++ b/migrations/0042-remove-users-lastLogin-column.ts @@ -0,0 +1,17 @@ +import { MigrationInterface, QueryRunner, TableColumn } from 'typeorm'; + +const TABLE_NAME = 'Users'; + +export class RemoveUsersLastLoginColumn1711518997063 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.dropColumn(TABLE_NAME, 'lastLogin'); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.addColumn(TABLE_NAME, new TableColumn({ + name: 'lastLogin', + type: 'timestamptz', + default: 'CURRENT_TIMESTAMP(6)', + })); + } +} diff --git a/models/UserModel.ts b/models/UserModel.ts index 6f4e09cb..677ec3f1 100644 --- a/models/UserModel.ts +++ b/models/UserModel.ts @@ -65,9 +65,6 @@ export class UserModel extends BaseEntity { @Column('integer', { default: 0 }) credits: number; - @Column('timestamptz', { default: () => 'CURRENT_TIMESTAMP(6)' }) - lastLogin: Date; - @OneToMany((type) => ActivityModel, (activity) => activity.user, { cascade: true }) activities: ActivityModel[]; diff --git a/package.json b/package.json index 188180e1..71253733 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.4.3", + "version": "3.4.4", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ From 51d727f25e7686fd58a58a2a35cde047117665d5 Mon Sep 17 00:00:00 2001 From: Max Weng <73797155+maxwn04@users.noreply.github.com> Date: Fri, 29 Mar 2024 20:29:48 -0700 Subject: [PATCH 27/30] Add return type for CancelMerchOrder (#404) * cancel merch order return type * lint * cancel order response test * reorder API reponses * lint * bumped package.json to 3.4.5 --------- Co-authored-by: Nikhil Dange --- api/controllers/MerchStoreController.ts | 6 ++++-- package.json | 2 +- services/MerchStoreService.ts | 3 ++- tests/merchOrder.test.ts | 9 ++++++--- types/ApiResponses.ts | 4 ++++ 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/api/controllers/MerchStoreController.ts b/api/controllers/MerchStoreController.ts index 4bcf915e..628d62e5 100644 --- a/api/controllers/MerchStoreController.ts +++ b/api/controllers/MerchStoreController.ts @@ -49,6 +49,7 @@ import { CompleteOrderPickupEventResponse, GetOrderPickupEventResponse, CancelOrderPickupEventResponse, + CancelMerchOrderResponse, } from '../../types'; import { UuidParam } from '../validators/GenericRequests'; import { AuthenticatedUser } from '../decorators/AuthenticatedUser'; @@ -309,10 +310,11 @@ export class MerchStoreController { } @Post('/order/:uuid/cancel') - async cancelMerchOrder(@Params() params: UuidParam, @AuthenticatedUser() user: UserModel) { + async cancelMerchOrder(@Params() params: UuidParam, + @AuthenticatedUser() user: UserModel): Promise { if (!PermissionsService.canAccessMerchStore(user)) throw new ForbiddenError(); const order = await this.merchStoreService.cancelMerchOrder(params.uuid, user); - return { error: null, order }; + return { error: null, order: order.getPublicOrderWithItems() }; } @Post('/order/:uuid/fulfill') diff --git a/package.json b/package.json index 71253733..be21ae41 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.4.4", + "version": "3.4.5", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/services/MerchStoreService.ts b/services/MerchStoreService.ts index 01618694..b32839c3 100644 --- a/services/MerchStoreService.ts +++ b/services/MerchStoreService.ts @@ -761,7 +761,7 @@ export default class MerchStoreService { /** * Cancels a merch order, refunding the user of its credits if the user is the one who cancelled the order. */ - public async cancelMerchOrder(orderUuid: Uuid, user: UserModel): Promise { + public async cancelMerchOrder(orderUuid: Uuid, user: UserModel): Promise { return this.transactions.readWrite(async (txn) => { const orderRespository = Repositories.merchOrder(txn); const order = await orderRespository.findByUuid(orderUuid); @@ -785,6 +785,7 @@ export default class MerchStoreService { type: ActivityType.ORDER_CANCELLED, description: `Order ${order.uuid} cancelled and refunded to ${customer.uuid} by ${user.uuid}`, }); + return order; }); } diff --git a/tests/merchOrder.test.ts b/tests/merchOrder.test.ts index f4fe09e7..4c7967fe 100644 --- a/tests/merchOrder.test.ts +++ b/tests/merchOrder.test.ts @@ -442,7 +442,8 @@ describe('merch orders', () => { // cancel order const { uuid } = placedOrderResponse.order; - await merchController.cancelMerchOrder({ uuid }, member); + const cancelOrderResponse = await merchController.cancelMerchOrder({ uuid }, member); + expect(cancelOrderResponse.order.uuid).toEqual(uuid); // get order, making sure state was updated and user has been refunded const cancelledOrderResponse = await merchController.getOneMerchOrder({ uuid }, member); @@ -518,7 +519,8 @@ describe('merch orders', () => { // cancel the order const { uuid } = placedOrderResponse.order; - await merchController.cancelMerchOrder({ uuid }, member); + const cancelOrderResponse = await merchController.cancelMerchOrder({ uuid }, member); + expect(cancelOrderResponse.order.uuid).toEqual(uuid); // make sure user has only been refunded 1 option worth of points await member.reload(); @@ -890,7 +892,8 @@ describe('merch orders', () => { const merchController = ControllerFactory.merchStore(conn, instance(emailService)); const placedOrder = await conn.manager.findOne(OrderModel, { user: member }, { relations: ['items'] }); const cancelOrderParams = { uuid: placedOrder.uuid }; - await merchController.cancelMerchOrder(cancelOrderParams, member); + const cancelOrderResponse = await merchController.cancelMerchOrder(cancelOrderParams, member); + expect(cancelOrderResponse.order.uuid).toEqual(cancelOrderParams.uuid); // place order with 2 items again const secondOrder = [{ option: option.uuid, quantity: 2 }]; diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index f449b2d5..d211623b 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -311,6 +311,10 @@ export interface VerifyMerchOrderResponse extends ApiResponse {} export interface EditMerchOrderResponse extends ApiResponse {} +export interface CancelMerchOrderResponse extends ApiResponse { + order: PublicOrderWithItems; +} + export interface GetCartResponse extends ApiResponse { cart: PublicOrderMerchItemOption[]; } From 90b8f98439540397d88f2ccda655b02a8e6f36f3 Mon Sep 17 00:00:00 2001 From: yimmyj <69327109+yimmyj@users.noreply.github.com> Date: Sat, 30 Mar 2024 21:53:09 -0700 Subject: [PATCH 28/30] Event Feedback API Routes (#412) * modified isUnusedAttendanceCode check if an event has already happened, and don't count it if so * a * modified isUnusedAttendanceCode check if an event has already happened, and don't count it if so * updated feedbackmodel & eventmodel with each other as fields * test * Event field to feedback model, feedback searchable w/ query params * migrations added for updated feedbackmodel, included event field * started on seeding * aa * feedback queryparams tested and working * refactored feedback tests to fit with new model, added tests for queryparams searching feedback * tests * lint * addressed design issues, added user as field for queryparams, refactored tests * lint * Requested changes addressed * fixed one test * removed profanity * renamed canRespondToFeedback and its instances * removed comments * refactored feedback and removed point awards * done :] * added clarifying comment * updated migration * bumped package.json --------- Co-authored-by: Nikhil Dange --- api/controllers/FeedbackController.ts | 13 +- api/validators/FeedbackControllerRequests.ts | 25 +- migrations/0043-add-feedback-event-colum.ts | 54 +++ models/EventModel.ts | 4 + models/FeedbackModel.ts | 15 +- package.json | 2 +- repositories/FeedbackRepository.ts | 40 +- services/FeedbackService.ts | 52 +-- services/PermissionsService.ts | 8 +- tests/Seeds.ts | 48 ++- tests/data/FeedbackFactory.ts | 26 +- tests/data/PortalState.ts | 13 +- tests/feedback.test.ts | 409 +++++++++++++++---- tests/sample.test.ts | 2 +- types/ApiRequests.ts | 12 +- types/ApiResponses.ts | 3 +- 16 files changed, 591 insertions(+), 135 deletions(-) create mode 100644 migrations/0043-add-feedback-event-colum.ts diff --git a/api/controllers/FeedbackController.ts b/api/controllers/FeedbackController.ts index a1ed9b83..91db2921 100644 --- a/api/controllers/FeedbackController.ts +++ b/api/controllers/FeedbackController.ts @@ -1,4 +1,5 @@ -import { Body, ForbiddenError, Get, JsonController, Params, Patch, Post, UseBefore } from 'routing-controllers'; +import { Body, ForbiddenError, Get, JsonController, Params, + Patch, Post, UseBefore, QueryParams } from 'routing-controllers'; import { AuthenticatedUser } from '../decorators/AuthenticatedUser'; import { UserModel } from '../../models/UserModel'; import PermissionsService from '../../services/PermissionsService'; @@ -9,6 +10,7 @@ import { UserAuthentication } from '../middleware/UserAuthentication'; import { SubmitFeedbackRequest, UpdateFeedbackStatusRequest, + FeedbackSearchOptions, } from '../validators/FeedbackControllerRequests'; @UseBefore(UserAuthentication) @@ -21,9 +23,10 @@ export class FeedbackController { } @Get() - async getFeedback(@AuthenticatedUser() user: UserModel): Promise { - const canSeeAllFeedback = PermissionsService.canRespondToFeedback(user); - const feedback = await this.feedbackService.getFeedback(canSeeAllFeedback, user); + async getFeedback(@QueryParams() options: FeedbackSearchOptions, + @AuthenticatedUser() user: UserModel): Promise { + const canSeeAllFeedback = PermissionsService.canSeeAllFeedback(user); + const feedback = await this.feedbackService.getFeedback(canSeeAllFeedback, user, options); return { error: null, feedback }; } @@ -39,7 +42,7 @@ export class FeedbackController { async updateFeedbackStatus(@Params() params: UuidParam, @Body() updateFeedbackStatusRequest: UpdateFeedbackStatusRequest, @AuthenticatedUser() user: UserModel): Promise { - if (!PermissionsService.canRespondToFeedback(user)) throw new ForbiddenError(); + if (!PermissionsService.canSeeAllFeedback(user)) throw new ForbiddenError(); const feedback = await this.feedbackService.updateFeedbackStatus(params.uuid, updateFeedbackStatusRequest.status); return { error: null, feedback }; } diff --git a/api/validators/FeedbackControllerRequests.ts b/api/validators/FeedbackControllerRequests.ts index c699b84c..da712713 100644 --- a/api/validators/FeedbackControllerRequests.ts +++ b/api/validators/FeedbackControllerRequests.ts @@ -1,22 +1,27 @@ import { Type } from 'class-transformer'; -import { IsDefined, IsNotEmpty, MinLength, ValidateNested } from 'class-validator'; +import { Allow, IsDefined, IsNotEmpty, MinLength, ValidateNested } from 'class-validator'; import { IsValidFeedbackType, IsValidFeedbackStatus } from '../decorators/Validators'; import { SubmitEventFeedbackRequest as ISubmitEventFeedbackRequest, SubmitFeedbackRequest as ISubmitFeedbackRequest, UpdateFeedbackStatusRequest as IUpdateFeedbackStatusRequest, + FeedbackSearchOptions as IFeedbackSearchOptions, Feedback as IFeedback, FeedbackType, FeedbackStatus, + Uuid, } from '../../types'; export class Feedback implements IFeedback { @IsDefined() @IsNotEmpty() - title: string; + event: Uuid; @IsDefined() - @MinLength(100) + source: string; + + @IsDefined() + @MinLength(20) description: string; @IsDefined() @@ -41,3 +46,17 @@ export class UpdateFeedbackStatusRequest implements IUpdateFeedbackStatusRequest @IsValidFeedbackStatus() status: FeedbackStatus; } + +export class FeedbackSearchOptions implements IFeedbackSearchOptions { + @Allow() + event?: string; + + @Allow() + status?: string; + + @Allow() + type?: string; + + @Allow() + user?: string; +} diff --git a/migrations/0043-add-feedback-event-colum.ts b/migrations/0043-add-feedback-event-colum.ts new file mode 100644 index 00000000..f9ea6a22 --- /dev/null +++ b/migrations/0043-add-feedback-event-colum.ts @@ -0,0 +1,54 @@ +import { MigrationInterface, QueryRunner, TableColumn, TableIndex, TableForeignKey } from 'typeorm'; + +const TABLE_NAME = 'Feedback'; +const OLD_NAME = 'title'; +const NEW_NAME = 'source'; + +export class AddFeedbackEventColum1711860173561 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`ALTER TABLE "${TABLE_NAME}" RENAME COLUMN "${OLD_NAME}" TO "${NEW_NAME}"`); + await queryRunner.query(`ALTER TABLE "${TABLE_NAME}" ALTER COLUMN "${NEW_NAME}" TYPE text`); + + await queryRunner.addColumn( + TABLE_NAME, + new TableColumn({ + name: 'event', + type: 'uuid', + isNullable: true, + }), + ); + + await queryRunner.createIndex( + TABLE_NAME, + new TableIndex({ + name: 'feedback_by_event_index', + columnNames: ['event'], + }), + ); + + await queryRunner.createForeignKey( + TABLE_NAME, + new TableForeignKey({ + columnNames: ['event'], + referencedTableName: 'Events', + referencedColumnNames: ['uuid'], + onDelete: 'CASCADE', + }), + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.dropForeignKey(TABLE_NAME, new TableForeignKey({ + columnNames: ['event'], + referencedTableName: 'Events', + referencedColumnNames: ['uuid'], + onDelete: 'CASCADE', + })); + + await queryRunner.dropIndex(TABLE_NAME, 'feedback_by_event_index'); + await queryRunner.dropColumn(TABLE_NAME, 'event'); + + await queryRunner.query(`ALTER TABLE "${TABLE_NAME}" ALTER COLUMN "${NEW_NAME}" TYPE varchar(255)`); + await queryRunner.query(`ALTER TABLE "${TABLE_NAME}" RENAME COLUMN "${NEW_NAME}" TO "${OLD_NAME}"`); + } +} diff --git a/models/EventModel.ts b/models/EventModel.ts index 99f2d20e..e4a5f40f 100644 --- a/models/EventModel.ts +++ b/models/EventModel.ts @@ -2,6 +2,7 @@ import * as moment from 'moment'; import { BaseEntity, Column, Entity, Index, PrimaryGeneratedColumn, OneToMany } from 'typeorm'; import { PublicEvent, Uuid } from '../types'; import { AttendanceModel } from './AttendanceModel'; +import { FeedbackModel } from './FeedbackModel'; import { ExpressCheckinModel } from './ExpressCheckinModel'; @Entity('Events') @@ -60,6 +61,9 @@ export class EventModel extends BaseEntity { @OneToMany((type) => AttendanceModel, (attendance) => attendance.event, { cascade: true }) attendances: AttendanceModel[]; + @OneToMany((type) => FeedbackModel, (feedback) => feedback.event, { cascade: true }) + feedback: FeedbackModel[]; + @OneToMany((type) => ExpressCheckinModel, (expressCheckin) => expressCheckin.event, { cascade: true }) expressCheckins: ExpressCheckinModel[]; diff --git a/models/FeedbackModel.ts b/models/FeedbackModel.ts index 1c9df5d8..2bc2348d 100644 --- a/models/FeedbackModel.ts +++ b/models/FeedbackModel.ts @@ -1,6 +1,7 @@ import { BaseEntity, Column, Entity, Index, JoinColumn, ManyToOne, PrimaryGeneratedColumn } from 'typeorm'; import { FeedbackStatus, FeedbackType, PublicFeedback, Uuid } from '../types'; import { UserModel } from './UserModel'; +import { EventModel } from './EventModel'; @Entity('Feedback') export class FeedbackModel extends BaseEntity { @@ -12,8 +13,15 @@ export class FeedbackModel extends BaseEntity { @Index('feedback_by_user_index') user: UserModel; - @Column('varchar', { length: 255 }) - title: string; + @ManyToOne((type) => EventModel, (event) => event.feedback, { onDelete: 'CASCADE' }) + @JoinColumn({ name: 'event' }) + @Index('feedback_by_event_index') + event: EventModel; + + // source refers to how the user heard about the event + // ie, Instagram, Portal, Discord, etc. + @Column('text') + source: string; @Column('text') description: string; @@ -31,7 +39,8 @@ export class FeedbackModel extends BaseEntity { return { uuid: this.uuid, user: this.user.getPublicProfile(), - title: this.title, + event: this.event.getPublicEvent(), + source: this.source, description: this.description, timestamp: this.timestamp, status: this.status, diff --git a/package.json b/package.json index be21ae41..d495d8ff 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.4.5", + "version": "3.5.0", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/repositories/FeedbackRepository.ts b/repositories/FeedbackRepository.ts index d0a4f24d..fbaa8fe8 100644 --- a/repositories/FeedbackRepository.ts +++ b/repositories/FeedbackRepository.ts @@ -1,21 +1,22 @@ -import { EntityRepository } from 'typeorm'; +import { EntityRepository, SelectQueryBuilder } from 'typeorm'; import { FeedbackModel } from '../models/FeedbackModel'; import { UserModel } from '../models/UserModel'; +import { EventModel } from '../models/EventModel'; import { BaseRepository } from './BaseRepository'; -import { Uuid } from '../types'; +import { FeedbackSearchOptions, Uuid } from '../types'; @EntityRepository(FeedbackModel) export class FeedbackRepository extends BaseRepository { - public async getAllFeedback(): Promise { - return this.getBaseFindQuery().getMany(); + public async getAllFeedback(options: FeedbackSearchOptions): Promise { + return this.getBaseFindQuery(options).getMany(); } public async findByUuid(uuid: Uuid): Promise { - return this.getBaseFindQuery().where({ uuid }).getOne(); + return this.getBaseFindQuery({}).where({ uuid }).getOne(); } public async getAllFeedbackForUser(user: UserModel): Promise { - return this.getBaseFindQuery().where({ user }).getMany(); + return this.getBaseFindQuery({}).where({ user }).getMany(); } public async upsertFeedback(feedback: FeedbackModel, @@ -24,9 +25,32 @@ export class FeedbackRepository extends BaseRepository { return this.repository.save(feedback); } - private getBaseFindQuery() { - return this.repository.createQueryBuilder('feedback') + public async hasUserSubmittedFeedback(user: UserModel, event: EventModel): Promise { + const count = await this.repository.count({ + where: { user, event }, + }); + return count > 0; + } + + private getBaseFindQuery(options: FeedbackSearchOptions): SelectQueryBuilder { + let query = this.repository.createQueryBuilder('feedback') .leftJoinAndSelect('feedback.user', 'user') + .leftJoinAndSelect('feedback.event', 'event') .orderBy('timestamp', 'DESC'); + + if (options.event) { + query = query.andWhere('event = :event').setParameter('event', options.event); + } + if (options.user) { + query = query.andWhere('"user" = :user').setParameter('user', options.user); + } + if (options.type) { + query = query.andWhere('type = :type').setParameter('type', options.type); + } + if (options.status) { + query = query.andWhere('status = :status').setParameter('status', options.status); + } + + return query; } } diff --git a/services/FeedbackService.ts b/services/FeedbackService.ts index 8b5690c6..12e23654 100644 --- a/services/FeedbackService.ts +++ b/services/FeedbackService.ts @@ -5,9 +5,8 @@ import { NotFoundError } from 'routing-controllers'; import { FeedbackModel } from '../models/FeedbackModel'; import { UserModel } from '../models/UserModel'; import Repositories, { TransactionsManager } from '../repositories'; -import { PublicFeedback, Feedback, Uuid, ActivityType, FeedbackStatus } from '../types'; +import { PublicFeedback, Feedback, Uuid, ActivityType, FeedbackStatus, FeedbackSearchOptions } from '../types'; import { UserError } from '../utils/Errors'; -import { Config } from '../config'; @Service() export default class FeedbackService { @@ -17,28 +16,41 @@ export default class FeedbackService { this.transactions = new TransactionsManager(entityManager); } - public async getFeedback(canSeeAllFeedback = false, user: UserModel): Promise { - const feedback = await this.transactions.readOnly(async (txn) => { + public async getFeedback(canSeeAllFeedback = false, user: UserModel, + options: FeedbackSearchOptions): Promise { + return this.transactions.readOnly(async (txn) => { const feedbackRepository = Repositories.feedback(txn); - if (canSeeAllFeedback) return feedbackRepository.getAllFeedback(); - return feedbackRepository.getAllFeedbackForUser(user); + if (canSeeAllFeedback) { + return (await feedbackRepository.getAllFeedback(options)) + .map((fb) => fb.getPublicFeedback()); + } + + const userFeedback = await feedbackRepository.getAllFeedbackForUser(user); + return userFeedback.map((fb) => fb.getPublicFeedback()); }); - return feedback.map((fb) => fb.getPublicFeedback()); } public async submitFeedback(user: UserModel, feedback: Feedback): Promise { - const addedFeedback = await this.transactions.readWrite(async (txn) => { + return this.transactions.readWrite(async (txn) => { + const event = await Repositories.event(txn).findByUuid(feedback.event); + if (!event) throw new NotFoundError('Event not found!'); + + const feedbackRepository = Repositories.feedback(txn); + + const hasAlreadySubmittedFeedback = await feedbackRepository.hasUserSubmittedFeedback(user, event); + if (hasAlreadySubmittedFeedback) throw new UserError('You have already submitted feedback for this event!'); + await Repositories.activity(txn).logActivity({ user, type: ActivityType.SUBMIT_FEEDBACK, }); - return Repositories.feedback(txn).upsertFeedback(FeedbackModel.create({ ...feedback, user })); + const addedFeedback = await feedbackRepository.upsertFeedback(FeedbackModel.create({ ...feedback, user, event })); + return addedFeedback.getPublicFeedback(); }); - return addedFeedback.getPublicFeedback(); } public async updateFeedbackStatus(uuid: Uuid, status: FeedbackStatus) { - const acknowledgedFeedback = await this.transactions.readWrite(async (txn) => { + return this.transactions.readWrite(async (txn) => { const feedbackRepository = Repositories.feedback(txn); const feedback = await feedbackRepository.findByUuid(uuid); if (!feedback) throw new NotFoundError('Feedback not found'); @@ -47,18 +59,12 @@ export default class FeedbackService { } const { user } = feedback; - if (status === FeedbackStatus.ACKNOWLEDGED) { - const pointsEarned = Config.pointReward.FEEDBACK_POINT_REWARD; - await Repositories.activity(txn).logActivity({ - user, - type: ActivityType.FEEDBACK_ACKNOWLEDGED, - pointsEarned, - }); - await Repositories.user(txn).addPoints(user, pointsEarned); - } - - return feedbackRepository.upsertFeedback(feedback, { status }); + await Repositories.activity(txn).logActivity({ + user, + type: ActivityType.FEEDBACK_ACKNOWLEDGED, + }); + const updatedFeedback = await feedbackRepository.upsertFeedback(feedback, { status }); + return updatedFeedback.getPublicFeedback(); }); - return acknowledgedFeedback.getPublicFeedback(); } } diff --git a/services/PermissionsService.ts b/services/PermissionsService.ts index c8e0e19f..21cef53a 100644 --- a/services/PermissionsService.ts +++ b/services/PermissionsService.ts @@ -9,15 +9,15 @@ export default class PermissionsService { } public static canSeeEventAttendances(user: UserModel): boolean { - return user.isAdmin(); + return user.isAdmin() || user.isMarketing(); } public static canSubmitFeedback(user: UserModel): boolean { - return user.state === UserState.ACTIVE; + return user.state === UserState.ACTIVE || user.state === UserState.PASSWORD_RESET; } - public static canRespondToFeedback(user: UserModel): boolean { - return user.isAdmin(); + public static canSeeAllFeedback(user: UserModel): boolean { + return user.isAdmin() || user.isMarketing(); } public static canCreateMilestones(user: UserModel): boolean { diff --git a/tests/Seeds.ts b/tests/Seeds.ts index a3626228..979db0fc 100644 --- a/tests/Seeds.ts +++ b/tests/Seeds.ts @@ -1,7 +1,8 @@ import * as moment from 'moment'; -import { UserAccessType, SocialMediaType } from '../types'; +import { UserAccessType, SocialMediaType, FeedbackStatus, FeedbackType } from '../types'; import { DatabaseConnection, EventFactory, MerchFactory, PortalState, UserFactory, ResumeFactory, UserSocialMediaFactory } from './data'; +import { FeedbackFactory } from './data/FeedbackFactory'; function getGraduationYear(n: number) { return moment().year() + n; @@ -121,6 +122,11 @@ async function seed(): Promise { accessType: UserAccessType.SPONSORSHIP_MANAGER, }); + // Used for testing feedback + const USER_FEEDBACK_1 = UserFactory.fake({ firstName: 'FeedbackerOne', lastName: 'Jones' }); + const USER_FEEDBACK_2 = UserFactory.fake({ firstName: 'FeedbackerTwo', lastName: 'Patel' }); + const USER_FEEDBACK_3 = UserFactory.fake({ firstName: 'FeedbackerThree', lastName: 'Smith' }); + // Used for testing various User Social Media const USER_SOCIAL_MEDIA_1 = UserFactory.fake(); const USER_SOCIAL_MEDIA_1_FACEBOOK = UserSocialMediaFactory.fake( @@ -694,6 +700,41 @@ async function seed(): Promise { orderLimit: 10, }); + // FEEDBACK SEEDING + + // Event with multiple feedbacks: PAST_AI_WORKSHOP_1 + const FEEDBACK_SAME_EVENT_1 = FeedbackFactory.fake({ user: USER_FEEDBACK_1, + event: PAST_AI_WORKSHOP_1, + description: 'Man this #$%& sucks', + type: FeedbackType.AI }); + const FEEDBACK_SAME_EVENT_2 = FeedbackFactory.fake({ user: USER_FEEDBACK_2, + event: PAST_AI_WORKSHOP_1, + type: FeedbackType.AI }); + + // User with multiple feedbacks: USER_FEEDBACK_3 + const FEEDBACK_SAME_USER_1 = FeedbackFactory.fake({ user: USER_FEEDBACK_3, + event: PAST_AI_WORKSHOP_1, + type: FeedbackType.CYBER }); + + const FEEDBACK_SAME_USER_2 = FeedbackFactory.fake({ user: USER_FEEDBACK_3, + event: PAST_AI_WORKSHOP_2, + type: FeedbackType.GENERAL }); + + const FEEDBACK_SUBMITTED = FeedbackFactory.fake({ user: USER_FEEDBACK_3, + event: PAST_AI_WORKSHOP_1, + status: FeedbackStatus.SUBMITTED, + type: FeedbackType.INNOVATE }); + + const FEEDBACK_IGNORED = FeedbackFactory.fake({ user: USER_FEEDBACK_3, + event: PAST_AI_WORKSHOP_1, + status: FeedbackStatus.IGNORED, + type: FeedbackType.GENERAL }); + + const FEEDBACK_ACKNOWLEDGED = FeedbackFactory.fake({ user: USER_FEEDBACK_3, + event: PAST_AI_WORKSHOP_1, + status: FeedbackStatus.ACKNOWLEDGED, + type: FeedbackType.BIT_BYTE }); + await new PortalState() .createUsers( ADMIN, @@ -717,6 +758,9 @@ async function seed(): Promise { USER_SOCIAL_MEDIA_2, USER_SOCIAL_MEDIA_3, USER_SOCIAL_MEDIA_ALL, + USER_FEEDBACK_1, + USER_FEEDBACK_2, + USER_FEEDBACK_3, USER_VISIBLE_RESUME, USER_HIDDEN_RESUME, ...otherMembers, @@ -822,6 +866,8 @@ async function seed(): Promise { USER_SOCIAL_MEDIA_ALL_PORTFOLIO, USER_SOCIAL_MEDIA_ALL_EMAIL) .createResumes(USER_VISIBLE_RESUME, RESUME_1) .createResumes(USER_HIDDEN_RESUME, RESUME_2) + .createFeedback(FEEDBACK_SAME_EVENT_1, FEEDBACK_SAME_EVENT_2, FEEDBACK_SAME_USER_1, FEEDBACK_SAME_USER_2, + FEEDBACK_ACKNOWLEDGED, FEEDBACK_IGNORED, FEEDBACK_SUBMITTED) .write(); } diff --git a/tests/data/FeedbackFactory.ts b/tests/data/FeedbackFactory.ts index b20f6d6c..35e82ca2 100644 --- a/tests/data/FeedbackFactory.ts +++ b/tests/data/FeedbackFactory.ts @@ -1,22 +1,28 @@ import * as faker from 'faker'; -import { Feedback, FeedbackType } from '../../types'; +import { v4 as uuid } from 'uuid'; +import { FeedbackStatus, FeedbackType } from '../../types'; import FactoryUtils from './FactoryUtils'; +import { UserFactory } from './UserFactory'; +import { EventFactory } from './EventFactory'; +import { FeedbackModel } from '../../models/FeedbackModel'; export class FeedbackFactory { - public static create(n: number) { + public static create(n: number): FeedbackModel[] { return FactoryUtils.create(n, FeedbackFactory.fake); } - public static fake(substitute?: Partial): Feedback { - const fake = { - title: faker.datatype.hexaDecimal(10), + public static fake(substitute?: Partial): FeedbackModel { + const fake = FeedbackModel.create({ + uuid: uuid(), + user: UserFactory.fake(), + event: EventFactory.fake(), + source: FactoryUtils.pickRandomValue(['Discord', 'Instagram', 'Portal']), + timestamp: new Date(), description: faker.lorem.words(100), + status: FeedbackStatus.SUBMITTED, type: FeedbackFactory.randomFeedbackType(), - }; - return { - ...fake, - ...substitute, - }; + }); + return FeedbackModel.merge(fake, substitute); } private static randomFeedbackType(): FeedbackType { diff --git a/tests/data/PortalState.ts b/tests/data/PortalState.ts index 1bd1426f..f19af280 100644 --- a/tests/data/PortalState.ts +++ b/tests/data/PortalState.ts @@ -9,7 +9,7 @@ import { MerchandiseCollectionModel } from '../../models/MerchandiseCollectionMo import { OrderModel } from '../../models/OrderModel'; import { UserModel } from '../../models/UserModel'; import { ActivityModel } from '../../models/ActivityModel'; -import { ActivityScope, ActivityType, Feedback } from '../../types'; +import { ActivityScope, ActivityType } from '../../types'; import { MerchandiseItemOptionModel } from '../../models/MerchandiseItemOptionModel'; import { OrderItemModel } from '../../models/OrderItemModel'; import { FeedbackModel } from '../../models/FeedbackModel'; @@ -133,6 +133,15 @@ export class PortalState { return this; } + public createFeedback(...feedback: FeedbackModel[]): PortalState { + for (let f = 0; f < feedback.length; f += 1) { + const fb = feedback[f]; + + this.feedback.push(FeedbackModel.create({ ...fb })); + } + return this; + } + public attendEvents(users: UserModel[], events: EventModel[], includesStaff = false): PortalState { for (let e = 0; e < events.length; e += 1) { const event = events[e]; @@ -208,7 +217,7 @@ export class PortalState { }); } - public submitFeedback(user: UserModel, feedback: Feedback[]): PortalState { + public submitFeedback(user: UserModel, feedback: FeedbackModel[]): PortalState { for (let f = 0; f < feedback.length; f += 1) { const fb = feedback[f]; this.feedback.push(FeedbackModel.create({ ...fb, user })); diff --git a/tests/feedback.test.ts b/tests/feedback.test.ts index d593ff18..2c826eed 100644 --- a/tests/feedback.test.ts +++ b/tests/feedback.test.ts @@ -1,12 +1,23 @@ import { validate } from 'class-validator'; import { plainToClass } from 'class-transformer'; -import { DatabaseConnection, PortalState, UserFactory } from './data'; +import { DatabaseConnection, EventFactory, PortalState, UserFactory } from './data'; import { FeedbackFactory } from './data/FeedbackFactory'; -import { ActivityScope, ActivityType, FeedbackStatus, UserAccessType } from '../types'; +import { ActivityScope, ActivityType, FeedbackStatus, FeedbackType, UserAccessType } from '../types'; import { Feedback } from '../api/validators/FeedbackControllerRequests'; -import { Config } from '../config'; import { ControllerFactory } from './controllers'; +function buildFeedbackRequest(feedback) { + return { + feedback: { + event: feedback.event.uuid, + source: feedback.source, + status: feedback.status, + type: feedback.type, + description: feedback.description, + }, + }; +} + beforeAll(async () => { await DatabaseConnection.connect(); }); @@ -21,31 +32,46 @@ afterAll(async () => { }); describe('feedback submission', () => { - test('properly persists on successful submission', async () => { + test('users can submit feedback', async () => { + const event = EventFactory.fake(); const conn = await DatabaseConnection.get(); const member = UserFactory.fake(); - const feedback = FeedbackFactory.fake(); + const feedback = FeedbackFactory.fake({ event }); await new PortalState() .createUsers(member) + .createEvents(event) .write(); const feedbackController = ControllerFactory.feedback(conn); + const submittedFeedbackResponse = await feedbackController.submitFeedback(buildFeedbackRequest(feedback), member); + + // check response + expect(submittedFeedbackResponse.feedback.description).toEqual(feedback.description); + expect(submittedFeedbackResponse.feedback.source).toEqual(feedback.source); + expect(submittedFeedbackResponse.feedback.type).toEqual(feedback.type); + expect(submittedFeedbackResponse.feedback.status).toEqual(FeedbackStatus.SUBMITTED); + expect(submittedFeedbackResponse.feedback.event.uuid).toEqual(feedback.event.uuid); - await feedbackController.submitFeedback({ feedback }, member); - const submittedFeedbackResponse = await feedbackController.getFeedback(member); + // check if it persists + const queriedFeedback = await feedbackController.getFeedback({}, member); + expect(queriedFeedback.feedback).toHaveLength(1); - expect(submittedFeedbackResponse.feedback).toHaveLength(1); - expect(submittedFeedbackResponse.feedback[0]).toStrictEqual({ - ...submittedFeedbackResponse.feedback[0], + expect(queriedFeedback.feedback[0]).toEqual({ + ...submittedFeedbackResponse.feedback, user: member.getPublicProfile(), + event: event.getPublicEvent(), + source: feedback.source, + description: feedback.description, status: FeedbackStatus.SUBMITTED, - ...feedback, + type: feedback.type, }); }); test('is invalidated when submission description is too short', async () => { - const feedback = FeedbackFactory.fake({ description: 'A short description' }); + const event = EventFactory.fake(); + + const feedback = FeedbackFactory.fake({ event, description: 'A short description' }); const errors = await validate(plainToClass(Feedback, feedback)); @@ -56,15 +82,30 @@ describe('feedback submission', () => { }); test('has proper activity scope and type', async () => { + const event = EventFactory.fake({ + title: 'AI: Intro to Neural Nets', + description: `Artificial neural networks (ANNs), usually simply called + neural networks (NNs), are computing systems vaguely inspired by the + biological neural networks that constitute animal brains. An ANN is based + on a collection of connected units or nodes called artificial neurons, + which loosely model the neurons in a biological brain.`, + committee: 'AI', + location: 'Qualcomm Room', + ...EventFactory.daysBefore(6), + attendanceCode: 'galaxybrain', + requiresStaff: true, + }); + const conn = await DatabaseConnection.get(); const member = UserFactory.fake(); - const feedback = FeedbackFactory.fake(); + const feedback = FeedbackFactory.fake({ event }); await new PortalState() .createUsers(member) + .createEvents(event) .write(); - await ControllerFactory.feedback(conn).submitFeedback({ feedback }, member); + await ControllerFactory.feedback(conn).submitFeedback(buildFeedbackRequest(feedback), member); const activityResponse = await ControllerFactory.user(conn).getCurrentUserActivityStream(member); const feedbackSubmissionActivity = activityResponse.activity[1]; @@ -73,127 +114,353 @@ describe('feedback submission', () => { }); test('admins can view feedback from any member', async () => { + const event = EventFactory.fake(); + const conn = await DatabaseConnection.get(); const [member1, member2] = UserFactory.create(2); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); - const [feedback1, feedback2] = FeedbackFactory.create(2); + const feedback1 = FeedbackFactory.fake({ event }); + const feedback2 = FeedbackFactory.fake({ event }); await new PortalState() .createUsers(member1, member2, admin) + .createEvents(event) .write(); const feedbackController = ControllerFactory.feedback(conn); - const submittedFeedback1Response = await feedbackController.submitFeedback({ feedback: feedback1 }, member1); - const submittedFeedback2Response = await feedbackController.submitFeedback({ feedback: feedback2 }, member2); - const allSubmittedFeedbackResponse = await feedbackController.getFeedback(admin); + const submittedFeedback1Response = await feedbackController.submitFeedback( + buildFeedbackRequest(feedback1), member1, + ); + const submittedFeedback2Response = await feedbackController.submitFeedback( + buildFeedbackRequest(feedback2), member2, + ); + + const allSubmittedFeedback = await feedbackController.getFeedback({}, admin); + + expect(allSubmittedFeedback.feedback).toHaveLength(2); - expect(allSubmittedFeedbackResponse.feedback).toHaveLength(2); - expect(allSubmittedFeedbackResponse.feedback).toEqual( - expect.arrayContaining([submittedFeedback1Response.feedback, submittedFeedback2Response.feedback]), + expect(allSubmittedFeedback.feedback).toEqual( + expect.arrayContaining([ + submittedFeedback1Response.feedback, + submittedFeedback2Response.feedback, + ]), ); }); test('members can view only their own feedback', async () => { + const event = EventFactory.fake(); + const conn = await DatabaseConnection.get(); const [member1, member2] = UserFactory.create(2); - const [feedback1, feedback2] = FeedbackFactory.create(2); + const feedback1 = FeedbackFactory.fake({ event, user: member1 }); + const feedback2 = FeedbackFactory.fake({ event, user: member2 }); await new PortalState() .createUsers(member1, member2) + .createEvents(event) .write(); const feedbackController = ControllerFactory.feedback(conn); - await feedbackController.submitFeedback({ feedback: feedback1 }, member1); - await feedbackController.submitFeedback({ feedback: feedback2 }, member2); - const user1Feedback = await feedbackController.getFeedback(member1); + await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member1); + await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); + const user1Feedback = await feedbackController.getFeedback({}, member1); expect(user1Feedback.feedback).toHaveLength(1); - expect(user1Feedback.feedback[0]).toMatchObject(feedback1); + + expect(user1Feedback.feedback[0]).toMatchObject({ + ...user1Feedback.feedback[0], + user: member1.getPublicProfile(), + event: event.getPublicEvent(), + source: feedback1.source, + description: feedback1.description, + status: FeedbackStatus.SUBMITTED, + type: feedback1.type, + }); }); - test('admin can acknowledge and reward points for feedback', async () => { + test('cannot be responded to after already being responded to', async () => { + const event = EventFactory.fake(); + const conn = await DatabaseConnection.get(); - const member = UserFactory.fake(); + const [member1, member2] = UserFactory.create(2); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); - const feedback = FeedbackFactory.fake(); + const feedback1 = FeedbackFactory.fake({ event, user: member1 }); + const feedback2 = FeedbackFactory.fake({ event, user: member2 }); await new PortalState() - .createUsers(member, admin) + .createUsers(member1, member2, admin) + .createEvents(event) .write(); - const userController = ControllerFactory.user(conn); const feedbackController = ControllerFactory.feedback(conn); - const submittedFeedbackResponse = await feedbackController.submitFeedback({ feedback }, member); + const feedbackToAcknowledgeResponse = await feedbackController.submitFeedback( + buildFeedbackRequest(feedback1), member1, + ); + const feedbackToIgnoreResponse = await feedbackController.submitFeedback( + buildFeedbackRequest(feedback2), member2, + ); - const status = FeedbackStatus.ACKNOWLEDGED; - const uuid = submittedFeedbackResponse.feedback; - const acknowledgedFeedback = await feedbackController.updateFeedbackStatus(uuid, { status }, admin); + const feedbackToAcknowledgeParams = { uuid: feedbackToAcknowledgeResponse.feedback.uuid }; + const acknowledged = { status: FeedbackStatus.ACKNOWLEDGED }; - const persistedUserResponse = await userController.getUser({ uuid: member.uuid }, admin); + const feedbackToIgnoreParams = { uuid: feedbackToIgnoreResponse.feedback.uuid }; + const ignored = { status: FeedbackStatus.IGNORED }; - const feedbackPointReward = Config.pointReward.FEEDBACK_POINT_REWARD; + await feedbackController.updateFeedbackStatus(feedbackToAcknowledgeParams, acknowledged, admin); + await feedbackController.updateFeedbackStatus(feedbackToIgnoreParams, ignored, admin); + + const errorMessage = 'This feedback has already been responded to'; + await expect(feedbackController.updateFeedbackStatus(feedbackToAcknowledgeParams, acknowledged, admin)) + .rejects.toThrow(errorMessage); - expect(acknowledgedFeedback.feedback.status).toEqual(FeedbackStatus.ACKNOWLEDGED); - expect(persistedUserResponse.user.points).toEqual(member.points + feedbackPointReward); + await expect(feedbackController.updateFeedbackStatus(feedbackToIgnoreParams, ignored, admin)) + .rejects.toThrow(errorMessage); }); - test('admin can ignore and not reward points for feedback', async () => { + test('get all feedback for an event', async () => { + const event1 = EventFactory.fake({ + title: 'AI: Intro to Neural Nets', + description: `Artificial neural networks (ANNs), usually simply called + neural networks (NNs), are computing systems vaguely inspired by the + biological neural networks that constitute animal brains. An ANN is based + on a collection of connected units or nodes called artificial neurons, + which loosely model the neurons in a biological brain.`, + committee: 'AI', + location: 'Qualcomm Room', + ...EventFactory.daysBefore(6), + attendanceCode: 'galaxybrain', + requiresStaff: true, + cover: null, + thumbnail: null, + eventLink: null, + }); + + const event2 = EventFactory.fake({ + title: 'Not the right event!', + description: `Artificial neural networks (ANNs), usually simply called + neural networks (NNs), are computing systems vaguely inspired by the + biological neural networks that constitute animal brains. An ANN is based + on a collection of connected units or nodes called artificial neurons, + which loosely model the neurons in a biological brain.`, + committee: 'AI', + location: 'Qualcomm Room', + ...EventFactory.daysBefore(6), + attendanceCode: 'galxybrain', + requiresStaff: true, + cover: null, + thumbnail: null, + eventLink: null, + }); + const conn = await DatabaseConnection.get(); - const member = UserFactory.fake(); + const [member1, member2] = UserFactory.create(2); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const feedback1 = FeedbackFactory.fake({ event: event1, user: member1 }); + const feedback2 = FeedbackFactory.fake({ event: event1, user: member2 }); + const feedback3 = FeedbackFactory.fake({ event: event2, user: member1 }); + + await new PortalState() + .createUsers(member1, member2, admin) + .createEvents(event1, event2) + .write(); + + const feedbackController = ControllerFactory.feedback(conn); + const fb1Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member1); + const fb2Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); + const fb3Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback3), member1); + const event1Feedback = await feedbackController.getFeedback({ event: event1.uuid }, admin); + const event2Feedback = await feedbackController.getFeedback({ event: event2.uuid }, admin); + + expect(event1Feedback.feedback).toHaveLength(2); + expect(event2Feedback.feedback).toHaveLength(1); + + expect(event1Feedback.feedback).toEqual( + expect.arrayContaining([ + fb1Response.feedback, + fb2Response.feedback, + ]), + ); + + expect(event2Feedback.feedback).toEqual( + expect.arrayContaining([ + fb3Response.feedback, + ]), + ); + }); + + test('get all feedback by status', async () => { + const event1 = EventFactory.fake(); + + const conn = await DatabaseConnection.get(); + const [member1, member2, member3] = UserFactory.create(3); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); - const feedback = FeedbackFactory.fake(); + const feedback1 = FeedbackFactory.fake({ event: event1, status: FeedbackStatus.ACKNOWLEDGED }); + const feedback2 = FeedbackFactory.fake({ event: event1, status: FeedbackStatus.ACKNOWLEDGED }); + const feedback3 = FeedbackFactory.fake({ event: event1, status: FeedbackStatus.SUBMITTED }); await new PortalState() - .createUsers(member, admin) + .createUsers(member1, member2, member3, admin) + .createEvents(event1) .write(); - const userController = ControllerFactory.user(conn); const feedbackController = ControllerFactory.feedback(conn); + const fb1Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member1); + const fb2Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); + const fb3Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback3), member3); + const status1Feedback = await feedbackController.getFeedback({ status: FeedbackStatus.ACKNOWLEDGED }, admin); + const status2Feedback = await feedbackController.getFeedback({ status: FeedbackStatus.SUBMITTED }, admin); + + expect(status1Feedback.feedback).toHaveLength(2); + expect(status2Feedback.feedback).toHaveLength(1); + + expect(status1Feedback.feedback).toEqual( + expect.arrayContaining([ + fb1Response.feedback, + fb2Response.feedback, + ]), + ); - const submittedFeedbackResponse = await feedbackController.submitFeedback({ feedback }, member); - const status = FeedbackStatus.IGNORED; - const uuid = submittedFeedbackResponse.feedback; - const ignoredFeedbackResponse = await feedbackController.updateFeedbackStatus(uuid, { status }, admin); + expect(status2Feedback.feedback).toEqual( + expect.arrayContaining([ + fb3Response.feedback, + ]), + ); + }); - const persistedUserResponse = await userController.getUser({ uuid: member.uuid }, admin); + test('get all feedback by type', async () => { + const event1 = EventFactory.fake(); + + const conn = await DatabaseConnection.get(); + const [member1, member2, member3] = UserFactory.create(3); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const feedback1 = FeedbackFactory.fake({ event: event1, type: FeedbackType.GENERAL, user: member1 }); + const feedback2 = FeedbackFactory.fake({ event: event1, type: FeedbackType.GENERAL, user: member2 }); + const feedback3 = FeedbackFactory.fake({ event: event1, type: FeedbackType.AI, user: member3 }); + + await new PortalState() + .createUsers(member1, member2, member3, admin) + .createEvents(event1) + .write(); - expect(ignoredFeedbackResponse.feedback.status).toEqual(FeedbackStatus.IGNORED); - expect(persistedUserResponse.user.points).toEqual(member.points); + const feedbackController = ControllerFactory.feedback(conn); + const fb1Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member1); + const fb2Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); + const fb3Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback3), member3); + const type1Feedback = await feedbackController.getFeedback({ type: FeedbackType.GENERAL }, admin); + const type2Feedback = await feedbackController.getFeedback({ type: FeedbackType.AI }, admin); + + expect(type1Feedback.feedback).toHaveLength(2); + expect(type2Feedback.feedback).toHaveLength(1); + + expect(type1Feedback.feedback).toEqual( + expect.arrayContaining([ + fb1Response.feedback, + fb2Response.feedback, + ]), + ); + + expect(type2Feedback.feedback).toEqual( + expect.arrayContaining([ + fb3Response.feedback, + ]), + ); }); - test('cannot be responded to after already being responded to', async () => { + test('get all feedback by member', async () => { + const event1 = EventFactory.fake(); + const conn = await DatabaseConnection.get(); const member = UserFactory.fake(); + const member2 = UserFactory.fake(); const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); - const [feedback1, feedback2] = FeedbackFactory.create(2); + const feedback1 = FeedbackFactory.fake({ event: event1, user: member }); + const feedback2 = FeedbackFactory.fake({ event: event1, user: member2 }); await new PortalState() - .createUsers(member, admin) + .createUsers(member, member2, admin) + .createEvents(event1) .write(); const feedbackController = ControllerFactory.feedback(conn); + const fb1Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member); + const fb2Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); - const feedbackToAcknowledgeResponse = await feedbackController.submitFeedback({ feedback: feedback1 }, member); - const feedbackToIgnoreResponse = await feedbackController.submitFeedback({ feedback: feedback2 }, member); + const type1Feedback = await feedbackController.getFeedback({ user: member.uuid }, admin); + const type2Feedback = await feedbackController.getFeedback({ user: member2.uuid }, admin); - const feedbackToAcknowledgeParams = { uuid: feedbackToAcknowledgeResponse.feedback.uuid }; - const feedbackToIgnoreParams = { uuid: feedbackToIgnoreResponse.feedback.uuid }; - const acknowledged = { status: FeedbackStatus.ACKNOWLEDGED }; - const ignored = { status: FeedbackStatus.IGNORED }; + expect(type1Feedback.feedback).toHaveLength(1); + expect(type2Feedback.feedback).toHaveLength(1); - await feedbackController.updateFeedbackStatus(feedbackToAcknowledgeParams, acknowledged, admin); - await feedbackController.updateFeedbackStatus(feedbackToIgnoreParams, ignored, admin); + expect(type1Feedback.feedback).toEqual( + expect.arrayContaining([ + fb1Response.feedback, + ]), + ); - const errorMessage = 'This feedback has already been responded to'; - await expect(feedbackController.updateFeedbackStatus(feedbackToAcknowledgeParams, acknowledged, admin)) - .rejects.toThrow(errorMessage); - await expect(feedbackController.updateFeedbackStatus(feedbackToAcknowledgeParams, ignored, admin)) - .rejects.toThrow(errorMessage); - await expect(feedbackController.updateFeedbackStatus(feedbackToIgnoreParams, acknowledged, admin)) - .rejects.toThrow(errorMessage); - await expect(feedbackController.updateFeedbackStatus(feedbackToIgnoreParams, ignored, admin)) - .rejects.toThrow(errorMessage); + expect(type2Feedback.feedback).toEqual( + expect.arrayContaining([ + fb2Response.feedback, + ]), + ); + }); + + test('get all feedback with multiple parameters', async () => { + const event1 = EventFactory.fake(); + + const event2 = EventFactory.fake(); + + const conn = await DatabaseConnection.get(); + const [member1, member2, member3] = UserFactory.create(3); + const admin = UserFactory.fake({ accessType: UserAccessType.ADMIN }); + const feedback1 = FeedbackFactory.fake({ event: event1, + status: FeedbackStatus.ACKNOWLEDGED, + type: FeedbackType.GENERAL, + user: member1 }); + const feedback2 = FeedbackFactory.fake({ event: event1, + status: FeedbackStatus.IGNORED, + type: FeedbackType.GENERAL, + user: member2 }); + const feedback3 = FeedbackFactory.fake({ event: event1, + status: FeedbackStatus.SUBMITTED, + type: FeedbackType.INNOVATE, + user: member3 }); + const feedback4 = FeedbackFactory.fake({ event: event2, + status: FeedbackStatus.ACKNOWLEDGED, + type: FeedbackType.GENERAL, + user: member1 }); + + await new PortalState() + .createUsers(member1, member2, member3, admin) + .createEvents(event1, event2) + .write(); + + const feedbackController = ControllerFactory.feedback(conn); + const fb1Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member1); + const fb2Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); + await feedbackController.submitFeedback(buildFeedbackRequest(feedback3), member3); + const fb4Response = await feedbackController.submitFeedback(buildFeedbackRequest(feedback4), member1); + const query1Feedback = await feedbackController.getFeedback({ event: event1.uuid, + type: FeedbackType.GENERAL }, admin); + const query2Feedback = await feedbackController.getFeedback({ status: FeedbackStatus.ACKNOWLEDGED, + type: FeedbackType.GENERAL }, admin); + + expect(query1Feedback.feedback).toHaveLength(2); + expect(query2Feedback.feedback).toHaveLength(2); + + expect(query1Feedback.feedback).toEqual( + expect.arrayContaining([ + fb1Response.feedback, + fb2Response.feedback, + ]), + ); + + expect(query2Feedback.feedback).toEqual( + expect.arrayContaining([ + fb1Response.feedback, + fb4Response.feedback, + ]), + ); }); }); diff --git a/tests/sample.test.ts b/tests/sample.test.ts index e8d95aa5..7b1c89b9 100644 --- a/tests/sample.test.ts +++ b/tests/sample.test.ts @@ -42,7 +42,7 @@ describe('sample test', () => { discountPercentage: 0, }); const orderPickupEvent = MerchFactory.fakeOrderPickupEvent(); - const feedback = FeedbackFactory.fake(); + const feedback = FeedbackFactory.fake({ event }); const state = new PortalState() .createUsers(user1, user2) diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index 44447fb9..cc00a740 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -1,5 +1,5 @@ -import { FeedbackStatus, FeedbackType, SocialMediaType, UserAccessType } from './Enums'; import { Uuid } from '.'; +import { FeedbackStatus, FeedbackType, SocialMediaType, UserAccessType } from './Enums'; // REQUEST TYPES @@ -45,7 +45,8 @@ export interface RegistrationRequest { // USER export interface Feedback { - title: string; + event: Uuid; + source: string; description: string; type: FeedbackType; } @@ -82,6 +83,13 @@ export interface UpdateFeedbackStatusRequest { status: FeedbackStatus; } +export interface FeedbackSearchOptions { + event?: string; + type?: string; + status?: string; + user?: string; +} + export interface InsertUserSocialMediaRequest { socialMedia: SocialMedia; } diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index d211623b..edafe946 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -357,7 +357,8 @@ export interface PrivateProfile extends PublicProfile { export interface PublicFeedback { uuid: Uuid, user: PublicProfile, - title: string; + event: PublicEvent, + source: string; description: string; timestamp: Date; status: FeedbackStatus; From 5a48e0558046aa7adff526f4c9b79c1a4ef03bf0 Mon Sep 17 00:00:00 2001 From: "Caogang (Marcelo) Shen" Date: Mon, 1 Apr 2024 15:34:14 -0700 Subject: [PATCH 29/30] Change the request format for social media routes (#423) * changed some formats * check edge case of duplicate items * edited some tests * lint fix * package bump --- api/controllers/UserController.ts | 11 +- .../UserSocialMediaControllerRequests.ts | 11 +- package.json | 2 +- services/UserSocialMediaService.ts | 59 ++++++-- tests/userSocialMedia.test.ts | 129 +++++++++++------- types/ApiRequests.ts | 7 +- types/ApiResponses.ts | 4 +- 7 files changed, 143 insertions(+), 80 deletions(-) diff --git a/api/controllers/UserController.ts b/api/controllers/UserController.ts index e80f4347..1ecb88e5 100644 --- a/api/controllers/UserController.ts +++ b/api/controllers/UserController.ts @@ -108,16 +108,15 @@ export class UserController { @AuthenticatedUser() user: UserModel): Promise { const userSocialMedia = await this.userSocialMediaService .insertSocialMediaForUser(user, insertSocialMediaRequest.socialMedia); - return { error: null, userSocialMedia: userSocialMedia.getPublicSocialMedia() }; + return { error: null, userSocialMedia: userSocialMedia.map((socialMedia) => socialMedia.getPublicSocialMedia()) }; } - @Patch('/socialMedia/:uuid') - async updateSocialMediaForUser(@Params() params: UuidParam, - @Body() updateSocialMediaRequest: UpdateSocialMediaRequest, + @Patch('/socialMedia') + async updateSocialMediaForUser(@Body() updateSocialMediaRequest: UpdateSocialMediaRequest, @AuthenticatedUser() user: UserModel): Promise { const userSocialMedia = await this.userSocialMediaService - .updateSocialMediaByUuid(user, params.uuid, updateSocialMediaRequest.socialMedia); - return { error: null, userSocialMedia: userSocialMedia.getPublicSocialMedia() }; + .updateSocialMediaByUuid(user, updateSocialMediaRequest.socialMedia); + return { error: null, userSocialMedia: userSocialMedia.map((socialMedia) => socialMedia.getPublicSocialMedia()) }; } @Delete('/socialMedia/:uuid') diff --git a/api/validators/UserSocialMediaControllerRequests.ts b/api/validators/UserSocialMediaControllerRequests.ts index 0a304e78..2f9a4672 100644 --- a/api/validators/UserSocialMediaControllerRequests.ts +++ b/api/validators/UserSocialMediaControllerRequests.ts @@ -20,20 +20,25 @@ export class SocialMedia implements ISocialMedia { } export class SocialMediaPatches implements ISocialMediaPatches { + @IsDefined() @IsNotEmpty() - url?: string; + uuid: string; + + @IsDefined() + @IsNotEmpty() + url: string; } export class InsertSocialMediaRequest implements IInsertUserSocialMediaRequest { @Type(() => SocialMedia) @ValidateNested() @IsDefined() - socialMedia: SocialMedia; + socialMedia: SocialMedia[]; } export class UpdateSocialMediaRequest implements IUpdateUserSocialMediaRequest { @Type(() => SocialMediaPatches) @ValidateNested() @IsDefined() - socialMedia: SocialMediaPatches; + socialMedia: SocialMediaPatches[]; } diff --git a/package.json b/package.json index d495d8ff..e854d21b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@acmucsd/membership-portal", - "version": "3.5.0", + "version": "3.5.1", "description": "REST API for ACM UCSD's membership portal.", "main": "index.d.ts", "files": [ diff --git a/services/UserSocialMediaService.ts b/services/UserSocialMediaService.ts index 6a13cf20..2f59e5e7 100644 --- a/services/UserSocialMediaService.ts +++ b/services/UserSocialMediaService.ts @@ -6,7 +6,7 @@ import { UserError } from '../utils/Errors'; import { UserSocialMediaModel } from '../models/UserSocialMediaModel'; import { UserModel } from '../models/UserModel'; import Repositories, { TransactionsManager } from '../repositories'; -import { Uuid, SocialMedia } from '../types'; +import { Uuid, SocialMedia, SocialMediaPatches } from '../types'; @Service() export default class UserSocialMediaService { @@ -22,29 +22,58 @@ export default class UserSocialMediaService { return userSocialMedia; } - public async insertSocialMediaForUser(user: UserModel, socialMedia: SocialMedia) { + public async insertSocialMediaForUser(user: UserModel, socialMedias: SocialMedia[]) { const addedSocialMedia = await this.transactions.readWrite(async (txn) => { + // checking duplicate + const setDuplicateType = new Set(); + socialMedias.forEach((socialMedia) => { + const { type } = socialMedia; + if (setDuplicateType.has(type)) { + throw new UserError(`Dupllicate type "${type}" found in the request`); + } + setDuplicateType.add(type); + }); + + // inserting social media const userSocialMediaRepository = Repositories.userSocialMedia(txn); - const isNewSocialMediaType = await userSocialMediaRepository.isNewSocialMediaTypeForUser(user, socialMedia.type); - if (!isNewSocialMediaType) { - throw new UserError('Social media URL of this type has already been created for this user'); - } - return userSocialMediaRepository.upsertSocialMedia(UserSocialMediaModel.create({ ...socialMedia, user })); + const upsertedSocialMedias = await Promise.all(socialMedias.map(async (socialMedia) => { + const isNewSocialMediaType = await userSocialMediaRepository + .isNewSocialMediaTypeForUser(user, socialMedia.type); + if (!isNewSocialMediaType) { + throw new UserError(`Social media URL of type "${socialMedia.type}" has already been created for this user`); + } + return userSocialMediaRepository.upsertSocialMedia(UserSocialMediaModel.create({ ...socialMedia, user })); + })); + return upsertedSocialMedias; }); return addedSocialMedia; } public async updateSocialMediaByUuid(user: UserModel, - uuid: Uuid, - changes: Partial): Promise { + changes: SocialMediaPatches[]): Promise { const updatedSocialMedia = await this.transactions.readWrite(async (txn) => { + // checking duplicate + const setDuplicateUuid = new Set(); + + changes.forEach((socialMediaPatch) => { + const { uuid } = socialMediaPatch; + if (setDuplicateUuid.has(uuid)) { + throw new UserError(`Dupllicate UUID "${uuid}" found in the request`); + } + setDuplicateUuid.add(uuid); + }); + + // patching social media const userSocialMediaRepository = Repositories.userSocialMedia(txn); - const socialMedia = await userSocialMediaRepository.findByUuid(uuid); - if (!socialMedia) throw new NotFoundError('Social media URL not found'); - if (user.uuid !== socialMedia.user.uuid) { - throw new ForbiddenError('User cannot update a social media URL of another user'); - } - return userSocialMediaRepository.upsertSocialMedia(socialMedia, changes); + const modifiedSocialMedia = await Promise.all(changes.map(async (socialMediaPatches) => { + const socialMedia = await userSocialMediaRepository.findByUuid(socialMediaPatches.uuid); + if (!socialMedia) throw new NotFoundError(`Social media of UUID "${socialMediaPatches.uuid}" not found`); + if (user.uuid !== socialMedia.user.uuid) { + throw new ForbiddenError('User cannot update a social media URL of another user'); + } + return userSocialMediaRepository.upsertSocialMedia(socialMedia, { url: socialMediaPatches.url }); + })); + return modifiedSocialMedia; }); return updatedSocialMedia; } diff --git a/tests/userSocialMedia.test.ts b/tests/userSocialMedia.test.ts index f2b39778..5c70c72d 100644 --- a/tests/userSocialMedia.test.ts +++ b/tests/userSocialMedia.test.ts @@ -23,22 +23,31 @@ describe('social media URL submission', () => { test('properly persists on successful submission', async () => { const conn = await DatabaseConnection.get(); - let member = UserFactory.fake(); - const userSocialMedia = UserSocialMediaFactory.fake({ user: member }); + let member1 = UserFactory.fake(); + const socialMediaTypes = [SocialMediaType.FACEBOOK, SocialMediaType.TWITTER, SocialMediaType.LINKEDIN]; + const userSocialMediaList = socialMediaTypes.map((type) => UserSocialMediaFactory.fake({ user: member1, type })); + // sort by type to ensure order is consistent + userSocialMediaList.sort((a, b) => a.type.localeCompare(b.type)); + + // console.log("LIST", userSocialMediaList); await new PortalState() - .createUsers(member) + .createUsers(member1) .write(); const userController = ControllerFactory.user(conn); - await userController.insertSocialMediaForUser({ socialMedia: userSocialMedia }, member); - member = await conn.manager.findOne(UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }); - expect(member.userSocialMedia).toHaveLength(1); - expect(member.userSocialMedia[0]).toEqual({ - url: userSocialMedia.url, - type: userSocialMedia.type, - uuid: userSocialMedia.uuid, + await userController.insertSocialMediaForUser({ socialMedia: userSocialMediaList }, member1); + member1 = await conn.manager.findOne(UserModel, { uuid: member1.uuid }, { relations: ['userSocialMedia'] }); + + expect(member1.userSocialMedia).toHaveLength(3); + const userSocialMediaQuery = member1.userSocialMedia.sort((a, b) => a.type.localeCompare(b.type)); + userSocialMediaList.forEach((socialMedia, index) => { + expect(userSocialMediaQuery[index]).toEqual({ + url: socialMedia.url, + type: socialMedia.type, + uuid: socialMedia.uuid, + }); }); }); @@ -55,8 +64,9 @@ describe('social media URL submission', () => { const userController = ControllerFactory.user(conn); const userSocialMediaWithSameType = UserSocialMediaFactory.fake({ type: SocialMediaType.FACEBOOK }); - const errorMessage = 'Social media URL of this type has already been created for this user'; - await expect(userController.insertSocialMediaForUser({ socialMedia: userSocialMediaWithSameType }, member)) + const errorMessage = `Social media URL of type "${SocialMediaType.FACEBOOK}"` + + ' has already been created for this user'; + await expect(userController.insertSocialMediaForUser({ socialMedia: [userSocialMediaWithSameType] }, member)) .rejects.toThrow(errorMessage); }); }); @@ -68,22 +78,29 @@ describe('social media URL update', () => { test('is invalidated when updating social media URL of another user', async () => { const conn = await DatabaseConnection.get(); - let member = UserFactory.fake(); + let member1 = UserFactory.fake(); const unauthorizedMember = UserFactory.fake(); - const userSocialMedia = UserSocialMediaFactory.fake({ user: member, type: SocialMediaType.FACEBOOK }); + const socialMediaTypes = [SocialMediaType.FACEBOOK, SocialMediaType.TWITTER, SocialMediaType.LINKEDIN]; + const userSocialMediaList = socialMediaTypes.map((type) => UserSocialMediaFactory.fake({ user: member1, type })); + // sort by type to ensure order is consistent + userSocialMediaList.sort((a, b) => a.type.localeCompare(b.type)); await new PortalState() - .createUsers(member, unauthorizedMember) - .createUserSocialMedia(member, userSocialMedia) + .createUsers(member1, unauthorizedMember) .write(); const userController = ControllerFactory.user(conn); - member = await conn.manager.findOne(UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }); + await userController.insertSocialMediaForUser({ socialMedia: userSocialMediaList }, member1); + member1 = await conn.manager.findOne(UserModel, { uuid: member1.uuid }, { relations: ['userSocialMedia'] }); + + expect(member1.userSocialMedia).toHaveLength(3); const errorMessage = 'User cannot update a social media URL of another user'; - const uuidParams = { uuid: member.userSocialMedia[0].uuid }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await expect(userController.updateSocialMediaForUser(uuidParams, socialMediaParams, unauthorizedMember)) + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: member1.userSocialMedia[0].uuid, + }] }; + await expect(userController.updateSocialMediaForUser(socialMediaParams, unauthorizedMember)) .rejects.toThrow(errorMessage); }); @@ -97,10 +114,12 @@ describe('social media URL update', () => { const userController = ControllerFactory.user(conn); - const errorMessage = 'Social media URL not found'; - const missingEntityUuid = { uuid: faker.datatype.uuid() }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await expect(userController.updateSocialMediaForUser(missingEntityUuid, socialMediaParams, member)) + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: faker.datatype.uuid(), + }] }; + const errorMessage = `Social media of UUID "${socialMediaParams.socialMedia[0].uuid}" not found`; + await expect(userController.updateSocialMediaForUser(socialMediaParams, member)) .rejects.toThrow(errorMessage); }); }); @@ -130,7 +149,7 @@ describe('social media URL delete', () => { }); }); -describe('social media URL update', () => { +describe('social media URL update concurrency', () => { let conn : Connection; let member : UserModel; let userController : UserController; @@ -183,14 +202,16 @@ describe('social media URL update', () => { test.concurrent('concurrent updates properly persist on successful submission 0', async () => { await waitForFlag(); const index = 0; - const uuidParams = { uuid: member.userSocialMedia[index].uuid }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: member.userSocialMedia[index].uuid, + }] }; + await userController.updateSocialMediaForUser(socialMediaParams, member); const expectedUserSocialMedia0 = { - url: socialMediaParams.socialMedia.url, + url: socialMediaParams.socialMedia[0].url, type: member.userSocialMedia[index].type, - uuid: uuidParams.uuid, + uuid: socialMediaParams.socialMedia[0].uuid, }; const updatedMember = await conn.manager.findOne( UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, @@ -207,14 +228,16 @@ describe('social media URL update', () => { test.concurrent('concurrent updates properly persist on successful submission 1', async () => { await waitForFlag(); const index = 1; - const uuidParams = { uuid: member.userSocialMedia[index].uuid }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: member.userSocialMedia[index].uuid, + }] }; + await userController.updateSocialMediaForUser(socialMediaParams, member); const expectedUserSocialMedia1 = { - url: socialMediaParams.socialMedia.url, + url: socialMediaParams.socialMedia[0].url, type: member.userSocialMedia[index].type, - uuid: uuidParams.uuid, + uuid: socialMediaParams.socialMedia[0].uuid, }; const updatedMember = await conn.manager.findOne( UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, @@ -231,14 +254,16 @@ describe('social media URL update', () => { test.concurrent('concurrent updates properly persist on successful submission 2', async () => { await waitForFlag(); const index = 2; - const uuidParams = { uuid: member.userSocialMedia[index].uuid }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: member.userSocialMedia[index].uuid, + }] }; + await userController.updateSocialMediaForUser(socialMediaParams, member); const expectedUserSocialMedia2 = { - url: socialMediaParams.socialMedia.url, + url: socialMediaParams.socialMedia[0].url, type: member.userSocialMedia[index].type, - uuid: uuidParams.uuid, + uuid: socialMediaParams.socialMedia[0].uuid, }; const updatedMember = await conn.manager.findOne( UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, @@ -255,14 +280,16 @@ describe('social media URL update', () => { test.concurrent('concurrent updates properly persist on successful submission 3', async () => { await waitForFlag(); const index = 3; - const uuidParams = { uuid: member.userSocialMedia[index].uuid }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: member.userSocialMedia[index].uuid, + }] }; + await userController.updateSocialMediaForUser(socialMediaParams, member); const expectedUserSocialMedia3 = { - url: socialMediaParams.socialMedia.url, + url: socialMediaParams.socialMedia[0].url, type: member.userSocialMedia[index].type, - uuid: uuidParams.uuid, + uuid: socialMediaParams.socialMedia[0].uuid, }; const updatedMember = await conn.manager.findOne( UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, @@ -279,14 +306,16 @@ describe('social media URL update', () => { test.concurrent('concurrent updates properly persist on successful submission 4', async () => { await waitForFlag(); const index = 4; - const uuidParams = { uuid: member.userSocialMedia[index].uuid }; - const socialMediaParams = { socialMedia: { url: faker.internet.url() } }; - await userController.updateSocialMediaForUser(uuidParams, socialMediaParams, member); + const socialMediaParams = { socialMedia: [{ + url: faker.internet.url(), + uuid: member.userSocialMedia[index].uuid, + }] }; + await userController.updateSocialMediaForUser(socialMediaParams, member); const expectedUserSocialMedia4 = { - url: socialMediaParams.socialMedia.url, + url: socialMediaParams.socialMedia[0].url, type: member.userSocialMedia[index].type, - uuid: uuidParams.uuid, + uuid: socialMediaParams.socialMedia[0].uuid, }; const updatedMember = await conn.manager.findOne( UserModel, { uuid: member.uuid }, { relations: ['userSocialMedia'] }, diff --git a/types/ApiRequests.ts b/types/ApiRequests.ts index cc00a740..87a6b6dc 100644 --- a/types/ApiRequests.ts +++ b/types/ApiRequests.ts @@ -91,15 +91,16 @@ export interface FeedbackSearchOptions { } export interface InsertUserSocialMediaRequest { - socialMedia: SocialMedia; + socialMedia: SocialMedia[]; } export interface SocialMediaPatches { - url?: string; + uuid: string; + url: string; } export interface UpdateUserSocialMediaRequest { - socialMedia: SocialMediaPatches; + socialMedia: SocialMediaPatches[]; } // LEADERBOARD diff --git a/types/ApiResponses.ts b/types/ApiResponses.ts index edafe946..4bc289d6 100644 --- a/types/ApiResponses.ts +++ b/types/ApiResponses.ts @@ -417,11 +417,11 @@ export interface GetUserSocialMediaResponse extends ApiResponse { } export interface InsertSocialMediaResponse extends ApiResponse { - userSocialMedia: PublicUserSocialMedia; + userSocialMedia: PublicUserSocialMedia[]; } export interface UpdateSocialMediaResponse extends ApiResponse { - userSocialMedia: PublicUserSocialMedia; + userSocialMedia: PublicUserSocialMedia[]; } export interface DeleteSocialMediaResponse extends ApiResponse {} From c3d599cddfecfab03b51de1505451fdeba5b07b7 Mon Sep 17 00:00:00 2001 From: Nikhil Dange Date: Mon, 1 Apr 2024 23:22:09 -0700 Subject: [PATCH 30/30] Hotfix: Fix inconsistent feedback query (#424) * temporary fixing query * fixed query * better query --- repositories/FeedbackRepository.ts | 12 ++++++++++-- services/FeedbackService.ts | 2 +- tests/feedback.test.ts | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/repositories/FeedbackRepository.ts b/repositories/FeedbackRepository.ts index fbaa8fe8..aa6250d4 100644 --- a/repositories/FeedbackRepository.ts +++ b/repositories/FeedbackRepository.ts @@ -15,8 +15,16 @@ export class FeedbackRepository extends BaseRepository { return this.getBaseFindQuery({}).where({ uuid }).getOne(); } - public async getAllFeedbackForUser(user: UserModel): Promise { - return this.getBaseFindQuery({}).where({ user }).getMany(); + // temporary fix for getting feedback for a user for an event + public async getStandardUserFeedback(user: UserModel, options: FeedbackSearchOptions): Promise { + let query = this.getBaseFindQuery(options); + query = query.andWhere('feedback.user = :user', { user: user.uuid }); + + return query.getMany(); + } + + public async getAllFeedbackForUser(user: UserModel, options: FeedbackSearchOptions): Promise { + return this.getBaseFindQuery(options).where({ user }).getMany(); } public async upsertFeedback(feedback: FeedbackModel, diff --git a/services/FeedbackService.ts b/services/FeedbackService.ts index 12e23654..f8d5f059 100644 --- a/services/FeedbackService.ts +++ b/services/FeedbackService.ts @@ -25,7 +25,7 @@ export default class FeedbackService { .map((fb) => fb.getPublicFeedback()); } - const userFeedback = await feedbackRepository.getAllFeedbackForUser(user); + const userFeedback = await feedbackRepository.getStandardUserFeedback(user, options); return userFeedback.map((fb) => fb.getPublicFeedback()); }); } diff --git a/tests/feedback.test.ts b/tests/feedback.test.ts index 2c826eed..f2516307 100644 --- a/tests/feedback.test.ts +++ b/tests/feedback.test.ts @@ -164,7 +164,7 @@ describe('feedback submission', () => { const feedbackController = ControllerFactory.feedback(conn); await feedbackController.submitFeedback(buildFeedbackRequest(feedback1), member1); await feedbackController.submitFeedback(buildFeedbackRequest(feedback2), member2); - const user1Feedback = await feedbackController.getFeedback({}, member1); + const user1Feedback = await feedbackController.getFeedback({ user: member1.uuid }, member1); expect(user1Feedback.feedback).toHaveLength(1);