Files
redefined-designs/docs/superpowers/plans/2026-09-03-upload-link-email.md
bermudalambandClaude Opus 5 a9e865b4bc docs(intake): plan emailing the upload link (#260)
Four tasks over the approved design: sendMail gains an outcome, the column and template land together, the route requires an address and reports what the send did, and the admin screen asks for it.

sendMail goes first deliberately. Everything else depends on being able to tell a skipped send from a real one, and it is the only change touching a file seven other things already use — so if it is going to break anything, it should break before three tasks are stacked on top of it.

The plan is explicit that no existing caller changes. Ignoring a returned value is legal, which is what makes widening the return type additive rather than breaking, and re-deriving "would this address be blocked?" in the route would have duplicated isAllowedRecipient and the SMTP check at a second site.

Two specs in unrelated features create links with only a label, and the route will refuse that. They are fixed in the same task as the form rather than left for the suite to find, because the alternative is two unrelated features going red on somebody else's branch. That cost is named in the plan rather than discovered.

One inaccuracy is left in deliberately and said out loud: an SMTP rejection is reported as skipped-unconfigured rather than a fourth outcome. The distinction is real, nothing consumes it, and the admin's next action is identical either way — copy the link and send it by hand.

Self-review caught the failure mode from #281, where tasks referred to helpers that did not exist. Task 4 originally said the created-link state "may not be called created". Reading the component showed it is `issued` and holds a bare URL string with nowhere to put a delivery outcome, so the plan now adds a separate `mailed` state beside it rather than widening the one-time token display. Every name in that task is now one that exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-03 17:32:09 -05:00

33 KiB

Upload Link Email Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Make an email address part of creating an upload link, and send the link to it.

Architecture: One new nullable column, one new editable mail template, and a return value on sendMail so the route can tell the admin whether the mail actually went. The link is created whether or not the send succeeds, and the response says which.

Tech Stack: Express 4 + TypeScript, pg, node-pg-migrate, nodemailer, Jest + supertest, React + antd (antd/es/... deep imports), Playwright.

Spec: docs/superpowers/specs/2026-09-03-upload-link-email-design.md Issue: #260

Global Constraints

  • A failed send never loses the link and never becomes a 500. The link is created, the send attempted, and the outcome reported. The token is shown exactly once, so rolling back on a send failure would discard work that succeeded.
  • mail.outcome is always present, on success as well as failure, and is always one of 'sent' | 'skipped-unconfigured' | 'skipped-blocked'. A field that appears only on failure is one every consumer must remember to check for.
  • No existing sendMail caller changes. There are seven and every one ignores the return value.
  • upload_links.contact_email is nullable. Links already exist in QA; the requirement lives in the route, not the column.
  • antd imports are deep and from es: import Input from 'antd/es/input';. Never import { Input } from 'antd'.
  • Verify the frontend with npm run build, never a bare npx tsc --noEmit — the app tsconfig excludes tests/, and a green bare tsc once broke a deploy here.
  • Branch: feature/260-email-the-upload-link, already created off main, already carrying the spec commit. Commit there. Subjects end (#260). Commit bodies are not hard-wrapped — one long line per paragraph, blank lines between. Write each message to a temporary file and use git commit -F <file>, then delete it. Do not push; the user pushes.
  • End every commit message with: Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
  • Integration tests need the test database: cd backend && npm run db:test:up. Port 55432 is Hyper-V-reserved on this machine; if it will not bind, set TEST_PGPORT rather than editing the compose file.
  • Do NOT run scripts/start-local.ps1, scripts/run-tests.ps1, or any nvm script. They prompt for UAC elevation and have previously stripped Node from this machine. Playwright needs the stack running, which only the user can start.

Task 1: sendMail reports what it did

Files:

  • Modify: backend/src/mailer.ts:88-113
  • Test: backend/tests/unit/mailOutcome.test.ts (create)

Interfaces:

  • Consumes: nothing.
  • Produces:
    • export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';
    • sendMail(to: string, subject: string, html: string): Promise<MailOutcome>

Why first. Every later task depends on being able to tell a skipped send from a real one, and this is the only change that touches a file seven other things already use.

  • Step 1: Write the failing test

Create backend/tests/unit/mailOutcome.test.ts:

import { sendMail } from '../../src/mailer';

/**
 * What sendMail says it did.
 *
 * It returns early in two cases that are indistinguishable from success at the
 * call site — SMTP unconfigured, and the recipient not on MAIL_ALLOWLIST — and
 * #260 needs to tell them apart so the admin is not told a link was emailed
 * when it was not.
 *
 * Only the two skip paths are covered. A real send needs an SMTP server, which
 * a unit test has no business starting; the integration test in Task 4 covers
 * the route's behaviour instead.
 */
describe('what sendMail reports', () => {
  const original = { ...process.env };

  afterEach(() => {
    process.env = { ...original };
  });

  it('says so when SMTP is not configured', async () => {
    delete process.env.SMTP_USER;
    delete process.env.SMTP_PASSWORD;

    await expect(sendMail('someone@example.com', 'subject', '<p>body</p>')).resolves.toBe(
      'skipped-unconfigured'
    );
  });

  // The case that matters most: QA restricts delivery, and a blocked address
  // previously returned exactly as though it had sent.
  it('says so when the recipient is not on the allowlist', async () => {
    process.env.SMTP_USER = 'user';
    process.env.SMTP_PASSWORD = 'password';
    process.env.MAIL_ALLOWLIST = 'allowed@example.com';

    await expect(sendMail('someone-else@example.com', 'subject', '<p>body</p>')).resolves.toBe(
      'skipped-blocked'
    );
  });

  it('does not report blocked for an address that is on the allowlist', async () => {
    process.env.SMTP_USER = 'user';
    process.env.SMTP_PASSWORD = 'password';
    process.env.MAIL_ALLOWLIST = 'allowed@example.com';

    // Not asserting 'sent': that would need a live SMTP server. Asserting only
    // that the allowlist did not refuse it, which is this test's subject.
    await expect(
      sendMail('allowed@example.com', 'subject', '<p>body</p>').catch(() => 'threw')
    ).resolves.not.toBe('skipped-blocked');
  });
});
  • Step 2: Run it to verify it fails
cd backend && npx jest -c jest.unit.config.js tests/unit/mailOutcome.test.ts

Expected: FAIL — sendMail resolves to undefined, not 'skipped-unconfigured'.

  • Step 3: Write the implementation

In backend/src/mailer.ts, add the type above sendMail:

/**
 * What a send attempt actually did.
 *
 * `sendMail` returns early in two cases that used to be indistinguishable from
 * success — no SMTP credentials, and a recipient outside MAIL_ALLOWLIST — which
 * meant a caller could report "emailed" for a message nobody would ever
 * receive. QA restricts delivery by design, so that was not a hypothetical: it
 * is the normal case there. See #260.
 */
export type MailOutcome = 'sent' | 'skipped-unconfigured' | 'skipped-blocked';

Then change the signature and the three exits, leaving every existing comment in place:

export async function sendMail(to: string, subject: string, html: string): Promise<MailOutcome> {
  if (!process.env.SMTP_USER || !process.env.SMTP_PASSWORD) {
    console.warn(`SMTP not configured — skipping email to ${to}: "${subject}"`);
    return 'skipped-unconfigured';
  }
  if (!isAllowedRecipient(to, process.env.MAIL_ALLOWLIST)) {
    console.warn(`[mail-blocked] ${to} is not on MAIL_ALLOWLIST — skipping "${subject}"`);
    return 'skipped-blocked';
  }

  await transporter.sendMail({
    from: process.env.SMTP_FROM || process.env.SMTP_USER,
    to,
    subject,
    html
  });
  return 'sent';
}

Change nothing else in this file, and no call site. All seven callers ignore the return value, which is legal — that is what makes this additive rather than breaking.

  • Step 4: Run the test and the whole unit suite
cd backend && npx jest -c jest.unit.config.js tests/unit/mailOutcome.test.ts && npm run test:unit && npm run build && npm run lint

Expected: PASS, 3 tests; the full unit suite still green; clean build and lint. The build is what proves no caller broke.

  • Step 5: Commit
git add backend/src/mailer.ts backend/tests/unit/mailOutcome.test.ts
git commit -F- <<'EOF'
feat(mail): have sendMail say what it actually did (#260)

It returned Promise<void> and returned early in two cases that were indistinguishable from success at the call site: no SMTP credentials, and a recipient outside MAIL_ALLOWLIST. A caller could therefore report that it had emailed somebody a message nobody would ever receive, and in QA — which restricts delivery deliberately, as its entire safety property — that is the normal case rather than an edge one.

It now returns a MailOutcome saying which of the three happened. No existing caller changes: there are seven and every one ignores the result, so this is additive. Re-deriving the answer at a second site would have duplicated isAllowedRecipient and the SMTP check, which is exactly the drift the guard-in-one-place comment above them exists to prevent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EOF

Task 2: The column and the template

Files:

  • Create: backend/migrations/1787700000000_add-upload-link-contact-email.js
  • Modify: backend/src/db-drizzle/schema.ts (the uploadLinks block)
  • Modify: backend/src/emailTemplates.ts (the TemplateKey union and TEMPLATES)
  • Test: backend/tests/unit/uploadLinkTemplate.test.ts (create)

Interfaces:

  • Consumes: nothing from Task 1.

  • Produces: upload_links.contact_email TEXT (nullable); the 'uploadLink' TemplateKey with required: ['submitUrl'] and available: ['submitUrl', 'label', 'submissionsAllowed'].

  • Step 1: Write the failing test

Create backend/tests/unit/uploadLinkTemplate.test.ts:

import { TEMPLATES, missingPlaceholders, renderTemplate } from '../../src/emailTemplates';

/**
 * The template that carries an upload link (#260).
 *
 * The one failure worth making impossible is a link email with no link in it:
 * it sends, it looks fine in the log, and it is useless to the person who gets
 * it. That is the same guard `verification` has on `verifyUrl`.
 */
describe('the upload link template', () => {
  it('is offered in the admin like the others', () => {
    expect(TEMPLATES.uploadLink.label).toBeTruthy();
  });

  it('requires the submit link', () => {
    expect(TEMPLATES.uploadLink.required).toContain('submitUrl');
  });

  it('refuses a body that has no link in it', () => {
    expect(missingPlaceholders('uploadLink', 'Hello, a link is on its way.')).toEqual(['submitUrl']);
  });

  it('accepts a body that has one', () => {
    expect(missingPlaceholders('uploadLink', 'Send your items: {{submitUrl}}')).toEqual([]);
  });

  it('offers the label and the allowance as placeholders', () => {
    expect(TEMPLATES.uploadLink.available).toEqual(
      expect.arrayContaining(['submitUrl', 'label', 'submissionsAllowed'])
    );
  });

  // The default body has to satisfy the guard it declares, or the feature ships
  // unable to send its own default.
  it('has a default body that satisfies its own requirement', () => {
    expect(missingPlaceholders('uploadLink', TEMPLATES.uploadLink.defaultBody)).toEqual([]);
  });

  it('renders the link into the body', () => {
    const rendered = renderTemplate(
      'uploadLink',
      { subject: 'Send us your items', body: 'Here: {{submitUrl}}' },
      { submitUrl: 'https://example.com/submit/abc', label: 'Sarah', submissionsAllowed: '25 items' }
    );

    expect(rendered.html).toContain('https://example.com/submit/abc');
  });
});

Read renderTemplate's real signature in backend/src/emailTemplates.ts before writing this — the third argument is the placeholder values, and the second is the stored subject/body. Adjust the call above to match it exactly rather than guessing.

  • Step 2: Run it to verify it fails
cd backend && npx jest -c jest.unit.config.js tests/unit/uploadLinkTemplate.test.ts

Expected: FAIL — TEMPLATES.uploadLink is undefined.

  • Step 3: Add the template

In backend/src/emailTemplates.ts, add to the TemplateKey union:

  | 'intakeDraft'
  | 'uploadLink';

And add to TEMPLATES, following the shape of intakeDraft:

  uploadLink: {
    label: 'Upload link for a contributor',
    // The link itself, for the same reason verification requires verifyUrl: an
    // email inviting somebody to send in photos, with no way to do it, sends
    // perfectly happily and wastes everyone's time.
    required: ['submitUrl'],
    available: ['submitUrl', 'label', 'submissionsAllowed'],
    defaultSubject: 'Send us your items',
    defaultBody:
      'You can send us photos of items you would like us to sell.\n\n' +
      '[Send in an item]({{submitUrl}})\n\n' +
      'You can send {{submissionsAllowed}}. Photograph one item at a time, and tell us anything you know about it — where it came from, what it is made of, any damage. A photo cannot show any of that.\n\n' +
      'Keep this link to yourself: anyone who has it can send us items in your name.'
  }
  • Step 4: Write the migration

Create backend/migrations/1787700000000_add-upload-link-contact-email.js:

exports.up = (pgm) => {
  pgm.sql(`
    -- Where the link was sent (#260). Nullable, and deliberately so: links
    -- already exist in QA and a migration cannot invent an address for them, so
    -- they are grandfathered rather than backfilled with something untrue.
    --
    -- The requirement lives in the create route instead, which is where new
    -- links are actually made. A NOT NULL column would have forced a choice
    -- between inventing data and refusing to migrate.
    ALTER TABLE upload_links
      ADD COLUMN IF NOT EXISTS contact_email TEXT;
  `);
};

exports.down = (pgm) => {
  pgm.sql(`ALTER TABLE upload_links DROP COLUMN IF EXISTS contact_email;`);
};
  • Step 5: Update the Drizzle mirror

backend/src/db-drizzle/schema.ts is generated by drizzle-kit pull, and drizzleSchema.integration.test.ts asserts it declares every column of every table. The local dev database is probably not running, so add the line by hand in the uploadLinks block, matching the file's tab indentation and generated style:

	contactEmail: text("contact_email"),

text is already imported — do not add an import.

  • Step 6: Run the tests
cd backend && npm run db:test:up && npx jest -c jest.unit.config.js tests/unit/uploadLinkTemplate.test.ts && npm run test:integration -- drizzleSchema && npm run build && npm run lint

Expected: PASS. The Drizzle guard is what proves the mirror is not stale.

  • Step 7: Commit
git add backend/migrations/1787700000000_add-upload-link-contact-email.js backend/src/db-drizzle/schema.ts backend/src/emailTemplates.ts backend/tests/unit/uploadLinkTemplate.test.ts
git commit -F- <<'EOF'
feat(intake): record where an upload link was sent, and how to say it (#260)

Adds upload_links.contact_email and the uploadLink mail template.

The column is nullable on purpose. Links already exist in QA and a migration cannot invent addresses for them, so they are grandfathered rather than backfilled with something untrue; the requirement belongs in the create route, which is where new links are actually made.

The template requires submitUrl, the same guard verification has on verifyUrl. An email inviting somebody to send in photos, with no way for them to do it, sends perfectly happily and looks fine in the log — it is the one failure here worth making impossible, and a test asserts the default body satisfies the guard it declares.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EOF

Task 3: The route requires an address and sends the mail

Files:

  • Modify: backend/src/routes/adminUploadLinks.ts (LINK_SELECT at :29, the create route at :59)
  • Test: backend/tests/integration/uploadLinks.integration.test.ts

Interfaces:

  • Consumes: MailOutcome and sendMail from Task 1; the column and template from Task 2; isValidEmail from ../utils; renderTemplate from ../emailTemplates; loadStoredTemplate from ./adminEmailTemplates.

  • Produces: POST /api/admin/upload-links requires email; responds 201 { ...link, token, url, mail: { sent: boolean, outcome: MailOutcome } }. GET returns contact_email on every row.

  • Step 1: Write the failing test

Append to backend/tests/integration/uploadLinks.integration.test.ts, following the arrangement its existing cases use. Read them first — they use request(app) directly.

describe('emailing the link to its recipient', () => {
  const original = { ...process.env };

  afterEach(() => {
    process.env = { ...original };
  });

  it('refuses to create a link with no address', async () => {
    const res = await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Sarah' });

    expect(res.status).toBe(400);
    expect(res.body.error).toMatch(/email/i);
  });

  it('refuses an address that is not one', async () => {
    const res = await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Sarah', email: 'not-an-address' });

    expect(res.status).toBe(400);
    expect(res.body.error).toMatch(/email/i);
  });

  it('stores the address on the link', async () => {
    const res = await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Sarah', email: 'sarah@example.com' });

    expect(res.status).toBe(201);

    const { rows } = await pool.query<{ contact_email: string }>(
      `SELECT contact_email FROM upload_links WHERE id = $1`,
      [res.body.id]
    );
    expect(rows[0]?.contact_email).toBe('sarah@example.com');
  });

  // The point of the whole change: QA blocks delivery by design, so a link that
  // was not actually emailed must not be reported as though it was.
  it('says the mail was not sent when SMTP is not configured', async () => {
    delete process.env.SMTP_USER;
    delete process.env.SMTP_PASSWORD;

    const res = await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Sarah', email: 'sarah@example.com' });

    expect(res.status).toBe(201);
    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-unconfigured' });
  });

  it('says the mail was not sent when the address is not allowlisted', async () => {
    process.env.SMTP_USER = 'user';
    process.env.SMTP_PASSWORD = 'password';
    process.env.MAIL_ALLOWLIST = 'someone@example.com';

    const res = await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Sarah', email: 'sarah@example.com' });

    expect(res.status).toBe(201);
    expect(res.body.mail).toEqual({ sent: false, outcome: 'skipped-blocked' });
  });

  // A send that could not happen must never cost the admin the link, because
  // the token is shown exactly once and a rollback would hand them a different
  // one on the retry.
  it('still returns a usable link when the mail did not go', async () => {
    delete process.env.SMTP_USER;

    const res = await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Sarah', email: 'sarah@example.com' });

    expect(res.body.token).toBeTruthy();
    expect(res.body.url).toContain(res.body.token);
    expect(res.body.mail.sent).toBe(false);
  });

  it('lists the address, and tolerates a link that has none', async () => {
    await pool.query(`INSERT INTO upload_links (label, token_hash) VALUES ('Older link', 'digest')`);
    await request(app)
      .post('/api/admin/upload-links')
      .send({ label: 'Newer link', email: 'sarah@example.com' });

    const res = await request(app).get('/api/admin/upload-links');

    const older = res.body.find((row: { label: string }) => row.label === 'Older link');
    const newer = res.body.find((row: { label: string }) => row.label === 'Newer link');
    expect(older.contact_email).toBeNull();
    expect(newer.contact_email).toBe('sarah@example.com');
  });
});
  • Step 2: Run it to verify it fails
cd backend && npm run test:integration -- uploadLinks

Expected: FAIL — links are created without an address and there is no mail in the response.

  • Step 3: Write the implementation

In backend/src/routes/adminUploadLinks.ts, add the imports:

import { sendMail } from '../mailer';
import { renderTemplate } from '../emailTemplates';
import { loadStoredTemplate } from './adminEmailTemplates';
import { isValidEmail } from '../utils';

Add contact_email to LINK_SELECT and to the UploadLinkRow interface:

const LINK_SELECT = `
  SELECT id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at
  FROM upload_links
`;
interface UploadLinkRow {
  id: number;
  label: string;
  contact_email: string | null;
  revoked_at: string | null;
  submission_count: number;
  max_submissions: number | null;
  last_used_at: string | null;
  created_at: string;
}

Add a helper above the create route:

/**
 * What the mail tells the recipient about how much they may send.
 *
 * Words rather than a bare number for an uncapped link, so the sentence reads
 * as a sentence instead of showing an empty space where a figure should be.
 * An uncapped link is a deliberate choice the admin already had to make, so it
 * is emailable like any other.
 */
function submissionsAllowed(maxSubmissions: number | null): string {
  if (maxSubmissions === null) return 'as many items as you like';
  return maxSubmissions === 1 ? '1 item' : `${maxSubmissions} items`;
}

In the create route, validate the address immediately after the label check:

  const email = typeof req.body?.email === 'string' ? req.body.email.trim() : '';
  if (email === '' || !isValidEmail(email)) {
    return res.status(400).json({ error: 'a valid email address is required' });
  }

Store it in the INSERT:

  const { rows } = await pool.query<UploadLinkRow>(
    `INSERT INTO upload_links (label, token_hash, max_submissions, contact_email)
     VALUES ($1, $2, $3, $4)
     RETURNING id, label, contact_email, revoked_at, submission_count, max_submissions, last_used_at, created_at`,
    [label, hashToken(token), maxSubmissions, email]
  );

And replace the final response with the send and the report:

  const url = `${base}/submit/${token}`;

  // Awaited, and its outcome reported rather than swallowed. Every other sender
  // in this codebase fires and forgets because nobody is waiting on the answer;
  // here somebody is — the admin is looking at the screen, and whether they
  // now have to send the link by hand is the thing they need to know.
  //
  // A failure does not roll the link back. The token is shown exactly once, so
  // a rollback would leave the admin retrying and holding a different link,
  // discarding work that succeeded for the sake of tidiness.
  let outcome: MailOutcome = 'skipped-unconfigured';
  try {
    const template = renderTemplate('uploadLink', await loadStoredTemplate('uploadLink'), {
      submitUrl: url,
      label: link.label,
      submissionsAllowed: submissionsAllowed(link.max_submissions)
    });
    outcome = await sendMail(email, template.subject, template.html);
  } catch (err) {
    // Reported, not thrown. The link exists and is usable; the admin needs to
    // be told the mail did not go, not handed a 500 for a link that was made.
    console.error(`[upload-links] could not email ${email}:`, err);
    outcome = 'skipped-unconfigured';
  }

  res.status(201).json({
    ...link,
    token,
    url,
    mail: { sent: outcome === 'sent', outcome }
  });

Import MailOutcome as a type alongside sendMail. Check renderTemplate's real signature and adjust the call to match it exactly.

A note on the catch: an SMTP rejection lands here and is reported as skipped-unconfigured, which is not strictly accurate. Leave it. Adding a fourth outcome for "the server refused it" would be a real distinction, but nothing consumes it and the admin's action is identical either way — copy the link and send it by hand. Say so in the commit rather than inventing the case.

  • Step 4: Run the tests
cd backend && npm run test:integration -- uploadLinks && npx jest -c jest.unit.config.js tests/unit/routesAreWrapped.test.ts && npm run build && npm run lint

Expected: PASS. The wrapper guard confirms no handler was added unwrapped.

  • Step 5: Run the whole backend suite
cd backend && npm run test:unit && npm run test:integration

Expected: everything green. This is what catches any other test that created a link with only a label.

  • Step 6: Commit
git add backend/src/routes/adminUploadLinks.ts backend/tests/integration/uploadLinks.integration.test.ts
git commit -F- <<'EOF'
feat(intake): require an address for an upload link and send the link to it (#260)

Creating a link now requires a valid email address and mails the link to it, which is the whole point: getting a link to a contributor was previously a copy-and-paste into whatever the admin happened to use.

The send is awaited and its outcome reported, unlike every other sender in this codebase, which fires and forgets because nobody is waiting on the answer. Here somebody is. The admin is looking at the screen, and whether they now have to send the link by hand is exactly the thing they need to know — and QA blocks delivery to any address outside MAIL_ALLOWLIST by design, so a link that was never emailed would otherwise look precisely like one that was.

A send that could not happen does not roll the link back. The token is displayed exactly once, so a rollback would leave the admin retrying and holding a different link, discarding work that had succeeded. They end up with a usable link and an honest statement about delivery instead.

One inaccuracy left deliberately: an SMTP rejection is reported as skipped-unconfigured rather than a fourth outcome of its own. The distinction is real but nothing consumes it, and the admin's next action is identical either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EOF

Task 4: The admin screen, and the callers this breaks

Files:

  • Modify: frontend/src/admin/UploadLinks.tsx
  • Modify: frontend/tests/e2e/admin-upload-links.spec.ts
  • Modify: frontend/tests/e2e/intake-submit.spec.ts:18, frontend/tests/e2e/admin-draft-queue.spec.ts:16

Interfaces:

  • Consumes: the route from Task 3.
  • Produces: nothing later tasks depend on.

Why the last three files are here. Both specs create a link with api.post('/api/admin/upload-links', { data: { label } }), which the route now refuses. They are not incidental damage — they are the cost of the requirement, and leaving them for the suite to find later would mean two unrelated features going red.

  • Step 1: Add the field to the form

In frontend/src/admin/UploadLinks.tsx, add state beside label (around line 38):

  const [email, setEmail] = useState('');

Add the input after the label input (around line 105), matching the surrounding style and using a deep antd import if Input is not already imported:

        <Input
          aria-label="Contributor email"
          placeholder="Where should the link be sent?"
          value={email}
          onChange={(e) => setEmail(e.target.value)}
        />

Send it (around line 68):

        label,
        email,
        maxSubmissions: unlimited ? null : Number(cap)

Disable the button until both are filled (around line 120):

disabled={label.trim() === '' || email.trim() === ''}

Reset it in the create handler, beside the existing setLabel(''):

    setIssued(created.url);
    setLabel('');
    setEmail('');
    setCap(DEFAULT_CAP);

Add the column to the table (around line 153):

          { title: 'Label', dataIndex: 'label' },
          { title: 'Sent to', dataIndex: 'contact_email' },
  • Step 2: Tell the admin when the mail did not go

The component holds the created link as const [issued, setIssued] = useState<string | null>(null) and sets it with setIssued(created.url) — a bare URL string, with nowhere to put a delivery outcome. Add a second piece of state beside it rather than widening issued, so the existing one-time-token display is untouched:

  // Whether the link that is currently on screen was actually emailed. Separate
  // from `issued` so the one-time display of the token keeps working exactly as
  // it did; this only adds a note beside it.
  const [mailed, setMailed] = useState<boolean | null>(null);

Extend UploadLink with the new column, so the table can show it:

interface UploadLink {
  id: number;
  label: string;
  contact_email: string | null;
  revoked_at: string | null;

In the create handler, set it from the response alongside setIssued:

    const created = await res.json();
    setIssued(created.url);
    setMailed(created.mail.sent);

And reset it wherever a new create begins, beside setError(null), so a previous link's outcome cannot be read as this one's:

    setMailed(null);

Then render the warning next to wherever issued is displayed. The link stays on screen regardless — that is the point of not rolling back:

      {issued && mailed === false && (
        <Alert
          type="warning"
          showIcon
          message="The link was not emailed"
          description="Copy it below and send it yourself. This is normal where no mail is configured, and in QA, where delivery is restricted to a fixed list of addresses."
        />
      )}

Alert is imported with a deep import if it is not already: import Alert from 'antd/es/alert';. Check the file's existing imports first — several antd components are already imported there.

  • Step 3: Fix the two specs that create links without an address

In frontend/tests/e2e/intake-submit.spec.ts (line 18) and frontend/tests/e2e/admin-draft-queue.spec.ts (line 16), add an address to the data object:

    data: { label: `Intake spec ${RUN}`, email: `intake-${RUN}@example.com` }

Use the label each file already builds and an address carrying the same run id, so a failure names the run that caused it. @example.com is reserved for exactly this and cannot reach a real person.

  • Step 4: Update the upload-links spec

In frontend/tests/e2e/admin-upload-links.spec.ts, every test that fills Link label must now also fill Contributor email before the create button is enabled. Add to each of the three:

    await page.getByLabel('Contributor email').fill(`${uniqueSuffix()}@example.com`);

And add one case asserting the requirement, inside the existing describe:

  // The address is the point of the change: a link nobody can be sent is the
  // thing this replaced.
  test('will not create a link without an address', async ({ page, admin }) => {
    await admin.open('Upload Links');
    await page.getByLabel('Link label').fill(`Nameless ${uniqueSuffix()}`);

    await expect(page.getByRole('button', { name: 'Create link' })).toBeDisabled();
  });

Read the file first: the create button's accessible name may not be Create link, and admin.open's argument must match the real tab name. Use what is there.

  • Step 5: Verify the frontend
cd frontend && npm run build && npm run lint && npm run test:unit

npm run build, not a bare npx tsc --noEmit — the app tsconfig excludes tests/.

Expected: clean build, clean lint, unit tests pass.

  • Step 6: Run the e2e suite

The local stack must be running. Do not start it yourself — ask the user.

cd frontend && npx playwright test admin-upload-links intake-submit admin-draft-queue --project=chromium

Then the whole suite, which is what finds anything else that created a link with only a label:

cd frontend && npx playwright test --project=chromium

Expected: all green.

  • Step 7: Commit
git add frontend/src/admin/UploadLinks.tsx frontend/tests/e2e/admin-upload-links.spec.ts frontend/tests/e2e/intake-submit.spec.ts frontend/tests/e2e/admin-draft-queue.spec.ts
git commit -F- <<'EOF'
feat(admin): ask for the contributor's address when creating a link (#260)

The address is now a required field beside the label, the links table shows where each link was sent, and the admin is told plainly when the mail did not go — with the link still on screen to copy, which is the case that matters in QA and in local development where there is no mail at all.

Two specs in unrelated features created links with only a label and the route now refuses that, so they are updated here rather than left to go red on somebody else's branch. That is the cost of making the address required, and it is a small one: the compiler and the suite find every call site.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EOF

After the plan

  • The branch is feature/260-email-the-upload-link. Do not push — the user pushes and merges.
  • The PR closes #260 and should say plainly that the mail carries the working token, and why that trade was judged acceptable: an upload link grants only "submit into a queue a person must approve", bounded by max_submissions and #227's ceiling.
  • Per standing practice, follow with a separate SonarQube cleanup issue and PR — never folded into this branch.
  • Testing this in QA needs care. MAIL_ALLOWLIST is hardcoded to a single address in docker-compose.qa.yml and is deliberately not a stack variable, because it is the entire safety property stopping a QA run emailing real people. Use that address or a +suffix variant of it. Any other address will report skipped-blocked, which is the feature working correctly.