Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][MIG] Migration of module mail_tracking_mailgun #1073

Closed
wants to merge 52 commits into from

Conversation

alan196
Copy link

@alan196 alan196 commented Feb 21, 2023

Proposed changes

Migration of module mail_tracking_mailgun

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Migration Update

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation

Notes

This PR depends on #1029

I have tested functionally with PR #1029 and cherry-pick #1056

@alan196 alan196 mentioned this pull request Feb 21, 2023
38 tasks
# See https://documentation.mailgun.com/en/latest/user_manual.html#routes
try:
self._mail_tracking_mailgun_webhook_verify(
**request.jsonrequest["signature"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, The request object is not the same in version 16 .
**request.jsonrequest["signature"]
should be
**request.dispatcher.jsonrequest["signature"]

the same for event-data

Thanks

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alan196 hi could you see this?

antespi and others added 28 commits July 20, 2023 03:13
…ent events (OCA#82)

[IMP] mail_tracking: Speed installation time, discard concurrent events and other fixes
Both Carlos and me have work a lot in this module, maybe not coding but much testing and definition in usability
When this method is called without an event (a.k.a. `event=None`), this method produces the following error:

```
Traceback (most recent call last):
  File "/opt/odoo/auto/addons/mail/models/mail_mail.py", line 278, in send
    res = IrMailServer.send_email(msg, mail_server_id=mail.mail_server_id.id)
  File "/opt/odoo/auto/addons/mail_tracking/models/ir_mail_server.py", line 88, in send_email
    tracking_email.smtp_error(self, smtp_server_used, e)
  File "/opt/odoo/auto/addons/mail_tracking/models/mail_tracking_email.py", line 213, in smtp_error
    self.sudo()._partners_email_bounced_set('error')
  File "/opt/odoo/auto/addons/mail_tracking/models/mail_tracking_email.py", line 203, in
_partners_email_bounced_set
    ]).email_bounced_set(self, reason, event=event)
  File "/opt/odoo/auto/addons/mail_tracking_mailgun/models/res_partner.py", line 22, in
email_bounced_set
    self._email_bounced_set(reason, event)
  File "/opt/odoo/auto/addons/mail_tracking_mailgun/models/res_partner.py", line 33, in
_email_bounced_set
    event['Message-Id'] or '') TypeError: 'NoneType' object has no attribute '__getitem__'
```

So, we now assume we do not always have an event.
- Configurable partner email auto check.
Currently translated at 16.7% (4 of 24 strings)

Translation: social-11.0/social-11.0-mail_tracking_mailgun
Translate-URL: https://translation.odoo-community.org/projects/social-11-0/social-11-0-mail_tracking_mailgun/fr/
- In case the sending domain is different from the one configured in the
mail.domain.catchall setting.
Currently translated at 95.8% (23 of 24 strings)

Translation: social-12.0/social-12.0-mail_tracking_mailgun
Translate-URL: https://translation.odoo-community.org/projects/social-12-0/social-12-0-mail_tracking_mailgun/pt/
Jairo Llopis and others added 19 commits July 20, 2023 03:13
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
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: social-14.0/social-14.0-mail_tracking_mailgun
Translate-URL: https://translation.odoo-community.org/projects/social-14-0/social-14-0-mail_tracking_mailgun/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: social-15.0/social-15.0-mail_tracking_mailgun
Translate-URL: https://translation.odoo-community.org/projects/social-15-0/social-15-0-mail_tracking_mailgun/
Mass mailing are tracked from mail.trace as the don't store a message in
the db. In order to gather the message_id and be able to do manual
checks to mailgun, that's the table where we should get the message id.

TT40816
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: social-15.0/social-15.0-mail_tracking_mailgun
Translate-URL: https://translation.odoo-community.org/projects/social-15-0/social-15-0-mail_tracking_mailgun/
@alan196 alan196 force-pushed the 16.0-mig-mail_tracking_mailgun branch from b8b05f9 to afbb3f0 Compare July 20, 2023 09:17
@chienandalu
Copy link
Member

I don't know if this PR is in progress. In any case, take this recent patch into consideration: #1190

@rafaelbn rafaelbn added this to the 16.0 milestone Oct 19, 2023
@rafaelbn
Copy link
Member

/ocabot migration mail_tracking_mailgun

@rafaelbn
Copy link
Member

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

@rafaelbn The rebase process failed, because command git push --force Jarsa-dev tmp-pr-1073:16.0-mig-mail_tracking_mailgun failed with output:

remote: Permission to Jarsa-dev/social.git denied to OCA-git-bot.
fatal: unable to access 'https://github.com/Jarsa-dev/social/': The requested URL returned error: 403

@tobru
Copy link

tobru commented Oct 31, 2023

I gave this PR a try in an Odoo 16 test environment and so far, it looks good, works as expected.

@alan196 alan196 closed this Oct 31, 2023
@alan196 alan196 deleted the 16.0-mig-mail_tracking_mailgun branch May 7, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.