diff --git a/app/services/notifications/journal_wp_notification_service.rb b/app/services/notifications/journal_wp_notification_service.rb index 43fab77070..37690bb5ba 100644 --- a/app/services/notifications/journal_wp_notification_service.rb +++ b/app/services/notifications/journal_wp_notification_service.rb @@ -43,7 +43,7 @@ class Notifications::JournalWpNotificationService def call(journal, send_notifications) return nil if abort_sending?(journal, send_notifications) - notification_receivers(journal.journable, journal).each do |recipient_id, channel_reasons| + notification_receivers(journal).each do |recipient_id, channel_reasons| create_notification(journal, recipient_id, channel_reasons) @@ -77,23 +77,22 @@ class Notifications::JournalWpNotificationService } end - def notification_receivers(work_package, journal) + def notification_receivers(journal) receivers = receivers_hash - add_receiver(receivers, settings_of_mentioned(journal), :mentioned) - add_receiver(receivers, settings_of_involved(journal), :involved) - add_receiver(receivers, settings_of_watched(work_package), :watched) - add_receiver(receivers, settings_of_subscribed(journal), :subscribed) - add_receiver(receivers, settings_of_commented(journal), :commented) - add_receiver(receivers, settings_of_created(journal), :created) - add_receiver(receivers, settings_of_processed(journal), :processed) - add_receiver(receivers, settings_of_prioritized(journal), :prioritized) + %i(mentioned involved watched subscribed commented created processed prioritized).each do |reason| + add_receivers_by_reason(receivers, journal, reason) + end remove_self_recipient(receivers, journal) receivers end + def add_receivers_by_reason(receivers, journal, reason) + add_receiver(receivers, send(:"settings_of_#{reason}", journal), reason) + end + def settings_of_mentioned(journal) applicable_settings(mentioned_ids(journal), journal.data.project, @@ -118,7 +117,9 @@ class Notifications::JournalWpNotificationService :all) end - def settings_of_watched(work_package) + def settings_of_watched(journal) + work_package = journal.journable + applicable_settings(work_package.watcher_users, work_package.project, :watched) @@ -194,11 +195,7 @@ class Notifications::JournalWpNotificationService end def send_notification?(journal, send_notifications) - send_notifications && ::UserMailer.perform_deliveries && send_notification_setting?(journal) - end - - def send_notification_setting?(journal) - notify_for_wp_updated?(journal) || journal.initial? + send_notifications && ::UserMailer.perform_deliveries end def mention_matches(journal) @@ -222,14 +219,6 @@ class Notifications::JournalWpNotificationService } end - def notify_for_wp_updated?(journal) - notification_enabled?('work_package_updated') && !journal.initial? - end - - def notification_enabled?(name) - Setting.notified_events.include?(name) - end - def abort_sending?(journal, send_notifications) !send_notification?(journal, send_notifications) || journal.noop? end diff --git a/config/locales/en.yml b/config/locales/en.yml index 24594ced0c..1d0508b4dd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1900,7 +1900,6 @@ en: label_work_package_status_new: "New status" label_work_package_status_plural: "Work package statuses" label_work_package_types: "Work package types" - label_work_package_updated: "Work package updated" label_work_package_tracking: "Work package tracking" label_work_package_view_all: "View all work packages" label_workflow: "Workflow" diff --git a/config/settings.yml b/config/settings.yml index 95c703bd4b..647f98ea15 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -236,7 +236,6 @@ cross_project_work_package_relations: notified_events: serialized: true default: - - work_package_updated - news_added - news_comment_added - file_added diff --git a/db/migrate/20210802114054_add_notification_setting_options.rb b/db/migrate/20210802114054_add_notification_setting_options.rb index 8cc31263f3..84ae52751d 100644 --- a/db/migrate/20210802114054_add_notification_setting_options.rb +++ b/db/migrate/20210802114054_add_notification_setting_options.rb @@ -1,13 +1,34 @@ class AddNotificationSettingOptions < ActiveRecord::Migration[6.1] def change + add_notification_settings_options + update_notified_events + + # TODO: add index to all boolean fields + end + + def add_notification_settings_options change_table :notification_settings, bulk: true do |t| t.boolean :work_package_commented, default: false t.boolean :work_package_created, default: false t.boolean :work_package_processed, default: false t.boolean :work_package_prioritized, default: false end + end - # TODO: remove existing notification setting from settings - # TODO: add index to all boolean fields + def update_notified_events + event_types = %w(work_package_added work_package_updated work_package_note_added status_updated work_package_priority_updated) + + # rubocop:disable Rails/WhereExists + # The Setting.exists? method is overwritten + reversible do |dir| + dir.up do + Setting.notified_events = Setting.notified_events - event_types if Setting.where(name: 'notified_events').exists? + end + + dir.down do + Setting.notified_events = Setting.notified_events + event_types if Setting.where(name: 'notified_events').exists? + end + end + # rubocop:enable Rails/WhereExists end end diff --git a/lib/open_project/notifiable.rb b/lib/open_project/notifiable.rb index adbd9bc3aa..e78a9254d4 100644 --- a/lib/open_project/notifiable.rb +++ b/lib/open_project/notifiable.rb @@ -30,7 +30,6 @@ module OpenProject NOTIFIABLE = [ - %w(work_package_updated), %w(news_added), %w(news_comment_added), %w(file_added), diff --git a/spec/lib/open_project/notifiable_spec.rb b/spec/lib/open_project/notifiable_spec.rb index 7de7a6ec8c..f76aa6aa61 100644 --- a/spec/lib/open_project/notifiable_spec.rb +++ b/spec/lib/open_project/notifiable_spec.rb @@ -32,8 +32,8 @@ require 'spec_helper' describe OpenProject::Notifiable do describe '#all' do it 'matches expected list' do - expected = %w(work_package_updated news_added news_comment_added - file_added message_posted wiki_content_added wiki_content_updated membership_added membership_updated) + expected = %w(news_added news_comment_added file_added message_posted wiki_content_added + wiki_content_updated membership_added membership_updated) expect(described_class.all.map(&:name)) .to match_array(expected) diff --git a/spec/services/notifications/journal_wp_notification_service_spec.rb b/spec/services/notifications/journal_wp_notification_service_spec.rb index 24d3d026b7..ceffcc9b1a 100644 --- a/spec/services/notifications/journal_wp_notification_service_spec.rb +++ b/spec/services/notifications/journal_wp_notification_service_spec.rb @@ -120,9 +120,6 @@ describe Notifications::JournalWpNotificationService, with_settings: { journal_a work_package.journals.last end let(:send_notifications) { true } - let(:notification_setting) do - %w(work_package_updated) - end def call described_class.call(journal, send_notifications) @@ -133,7 +130,6 @@ describe Notifications::JournalWpNotificationService, with_settings: { journal_a allow(OpenProject::Notifications).to receive(:send) # ... and do nothing login_as(author) - allow(Setting).to receive(:notified_events).and_return(notification_setting) end shared_examples_for 'creates notification' do diff --git a/spec/workers/notifications/journal_completed_job_integration_spec.rb b/spec/workers/notifications/journal_completed_job_integration_spec.rb index 5868e8b040..83d1140e1c 100644 --- a/spec/workers/notifications/journal_completed_job_integration_spec.rb +++ b/spec/workers/notifications/journal_completed_job_integration_spec.rb @@ -53,9 +53,9 @@ describe Notifications::JournalCompletedJob, type: :model do author: author, assigned_to: recipient) end - let(:journal) { journal_1 } - let(:journal_1) { work_package.journals.first } - let(:journal_2) do + let(:journal) { journal1 } + let(:journal1) { work_package.journals.first } + let(:journal2) do work_package.add_journal author, 'something I have to say' work_package.save(validate: false) work_package.journals.last @@ -93,7 +93,7 @@ describe Notifications::JournalCompletedJob, type: :model do end describe 'journal creation' do - context 'work_package_created' do + context 'with the work package being created' do before do FactoryBot.create(:work_package, project: project) end @@ -101,7 +101,7 @@ describe Notifications::JournalCompletedJob, type: :model do it_behaves_like 'sends notification' end - context 'work_package_updated' do + context 'with the work package being updated' do before do work_package.add_journal(author) work_package.subject = 'A change to the issue' @@ -111,7 +111,7 @@ describe Notifications::JournalCompletedJob, type: :model do it_behaves_like 'sends notification' end - context 'work_package_note_added' do + context 'with the journal being updated with a note' do before do work_package.add_journal(author, 'This update has a note') work_package.save!(validate: false) @@ -122,7 +122,7 @@ describe Notifications::JournalCompletedJob, type: :model do end end - context 'for wiki page content' do + context 'with wiki page content' do let(:wiki_page_content) do wiki = FactoryBot.create(:wiki, project: project) @@ -130,9 +130,9 @@ describe Notifications::JournalCompletedJob, type: :model do FactoryBot.create(:wiki_page_with_content, wiki: wiki).content end - let(:journal) { journal_1 } - let(:journal_1) { wiki_page_content.journals.first } - let(:journal_2) do + let(:journal) { journal1 } + let(:journal1) { wiki_page_content.journals.first } + let(:journal2) do wiki_page_content.add_journal author, 'something I have to say' wiki_page_content.save(validate: false) wiki_page_content.journals.last