[IMP] mail_tracking{,_mailgun}: process sent events and fix wrong failure event mappings... and refactor #804

yajo · 2021-11-23T12:16:53Z

FW-port #787.

Up until now, the `sent` event type was never processed, probably because it "made no sense", as an unsent email would never trigger an event. However, it makes sense to process it because you may have a local relay that transmits mails over to the mail provider. In those circumstances, you should have 2 "sent" events (one from the relay and another one from the provider). Also marked some useless parts of code for removal. @Tecnativa TT32365

pedrobaeza

Tested on own instance

chienandalu

👍

Before this patch, the module was designed after the [deprecated Mailgun webhooks][3]. However Mailgun had the [events API][2] which was quite different. Modern Mailgun has deprecated those webhooks and instead uses new ones that include the same payload as the events API, so you can reuse code. However, this was incorrectly reusing the code inversely: trying to process the events API through the same code prepared for the deprecated webhooks. Besides, both `failed` and `rejected` mailgun events were mapped to `error` state, but that was also wrong because [`mail_tracking` doesn't have an `error` state][1]. So the logic of the whole module is changed, adapting it to process the events API payload, both through controllers (prepared for the new webhooks) and manual updates that directly call the events API. Also, `rejected` is now translated into `reject`, and `failed` is translated into `hard_bounce` or `soft_bounce` depending on the severity, as specified by [mailgun docs][2]. Also, `bounced` and `dropped` mailgun states are removed because they don't exist, and instead `failed` and `rejected` properly get their metadata. Of course, to know the severity, now the method to obtain that info must change, it' can't be a simple dict anymore. Added more parameters because for example modern Mailgun uses different keys for signing payload than for accessing the API. As there are so many parameters, configuration is now possible through `res.config.settings`. Go there to autoregister webhooks too. Since the new webhooks are completely incompatible with the old supposedly-abstract webhooks controllers (that were never really that abstract), support for old webhooks is removed, and it will be removed in the future from `mail_tracking` directly. There is a migration script that attempts to unregister old webhooks and register new ones automatically. [1]: https://github.com/OCA/social/blob/f73de421e28a43d018176f61725a3a59665f715d/mail_tracking/models/mail_tracking_event.py#L31-L42 [2]: https://documentation.mailgun.com/en/latest/api-events.html#event-types [3]: https://documentation.mailgun.com/en/latest/api-webhooks-deprecated.html

pedrobaeza · 2021-11-25T09:46:39Z

/ocabot merge nobump

OCA-git-bot · 2021-11-25T09:46:50Z

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 13.0-ocabot-merge-pr-804-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot · 2021-11-25T10:30:34Z

Congratulations, your PR was merged at ff958d8. Thanks a lot for contributing to OCA. ❤️

HviorForgeFlow · 2021-11-25T11:25:13Z

Post-migration script seems wrong @yajo @pedrobaeza

2021-11-25 11:17:35,608 1 ERROR test OpenUpgrade: mail_tracking_mailgun: error in migration script mail_tracking_mailgun/migrations/13.0.2.0.0/post-migration.py: create() missing 1 required positional argument: 'vals_list' 
2021-11-25 11:17:35,608 1 ERROR test OpenUpgrade: create() missing 1 required positional argument: 'vals_list' 
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/openupgradelib/openupgrade.py", line 1876, in wrapped_function
    if use_env2 else cr, version)
  File "/opt/odoo/auto/addons/mail_tracking_mailgun/migrations/13.0.2.0.0/post-migration.py", line 18, in migrate
    settings = env["res.config.settings"].create()
TypeError: create() missing 1 required positional argument: 'vals_list'
2021-11-25 11:17:35,617 1 WARNING test odoo.modules.loading: Transient module states were reset 
2021-11-25 11:17:35,617 1 ERROR test odoo.modules.registry: Failed to load registry 
Traceback (most recent call last):
  File "/opt/odoo/custom/src/odoo/odoo/modules/registry.py", line 86, in new
    odoo.modules.load_modules(registry._db, force_demo, status, update_module)
  File "/opt/odoo/custom/src/odoo/odoo/modules/loading.py", line 424, in load_modules
    force, status, report, loaded_modules, update_module, models_to_check)
  File "/opt/odoo/custom/src/odoo/odoo/modules/loading.py", line 315, in load_marked_modules
    perform_checks=perform_checks, models_to_check=models_to_check
  File "/opt/odoo/custom/src/odoo/odoo/modules/loading.py", line 230, in load_module_graph
    migrations.migrate_module(package, 'post')
  File "/opt/odoo/custom/src/odoo/odoo/modules/migration.py", line 180, in migrate_module
    migrate(self.cr, installed_version)
  File "/usr/local/lib/python3.6/site-packages/openupgradelib/openupgrade.py", line 1876, in wrapped_function
    if use_env2 else cr, version)
  File "/opt/odoo/auto/addons/mail_tracking_mailgun/migrations/13.0.2.0.0/post-migration.py", line 18, in migrate
    settings = env["res.config.settings"].create()
TypeError: create() missing 1 required positional argument: 'vals_list'
2021-11-25 11:17:35,617 1 CRITICAL test odoo.service.server: Failed to initialize database `prod`. 
Traceback (most recent call last):
  File "/opt/odoo/custom/src/odoo/odoo/service/server.py", line 1192, in preload_registries
    registry = Registry.new(dbname, update_module=update_module)
  File "/opt/odoo/custom/src/odoo/odoo/modules/registry.py", line 86, in new
    odoo.modules.load_modules(registry._db, force_demo, status, update_module)
  File "/opt/odoo/custom/src/odoo/odoo/modules/loading.py", line 424, in load_modules
    force, status, report, loaded_modules, update_module, models_to_check)
  File "/opt/odoo/custom/src/odoo/odoo/modules/loading.py", line 315, in load_marked_modules
    perform_checks=perform_checks, models_to_check=models_to_check
  File "/opt/odoo/custom/src/odoo/odoo/modules/loading.py", line 230, in load_module_graph
    migrations.migrate_module(package, 'post')
  File "/opt/odoo/custom/src/odoo/odoo/modules/migration.py", line 180, in migrate_module
    migrate(self.cr, installed_version)
  File "/usr/local/lib/python3.6/site-packages/openupgradelib/openupgrade.py", line 1876, in wrapped_function
    if use_env2 else cr, version)
  File "/opt/odoo/auto/addons/mail_tracking_mailgun/migrations/13.0.2.0.0/post-migration.py", line 18, in migrate
    settings = env["res.config.settings"].create()
TypeError: create() missing 1 required positional argument: 'vals_list'
2021-11-25 11:17:35,618 1 INFO test odoo.service.server: Stopping gracefully

Syncing from upstream OCA/social (15.0)

pedrobaeza added this to the 13.0 milestone Nov 25, 2021

pedrobaeza force-pushed the 13.0-mail_tracking_mailgun-refactor branch from 4cd24f8 to 28a30f1 Compare November 25, 2021 09:24

pedrobaeza approved these changes Nov 25, 2021

View reviewed changes

chienandalu approved these changes Nov 25, 2021

View reviewed changes

OCA-git-bot added the approved label Nov 25, 2021

pedrobaeza force-pushed the 13.0-mail_tracking_mailgun-refactor branch from 28a30f1 to 8681bc0 Compare November 25, 2021 09:46

OCA-git-bot added the bot is merging ⏳ label Nov 25, 2021

OCA-git-bot merged commit c41f974 into OCA:13.0 Nov 25, 2021

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Nov 25, 2021

HviorForgeFlow mentioned this pull request Nov 25, 2021

[13.0][FIX] mail_tracking_mailgun: Fix post-migration res.config.settings create #809

Closed

MiquelRForgeFlow mentioned this pull request Feb 25, 2022

[14.0][IMP] mail_tracking_mailgun: refactor to support modern webhooks (forward port) #850

Merged

This was referenced Jan 26, 2023

[14.0][FIX] mail_tracking: forward ports #1047

Merged

[15.0][FIX] mail_tracking: forward ports #1057

Merged

SiesslPhillip pushed a commit to grueneerde/OCA-social that referenced this pull request Nov 20, 2024

Merge pull request OCA#804 from OCA/15.0

c2a9379

Syncing from upstream OCA/social (15.0)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IMP] mail_tracking{,_mailgun}: process sent events and fix wrong failure event mappings... and refactor #804

[IMP] mail_tracking{,_mailgun}: process sent events and fix wrong failure event mappings... and refactor #804

yajo commented Nov 23, 2021

pedrobaeza left a comment

chienandalu left a comment

pedrobaeza commented Nov 25, 2021

OCA-git-bot commented Nov 25, 2021

OCA-git-bot commented Nov 25, 2021

HviorForgeFlow commented Nov 25, 2021

[IMP] mail_tracking{,_mailgun}: process sent events and fix wrong failure event mappings... and refactor #804

[IMP] mail_tracking{,_mailgun}: process sent events and fix wrong failure event mappings... and refactor #804

Conversation

yajo commented Nov 23, 2021

pedrobaeza left a comment

Choose a reason for hiding this comment

chienandalu left a comment

Choose a reason for hiding this comment

pedrobaeza commented Nov 25, 2021

OCA-git-bot commented Nov 25, 2021

OCA-git-bot commented Nov 25, 2021

HviorForgeFlow commented Nov 25, 2021