<%= setting_time_field :notification_email_digest_time,
container_class: '-xslim',
diff --git a/app/views/user_mailer/work_package_added.html.erb b/app/views/user_mailer/work_package_added.html.erb
deleted file mode 100644
index 5c2fbbf265..0000000000
--- a/app/views/user_mailer/work_package_added.html.erb
+++ /dev/null
@@ -1,33 +0,0 @@
-<%#-- copyright
-OpenProject is an open source project management software.
-Copyright (C) 2012-2021 the OpenProject GmbH
-
-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-2013 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 COPYRIGHT and LICENSE files for more details.
-
-++#%>
-
-<%= t(:text_work_package_added, id: "##{@issue.id}", author: @issue.author) %>
-<%= format_text(@journal.notes, only_path: false, object: @issue, project: @issue.project) %>
-
-<%= render partial: 'issue_details', locals: { issue: @issue } %>
diff --git a/app/views/user_mailer/work_package_added.text.erb b/app/views/user_mailer/work_package_added.text.erb
deleted file mode 100644
index de377e7f82..0000000000
--- a/app/views/user_mailer/work_package_added.text.erb
+++ /dev/null
@@ -1,34 +0,0 @@
-<%#-- copyright
-OpenProject is an open source project management software.
-Copyright (C) 2012-2021 the OpenProject GmbH
-
-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-2013 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 COPYRIGHT and LICENSE files for more details.
-
-++#%>
-
-<%= t(:text_work_package_added, id: "##{@issue.id}", author: @issue.author) %>
-
-<%= @journal.notes if @journal.notes? %>
-----------------------------------------
-<%= render partial: 'issue_details', locals: { issue: @issue } %>
diff --git a/app/views/user_mailer/work_package_updated.html.erb b/app/views/user_mailer/work_package_updated.html.erb
deleted file mode 100644
index c33c2ae218..0000000000
--- a/app/views/user_mailer/work_package_updated.html.erb
+++ /dev/null
@@ -1,37 +0,0 @@
-<%#-- copyright
-OpenProject is an open source project management software.
-Copyright (C) 2012-2021 the OpenProject GmbH
-
-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-2013 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 COPYRIGHT and LICENSE files for more details.
-
-++#%>
-<%= t(:text_work_package_updated, id: "##{@issue.id}", author: @journal.user) %>
-
- <% @journal.details.each do |detail| %>
- - <%= @journal.render_detail(detail, only_path: false) %>
- <% end %>
-
-<%= format_text(@journal.notes, only_path: false, object: @issue, project: @issue.project) %>
-
-<%= render partial: 'issue_details', locals: { issue: @issue } %>
diff --git a/app/views/user_mailer/work_package_updated.text.erb b/app/views/user_mailer/work_package_updated.text.erb
deleted file mode 100644
index 2178fe2729..0000000000
--- a/app/views/user_mailer/work_package_updated.text.erb
+++ /dev/null
@@ -1,38 +0,0 @@
-<%#-- copyright
-OpenProject is an open source project management software.
-Copyright (C) 2012-2021 the OpenProject GmbH
-
-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-2013 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 COPYRIGHT and LICENSE files for more details.
-
-++#%>
-
-<%= t(:text_work_package_updated, id: "##{@issue.id}", author: @journal.user) %>
-
-<% @journal.details.each do |detail| %>
- <%= @journal.render_detail(detail, no_html: true, only_path: false) %>
-<% end %>
-
-<%= @journal.notes if @journal.notes? %>
-----------------------------------------
-<%= render partial: 'issue_details', locals: { issue: @issue } %>
diff --git a/app/workers/notifications/workflow_job.rb b/app/workers/notifications/workflow_job.rb
index e562658b48..53e13fc722 100644
--- a/app/workers/notifications/workflow_job.rb
+++ b/app/workers/notifications/workflow_job.rb
@@ -60,9 +60,7 @@ class Notifications::WorkflowJob < ApplicationJob
end
state :send_mails,
- wait: -> {
- Setting.notification_email_delay_minutes.minutes + Setting.journal_aggregation_time_minutes.to_i.minutes
- } do |*notification_ids|
+ wait: -> { Setting.journal_aggregation_time_minutes.to_i.minutes } do |*notification_ids|
next unless notification_ids
Notification
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 62534df342..8a284a67b5 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2452,7 +2452,6 @@ en:
setting_enabled_scm: "Enabled SCM"
setting_enabled_projects_columns: "Visible in project list"
setting_notification_retention_period_days: "Notification retention period"
- setting_notification_email_delay_minutes: "Email sending delay"
setting_notification_email_digest_time: "Email digest time"
setting_feeds_enabled: "Enable Feeds"
setting_feeds_limit: "Feed content limit"
@@ -2535,7 +2534,6 @@ en:
retention_text: >
Set the number of days notification events for users (the source for in-app notifications)
will be kept in the system. Any events older than this time will be deleted.
- delay_minutes_explanation: "Email sending can be delayed to allow users with configured in app notification to confirm the notification within the application before a mail is sent out. Users who read a notification within the application will not receive an email for the already read notification."
email_digest_explanation: "Once a day, an email digest can be sent out containing a collection of all the notification users subscribed to. The setting is relative to each users configured time zone, so e.g. 8:00 will be executed at 7:00 UTC for users in UTC+1 and 9:00 UTC for those in UTC-1."
events_explanation: 'Governs for which event an email is sent out. Work packages are excluded from this list as the notifications for them can be configured specifically for every user.'
display:
diff --git a/config/settings.yml b/config/settings.yml
index 0f28c025fe..29dab8222e 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -382,8 +382,5 @@ apiv3_docs_enabled:
notification_retention_period_days:
default: 30
format: int
-notification_email_delay_minutes:
- default: 15
- format: int
notification_email_digest_time:
default: '08:00'
diff --git a/spec/features/notifications/immediate_reminder_spec.rb b/spec/features/notifications/immediate_reminder_spec.rb
index cf2f82bbcf..a148016a23 100644
--- a/spec/features/notifications/immediate_reminder_spec.rb
+++ b/spec/features/notifications/immediate_reminder_spec.rb
@@ -21,7 +21,7 @@ describe "Immediate reminder settings", type: :feature, js: true do
reminders_settings_page.reload!
- reminders_settings_page.set_immediate_reminder :mentioned, true
+ reminders_settings_page.expect_immediate_reminder :mentioned, true
expect(pref.reload.immediate_reminders[:mentioned]).to eq true
end
diff --git a/spec/lib/open_project/hook_spec.rb b/spec/lib/open_project/hook_spec.rb
index dd87cc078e..e3005e8908 100644
--- a/spec/lib/open_project/hook_spec.rb
+++ b/spec/lib/open_project/hook_spec.rb
@@ -254,9 +254,8 @@ describe OpenProject::Hook do
.and_return(wp)
end
end
- let(:journal) { FactoryBot.build_stubbed(:work_package_journal, journable: work_package) }
let!(:comparison_mail) do
- UserMailer.work_package_added(user, journal, author).deliver_now
+ UserMailer.work_package_watcher_changed(work_package, user, author, :added).deliver_now
ActionMailer::Base.deliveries.last
end
@@ -264,7 +263,7 @@ describe OpenProject::Hook do
test_hook_controller_class.new.call_hook(:view_layouts_base_html_head)
ActionMailer::Base.deliveries.clear
- UserMailer.work_package_added(user, journal, author).deliver_now
+ UserMailer.work_package_watcher_changed(work_package, user, author, :added).deliver_now
mail2 = ActionMailer::Base.deliveries.last
assert_equal comparison_mail.text_part.body.encoded, mail2.text_part.body.encoded
diff --git a/spec/mailers/digest_mailer_spec.rb b/spec/mailers/digest_mailer_spec.rb
index 89c51353ed..e807a4e89b 100644
--- a/spec/mailers/digest_mailer_spec.rb
+++ b/spec/mailers/digest_mailer_spec.rb
@@ -78,11 +78,6 @@ describe DigestMailer, type: :mailer do
let(:mail_body) { mail.body.parts.detect { |part| part['Content-Type'].value == 'text/html' }.body.to_s }
- before do
- allow(CustomStyle.current)
- .to receive(:logo).and_return(nil)
- end
-
it 'notes the day and the number of notifications in the subject' do
expect(mail.subject)
.to eql "OpenProject - 1 unread notification"
@@ -140,5 +135,347 @@ describe DigestMailer, type: :mailer do
.to eql({})
end
end
+
+ describe 'journal details in plain mail', with_settings: { plain_text_mail: '1' } do
+ subject(:mail) { described_class.work_packages(recipient.id, notifications.map(&:id)).body.encoded.gsub("\r\n", "\n") }
+
+ context 'with changed done ratio' do
+ before do
+ allow(journal).to receive(:details).and_return('done_ratio' => [40, 100])
+ end
+
+ it 'displays changed done ratio' do
+ expect(subject).to include("Progress (%) changed from 40 to 100")
+ end
+ end
+
+ context 'with new done ratio' do
+ before do
+ allow(journal).to receive(:details).and_return('done_ratio' => [nil, 100])
+ end
+
+ it 'displays new done ratio' do
+ expect(subject).to include("Progress (%) changed from 0 to 100")
+ end
+ end
+
+ context 'with deleted done ratio' do
+ before do
+ allow(journal).to receive(:details).and_return('done_ratio' => [50, nil])
+ end
+
+ it 'displays deleted done ratio' do
+ expect(subject).to include("Progress (%) changed from 50 to 0")
+ end
+ end
+
+ describe 'start_date attribute' do
+ before do
+ allow(journal).to receive(:details).and_return('start_date' => %w[2010-01-01 2010-01-31])
+ end
+
+ it 'old date should be formatted' do
+ expect(subject).to match('01/01/2010')
+ end
+
+ it 'new date should be formatted' do
+ expect(subject).to match('01/31/2010')
+ end
+ end
+
+ describe 'due_date attribute' do
+ before do
+ allow(journal).to receive(:details).and_return('due_date' => %w[2010-01-01 2010-01-31])
+ end
+
+ it 'old date should be formatted' do
+ expect(subject).to match('01/01/2010')
+ end
+
+ it 'new date should be formatted' do
+ expect(subject).to match('01/31/2010')
+ end
+ end
+
+ describe 'project attribute' do
+ let(:project1) { FactoryBot.create(:project) }
+ let(:project2) { FactoryBot.create(:project) }
+
+ before do
+ allow(journal).to receive(:details).and_return('project_id' => [project1.id, project2.id])
+ end
+
+ it "shows the old project's name" do
+ expect(subject).to match(project1.name)
+ end
+
+ it "shows the new project's name" do
+ expect(subject).to match(project2.name)
+ end
+ end
+
+ describe 'attribute issue status' do
+ let(:status1) { FactoryBot.create(:status) }
+ let(:status2) { FactoryBot.create(:status) }
+
+ before do
+ allow(journal).to receive(:details).and_return('status_id' => [status1.id, status2.id])
+ end
+
+ it "shows the old status' name" do
+ expect(subject).to match(status1.name)
+ end
+
+ it "shows the new status' name" do
+ expect(subject).to match(status2.name)
+ end
+ end
+
+ describe 'attribute type' do
+ let(:type1) { FactoryBot.create(:type_standard) }
+ let(:type2) { FactoryBot.create(:type_bug) }
+
+ before do
+ allow(journal).to receive(:details).and_return('type_id' => [type1.id, type2.id])
+ end
+
+ it "shows the old type's name" do
+ expect(subject).to match(type1.name)
+ end
+
+ it "shows the new type's name" do
+ expect(subject).to match(type2.name)
+ end
+ end
+
+ describe 'attribute assigned to' do
+ let(:assignee1) { FactoryBot.create(:user) }
+ let(:assignee2) { FactoryBot.create(:user) }
+
+ before do
+ allow(journal).to receive(:details).and_return('assigned_to_id' => [assignee1.id, assignee2.id])
+ end
+
+ it "shows the old assignee's name" do
+ expect(subject).to match(assignee1.name)
+ end
+
+ it "shows the new assignee's name" do
+ expect(subject).to match(assignee2.name)
+ end
+ end
+
+ describe 'attribute priority' do
+ let(:priority1) { FactoryBot.create(:priority) }
+ let(:priority2) { FactoryBot.create(:priority) }
+
+ before do
+ allow(journal).to receive(:details).and_return('priority_id' => [priority1.id, priority2.id])
+ end
+
+ it "shows the old priority's name" do
+ expect(subject).to match(priority1.name)
+ end
+
+ it "shows the new priority's name" do
+ expect(subject).to match(priority2.name)
+ end
+ end
+
+ describe 'attribute category' do
+ let(:category1) { FactoryBot.create(:category) }
+ let(:category2) { FactoryBot.create(:category) }
+
+ before do
+ allow(journal).to receive(:details).and_return('category_id' => [category1.id, category2.id])
+ end
+
+ it "shows the old category's name" do
+ expect(subject).to match(category1.name)
+ end
+
+ it "shows the new category's name" do
+ expect(subject).to match(category2.name)
+ end
+ end
+
+ describe 'attribute version' do
+ let(:version1) { FactoryBot.create(:version) }
+ let(:version2) { FactoryBot.create(:version) }
+
+ before do
+ allow(journal).to receive(:details).and_return('version_id' => [version1.id, version2.id])
+ end
+
+ it "shows the old version's name" do
+ expect(subject).to match(version1.name)
+ end
+
+ it "shows the new version's name" do
+ expect(subject).to match(version2.name)
+ end
+ end
+
+ describe 'attribute estimated hours' do
+ let(:estimated_hours1) { 30.5678 }
+ let(:estimated_hours2) { 35.912834 }
+
+ before do
+ allow(journal).to receive(:details).and_return('estimated_hours' => [estimated_hours1, estimated_hours2])
+ end
+
+ it 'shows the old estimated hours' do
+ expect(subject).to match('%.2f' % estimated_hours1)
+ end
+
+ it 'shows the new estimated hours' do
+ expect(subject).to match('%.2f' % estimated_hours2)
+ end
+ end
+
+ describe 'custom field' do
+ let(:expected_text) { 'original, unchanged text' }
+ let(:expected_text2) { 'modified, new text' }
+ let(:custom_field) do
+ FactoryBot.create :work_package_custom_field,
+ field_format: 'text'
+ end
+
+ before do
+ allow(journal).to receive(:details).and_return("custom_fields_#{custom_field.id}" => [expected_text, expected_text2])
+ end
+
+ it 'shows the old custom field value' do
+ expect(subject).to match(expected_text)
+ end
+
+ it 'shows the new custom field value' do
+ expect(subject).to match(expected_text2)
+ end
+ end
+ end
+
+ describe 'journal details in html mail' do
+ subject(:mail) do
+ described_class.work_packages(recipient.id, notifications.map(&:id)).body.parts[1].body.to_s.gsub("\r\n", "\n")
+ end
+
+ let(:expected_translation) do
+ I18n.t(:done_ratio, scope: %i[activerecord
+ attributes
+ work_package])
+ end
+ let(:expected_prefix) { "
#{expected_translation}" }
+
+ context 'with changed done ratio' do
+ let(:expected) do
+ "#{expected_prefix} changed from
40 to 100"
+ end
+
+ before do
+ allow(journal).to receive(:details).and_return('done_ratio' => [40, 100])
+ end
+
+ it 'displays changed done ratio' do
+ expect(subject).to include(expected)
+ end
+ end
+
+ context 'with changed subject to long value' do
+ let(:old_subject) { 'foo' }
+ let(:new_subject) { 'abcd' * 25 }
+ let(:expected) do
+ "
Subject changed from
#{old_subject} to " \
+ "
#{new_subject}"
+ end
+
+ before do
+ allow(journal).to receive(:details).and_return('subject' => [old_subject, new_subject])
+ end
+
+ it 'displays changed subject with newline' do
+ expect(subject).to include(expected)
+ end
+ end
+
+ context 'with new done ratio' do
+ let(:expected) do
+ "#{expected_prefix} changed from
0 to 100"
+ end
+
+ before do
+ allow(journal).to receive(:details).and_return('done_ratio' => [nil, 100])
+ end
+
+ it 'displays new done ratio' do
+ expect(subject).to include(expected)
+ end
+ end
+
+ context 'with deleted done ratio' do
+ let(:expected) { "#{expected_prefix} changed from
50 to 0" }
+
+ before do
+ allow(journal).to receive(:details).and_return('done_ratio' => [50, nil])
+ end
+
+ it 'displays deleted done ratio' do
+ expect(subject).to include(expected)
+ end
+ end
+
+ describe 'attachments', with_settings: { host_name: "mydomain.foo" } do
+ shared_let(:attachment) { FactoryBot.create(:attachment) }
+ let(:journal) do
+ FactoryBot.build_stubbed(:work_package_journal)
+ end
+
+ context 'when added' do # rubocop:disable Rspec/NestedGroups
+ before do
+ allow(journal).to receive(:details).and_return("attachments_#{attachment.id}" => [nil, attachment.filename])
+ end
+
+ it "shows the attachment's filename" do
+ expect(subject).to include(attachment.filename)
+ end
+
+ it "links correctly" do
+ expect(subject).to include("
")
+ end
+
+ context 'with a suburl', with_config: { rails_relative_url_root: '/rdm' } do # rubocop:disable Rspec/NestedGroups
+ it "links correctly" do
+ expect(subject).to include("")
+ end
+ end
+
+ it "shows status 'added'" do
+ expect(subject).to include('added')
+ end
+
+ it "shows no status 'deleted'" do
+ expect(subject).not_to include('deleted')
+ end
+ end
+
+ context 'when removed' do # rubocop:disable Rspec/NestedGroups
+ before do
+ allow(journal).to receive(:details).and_return("attachments_#{attachment.id}" => [attachment.filename, nil])
+ end
+
+ it "shows the attachment's filename" do
+ expect(subject).to include(attachment.filename)
+ end
+
+ it "shows no status 'added'" do
+ expect(subject).not_to include('added')
+ end
+
+ it "shows status 'deleted'" do
+ expect(subject).to include('deleted')
+ end
+ end
+ end
+ end
end
end
diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb
index 685f7e72b9..dc0436da96 100644
--- a/spec/mailers/user_mailer_spec.rb
+++ b/spec/mailers/user_mailer_spec.rb
@@ -156,101 +156,6 @@ describe UserMailer, type: :mailer do
end
end
- describe '#work_package_added' do
- before do
- described_class.work_package_added(recipient, journal, user).deliver_now
- end
-
- it_behaves_like 'mail is sent' do
- it 'contains the WP subject in the mail subject' do
- expect(deliveries.first.subject)
- .to include(work_package.subject)
- end
-
- it 'has the desired "Precedence" header' do
- expect(deliveries.first['Precedence'].value)
- .to eql 'bulk'
- end
-
- it 'has the desired "Auto-Submitted" header' do
- expect(deliveries.first['Auto-Submitted'].value)
- .to eql 'auto-generated'
- end
-
- it 'carries a message_id' do
- expect(deliveries.first.message_id)
- .to eql(described_class.generate_message_id(journal, recipient))
- end
-
- it 'does not reference' do
- expect(deliveries.first.references)
- .to be_nil
- end
-
- context 'with plain_text_mail active', with_settings: { plain_text_mail: 1 } do
- it 'only sends plain text' do
- expect(mail.content_type)
- .to match /text\/plain/
- end
- end
-
- context 'with plain_text_mail inactive', with_settings: { plain_text_mail: 0 } do
- it 'sends a multipart mail' do
- expect(mail.content_type)
- .to match /multipart\/alternative/
- end
- end
- end
-
- it_behaves_like 'does not send mails to author'
- end
-
- describe '#work_package_updated' do
- before do
- described_class.work_package_updated(recipient, journal, user).deliver_now
- end
-
- it_behaves_like 'mail is sent' do
- it 'carries a message_id' do
- expect(deliveries.first.message_id)
- .to eql(described_class.generate_message_id(journal, recipient))
- end
-
- it 'references the message_id' do
- expect(deliveries.first.references)
- .to eql described_class.generate_message_id(journal, recipient)
- end
-
- context 'with a link' do
- let(:work_package) do
- FactoryBot.build_stubbed(:work_package,
- type: type_standard,
- description: "Some text with a reference to ##{referenced_wp.id}")
- end
-
- let(:referenced_wp) do
- FactoryBot.build_stubbed(:work_package)
- end
-
- it 'renders the link' do
- expect(html_body)
- .to have_link("##{referenced_wp.id}", href: work_package_url(referenced_wp, host: Setting.host_name))
- end
-
- context 'with a relative url root',
- with_config: { rails_relative_url_root: '/subpath' } do
- it 'renders the link' do
- expect(html_body)
- .to have_link("##{referenced_wp.id}",
- href: work_package_url(referenced_wp, host: Setting.host_name, script_name: '/subpath'))
- end
- end
- end
- end
-
- it_behaves_like 'does not send mails to author'
- end
-
describe '#work_package_watcher_changed' do
let(:watcher_changer) { user }
@@ -412,18 +317,27 @@ describe UserMailer, type: :mailer do
end
describe '#message_id' do
- describe 'same user' do
- let(:journal2) { FactoryBot.build_stubbed(:work_package_journal) }
-
- before do
- allow(journal2).to receive(:journable).and_return(work_package)
- allow(journal2).to receive(:user).and_return(user)
- allow(journal2).to receive(:created_at).and_return(journal.created_at + 5.seconds)
+ let(:project) { FactoryBot.build_stubbed(:project) }
+ let(:message) do
+ FactoryBot.build_stubbed(:message).tap do |m|
+ allow(m)
+ .to receive(:project)
+ .and_return project
end
+ end
+ let(:message2) do
+ FactoryBot.build_stubbed(:message).tap do |m|
+ allow(m)
+ .to receive(:project)
+ .and_return project
+ end
+ end
+ let(:author) { FactoryBot.build_stubbed(:user) }
+ describe 'same user' do
subject do
- message_ids = [journal, journal2].each_with_object([]) do |j, l|
- l << described_class.work_package_updated(user, j).message_id
+ message_ids = [message, message2].map do |m|
+ described_class.message_posted(user, m, author).message_id
end
message_ids.uniq.count
@@ -435,13 +349,9 @@ describe UserMailer, type: :mailer do
describe 'same timestamp' do
let(:user2) { FactoryBot.build_stubbed(:user) }
- before do
- allow(work_package).to receive(:recipients).and_return([user, user2])
- end
-
subject do
- message_ids = [user, user2].each_with_object([]) do |u, l|
- l << described_class.work_package_updated(u, journal).message_id
+ message_ids = [user, user2].map do |user|
+ described_class.message_posted(user, message, author).message_id
end
message_ids.uniq.count
@@ -451,351 +361,6 @@ describe UserMailer, type: :mailer do
end
end
- describe 'journal details' do
- subject { described_class.work_package_updated(user, journal).body.encoded.gsub("\r\n", "\n") }
-
- describe 'plain text mail' do
- before do
- allow(Setting).to receive(:plain_text_mail).and_return('1')
- end
-
- context 'with changed done ratio' do
- before do
- allow(journal).to receive(:details).and_return('done_ratio' => [40, 100])
- end
-
- it 'displays changed done ratio' do
- expect(subject).to include("Progress (%) changed from 40 to 100")
- end
- end
-
- context 'with new done ratio' do
- before do
- allow(journal).to receive(:details).and_return('done_ratio' => [nil, 100])
- end
-
- it 'displays new done ratio' do
- expect(subject).to include("Progress (%) changed from 0 to 100")
- end
- end
-
- context 'with deleted done ratio' do
- before do
- allow(journal).to receive(:details).and_return('done_ratio' => [50, nil])
- end
-
- it 'displays deleted done ratio' do
- expect(subject).to include("Progress (%) changed from 50 to 0")
- end
- end
-
- describe 'start_date attribute' do
- before do
- allow(journal).to receive(:details).and_return('start_date' => %w[2010-01-01 2010-01-31])
- end
-
- it 'old date should be formatted' do
- expect(subject).to match('01/01/2010')
- end
-
- it 'new date should be formatted' do
- expect(subject).to match('01/31/2010')
- end
- end
-
- describe 'due_date attribute' do
- before do
- allow(journal).to receive(:details).and_return('due_date' => %w[2010-01-01 2010-01-31])
- end
-
- it 'old date should be formatted' do
- expect(subject).to match('01/01/2010')
- end
-
- it 'new date should be formatted' do
- expect(subject).to match('01/31/2010')
- end
- end
-
- describe 'project attribute' do
- let(:project1) { FactoryBot.create(:project) }
- let(:project2) { FactoryBot.create(:project) }
-
- before do
- allow(journal).to receive(:details).and_return('project_id' => [project1.id, project2.id])
- end
-
- it "shows the old project's name" do
- expect(subject).to match(project1.name)
- end
-
- it "shows the new project's name" do
- expect(subject).to match(project2.name)
- end
- end
-
- describe 'attribute issue status' do
- let(:status1) { FactoryBot.create(:status) }
- let(:status2) { FactoryBot.create(:status) }
-
- before do
- allow(journal).to receive(:details).and_return('status_id' => [status1.id, status2.id])
- end
-
- it "shows the old status' name" do
- expect(subject).to match(status1.name)
- end
-
- it "shows the new status' name" do
- expect(subject).to match(status2.name)
- end
- end
-
- describe 'attribute type' do
- let(:type1) { FactoryBot.create(:type_standard) }
- let(:type2) { FactoryBot.create(:type_bug) }
-
- before do
- allow(journal).to receive(:details).and_return('type_id' => [type1.id, type2.id])
- end
-
- it "shows the old type's name" do
- expect(subject).to match(type1.name)
- end
-
- it "shows the new type's name" do
- expect(subject).to match(type2.name)
- end
- end
-
- describe 'attribute assigned to' do
- let(:assignee1) { FactoryBot.create(:user) }
- let(:assignee2) { FactoryBot.create(:user) }
-
- before do
- allow(journal).to receive(:details).and_return('assigned_to_id' => [assignee1.id, assignee2.id])
- end
-
- it "shows the old assignee's name" do
- expect(subject).to match(assignee1.name)
- end
-
- it "shows the new assignee's name" do
- expect(subject).to match(assignee2.name)
- end
- end
-
- describe 'attribute priority' do
- let(:priority1) { FactoryBot.create(:priority) }
- let(:priority2) { FactoryBot.create(:priority) }
-
- before do
- allow(journal).to receive(:details).and_return('priority_id' => [priority1.id, priority2.id])
- end
-
- it "shows the old priority's name" do
- expect(subject).to match(priority1.name)
- end
-
- it "shows the new priority's name" do
- expect(subject).to match(priority2.name)
- end
- end
-
- describe 'attribute category' do
- let(:category1) { FactoryBot.create(:category) }
- let(:category2) { FactoryBot.create(:category) }
-
- before do
- allow(journal).to receive(:details).and_return('category_id' => [category1.id, category2.id])
- end
-
- it "shows the old category's name" do
- expect(subject).to match(category1.name)
- end
-
- it "shows the new category's name" do
- expect(subject).to match(category2.name)
- end
- end
-
- describe 'attribute version' do
- let(:version1) { FactoryBot.create(:version) }
- let(:version2) { FactoryBot.create(:version) }
-
- before do
- allow(journal).to receive(:details).and_return('version_id' => [version1.id, version2.id])
- end
-
- it "shows the old version's name" do
- expect(subject).to match(version1.name)
- end
-
- it "shows the new version's name" do
- expect(subject).to match(version2.name)
- end
- end
-
- describe 'attribute estimated hours' do
- let(:estimated_hours1) { 30.5678 }
- let(:estimated_hours2) { 35.912834 }
-
- before do
- allow(journal).to receive(:details).and_return('estimated_hours' => [estimated_hours1, estimated_hours2])
- end
-
- it 'shows the old estimated hours' do
- expect(subject).to match('%.2f' % estimated_hours1)
- end
-
- it 'shows the new estimated hours' do
- expect(subject).to match('%.2f' % estimated_hours2)
- end
- end
-
- describe 'custom field' do
- let(:expected_text) { 'original, unchanged text' }
- let(:expected_text2) { 'modified, new text' }
- let(:custom_field) do
- FactoryBot.create :work_package_custom_field,
- field_format: 'text'
- end
-
- before do
- allow(journal).to receive(:details).and_return("custom_fields_#{custom_field.id}" => [expected_text, expected_text2])
- end
-
- it 'shows the old custom field value' do
- expect(subject).to match(expected_text)
- end
-
- it 'shows the new custom field value' do
- expect(subject).to match(expected_text2)
- end
- end
-
- describe 'attachments' do
- shared_let(:attachment) { FactoryBot.create(:attachment) }
-
- context 'when added' do # rubocop:disable Rspec/NestedGroups
- before do
- allow(journal).to receive(:details).and_return("attachments_#{attachment.id}" => [nil, attachment.filename])
- end
-
- it "shows the attachment's filename" do
- expect(subject).to match(attachment.filename)
- end
-
- it "links correctly" do
- expect(subject).to match("")
- end
-
- context 'with a suburl', with_config: { rails_relative_url_root: '/rdm' } do # rubocop:disable Rspec/NestedGroups
- it "links correctly" do
- expect(subject).to match("")
- end
- end
-
- it "shows status 'added'" do
- expect(subject).to match('added')
- end
-
- it "shows no status 'deleted'" do
- expect(subject).not_to match('deleted')
- end
- end
-
- context 'when removed' do # rubocop:disable Rspec/NestedGroups
- before do
- allow(journal).to receive(:details).and_return("attachments_#{attachment.id}" => [attachment.filename, nil])
- end
-
- it "shows the attachment's filename" do
- expect(subject).to match(attachment.filename)
- end
-
- it "shows no status 'added'" do
- expect(subject).not_to match('added')
- end
-
- it "shows status 'deleted'" do
- expect(subject).to match('deleted')
- end
- end
- end
- end
-
- describe 'html mail' do
- let(:expected_translation) do
- I18n.t(:done_ratio, scope: %i[activerecord
- attributes
- work_package])
- end
- let(:expected_prefix) { "#{expected_translation}" }
-
- before do
- allow(Setting).to receive(:plain_text_mail).and_return('0')
- end
-
- context 'with changed done ratio' do
- let(:expected) do
- "#{expected_prefix} changed from 40 to 100"
- end
-
- before do
- allow(journal).to receive(:details).and_return('done_ratio' => [40, 100])
- end
-
- it 'displays changed done ratio' do
- expect(subject).to include(expected)
- end
- end
-
- context 'with changed subject to long value' do
- let(:old_subject) { 'foo' }
- let(:new_subject) { 'abcd' * 25 }
- let(:expected) do
- "Subject changed from #{old_subject}
to " \
- "#{new_subject}"
- end
-
- before do
- allow(journal).to receive(:details).and_return('subject' => [old_subject, new_subject])
- end
-
- it 'displays changed subject with newline' do
- expect(subject).to include(expected)
- end
- end
-
- context 'with new done ratio' do
- let(:expected) do
- "#{expected_prefix} changed from 0 to 100"
- end
-
- before do
- allow(journal).to receive(:details).and_return('done_ratio' => [nil, 100])
- end
-
- it 'displays new done ratio' do
- expect(subject).to include(expected)
- end
- end
-
- context 'with deleted done ratio' do
- let(:expected) { "#{expected_prefix} changed from 50 to 0" }
-
- before do
- allow(journal).to receive(:details).and_return('done_ratio' => [50, nil])
- end
-
- it 'displays deleted done ratio' do
- expect(subject).to include(expected)
- end
- end
- end
- end
-
describe 'localization' do
context 'with the user having a language configured',
with_settings: { available_languages: %w[en de],
diff --git a/spec/models/mail_handler_spec.rb b/spec/models/mail_handler_spec.rb
index 2cfe9a52cd..71ec4fd4d0 100644
--- a/spec/models/mail_handler_spec.rb
+++ b/spec/models/mail_handler_spec.rb
@@ -245,36 +245,15 @@ describe MailHandler, type: :model do
.not_to match(/^Start Date:/i)
end
- it 'sends notifications' do
- FactoryBot.create(:user, member_in_project: project, member_with_permissions: %i(view_work_packages))
+ it 'sends notifications to watching users' do
+ # User gets all updates
+ user = FactoryBot.create(:user, member_in_project: project, member_with_permissions: %i(view_work_packages))
expect do
perform_enqueued_jobs do
subject
end
- end.to change(ActionMailer::Base.deliveries, :count).by(1)
- end
-
- context 'with a user watching every creation' do
- let!(:other_user) do
- FactoryBot.create(:user,
- member_in_project: project,
- member_with_permissions: %i[view_work_packages])
- end
-
- it 'sends a mail as a work package has been created' do
- perform_enqueued_jobs do
- subject
- end
-
- # Email notification should be sent
- mail = ActionMailer::Base.deliveries.last
-
- expect(mail)
- .not_to be_nil
- expect(mail.subject)
- .to include('New ticket on a given project')
- end
+ end.to change(Notification.where(recipient: user), :count).by(1)
end
end
@@ -486,7 +465,7 @@ describe MailHandler, type: :model do
perform_enqueued_jobs do
subject
end
- end.to change(ActionMailer::Base.deliveries, :count).by(2)
+ end.to change(Notification, :count).by(2)
end
end
diff --git a/spec/models/work_package/work_package_notifications_spec.rb b/spec/models/work_package/work_package_notifications_spec.rb
deleted file mode 100644
index cc1f1d36ff..0000000000
--- a/spec/models/work_package/work_package_notifications_spec.rb
+++ /dev/null
@@ -1,99 +0,0 @@
-#-- encoding: UTF-8
-
-#-- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) 2012-2021 the OpenProject GmbH
-#
-# 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-2013 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 COPYRIGHT and LICENSE files for more details.
-#++
-
-require 'spec_helper'
-
-##
-# Tests that email notifications will be sent upon creating or changing a work package.
-describe WorkPackage, type: :model do
- describe 'email notifications' do
- let(:user) { FactoryBot.create(:admin) }
- let(:current_user) { FactoryBot.create :admin }
- let(:project) { FactoryBot.create :project }
- let(:work_package) do
- FactoryBot.create :work_package,
- author: user,
- subject: 'I can see you',
- project: project
- end
-
- context 'after creation' do
- it "are sent to the work package's author" do
- perform_enqueued_jobs do
- work_package
- end
-
- mail = ActionMailer::Base.deliveries.detect { |m| m.subject.include? 'I can see you' }
-
- expect(mail).to be_present
- end
-
- context 'with email notifications disabled' do
- let(:user) do
- notification_settings = [
- FactoryBot.build(:mail_notification_setting, mentioned: false, involved: false, watched: false, all: false),
- FactoryBot.build(:in_app_notification_setting, mentioned: false, involved: false, watched: false, all: false)
- ]
-
- FactoryBot.create :admin,
- notification_settings: notification_settings
- end
-
- let(:project) do
- FactoryBot.create :project, members: { user => [FactoryBot.create(:role)] }
- end
-
- it "are not sent to the work package's author" do
- perform_enqueued_jobs do
- work_package
- end
-
- mail = ActionMailer::Base.deliveries.detect { |m| m.subject.include? 'I can see you' }
-
- expect(mail).not_to be_present
- end
- end
- end
-
- describe 'after update' do
- before do
- perform_enqueued_jobs do
- work_package.update subject: 'the wind of change'
- end
- end
-
- it "are sent to the work package's author" do
- mail = ActionMailer::Base.deliveries.detect { |m| m.subject.include? 'the wind of change' }
-
- expect(mail).to be_present
- end
- end
- end
-end
diff --git a/spec/requests/api/v3/work_packages/create_resource_spec.rb b/spec/requests/api/v3/work_packages/create_resource_spec.rb
index c8c64132ea..1334101063 100644
--- a/spec/requests/api/v3/work_packages/create_resource_spec.rb
+++ b/spec/requests/api/v3/work_packages/create_resource_spec.rb
@@ -77,26 +77,26 @@ describe 'API v3 Work package resource',
describe 'notifications' do
let(:other_user) { FactoryBot.create(:user, member_in_project: project, member_with_permissions: permissions) }
- it 'sends a mail by default' do
- expect(ActionMailer::Base.deliveries.size)
- .to be 1
+ it 'creates a notification' do
+ expect(Notification.where(recipient: other_user, resource: WorkPackage.last))
+ .to exist
end
context 'without notifications' do
let(:path) { "#{api_v3_paths.work_packages}?notify=false" }
- it 'sends no mail' do
- expect(ActionMailer::Base.deliveries.size)
- .to be 0
+ it 'creates no notification' do
+ expect(Notification)
+ .not_to exist
end
end
context 'with notifications' do
let(:path) { "#{api_v3_paths.work_packages}?notify=true" }
- it 'sends a mail' do
- expect(ActionMailer::Base.deliveries.size)
- .to be 1
+ it 'creates a notification' do
+ expect(Notification.where(recipient: other_user, resource: WorkPackage.last))
+ .to exist
end
end
end
diff --git a/spec/requests/api/v3/work_packages/update_resource_spec.rb b/spec/requests/api/v3/work_packages/update_resource_spec.rb
index d3dc2364ec..cdbdaabef1 100644
--- a/spec/requests/api/v3/work_packages/update_resource_spec.rb
+++ b/spec/requests/api/v3/work_packages/update_resource_spec.rb
@@ -124,9 +124,9 @@ describe 'API v3 Work package resource',
context 'without the parameter' do
let(:params) { update_params }
- it 'sends a mail' do
- expect(ActionMailer::Base.deliveries.length)
- .to eq 1
+ it 'creates a notification' do
+ expect(Notification.where(recipient: other_user, resource: work_package))
+ .to exist
end
end
@@ -134,9 +134,9 @@ describe 'API v3 Work package resource',
let(:patch_path) { "#{api_v3_paths.work_package work_package.id}?notify=false" }
let(:params) { update_params }
- it 'sends no mail' do
- expect(ActionMailer::Base.deliveries)
- .to be_empty
+ it 'creates no notification' do
+ expect(Notification)
+ .not_to exist
end
end
@@ -144,9 +144,9 @@ describe 'API v3 Work package resource',
let(:patch_path) { "#{api_v3_paths.work_package work_package.id}?notify=Something" }
let(:params) { update_params }
- it 'sends a mail' do
- expect(ActionMailer::Base.deliveries.length)
- .to eq 1
+ it 'creates a notification' do
+ expect(Notification.where(recipient: other_user, resource: work_package))
+ .to exist
end
end
end
diff --git a/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb b/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb
index 3585ba1468..2aa41ee668 100644
--- a/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb
+++ b/spec/requests/api/v3/work_packages/work_packages_by_project_resource_spec.rb
@@ -319,26 +319,26 @@ describe API::V3::WorkPackages::WorkPackagesByProjectAPI, type: :request, conten
describe 'notifications' do
let(:other_user) { FactoryBot.create(:user, member_in_project: project, member_with_permissions: %i(view_work_packages)) }
- it 'sends a mail by default' do
- expect(ActionMailer::Base.deliveries.length)
- .to be 1
+ it 'creates a notification' do
+ expect(Notification.where(recipient: other_user, resource: WorkPackage.last))
+ .to exist
end
context 'without notifications' do
let(:path) { "#{api_v3_paths.work_packages_by_project(project.id)}?notify=false" }
- it 'sends no mail' do
- expect(ActionMailer::Base.deliveries)
- .to be_empty
+ it 'creates no notification' do
+ expect(Notification)
+ .not_to exist
end
end
context 'with notifications' do
let(:path) { "#{api_v3_paths.work_packages_by_project(project.id)}?notify=true" }
- it 'sends a mail' do
- expect(ActionMailer::Base.deliveries.length)
- .to be 1
+ it 'creates a notification' do
+ expect(Notification.where(recipient: other_user, resource: WorkPackage.last))
+ .to exist
end
end
end
diff --git a/spec/services/notifications/mail_service_spec.rb b/spec/services/notifications/mail_service_spec.rb
index 72804296f7..900116173b 100644
--- a/spec/services/notifications/mail_service_spec.rb
+++ b/spec/services/notifications/mail_service_spec.rb
@@ -41,93 +41,6 @@ describe Notifications::MailService, type: :model do
end
let(:instance) { described_class.new(notification) }
- context 'with a work package journal notification' do
- let(:journal) do
- FactoryBot.build_stubbed(:work_package_journal).tap do |j|
- allow(j)
- .to receive(:initial?)
- .and_return(journal_initial)
- end
- end
- let(:read_ian) { false }
- let(:notification) do
- FactoryBot.build_stubbed(:notification,
- journal: journal,
- recipient: recipient,
- actor: actor,
- read_ian: read_ian)
- end
- let(:journal_initial) { false }
-
- let(:mail) do
- mail = instance_double(ActionMailer::MessageDelivery)
-
- allow(UserMailer)
- .to receive(:work_package_added)
- .and_return(mail)
-
- allow(UserMailer)
- .to receive(:work_package_updated)
- .and_return(mail)
-
- allow(mail)
- .to receive(:deliver_later)
-
- mail
- end
-
- before do
- mail
- end
-
- context 'with the notification being for an initial journal' do
- let(:journal_initial) { true }
-
- it 'sends a mail' do
- call
-
- expect(UserMailer)
- .to have_received(:work_package_added)
- .with(recipient,
- journal,
- journal.user)
-
- expect(mail)
- .to have_received(:deliver_later)
- end
- end
-
- context 'with the notification being for an update journal' do
- let(:journal_initial) { false }
-
- it 'sends a mail' do
- call
-
- expect(UserMailer)
- .to have_received(:work_package_updated)
- .with(recipient,
- journal,
- journal.user)
-
- expect(mail)
- .to have_received(:deliver_later)
- end
- end
-
- context 'with the notification read in app already' do
- let(:read_ian) { true }
-
- it 'sends no mail' do
- call
-
- expect(UserMailer)
- .not_to have_received(:work_package_added)
- expect(UserMailer)
- .not_to have_received(:work_package_updated)
- end
- end
- end
-
context 'with a wiki_content journal notification' do
let(:journal) do
FactoryBot.build_stubbed(:wiki_content_journal,
@@ -392,10 +305,9 @@ describe Notifications::MailService, type: :model do
end
context 'with a different journal notification' do
- # This is actually not supported by now but serves as a test
let(:journal) do
FactoryBot.build_stubbed(:journal,
- journable: FactoryBot.build_stubbed(:user))
+ journable: FactoryBot.build_stubbed(:work_package))
end
let(:notification) do
FactoryBot.build_stubbed(:notification,
@@ -404,9 +316,15 @@ describe Notifications::MailService, type: :model do
actor: actor)
end
- it 'raises an error' do
+ # did that before
+ it 'does nothing' do
+ expect { call }
+ .not_to raise_error(ArgumentError)
+ end
+
+ it 'does not send a mail' do
expect { call }
- .to raise_error(ArgumentError)
+ .not_to change(ActionMailer::Base.deliveries, :count)
end
end
end
diff --git a/spec/workers/notifications/workflow_job_spec.rb b/spec/workers/notifications/workflow_job_spec.rb
index 8a8a2e1cc9..81bc37a9de 100644
--- a/spec/workers/notifications/workflow_job_spec.rb
+++ b/spec/workers/notifications/workflow_job_spec.rb
@@ -83,7 +83,6 @@ describe Notifications::WorkflowJob, type: :model do
.and_return(Time.current)
expected_time = Time.current +
- Setting.notification_email_delay_minutes.minutes +
Setting.journal_aggregation_time_minutes.to_i.minutes
expect { perform_job }