OCA / social

Addons concerning odoo's social ERP features and messaging in general
https://odoo-community.org/project/99
GNU Affero General Public License v3.0
157 stars 600 forks source link

[IMP] mail_post_defer: defer more mails #1372

Closed yajo closed 3 months ago

yajo commented 3 months ago

In the previous version of this module, only emails sent through the chatter were explicitly deferred. However, there are more mails that are auto-generated by Odoo and could benefit a lot from the deferring system. One of such cases is the automatic assignation mail.

To cover that and more cases, now the auto-deferring is done at a lower level. If we get some hint that the mail sending is forced, or that it's already deferred, we skip the machinery. If we get nothing about all that, then we just add our defaults to avoid force-sending and defer by default.

The result is that more Odoo internal systems will use the mail queue, and thus there'll be less blocking.

As an exception, when running _send_notifications(), we force-disable the deferring system. That is executed by a cron, or by the user when force-sending a notification. So, only in that case the queue should be skipped by default.

@moduon MT-6204

OCA-git-bot commented 3 months ago

Hi @Yajo, some modules you are maintaining are being modified, check this out!

etobella commented 3 months ago

Is this PR the fix for this?

https://github.com/OCA/social/pull/1305#issuecomment-2119174211

etobella commented 3 months ago

This module is deferring notifications too. I think it was not intended, isn't it?

yajo commented 3 months ago

Please notice that one assertion done in the test could provide a false positive due to https://github.com/odoo/odoo/pull/166676, but it's not this PR failure. Luckily it would still fail due to other assertions, so we can continue here and hope for the best regarding that PR.

yajo commented 3 months ago

This module exists to defer notifications @etobella. It seems to me like it's doing what it should, and I don't think this will fix your problem.

It's just using one native odoo mechanism more than standard Odoo uses it. But if your modules support that system, both should be compatible.

If it's just tests that break, you can mark as rebel or skip some tests conditionally.

etobella commented 3 months ago

Then this PR should not affect other modules tests.

In that case, you should make a change in behavior in order to not affect other modules tests. (For example, check that it is not testing or a context variable is passed)

The module breaking the defualt behavior is this one, so if the patch is not enabled, it should be considered as rebel :wink:

etobella commented 3 months ago

Thanks @yajo :smile:

yajo commented 3 months ago

Yes, I'm marking it rebel here.

pedrobaeza commented 3 months ago

We need to solve the CI problem with mail_gateway, so merging:

/ocabot merge minor

OCA-git-bot commented 3 months ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 16.0-ocabot-merge-pr-1372-by-pedrobaeza-bump-minor, awaiting test results.

OCA-git-bot commented 3 months ago

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