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.

Sign up for GitHub

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

Jump to bottom

[15.0][MIG] mail_notification_custom_subject: Migration to 15.0 #982

Merged
merged 12 commits into from
Oct 25, 2022
Merged

[15.0][MIG] mail_notification_custom_subject: Migration to 15.0 #982

merged 12 commits into from
Oct 25, 2022

Conversation

victoralmau
Copy link
Member

Migration to 15.0

Please @CarlosRoca13 and @chienandalu can you review it?

@Tecnativa TT36470

joao-p-marques and others added 9 commits October 19, 2022 12:40
@joao-p-marques @pedrobaeza @victoralmau
[ADD] mail_notification_custom_subject: Apply a custom subject to mai…
586be98 
…l notifications

This module allows you to specify templates to override the subject on the notification
emails sent by Odoo.

TT27191

Co-authored-by: Pedro M. Baeza <pedro.baeza@tecnativa.com>
@CarlosRoca13 @victoralmau
[IMP] mail_notification_custom_subject: black, isort, prettier
1bb5aee
@CarlosRoca13 @victoralmau
[MIG] mail_notification_custom_subject: Migration to v13.0
7b747ff
@victoralmau
[IMP] mail_notification_custom_subject: Add comment to explain test_b…
51c4d08 
…ad_template_does_not_break test
@victoralmau
[IMP] mail_notification_custom_subject: black, isort, prettier
d5f0d97
@victoralmau
[MIG] mail_notification_custom_subject: Migration to 14.0
14cc2d9
@oca-travis @victoralmau
[UPD] Update mail_notification_custom_subject.pot
ddc9efe 
[UPD] README.rst
@olgamarcocb @victoralmau
[FIX] mail_notification_custom_subject: Error modifying subtype_id
4b5eeec 
[IMP] update dotfiles [ci skip]
@victoralmau
[IMP] mail_notification_custom_subject: black, isort, prettier
405f4ef
@victoralmau victoralmau force-pushed the 15.0-mig-mail_notification_custom_subject branch from 36cd7b5 to e3d5033 Compare October 19, 2022 11:02
@victoralmau victoralmau force-pushed the 15.0-mig-mail_notification_custom_subject branch from e3d5033 to fcfd428 Compare October 19, 2022 11:27
@victoralmau victoralmau force-pushed the 15.0-mig-mail_notification_custom_subject branch from fcfd428 to dcf0748 Compare October 19, 2022 11:29
chienandalu
chienandalu reviewed Oct 24, 2022
View reviewed changes
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Tested functionaly. Some formal comments:

mail_notification_custom_subject/models/mail_thread.py Show resolved Hide resolved
mail_notification_custom_subject/models/mail_thread.py Show resolved Hide resolved
mail_notification_custom_subject/models/mail_thread.py Show resolved Hide resolved
@victoralmau victoralmau force-pushed the 15.0-mig-mail_notification_custom_subject branch from dcf0748 to fede458 Compare October 24, 2022 10:55
@victoralmau
Copy link
Member Author

Tested functionaly. Some formal comments:

Changes done, but it seems to have side effects on other modules. Any suggestions to solve it?

@pedrobaeza
Copy link
Member

You can't do that, as not the whole inheritance chain is respecting keyword arguments, so better to stick to the whole form.

@victoralmau victoralmau force-pushed the 15.0-mig-mail_notification_custom_subject branch from fede458 to 3863865 Compare October 25, 2022 06:06
@victoralmau
Copy link
Member Author

Changes done (last changes undone)

@pedrobaeza
Copy link
Member

/ocabot merge nobump
/ocabot migration mail_notification_custom_subject

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Oct 25, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 25, 2022
Open
36 tasks
@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 15.0-ocabot-merge-pr-982-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 7b6c452 into OCA:15.0 Oct 25, 2022
@OCA-git-bot
Copy link
Contributor

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

@chienandalu
Copy link
Member

You can't do that, as not the whole inheritance chain is respecting keyword arguments, so better to stick to the whole form.

Odoo's choice, not mine 😄 :

@pedrobaeza pedrobaeza deleted the 15.0-mig-mail_notification_custom_subject branch October 25, 2022 07:11
@pedrobaeza
Copy link
Member

Well, that ones probably will raise an error with this same chain... Maybe it's a problem in one OCA module, but that form requires that you always call super with all the keywords.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chienandalu chienandalu chienandalu left review comments

@CarlosRoca13 CarlosRoca13 CarlosRoca13 approved these changes

Assignees
No one assigned
Labels
merged 🎉
Projects
None yet
Milestone
15.0
Development

Successfully merging this pull request may close these issues.
8 participants
@victoralmau @pedrobaeza @OCA-git-bot @chienandalu @CarlosRoca13 @joao-p-marques @oca-travis @olgamarcocb