fix(registry): drop broken /mail/message/update_content override

[gonzalesedwin1123]

Why is this change needed?

Fixes #419. spp_registry/controllers/mail.py overrode /mail/message/update_content. The override came from the OpenG2P g2p_registry_base port (Odoo 17). It was already redundant there: stock 17 (mail/controllers/thread.py:121) granted author-or-administrator editing, and the override only narrowed "administrator" to base.group_system and answered with an AccessError instead of a 404.

On Odoo 19 the override never worked, and it is broken in three independent ways, not just the one the issue reports:

1. The Odoo 19 web client (mail/static/src/core/common/message_model.js:581, :677) sends {message_id, update_data: {...}}, while the override's signature still required the Odoo 17 positional body / attachment_ids. Every real Edit or Delete of a chatter message — on any model, the route is global — failed with TypeError in the JSON-RPC envelope before the body ever ran. Message deletion in the UI goes through this same route (removeParams).
2. Behind that, ir.attachment._check_attachments_access no longer exists (the issue's finding; replaced upstream by _has_attachments_ownership).
3. mail.message.message_format() no longer exists either (Store replaced it).

So this is a user-visible production bug (edit/delete a chatter message → error toast), not only a test-suite wart.

Stock Odoo 19 already implements what the override carried: ThreadController._can_edit_message (thread.py:263) is author OR res.users._is_admin(), and base.group_system implies base.group_erp_manager. Upstream additionally requires the editor to be allowed to post on the thread (_get_message_with_access(mode="create"), _mail_post_access = "write"), which the override skipped.

How was the change implemented?

• Deleted SPPThreadController and the imports only it used, plus the pre-existing unused logging/logger in the same import block (the only logger rather than _logger in the repo). The route is stock Odoo 19 again. SPPAttachmentController (/mail/attachment/delete) is untouched — see spp_registry: decide the fate of the /mail/attachment/delete override (policy differs from stock Odoo 19) and finish its two placeholder tests #523 for why it is not simply redundant.
• spp_registry 19.0.2.2.4 → 19.0.2.2.5 with a readme/HISTORY.md entry that also states the access-scope change below. README.rst / static/description/index.html are applied verbatim from CI's pinned oca-gen-addon-readme diff (docs commit).
• Dropped the three now-orphaned i18n entries for the deleted AccessError string (spp_registry.pot, es.po, fr.po), same shape as the module's 19.0.2.1.4 cleanup; no new translatable strings.

Behaviour change vs. the (never working on 19) override, stated in HISTORY: who may edit another user's message follows Odoo's own administrator test (superuser or base.group_erp_manager "Administration: Access Rights", which base.group_system implies) instead of group_system only — a strict superset; no OpenSPP role implies either group, so in practice the eligible set is unchanged. The editor must additionally be allowed to post on the record. Denials are a 404 NotFound rather than a 403-style AccessError (better anti-enumeration).

New unit tests

spp_registry/tests/test_mail_controllers.py::TestMailMessageUpdateContentController rewritten to the Odoo 19 payload, 8 tests, all running (previously 5 methods of which 3 were skipTest placeholders):

• Edit (update_data: {body, attachment_ids: []}): test_author_can_update_own_message, test_admin_can_update_any_message (un-skipped), test_bystander_cannot_update_anothers_message, test_unauthenticated_request_is_denied.
• Delete (update_data = Message.removeParams, exactly what the client sends): test_author_can_delete_own_message, test_admin_can_delete_any_message, test_bystander_cannot_delete_anothers_message, test_unauthenticated_delete_is_denied.
• Denials assert error.code == 404 and error.data.name == "werkzeug.exceptions.NotFound" and that the body is unchanged, so a controller crash (TypeError / AttributeError) can never pass as a denial again — the failure mode spp_registry: /mail/message/update_content controller is broken on Odoo 19 — calls removed ir.attachment._check_attachments_access #419 exposed.
• Both gates raise the same bare NotFound, so the bystander tests first assert the bystander can post on the thread (has_access("write")), pinning that the 404 came from the author gate.
• Fixture users are spp_registry.group_registry_officer (the module's own persona with write on res.partner, security/ir.model.access.csv:28) rather than a generic base group, so a regression in the module's ACL would surface here. Stock group_user is read-only on res.partner and cannot post on the thread.
• test_message_without_model_returns_not_found (an unimplemented skipTest TODO for a branch of the deleted override) removed. The two remaining skips are in the untouched TestMailAttachmentDeleteController and are tracked in spp_registry: decide the fate of the /mail/attachment/delete override (policy differs from stock Odoo 19) and finish its two placeholder tests #523.

Against the old controller the Edit tests fail RED with TypeError: ... missing 2 required positional arguments: 'body' and 'attachment_ids' (reproducing break 1 live); against this branch all 8 are green.

Unit tests executed by the author

./scripts/test_single_module.sh spp_registry --test-tags=/spp_registry:TestMailMessageUpdateContentController,/spp_registry:TestMailAttachmentDeleteController
→ 0 failed, 0 error(s) of 13 tests (8 update_content + 5 attachment_delete incl. its 2 pre-existing skips); 0 ERROR lines

Full spp_registry suite locally on the fix commit's code: 0 failed, 0 errors of 284 post-install tests (406 incl. at_install), 0 ERROR lines. Install-then--u spp_registry upgrade check on a fresh DB: clean, ir_module_module.latest_version = 19.0.2.2.5. CI test (spp_registry) re-runs the suite on every head.

How to test manually

Prerequisites: two internal users — U1 (Registry Officer, or any internal user with Contacts write) and Admin (Settings access) — and a third internal user U2. Any record with a chatter works; a registrant form is used below.

1. On 19.0: log in as U1, open a registrant, Log note "hello", hover the note → ⋯ → Edit, change the text, Save → red error toast (TypeError in the RPC). Hover → ⋯ → Delete → same error.
2. On this branch, as U1: Edit and Delete of U1's own note both succeed.
3. As Admin: open the same registrant, Edit and Delete a note authored by U1 → both succeed (administrator path).
4. As U2: U1's note shows no Edit/Delete under ⋯ (stock allowsEdition = admin OR self-authored). Calling the route directly returns a 404 envelope and the note is unchanged.
5. Repeat step 2 on a non-registry record (e.g. a Program) to confirm the route is global.

Related links

• Closes spp_registry: /mail/message/update_content controller is broken on Odoo 19 — calls removed ir.attachment._check_attachments_access #419
• Follow-up: spp_registry: decide the fate of the /mail/attachment/delete override (policy differs from stock Odoo 19) and finish its two placeholder tests #523 — the sibling /mail/attachment/delete override enforces a different policy than stock 19 (author-or-group_system vs write-on-attachment-or-token), so it needs a decision rather than a deletion; also tracks the two placeholder skips in TestMailAttachmentDeleteController.

[gonzalesedwin1123]

Full spp_registry suite on 8eb86f08 — clean

./scripts/test_single_module.sh spp_registry
→ 0 failed, 0 error(s) of 284 tests (406 incl. at_install), 0 ERROR lines in the log

The 25 WARNING lines are all pre-existing on 19.0 (duplicate field labels in spp_vocabulary, display_name search on spp.registry.id, phone_validation not installed, Odoo 19 deprecation notices). The two You do not have permission to delete this attachment. warnings are the attachment-delete denial tests doing their job.

Ready for review once CI is green. README.rst will be trued up from CI's oca-gen-addon-readme diff if it reports drift from the new HISTORY fragment.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.89%. Comparing base (1a3c591) to head (283b824).

Additional details and impacted files

@@            Coverage Diff             @@
##             19.0     #522      +/-   ##
==========================================
- Coverage   76.91%   76.89%   -0.02%     
==========================================
  Files         704      740      +36     
  Lines       45774    48004    +2230     
==========================================
+ Hits        35205    36913    +1708     
- Misses      10569    11091     +522     

Flag	Coverage Δ
spp_analytics	93.25% <ø> (ø)
spp_api_v2	79.99% <ø> (ø)
spp_api_v2_change_request	73.37% <ø> (ø)
spp_api_v2_cycles	71.03% <ø> (ø)
spp_api_v2_data	77.77% <ø> (ø)
spp_api_v2_entitlements	70.23% <ø> (ø)
spp_api_v2_gis	74.60% <ø> (ø)
spp_api_v2_products	65.86% <ø> (ø)
spp_api_v2_programs	92.22% <ø> (ø)
spp_api_v2_service_points	71.03% <ø> (ø)
spp_api_v2_simulation	71.19% <ø> (ø)
spp_api_v2_vocabulary	57.75% <ø> (?)
spp_approval	50.85% <ø> (ø)
spp_area	80.16% <ø> (?)
spp_area_hdx	81.60% <ø> (?)
spp_audit	72.13% <ø> (?)
spp_base_common	91.07% <ø> (ø)
spp_programs	67.58% <ø> (ø)
spp_registry	89.83% <ø> (+0.82%)	⬆️
spp_security	69.56% <ø> (ø)

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

Files with missing lines	Coverage Δ
spp_registry/controllers/mail.py	64.70% <ø> (+5.44%)	⬆️

... and 36 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]

Ready for review — CI green on 6526cb1c

• All checks pass (pre-commit, module tests, security scans; Trivy skipped as usual). README.rst / index.html were trued up from CI's own oca-gen-addon-readme diff, applied verbatim.
• Independent code-review pass found nothing blocking. Its one factual finding is fixed: stock Odoo 17 already granted author-or-administrator editing (thread.py:121 on the 17.0 branch), so the override was redundant even before the port; HISTORY and the commit body now say so. Also added a comment on the anonymous test explaining that the public user is refused at the thread-post-access gate, and dropped the three stale i18n entries for the deleted error string.
• Left alone (pre-existing, out of scope): the unused logging import / logger in spp_registry/controllers/mail.py, and the sibling /mail/attachment/delete override noted above as a possible follow-up.
• Test count note for the reviewer: the class goes from 5 methods to 4, but 3 of the 5 were skipTest placeholders; the removed one (test_message_without_model_returns_not_found) was an unimplemented TODO for a branch of the deleted override. All 4 remaining tests now run for real.

[gonzalesedwin1123]

Ready for review — CI green on 283b8244 (after the expert-review round)

• Review round applied (commit 15a1b10f): Delete-path tests (removeParams) ×4, fixture users are now spp_registry.group_registry_officer, bystander tests pin that the bystander may post on the thread (so the 404 is the author gate), docstrings and HISTORY corrected (leads with the signature break; states the administrator-scope change and the post-on-record requirement), pre-existing dead logger dropped. PR body and title brought in line with the tree. Follow-up spp_registry: decide the fate of the /mail/attachment/delete override (policy differs from stock Odoo 19) and finish its two placeholder tests #523 filed for the sibling /mail/attachment/delete override (different policy than stock, needs a decision) and the two placeholder skips.
• Generated files: README.rst / index.html applied verbatim from CI's oca-gen-addon-readme diff (commit 283b8244, 0 masked characters).
• CI on the true head: all 34 checks pass (Trivy skipped as usual). test (spp_registry): 0 failed, 0 error(s) of 288 tests; all 8 TestMailMessageUpdateContentController tests started, none skipped. pre-commit green, Semgrep/CodeQL/Gitleaks/dependency scan green. MERGEABLE.
• Four commits, diffable one by one: fix → CI README → review-round tests → CI README.