Skip to content

Commit

Permalink
Merge pull request opf#7927 from EgorrodE/feature/21304-removing-watc…
Browse files Browse the repository at this point in the history
…her-sends-email

[#21304] [FEATURE] [Watchers] Removing watcher sends email to removed watcher
  • Loading branch information
ulferts authored Dec 16, 2019
2 parents e6d750e + b503f57 commit 1376444
Show file tree
Hide file tree
Showing 15 changed files with 267 additions and 58 deletions.
5 changes: 3 additions & 2 deletions app/mailers/user_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ def work_package_updated(user, journal, author = User.current)
end
end

def work_package_watcher_added(work_package, user, watcher_setter)
def work_package_watcher_changed(work_package, user, watcher_changer, action)
User.execute_as user do
@issue = work_package
@watcher_setter = watcher_setter
@watcher_changer = watcher_changer
@action = action

set_work_package_headers(work_package)
message_id work_package, user
Expand Down
40 changes: 40 additions & 0 deletions app/models/watcher_added_notification_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#-- encoding: UTF-8

#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++

class WatcherAddedNotificationMailer < WatcherNotificationMailer
class << self
private

def perform_notification_job(watcher, watcher_changer)
DeliverWatcherAddedNotificationJob
.perform_later(watcher.id, watcher.user.id, watcher_changer.id)
end
end
end
19 changes: 12 additions & 7 deletions app/models/watcher_notification_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#-- encoding: UTF-8

#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
Expand Down Expand Up @@ -29,18 +30,22 @@

class WatcherNotificationMailer
class << self
def handle_watcher(watcher, watcher_setter)
def handle_watcher(watcher, watcher_changer)
# We only handle this watcher setting if associated user wants to be notified
# about it.
return unless notify_about_watcher_added?(watcher, watcher_setter)
return unless notify_about_watcher_changed?(watcher, watcher_changer)

unless other_jobs_queued?(watcher.watchable)
DeliverWatcherNotificationJob.perform_later(watcher.id, watcher.user.id, watcher_setter.id)
perform_notification_job(watcher, watcher_changer)
end
end

private

def perform_notification_job(_watcher, _watcher_changer)
raise NotImplementedError, 'Subclass has to implement #notification_job'
end

# HACK: TODO this needs generalization as well as performance improvements
# We need to make sure no work package created or updated job is queued to avoid sending two
# mails in short succession.
Expand All @@ -49,8 +54,8 @@ def other_jobs_queued?(work_package)
"%NotificationJob%journal_id: #{work_package.journals.last.id}%").exists?
end

def notify_about_watcher_added?(watcher, watcher_setter)
return false if notify_about_self_watching?(watcher, watcher_setter)
def notify_about_watcher_changed?(watcher, watcher_changer)
return false if notify_about_self_watching?(watcher, watcher_changer)

case watcher.user.mail_notification
when 'only_my_events'
Expand All @@ -62,8 +67,8 @@ def notify_about_watcher_added?(watcher, watcher_setter)
end
end

def notify_about_self_watching?(watcher, watcher_setter)
watcher.user == watcher_setter && !watcher.user.pref.self_notified?
def notify_about_self_watching?(watcher, watcher_changer)
watcher.user == watcher_changer && !watcher.user.pref.self_notified?
end

def watching_selected_includes_project?(watcher)
Expand Down
41 changes: 41 additions & 0 deletions app/models/watcher_removed_notification_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#-- encoding: UTF-8

#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++

class WatcherRemovedNotificationMailer < WatcherNotificationMailer
class << self
private

def perform_notification_job(watcher, watcher_changer)
# As watcher is already destroyed we need to pass a hash
DeliverWatcherRemovedNotificationJob
.perform_later(watcher.attributes, watcher.user.id, watcher_changer.id)
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
See docs/COPYRIGHT.rdoc for more details.
++#%>
<%= t(:text_work_package_watcher_added, id: "##{@issue.id}", watcher_setter: @watcher_setter) %>
<%= t("text_work_package_watcher_#{@action}", id: "##{@issue.id}", watcher_changer: @watcher_changer) %>
<hr />
<%= render partial: 'issue_details', locals: { issue: @issue } %>
<p>
<%= format_text(t(:text_latest_note, note: last_issue_note(@issue)),
only_path: false,
object: @issue,
project: @issue.project) %>
only_path: false,
object: @issue,
project: @issue.project) %>
</p>
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ See docs/COPYRIGHT.rdoc for more details.
++#%>
<%= t(:text_work_package_watcher_added, id: "##{@issue.id}", watcher_setter: @watcher_setter) %>
<%= t("text_work_package_watcher_#{@action}", id: "##{@issue.id}", watcher_changer: @watcher_changer) %>

----------------------------------------
<%= render partial: 'issue_details', locals: { issue: @issue } %>
Expand Down
37 changes: 37 additions & 0 deletions app/workers/deliver_watcher_added_notification_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#-- encoding: UTF-8

#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++

class DeliverWatcherAddedNotificationJob < DeliverWatcherNotificationJob
def render_mail(recipient:, sender:)
return unless watcher

UserMailer.work_package_watcher_changed(watcher.watchable, recipient, sender, 'added')
end
end
12 changes: 5 additions & 7 deletions app/workers/deliver_watcher_notification_job.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#-- encoding: UTF-8

#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
Expand Down Expand Up @@ -28,17 +29,14 @@
#++

class DeliverWatcherNotificationJob < DeliverNotificationJob

def perform(watcher_id, recipient_id, watcher_setter_id)
def perform(watcher_id, recipient_id, watcher_changer_id)
@watcher_id = watcher_id

super(recipient_id, watcher_setter_id)
super(recipient_id, watcher_changer_id)
end

def render_mail(recipient:, sender:)
return nil unless watcher

UserMailer.work_package_watcher_added(watcher.watchable, recipient, sender)
def render_mail(recipient:, sender:) # rubocop:disable UnusedMethodArgument
raise NotImplementedError, 'Subclass has to implement #render_mail'
end

private
Expand Down
44 changes: 44 additions & 0 deletions app/workers/deliver_watcher_removed_notification_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#-- encoding: UTF-8

#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++

class DeliverWatcherRemovedNotificationJob < DeliverWatcherNotificationJob
# As watcher is already destroyed we need to pass a hash
def perform(watcher_attributes, recipient_id, watcher_remover_id)
@watcher = Watcher.new(watcher_attributes)

super(watcher.id, recipient_id, watcher_remover_id)
end

def render_mail(recipient:, sender:)
return unless watcher

UserMailer.work_package_watcher_changed(watcher.watchable, recipient, sender, 'removed')
end
end
6 changes: 5 additions & 1 deletion config/initializers/subscribe_listeners.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,9 @@
end

OpenProject::Notifications.subscribe('watcher_added') do |payload|
WatcherNotificationMailer.handle_watcher(payload[:watcher], payload[:watcher_setter])
WatcherAddedNotificationMailer.handle_watcher(payload[:watcher], payload[:watcher_setter])
end

OpenProject::Notifications.subscribe('watcher_removed') do |payload|
WatcherRemovedNotificationMailer.handle_watcher(payload[:watcher], payload[:watcher_remover])
end
3 changes: 2 additions & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2316,7 +2316,8 @@ en:
text_work_package_category_destroy_question: "Some work packages (%{count}) are assigned to this category. What do you want to do?"
text_work_package_category_reassign_to: "Reassign work packages to this category"
text_work_package_updated: "Work package %{id} has been updated by %{author}."
text_work_package_watcher_added: "You have been added as a watcher to Work package %{id} by %{watcher_setter}."
text_work_package_watcher_added: "You have been added as a watcher to Work package %{id} by %{watcher_changer}."
text_work_package_watcher_removed: "You have been removed from watchers of Work package %{id} by %{watcher_changer}."
text_work_packages_destroy_confirmation: "Are you sure you want to delete the selected work package(s)?"
text_work_packages_ref_in_commit_messages: "Referencing and fixing work packages in commit messages"
text_journal_added: "%{label} %{value} added"
Expand Down
7 changes: 6 additions & 1 deletion lib/services/remove_watcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ def initialize(work_package, user)
end

def run(success: -> {}, failure: -> {})
if @work_package.watcher_users.include?(@user)
watcher = @work_package.watchers.find_by_user_id(@user.id)

if watcher.present?
@work_package.watcher_users.delete(@user)
success.call
OpenProject::Notifications.send('watcher_removed',
watcher: watcher,
watcher_remover: User.current)
else
failure.call
end
Expand Down
19 changes: 14 additions & 5 deletions spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@
end

shared_examples_for 'mail is sent' do
let(:letters_sent_count) { 1 }

it 'actually sends a mail' do
expect(ActionMailer::Base.deliveries.size).to eql(1)
expect(ActionMailer::Base.deliveries.size).to eql(letters_sent_count)
end

it 'is sent to the recipient' do
Expand All @@ -66,6 +68,12 @@
end
end

shared_examples_for 'multiple mails are sent' do |set_letters_sent_count|
it_behaves_like 'mail is sent' do
let(:letters_sent_count) { set_letters_sent_count }
end
end

shared_examples_for 'mail is not sent' do
it 'sends no mail' do
expect(ActionMailer::Base.deliveries.size).to eql(0)
Expand Down Expand Up @@ -130,13 +138,14 @@
it_behaves_like 'does only send mails to author if permitted'
end

describe '#work_package_watcher_added' do
let(:watcher_setter) { user }
describe '#work_package_watcher_changed' do
let(:watcher_changer) { user }
before do
UserMailer.work_package_watcher_added(work_package, recipient, watcher_setter).deliver_now
UserMailer.work_package_watcher_changed(work_package, recipient, watcher_changer, 'added').deliver_now
UserMailer.work_package_watcher_changed(work_package, recipient, watcher_changer, 'removed').deliver_now
end

it_behaves_like 'mail is sent'
include_examples 'multiple mails are sent', 2

it 'contains the WP subject in the mail subject' do
expect(ActionMailer::Base.deliveries.first.subject).to include(work_package.subject)
Expand Down
Loading

0 comments on commit 1376444

Please sign in to comment.