feat(quotes): send to client + portal accept/decline sign-off (PMS-673) #454

Merged

longjacksonle merged 5 commits from feat/PMS-673-quote-send-and-portal-signoff into main 2026-07-21 19:33:45 +02:00

Conversation 0 Commits 5 Files changed 8 +1325 -7

longjacksonle commented 2026-07-21 19:30:30 +02:00

Owner

Copy link

What

PMS-673, phase 3 of the PMS-670 Quotes epic, and the part that delivers the ticket's stated requirement: quotes can be sent to clients for sign-off before work starts.

Stacked on feat/PMS-672-quotes-module-crud (PR #453). Review and merge that one first; this branch targets main so its diff will shrink to just these five commits once #453 lands.

Routes

Staff: POST /quotes/{id}/send
Portal: GET /portal/quotes · GET /portal/quotes/{id} · POST /portal/quotes/{id}/accept · POST /portal/quotes/{id}/decline

Design notes worth reviewing

Why client acceptance is not on the approvals table. ticket_approvals 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 that surface completely untouched and remains the gate for sending; client acceptance is state on the quote itself. Two different actors, two different events.

Send is gated on approved and mails after commit. A mail failure must not roll back a transition the customer may already have been told about out of band, and the reverse is recoverable by resending, so the transition is the durable half. Delivery failures are logged rather than surfaced: the quote is already sent, and a bounced mail must not become a 500 that makes the caller retry a transition that already happened.

Nothing pre-sent reaches the portal. is_client_visible excludes draft / submitted (unfinished), approved (only means staff cleared it to go out), and rejected / cancelled (killed internally, so showing them would leak a negotiation the customer was never part of). A unit test pins the predicate against the SQL bind list so a future status cannot be added to one and forgotten in the other.

404, not 403, throughout. A quote belonging to another company, or not yet issued, is 404 on read AND on decide, so a contact cannot learn it exists by watching the error change from 404 to 409.

Expiry is derived, not swept. effective_status reinterprets a sent quote past valid_until as expired at read time. There is then no window in which an expired quote is still acceptable and no background machinery to operate; the cost is that the stored column stays sent until something writes the row. Only sent is reinterpreted, so an accepted or converted quote does not expire retroactively. valid_until is inclusive, so a customer signing on the deadline is not turned away.

Its own rate limiter. Separate from PortalLoginLimiter because the buckets mean different things: login throttles credential guessing by an unauthenticated caller, this throttles actions by an already authenticated contact. Sharing quota would let a burst of decisions lock the contact out of logging back in.

Verify

6 integration tests in tests/quote_signoff.rs plus 3 new unit tests:

• Send is refused from draft (409), succeeds from approved, stamps sent_at, is refused a second time, and is audited.
• A portal contact sees exactly their own company's issued quote; their own company's draft and another company's issued quote are both 404 by direct id, and the other company's quote also 404s on accept.
• Accept records contact / timestamp / notes, cannot be repeated, and cannot be flipped to decline. The quote stays visible to the client afterwards.
• Decline works with no request body at all.
• The expiry test backdates valid_until, asserts the stored status is still sent while the read reports expired and accept 409s, then sets valid_until to today and accepts successfully, pinning the boundary as inclusive.
• A staff bearer token is rejected outright on the portal sign-off, and anonymous access 401s.

Full suite passes and clippy is clean at CI strength (--all-targets -- -D warnings). Same pre-existing unrelated tests/readiness.rs failure as #453, confirmed identical on clean main.

Follow-ups

PMS-674 (convert an accepted quote to a Project), PMS-675 (SPA).

## What PMS-673, phase 3 of the PMS-670 Quotes epic, and the part that delivers the ticket's stated requirement: quotes can be sent to clients for sign-off before work starts. **Stacked on `feat/PMS-672-quotes-module-crud` (PR #453). Review and merge that one first;** this branch targets `main` so its diff will shrink to just these five commits once #453 lands. ## Routes Staff: `POST /quotes/{id}/send` Portal: `GET /portal/quotes` · `GET /portal/quotes/{id}` · `POST /portal/quotes/{id}/accept` · `POST /portal/quotes/{id}/decline` ## Design notes worth reviewing **Why client acceptance is not on the approvals table.** `ticket_approvals` 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 that surface completely untouched and remains the gate for sending; client acceptance is state on the quote itself. Two different actors, two different events. **Send is gated on `approved` and mails after commit.** A mail failure must not roll back a transition the customer may already have been told about out of band, and the reverse is recoverable by resending, so the transition is the durable half. Delivery failures are logged rather than surfaced: the quote is already `sent`, and a bounced mail must not become a 500 that makes the caller retry a transition that already happened. **Nothing pre-`sent` reaches the portal.** `is_client_visible` excludes `draft` / `submitted` (unfinished), `approved` (only means staff cleared it to go out), and `rejected` / `cancelled` (killed internally, so showing them would leak a negotiation the customer was never part of). A unit test pins the predicate against the SQL bind list so a future status cannot be added to one and forgotten in the other. **404, not 403, throughout.** A quote belonging to another company, or not yet issued, is 404 on read AND on decide, so a contact cannot learn it exists by watching the error change from 404 to 409. **Expiry is derived, not swept.** `effective_status` reinterprets a `sent` quote past `valid_until` as `expired` at read time. There is then no window in which an expired quote is still acceptable and no background machinery to operate; the cost is that the stored column stays `sent` until something writes the row. Only `sent` is reinterpreted, so an accepted or converted quote does not expire retroactively. `valid_until` is inclusive, so a customer signing on the deadline is not turned away. **Its own rate limiter.** Separate from `PortalLoginLimiter` because the buckets mean different things: login throttles credential guessing by an unauthenticated caller, this throttles actions by an already authenticated contact. Sharing quota would let a burst of decisions lock the contact out of logging back in. ## Verify 6 integration tests in `tests/quote_signoff.rs` plus 3 new unit tests: - Send is refused from `draft` (409), succeeds from `approved`, stamps `sent_at`, is refused a second time, and is audited. - A portal contact sees exactly their own company's issued quote; their own company's draft and another company's issued quote are both 404 by direct id, and the other company's quote also 404s on accept. - Accept records contact / timestamp / notes, cannot be repeated, and cannot be flipped to decline. The quote stays visible to the client afterwards. - Decline works with no request body at all. - The expiry test backdates `valid_until`, asserts the stored status is still `sent` while the read reports `expired` and accept 409s, then sets `valid_until` to today and accepts successfully, pinning the boundary as inclusive. - A staff bearer token is rejected outright on the portal sign-off, and anonymous access 401s. Full suite passes and clippy is clean at CI strength (`--all-targets -- -D warnings`). Same pre-existing unrelated `tests/readiness.rs` failure as #453, confirmed identical on clean `main`. ## Follow-ups PMS-674 (convert an accepted quote to a Project), PMS-675 (SPA).

longjacksonle added 9 commits 2026-07-21 19:30:30 +02:00

feat(quotes): quote + line DTOs and the lifecycle status model (PMS-672) ... 5955c86793

Phase 2 of the PMS-670 Quotes epic. Adds the DTO layer over the schema PMS-671 landed: `QuoteStatus` and `QuoteLineType` mirroring their CHECK constraints, the request/response shapes, and the filter.

Three predicates on `QuoteStatus` carry the rules the rest of the module enforces, kept here so they are unit-testable and stated once. `is_frozen` covers the issued and terminal states (`sent` onwards, plus `cancelled`) where no staff write is legitimate at all. `allows_content_edit` is deliberately narrower than the inverse: a quote in `submitted` or `approved` still has to advance through the workflow, but its figures must not move underneath the approver looking at them, so content edits are confined to `draft` and `rejected`. `is_staff_settable` excludes every status owned by another actor or route (`sent`, the client's `accepted`/`declined`, derived `expired`, and `converted`), so a staff user cannot forge a client's acceptance with a plain header update.

`CreateQuoteRequest` deliberately has no `subtotal` or `total` field. Totals are derived from the lines server-side, so accepting one would only invite the caller to believe it mattered.

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

feat(quotes): quotes service with server-computed totals (PMS-672) ... 2bcc11723e

CRUD over quotes and their line items. Mirrors `BillingService` throughout, since invoices are the same parent/child money shape: gapless per-tenant numbering via a row-locked `quote_sequences` bump inside the caller's transaction, `begin_with_tenant` on every method so RLS scopes the statements, and the same audit-write-in-the-same-transaction posture.

`recompute_totals` is the single place totals are derived, and every mutating path funnels through it, so a stored total always equals the sum of the quote's lines plus tax. It runs even when only `tax_amount` changed, because that moves `total` without touching a line.

`assert_company_in_tenant` and `assert_contact_in_tenant` reject ids belonging to another tenant. The foreign keys alone are not enough: FK checks bypass RLS, so a caller passing a foreign company id would otherwise get a quote that silently links across tenants. Same guard and same reason as `assert_payment_term_in_tenant` (PMS-333).

`DELETE` is modelled as a `cancelled` transition rather than a row delete. A quote that reached a customer is a commercial record, and the approvals rows referencing it (`target='quote'`) would be orphaned by a hard delete.

Unit tests pin the three status predicates and the string round-trips.

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

feat(quotes): mount the quotes routes under /api/v1 (PMS-672) ... b630b68f40

Routes for the quote and its lines, merged rather than nested so the URLs stay flat and sit alongside the `/quotes/{id}/approvals` paths that `modules::approvals` already owns.

RBAC mirrors the other money-bearing surfaces exactly rather than inventing a policy: the `billing` module gate plus `RequireFinance`, the same pair `invoices`, `contracts`, and `rate-cards` use. PMS-350 established that financial surfaces are finance-gated on reads as well as writes, and a quote is a priced commercial document, so it belongs in that set.

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

test(quotes): integration coverage for CRUD, totals, freezing, tenancy (PMS-672) ... 9196a2bf80

Pins the guarantees the ticket calls out: CRUD round-trip with per-tenant number allocation, totals recomputed on every line mutation, a caller-supplied total never persisted, `sent` quotes rejecting every edit with 409, content freezing at `submitted` while the status still advances, staff unable to forge the client's decision, and quotes invisible across tenants on every route including the list.

The cross-tenant test enables the `billing` module for the second tenant on purpose: without it the module gate 404s before the tenant check is reached and the test would pass vacuously. It also needs its own login helper, because PMS-138 binds the login lookup to `(tenant_id, email)` and falls back to the default tenant, so `common::login` can only ever reach default-tenant users.

Also adds `/api/v1/quotes` to the RBAC coverage matrix next to the other financial surfaces, and asserts the pre-existing approvals surface still works against a quote created through the new API rather than a raw INSERT.

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

feat(email): quote-ready template for the client sign-off link (PMS-673) ... ece3d9a69f

Adds `Mailer::send_quote_ready`, the mail that carries a client the link to accept or decline their quote. Implemented as a default method composing a plain-text body over `send_text`, so every mailer inherits it without a per-impl override, matching how PMS-657 and PMS-658 added theirs.

`valid_until` is optional: a quote need not carry an expiry, and when it does not the body omits the deadline line rather than printing a placeholder date a customer might act on.

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

feat(portal): rate limiter for the quote decision routes (PMS-673) ... f657485b0d

Accepting or declining a quote is a commercial commitment that the client cannot reverse, so the routes get their own throttle: 30/min per IP, 10/min per contact.

Deliberately a separate limiter from `PortalLoginLimiter` rather than a reused one. The buckets mean different things: login throttles credential guessing by an unauthenticated caller, this throttles actions by an already authenticated contact. Sharing quota would let a burst of decisions lock the contact out of logging back in. Quotas are roomier than login's for the same reason: a contact clicking through several quotes in one sitting is normal behaviour, not an attack.

Same in-memory, per-replica caveat as the login limiter. The durable guarantee is the state machine, which accepts a decision only from `sent` and 409s every repeat.

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

feat(quotes): issue to the client and record their sign-off (PMS-673) ... 7ff7d54442

Phase 3 of the PMS-670 Quotes epic, and the part that delivers the ticket's actual requirement: quotes can be sent to clients for sign-off before work starts.

`send_quote` is allowed only from `approved`. Internal sign-off is the gate and it runs on the existing polymorphic approvals surface, which this change does not touch. The mail goes out AFTER the transaction commits: a mail failure must not roll back a transition the customer may already have been told about out of band, and the reverse is recoverable by resending, so the transition is the durable half. Delivery failures are logged rather than surfaced, because the quote is already `sent` and a bounced mail must not become a 500 that makes the caller retry a transition that already happened.

Client acceptance lives on the quote itself rather than on `ticket_approvals`, which constrains approvers to internal staff (`approver_user_id` XOR `approver_role`) and so cannot express "the customer signed off". `decide_quote` checks the company scope first and as a 404, so a contact cannot learn another company's quote exists by watching the error change from 404 to 409.

`is_client_visible` keeps everything before `sent` out of the portal: a `draft` or `submitted` quote is unfinished, `approved` only means staff cleared it to go out, and `rejected` / `cancelled` were killed internally, so showing them would leak a negotiation the customer was never part of.

Expiry is applied at read time by `effective_status` rather than by a sweeper. There is then no window in which an expired quote is still acceptable and no background machinery to operate; the cost is that the stored status stays `sent` until something writes the row. Only `sent` is reinterpreted, so a quote already accepted or converted does not expire retroactively. `valid_until` is inclusive, so a customer signing on the deadline is not turned away.

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

feat(quotes): send route plus the portal sign-off routes (PMS-673) ... fd88bdec24

Adds `POST /quotes/{id}/send` on the staff surface, and `/portal/quotes` (list), `/portal/quotes/{id}` (detail), and `/portal/quotes/{id}/accept|decline` on the client surface.

The portal reads force the company scope from the authenticated `CurrentContact` rather than a query param, so a contact only ever sees its own company's quotes, and a quote that is not theirs or not yet issued returns 404 rather than 403 so the portal never confirms it exists. Same posture as the existing portal invoice routes.

Accept and decline share one handler body, since they differ only in the outcome recorded, and their JSON body is optional: accepting with nothing to say is the common case and requiring `{}` would be a needless 415.

`TicketService` and `notifications_routes` now take clones of the mailer and dispatcher they previously consumed, because quotes needs both as well.

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

test(quotes): integration coverage for send and client sign-off (PMS-673) ... 114254867e

Covers send being refused from `draft` and refused a second time, `sent_at` being stamped, and the transition landing in the audit log. On the portal side: a contact sees only their own company's issued quotes, an internal draft and another company's quote are both 404 by direct id, accept records contact/timestamp/notes and cannot be repeated or flipped to decline, decline works with no request body at all, and a staff bearer token is rejected on the portal sign-off entirely.

The expiry test backdates `valid_until` and asserts the stored status is still `sent` while the read reports `expired`, pinning that expiry is derived rather than written. It then sets `valid_until` to today and accepts successfully, pinning the boundary as inclusive so a customer signing on the deadline is not turned away.

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

longjacksonle merged commit b5c11a0db4 into main 2026-07-21 19:33:45 +02:00

longjacksonle deleted branch feat/PMS-673-quote-send-and-portal-signoff 2026-07-21 19:33:45 +02:00

longjacksonle referenced this pull request from a commit 2026-07-21 19:33:47 +02:00

Merge pull request 'feat(quotes): send to client + portal accept/decline sign-off (PMS-673)' (#454) from feat/PMS-673-quote-send-and-portal-signoff into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

psa-systems/mokosh-server!454

No description provided.