-
Notifications
You must be signed in to change notification settings - Fork 35
Added email notification on topic assignment #9
Conversation
This definitely would need to be a config option default off.
…On Mon, Nov 13, 2017 at 7:05 AM spog163 ***@***.***> wrote:
Hi, could you add the email notification on topic asssignment to your
amazing plugin, please?
Greetings from Russia:)
------------------------------
You can view, comment on, or merge this pull request online at:
#9
Commit Summary
- Added email notification on topic assignment
File Changes
- *A* app/mailers/assing_mailer.rb
<https://github.com/discourse/discourse-assign/pull/9/files#diff-0>
(16)
- *M* config/locales/server.en.yml
<https://github.com/discourse/discourse-assign/pull/9/files#diff-1>
(11)
- *M* lib/topic_assigner.rb
<https://github.com/discourse/discourse-assign/pull/9/files#diff-2>
(5)
Patch Links:
- https://github.com/discourse/discourse-assign/pull/9.patch
- https://github.com/discourse/discourse-assign/pull/9.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABc7VQvDMShft_47kuYzyFWilybKfOvdks5s2Fq4gaJpZM4Qb51G>
.
|
@spog163 let us know once this is setup as a site setting. |
lib/topic_assigner.rb
Outdated
@@ -110,6 +112,9 @@ def assign(assign_to, silent: false) | |||
user_ids: staff_ids | |||
) | |||
|
|||
message = AssignMailer.send_assignment(assign_to.email, @topic, @assigned_by) |
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.
Should this unconditionally send out the email even if the user mutes the topic?
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.
Let me answer, @spog163 and I worked on this improvement. Currently it sends emails always without an option to disable. Our task was pointed to staff users, who need to have an email notification always. Agree, that this setting must be an option (disabled by default) and depend on mute conditions. So, let us to make some updates first :)
en correct localizations, added russian translation
@SamSaffron, @coding-horror Added global options as site settings + russian translation (@rapekas) |
config/locales/server.en.yml
Outdated
@@ -10,8 +10,21 @@ en: | |||
unassign_on_group_archive: "When a message is archived by a group, unassign message (reassign if moved back to inbox)" | |||
unassign_on_close: "When a topic is closed unassign topic" | |||
assign_locks_flags: "When a topic is assigned to a staff member, its flags can only be handled by that person" | |||
assign_mailer_enabled: "When enabled, the assigned user will receive a notification email on each assignment" | |||
assign_mailer_disabled_for_muted_topics: "When enabled, the assigned user will not receive emails on muted topics" |
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.
This is overdoing it I would remove this option, if email_on_assignment is enabled then it should never assign on muted topics you should not be allowed to override it.
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 for reviewing! Actually user always receive emails on muted topics if 'assign_mailer_disabled_for_muted_topics' disabled or absent. So it is not overrided by any other site setting.
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.
Sure I understand, I just feel it is an option we don't need to provide.... I would like to delete the setting "assign_mailer_disabled_for_muted_topics"
Removed override option for muted topics
@SamSaffron Ok, we removed the override option for muted topics, please review. |
Looks great 👍 It's missing a test though. |
Thanks @rapekas maybe if you have some time a bit later submit a PR for the test, thanks for the contribution! |
Hi, could you add the email notification on topic asssignment to your amazing plugin, please?
Greetings from Russia:)