allow adding tags as a custom subject format for emails #5846
Conversation
|
You've signed the CLA, majakomel. Thank you! This pull request is ready for review. |
app/mailers/user_notifications.rb
Outdated
| @@ -433,6 +443,14 @@ def send_notification_email(opts) | |||
| show_category_in_subject = nil | |||
| end | |||
|
|
|||
| # tag names | |||
| tags = Topic.find_by(id: post.topic_id)&.tags | |||
| if opts[:show_tags_in_subject] && post.topic_id && tags | |||
There was a problem hiding this comment.
I would first check tags are allowed in subject and only then do the SQL query.
There was a problem hiding this comment.
Also, there should probably be a limit of the number of tags retrieved.
There was a problem hiding this comment.
What would be the best limit? 3 tags? Should there be any specific order as well then?
There was a problem hiding this comment.
3 seems fine. I would keep the same order as in the post.
|
This looks good to me, should we be adding spaces between tags or having them this close together? Note we need to wait 2 weeks to merge this in cause we are stabalizing the 2.0 release at the moment. |
|
This pull request has been mentioned on Discourse Meta. There might be relevant details there: https://meta.discourse.org/t/allow-adding-tags-as-a-custom-subject-format-for-emails/84424/4 |
|
cool, merging now |
Meta: https://meta.discourse.org/t/allow-adding-tags-as-a-custom-subject-format-for-emails/84424