feat(approvals): polymorphic approval surface across tickets + time-entries (PMS-470) #351
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/mokosh-server!351
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/PMS-470-polymorphic-approvals"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
PMS-451 phase 1 shipped per-ticket approvals: a tight, single-entity surface that proved out. With the second consumer (time entries above a billable threshold) now real, phase 2 widens the table + service + routes to drive sign-off on every entity without duplicating the request / decide / cancel machinery. Phase-1 callers continue to work byte-identically: the existing
/tickets/{id}/approvalspaths andApprovalsService::list_for_ticket / createsignatures are preserved as thin wrappers over the generic polymorphic core.What landed:
Migration
077_approvals_polymorphic.sqlwidensticket_approvalswithtarget VARCHAR(20) NOT NULL DEFAULT 'ticket' CHECK (...)andentity_id UUID NOT NULL. Existing rows backfill totarget='ticket', entity_id=ticket_id. The legacyticket_idis relaxed to NULL so non-ticket targets can leave it empty; a follow-up release drops the column entirely once every consumer has migrated to(target, entity_id). New indexes on(tenant_id, target, entity_id)plus partial pending indexes scoped by target keep every per-entity query a one-index scan.DTOs in
approvals::models:ApprovalResponsegainstarget+entity_idand demotesticket_idtoOption<Uuid>(Some on ticket rows for backwards compat, None elsewhere). NewApprovalTargetenum (Ticket / ChangeRequest / Quote / TimeEntry) is the route-layer discriminator withas_str()/from_str()for the column round-trip.ApprovalsServicekeeps every phase-1 signature compile-compatible and adds polymorphic siblings:list_for_entity(target, entity_id),create_for_entity(target, entity_id, ...). The legacylist_for_ticket/createwrap them withApprovalTarget::Ticket.pending_for_useralready spans the whole table, so it picks up the wider surface for free; rows arrive withtargetpopulated so the SPA can render a per-target entity link without re-deriving from the URL.approvals::routesmounts the per-entity prefixes:/tickets/{id}/approvals(phase-1, untouched),/time-entries/{id}/approvals(live; parent existence checked against thetime_entriestable),/change-requests/{id}/approvals+/quotes/{id}/approvals(registered but return 400 with a "parent table not yet defined" hint until the schemas land; tracked in the PMS-484 follow-up). The sharedassert_parent_exists(table, tenant_id, entity_id)helper enforces tenant-scoped 404s before the approval gets written so a guessed UUID outside the caller's scope never produces a row.Integration tests:
tests/ticket_approvals.rs(phase-1, untouched) still passes verbatim. The DTO change is additive on the wire so existing consumers see the same shape with two extra fields.tests/approvals_polymorphic.rs(new): atime_entryapproval round-trips through/time-entries/{id}/approvalswithtarget='time_entry'and surfaces in the caller's pending queue; aticketapproval still carries the newtarget='ticket'+entity_idfields alongside the legacyticket_id; the placeholder change-requests / quotes routes 400 cleanly.Follow-up (filed as PMS-484): land the
change_requestsandquotesparent tables, wireassert_parent_existson those routes, and remove the placeholder 400 handlers.#PMS-470
PMS-451 phase 1 shipped per-ticket approvals: a tight, single-entity surface that proved out. With the second consumer (time entries above a billable threshold) now real, phase 2 widens the table + service + routes to drive sign-off on every entity without duplicating the request / decide / cancel machinery. Phase-1 callers continue to work byte-identically: the existing `/tickets/{id}/approvals` paths and `ApprovalsService::list_for_ticket / create` signatures are preserved as thin wrappers over the generic polymorphic core. What landed: * Migration `077_approvals_polymorphic.sql` widens `ticket_approvals` with `target VARCHAR(20) NOT NULL DEFAULT 'ticket' CHECK (...)` and `entity_id UUID NOT NULL`. Existing rows backfill to `target='ticket', entity_id=ticket_id`. The legacy `ticket_id` is relaxed to NULL so non-ticket targets can leave it empty; a follow-up release drops the column entirely once every consumer has migrated to `(target, entity_id)`. New indexes on `(tenant_id, target, entity_id)` plus partial pending indexes scoped by target keep every per-entity query a one-index scan. * DTOs in `approvals::models`: `ApprovalResponse` gains `target` + `entity_id` and demotes `ticket_id` to `Option<Uuid>` (Some on ticket rows for backwards compat, None elsewhere). New `ApprovalTarget` enum (`Ticket / ChangeRequest / Quote / TimeEntry`) is the route-layer discriminator with `as_str()` / `from_str()` for the column round-trip. * `ApprovalsService` keeps every phase-1 signature compile-compatible and adds polymorphic siblings: `list_for_entity(target, entity_id)`, `create_for_entity(target, entity_id, ...)`. The legacy `list_for_ticket` / `create` wrap them with `ApprovalTarget::Ticket`. `pending_for_user` already spans the whole table, so it picks up the wider surface for free; rows arrive with `target` populated so the SPA can render a per-target entity link without re-deriving from the URL. * `approvals::routes` mounts the per-entity prefixes: `/tickets/{id}/approvals` (phase-1, untouched), `/time-entries/{id}/approvals` (live; parent existence checked against the `time_entries` table), `/change-requests/{id}/approvals` + `/quotes/{id}/approvals` (registered but return 400 with a "parent table not yet defined" hint until the schemas land; tracked in the PMS-484 follow-up). The shared `assert_parent_exists(table, tenant_id, entity_id)` helper enforces tenant-scoped 404s before the approval gets written so a guessed UUID outside the caller's scope never produces a row. Integration tests: * `tests/ticket_approvals.rs` (phase-1, untouched) still passes verbatim. The DTO change is additive on the wire so existing consumers see the same shape with two extra fields. * `tests/approvals_polymorphic.rs` (new): a `time_entry` approval round-trips through `/time-entries/{id}/approvals` with `target='time_entry'` and surfaces in the caller's pending queue; a `ticket` approval still carries the new `target='ticket'` + `entity_id` fields alongside the legacy `ticket_id`; the placeholder change-requests / quotes routes 400 cleanly. Follow-up (filed as PMS-484): land the `change_requests` and `quotes` parent tables, wire `assert_parent_exists` on those routes, and remove the placeholder 400 handlers. #PMS-470