Skip to content

Commit

Permalink
Merge branch 'master' into bugfix/mock-lowercased-emails
Browse files Browse the repository at this point in the history
  • Loading branch information
maxwn04 authored Mar 7, 2024
2 parents 04790f7 + 9aac2d1 commit e58115f
Show file tree
Hide file tree
Showing 13 changed files with 350 additions and 48 deletions.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@acmucsd/membership-portal",
"version": "3.2.0",
"version": "3.4.1",
"description": "REST API for ACM UCSD's membership portal.",
"main": "index.d.ts",
"files": [
Expand Down Expand Up @@ -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",
Expand All @@ -55,6 +56,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",
Expand All @@ -70,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",
Expand Down
31 changes: 29 additions & 2 deletions repositories/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);
Expand Down Expand Up @@ -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<T>(fn: (transactionalEntityManager: EntityManager) => Promise<T>): Promise<T> {
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<T>(fn: (transactionalEntityManager: EntityManager) => Promise<T>): Promise<T> {
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,
});
}
}
4 changes: 3 additions & 1 deletion services/EventService.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
}
Expand Down
14 changes: 14 additions & 0 deletions services/UserAccountService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<UserModel> {
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions tests/Seeds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,6 @@ async function seed(): Promise<void> {
});
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({
Expand Down Expand Up @@ -542,7 +541,6 @@ async function seed(): Promise<void> {
];
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({
Expand Down Expand Up @@ -591,7 +589,6 @@ async function seed(): Promise<void> {
});
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({
Expand All @@ -601,12 +598,10 @@ async function seed(): Promise<void> {
});
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 = [
Expand Down
58 changes: 27 additions & 31 deletions tests/admin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
1 change: 1 addition & 0 deletions tests/data/DatabaseConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class DatabaseConnection {
'Orders',
'OrderPickupEvents',
'MerchandiseItemOptions',
'MerchandiseItemPhotos',
'MerchandiseItems',
'MerchandiseCollections',
'Attendances',
Expand Down
4 changes: 2 additions & 2 deletions tests/data/EventFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 4 additions & 0 deletions tests/data/FactoryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
}
}
2 changes: 1 addition & 1 deletion tests/data/MerchFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
52 changes: 51 additions & 1 deletion tests/event.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
});
});
Loading

0 comments on commit e58115f

Please sign in to comment.