Skip to content

Tell the story of writing to the people who booked - #2034

Merged
stefan-burke merged 4 commits into
mainfrom
claude/convert-tests-cucumber-bq6mji
Aug 4, 2026
Merged

Tell the story of writing to the people who booked#2034
stefan-burke merged 4 commits into
mainfrom
claude/convert-tests-cucumber-bq6mji

Conversation

@stefan-burke

@stefan-burke stefan-burke commented Aug 4, 2026

Copy link
Copy Markdown
Member

Bulk email had 2,047 lines of tests across nine files and no story at all. It is a real thing an owner does — write one message to everyone booked onto a listing — so it now has one.

The story

specs/attendees/writing-to-the-people-who-booked.feature, 8 scenarios. It follows the owner from the listing's own page, through writing the message, to sending it.

Rule What it says
The message is shown before it goes Writing never sends. The owner sees what they wrote and how many people it would reach, and only then is offered a way to send.
The site only sends for them with a provider of their own Without one they can still write and check a message, and still open it as a draft in their own email app. What needs a provider is the site sending it for them, and the preview says so rather than leaving a button that would not work.
A promotion leaves out anyone who asked not to hear The owner is told how many are being left out, and those people are not written to. A promotion with nobody left is refused rather than sent to no one.
News about a booking still reaches everyone A message that is not a promotion is news about something the person booked, so it reaches them even if they unsubscribed from promotions.
The way in is only offered where it works The listing offers a way to write to its attendees only when somebody left an address, so the owner is never given a link that leads nowhere.

Every step goes through the page the owner is looking at: the way in on the listing, the form it opens, and the Send button on the preview.

Proving it bites. With the unsubscribed filter removed, both promotion scenarios failed. With the Send button always rendered, the no-provider one failed. With the listing's way in always shown, the no-addresses one failed. All three were put back afterwards.

What the story replaced, and what it did not

Two link tests are gone: whether the way in is offered at all, to an owner and only when somebody left an address, is now the story's job.

Three tests I first deleted are back. Coverage failed and was right to: Cucumber runs do not count towards coverage, so deleting them took the only direct cover of three branches with them — the unsubscribed set a promotion is built against, the refusal when it leaves nobody, and the skipped-count the preview only shows when somebody is being left out. Each is now headed by a note saying which story tells the same thing in the owner's terms, and why the direct test still has to exist.

Everything else stays, because the story does not tell it: the manager's view of the same page, the exact wording of every refusal, the draft being cleared, the activity log, the BCC fallback, and the saved-template pages.

Three things the story needed, which stand on their own

The refusal wording lived in the code, not the catalog. The bulk-email flash messages were the last raw strings in a feature handler, so a story could only assert a refusal by copying its words — and a reworded refusal would have left the story green. They now live in the message catalog like every other message the site says, and the send confirmation gets its "1 recipient" / "2 recipients" from ICU instead of building it by hand.

The test browser could not be asked whether a page offers a way somewhere. It could only be told after trying. "Sending is switched off" is a notice and a button nobody can press, and the story proves both. The new question reuses the same button-reading the existing send already did, which is now one shared helper rather than two copies.

Two stories were installing the same watch on outgoing messages. The contact story and this one each stood in for the outside world, remembered every send, and put the real thing back at the end. That is now one helper taking only the answers, which is the part that actually differs.

Two things that nearly slipped through

A rule that said something untrue. The story first claimed that without an email provider the owner is "offered no way to send it". The preview always offers to open the message as a draft in their own email app, whatever provider they have — so a reader would have learned the opposite of the truth, and the story catalog would have published it, with every scenario passing. Caught in review. The rule is now about the site sending for them, which is the part that actually needs a provider, and a new step asserts the draft option is genuinely still there, so the story would notice if that fallback ever disappeared.

A link found by its words. The first version reached the compose page by clicking the first link reading "Email" — and landed on the settings page, because more than one admin link says "Email". It only surfaced because the step checks that the way in really leads where it should. Both readers now find the link by where it goes, not by its words, which is the rule this repository already applies to row arrows.

Checks

precommit passes in full: lint, typecheck, cpd at 0 clones, the copy check, the edge build, and the whole suite at 100% line and branch coverage. The spec suite is 224 scenarios and 1,460 steps, up from 216.

🤖 Generated with Claude Code

https://claude.ai/code/session_01P4fF7zjiMi9bkHG22oFzh1


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added listing-owner messaging for booked attendees, including message previews, delivery, provider validation, drafts, and promotional opt-out handling.
    • Added booking-news delivery support and conditional availability of listing email actions.
  • Improvements

    • Localized bulk-email validation, preview, error, and success messages.
  • Tests

    • Added comprehensive acceptance and integration coverage for recipient selection, unsubscribes, previews, sending, provider behavior, and form actions.

claude added 3 commits August 4, 2026 10:06
A new story, specs/attendees/writing-to-the-people-who-booked.feature (8
scenarios), follows the owner writing one message to everyone booked onto a
listing: reaching it from the listing itself, being shown the message before
anything goes out, and sending it.

The rules it tells:

- The message is shown before it goes, with who it would reach.
- Sending needs an email provider of the owner's own. Without one they can
  still write and check a message, but are offered no way to send it.
- A promotion leaves out anyone who asked not to hear, the owner is told how
  many are being left out, and a promotion with nobody left is refused.
- News about a booking still reaches somebody who unsubscribed from
  promotions.
- The listing offers a way in only when somebody left an address to write to.

Proving it bites: with the unsubscribed filter removed both promotion
scenarios failed; with the Send button always rendered the no-provider one
failed; with the listing's Email action always shown the no-addresses one
failed. All three put back afterwards.

Five direct tests the story now tells are gone, along with the seeding helper
whose only two callers they were. What stays covers what the story cannot: the
manager's view of the same page, the flash wording of every refusal, the
draft being cleared, the activity log, the BCC fallback, and the template
pages.

Two things the story needed, which are improvements in their own right:

- The bulk-email flash messages were the last raw strings in a feature
  handler, so a story could only assert its refusals by copying their wording.
  They now live in the message catalog like every other message, and the send
  confirmation gets its plural from ICU rather than building it by hand.
- The test browser can now be asked whether a page offers any way to post to
  an address, rather than only being told after trying. "Sending is disabled"
  is a rendered notice *and* a button nobody can press, and the story proves
  both. The check reuses the same button-reading `submitFormAt` already did,
  which is now one shared helper.

The way in to the compose page is found by where the link goes, not by its
words: an early version clicked the first link reading "Email" and landed on
the settings page.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4fF7zjiMi9bkHG22oFzh1
The contact story and the new bulk-email one each installed a stand-in for the
outside world, remembered every send, and put the real thing back at the end —
the same three lines twice, which jscpd rightly flagged. `watchesOutgoing`
takes the answers and hands back the set-up, so each story is left with only
the part that differs: contact answers the spam check and the provider, bulk
email answers the provider.

Also stops a story quietly booking fewer people than it asked for. The list of
people to book has two addresses in it, so asking for three used to book two
and every count below it would have been right for the wrong reason. It now
fails and says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4fF7zjiMi9bkHG22oFzh1
Coverage caught the trap E2E_TESTS.md documents: a story may never be the only
cover of a production line or branch, because Cucumber runs do not count
towards coverage. Deleting the two send tests and the preview one took the
only direct cover of three branches with them — the unsubscribed set a
promotion is built against, the refusal when it leaves nobody, and the
skipped-count the preview only renders when somebody is being left out.

All three are back, each headed by a note saying which story tells the same
thing in the owner's terms and why the direct test still has to exist. The two
listing-page link tests stay deleted: the story covers those lines through
production code that other tests also walk.

The new `offersAWayToPost` had the same problem — only the story pressed it —
so it gets four cases in the suite that already owns the "which form would a
press really belong to" question: a live button, one switched off, one that
sends nothing, and an address no form posts to.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4fF7zjiMi9bkHG22oFzh1
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This change adds attendee bulk-email acceptance coverage, shared outgoing-request and browser form helpers, localized bulk-email messages, and integration-test updates for recipient filtering and preview behavior.

Changes

Attendee messaging

Layer / File(s) Summary
Form submission discovery
test/test-utils/test-browser.ts, test/test-utils/test-browser/forms.ts, test/test-utils/test-browser/posting-to-an-address.test.ts
Adds reusable button filtering, POST-path discovery, and availability checks for browser tests.
Outgoing request and world state support
test/specs/support/outgoing.ts, test/specs/support/contact.ts, test/specs/support/world.ts, test/specs/support/memory.ts
Centralizes outgoing request recording and stores booked addresses, recorded calls, and written message text in the test world.
Bulk-email acceptance flow
specs/attendees/*, test/specs/steps/bulk-email.ts, test/specs/support/bulk-email.ts, test/scripts/specs/catalog.test.ts
Adds scenarios and helpers for booking attendees, previewing messages, filtering promotional recipients, sending messages, using drafts, and checking listing actions.
Localized bulk-email responses
src/features/admin/bulk-email.ts, src/locales/en/bulk-email.json, test/integration/server/bulk-email/*
Localizes bulk-email validation and result messages and adds direct coverage for skipped unsubscribed recipients and related branches.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Owner
  participant ListingPage
  participant BulkEmailForm
  participant EmailProvider
  Owner->>ListingPage: Open attendee email action
  ListingPage->>BulkEmailForm: Show recipients and compose form
  Owner->>BulkEmailForm: Submit message for preview
  BulkEmailForm-->>Owner: Show recipients and send options
  Owner->>BulkEmailForm: Confirm previewed message
  BulkEmailForm->>EmailProvider: Send to eligible attendees
  EmailProvider-->>BulkEmailForm: Return delivery result
  BulkEmailForm-->>Owner: Show send result
Loading

Possibly related PRs

  • chobbledotcom/tickets#2025: Overlaps with the TestBrowser form-navigation utilities, including submitFormAt, wayToPost, and pressableOn.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding a story about writing to people booked onto a listing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/convert-tests-cucumber-bq6mji
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/convert-tests-cucumber-bq6mji

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0ffcac366

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread test/specs/support/bulk-email.ts Outdated
Comment thread test/test-utils/test-browser.ts Outdated
Comment thread specs/attendees/writing-to-the-people-who-booked.feature Outdated
Comment thread test/specs/support/outgoing.ts Outdated
**The rule said something untrue.** The story claimed that without an email
provider of their own the owner is "offered no way to send it". The preview
always offers to open the message as a draft in their own email app, whatever
provider they have — so a reader would have learned the opposite of the truth,
and the catalog would have published it. The rule is now about the site
sending *for* them, which is the part that actually needs a provider, and the
scenario asserts the draft is still offered alongside. Re-checked that it
bites: with the Send button always rendered, the scenario still fails.

**A refused send now proves no request happened.** Reading the addresses back
let a send that went out carrying nothing readable pass as "nothing was
written to anybody". That step now asks how many times the provider was asked
at all, and reading addresses fails loudly on a send with no readable
recipients rather than coercing it to an empty list.

**The pressability logic moved to where pure markup-reading lives.**
`pressableOn` and the "which form would a press really belong to" search are
plain functions over markup with no request behind them, which is exactly what
`test-browser/forms.ts` is for. `test-browser.ts` is back to the browser
itself at 440 lines, and both `submitFormAt` and `offersAWayToPost` now call
one shared `wayToPost`.

**The shared watcher factory names its return type**, so a change to it fails
where it is written rather than leaking an inferred shape to whichever story
reads it next.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4fF7zjiMi9bkHG22oFzh1
@stefan-burke
stefan-burke added this pull request to the merge queue Aug 4, 2026
Merged via the queue into main with commit d495f8f Aug 4, 2026
3 checks passed
@stefan-burke
stefan-burke deleted the claude/convert-tests-cucumber-bq6mji branch August 4, 2026 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants