Merge pull request 'feat: tell favoriters when an item is withdrawn (#34)' (#54) from feature/favorites into main

Reviewed-on: #54

backend/src/favoriteAlerts.ts

@@ -7,11 +7,51 @@ import { sendMail } from './mailer';
 export const FAVORITE_ALERTS_CONSENT_TEXT =
   'Email me when an item I have favorited is sold to someone else, so I know it is no longer available.';
 
-interface Recipient {
+export interface FavoriteRecipient {
   email: string;
   item_name: string;
 }
 
+// Gathered separately from sending because deleting an item cascades its
+// favorites away: the recipients have to be read *before* the row goes, while
+// the send has to happen *after*, so nobody is told about a withdrawal that
+// then failed.
+export async function collectFavoriteRecipients(
+  itemIds: number[],
+  excludeCustomerId: number | null,
+  onlyUnsold = false
+): Promise<FavoriteRecipient[]> {
+  if (!itemIds.length) return [];
+
+  const { rows } = await pool.query<FavoriteRecipient>(
+    `SELECT c.email, i.name AS item_name
+     FROM favorites f
+     JOIN customers c ON c.id = f.customer_id
+     JOIN items i ON i.id = f.item_id
+     WHERE f.item_id = ANY($1::int[])
+       AND c.favorite_alerts = true
+       AND c.disabled_at IS NULL
+       AND ($2::int IS NULL OR c.id <> $2::int)
+       AND ($3::boolean = false OR i.status <> 'sold')`,
+    [itemIds, excludeCustomerId, onlyUnsold]
+  );
+  return rows;
+}
+
+// One message per item per person, never batched: each is about a specific
+// thing the customer asked to hear about. Sent independently so one bad
+// address cannot stop the rest — and whatever prompted this has already
+// happened regardless of whether the mail goes out.
+function send(recipients: FavoriteRecipient[], subject: (name: string) => string, body: (name: string) => string): void {
+  for (const recipient of recipients) {
+    sendMail(recipient.email, subject(recipient.item_name), body(recipient.item_name))
+      .catch(err => console.error('favorite alert failed', err));
+  }
+}
+
+const FOOTER = `<p>You are receiving this because you asked to be told when a favorited item becomes
+   unavailable. You can turn these off on your account page.</p>`;
+
 // Called *after* the sale has been committed, never inside the transaction.
 // Emailing about a sale that then rolled back would be worse than a late
 // notification, and the transaction should not be held open for SMTP.
@@ -19,32 +59,28 @@ interface Recipient {
 // `buyerId` is excluded: telling customers the item they just bought is no
 // longer available reads as a bug.
 export async function notifyFavoritersOfSale(itemIds: number[], buyerId: number | null): Promise<void> {
-  if (!itemIds.length) return;
+  const recipients = await collectFavoriteRecipients(itemIds, buyerId);
-
+  send(
-  const { rows } = await pool.query<Recipient>(
+    recipients,
-    `SELECT c.email, i.name AS item_name
+    name => `"${name}" has been sold`,
-     FROM favorites f
+    name => `<p>An item you favorited has been sold to another customer, so it is no longer available.</p>
-     JOIN customers c ON c.id = f.customer_id
+       <p><b>${name}</b></p>
-     JOIN items i ON i.id = f.item_id
-     WHERE f.item_id = ANY($1::int[])
-       AND c.favorite_alerts = true
-       AND c.disabled_at IS NULL
-       AND ($2::int IS NULL OR c.id <> $2::int)`,
-    [itemIds, buyerId]
-  );
-
-  for (const recipient of rows) {
-    // Sent one at a time and independently: one failed address must not stop
-    // the rest, and the sale itself has already succeeded regardless.
-    sendMail(
-      recipient.email,
-      `"${recipient.item_name}" has been sold`,
-      `<p>An item you favorited has been sold to another customer, so it is no longer available.</p>
-       <p><b>${recipient.item_name}</b></p>
        <p>Every piece is one of a kind, so this one will not be restocked. You can browse what is
           still available at <a href="${process.env.PUBLIC_URL}">Redefined Designs</a>.</p>
-       <p>You are receiving this because you asked to be told when a favorited item is sold. You can
+       ${FOOTER}`
-          turn these off on your account page.</p>`
+  );
-    ).catch(err => console.error('favorite sold notification failed', err));
 }
+
+// Sent when an item is withdrawn from sale rather than sold. Recipients must be
+// collected before the delete, since the favorites rows cascade with the item.
+export function notifyFavoritersOfRemoval(recipients: FavoriteRecipient[]): void {
+  send(
+    recipients,
+    name => `"${name}" is no longer available`,
+    name => `<p>An item you favorited has been withdrawn and is no longer available.</p>
+       <p><b>${name}</b></p>
+       <p>You can browse what is still available at
+          <a href="${process.env.PUBLIC_URL}">Redefined Designs</a>.</p>
+       ${FOOTER}`
+  );
 }

backend/src/routes/admin.ts

@@ -8,7 +8,7 @@ import { ADMIN_ITEM_SELECT } from '../itemSelect';
 import { asyncRoute } from '../asyncRoute';
 import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
 import { tagColorFor } from '../utils';
-import { notifyFavoritersOfSale } from '../favoriteAlerts';
+import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecipients } from '../favoriteAlerts';
 
 const router = Router();
 
@@ -229,10 +229,21 @@ router.put('/items/:id', uploadImages, async (req: Request, res: Response) => {
   }
 });
 
-router.delete('/items/:id', async (req: Request, res: Response) => {
+router.delete('/items/:id', asyncRoute(async (req: Request, res: Response) => {
-  await pool.query(`DELETE FROM items WHERE id = $1`, [req.params.id]);
+  const itemId = Number(req.params.id);
+
+  // Collected before the delete: favorites cascade with the item, so after it
+  // is gone there is no record of who was watching. Restricted to unsold items
+  // because anyone watching a sold one has already been told it went.
+  const recipients = await collectFavoriteRecipients([itemId], null, true);
+
+  await pool.query(`DELETE FROM items WHERE id = $1`, [itemId]);
+
+  // Sent only once the delete has succeeded, so nobody hears about a withdrawal
+  // that did not happen.
+  notifyFavoritersOfRemoval(recipients);
   res.status(204).end();
-});
+}));
 
 router.delete('/items/:id/images/:imageId', async (req: Request, res: Response) => {
   await pool.query(`DELETE FROM item_images WHERE id = $1 AND item_id = $2`, [req.params.imageId, req.params.id]);

backend/tests/integration/favorites.integration.test.ts

@@ -43,6 +43,13 @@ function soldNotificationsTo(): string[] {
     .map(call => String(call[0]));
 }
 
+// Recipients of the "no longer available" mail sent when an item is withdrawn.
+function removalNotificationsTo(): string[] {
+  return sentMail.mock.calls
+    .filter(call => String(call[1]).includes('no longer available'))
+    .map(call => String(call[0]));
+}
+
 describe('favoriting items', () => {
   it('records a favorite and lists it back', async () => {
     const itemId = await createItem('Oak table');
@@ -240,3 +247,60 @@ describe('notifying when a favorited item sells', () => {
     expect(soldNotificationsTo()).toEqual([]);
   });
 });
+
+describe('notifying when a favorited item is deleted', () => {
+  it('emails opted-in favoriters that it is no longer available', async () => {
+    const itemId = await createItem('Withdrawn item');
+    const { agent } = await register('withdrawn@example.com');
+    await agent.post(`/api/customers/me/favorites/${itemId}`);
+    await agent.put('/api/customers/me/favorite-alerts').send({ enabled: true });
+
+    expect((await request(app).delete(`/api/admin/items/${itemId}`)).status).toBe(204);
+
+    expect(removalNotificationsTo()).toEqual(['withdrawn@example.com']);
+  });
+
+  it('does not email a favoriter who never opted in', async () => {
+    const itemId = await createItem('Withdrawn item');
+    const { agent } = await register('quiet@example.com');
+    await agent.post(`/api/customers/me/favorites/${itemId}`);
+
+    await request(app).delete(`/api/admin/items/${itemId}`);
+
+    expect(removalNotificationsTo()).toEqual([]);
+  });
+
+  it('does not tell them twice when the item had already sold', async () => {
+    const itemId = await createItem('Already sold');
+    const { agent } = await register('twice-told@example.com');
+    await agent.post(`/api/customers/me/favorites/${itemId}`);
+    await agent.put('/api/customers/me/favorite-alerts').send({ enabled: true });
+
+    await request(app).post(`/api/admin/items/${itemId}/mark-sold`);
+    expect(soldNotificationsTo()).toEqual(['twice-told@example.com']);
+
+    // They already know it has gone; deleting the record should not say so again.
+    await request(app).delete(`/api/admin/items/${itemId}`);
+    expect(removalNotificationsTo()).toEqual([]);
+  });
+
+  it('does not notify a disabled customer', async () => {
+    const itemId = await createItem('Withdrawn item');
+    const { agent, id } = await register('disabled-del@example.com');
+    await agent.post(`/api/customers/me/favorites/${itemId}`);
+    await agent.put('/api/customers/me/favorite-alerts').send({ enabled: true });
+    await request(app).post(`/api/admin/customers/${id}/disable`).expect(204);
+
+    await request(app).delete(`/api/admin/items/${itemId}`);
+
+    expect(removalNotificationsTo()).toEqual([]);
+  });
+
+  it('still deletes the item when nobody favorited it', async () => {
+    const itemId = await createItem('Unloved');
+    expect((await request(app).delete(`/api/admin/items/${itemId}`)).status).toBe(204);
+
+    const { rows } = await pool.query(`SELECT COUNT(*)::int AS n FROM items WHERE id = $1`, [itemId]);
+    expect(rows[0].n).toBe(0);
+  });
+});