Skip to content

feat(booking-requests): add optional booking-requests module - #348

Merged
telivity-otaip merged 18 commits into
mainfrom
cursor/booking-requests-package-4a4f
Aug 28, 2026
Merged

feat(booking-requests): add optional booking-requests module#348
telivity-otaip merged 18 commits into
mainfrom
cursor/booking-requests-package-4a4f

Conversation

@telivity-otaip

@telivity-otaip telivity-otaip commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds @telivityhaip/booking-requests as an optional workspace package for request-first direct booking. Core instant booking remains the default when the feature flag is off.

Architecture (Agustin review addressed)

  • Real package boundary — Nest vertical slice (controllers, DTOs, domain services, Stripe handler, unit specs) lives in packages/booking-requests. BookingRequestModule.forRoot(...) is a real DynamicModule with package-local DI ports; apps/api only wires ports via booking-requests.bootstrap.ts (useExisting) when HAIP_BOOKING_REQUESTS=true.
  • Runnable migrator — compiled dist/database/migrate.js + SQL assets; Docker ships the package artifact.
  • UI flagVITE_HAIP_BOOKING_REQUESTS wired through Dockerfile / compose / release build-args.
  • Fail-safebookingMode=request rejected when the module flag is off.
  • Core schema — request-only audit DDL and payment indexes removed from core push-schema/drizzle; thin hooks kept (payments.booking_request_id, idempotency, accepted pricing snapshot, charge provenance, booking_mode config columns for the fail-safe). Contract covered by push-schema-kept-fields.spec.ts.
  • Flag-OFF regression gate — core migrations only, instant book + Stripe refunds still work.
  • Migration ledger + checksum; feat(booking-engine): add request-first booking workflow #347 safety/remediation specs ported into the package.

Enablement

  1. pnpm db:migrate
  2. pnpm db:migrate:booking-requests
  3. HAIP_BOOKING_REQUESTS=true in apps/api/.env
  4. VITE_HAIP_BOOKING_REQUESTS=true for dashboard/booking UI (Docker build-arg)
  5. Set property bookingMode=request in booking engine admin

See packages/booking-requests/README.md.

How to test

# Flag off — default-install safety (core migrate only)
pnpm --filter @telivityhaip/api exec vitest run src/modules/booking-request/booking-request-flag-off-instant-booking.regression.spec.ts

# Package unit + API booking-request/booking-engine suites
pnpm --filter @telivityhaip/booking-requests test
pnpm --filter @telivityhaip/api exec vitest run src/modules/booking-request/ src/modules/booking-engine/

Contributor credit

@agustinjch

cursoragent and others added 2 commits August 26, 2026 18:21
Introduce @telivityhaip/booking-requests as a deploy-time optional module
(HAIP_BOOKING_REQUESTS=true) with separate migrations, Stripe handler
delegation, and dashboard/booking widget feature flags. Core instant booking
paths stay unchanged when the flag is off.

Includes booking request API/controllers, schema split (0022-0032), email
transport hardening, webhook logicalEventId dedup, and CI release-gate job.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Remove email transport, webhook dedup, shared utils, test-count sync, and
core webhook migration changes that land in separate focused PRs. Document
booking-requests in README packages section and optional enablement steps.

Depends on #349, #350, #351, #352, and #353.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@telivity-otaip telivity-otaip changed the title Add opt-in booking-requests package feat(booking-requests): add optional booking-requests module Aug 26, 2026
@telivity-otaip
telivity-otaip marked this pull request as ready for review August 26, 2026 19:46
cursoragent and others added 3 commits August 26, 2026 19:56
Re-include prerequisite core changes so typecheck and docker seed pass.
Add booking-requests to CI/Docker builds. Sync README to 2120 tests / 252 files.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
push-schema now creates adjusts_charge_id and source_key so seed and
docker init succeed. Webhook logicalEventId spec uses reservation.created
from the core WEBHOOK_EVENTS catalog.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Static imports pulled @telivityhaip/booking-requests into the default demo
image and crashed startup with missing @nestjs/common. Gate the optional
package behind HAIP_BOOKING_REQUESTS and read the flag from core seams.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/booking-requests-package-4a4f branch from 4da34ca to ee16f70 Compare August 26, 2026 20:28
Avoid loading @telivityhaip/booking-requests at startup when
HAIP_BOOKING_REQUESTS is off (docker demo). Preload before Nest bootstrap
when the flag is enabled; read the flag from core payment seams in
DatabaseModule.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/booking-requests-package-4a4f branch from 1461b8f to 5b6a37e Compare August 26, 2026 20:41
Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@agustinjch

Copy link
Copy Markdown
Collaborator

Thanks for doing the separation. I reviewed the current head 4c5a3ae and verified that the instant-booking Stripe refund path is preserved when the package is enabled, including partial/cumulative refund movements and folio recalculation. The direction of the Stripe delegation seam is good.

I do think #348 needs another architecture/packaging pass before it is mergeable:

  1. The production artifact cannot run the advertised package migrations. db:migrate points to tsx src/database/migrate.ts, but the package/Docker artifact contains only dist and package.json; the migrator is not a tsup entry and the SQL files are not copied. Please build a runnable migrator, ship the SQL assets, and test the packaged artifact rather than only the source checkout.
  2. The UI opt-in is not propagated into Docker builds. Dashboard and booking use VITE_HAIP_BOOKING_REQUESTS at build time, but the Dockerfile/release build does not declare or pass it. The API can therefore enable the module while both UIs compile it out. Please wire the build argument through the supported deployment path, or expose capability from the API instead of compiling it independently.
  3. The two opt-ins are not composed safely. With HAIP_BOOKING_REQUESTS off, a persisted or API-written bookingMode=request still makes core BookingEngineService.book() reject instant booking while request controllers are absent. Please reject request-mode configuration when the deployment module is unavailable and make startup/runtime behavior fail safe if the two switches disagree.
  4. The package boundary is currently mostly a facade. Only the schema/migrations and a small registration wrapper live under packages/booking-requests; the controllers, domain services, Stripe handler, UI, and most tests remain in apps/*, and the package receives the API's own BookingRequestModule. The stripeHandlerToken option is also unused. This does not yet provide the isolation we discussed. Please move the vertical-slice implementation behind the package entry point and leave narrow neutral hooks in core.
  5. Core migration still installs request-specific storage. Core push-schema.ts and core Drizzle schemas add booking mode, request payment provenance, accepted-pricing fields, indexes, and constraints before db:migrate:booking-requests runs. Please separate request-only DDL/schema from core; keep in core only fields that are independently justified for normal HAIP.
  6. The exact default-install regression gate is missing. The current default-flow test sets HAIP_BOOKING_REQUESTS=true, runs both migration sets, and loads the module. Please add a mandatory gate with the flag absent, core migrations only, normal API boot, instant booking, and partial/full Stripe refunds. That is the safety contract this split is intended to protect.

Additional hardening:

I am happy to review the updated package again. The feature work is strong; the blockers are about making the isolation and deployment behavior match the agreed architecture.

cursoragent and others added 3 commits August 27, 2026 09:11
Resolve guest-comms email stack conflicts using main (#349).
Regenerate README test counts after merge.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Resolve conflicts with #349#353: take main for confirmation-number,
connect events, and connect-booking token re-export; merge Stripe
financial-state so core one-arg classifyHaipMetadata coexists with
booking-request correlation helpers and webhook delegation.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
- Ship runnable migrator + SQL in dist; production db:migrate via node
- Wire VITE_HAIP_BOOKING_REQUESTS through Docker/compose/release
- Reject bookingMode=request when HAIP_BOOKING_REQUESTS is off
- Ledger + checksum for package migrations (auto-commit for PG enums)
- Drop unused stripeHandlerToken from root module options
- Fix EmailResult outcomeUnknown after #349 status contract
- Point regression/e2e installs at run-migrations.js (post-#350)
- Harden push-schema CLI path resolution; sync README (2175/259)

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@telivity-otaip

Copy link
Copy Markdown
Collaborator Author

Thanks @agustinjch — working through the packaging/safety pass.

Landed on 61c86b3 (plus earlier main merge 99ed4f2):

  • Runnable package migrator shipped in dist + SQL assets; Docker copies booking-requests node_modules
  • VITE_HAIP_BOOKING_REQUESTS wired through Dockerfile / compose / release build-args
  • bookingMode=request rejected when HAIP_BOOKING_REQUESTS is off (API + settings UI)
  • Package migration ledger (booking_requests_schema_migrations + checksum); per-file auto-commit because PG forbids using a new enum value in the same transaction as ADD VALUE
  • Removed unused stripeHandlerToken from root module options
  • Fixed mailer for feat(email): add bounded timeouts for outbound email providers #349 EmailResult.status / outcomeUnknown
  • Regression installs now use run-migrations.js (post-feat(webhook): dedupe deliveries with logicalEventId #350)

Still open (as you called out): moving the full vertical slice behind packages/booking-requests, and fully removing request-only DDL from core push-schema. Those are the remaining architecture blockers before this is truly mergeable as an optional package.

Appreciate the review.

cursoragent and others added 3 commits August 27, 2026 18:55
#350 moved webhook logical_event_id to migration 0022. Remove the column
and unique index from push-schema so the pre-0022 upgrade regression
passes again.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Matches CI after push-schema logical_event_id cleanup (all packages green).

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
…y specs

- Add a flag-OFF default-install regression spec: only core migrations run,
  HAIP_BOOKING_REQUESTS is unset, AppModule boots without the
  booking-requests module/tables, request mode is rejected, and instant
  booking + deposit capture + partial/full refund still work. Runs
  automatically under `pnpm test` (apps/api's normal *.spec.ts glob), so it
  is wired into CI without any workflow changes.
- Extract the ephemeral-database subprocess helpers shared by that spec and
  the existing default-flow regression spec into
  regression-database-utils.ts (createdb/dropdb, sanitized child-process
  errors) instead of duplicating them.
- Port PR #347's booking-request-schema.spec.ts and
  booking-request-migration-safety.spec.ts into the package, scoped to the
  tables/migrations this package now owns (the duplicate push-schema DDL
  those specs cross-checked no longer exists — it moved into this package's
  migrations).
- Port PR #347's booking-request-remediation.postgres.spec.ts, replaying
  migration 0032's SQL directly (instead of through push-schema) since the
  ledger-based migrator can't be re-run against a manually-reverted schema.
  Kept opt-in via BOOKING_REQUEST_REMEDIATION_LIVE_PG like the original.
- Document remaining scope (vertical-slice move, push-schema de-pollution)
  in the package README instead of attempting it in this pass.
- Sync README/test-stats test counts (2211 tests, 263 files).

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@telivity-otaip

Copy link
Copy Markdown
Collaborator Author

Follow-up on bb761dd (on top of 61c86b3):

  • Added the mandatory flag-OFF default-install regression gate: HAIP_BOOKING_REQUESTS unset, only core migrations run, AppModule boots without the booking-requests module (no booking_requests tables, no BookingRequestService provider), bookingMode=request is rejected, and instant booking + deposit capture + partial/full Stripe refund still pass. It's a normal *.spec.ts file so it runs under pnpm test without any CI workflow changes.
  • Extracted the ephemeral-database subprocess helpers shared by that spec and the existing default-flow regression spec into regression-database-utils.ts instead of duplicating them.
  • Ported feat(booking-engine): add request-first booking workflow #347's booking-request-schema.spec.ts, booking-request-migration-safety.spec.ts, and booking-request-remediation.postgres.spec.ts into the package. Scoped the first two to the tables/migrations this package now owns (the push-schema duplication they used to cross-check is gone — that DDL lives only in the package's migrations now). The remediation spec now replays migration 0032's SQL directly instead of going through push-schema, since the ledger-based migrator can't be re-run against a manually-reverted schema the way push-schema's idempotent re-apply could; kept it opt-in via BOOKING_REQUEST_REMEDIATION_LIVE_PG like the original.
  • README/test-stats synced (2211 tests, 263 files).

Still open, documented in packages/booking-requests/README.md: moving the vertical slice (controllers/services/UI) behind the package, and fully de-duplicating the remaining request-related columns core push-schema still carries.

cursoragent and others added 4 commits August 27, 2026 21:15
…ethod_collection/form_questions

Adds BOOKING_REQUEST_CONFIG_FIELDS_PORT + DrizzleBookingRequestConfigFieldsAdapter
so the package owns reading/writing booking_engine_config's request-mode-only
columns via its own Drizzle table fragment, instead of core declaring them.
Wired into BookingRequestModule.forRoot() (global) so it's injectable into
core's BookingEngineConfigService without that module importing this package.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Removes booking_mode/payment_method_collection/form_questions ALTERs on
booking_engine_config, audit_logs.booking_request_id (+ its timeline index),
and the request-shape unique indexes/checks on payments
(payments_property_request_*_unique, booking_request_parent_positive_check,
booking_request_child_shape_check) from core's push-schema.ts and Drizzle
schema. These are now declared and migrated exclusively by
packages/booking-requests. payments.booking_request_id/idempotency_key,
reservations.accepted_pricing_snapshot, and charges
adjusts_charge_id/source_key stay in core as documented in
push-schema-kept-fields.spec.ts.

Also adds the DRIZZLE injection token to @telivityhaip/database so packages
outside apps/api (booking-requests) can inject the shared Drizzle client
without importing apps/api.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Own controllers/services/DTOs behind package ports; strip request-only
audit DDL and payment indexes from core schema; keep thin config/payment hooks.

- Nest controllers, DTOs, services, Stripe handler, and pricing/money/
  state/db/ledger/reconciler/template helpers now live in
  packages/booking-requests/src (http/ + domain/), with their unit specs.
  BookingRequestModule.forRoot(...) is a real DynamicModule owning those
  controllers/providers directly, not a facade over apps/api classes.
- apps/api/src/modules/booking-request/ keeps only the e2e, authorization,
  default-flow-regression, flag-off-instant-booking.regression, and
  transaction-seams specs plus regression-database-utils.ts.
- apps/api/src/booking-requests.bootstrap.ts wires every package-local port
  (folio, webhook, email, reservation, rate-plan, guest, ancillary,
  availability, booking-engine, booking-engine-config, plus guard-bridge
  ports) to the concrete core singleton via `useExisting`.
- Guard bridge classes (BookingKeyGuardBridge, BookingEngineScopeGuardBridge,
  BookingThrottleGuardBridge) resolve the "guards landmine": @UseGuards(...)
  is populated from decorator metadata, a separate path from `providers`, so
  a bare useExisting binding on an abstract port class is silently dropped —
  the bridges are real injectable classes referenced in @UseGuards(...).
- DRIZZLE token declaration moved to @telivityhaip/database; apps/api's
  DatabaseModule still @Global-provides it and merges in the package's
  optional schema only when HAIP_BOOKING_REQUESTS is on.
- Relocated pure/shared pieces to packages/shared: SAVED_PAYMENT_METHOD_GATEWAY
  / PAYMENT_GATEWAY / BOOKING_REQUEST_STRIPE_HANDLER interfaces, IsMoneyString,
  canonical calendar date validators, stayDates, AuditActor helpers,
  RequirePermissions/Public decorators, stripe-financial-state helpers, and
  the pure payment-ledger math (remainingCapturedAmount/sumRefundChildren) —
  bookingRequestPaymentSumWhere stays in core payment-ledger.
- Schema de-pollution: removed audit_logs.booking_request_id (+ its timeline
  index) and the request-shape payments unique indexes/checks from core
  push-schema/drizzle; kept booking_engine_config.booking_mode /
  payment_method_collection / form_questions, payments.booking_request_id /
  idempotency_key, reservations.accepted_pricing_snapshot, and charges
  adjusts_charge_id/source_key as thin config/payment hooks core still reads
  directly. packages/database/src/push-schema-kept-fields.spec.ts guards the
  contract; packages/booking-requests declares its own local audit table
  extension for the timeline index it still owns.
- EventEmitterModule.forRoot() re-enabled `wildcard: true` — required for the
  webhook fan-out (@onevent('**')) to receive booking-request events.
- README's "Package boundary" section replaces the old "Remaining work"
  deferrals list with an accurate description of the current split.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
Shared now requires @nestjs/common at runtime after the booking-requests
boundary move; copy packages/shared/node_modules into the API image so
demo smoke can boot. Sync published test counts to 2222/266.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@telivity-otaip

Copy link
Copy Markdown
Collaborator Author

@agustinjch All six architecture blockers from your review are done on head 05e6b31 — CI is green (lint/typecheck, tests, build, release smoke, one-command demo smoke).

# Item Status
1 Runnable package migrator in dist + SQL in Docker Done
2 VITE_HAIP_BOOKING_REQUESTS through Docker/release Done
3 Fail-safe: reject bookingMode=request when HAIP_BOOKING_REQUESTS off Done
4 Real package boundary — Nest vertical slice behind packages/booking-requests Done
5 Request-only DDL out of core push-schema / drizzle Done
6 Flag-OFF default-install regression (core migrate only, instant book + Stripe refunds) Done

Also landed: migration ledger + checksum, unused stripeHandlerToken removed, #349#353 rebase, #347 migration safety/remediation specs ported into the package, README counts synced (2222 / 266).

Package boundary now: controllers/DTOs/domain services/Stripe handler/unit specs live under packages/booking-requests. BookingRequestModule.forRoot(...) is a real DynamicModule with package-local ports; apps/api only binds them via booking-requests.bootstrap.ts (useExisting) when the flag is on. apps/api/src/modules/booking-request/ keeps only e2e/auth/regression specs.

Core schema: request-only audit_logs.booking_request_id (+ index) and request-shape payment indexes/checks removed from core. Thin hooks kept on purpose: payments.booking_request_id / idempotency_key, reservations.accepted_pricing_snapshot, charges.source_key / adjusts_charge_id, and booking_engine_config.booking_mode / payment_method_collection / form_questions (fail-safe gate still runs with the flag off). Contract in packages/database/src/push-schema-kept-fields.spec.ts.

Ready for your re-review / merge whenever you are. Thanks again for the packaging pass — it made the isolation real.

When HAIP_BOOKING_REQUESTS=true, AppModule evaluates bookingRequestsModules()
at import time; dynamic-import AppModule after preload so flag-on dev/prod
boot does not crash before Nest starts.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@telivity-otaip

Copy link
Copy Markdown
Collaborator Author

Follow-up on e71537b (CI green again): extra verification caught a flag-on boot crash — AppModule was evaluating bookingRequestsModules() at import time before preloadBookingRequestsModules() ran in main.ts, so HAIP_BOOKING_REQUESTS=true could not start in dev/prod (tests passed because they preload manually; demo smoke passes because the flag defaults off). Fixed by dynamic-importing AppModule after preload. Re-verified flag-on dev/prod boot + booking-request routes, full suite, regressions, and e2e.

@telivity-otaip
telivity-otaip merged commit ee36db0 into main Aug 28, 2026
5 checks passed
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.

3 participants