Tell holidays, discount codes, and the news as Cucumber stories - #2017
Merged
Conversation
Three new features: a holiday takes its days off a daily listing's menu and deleting it opens them again, while plain places sell right through; a promo code box waits for a code to exist, the right code cuts only the total, a wrong one quietly buys nothing, and a paid booking counts the use in the books; news posts are read at their own address, the News link waits for the first post, and taking one down needs its exact name. The support drives the real admin and booking forms throughout, and the test browser now honours a pressed button's formaction — the quote button aims the booking form at the price summary, as a real browser would. Two integration tests whose whole claim the stories now tell are removed: the holiday-date exclusion on the single daily page, and the news nav link waiting for the first post (its bare-title claim stays behind under its own name). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
The news post and holiday helpers now create through one shared makesRecordThroughForm and delete through the existing typed-name walk behind each record's Actions tab, and the discount journeys share one booking-page opener that owns their common signature. One news step body and the twin promo-box steps fold into helpers too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdded Cucumber specifications and shared test support for holiday availability, discount-code bookings, and public news publishing. Updated browser form handling, test-world record storage, checkout exports, story catalog expectations, and related integration coverage. ChangesAcceptance specification coverage
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/servicing/telling-people-the-news.feature`:
- Around line 59-65: Add an assertion to the “news.exact-name-removes-the-post”
scenario verifying that the front page no longer offers the News link after the
final post is deleted, while preserving the existing /news 404 check.
- Around line 51-57: Update the “owner is told the post name does not match”
scenario to use the case-only variant “spring fair” when attempting removal,
while preserving the exact-name rejection assertion and confirming “Spring fair”
remains published.
In `@test/specs/steps/holidays.ts`:
- Around line 68-87: Extract a curried factory for the repeated holiday step
implementations, following the boxExpected(offered: boolean) pattern from
discount-codes.ts. Have the factory call listingOffersDay with the captured
expected boolean and register the three step phrases through it, preserving each
phrase’s current true or false expectation.
In `@test/specs/support/browser.ts`:
- Around line 157-170: Annotate the exported makesRecordThroughForm factory with
an explicit named callback return type, covering its world, name, and fields
parameters and Promise<void> result. Keep the existing inner callback behavior
unchanged and define or reuse the appropriate callback type at the factory
boundary.
In `@test/specs/support/discount-codes.ts`:
- Around line 44-51: Update the row selection in the modifier-link lookup to
compare the rendered link text against code exactly rather than using substring
containment. Preserve the existing id extraction and
world.things.remember("record", code, Number(id)) behavior, while allowing only
the row whose normalized text exactly matches the newly created modifier code.
- Around line 25-51: Extract the list-redirect creation logic from
organiserCreatesCode into a reusable list-based record-creation factory in
browser.ts, separate from makesRecordThroughForm’s ID-redirect behavior. Update
organiserCreatesCode in discount-codes.ts to use the new factory while
preserving code matching, missing-link failure, and record storage. Leave
news.ts and holidays.ts on makesRecordThroughForm unchanged.
In `@test/specs/support/news.ts`:
- Around line 53-61: Update the news link lookup in the support helper to use
requiredWorldValue for the matching post link instead of the optional find
result and manual missing-target error branch. Preserve the existing name-based
card matching and pass the resulting required value through the shared helper.
- Around line 1-5: Shorten or remove the explanatory comments at
test/specs/support/news.ts lines 1-5, 30-32, and 47-49; remove the newsPageReads
description at test/specs/steps/news.ts line 38 and the historical coverage note
at test/integration/server/news-public.test.ts lines 78-79. Keep only the
typed-confirmation reason at news.ts lines 30-32 and the reason for following
the served link at lines 47-49; make retained comments brief, current, and
plain-language.
In `@test/test-utils/test-browser.ts`:
- Around line 256-257: Update the buttonAction parsing near attrValue to use the
shared boundary-aware attribute parser instead of the formaction-specific regex,
supporting single-quoted, double-quoted, and whitespace-separated attributes
while excluding similarly named attributes such as data-formaction. Extend the
shared parser only if necessary, and add regression tests covering these valid
forms and the similarly named attribute case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3987d556-7b4b-46c6-bc8a-d2712de2ff5e
📒 Files selected for processing (17)
specs/bookings/taking-a-holiday.featurespecs/payments/booking-with-a-discount-code.featurespecs/servicing/telling-people-the-news.featuretest/integration/server/news-public.test.tstest/integration/server/public/daily-listings-single.test.tstest/scripts/specs/catalog.test.tstest/specs/steps/discount-codes.tstest/specs/steps/holidays.tstest/specs/steps/news.tstest/specs/support/browser.tstest/specs/support/discount-codes.tstest/specs/support/holidays.tstest/specs/support/memory.tstest/specs/support/news.tstest/test-utils/order-journey.tstest/test-utils/test-browser.test.tstest/test-utils/test-browser.ts
💤 Files with no reviewable changes (1)
- test/integration/server/public/daily-listings-single.test.ts
The news story now checks the News link is gone once the last post is deleted. The three holiday day steps fold into one curried factory, the record factory carries a named public type, the modifier list row is matched whole so SAVE100 can never stand in for SAVE10, the news link lookup fails through requiredWorldValue, formaction parsing takes a space boundary so data-formaction cannot aim a form (with a regression test), and the wordier comments are trimmed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8bf1920af
ℹ️ 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".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
Three more corners of the product now have their rules told as plain-English Cucumber stories, following on from #2011: holidays, discount codes, and news posts. Every story drives the real served pages — the organiser's admin forms, the customer's booking page, the type-the-name delete walk behind each record's Actions tab — so a form that stopped rendering a field, or a link that stopped being offered, fails the story by name.
The three stories
The organiser takes a holiday (
specs/bookings/taking-a-holiday.feature) — adding a holiday takes its days off a daily listing's date choices while the days either side stay on offer; deleting the holiday opens the days straight back up (a rule no test proved end to end before); and a holiday never touches something sold as plain places — the Gala keeps selling right through it.A customer books with a discount code (
specs/payments/booking-with-a-discount-code.feature) — the promo code box waits for a code to exist; the right code keeps the place at full price and cuts only the total, shown as its own discount line; the code works typed in any case; a wrong code quietly buys nothing (truthfully: the site says nothing about it, so a guessed code gives nothing away); and a paid booking counts the use in the books and the activity log. The paid scenario is honest end to end: the checkout intent is captured from the real form submit — the typed code resolved by production code — and completed through the provider webhook with the app's own metadata builder.An owner tells people the news (
specs/servicing/telling-people-the-news.feature) — a post is read at its own address by following the news page's served link; the News link waits for the first post so nobody is led to an empty page; and taking a post down needs its exact name.Harness improvement
The booking page's "Show total" button aims its form at the price summary with a
formaction, which the test browser used to ignore. It now honours a pressed button'sformactionthe way a real browser does, with a direct regression test — that is what lets the quote scenarios press the page's own button instead of posting to an address by hand.What was removed, and where each claim went
Two integration tests said only what the stories now say, so they are replaced:
daily-listings-single.test.ts) — the claim is the holiday story's first rule; the date-filtering logic keeps its direct coverage intest/shared/dates.test.ts(holiday, holiday-range, and multi-day-span exclusion).news-public.test.ts) — the claim is the news story's second rule; the test's one unrelated claim (the list page's bare<title>News</title>when the site has no name) stays behind under its own name.Everything else stays as direct technical contracts on purpose: news SEO meta, galleries and RSS XML, slug uniqueness and concurrency, holiday CRUD validation and status codes, and the
/calculateendpoint's exact payload contract. The stories tell the journeys; those tests pin the wire.New support folds onto the existing shared mechanisms rather than adding parallel ones: records are created through one shared
makesRecordThroughForm, deleted through the existing typed-name walk, and the two discount journeys share one curried booking-page opener.Checks
Full precommit green (typecheck, lint, duplication at zero, the whole suite, copy checks), 100% line and branch coverage, and the spec suite now runs 189 scenarios / 1,241 steps, all passing, with
specs:checkvalidating 46 stories and 129 rules. No production TypeScript changed, so the mutation gate has nothing new to test.🤖 Generated with Claude Code
https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7
Generated by Claude Code
Summary by CodeRabbit