feat(quotes): convert an accepted quote into a Project (PMS-674) #455

Merged

longjacksonle merged 3 commits from feat/PMS-674-convert-quote-to-project into main

2026-07-21 20:20:24 +02:00

longjacksonle commented

2026-07-21 19:46:10 +02:00

Owner

What

PMS-674, phase 4 of the PMS-670 Quotes epic: the "upon approval, it becomes a project" step. Adds POST /quotes/{id}/convert, allowed only once the client has accepted.

The quote supplies the client, name, scope, and budget. The request supplies only what a quote cannot know: who runs it, when, and how it bills. Those mapped fields are deliberately absent from the request body, because letting the caller restate them would allow the project to disagree with the quote the client signed.

A correction to what PMS-671 claimed

PMS-671 added UNIQUE (converted_project_id) and I described it as the idempotency guard for this phase. That was imprecise, and worth flagging for review rather than burying.

That constraint prevents two quotes pointing at one project. The failure mode here is the opposite: one quote spawning two projects, and nothing in the schema stops a second UPDATE from overwriting the link. So the constraint does not cover this case at all.

The actual guard is taking the quote row with SELECT ... FOR UPDATE before reading anything off it. A second concurrent request blocks until the first commits, then observes converted and returns the existing link. The UNIQUE constraint stays as a backstop for its own (different) failure.

This is verified, not asserted: the concurrency test produces four distinct projects with the lock removed, and exactly one with it in place.

Other design notes

Converting twice is not an error. It returns the same converted_project_id rather than 409ing, because a double-clicked Convert button is a UI event, not a caller mistake.

Atomic. The project insert and the quote transition share one transaction, so a failure on either leaves the quote exactly as it was rather than marked converted with no project behind it. Tested with a violated FK.

billing_method defaults to fixed_price, not the projects column default of time_and_materials. An accepted quote IS a fixed price the client agreed to, so inheriting the column default would quietly contradict what was signed.

Returns the quote, not the project. Every other route in the module returns a QuoteResponse, and the response carries converted_project_id for a caller that wants to fetch the project.

Verify

6 integration tests in tests/quote_convert.rs, each driving the full real path (create, submit, approve, send, client accepts via the portal, then convert):

Field mapping onto the project: name, description, company, budget_amount = quote total, project_type=client, status=planning, billing_method=fixed_price, project manager. Both audit rows present.
Refused from draft AND from approved, with no project created. Internal approval is not the client's signature.
Converting twice returns the same project and leaves exactly one row.
16 concurrent conversions: every racer 200s (no 500 leaks to the loser), all agree on one project id, and exactly one project exists.
A violated FK leaves the quote at accepted with no dangling link and no project.
Unknown billing method and inverted dates are 422 from the validator rather than 500s from the database CHECK, and leave the quote untouched.

Note on the concurrency test shape: two racers via tokio::join! is not enough. They serialise on the reqwest connection pool and the test passes even with the row lock removed. I found this by deliberately removing FOR UPDATE and watching the test still pass, then widened it to 16 tokio::spawned racers, which fails correctly without the lock.

Full suite passes and clippy is clean at CI strength (--all-targets -- -D warnings). Same pre-existing unrelated tests/readiness.rs failure as the earlier phases (dev .env supplies Infisical vars while that profile is off), confirmed identical on clean main.

Follow-ups

PMS-675 (Quotes UI in mokosh-apps) is the last phase of the epic. Seeding project tasks from quote_lines was left out as the ticket allowed; worth its own ticket if wanted.

## What PMS-674, phase 4 of the PMS-670 Quotes epic: the "upon approval, it becomes a project" step. Adds `POST /quotes/{id}/convert`, allowed only once the client has accepted. The quote supplies the client, name, scope, and budget. The request supplies only what a quote cannot know: who runs it, when, and how it bills. Those mapped fields are deliberately absent from the request body, because letting the caller restate them would allow the project to disagree with the quote the client signed. ## A correction to what PMS-671 claimed PMS-671 added `UNIQUE (converted_project_id)` and I described it as the idempotency guard for this phase. That was imprecise, and worth flagging for review rather than burying. That constraint prevents **two quotes pointing at one project**. The failure mode here is the opposite: **one quote spawning two projects**, and nothing in the schema stops a second `UPDATE` from overwriting the link. So the constraint does not cover this case at all. The actual guard is taking the quote row with `SELECT ... FOR UPDATE` before reading anything off it. A second concurrent request blocks until the first commits, then observes `converted` and returns the existing link. The UNIQUE constraint stays as a backstop for its own (different) failure. This is verified, not asserted: the concurrency test produces **four distinct projects** with the lock removed, and exactly one with it in place. ## Other design notes **Converting twice is not an error.** It returns the same `converted_project_id` rather than 409ing, because a double-clicked Convert button is a UI event, not a caller mistake. **Atomic.** The project insert and the quote transition share one transaction, so a failure on either leaves the quote exactly as it was rather than marked `converted` with no project behind it. Tested with a violated FK. **`billing_method` defaults to `fixed_price`,** not the `projects` column default of `time_and_materials`. An accepted quote IS a fixed price the client agreed to, so inheriting the column default would quietly contradict what was signed. **Returns the quote, not the project.** Every other route in the module returns a `QuoteResponse`, and the response carries `converted_project_id` for a caller that wants to fetch the project. ## Verify 6 integration tests in `tests/quote_convert.rs`, each driving the full real path (create, submit, approve, send, client accepts via the portal, then convert): - Field mapping onto the project: name, description, company, `budget_amount` = quote total, `project_type=client`, `status=planning`, `billing_method=fixed_price`, project manager. Both audit rows present. - Refused from `draft` AND from `approved`, with no project created. Internal approval is not the client's signature. - Converting twice returns the same project and leaves exactly one row. - 16 concurrent conversions: every racer 200s (no 500 leaks to the loser), all agree on one project id, and exactly one project exists. - A violated FK leaves the quote at `accepted` with no dangling link and no project. - Unknown billing method and inverted dates are 422 from the validator rather than 500s from the database CHECK, and leave the quote untouched. Note on the concurrency test shape: two racers via `tokio::join!` is **not** enough. They serialise on the reqwest connection pool and the test passes even with the row lock removed. I found this by deliberately removing `FOR UPDATE` and watching the test still pass, then widened it to 16 `tokio::spawn`ed racers, which fails correctly without the lock. Full suite passes and clippy is clean at CI strength (`--all-targets -- -D warnings`). Same pre-existing unrelated `tests/readiness.rs` failure as the earlier phases (dev `.env` supplies Infisical vars while that profile is off), confirmed identical on clean `main`. ## Follow-ups PMS-675 (Quotes UI in mokosh-apps) is the last phase of the epic. Seeding project tasks from `quote_lines` was left out as the ticket allowed; worth its own ticket if wanted.

longjacksonle added 3 commits

2026-07-21 19:46:10 +02:00

feat(quotes): convert an accepted quote into a Project (PMS-674) a61298d208

Phase 4 of the PMS-670 Quotes epic: the "upon approval, it becomes a project" step. The quote supplies the client, name, scope, and budget; the request supplies only what a quote cannot know (who runs it, when, and how it bills). Name, scope, client, and budget are deliberately absent from the request body, because letting the caller restate them would allow the project to disagree with the quote the client signed.

Idempotency rests on taking the quote row with `SELECT ... FOR UPDATE` before reading anything off it, so a second concurrent request blocks until the first commits and then observes `converted` and returns the existing link. This is worth being precise about: the `UNIQUE (converted_project_id)` constraint from PMS-671 does NOT cover this case. It prevents two quotes pointing at one project, whereas the failure here is one quote spawning two projects, and nothing in the schema stops a second UPDATE overwriting the link. The row lock is the guard; the constraint remains a backstop. A 16-way concurrent test produces four distinct projects with the lock removed and exactly one with it in place.

Converting an already-converted quote returns the same project rather than 409ing: a double-clicked Convert button is not an error, and the caller gets the same `converted_project_id` either way.

The project insert and the quote transition share one transaction, so a failure on either leaves the quote exactly as it was rather than marked converted with no project behind it.

`billing_method` defaults to `fixed_price` rather than inheriting the `projects` column default of `time_and_materials`: an accepted quote IS a fixed price the client agreed to, so the column default would quietly contradict what was signed.

Returns the updated quote rather than the project, matching every other route in the module; the response carries `converted_project_id` for a caller that wants to fetch it.

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

feat(quotes): mount POST /quotes/{id}/convert (PMS-674) 302bcd0426

Same `billing` gate + `RequireFinance` pair as the rest of the module. The body is optional because every field on it is, so converting with no scheduling detail is a bare POST.

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

test(quotes): integration coverage for quote-to-project conversion (PMS-674)

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

Details

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

Details

Integration / integration tests (pull_request) Successful in 5m57s

Details

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

Details

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

Details

bb2e456186

Covers the field mapping onto the project, refusal from `draft` and from `approved` (internal approval is not the client's signature), double conversion returning the same project with exactly one row created, atomicity via a violated FK leaving the quote at `accepted` with no dangling link, validation rejecting an unknown billing method and inverted dates as 422, and both audit rows.

The concurrency test fires 16 racers through `tokio::spawn` rather than two through `tokio::join!`. Two is not enough: they serialise on the reqwest connection pool and the test passes even with the row lock removed, which is false confidence. At 16 the windows genuinely overlap, and the test was verified to fail (four distinct projects) with `FOR UPDATE` taken out.

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