Conversation
The upstream NYU MuleSoft Identity API has a p50 latency of ~2.7s in production (measured from App Engine request logs) and holds memory on the App Engine instance while it waits. That long hold is one of the patterns contributing to the F1 instance OOM kills (200+ kills over 2 days, "too much memory" warnings in appengine logs) which in turn make unrelated requests (/api/firestore/list etc.) spike from ~100ms to seconds. netId is effectively permanent and the upstream identity record (name, dept_code, school, affiliations) is stable across many days, so a 7-day cache is safe and gives a huge win on both latency and instance memory pressure. Wrap `fetchNYUIdentity` with a Firestore-backed cache keyed by uniqueId (the call site already passes netId there). Cache hits return in a few ms instead of waiting on the upstream call. Cache misses fall through to the live fetch and write back the result; the write is fire-and-forget so a slow Firestore write doesn't re-introduce upstream latency for the caller. Cache read/write failures degrade silently to a live fetch — the cache is never the single point of failure. Both consumers (`/api/nyu/identity/[uniqueId]` and `/api/nyu/entitlements/[netId]`) call this function and so both pick up the caching transparently without any route change. Tests cover the eight branches that matter: fresh hit, expired hit, missing doc, cache read throws, upstream failure (no negative cache), cache write failure (still returns result), malformed cache document, and missing-token branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumps the npm_and_yarn group with 1 update in the /booking-app directory: [postcss](https://github.com/postcss/postcss). Updates `postcss` from 8.5.13 to 8.5.14 - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](postcss/postcss@8.5.13...8.5.14) --- updated-dependencies: - dependency-name: postcss dependency-version: 8.5.14 dependency-type: direct:development dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com>
…app/npm_and_yarn-06160b2e2d chore(deps-dev): bump postcss from 8.5.13 to 8.5.14 in /booking-app in the npm_and_yarn group across 1 directory
…updates Bumps the npm_and_yarn group with 2 updates in the /booking-app directory: [@tootallnate/once](https://github.com/TooTallNate/once) and [qs](https://github.com/ljharb/qs). Bumps the npm_and_yarn group with 1 update in the /booking-app/components/src/test directory: [qs](https://github.com/ljharb/qs). Updates `@tootallnate/once` from 2.0.0 to 2.0.1 - [Release notes](https://github.com/TooTallNate/once/releases) - [Changelog](https://github.com/TooTallNate/once/blob/v2.0.1/CHANGELOG.md) - [Commits](TooTallNate/once@2.0.0...v2.0.1) Updates `qs` from 6.15.0 to 6.15.2 - [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md) - [Commits](ljharb/qs@v6.15.0...v6.15.2) Updates `qs` from 6.14.2 to 6.15.2 - [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md) - [Commits](ljharb/qs@v6.15.0...v6.15.2) --- updated-dependencies: - dependency-name: "@tootallnate/once" dependency-version: 2.0.1 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: qs dependency-version: 6.15.2 dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: qs dependency-version: 6.15.2 dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com>
…app/npm_and_yarn-fdfe85c016 chore(deps): bump the npm_and_yarn group across 2 directories with 2 updates
- Associate booking form labels with their inputs via id/aria-labelledby - Convert clickable navBar Box/Typography into real <button>s - Add aria-label to icon-only IconButtons across admin and booking tables - Wrap NavBar/page content in <header>/<main> landmarks - Replace misused <label> tags with <span> for display-only text - Add rel="noopener noreferrer" + new-tab indication to external links - Replace href-less <a> in BookingStatusBar with focusable <button>
- Update RoomDetails CSS selector from `label` to `span` so the bold/spacing styling still applies after the recent label→span swap - Only render the "Why?" tooltip trigger in BookingStatusBar when an errorMessage exists, to avoid `aria-label="Why? null"` and an empty tooltip
fix(a11y): improve form labels, keyboard access, and landmarks
… concurrent misses, write expiresAt - URL-encode the `uniqueId` path segment so reserved characters in a netId cannot rewrite the upstream URL. - Coalesce concurrent cache misses for the same `uniqueId` via a module-level in-flight Promise map, so parallel callers (identity + entitlements on first page load) share a single upstream call instead of each spending ~2.7s and holding App Engine memory. - Write an `expiresAt` field (`cachedAt + 7d`) so a Firestore TTL policy on `nyu_identity_cache.expiresAt` can purge stale docs automatically. Read- time check still uses `cachedAt + CACHE_TTL_MS` as source of truth. Adds unit tests for dedup behavior, expiresAt, and URL encoding. NOTE: enabling the Firestore TTL policy on `nyu_identity_cache.expiresAt` is a manual GCP console step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds eslint-plugin-jsx-a11y as a devDependency and wires its recommended rule set into the TSX/JSX lint pass. Locks in the accessibility fixes from #1483 so regressions surface during local lint / IDE feedback. `jsx-a11y/no-autofocus` is downgraded to warn — Dialog primary buttons intentionally use autoFocus for focus management.
chore(eslint): enable jsx-a11y recommended rules
perf(nyu): cache NYU Identity API responses in Firestore (7-day TTL)
Bumps the npm_and_yarn group with 1 update in the /booking-app directory: [axios](https://github.com/axios/axios). Updates `axios` from 1.15.2 to 1.16.0 - [Release notes](https://github.com/axios/axios/releases) - [Changelog](https://github.com/axios/axios/blob/v1.x/CHANGELOG.md) - [Commits](axios/axios@v1.15.2...v1.16.0) --- updated-dependencies: - dependency-name: axios dependency-version: 1.16.0 dependency-type: direct:production dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com>
updated README for .env location
…app/npm_and_yarn-ac20752053 chore(deps): bump axios from 1.15.2 to 1.16.0 in /booking-app in the npm_and_yarn group across 1 directory
Follow-up fixes on top of the tenant-schema refactor: - mergeSchemaDefaults: coerce the raw document BEFORE adding defaults. The previous order injected empty nested defaults (emailNotifications, attestations, resource.training) that made legacy documents look like the new nested shape, so coercion took its new-shape branch and let the empty defaults shadow real legacy values (emailMessages, agreements, needsSafetyTraining / training URLs) — silently blanking them. - migrateResource: merge an existing nested `training` object with legacy flat fields so partially-migrated resources keep their form/info URLs. - isNewSchemaShape: stop requiring the optional `contextLabels` field so a nested document that omits it is not misdetected as legacy and blanked. - coerceTenantSchema: map legacy emailMessages and prefer non-empty attestations in the new-shape branch as well (defense in depth). - FormInput: rename the inverted `agreementsChecked` to `attestationsIncomplete` to match what it actually represents. - Extract server-safe schema types/defaults into `schemaTypes.ts` so API routes / coerceTenantSchema / firebase no longer pull the client-only SchemaProvider (createContext) into the server bundle. SchemaProvider now re-exports them and is marked "use client". Fixes the build. - Add a regression test asserting legacy email/agreement/training values survive mergeSchemaDefaults.
Two regressions surfaced by CI E2E after the schema refactor: - VIP/walk-in Submit stayed permanently disabled. The original branch turned the (previously no-op) agreements gate into a working `attestationsIncomplete` check, but the Agreement section only renders for the regular booking form (`!isMod && isBooking`). VIP and walk-in flows therefore had unsatisfiable required attestations. Gate the check on `isBooking` so it matches what is actually rendered. - ITP room 408 duration limit (student max 1h) no longer triggered. The test fixtures moved to a `resource()` helper that spreads `defaultResource`, injecting a placeholder top-level `maxHour`/`minHour` of -1. `getBookingHourLimits` prefers top-level limits, so the -1 (= unlimited) shadowed the real limits stored under `autoApproval`. Drop the inherited top-level limits unless a room sets them explicitly, matching how these rooms were defined before the refactor.
Address review feedback (Codex + Copilot): - coerceTenantSchema: in the new-shape branch, also pick up a legacy top-level `timeSensitiveRequestWarning`. A partially-migrated document (nested tenant/mappings/form but warning still top-level) otherwise dropped the warning silently, since only the legacy branch read it and the UI (BookingStatusBar) reads only the nested location. The nested value still wins when present. Both prod tenant docs carry this field top-level today. Add coerce regression tests. - emails.ts: import EmailNotifications from the server-safe `schemaTypes` instead of the client-only `SchemaProvider`, keeping the dependency direction consistent with the server/client split. - tenantSchema route: clarify that `raw=1` only skips environment calendar ID rewriting and never bypasses coercion (the editor consumes the coerced nested shape and persists it on save — the intended lazy migration).
Refactor schema
Tenant Schema Diff (development → production)This PR targets Key Differences by DocumentFull dry-run output |
Contributor
There was a problem hiding this comment.
Pull request overview
This “Prod release 6/3” PR performs a broad tenant-schema shape migration (legacy flat → canonical nested), updates email template plumbing to a new emailNotifications structure, improves accessibility across several UI components, and adds a Firestore-backed cache for NYU Identity lookups to reduce production latency/memory pressure.
Changes:
- Introduces canonical nested tenant schema types/defaults (
schemaTypes), pluscoerceTenantSchemato transparently normalize legacy Firestore documents and prevent silent data loss during default-merging. - Renames/rewires email template configuration from
emailMessages→emailNotificationsacross server code, state machines, API routes, and tests. - Adds NYU Identity caching + in-flight request coalescing; enables
eslint-plugin-jsx-a11yand applies multiple a11y fixes (ARIA labels, semantic buttons, focus-visible styles).
Reviewed changes
Copilot reviewed 89 out of 91 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates local env file instructions (now references .env). |
| booking-app/README.md | Updates local env file instructions for app subdirectory. |
| booking-app/package.json | Bumps axios; adds eslint-plugin-jsx-a11y. |
| booking-app/package-lock.json | Locks updated dependency versions. |
| booking-app/eslint.config.mjs | Enables jsx-a11y recommended rules for JSX/TSX. |
| booking-app/scripts/syncTenantSchemas.ts | Adjusts new-schema creation to rely on defaults/coercion. |
| booking-app/scripts/schemaDefaults.ts | Coerces schema before add-only default merge to avoid shadowing legacy values. |
| booking-app/lib/tenant/coerceTenantSchema.ts | Adds schema coercion layer (legacy/new-shape normalization + migrations). |
| booking-app/lib/tenant/getCachedTenantSchema.ts | Coerces Firestore schema reads and bypass-auth test schemas before caching. |
| booking-app/lib/utils/testTenantSchema.ts | Rebuilds E2E tenant fixtures using canonical nested schema defaults. |
| booking-app/lib/server/nyuIdentity.ts | Adds Firestore cache + TTL fields + in-flight coalescing + URL encoding for upstream fetch. |
| booking-app/lib/firebase/firebase.ts | Switches schema type import to server-safe schemaTypes. |
| booking-app/lib/stateMachines/xstateEffects.ts | Uses emailNotifications for canceled email headers. |
| booking-app/lib/stateMachines/xstateTransitions.ts | Uses emailNotifications for no-show messaging. |
| booking-app/lib/stateMachines/effects/declinedEffects.ts | Uses emailNotifications for declined messaging. |
| booking-app/app/api/tenantSchema/[tenant]/route.ts | Always coerces schema output; enforces tenantId on PUT; clarifies raw=1 behavior. |
| booking-app/app/api/safety_training_form/route.ts | Coerces schema and reads training form from nested training/resource.training. |
| booking-app/app/api/checkout-processing/route.ts | Uses emailNotifications.checkedOut. |
| booking-app/app/api/checkin-processing/route.ts | Uses emailNotifications.checkedIn. |
| booking-app/app/api/bookingsDirect/route.ts | Uses emailNotifications for VIP/walk-in confirmation headers. |
| booking-app/app/api/bookings/route.ts | Uses emailNotifications for request/approval email headers. |
| booking-app/app/api/bookings/edit/route.ts | Uses emailNotifications for edit notification headers. |
| booking-app/app/[tenant]/layout.tsx | Updates metadata/title/icons to nested tenant branding; wraps layout with semantic <header>/<main>. |
| booking-app/components/src/client/routes/components/schemaTypes.ts | Adds server-safe canonical schema types + defaults + generateDefaultSchema. |
| booking-app/components/src/client/routes/components/SchemaProvider.tsx | Becomes client wrapper that re-exports server-safe schema types/defaults. |
| booking-app/components/src/client/routes/components/SchemaProviderWrapper.tsx | Updates logs to new tenantId/nested name fields. |
| booking-app/components/src/client/routes/components/Provider.tsx | Uses tenantId; maps nested resource.training into legacy-shaped fields where needed. |
| booking-app/components/src/client/routes/components/navBar.tsx | Updates schema reads to nested shape; improves accessibility (button semantics/focus styles). |
| booking-app/components/src/client/routes/components/ListTableRow.tsx | Adds aria-label to remove IconButton. |
| booking-app/components/src/client/routes/components/AddRow.tsx | Adds aria-label to add IconButton. |
| booking-app/components/src/client/routes/components/AddDepartmentRow.tsx | Adds aria-label to add IconButton. |
| booking-app/components/src/client/routes/components/bookingTable/BookingTableRow.tsx | Adds aria-label for details IconButton. |
| booking-app/components/src/client/routes/components/bookingTable/Bookings.tsx | Reads services toggles from nested form.services; adds aria-label for details button. |
| booking-app/components/src/client/routes/components/bookingTable/BookingTableFilters.tsx | Reads origin/service flags from nested origins/form.services. |
| booking-app/components/src/client/routes/components/bookingTable/MoreInfoModal.tsx | Reads nested schema flags; adds aria-labels; replaces <label> with <span> for non-form labels. |
| booking-app/components/src/client/routes/booking/components/FormInput.tsx | Migrates schema reads to nested shape; gates submit on required attestations; adds safer link attrs/text. |
| booking-app/components/src/client/routes/booking/components/BookingStatusBar.tsx | Uses nested warning config; replaces non-link “Why?” with accessible button; safer link attrs. |
| booking-app/components/src/client/routes/booking/components/BookingSelection.tsx | Replaces <label> with <span> for non-form labels (and updates styling selector). |
| booking-app/components/src/client/routes/booking/components/BookingFormInputs.tsx | Improves a11y by wiring ids/aria attributes to inputs/switches/selects. |
| booking-app/components/src/client/routes/booking/components/BookingFormMediaServices.tsx | Reads service flags from nested form.services. |
| booking-app/components/src/client/routes/booking/components/BookingFormEquipmentServices.tsx | Reads service flags from nested form.services. |
| booking-app/components/src/client/routes/booking/hooks/useCheckAutoApproval.tsx | Switches from tenant to tenantId usage across logs/machine inputs. |
| booking-app/components/src/client/routes/booking/formPages/LandingPage.tsx | Uses nested tenant branding fields for title/copy. |
| booking-app/components/src/client/routes/booking/formPages/UserRolePage.tsx | Migrates mapping reads to nested mappings.*. |
| booking-app/components/src/client/routes/booking/formPages/SelectRoomPage.tsx | Reads training fields from nested resource.training. |
| booking-app/components/src/client/routes/admin/components/ServiceApproverUsers.tsx | Uses tenantId; adds aria-label for add button; formatting cleanup. |
| booking-app/components/src/client/routes/admin/components/PreBan.tsx | Adds aria-label for details button; formatting cleanup. |
| booking-app/components/src/client/routes/admin/components/policySettings/BookingBlackoutPeriods.tsx | Adds aria-labels to edit/delete IconButtons. |
| booking-app/components/src/client/routes/admin/components/BookingActions.tsx | Adds aria-labels to confirm IconButtons. |
| booking-app/components/src/client/routes/super/schemaEditorUtils.ts | Adds getByPath helper for nested schema editor fields. |
| booking-app/components/src/client/routes/super/schemaEditor.tsx | Migrates schema editor UI to nested schema fields; renames agreements→attestations; updates email notifications fields; adds aria-labels. |
| booking-app/components/src/server/emails.ts | Returns emailNotifications and coerces schema from Firestore before reading. |
| booking-app/components/src/server/db.ts | Uses emailNotifications for multiple booking-status emails (declined/closed/late-cancel/no-show). |
| booking-app/components/src/server/admin.ts | Uses emailNotifications for approval-related headers. |
| booking-app/components/src/tenantPolicy.ts | Coerces schema from Firestore before reading ccEmails. |
| booking-app/components/src/testHelpers/testTenantSchemas.ts | Updates unit-test tenant fixtures to canonical nested shape; migrates email templates. |
| booking-app/components/src/test/package-lock.json | Updates test package lock dependency versions. |
| booking-app/tests/unit/schema-completeness.unit.test.ts | Updates schema key list to reflect new canonical nested schema shape. |
| booking-app/tests/unit/schemaDefaults.addOnly.unit.test.ts | Updates mapping/context label assertions; adds regression test for legacy-data not being shadowed by injected defaults. |
| booking-app/tests/unit/schema-permission-labels.unit.test.ts | Renames permission labels to tenant.contextLabels assertions. |
| booking-app/tests/unit/schema-editor-utils.unit.test.ts | Updates diff path assertions to emailNotifications.*. |
| booking-app/tests/unit/coerce-tenant-schema.unit.test.ts | Adds coercion coverage for legacy timeSensitiveRequestWarning placement. |
| booking-app/tests/unit/nyuIdentity-cache.unit.test.ts | Adds unit tests covering caching, TTL, malformed cache docs, and concurrency coalescing. |
| booking-app/tests/unit/tenant-policy.unit.test.ts | Simplifies mock schema creation using generateDefaultSchema. |
| booking-app/tests/unit/server-db.unit.test.ts | Updates email template mocking to emailNotifications. |
| booking-app/tests/unit/no-show-system-attribution.unit.test.ts | Updates email template mocking + schemaName presence. |
| booking-app/tests/unit/xstate-effects-handlers.unit.test.ts | Updates email template mocking to emailNotifications. |
| booking-app/tests/unit/api-tenant-schema.unit.test.ts | Updates API schema expectations to nested tenant.*; enforces tenantId on save. |
| booking-app/tests/unit/api-approve.unit.test.ts | Updates mocked email config shape. |
| booking-app/tests/unit/MyBookingsPage.unit.test.tsx | Uses generateDefaultSchema-based SchemaContext value. |
| booking-app/tests/unit/modification-features.unit.test.tsx | Updates SchemaProvider mock to nested shape using generateDefaultSchema. |
| booking-app/tests/unit/form-context-field-visibility.unit.test.tsx | Coerces raw legacy mock schemas to canonical shape inside tests. |
| booking-app/tests/unit/BookingTableSortReset.unit.test.tsx | Uses generateDefaultSchema for SchemaContext. |
| booking-app/tests/unit/BookingTableFilters.user.unit.test.tsx | Updates mock schema to nested tenant/form/origins shape. |
| booking-app/tests/unit/Bookings.sorting.unit.test.tsx | Uses generateDefaultSchema for SchemaContext. |
| booking-app/tests/unit/Bookings.services.user.unit.test.tsx | Updates schema builder to nested form.services overrides. |
| booking-app/tests/unit/Bookings.dateRangeDefault.unit.test.tsx | Uses generateDefaultSchema for SchemaContext. |
| booking-app/tests/unit/BookingActions.unit.test.tsx | Uses generateDefaultSchema for SchemaContext. |
| booking-app/tests/unit/BookingActions.equipment.unit.test.tsx | Uses generateDefaultSchema for SchemaContext. |
| booking-app/tests/unit/booking-edit-resubmission-resets-services.unit.test.ts | Updates mocked email templates to emailNotifications. |
| booking-app/tests/unit/booking-edit-api-integration.unit.test.ts | Updates mocked email templates to emailNotifications. |
| booking-app/tests/unit/auto-checkout-system-attribution.unit.test.ts | Updates mocked checkout email template key. |
| booking-app/tests/e2e/helpers/mock-routes.ts | Replaces inline mock schema with getTestTenantSchema; adjusts mocked recipients payload shape. |
| booking-app/tests/e2e/helpers/itp-mock-routes.ts | Replaces inline ITP mock schema with getTestTenantSchema; adjusts mocked recipients payload shape. |
| booking-app/tests/e2e/helpers/itp-test-utils.ts | Updates comments to reflect nested form flags. |
| booking-app/tests/e2e/helpers/booking-test-helpers.ts | Updates agreement wording to attestations. |
| booking-app/tests/e2e/walk-in-flow.e2e.test.ts | Updates comments to “attestations”. |
| booking-app/tests/e2e/vip-flow.e2e.test.ts | Updates comments to “attestations”. |
| booking-app/tests/e2e/safety-training.e2e.test.ts | Updates comment to reflect nested training flag. |
| booking-app/tests/e2e/itp-booking-flow.e2e.test.ts | Updates comments to reflect nested form flags. |
| booking-app/tests/e2e/itp-admin-ui-visibility.e2e.test.ts | Updates comments to reflect nested form flags. |
Files not reviewed (2)
- booking-app/components/src/test/package-lock.json: Language not supported
- booking-app/package-lock.json: Language not supported
| ``` | ||
|
|
||
| 4. 🔐 Obtain the `.env.local` file from a project administrator (Riho or Nima) and place it in the root directory of the project. | ||
| 4. 🔐 Obtain the `.env` file from a project administrator (Riho or Nima) and place it in the `booking-app` directory. |
Comment on lines
+55
to
56
| - Request the `.env` file from project admin or another dev | ||
| - Never commit the `.env` file to version control |
Comment on lines
108
to
110
| #### Notes | ||
| - **Environment:** Ensure you have the **development** `.env.local` file properly configured for your testing environment. | ||
| - **Environment:** Ensure you have the **development** `.env` file properly configured for your testing environment. | ||
| - **Playwright Setup:** If running Playwright tests for the first time, install the required browsers: |
| 3. Install the dependencies: | ||
| `npm install` | ||
| 4. Obtain the `.env` file from a project administrator and place it in the root directory of the project. | ||
| 4. Obtain the `.env` file from a project administrator and place it in the `booking-app` directory. |
Comment on lines
+25
to
44
| const LogoBox = styled("button")` | ||
| cursor: pointer; | ||
| display: flex; | ||
| align-items: flex-end; | ||
| background: none; | ||
| border: none; | ||
| padding: 0; | ||
| font: inherit; | ||
| color: inherit; | ||
| text-align: left; | ||
|
|
||
| img { | ||
| margin-right: 8px; | ||
| } | ||
|
|
||
| &:focus-visible { | ||
| outline: 2px solid currentColor; | ||
| outline-offset: 2px; | ||
| } | ||
| `; |
This branch was previously deployed
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
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.
Summary of Changes
Schema Changes
Checklist
Screenshots / Video