Skip to content

fix: pass event.language when sending order-ready notification - #239

Merged
IObert merged 15 commits into
mainfrom
fix/orderReadyNotificationLanguage
Jul 27, 2026
Merged

fix: pass event.language when sending order-ready notification#239
IObert merged 15 commits into
mainfrom
fix/orderReadyNotificationLanguage

Conversation

@IObert

@IObert IObert commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The "Mark order ready" button called getOrderReadyMessage without the language argument, so it silently defaulted to English regardless of the event's configured language.
  • The "Send reminder" button already threaded event.language through correctly (getOrderReadyReminderMessage) — this brings the order-ready path in line with it.

Test plan

  • npx tsc --noEmit passes
  • Manually mark an order ready on a non-English event and confirm the notification uses the correct language template

IObert and others added 15 commits July 24, 2026 14:19
The "Mark order ready" button called getOrderReadyMessage without the
language argument, silently defaulting to English regardless of the
event's configured language. The reminder button already threaded
event.language through correctly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
workers was hardcoded to 1 on CI, serializing all 150 test instances
(30 tests x 5 browser projects) despite fullyParallel: true, turning a
~2 min suite into 25+ min and causing PR test runs to approach the
job timeout.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…fixture

"should not be able to select more than 9 menu items + navigate to
smoothie" mutated the shared "test-event" fixture's item selection and
mode, which every other e2e spec (browse-orders, kiosk-render,
order-terminal) also reads. Under parallel workers, that mutation raced
with concurrent reads/renders of the same event, causing deterministic
failures (missing "Espresso" option, missing menu items, broken
pagination) that were masked for years by forcing workers: 1 on CI.

Give this one test its own private, per-worker event instead, and drop
the now-unnecessary describe.configure({ mode: "serial" }) guard, since
no test in that block mutates shared state anymore.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
It was only ever read as a fallback default sender when
TWILIO_MESSAGING_SERVICE_SID is unset, but that var is required at
startup (instrumentation.ts throws if missing), so the fallback branch
was unreachable. Dropped from code, samples, docs, and the deploy
pipeline.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Next.js never overrides env vars already present in process.env with
values from .env.local. If TWILIO_API_KEY/API_SECRET/ACCOUNT_SID are
exported in the shell (e.g. leftover from another project), the app
silently boots against the wrong Twilio account and 404s on resources
that only exist under the intended account. test/test:e2e already
worked around this; apply the same fix to dev, which has no such
existing account-mismatch risk in CI since it's never run there.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…load

The previous env -u approach relied on Next's own .env.local fallback
to refill the unset vars, but Next.js deliberately skips loading
.env.local when NODE_ENV=test -- which broke Playwright's webServer
(spawned as "NODE_ENV=test pnpm run dev"), since it no longer inherited
the correct values from the parent playwright-config process after
they were stripped.

Preload dotenv with override:true instead, so .env.local's values win
regardless of NODE_ENV or what's already in the shell.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cap test

Follow-on fixes found while verifying the isolation from 3d57c71 under
real parallel load:

- createEvent() hardcoded name: "TestEvent" for every slug, so the
  private event's card duplicated the shared fixture's heading on the
  home page, breaking any concurrently-running test asserting against
  a single "TestEvent" match. Added a name param.
- The chosen replacement name still started with "TestEvent", which
  still collided with a non-exact getByRole(..., { name: "TestEvent" })
  match elsewhere in the file (Playwright's name matcher is substring
  by default). Renamed to "MenuCapEvent{parallelIndex}", which shares
  no substring with "TestEvent".
- The fixed 2s waitForTimeout before interacting assumed the shared
  fixture's long-settled state; a freshly-created event's Sync data
  can take longer to propagate under concurrent load. Wait for the
  actual rendered menu state instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…creen

On narrow/short viewports (e.g. Mobile Chrome, 393x727), the fixed
px-24/pt-16 padding plus a text-4xl intro paragraph pushed the "Order
Item" trigger far enough down the page that Radix's Select popover had
no room left to render within the viewport -- confirmed via direct DOM
inspection: the listbox rendered at y:735-782 while the viewport was
only 727px tall, so the option was literally unclickable.

Scale both down at narrow widths (px-4/pt-6, text-xl) while keeping the
original large-kiosk-display sizing at md: and up, where this was never
an issue.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… nested <button>, sanitize slug diacritics

- Use aria-disabled instead of native disabled on the Create Event button so the
  Radix tooltip explaining why it's inactive still fires on hover/focus.
- Replace the decorative Radix Checkbox in menu-select with a plain div indicator
  to stop nesting a <button> inside the menu item/modifier <button>, which broke
  hydration.
- Strip diacritics/non-ASCII characters when deriving an event slug from its name
  (e.g. São Paulo -> sao-paulo) to avoid unusable slugs.
Orders placed through /api/order (the manual and kiosk order form) had no
channel set and fell into the generic "other" bucket on the stats page's
Orders by Channel chart. Tag them explicitly as "api" so the breakdown
distinguishes SMS/WhatsApp/RCS orders from ones entered through the API.
…nd Moka coffee

Includes menu icon mappings for the new items.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…s-channel mixups

Twilio's Messaging Service could auto-select a different sender (e.g. RCS
instead of WhatsApp) for a customer than the one they last messaged in on,
confusing customers and silently failing if the chosen sender's 24h session
window wasn't open. Now every reply pins the exact sender the attendee's
Twilio-side "To" arrived on, persisted per-attendee and threaded through all
send paths (webhook replies, AI agent, QR/profile flows, order notifications,
broadcasts, and the broadcast script, which also now fixes to's missing
channel prefix). Also removes the dead/unwired Conversation Orchestrator JSON
webhook branch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion

Client Components can't receive a raw Twilio SDK class instance across the
Server Action boundary. ordersList.tsx was calling createSyncMapItemIfNotExists
directly to read an attendee's pinned sender, which returns a SyncMapItemInstance
and crashed with "Only plain objects... Classes... not supported" when marking
an order ready. Added getPinnedSender() as a proper Server Action that resolves
the lookup server-side and returns just the plain string.

Also localizes AI agent tool responses, the "forget me" flow, and order content
templates for pt-BR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
onSelectionChange fired one fire-and-forget PUT per click carrying the
full selection at that instant, with no ordering guarantee. Under
network latency (reproducible on CI, rare locally), a later click's
PUT could complete before an earlier one, and the real-time Sync
subscription would then push the earlier (smaller) selection back into
local state, regressing the visible item count -- causing
browse-events.spec.ts's 10-item-cap test to fail/flake in CI while
passing locally.

Debounce the save like its neighbors updateMenuItemField/
updateModifierField already do, so only one PUT with the latest state
is ever in flight. Also make the test wait for each click's count to
land before firing the next, as a second line of defense.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@IObert
IObert merged commit bbabf5d into main Jul 27, 2026
7 checks passed
@IObert
IObert deleted the fix/orderReadyNotificationLanguage branch July 27, 2026 19:43
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.

1 participant