-
Notifications
You must be signed in to change notification settings - Fork 5
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
Changes from all commits
961db7d
cb6864c
0bcfc38
71af1c9
147da54
32daa74
15e2d71
c6db3f5
175af99
7b3a257
c602d86
09c1a0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. u should save the event into the repository after editing the linkedEvent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets also create a unit test to test for this There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'); | ||
} | ||
|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,6 +110,7 @@ export class MerchFactory { | |
orderLimit: FactoryUtils.getRandomNumber(1, 5), | ||
status: OrderPickupEventStatus.ACTIVE, | ||
orders: [], | ||
linkedEvent: null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.