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
add modifiers to message_builder so plugins can customize subject/body/html #26867
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.
It would be nice to add a few specs to ensure the modifiers are working as expected.
Also I think you forgot to run stree
. 😄
lib/email/message_builder.rb
Outdated
@@ -126,6 +126,7 @@ def subject | |||
else | |||
subject = @opts[:subject] | |||
end | |||
subject = DiscoursePluginRegistry.apply_modifier(:user_notification_subject, subject,@opts) |
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.
Message builder is also used for other e-mails type, not only notifications.
The modifiers name should be more generic, something like message_builder_subject
for example.
lib/email/message_builder.rb
Outdated
@@ -164,6 +165,7 @@ def html_part | |||
html_body: html_override.html_safe, | |||
}, | |||
) | |||
html = DiscoursePluginRegistry.apply_modifier(:user_notification_html_part, html,@opts) |
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.
Like above, maybe message_builder_html_part
?
lib/email/message_builder.rb
Outdated
@@ -184,6 +186,7 @@ def body | |||
body << "\n" | |||
body << @template_args[:unsubscribe_instructions] | |||
end | |||
body = DiscoursePluginRegistry.apply_modifier(:user_notification_body, body,@opts) |
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.
Like above, maybe message_builder_body
?
(also add notification_type to email_opts so plugins can access it)