-
-
Notifications
You must be signed in to change notification settings - Fork 606
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
[15.0][IMP][mail_show_follower] Customize notification appearance #866
[15.0][IMP][mail_show_follower] Customize notification appearance #866
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing! Thank you! 👍
@cvinh you have here all you need
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! It seems a good job indeed. I feel it's a bit over-engineered on some points, commented below.
Please attend those comments, to reduce the feature set to only what's really needed, so it becomes more maintainable.
Also, if possible, add tests.
Apart from that, the core feature you're adding seems a great addition indeed. Good job, thanks!
98a317e
to
6238e44
Compare
4a5358c
to
38ba924
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The improvement looks a lot better now!
There's one point where I think it still needs a bit of enhancement, and other points where I have suggestions or doubts.
Thanks for your great job and patience, this is becoming in very good shape. 😊
Added settings to customize notification and translations to important parts of the message. [FIX] Security fixes and simplified customization Field `show_followers_partner_format` formatted with %(param)s Removed CSS customizations [FIX] README options [FIX] Index.html configure [FIX] Several changes - Removed unnecesary properties on res_config_settings - Markup safe on partner_format - Use email_domain_extract instead own one
38ba924
to
59966f0
Compare
/ocabot merge minor |
What a great day to merge this nice PR. Let's do it! |
@yajo your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-866-by-Yajo-bump-minor. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
/ocabot merge minor |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 02846ae. Thanks a lot for contributing to OCA. ❤️ |
Added settings to customize notification and translations to important parts of the message.
show_followers_message_sent_to
: The first part of the messageshow_followers_partner_format
: Partner format for emails. You can chooose which fields to showshow_followers_message_response_warning
: Additional text if you want to warn for the reply behaviorIncludes a preview of the message on settings (only visible if you are in debug mode)
The lang of the message is on the same as the recipient. Some customization settings are translatable (enable another language to test properly)
@yajo @rafaelbn @HaraldPanten @ValentinVinagre @pedrobaeza @flotho @cvinh @tafaRU please review 😄
MT-431 @moduon