Skip to content

[18.0][MIG] scheduler_error_mailer - #3084

Merged
OCA-git-bot merged 44 commits into
OCA:18.0from
astirpe:18_mig_scheduler_error_mailer
Jun 27, 2025
Merged

[18.0][MIG] scheduler_error_mailer#3084
OCA-git-bot merged 44 commits into
OCA:18.0from
astirpe:18_mig_scheduler_error_mailer

Conversation

@astirpe

@astirpe astirpe commented Oct 17, 2024

Copy link
Copy Markdown
Member

No description provided.

Comment thread scheduler_error_mailer/models/ir_cron.py Outdated
@astirpe
astirpe force-pushed the 18_mig_scheduler_error_mailer branch 4 times, most recently from d3b6403 to 077b944 Compare November 26, 2024 11:54
@astirpe
astirpe marked this pull request as ready for review November 26, 2024 11:59
@StefanRijnhart

Copy link
Copy Markdown
Member

/ocabot migration scheduler_error_mailer

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Feb 7, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Feb 7, 2025
42 tasks

@StefanRijnhart StefanRijnhart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests are fixed in #3190. Please rebase once that one is merged.

@astirpe

astirpe commented Feb 7, 2025

Copy link
Copy Markdown
Member Author

@StefanRijnhart I will, thank you!

@astirpe
astirpe force-pushed the 18_mig_scheduler_error_mailer branch from 077b944 to fb57bb5 Compare February 7, 2025 14:30
@astirpe

astirpe commented Feb 7, 2025

Copy link
Copy Markdown
Member Author

rebased

@CRogos CRogos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0

Squash administrative commits (if any) with the previous commit for reducing commit noise. Check https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate for details.
Here are some examples:
image

Maybe we could also improve the test and test "def _callback" instead of "def _handle_callback_exception"?

Comment thread scheduler_error_mailer/models/ir_cron.py
Comment thread scheduler_error_mailer/tests/test_scheduler_error_mailer.py Outdated
@CRogos

CRogos commented Apr 10, 2025

Copy link
Copy Markdown
Contributor

@astirpe are you still working on this?

@astirpe

astirpe commented Apr 10, 2025

Copy link
Copy Markdown
Member Author

@CRogos Thank you for your review! I'm very busy with projects, I could continue with this when I can find some time. Sorry

@astirpe
astirpe force-pushed the 18_mig_scheduler_error_mailer branch 2 times, most recently from 4cc7a69 to 33debe6 Compare April 12, 2025 18:30
@StefanRijnhart

Copy link
Copy Markdown
Member

@CRogos Is it alright for you now?

@StefanRijnhart

Copy link
Copy Markdown
Member

Oh, we're getting errors because the mail template body is no longer valid due to the ir.cron datamodel changes. @astirpe would you consider taking astirpe#1?

@astirpe

astirpe commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

Thank you @StefanRijnhart !

@CRogos CRogos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you squash your latest changes with the Migration commit and also cleanup the commits above according to the migration guide?

self._handle_callback_exception(cron_name, server_action_id, e)

# Re-raise the original job exception
raise e

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can't we write raise without the e to reraise?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure this is the best option?

Couldn't we just call load_data with the ir_cron_email_tpl.xml file?
https://github.com/OCA/openupgradelib/blob/37cc7eeb1383fbe7c3d5cd2666af3b73338a90cd/openupgradelib/openupgrade.py#L292

Something like this:
OCA/account-financial-reporting@8ba049e8

@StefanRijnhart StefanRijnhart Apr 16, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That would be equivalent to calling reset_template on the template, but it would reset other fields as well such as subject and recipients which are customized in our setups. Also, this bit takes care to reset the body in each language (as per code taken from reset_template).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@StefanRijnhart

Copy link
Copy Markdown
Member

Apologies @astirpe, it turns out that in migrated databases the template_fs of the template might not be set, so here is astirpe#2. And please rebase and squash all migration commits as @CRogos suggests.

@astirpe

astirpe commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

I will do the rebase and squash, thanks!

@api.model
def _test_scheduler_failure(self):
"""This function is used to test and debug this module."""
raise UserError(_("Task failure with UID = %d.") % self._uid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_ should also be replaced

raise UserError(self.env._("Task failure with UID = %(uid)d.", uid=self._uid))

@astirpe
astirpe force-pushed the 18_mig_scheduler_error_mailer branch from 2eab569 to a743176 Compare April 16, 2025 18:12
def _test_scheduler_failure(self):
"""This function is used to test and debug this module."""
raise UserError(_("Task failure with UID = %d.") % self._uid)
raise UserError(_("Task failure with UID = %(uid)d."), uid=self._uid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-18.0

You can replace the call to translate a string () with self.env. for getting some performance improvement in some cases. See odoo/odoo#174844.

ibuioli and others added 17 commits June 27, 2025 08:53
Currently translated at 100.0% (8 of 8 strings)

Translation: server-tools-15.0/server-tools-15.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-15-0/server-tools-15-0-scheduler_error_mailer/es_AR/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-16.0/server-tools-16.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-scheduler_error_mailer/
Currently translated at 100.0% (9 of 9 strings)

Translation: server-tools-16.0/server-tools-16.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-scheduler_error_mailer/es_AR/
Currently translated at 100.0% (9 of 9 strings)

Translation: server-tools-16.0/server-tools-16.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-scheduler_error_mailer/es/
Update translation files

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

Translation: server-tools-16.0/server-tools-16.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-scheduler_error_mailer/
[BOT] post-merge updates

Update translation files

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

Translation: server-tools-17.0/server-tools-17.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-scheduler_error_mailer/
Translated using Weblate (Italian)

Currently translated at 100.0% (8 of 8 strings)

Translation: server-tools-17.0/server-tools-17.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-scheduler_error_mailer/it/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-tools-17.0/server-tools-17.0-scheduler_error_mailer
Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-scheduler_error_mailer/zh_CN/
Update scheduler_error_mailer/models/ir_cron.py

Co-authored-by: Christopher Rogos <crogos@gmail.com>
Update scheduler_error_mailer/models/ir_cron.py
C#o-authored-by: Christopher Rogos <crogos@gmail.com>
…del changes

[FIX] scheduler_error_mailer: migrated template might not have template_fs
@astirpe
astirpe force-pushed the 18_mig_scheduler_error_mailer branch from 05d6184 to d4cee9d Compare June 27, 2025 06:54
@astirpe

astirpe commented Jun 27, 2025

Copy link
Copy Markdown
Member Author

@CRogos squashed, thanks!

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@StefanRijnhart

Copy link
Copy Markdown
Member

/ocabot merge nobump

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-3084-by-StefanRijnhart-bump-nobump, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 9e5caeb into OCA:18.0 Jun 27, 2025
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 4d92829. 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.