-
Notifications
You must be signed in to change notification settings - Fork 0
SSF-233 Pantry Confirm Delivery Email #194
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -34,7 +34,8 @@ import { DataSource, EntityManager, In } from 'typeorm'; | |
| import { EmailsService } from '../emails/email.service'; | ||
| import { Allocation } from '../allocations/allocations.entity'; | ||
| import { mock } from 'jest-mock-extended'; | ||
| import { emailTemplates } from '../emails/emailTemplates'; | ||
| import { emailTemplates, EMAIL_REDIRECT_URL } from '../emails/emailTemplates'; | ||
| import { ApplicationStatus } from '../shared/types'; | ||
|
|
||
| // Set 1 minute timeout for async DB operations | ||
| jest.setTimeout(60000); | ||
|
|
@@ -1747,4 +1748,144 @@ ${request.pantry.shipmentAddressCity}, ${request.pantry.shipmentAddressState} ${ | |
| warnSpy.mockRestore(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('sendConfirmDeliveryReminders', () => { | ||
| // Orders eligible for a reminder: shipped, approved pantry, and shipped at | ||
| // least a week ago (matching the service query). | ||
| const eligibleOrders = async (): Promise<Order[]> => | ||
| testDataSource | ||
| .getRepository(Order) | ||
| .createQueryBuilder('order') | ||
| .leftJoinAndSelect('order.request', 'request') | ||
| .leftJoinAndSelect('request.pantry', 'pantry') | ||
| .leftJoinAndSelect('pantry.pantryUser', 'pantryUser') | ||
| .leftJoinAndSelect('order.assignee', 'assignee') | ||
| .leftJoinAndSelect('order.foodManufacturer', 'foodManufacturer') | ||
| .where('order.status = :status', { status: OrderStatus.SHIPPED }) | ||
| .andWhere('pantry.status = :pantryStatus', { | ||
| pantryStatus: ApplicationStatus.APPROVED, | ||
| }) | ||
| .andWhere("order.shippedAt <= NOW() - INTERVAL '7 days'") | ||
| .getMany(); | ||
|
|
||
| const expectedMessageFor = (order: Order) => | ||
| emailTemplates.pantryConfirmDeliveryReminder({ | ||
| pantryName: order.request.pantry.pantryName, | ||
| fmName: order.foodManufacturer.foodManufacturerName, | ||
| confirmDeliveryLink: `${EMAIL_REDIRECT_URL}/pantry-order-management?orderId=${order.orderId}&action=confirm-delivery`, | ||
| volunteerName: `${order.assignee.firstName} ${order.assignee.lastName}`, | ||
| volunteerEmail: order.assignee.email, | ||
| }); | ||
|
|
||
| it('logs a warning and sends no emails when there are no unconfirmed deliveries', async () => { | ||
| await testDataSource.query( | ||
| `UPDATE orders SET status = $1 WHERE status = $2`, | ||
| [OrderStatus.DELIVERED, OrderStatus.SHIPPED], | ||
| ); | ||
| const warnSpy = jest.spyOn(service['logger'], 'warn'); | ||
|
|
||
| await service.sendConfirmDeliveryReminders(); | ||
|
|
||
| expect(warnSpy).toHaveBeenCalledWith( | ||
| expect.stringContaining( | ||
| 'No pantries with unconfirmed deliveries, skipping email sending.', | ||
| ), | ||
| ); | ||
| expect(mockEmailsService.sendEmails).not.toHaveBeenCalled(); | ||
|
|
||
| warnSpy.mockRestore(); | ||
| }); | ||
|
|
||
| it('sends one personalized reminder per unconfirmed order', async () => { | ||
| const warnSpy = jest.spyOn(service['logger'], 'warn'); | ||
| const orders = await eligibleOrders(); | ||
| expect(orders.length).toBeGreaterThan(0); | ||
|
|
||
| await service.sendConfirmDeliveryReminders(); | ||
|
|
||
| expect(mockEmailsService.sendEmails).toHaveBeenCalledTimes(orders.length); | ||
| for (const order of orders) { | ||
| const message = expectedMessageFor(order); | ||
| expect(mockEmailsService.sendEmails).toHaveBeenCalledWith({ | ||
| toEmail: order.request.pantry.pantryUser.email, | ||
| subject: message.subject, | ||
| bodyHtml: message.bodyHTML, | ||
| }); | ||
| } | ||
| expect(warnSpy).not.toHaveBeenCalled(); | ||
|
|
||
| warnSpy.mockRestore(); | ||
| }); | ||
|
|
||
| it('sends a separate reminder for each unconfirmed order, even within the same pantry', async () => { | ||
| const orderRepo = testDataSource.getRepository(Order); | ||
| const existingShippedOrder = await orderRepo.findOne({ | ||
| where: { status: OrderStatus.SHIPPED }, | ||
| }); | ||
| if (!existingShippedOrder) | ||
| throw new Error('Missing existingShippedOrder test object'); | ||
|
|
||
| const before = (await eligibleOrders()).length; | ||
|
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. what is the point of this before variable? can we not just check how many unconfirmed orders we have for this pantry after adding this secondOrder, and then just check the emails service is called for this one? |
||
|
|
||
| // Add a second shipped order to the same request (same pantry), shipped | ||
| // long enough ago to be eligible. | ||
| const secondOrder = orderRepo.create({ | ||
| requestId: existingShippedOrder.requestId, | ||
| foodManufacturerId: existingShippedOrder.foodManufacturerId, | ||
| assigneeId: existingShippedOrder.assigneeId, | ||
| status: OrderStatus.SHIPPED, | ||
| shippedAt: new Date('2024-02-03T08:00:00Z'), | ||
| }); | ||
| await orderRepo.save(secondOrder); | ||
|
|
||
| await service.sendConfirmDeliveryReminders(); | ||
|
|
||
| expect(mockEmailsService.sendEmails).toHaveBeenCalledTimes(before + 1); | ||
| }); | ||
|
|
||
| it('does not send a reminder for an order shipped less than a week ago', async () => { | ||
| const orderRepo = testDataSource.getRepository(Order); | ||
|
|
||
| await testDataSource.query( | ||
| `UPDATE orders SET status = $1 WHERE status = $2`, | ||
| [OrderStatus.DELIVERED, OrderStatus.SHIPPED], | ||
| ); | ||
|
|
||
| const template = await orderRepo.findOne({ | ||
| where: { status: OrderStatus.DELIVERED }, | ||
| }); | ||
| if (!template) throw new Error('Missing order template'); | ||
|
|
||
| const recentOrder = orderRepo.create({ | ||
| requestId: template.requestId, | ||
| foodManufacturerId: template.foodManufacturerId, | ||
| assigneeId: template.assigneeId, | ||
| status: OrderStatus.SHIPPED, | ||
| shippedAt: new Date(), | ||
| }); | ||
| await orderRepo.save(recentOrder); | ||
|
|
||
| await service.sendConfirmDeliveryReminders(); | ||
|
|
||
| expect(mockEmailsService.sendEmails).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('logs a warning and continues when sending a reminder fails', async () => { | ||
| const warnSpy = jest.spyOn(service['logger'], 'warn'); | ||
| mockEmailsService.sendEmails.mockRejectedValueOnce( | ||
| new Error('SES failure'), | ||
| ); | ||
|
|
||
| await expect( | ||
| service.sendConfirmDeliveryReminders(), | ||
| ).resolves.toBeUndefined(); | ||
|
|
||
| expect(mockEmailsService.sendEmails).toHaveBeenCalled(); | ||
| expect(warnSpy).toHaveBeenCalledWith( | ||
| expect.stringContaining('Failed to send confirm delivery reminder to'), | ||
| ); | ||
|
|
||
| warnSpy.mockRestore(); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ import { ApplicationStatus } from '../shared/types'; | |
| import { VolunteerOrder } from '../volunteers/types'; | ||
| import { EmailsService } from '../emails/email.service'; | ||
| import { FoodRequest } from '../foodRequests/request.entity'; | ||
| import { emailTemplates } from '../emails/emailTemplates'; | ||
| import { emailTemplates, EMAIL_REDIRECT_URL } from '../emails/emailTemplates'; | ||
| import { UsersService } from '../users/users.service'; | ||
| import { OrderSummary } from '../pantries/types'; | ||
| import { PantriesService } from '../pantries/pantries.service'; | ||
|
|
@@ -556,6 +556,55 @@ ${request.pantry.shipmentAddressCity}, ${request.pantry.shipmentAddressState} ${ | |
| } | ||
| } | ||
|
|
||
| async sendConfirmDeliveryReminders(): Promise<void> { | ||
| // One reminder per unconfirmed order (status still SHIPPED). The loop stops | ||
| // for an order once it becomes DELIVERED. Reminders only start a week after | ||
| // the order shipped | ||
| const orders = await this.repo | ||
| .createQueryBuilder('order') | ||
| .leftJoinAndSelect('order.request', 'request') | ||
| .leftJoinAndSelect('request.pantry', 'pantry') | ||
| .leftJoinAndSelect('pantry.pantryUser', 'pantryUser') | ||
| .leftJoinAndSelect('order.assignee', 'assignee') | ||
| .leftJoinAndSelect('order.foodManufacturer', 'foodManufacturer') | ||
| .where('order.status = :status', { status: OrderStatus.SHIPPED }) | ||
| .andWhere('pantry.status = :pantryStatus', { | ||
| pantryStatus: ApplicationStatus.APPROVED, | ||
| }) | ||
| .andWhere("order.shippedAt <= NOW() - INTERVAL '7 days'") | ||
| .getMany(); | ||
|
|
||
| if (orders.length === 0) { | ||
| this.logger.warn( | ||
|
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 isnt really something being wrong, can we just make it a logger.log instead? |
||
| 'No pantries with unconfirmed deliveries, skipping email sending.', | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| for (const order of orders) { | ||
| const toEmail = order.request.pantry.pantryUser.email; | ||
| const message = emailTemplates.pantryConfirmDeliveryReminder({ | ||
| pantryName: order.request.pantry.pantryName, | ||
| fmName: order.foodManufacturer.foodManufacturerName, | ||
| confirmDeliveryLink: `${EMAIL_REDIRECT_URL}/pantry-order-management?orderId=${order.orderId}&action=confirm-delivery`, | ||
| volunteerName: `${order.assignee.firstName} ${order.assignee.lastName}`, | ||
| volunteerEmail: order.assignee.email, | ||
| }); | ||
|
|
||
| try { | ||
| await this.emailsService.sendEmails({ | ||
| toEmail, | ||
| subject: message.subject, | ||
| bodyHtml: message.bodyHTML, | ||
| }); | ||
| } catch { | ||
| this.logger.warn( | ||
| `Failed to send confirm delivery reminder to ${toEmail}.`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| async getOrdersByPantry(pantryId: number): Promise<OrderSummary[]> { | ||
| validateId(pantryId, 'Pantry'); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { Injectable, Logger } from '@nestjs/common'; | ||
| import { Cron } from '@nestjs/schedule'; | ||
| import { OrdersService } from './order.service'; | ||
|
|
||
| @Injectable() | ||
| export class OrdersSchedulerService { | ||
| private readonly logger = new Logger(OrdersSchedulerService.name); | ||
|
|
||
| constructor(private readonly ordersService: OrdersService) {} | ||
|
|
||
| // 12 PM on every Monday | ||
| @Cron('0 0 12 * * 1', { timeZone: 'America/New_York' }) | ||
| async handleWeeklyConfirmDeliveryReminder() { | ||
| this.logger.log('Running weekly confirm-delivery reminder cron job'); | ||
| await this.ordersService.sendConfirmDeliveryReminders(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,13 +125,13 @@ const PantryOrderManagement: React.FC = () => { | |
|
|
||
| useEffect(() => { | ||
| const orderIdFromUrl = searchParams.get('orderId'); | ||
| const action = searchParams.get('action'); | ||
| const allOrders = Object.values(statusOrders).flat(); | ||
| if (!orderIdFromUrl || allOrders.length === 0) return; | ||
|
|
||
| const id = Number(orderIdFromUrl); | ||
| const match = allOrders.find((o) => o.orderId === id); | ||
| if (match) { | ||
| setSelectedOrderId(match.orderId); | ||
| // Paginate the containing status to the page that holds this order. | ||
| for (const status of Object.values(OrderStatus)) { | ||
| const sorted = [...statusOrders[status]].sort((a, b) => | ||
|
|
@@ -146,6 +146,16 @@ const PantryOrderManagement: React.FC = () => { | |
| break; | ||
| } | ||
| } | ||
|
|
||
| if ( | ||
| action === 'confirm-delivery' && | ||
| match.status === OrderStatus.SHIPPED | ||
| ) { | ||
| setSelectedActionOrder(match); | ||
| navigate(ROUTES.PANTRY_ORDER_MANAGEMENT, { replace: true }); | ||
|
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. for most of our modals, i think we navigate only on closing, as opposed to opening them and then renavigating. can we follow that logic here? |
||
| } else { | ||
| setSelectedOrderId(match.orderId); | ||
| } | ||
| } else { | ||
| navigate(ROUTES.PANTRY_ORDER_MANAGEMENT, { replace: true }); | ||
| } | ||
|
|
@@ -237,7 +247,10 @@ const PantryOrderManagement: React.FC = () => { | |
| orderId={selectedActionOrder.orderId} | ||
| orderCreatedAt={selectedActionOrder.createdAt} | ||
| isOpen={true} | ||
| onClose={() => setSelectedActionOrder(null)} | ||
| onClose={() => { | ||
| setSelectedActionOrder(null); | ||
| navigate(ROUTES.PANTRY_ORDER_MANAGEMENT, { replace: true }); | ||
| }} | ||
| onSuccess={() => { | ||
| fetchOrders(); | ||
| setAlertMessage('Delivery Confirmed', AlertStatus.INFO); | ||
|
|
||
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 we add a test somewhere that makes sure each of the orders we use to send an email for is SHIPPED?