feat(quotes): promote the quote stub to a real entity + quote_lines (PMS-671) #452

Merged

longjacksonle merged 2 commits from feat/PMS-671-quotes-entity-schema into main

2026-07-21 18:46:14 +02:00

longjacksonle commented

2026-07-21 18:44:10 +02:00

Owner

What

PMS-671, phase 1 of the PMS-670 Quotes epic. Promotes the deliberate quotes stub into the real sales entity and adds quote_lines.

078_change_requests_and_quotes.sql (PMS-484) created quotes purely so the PMS-470 polymorphic approvals routes had a parent row to existence-check against. Its own header says the richer sales workflow is its own follow-up and that it is "intentionally NOT trying to be the final shape". This is that follow-up.

Migrations are immutable, so 078 is untouched and everything lands in a new 092.

Design notes worth reviewing

Client sign-off is not modelled on ticket_approvals. That table constrains approvers to internal staff (approver_user_id XOR approver_role, both staff concepts), so it cannot express "the customer signed off". Internal approval stays on the existing approvals surface with no changes; client acceptance becomes state on the quote itself (decided_at, decided_by_contact_id, decision_notes). Two different actors, two different events.

Money reconciled to DECIMAL(12,2) / VARCHAR(3). The stub's total_cents BIGINT / CHAR(3) was the odd one out; invoices, invoice_lines, and projects.budget_amount all use DECIMAL. Reconciling means a quote total, an invoice total, and a project budget compare without unit conversion at every call site. total_cents is retained as deprecated-but-readable rather than dropped, the same posture PMS-470 took with the legacy ticket_approvals.ticket_id.

accepted / declined are deliberately distinct from approved / rejected. The four values 078 shipped keep their exact meaning as the internal outcomes, so a reader can always tell a client decision from an internal one.

company_id is NOT NULL. Safe because no production code path inserts into quotes (there is no quotes module yet; the only INSERT in the tree is an integration test), so the table is empty in every real database. Written as add-then-enforce rather than a single NOT NULL add so a future backfill has an obvious seam.

UNIQUE (converted_project_id) is the real idempotency guard for the phase-4 conversion: an application-level "have we converted yet?" check can be passed by two concurrent requests, a unique index cannot.

Verify

Applied against a real Postgres that already had 078 through 091, then verified directly:

Migration applies cleanly; a second sqlx migrate run is a no-op.
company_id NOT NULL enforced; a company-less insert is rejected.
Status CHECK accepts all ten lifecycle values and rejects an unknown one.
converted_project_id UNIQUE rejects a second quote linking to the same project.
quote_number is unique per tenant while multiple NULLs coexist.
updated_at trigger fires on UPDATE (verified across separate transactions, since NOW() is frozen within one).
currency is no longer blank-padded.

cargo test --test approvals_polymorphic passes 5/5 including quote_approval_round_trip. Because sqlx::test provisions a fresh database and runs every migration, that also proves 092 applies from scratch and the existing /quotes/{id}/approvals routes still work against the richer table.

Full integration suite passes. just check is clean, including the migration prefix and immutability guards.

One pre-existing unrelated failure: tests/readiness.rs::ready_returns_ok_when_db_reachable_and_infisical_unconfigured fails locally because the dev .env supplies Infisical vars to the container while the Infisical profile is not running, so /ready 503s instead of reporting the probe as skipped. Confirmed identical on a clean main tree with these changes stashed, and CI does not set those vars.

Follow-ups

Phases 2 to 5 are PMS-672 (module CRUD with server-computed totals), PMS-673 (send + portal accept/decline), PMS-674 (convert to Project), PMS-675 (SPA).

## What PMS-671, phase 1 of the PMS-670 Quotes epic. Promotes the deliberate `quotes` stub into the real sales entity and adds `quote_lines`. `078_change_requests_and_quotes.sql` (PMS-484) created `quotes` purely so the PMS-470 polymorphic approvals routes had a parent row to existence-check against. Its own header says the richer sales workflow is its own follow-up and that it is "intentionally NOT trying to be the final shape". This is that follow-up. Migrations are immutable, so `078` is untouched and everything lands in a new `092`. ## Design notes worth reviewing **Client sign-off is not modelled on `ticket_approvals`.** That table constrains approvers to internal staff (`approver_user_id` XOR `approver_role`, both staff concepts), so it cannot express "the customer signed off". Internal approval stays on the existing approvals surface with no changes; client acceptance becomes state on the quote itself (`decided_at`, `decided_by_contact_id`, `decision_notes`). Two different actors, two different events. **Money reconciled to `DECIMAL(12,2)` / `VARCHAR(3)`.** The stub's `total_cents BIGINT` / `CHAR(3)` was the odd one out; `invoices`, `invoice_lines`, and `projects.budget_amount` all use DECIMAL. Reconciling means a quote total, an invoice total, and a project budget compare without unit conversion at every call site. `total_cents` is retained as deprecated-but-readable rather than dropped, the same posture PMS-470 took with the legacy `ticket_approvals.ticket_id`. **`accepted` / `declined` are deliberately distinct from `approved` / `rejected`.** The four values `078` shipped keep their exact meaning as the internal outcomes, so a reader can always tell a client decision from an internal one. **`company_id` is NOT NULL.** Safe because no production code path inserts into `quotes` (there is no quotes module yet; the only INSERT in the tree is an integration test), so the table is empty in every real database. Written as add-then-enforce rather than a single NOT NULL add so a future backfill has an obvious seam. **`UNIQUE (converted_project_id)`** is the real idempotency guard for the phase-4 conversion: an application-level "have we converted yet?" check can be passed by two concurrent requests, a unique index cannot. ## Verify Applied against a real Postgres that already had `078` through `091`, then verified directly: - Migration applies cleanly; a second `sqlx migrate run` is a no-op. - `company_id` NOT NULL enforced; a company-less insert is rejected. - Status CHECK accepts all ten lifecycle values and rejects an unknown one. - `converted_project_id` UNIQUE rejects a second quote linking to the same project. - `quote_number` is unique per tenant while multiple NULLs coexist. - `updated_at` trigger fires on UPDATE (verified across separate transactions, since `NOW()` is frozen within one). - `currency` is no longer blank-padded. `cargo test --test approvals_polymorphic` passes 5/5 including `quote_approval_round_trip`. Because `sqlx::test` provisions a fresh database and runs every migration, that also proves `092` applies from scratch and the existing `/quotes/{id}/approvals` routes still work against the richer table. Full integration suite passes. `just check` is clean, including the migration prefix and immutability guards. One pre-existing unrelated failure: `tests/readiness.rs::ready_returns_ok_when_db_reachable_and_infisical_unconfigured` fails locally because the dev `.env` supplies Infisical vars to the container while the Infisical profile is not running, so `/ready` 503s instead of reporting the probe as skipped. Confirmed identical on a clean `main` tree with these changes stashed, and CI does not set those vars. ## Follow-ups Phases 2 to 5 are PMS-672 (module CRUD with server-computed totals), PMS-673 (send + portal accept/decline), PMS-674 (convert to Project), PMS-675 (SPA).

longjacksonle added 2 commits

2026-07-21 18:44:10 +02:00

feat(quotes): promote the quote stub to a real entity + quote_lines (PMS-671) e233923d67

078 (PMS-484) shipped `quotes` purely so the polymorphic approvals routes had a parent row to existence-check against, and its own header says the richer sales workflow is a follow-up that is "intentionally NOT trying to be the final shape". This is that follow-up, phase 1 of the PMS-670 Quotes epic.

Adds the associations the entity was missing: `company_id` (the client, mandatory - the portal scopes every client-visible read by the signed-in contact's company), `billing_contact_id`, `quote_number` with a per-tenant `quote_sequences` table mirroring `invoice_sequences`, a real `description` for the statement of work, `valid_until`, and the `sent_at` / `decided_at` / `decided_by_contact_id` / `decision_notes` sign-off record.

Client sign-off is deliberately NOT modelled on `ticket_approvals`: that table constrains approvers to internal staff (`approver_user_id` XOR `approver_role`, both staff concepts) and so cannot express "the customer signed off". Internal approval stays on the existing approvals surface untouched; client acceptance lives on the quote itself.

Reconciles money to the `DECIMAL(12,2)` / `VARCHAR(3)` the rest of the system uses, so a quote total, an invoice total, and a project budget compare without unit conversion. `total_cents` is retained as deprecated-but-readable rather than dropped, the same posture PMS-470 took with the legacy `ticket_approvals.ticket_id`.

Widens the status CHECK to the full lifecycle. The four values 078 shipped keep their exact meaning as the internal outcomes; `accepted` / `declined` are deliberately different words from `approved` / `rejected` so a reader can always tell a client decision from an internal one.

Adds `quote_lines` mirroring `invoice_lines`, a UNIQUE on `converted_project_id` so the phase-4 conversion cannot create two projects even under concurrency, and the standard `updated_at` trigger (078 created the table after the 024/043 trigger sweeps, so nothing was maintaining the column).

The NOT NULL adds are safe: no production code path inserts into `quotes` (there is no quotes module yet; the only INSERT in the tree is an integration test), so the table is empty in every real database.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

test(approvals): seed a company for the quote fixture (PMS-671)

E2E / Playwright against staging (pull_request) Successful in 40s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 1m42s

Details

Create release / Gate (release-branch merges only) (pull_request) Successful in 0s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

Integration / integration tests (pull_request) Successful in 8m35s

Details

2b97c45c60

`quotes.company_id` is now mandatory, so the polymorphic-approvals quote fixture has to seed a company. Also switches the seeded amount from the deprecated `total_cents` to the new `total DECIMAL(12,2)`, so the fixture exercises the column new code will actually read.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>