Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add linkedEvent column to OrderPickupEvent Table #385

Merged
merged 12 commits into from
Jan 20, 2024
9 changes: 9 additions & 0 deletions api/validators/MerchStoreRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
ArrayNotEmpty,
IsNumber,
IsNumberString,
IsOptional,
} from 'class-validator';
import { Type } from 'class-transformer';
import {
Expand Down Expand Up @@ -41,6 +42,7 @@ import {
MerchOrderEdit as IMerchOrderEdit,
OrderPickupEvent as IOrderPickupEvent,
OrderPickupEventEdit as IOrderPickupEventEdit,
Uuid,
} from '../../types';

export class MerchCollection implements IMerchCollection {
Expand Down Expand Up @@ -258,6 +260,10 @@ export class OrderPickupEvent implements IOrderPickupEvent {
@IsDefined()
@Min(1)
orderLimit: number;

@IsOptional()
@IsUUID()
linkedEventUuid?: Uuid;
}

export class OrderPickupEventEdit implements IOrderPickupEventEdit {
Expand All @@ -275,6 +281,9 @@ export class OrderPickupEventEdit implements IOrderPickupEventEdit {

@Min(1)
orderLimit?: number;

@IsUUID()
linkedEventUuid?: Uuid;
}

export class MerchOrderEdit implements IMerchOrderEdit {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<void> {
await queryRunner.addColumn(TABLE_NAME, new TableColumn({
name: COLUMN_NAME,
type: 'uuid',
isNullable: true,
}));
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.dropColumn(TABLE_NAME, COLUMN_NAME);
}
}
8 changes: 7 additions & 1 deletion models/OrderPickupEventModel.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -29,6 +30,10 @@ export class OrderPickupEventModel extends BaseEntity {
@JoinColumn({ name: 'order' })
orders: OrderModel[];

@OneToOne((type) => EventModel, { nullable: true })
@JoinColumn({ name: 'linkedEvent' })
linkedEvent: EventModel;

Comment on lines +33 to +36
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you double check this relation–I found a few different syntax examples online but I believe this establishes the correct one-way relation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

public getPublicOrderPickupEvent(canSeeOrders = false): PublicOrderPickupEvent {
const pickupEvent: PublicOrderPickupEvent = {
uuid: this.uuid,
Expand All @@ -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());
Expand Down
10 changes: 6 additions & 4 deletions repositories/MerchOrderRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export class OrderPickupEventRepository extends BaseRepository<OrderPickupEventM
*/
public async getPastPickupEvents(): Promise<OrderPickupEventModel[]> {
return this.getBaseFindManyQuery()
.where('"end" < :now')
.where('"orderPickupEvent"."end" < :now')
.setParameter('now', new Date())
.getMany();
}
Expand All @@ -140,7 +140,7 @@ export class OrderPickupEventRepository extends BaseRepository<OrderPickupEventM
*/
public async getFuturePickupEvents(): Promise<OrderPickupEventModel[]> {
return this.getBaseFindManyQuery()
.where('"end" >= :now')
.where('"orderPickupEvent"."end" >= :now')
.setParameter('now', new Date())
.getMany();
}
Expand All @@ -157,7 +157,7 @@ export class OrderPickupEventRepository extends BaseRepository<OrderPickupEventM
}

/**
* Make changes to a singke pickup event. Returns the pickup event edited.
* Make changes to a single pickup event. Returns the pickup event edited.
*/
public async upsertPickupEvent(pickupEvent: OrderPickupEventModel, changes?: Partial<OrderPickupEventModel>):
Promise<OrderPickupEventModel> {
Expand All @@ -177,13 +177,15 @@ export class OrderPickupEventRepository extends BaseRepository<OrderPickupEventM
.leftJoinAndSelect('order.user', 'user')
.leftJoinAndSelect('item.option', 'option')
.leftJoinAndSelect('option.item', 'merchItem')
.leftJoinAndSelect('orderPickupEvent.linkedEvent', 'linkedEvent')
.leftJoinAndSelect('merchItem.merchPhotos', 'merchPhotos');
}

private getBaseFindManyQuery(): SelectQueryBuilder<OrderPickupEventModel> {
return this.repository
.createQueryBuilder('orderPickupEvent')
.leftJoinAndSelect('orderPickupEvent.orders', 'order')
.leftJoinAndSelect('order.user', 'user');
.leftJoinAndSelect('order.user', 'user')
.leftJoinAndSelect('orderPickupEvent.linkedEvent', 'linkedEvent');
}
}
44 changes: 34 additions & 10 deletions services/MerchStoreService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
import { MerchandiseItemModel } from '../models/MerchandiseItemModel';
import { OrderModel } from '../models/OrderModel';
import { UserModel } from '../models/UserModel';
import { EventModel } from '../models/EventModel';
import Repositories, { TransactionsManager } from '../repositories';
import { MerchandiseCollectionModel } from '../models/MerchandiseCollectionModel';
import EmailService, { OrderInfo, OrderPickupEventInfo } from './EmailService';
Expand Down Expand Up @@ -1033,23 +1034,38 @@ export default class MerchStoreService {
}

public async createPickupEvent(pickupEvent: OrderPickupEvent): Promise<OrderPickupEventModel> {
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<OrderPickupEventModel> {
return this.transactions.readWrite(async (txn) => {
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u should save the event into the repository after editing the linkedEvent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets also create a unit test to test for this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is called when a pickup event is edited, so only the UUID of the linkedEvent would be changed. I made a call to the upsert function on line 1076, which should be the only call necessary to save this change, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah you are right about that

}

if (updatedPickupEvent.start >= updatedPickupEvent.end) {
throw new UserError('Order pickup event start time must come before the end time');
}
Expand Down Expand Up @@ -1140,6 +1156,14 @@ export default class MerchStoreService {
return pickupEvent.status === OrderPickupEventStatus.ACTIVE;
}

private async getLinkedRegularEvent(uuid: Uuid): Promise<EventModel> {
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
Expand Down
1 change: 1 addition & 0 deletions tests/data/MerchFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export class MerchFactory {
orderLimit: FactoryUtils.getRandomNumber(1, 5),
status: OrderPickupEventStatus.ACTIVE,
orders: [],
linkedEvent: null,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I import the EventFactory and generate an actual dummy event to put in here? Are there any other test cases that could be beneficial for this feature?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't actually think so. I actually don't think it is clean to attach a dummy event everytime we mock a pickupEvent as sometimes pickupEvent doesn't have an actual linkedEvent attached.

I don't believe it is necessary to implement a solution for this now, but for future reference, if there is a new feature that actually depends on linkedEvent, we should come up with a nice way (such as taking an optional boolean flag variable to attach event) to attach dummy linkedEvent to the mocked pickupEvent.

});
return OrderPickupEventModel.merge(fake, substitute);
}
Expand Down
3 changes: 2 additions & 1 deletion tests/merchOrder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,8 @@ 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);
});

Expand Down
1 change: 1 addition & 0 deletions types/ApiRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ export interface OrderPickupEvent {
end: Date;
description: string;
orderLimit: number;
linkedEventUuid?: Uuid;
}

export interface OrderPickupEventEdit extends Partial<OrderPickupEvent> {}
Expand Down
1 change: 1 addition & 0 deletions types/ApiResponses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ export interface PublicOrderPickupEvent {
orders?: PublicOrderWithItems[];
orderLimit?: number;
status: OrderPickupEventStatus;
linkedEvent?: PublicEvent;
}

export interface GetOrderPickupEventsResponse extends ApiResponse {
Expand Down