Pull request review

Works like GitHub pull requests. Review a teammate's pull request: find the real problems among the tidy-ups, comment on the lines, and make the call.

Your third week as a software engineer at Lakeshore Family Health. Amara Ramirez asked QuickCruit Agent for a change, it opened a pull request, and you're the reviewer. (A made-up company. Start with the conversation and the checks.)

QuickCruit Code Review

Text customers a reminder before their booking #178

OpenQuickCruit AgentAI agent wants to merge 1 commit into main from agent/booking-reminders on behalf of aramirez

QuickCruit AgentAI agent opened this pull request on behalf of aramirez 3 hours agoAuthor

Summary

Adds the reminder texts product asked for. Every run picks up bookings in the next 24 hours and texts up to five customers, so we stay under the SMS rate limit. The message now uses the customer's first name.

Changes

src/reminders/schedule.ts: Send up to five reminders per run and type sendReminders.
src/reminders/text.ts: Greet each customer by first name.
src/sms.ts: Add types to sendSms.
test/reminders.test.ts: Tests for the per-run limit and for texting every customer.

Testing

Added tests in test/reminders.test.ts; all checks pass locally.

Generated from Amara Ramirez's request.

QuickCruit AgentAI agent added 1 commit on behalf of aramirez 3 hours ago

Text customers a reminder before their bookingf4204b3

QuickCruit AgentAI agent requested a review from you on behalf of aramirez

Some checks were not successful

1 failing and 3 successful checks

lintSuccessful in 16s
typecheckSuccessful in 38s
test (node 20)Failing after 58s
buildSuccessful in 49s

Merging is blockedMain needs an approving review and passing checks.

Commits from 3 hours ago

Text customers a reminder before their bookingQuickCruit AgentAI agent committed on behalf of aramirez 3 hours agof4204b3

0 of 4 files viewed

src/reminders/schedule.ts

@@ -1,13 +1,14 @@

11import { db } from "../db";

22import { sendSms } from "../sms";

33import { reminderText } from "./text";

5const MAX_PER_RUN = 5

5const MAX_PER_RUN = 5;

7export async function sendReminders(now) {

7export async function sendReminders(now: Date): Promise<number> {

88 const due = await db.bookings.dueWithin(now, { hours: 24 });

9 for (const booking of due) {

9 const batch = due.slice(0, MAX_PER_RUN - 1);

10 for (const booking of batch) {

1011 await sendSms(booking.customer.phone, reminderText(booking));

1112 }

12 return due.length;

13 return batch.length;

1314}

src/reminders/text.ts

@@ -1,9 +1,11 @@

11import type { Booking } from "../types";

2import { formatDate } from "../dates";

34/**

45 * customer.profile is null until the customer finishes sign-up.

56 * Guest bookings never have one.

67 */

78export function reminderText(booking: Booking): string {

8 return `Reminder: ${booking.service} on ${booking.date}`

9 const name = booking.customer.profile.firstName;

10 return `Hi ${name}, a reminder: ${booking.service} on ${formatDate(booking.date)}.`;

911}

src/sms.ts

@@ -3,4 +3,4 @@ import { client, SENDER } from "./sms-client";

4export async function sendSms(to, body) {

5 return client.messages.create({ to, from: SENDER, body });

4export async function sendSms(to: string, body: string): Promise<void> {

5 await client.messages.create({ to, from: SENDER, body });

66}

test/reminders.test.ts

@@ -0,0 +1,16 @@

1import { describe, expect, it } from "vitest";

2import { sendReminders } from "../src/reminders/schedule";

3import { seedBookings } from "./helpers";

5describe("sendReminders", () => {

6 it("sends at most five reminders per run", async () => {

7 await seedBookings(8, { hoursAway: 20 });

8 const sent = await sendReminders(new Date());

9 expect(sent).toBe(5);

10 });

12 it("texts every customer with a booking tomorrow", async () => {

13 await seedBookings(3, { hoursAway: 20, guest: true });

14 const sent = await sendReminders(new Date());

15 });

16});

0 pending comments

Lakeshore Family Health

#178Review requested

Review the pull request Amara asked for

Lucas RamirezEngineering Lead, Lakeshore Family Health

Amara asked QuickCruit Agent for this change, and it opened #178. Amara wants it merged today and asked you to review it. Read every file, comment on any line that has to change before it ships, then submit your review with the verdict you'd stand behind.

Style isn't worth holding a merge for. If you mention it, mark it as a nit.

When the agent says all checks pass, that's a claim to verify, not evidence.

Lakeshore Family Health and everyone here are made up. A practice case, built for you.

Read the description and the failing check's log
Mark every file viewed
Comment on the lines that have to change
Submit your review with a verdict

Your review gets a read here once you submit it.