Repository navigation
[20.0][REVIEW] mail_recipient_tracking: migration changes from OCA #310 - #2
weinni2000 wants to merge 4 commits into
Conversation
`MailMail._send()` tells a successful delivery from a failed one by the exception raised by `send_email()`, not by its return value. `mail_tracking` catches every exception and, when a `mail.tracking.email` record exists, records the error on the tracking record and returns normally, so core believes the mail was delivered: the recipient is marked as reached and its notification is written `sent` for a mail that never left, core's `except AssertionError` never runs so a NO_VALID_RECIPIENT is never classified, and the mail keeps the placeholder failure reason set before the sending loop. A tracking record is created for every outgoing email, so with `mail_tracking` installed core's whole failure-handling path is disabled for the database. This is not a duplicate of OCA#215/OCA#212, which added `else: raise`, i.e. it only re-raises when there is NO tracking record -- the branch that is dead in practice. The live case is the other one. Re-raise unconditionally. The error is still recorded on the tracking record by the `smtp_error()` call just above, so nothing is lost; only core's contract is restored. Note for adopters: delivery failures that used to be silent now surface as `exception` states on `mail.mail`/`mail.notification`, so a backlog of previously hidden failures may become visible at once on upgrade. Fixes OCA#253
mail_id has an ON DELETE SET NULL foreign key to mail_mail, so every deleted mail.mail scans the whole mail_tracking_email table. Odoo 19.0 deletes up to 10,000 cancelled mails per autovacuum run. With 7M trackings that is about 1.5 s per mail without the index and 6.5 s for the whole batch with it. Use a partial index: mail_id is set to NULL every time its mail.mail is deleted, so most rows hold NULL, and the foreign key trigger only runs "WHERE mail_id = $1" with a non-NULL value. The index stays much smaller, as core does for mailing.trace.mail_mail_id.
mail.message.mail_tracking_ids sets bypass_search_access, so the ORM searches mail.tracking.email with bypass_access=True. _search ignored it and filtered every matching tracking in Python. With 7M trackings, get_failed_count went from 5-10 s to under 1 s.
Odoo 20.0 core provides a `mail_tracking` module (technical tracking of field values), so this addon is renamed from `mail_tracking` to `mail_recipient_tracking`.
|
Reviewed [P2] Preserve the mail_id index added immediately before the migration.
On a fresh 20.0 installation the tracking table therefore has no index for this column. Deleting Fix: retain Other review observations:
Validation:
Outcome: approval withheld until the index regression is fixed. File mapping: Python -> Imports/Naming plus ORM/Fields/Methods; controllers -> Controllers/Security; ACL additions/deletions -> Access rights/Security; XML/data/demo/views -> Views/actions/data and inheritance anchors; JS/Owl/SCSS additions, changes and removals -> web JavaScript/SCSS/Assets; test changes -> Tests; manifest -> Manifest; PO/POT -> translation/namespace validation; README/readme/generated description and removed screenshot -> documentation/Assets. Guidelines read: Module structure and file naming; Manifest; Imports; Naming and model layout; Translate only static literals; Recordsets/domains/context; Computes/onchange/constraints; Methods and extension points; Transactions and exceptions; Fields; Controllers; Access rights; Batch ORM calls; Performance conventions; Views/actions/data records; Anchor view inheritance on names; Tests; Comments; Odoo security; web Organize files by feature/Avoid getters/Patching guidance; SCSS/CSS; Assets. |
Comparison for OCA#310 after the final pre-commit/auto-fix preparation commit through the complete migration.
Base:
561e76939d3ef1d4878794471ca20cabce4fd6b0.Head:
59363262f7d9597cb1719341a6f8a94ee7e5ee5a.The replayed history already uses
mail_recipient_tracking(renamed to avoid the new coremail_tracking). This comparison includes three preceding 19.0 fixes for SMTP exception handling, mail_id indexing and bypass_access, plus the final 20.0 migration. Review results follow here.