Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/backend/src/donations/donations.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { DonationItem } from '../donationItems/donationItems.entity';
import { DonationItemsModule } from '../donationItems/donationItems.module';
import { Allocation } from '../allocations/allocations.entity';
import { AllocationModule } from '../allocations/allocations.module';
import { EmailsModule } from '../emails/email.module';

@Module({
imports: [
Expand All @@ -22,6 +23,7 @@ import { AllocationModule } from '../allocations/allocations.module';
forwardRef(() => AuthModule),
DonationItemsModule,
AllocationModule,
EmailsModule,
],
controllers: [DonationsController],
providers: [DonationService, DonationsSchedulerService],
Expand Down
5 changes: 5 additions & 0 deletions apps/backend/src/donations/donations.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import { FoodType } from '../donationItems/types';
import { DonationItemsService } from '../donationItems/donationItems.service';
import { DonationItem } from '../donationItems/donationItems.entity';
import { EmailsService } from '../emails/email.service';

jest.setTimeout(60000);

Expand Down Expand Up @@ -126,6 +127,10 @@ describe('DonationService', () => {
provide: DataSource,
useValue: testDataSource,
},
{
provide: EmailsService,
useValue: { sendEmails: jest.fn() },
},
],
}).compile();

Expand Down
48 changes: 34 additions & 14 deletions apps/backend/src/donations/donations.service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
BadRequestException,
Injectable,
Logger,
NotFoundException,
} from '@nestjs/common';
import { InjectDataSource, InjectRepository } from '@nestjs/typeorm';
Expand All @@ -15,11 +14,11 @@ import { DonationItemsService } from '../donationItems/donationItems.service';
import { ReplaceDonationItemsDto } from '../donationItems/dtos/create-donation-items.dto';
import { DonationItem } from '../donationItems/donationItems.entity';
import { Allocation } from '../allocations/allocations.entity';
import { EmailsService } from '../emails/email.service';
import { emailTemplates } from '../emails/emailTemplates';

@Injectable()
export class DonationService {
private readonly logger = new Logger(DonationService.name);

constructor(
@InjectRepository(Donation) private repo: Repository<Donation>,
@InjectRepository(Allocation)
Expand All @@ -30,6 +29,7 @@ export class DonationService {
private manufacturerRepo: Repository<FoodManufacturer>,
private donationItemsService: DonationItemsService,
@InjectDataSource() private dataSource: DataSource,
private emailsService: EmailsService,
) {}

async findOne(donationId: number): Promise<Donation> {
Expand Down Expand Up @@ -203,13 +203,22 @@ export class DonationService {
break;
}

this.logger.log(`Placeholder for sending automated email`);
const { subject, bodyHTML } =
emailTemplates.fmRecurringDonationReminder({
fmName: donation.foodManufacturer.foodManufacturerName,
});

try {
const fmEmails = [
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should be sending the email to the FMRep's contact email as well as maybe this secondary one? @sam-schu could you confirm?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, it should only be the representative's email!

donation.foodManufacturer.secondaryContactEmail,
].filter((e): e is string => e !== null);

/**
* IMPORTANT: future logic below should only proceed if the email is successfully sent
*/
const emailSent = true;
if (!emailSent) continue;
if (fmEmails.length > 0) {
await this.emailsService.sendEmails(fmEmails, subject, bodyHTML);
}
} catch (e) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should throw InternalServerErrorException to mimic Dalton's existing pattern

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we should throw an exception for email failures in this method, continue/break as the existing placeholder code had is the right thing to do for control flow. The service functions Dalton updated were called by controller methods and the email sending was the last thing done in the function, so the exception would serve to let the caller know that the email failed but wouldn't affect the functionality otherwise because the function was going to return anyway. This function is called by a cron job to send all the reminders for a day, and we want to continue trying to send emails for other donations even if one of them fails. It would be good to log a warning in this case though

continue;
}

dates.splice(i, 1);
i--;
Expand All @@ -225,11 +234,22 @@ export class DonationService {

// cascading recalculation of next dates when replacement dates are also expired
while (nextDate.getTime() <= today.getTime() && occurrences > 0) {
this.logger.log(
`Placeholder for sending automated email for replacement date`,
);
const cascadeEmailSent = true;
if (!cascadeEmailSent) break;
const { subject: cs, bodyHTML: cb } =
emailTemplates.fmRecurringDonationReminder({
fmName: donation.foodManufacturer.foodManufacturerName,
});

try {
const fmEmails = [
donation.foodManufacturer.secondaryContactEmail,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment about email

].filter((e): e is string => e !== null);

if (fmEmails.length > 0) {
await this.emailsService.sendEmails(fmEmails, cs, cb);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we rename cs and cb to subject and bodyHTML for consistency

}
} catch (e) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same as above

break;
}

occurrences -= 1;

Expand Down
70 changes: 70 additions & 0 deletions apps/backend/src/emails/emailTemplates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,74 @@ export const emailTemplates = {
<p>Best regards,<br />The Securing Safe Food Team</p>
`,
}),

fmRecurringDonationReminder: (params: { fmName: string }): EmailTemplate => ({
subject: 'Reminder: Submit Your Scheduled Recurring Donation with SSF',
bodyHTML: `
<p>Hi ${params.fmName},</p>
<p>
This is a friendly reminder from Securing Safe Food that your recurring donation
schedule indicates a new donation submission is due.
</p>
<p>
When you have a moment, please log into your account and submit your current
donation availability so we can continue matching your contributions with pantry requests.
</p>
<p>
We greatly appreciate your continued generosity and support of our mission. Your
recurring donations make a meaningful and consistent impact for the communities we serve.
</p>
<p>Best regards,<br />The Securing Safe Food Team</p>
`,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we add the additional link? The email template doc says:

"Additional Content (Links, Images, etc)
Link to make new donation (otherwise just the login link)"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For this, sorry the details weren't in the ticket, but we should use the resubmitDonationId query param in the link to navigate to the particular donation to resubmit - e.g., the end of the link might look like ?resubmitDonationId=5 if we're sending the reminder for donation 5 (the query param will not do anything yet but is being implemented this week in the frontend)

}),

trackingLinkAvailable: (params: {
pantryName: string;
fmName: string;
trackingLink: string;
volunteerName: string;
volunteerEmail: string;
}): EmailTemplate => ({
subject: `Tracking Information for your ${params.fmName} delivery (Securing Safe Food)`,
bodyHTML: `
<p>Hi ${params.pantryName},</p>
<p>
Good news! Tracking information is now available for your upcoming SSF delivery
from ${params.fmName}. You can use this tracking information to monitor the
status of your shipment or log into your portal for more information on your
expected donation.
</p>
<p>
Tracking Link: <a href="${params.trackingLink}">${params.trackingLink}</a>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Make "Tracking Link" bold

</p>
<p>
You can use the tracking link above to monitor your shipment, or <a href="${EMAIL_REDIRECT_URL}/login">log into your portal</a> for full order details and updates.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure if this should be a link for the "log into your portal", @sam-schu can you clarify?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it doesn't hurt!

</p>
<p>
If you experience any issues or have questions, please contact your coordinator,
${params.volunteerName}, at <a href="mailto:${params.volunteerEmail}">${params.volunteerEmail}</a>, and our team will be happy to assist.
</p>
<p>Best regards,<br />The Securing Safe Food Team</p>
`,
}),

pantryConfirmsOrderDelivery: (params: {
volunteerName: string;
pantryName: string;
fmName: string;
}): EmailTemplate => ({
subject: `${params.pantryName} Confirmed for your ${params.fmName} Order`,
bodyHTML: `
<p>Hi ${params.volunteerName},</p>
<p>
${params.pantryName} has confirmed receipt of the most recent ${params.fmName}
order you are assigned to. Please log into the platform to review the completed
request or check for additional information.
</p>
<p>
Thank you for your coordination and support in helping reach this order to completion!
</p>
<p>Best regards,<br />The Securing Safe Food Team</p>
`,
}),
};
2 changes: 2 additions & 0 deletions apps/backend/src/orders/order.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { DonationItemsModule } from '../donationItems/donationItems.module';
import { Allocation } from '../allocations/allocations.entity';
import { DonationModule } from '../donations/donations.module';
import { Donation } from '../donations/donations.entity';
import { EmailsModule } from '../emails/email.module';

@Module({
imports: [
Expand All @@ -37,6 +38,7 @@ import { Donation } from '../donations/donations.entity';
ManufacturerModule,
DonationItemsModule,
DonationModule,
EmailsModule,
],
controllers: [OrdersController],
providers: [OrdersService],
Expand Down
42 changes: 40 additions & 2 deletions apps/backend/src/orders/order.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import { DonationService } from '../donations/donations.service';
import { ApplicationStatus } from '../shared/types';
import { Donation } from '../donations/donations.entity';
import { VolunteerOrder } from '../volunteers/types';
import { EmailsService } from '../emails/email.service';
import { emailTemplates } from '../emails/emailTemplates';

@Injectable()
export class OrdersService {
Expand All @@ -36,6 +38,7 @@ export class OrdersService {
private allocationsService: AllocationsService,
private donationService: DonationService,
@InjectDataSource() private dataSource: DataSource,
private emailsService: EmailsService,
) {}

// TODO: when order is created, set FM
Expand Down Expand Up @@ -391,6 +394,7 @@ export class OrdersService {
.execute();
}

// Updated confirmDelivery()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't think we need this

async confirmDelivery(
orderId: number,
dto: ConfirmDeliveryDto,
Expand All @@ -403,7 +407,10 @@ export class OrdersService {
throw new BadRequestException('Invalid date format for dateReceived');
}

const order = await this.repo.findOneBy({ orderId });
const order = await this.repo.findOne({
where: { orderId },
relations: ['request', 'request.pantry', 'foodManufacturer', 'assignee'],
});

if (!order) {
throw new NotFoundException(`Order ${orderId} not found`);
Expand All @@ -424,6 +431,18 @@ export class OrdersService {

await this.requestsService.updateRequestStatus(order.requestId);

const { subject, bodyHTML } = emailTemplates.pantryConfirmsOrderDelivery({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In Dalton's PR, he wrapped this logic in a try catch with an InternalServerErrorException, can we mimic that pattern here for consistency

volunteerName: `${order.assignee.firstName} ${order.assignee.lastName}`,
pantryName: order.request.pantry.pantryName,
fmName: order.foodManufacturer.foodManufacturerName,
});

await this.emailsService.sendEmails(
[order.assignee.email],
subject,
bodyHTML,
);

return updatedOrder;
}

Expand Down Expand Up @@ -472,7 +491,10 @@ export class OrdersService {
dto.trackingLink = sanitized;
}

const order = await this.repo.findOneBy({ orderId });
const order = await this.repo.findOne({
where: { orderId },
relations: ['request', 'request.pantry', 'foodManufacturer', 'assignee'],
});
if (!order) {
throw new NotFoundException(`Order ${orderId} not found`);
}
Expand Down Expand Up @@ -504,6 +526,22 @@ export class OrdersService {
) {
order.status = OrderStatus.SHIPPED;
order.shippedAt = new Date();

const { subject, bodyHTML } = emailTemplates.trackingLinkAvailable({
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same thing about try catch

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Given we will be throwing an exception if the email fails, we should make sure to send the email after saving the order to the database so we still update the order even if the email fails

pantryName: order.request.pantry.pantryName,
fmName: order.foodManufacturer.foodManufacturerName,
trackingLink: order.trackingLink,
volunteerName: `${order.assignee.firstName} ${order.assignee.lastName}`,
volunteerEmail: order.assignee.email,
});

const pantryEmails = [order.request.pantry.secondaryContactEmail].filter(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment here about email, I could be wrong though

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yes should be pantry representative email again!

(e): e is string => e !== null,
);

if (pantryEmails.length > 0) {
await this.emailsService.sendEmails(pantryEmails, subject, bodyHTML);
}
}

await this.repo.save(order);
Expand Down
Loading