feat(tickets): attach the procedure KB article to a ticket #495

Merged

longjacksonle merged 3 commits from feat/PMS-730-ticket-procedure-kb-article into main

2026-08-06 20:00:01 +02:00

longjacksonle commented

2026-08-06 19:38:00 +02:00

Owner

Groundwork for PMS-730 (client request forms / MACD): attach the KB article that describes HOW to perform the work a ticket asks for, so whoever picks the ticket up has the procedure in hand.

Corrects a wrong premise in the issue

PMS-730's background says "There is no ticket-to-article link anywhere in the schema". That is wrong: tickets.source_kb_article_id has existed since migration 068 (PMS-452), is bound on the ticket create path, and feeds the PMS-485 "top ticket-driving articles" widget. So this PR adds a SECOND link rather than a first, and the interesting question became whether to reuse the existing one.

Why a separate column rather than reusing `source_kb_article_id`

The two links point in opposite directions. source_kb_article_id records the article a ticket was opened FROM: the SPA offers "Open ticket about this article", stamps the column, and KnowledgeBaseService::list_top_ticket_driving_articles counts those stamps to answer "which articles drive the most tickets", i.e. where the docs are FAILING the user. A procedure link is the reverse: the ticket needs the article. Stamping the same column would fold every MACD request into that count and invert its meaning, reporting a working runbook as a documentation failure. Migration 099 carries the full rationale inline, and the new test pins the exclusion so a future refactor cannot quietly merge the two.

What is here

migrations/099_tickets_procedure_kb_article.sql: tickets.procedure_kb_article_id, FK to kb_articles with ON DELETE SET NULL (mirrors 068: retiring an article drops the linkage, not the ticket), plus a partial index on (tenant_id, procedure_kb_article_id) sized for PMS-732's "tracked time by request type, surfaced on the article" aggregation. No RLS work: a policy attaches per table, not per column, and tickets already carries the fail-closed tenant_isolation policy.
CreateTicketRequest accepts the field, defaulting to None, so the agent, portal, RMM and email-intake create paths are untouched. The create INSERT binds it.
Ticket and TicketResponse return it. TicketResponse also carries a JOINed procedure_kb_article_title, following the PMS-344 asset_id / asset_name shape, so the ticket detail can render the procedure link without a second fetch.
tests/knowledge_base.rs gains a test covering the round trip through create and get with the title resolved, the widget exclusion described above, and the ON DELETE SET NULL behaviour.

Deliberately not here

The change-type to article mapping that PMS-730 also implies. Its left-hand side is the request-type vocabulary that PMS-731's form definitions will own, so a standalone mapping table now would create a second vocabulary that PMS-731 then duplicates. It collapses naturally to a kb_article_id column on the form definition row and belongs in that issue.

Verification

cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, and the full cargo test --tests suite (714 passed, 0 failed, 71 test binaries) all green locally, plus check-migration-prefixes.nu, check-migration-immutability.nu and check-pool-safety.nu.

Note for anyone running the suite locally: just test-integration currently fails with 42501 permission denied to create database against a compose stack that has provisioned its roles, because src/db/provision.rs creates mokosh_migrator with LOGIN BYPASSRLS and no CREATEDB while #[sqlx::test] creates a database per test. CI is unaffected because integration.yml points DATABASE_URL at the postgres superuser. I ran with the superuser URL to match CI; the recipe divergence is worth its own fix and is not addressed here.

Groundwork for PMS-730 (client request forms / MACD): attach the KB article that describes HOW to perform the work a ticket asks for, so whoever picks the ticket up has the procedure in hand. ## Corrects a wrong premise in the issue PMS-730's background says "There is no ticket-to-article link anywhere in the schema". That is wrong: `tickets.source_kb_article_id` has existed since migration 068 (PMS-452), is bound on the ticket create path, and feeds the PMS-485 "top ticket-driving articles" widget. So this PR adds a SECOND link rather than a first, and the interesting question became whether to reuse the existing one. ## Why a separate column rather than reusing `source_kb_article_id` The two links point in opposite directions. `source_kb_article_id` records the article a ticket was opened FROM: the SPA offers "Open ticket about this article", stamps the column, and `KnowledgeBaseService::list_top_ticket_driving_articles` counts those stamps to answer "which articles drive the most tickets", i.e. where the docs are FAILING the user. A procedure link is the reverse: the ticket needs the article. Stamping the same column would fold every MACD request into that count and invert its meaning, reporting a working runbook as a documentation failure. Migration 099 carries the full rationale inline, and the new test pins the exclusion so a future refactor cannot quietly merge the two. ## What is here - `migrations/099_tickets_procedure_kb_article.sql`: `tickets.procedure_kb_article_id`, FK to `kb_articles` with `ON DELETE SET NULL` (mirrors 068: retiring an article drops the linkage, not the ticket), plus a partial index on `(tenant_id, procedure_kb_article_id)` sized for PMS-732's "tracked time by request type, surfaced on the article" aggregation. No RLS work: a policy attaches per table, not per column, and `tickets` already carries the fail-closed `tenant_isolation` policy. - `CreateTicketRequest` accepts the field, defaulting to `None`, so the agent, portal, RMM and email-intake create paths are untouched. The create INSERT binds it. - `Ticket` and `TicketResponse` return it. `TicketResponse` also carries a JOINed `procedure_kb_article_title`, following the PMS-344 `asset_id` / `asset_name` shape, so the ticket detail can render the procedure link without a second fetch. - `tests/knowledge_base.rs` gains a test covering the round trip through create and get with the title resolved, the widget exclusion described above, and the `ON DELETE SET NULL` behaviour. ## Deliberately not here The change-type to article mapping that PMS-730 also implies. Its left-hand side is the request-type vocabulary that PMS-731's form definitions will own, so a standalone mapping table now would create a second vocabulary that PMS-731 then duplicates. It collapses naturally to a `kb_article_id` column on the form definition row and belongs in that issue. ## Verification `cargo fmt --all --check`, `cargo clippy --all-targets -- -D warnings`, and the full `cargo test --tests` suite (714 passed, 0 failed, 71 test binaries) all green locally, plus `check-migration-prefixes.nu`, `check-migration-immutability.nu` and `check-pool-safety.nu`. Note for anyone running the suite locally: `just test-integration` currently fails with `42501 permission denied to create database` against a compose stack that has provisioned its roles, because `src/db/provision.rs` creates `mokosh_migrator` with `LOGIN BYPASSRLS` and no `CREATEDB` while `#[sqlx::test]` creates a database per test. CI is unaffected because `integration.yml` points `DATABASE_URL` at the `postgres` superuser. I ran with the superuser URL to match CI; the recipe divergence is worth its own fix and is not addressed here.

longjacksonle added 3 commits

2026-08-06 19:38:00 +02:00

feat(tickets): add procedure_kb_article_id for the MACD request flow a111402314

PMS-730 needs the person working a ticket to have the procedure attached: "this is a new-starter add, here is the article describing how we perform one". Nothing in the schema expresses that today.

It deliberately does not reuse migration 068's `tickets.source_kb_article_id` (PMS-452). That column records the article a ticket was opened FROM and feeds `list_top_ticket_driving_articles` (PMS-485), whose whole purpose is finding docs that are FAILING the user. Stamping it when we attach a procedure would fold every MACD request into that count and invert its meaning, reporting a working runbook as a documentation failure. The two links point in opposite directions, so they get separate columns.

ON DELETE SET NULL mirrors 068: retiring an article drops the linkage and leaves the ticket intact. The partial index serves PMS-732's "tracked time per request type, surfaced on the article" aggregation while keeping the majority of tickets, which carry no procedure, out of the index.

No RLS work is needed here because a policy attaches per table, not per column, and `tickets` already carries the fail-closed `tenant_isolation` policy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

feat(tickets): accept and return the procedure KB article 0f597e7f70

Threads the migration 099 column through the ticket surface: `CreateTicketRequest` accepts it (defaulting to None, so the agent, portal, RMM and email-intake create paths are unchanged), the create INSERT binds it, and the read model plus the wire DTO return it.

`TicketResponse` carries `procedure_kb_article_id` alongside a JOINed `procedure_kb_article_title`, following the PMS-344 `asset_id` / `asset_name` shape, so the ticket detail can render the procedure link without a second fetch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

test(kb): pin the procedure-article round trip and widget exclusion

E2E / Playwright against staging (pull_request) Failing after 7m6s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 10m57s

Details

Integration / integration tests (pull_request) Successful in 15m2s

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

f0645e8464

Covers the three properties the split between the two KB links depends on: the procedure article round-trips through create and get with its title resolved, a ticket whose only KB link is a procedure stays OUT of the PMS-485 "top ticket-driving articles" widget, and deleting the article NULLs the linkage instead of touching the ticket.

The widget assertion is the one that matters most: it is the regression guard for the reason migration 099 adds a column rather than reusing the PMS-452 one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011myibMMwyb6za3GVWJGkiX

longjacksonle scheduled this pull request to auto merge when all checks succeed

2026-08-06 19:42:15 +02:00