fix(api): consistent error statuses on change-request endpoints

[kneckinator]

Follow-up to #460, implementing the "noted, not changed" items from its review. Stacked on #460 — merge that first; this diff shows its commits until it lands.

What changes

All four items are about the same thing: the change-request endpoints reporting one condition with different statuses depending on which handler it happened to flow through.

1. ValidationError → 422 on state transitions (was 409)

create() and update() map ValidationError to 422 Unprocessable Entity; the transition endpoints mapped it to 409 Conflict. $reject and $request-revision raise ValidationError for a missing reason/notes — invalid input, not a conflict. All endpoints now report 422 uniformly via _status_for_odoo_error().

This is the API-contract decision #460 explicitly deferred ("pinned by a test with a note"); that test now asserts 422.

2. MissingError → 404 (was 409)

A record that vanishes mid-transition is "not found", not "conflict". Matches the platform's global handler (fastapi/error_handlers.py maps MissingError → 404).

3. Generic detail on 403 (anti-enumeration)

An AccessError message names models, records and record rules. The 403 branch now returns a fixed "Not authorized to perform this action" instead of str(e) — the same posture as the platform handler, which returns the literal string "AccessError". Non-authorization errors still pass their text through: it is written for the actor ("Only pending change requests can be rejected").

4. create() no longer reports an authorization failure as 500

create()'s bare except Exception swallowed AccessError into a 500. An explicit except (AccessError, AccessDenied) branch now reports 403 with the generic detail — the same shadowing shape #460 fixed on the transition endpoints, one layer up.

Not attempted

Full RFC 9457 Problem Details response bodies. That is an API-wide format migration touching every router in spp_api_v2*, not a change-request fix; if wanted, it deserves its own scoped PR.

update() is also untouched: it catches only ValidationError, so an AccessError there already propagates to the platform's global handler, which reports 403 with a generic detail.

Tests

TDD — the new/changed assertions were written and observed failing first. Unit tests cover each mapping (ValidationError→422, MissingError→404, generic 403 detail, passthrough detail otherwise); route-level tests exercise $reject returning 422, the 403 body not leaking the raw message, and create() returning 403 instead of 500. Full module suite green.

Merge order

After #460. 19.0.2.0.2 → 19.0.2.0.3.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.37%. Comparing base (3be1c39) to head (3e1892d).

Additional details and impacted files

@@            Coverage Diff             @@
##             19.0     #471      +/-   ##
==========================================
+ Coverage   76.13%   76.37%   +0.24%     
==========================================
  Files         661      654       -7     
  Lines       44026    44027       +1     
==========================================
+ Hits        33519    33627     +108     
+ Misses      10507    10400     -107     

Flag	Coverage Δ
spp_api_v2_change_request	73.37% <100.00%> (+1.45%)	⬆️
spp_api_v2_products	?
spp_base_common	91.07% <ø> (ø)
spp_consent	?
spp_irrigation	?
spp_programs	66.97% <ø> (ø)
spp_registry	87.79% <ø> (ø)
spp_security	69.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...pp_api_v2_change_request/routers/change_request.py	43.36% <100.00%> (+5.73%)	⬆️

... and 74 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gonzalesedwin1123]

Approving. All four deferred items from #460's review are implemented, each verified against the diff:

• ValidationError → 422 on transitions via _status_for_odoo_error, with the previously behaviour-documenting tests flipped to assert the new contract — and the change is a real contract change, correctly called out in HISTORY.
• MissingError → 404: correct, and the ordering in the helper is safe (MissingError subclasses UserError, not ValidationError, so the except UserError handlers catch it and the isinstance chain resolves it before the 409 fallthrough).
• Generic 403 detail: _detail_for_odoo_error applied on all six transition endpoints and create; the pass-through for actor-written UserError/ValidationError text is the right split, and the scope-check 403s that keep descriptive text don't leak internals.
• create() 403: the except (AccessError, AccessDenied) branch correctly precedes the bare except Exception, and the route test pins 500 → 403.

Also checked: the AST guard from #460 stays coherent with the new create handler (it catches no UserError, so it's rightly exempt), update()'s untouched state is justified (its AccessError propagates to the platform's global 403), version 19.0.2.0.3 stacks correctly on #460's 2.0.2, and CI is fully green.

One non-blocking observation: a MissingError raised mid-transition passes Odoo's raw message through as the 404 detail, which names the model and record id (e.g. spp.change.request(42,)). The endpoint identity already reveals the model and the client sent a reference, not an id, so there's nothing meaningful to enumerate — but if you want the detail surface fully uniform, a fixed "Change request not found" in _detail_for_odoo_error's MissingError case would match the endpoint's own not-found path. Fine either way.

Merge order stands: #460 first, then this. Merge authorization is Edwin's.