[MIG] mail_tracking_mailgun: Migration to 15.0 #909
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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/
This change let odoo process a state sent from mailgun legacy webhooks that seems to apply when the message is not sent because the related email has been mark us as spam or have bounced before. For solve this add two new states to _mailgun_event_type_mapping_mailgun_event_type_mapping method: * failed: Mailgun could not deliver the email to the recipient email server * rejected: Mailgun rejected the request to send/forward the email source in https://documentation.mailgun.com/en/latest/quickstart-events.html#events.
Currently translated at 12.5% (3 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/sl/
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/
nicomacr
force-pushed
the
15.0-mig-mail_tracking_mailgun
branch
3 times, most recently
from
22d2e72
to
a0e01fd
Compare
alan196
suggested changes
May 20, 2022
| @@ -405,7 +405,7 @@ def test_email_validity(self, mock_request): | |||
| } | |||
| with self.assertRaises(UserError): | |||
| self.partner.check_email_validity() | |||
| self.assertTrue(self.partner.email_bounced) | |||
| self.assertTrue(self.partner.email_bounced) | |||
There was a problem hiding this comment.
With this change, you don't execute this line, if the previous line raise an UserError the next line is not executed, I think this test is not valid in this way
There was a problem hiding this comment.
yes i just having testing and i forget to remove it. The reason is because i have problem with this test.
If you could give me some hint to solve this bug with this line of the test welcome.
Thanks
nicomacr
force-pushed
the
15.0-mig-mail_tracking_mailgun
branch
from
a0e01fd
to
2419ce5
Compare
chienandalu
suggested changes
Dec 22, 2022
There was a problem hiding this comment.
Thanks @nicomacr
I've been testing it locally and everything seems to work as expected.
Regarding the tests, you need to change this in order to pass them:
chienandalu
reviewed
Dec 22, 2022
nicomacr
force-pushed
the
15.0-mig-mail_tracking_mailgun
branch
from
2419ce5
to
d2792da
Compare
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at 0cb5254. Thanks a lot for contributing to OCA. ❤️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.