From a374212d8c28be3d6ba1d9cebb3647b5ba6063e3 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Thu, 26 Aug 2021 13:43:24 +0200 Subject: [PATCH 01/23] Add basic layout for showing notification rows in the daily email --- app/helpers/mail_digest_helper.rb | 8 +- app/mailers/digest_mailer.rb | 5 +- .../digest_mailer/work_packages.html.erb | 103 +++++++++++++++--- app/views/layouts/mailer.html.erb | 7 -- config/locales/en.yml | 4 +- 5 files changed, 98 insertions(+), 29 deletions(-) diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index bd3e9411f9..3bfaed2df5 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -43,6 +43,12 @@ module MailDigestHelper raw(I18n.t(:"mail.digests.work_packages.#{journal.initial? ? 'created_at' : 'updated_at'}", user: user, - timestamp: format_time(journal.created_at))) + timestamp: time_ago_in_words(journal.created_at))) + end + + def unique_reasons_of_notifications(notifications) + notifications + .map { |notification| notification.reason_mail_digest } + .uniq end end diff --git a/app/mailers/digest_mailer.rb b/app/mailers/digest_mailer.rb index 91c9e6973b..9a6fae4b42 100644 --- a/app/mailers/digest_mailer.rb +++ b/app/mailers/digest_mailer.rb @@ -54,11 +54,14 @@ class DigestMailer < ApplicationMailer open_project_headers User: recipient.name message_id nil, recipient + @aggregated_notifications = load_notifications(notification_ids) + .group_by(&:resource) + @notifications_by_project = load_notifications(notification_ids) .group_by(&:project) .transform_values { |of_project| of_project.group_by(&:resource) } - return if @notifications_by_project.empty? + return if @aggregated_notifications.empty? with_locale_for(recipient) do mail to: recipient.mail, diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 4eb1cd7b8d..70ec764cdd 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -1,25 +1,91 @@ +<% @aggregated_notifications.each do | work_package, notifications_by_work_package| %> +
+ + + + + + + + +
+ <%= work_package.status %> + + #<%= work_package.id %> + + - <%= work_package.project %> - + + <% unique_reasons = unique_reasons_of_notifications(notifications_by_work_package) %> + <% unique_reasons.each_with_index do |reason, index| %> + <%= I18n.t( + :"mail.digests.work_packages.reason.#{reason || :unknown}", + default: '-') %><%= ', ' unless unique_reasons.size-1 == index %> + <% end %> + + + <%= notifications_by_work_package.length %> + +
+ + + + + +
+ <%= work_package.type.to_s.upcase %> + + <%= work_package.subject %> +
+ + <% notifications_by_work_package.each do | notification | %> + <% notification.journal.details.each do |detail| %> + + + + <% end %> + <% end %> +
+ <%= notification.journal.render_detail(detail, only_path: false) %> + <%= digest_notification_timestamp_text(notification) %> +
+
+<% end %> + diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index 878021f2af..117a987cf1 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -40,13 +40,6 @@ See COPYRIGHT and LICENSE files for more details. color: #484848; background: #FFFFFF; } - - @media (prefers-color-scheme: dark) { - body { - color: #CCC !important; - background: #222222 !important; - } - } h1, h2, h3 { font-family: "Trebuchet MS", Verdana, sans-serif; margin: 0px; } h1 { font-size: 1.2em; } diff --git a/config/locales/en.yml b/config/locales/en.yml index 21af890dcd..a631cb103f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1969,7 +1969,7 @@ en: digests: time_frame: 'Summary of all events you subscribed to in the period between %{start} and %{end}' work_packages: - created_at: '%{user} created at %{timestamp}' + created_at: 'by %{user} %{timestamp} ago' reason: watched: 'watched' involved: 'assignee or responsible' @@ -1977,7 +1977,7 @@ en: subscribed: 'all' prefix: 'Received because of the notification setting: %{reason}' subject: 'Daily project summary for %{date} - %{number} updates' - updated_at: '%{user} updated at %{timestamp}' + updated_at: '%{timestamp} ago by %{user}' mail_body_account_activation_request: "A new user (%{value}) has registered. The account is pending your approval:" mail_body_account_information: "Your account information" From 70b23cabd6d610b9158632ee718076c09e952bee Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Fri, 27 Aug 2021 09:00:58 +0200 Subject: [PATCH 02/23] Link to notification center from the email notification --- app/helpers/mail_digest_helper.rb | 4 ++++ .../digest_mailer/work_packages.html.erb | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index 3bfaed2df5..f9db3cb487 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -51,4 +51,8 @@ module MailDigestHelper .map { |notification| notification.reason_mail_digest } .uniq end + + def notifications_path(id) + notifications_center_url(['details', id, 'activity']) + end end diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 70ec764cdd..8948c0e798 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -1,6 +1,13 @@ <% @aggregated_notifications.each do | work_package, notifications_by_work_package| %> -
- + +
+ -
<%= work_package.type.to_s.upcase %> + <%= work_package.subject %>
- +
<% notifications_by_work_package.each do | notification | %> <% notification.journal.details.each do |detail| %> @@ -59,7 +66,7 @@ <% end %> <% end %>
- + <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 05295a4725..6a1ee72bbf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1967,7 +1967,9 @@ en: mail: actions: 'Actions' digests: - time_frame: 'Summary of all events you subscribed to in the period between %{start} and %{end}' + center: 'Notification center' + time_frame: "At %{time} on %{weekday}, %{date}, you have %{number_unread} unread notifications in %{number_work_packages} work packages" + salutation: 'Hey %{user}!' work_packages: created: 'Created' created_at: 'by %{user} %{timestamp} ago' From 1310247689a90ac39dbc180cac03132a8286406e Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Mon, 30 Aug 2021 08:17:07 +0200 Subject: [PATCH 06/23] Add button to change notification settings in email --- app/views/digest_mailer/work_packages.html.erb | 16 +++++++++++++++- config/locales/en.yml | 3 ++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 2e43746060..85b95cbbc4 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -23,7 +23,7 @@ <% @aggregated_notifications.each do | work_package, notifications_by_work_package| %> <% end %> + + + + + + +
+ + <%= I18n.t(:'mail.digests.settings') %> + +
+ diff --git a/config/locales/en.yml b/config/locales/en.yml index 6a1ee72bbf..701fc7e0b9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1968,8 +1968,9 @@ en: actions: 'Actions' digests: center: 'Notification center' - time_frame: "At %{time} on %{weekday}, %{date}, you have %{number_unread} unread notifications in %{number_work_packages} work packages" salutation: 'Hey %{user}!' + settings: 'Change email settings' + time_frame: "At %{time} on %{weekday}, %{date}, you have %{number_unread} unread notifications in %{number_work_packages} work packages" work_packages: created: 'Created' created_at: 'by %{user} %{timestamp} ago' From 2a69c19f6b6dfcbb23bae1c8ae7134f7421a5f5b Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Mon, 30 Aug 2021 09:10:20 +0200 Subject: [PATCH 07/23] Limit the number of shown WPs in the daily email --- app/helpers/mail_digest_helper.rb | 4 ++-- app/mailers/digest_mailer.rb | 2 ++ .../digest_mailer/work_packages.html.erb | 20 +++++++++++++++++-- .../digest_mailer/work_packages.text.erb | 17 +++++++++++++--- config/locales/en.yml | 4 ++++ 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index 02356bf4e8..bbf1691c81 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -76,11 +76,11 @@ module MailDigestHelper if extended raw(I18n.t(:"mail.digests.work_packages.#{value}") + ' ' + - I18n.t(:"mail.digests.work_packages.#{value}", + I18n.t(:"mail.digests.work_packages.#{value}_at", user: user, timestamp: time_ago_in_words(journal.created_at))) else - raw(I18n.t(:"mail.digests.work_packages.#{value}", + raw(I18n.t(:"mail.digests.work_packages.#{value}_at", user: user, timestamp: time_ago_in_words(journal.created_at))) end diff --git a/app/mailers/digest_mailer.rb b/app/mailers/digest_mailer.rb index 8064f23736..7a585f9452 100644 --- a/app/mailers/digest_mailer.rb +++ b/app/mailers/digest_mailer.rb @@ -39,6 +39,8 @@ class DigestMailer < ApplicationMailer helper :mail_digest + MAX_SHOWN_WORK_PACKAGES = 15.freeze + class << self def generate_message_id(_, user) hash = "openproject.digest-#{user.id}-#{Time.current.strftime('%Y%m%d%H%M%S')}" diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 85b95cbbc4..2594dbb5ae 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -20,7 +20,7 @@
-<% @aggregated_notifications.each do | work_package, notifications_by_work_package| %> +<% @aggregated_notifications.first(DigestMailer::MAX_SHOWN_WORK_PACKAGES).each do | work_package, notifications_by_work_package| %> - + + <% if @aggregated_notifications.length > DigestMailer::MAX_SHOWN_WORK_PACKAGES %> + + <% number_of_overflowing_work_packages = @aggregated_notifications.length - DigestMailer::MAX_SHOWN_WORK_PACKAGES %> + <% if number_of_overflowing_work_packages === 1 %> + <%= I18n.t(:'mail.digests.work_packages.more_to_see_singular') %> + <% else %> + <%= I18n.t(:'mail.digests.work_packages.more_to_see_plural', number: number_of_overflowing_work_packages) %> + <% end %> + + + <%= I18n.t(:'mail.digests.work_packages.see_all') %> + + <% end %> + +<% @aggregated_notifications.first(DigestMailer::MAX_SHOWN_WORK_PACKAGES).each do | work_package, notifications_by_work_package| %> <%= "=" * (('# ' + work_package.id.to_s + work_package.subject).length + 4) %> = #<%= work_package.id %> <%= work_package.subject %> = @@ -7,11 +7,11 @@ <% notifications_by_work_package.each do | notification | %> <%= "-" * 20 %> + <% unique_reasons = unique_reasons_of_notifications(notifications_by_work_package) %> <%= digest_notification_timestamp_text( notification, html: false, - extended_text: true) %> (<%= I18n.t('mail.digests.work_packages.reason.prefix', - reason: I18n.t(:"mail.digests.work_packages.reason.#{notification.reason_mail_digest}")) %>) + extended_text: true) %> (<% unique_reasons.each_with_index do |reason, index| %><%= I18n.t(:"mail.digests.work_packages.reason.#{reason || :unknown}", default: '-') %><%= ', ' unless unique_reasons.size-1 == index %><% end %>) <% journal = notification.journal %> <% if journal.notes.present? %> @@ -25,3 +25,14 @@ <%= "-" * 20 %> <% end %> <% end %> + +<%= "-" * 100 %> + +<% if @aggregated_notifications.length > DigestMailer::MAX_SHOWN_WORK_PACKAGES %> +<% number_of_overflowing_work_packages = @aggregated_notifications.length - DigestMailer::MAX_SHOWN_WORK_PACKAGES %> +<% if number_of_overflowing_work_packages === 1 %> +<%= I18n.t(:'mail.digests.work_packages.more_to_see_singular') %> <%= I18n.t(:'mail.digests.work_packages.login_to_see_all') %> +<% else %> +<%= I18n.t(:'mail.digests.work_packages.more_to_see_plural', number: number_of_overflowing_work_packages) %> <%= I18n.t(:'mail.digests.work_packages.login_to_see_all') %> +<% end %> +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 701fc7e0b9..7baca0c65c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1974,12 +1974,16 @@ en: work_packages: created: 'Created' created_at: 'by %{user} %{timestamp} ago' + login_to_see_all: 'Log in to see all notifications.' + more_to_see_singular: 'There is 1 more work package with notifications.' + more_to_see_plural: 'There are %{number} more work packages with notifications.' reason: watched: 'watched' involved: 'assignee or responsible' mentioned: 'mentioned' subscribed: 'all' prefix: 'Received because of the notification setting: %{reason}' + see_all: 'See all' subject: 'Daily project summary for %{date} - %{number} updates' updated: 'Updated' updated_at: '%{timestamp} ago by %{user}' From 80253c71ea7bcb5d881db392dfef490f6ff394eb Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Mon, 30 Aug 2021 11:33:18 +0200 Subject: [PATCH 08/23] Show comment in notification row in daily email --- app/views/digest_mailer/work_packages.html.erb | 8 ++++++++ app/views/digest_mailer/work_packages.text.erb | 1 + config/locales/en.yml | 1 + 3 files changed, 10 insertions(+) diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 2594dbb5ae..3efed52cbe 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -78,6 +78,14 @@ <% notifications_by_work_package.each do | notification | %> + <% if notification.journal.notes.present? %> + + + + <% end %> <% notification.journal.details.each do |detail| %> diff --git a/app/views/digest_mailer/work_packages.text.erb b/app/views/digest_mailer/work_packages.text.erb index 46090e8d8a..4f60246ba4 100644 --- a/app/views/digest_mailer/work_packages.text.erb +++ b/app/views/digest_mailer/work_packages.text.erb @@ -1,3 +1,7 @@ +<%= I18n.t(:'mail.digests.salutation', user: @user.firstname) %> +<%= digest_summary_text(@notification_ids.length, @aggregated_notifications.length) %> +<%= "-" * 100 %> + <% @aggregated_notifications.first(DigestMailer::MAX_SHOWN_WORK_PACKAGES).each do | work_package, notifications_by_work_package| %> <%= "=" * (('# ' + work_package.id.to_s + work_package.subject).length + 4) %> From 57e9279434df0672e9431ca4255fc0f7affd8747 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Wed, 1 Sep 2021 14:34:15 +0200 Subject: [PATCH 11/23] Adapt test to new email layout --- config/locales/en.yml | 2 +- spec/mailers/digest_mailer_spec.rb | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 77057af0cf..6794035aaf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1974,7 +1974,7 @@ en: work_packages: comment_added: 'Comment added' created: 'Created' - created_at: 'by %{user} %{timestamp} ago' + created_at: '%{timestamp} ago by %{user} ' login_to_see_all: 'Log in to see all notifications.' more_to_see_singular: 'There is 1 more work package with notifications.' more_to_see_plural: 'There are %{number} more work packages with notifications.' diff --git a/spec/mailers/digest_mailer_spec.rb b/spec/mailers/digest_mailer_spec.rb index cca14eb1b9..7eb2eb0182 100644 --- a/spec/mailers/digest_mailer_spec.rb +++ b/spec/mailers/digest_mailer_spec.rb @@ -76,7 +76,7 @@ describe DigestMailer, type: :mailer do describe '#work_packages' do subject(:mail) { described_class.work_packages(recipient.id, notifications.map(&:id)) } - let(:mail_body) { mail.body.parts.detect { |part| part['Content-Type'].value == 'text/html' }.body.to_s } + let(:mail_body) { mail.body.encoded } it 'notes the day and the number of notifications in the subject' do expect(mail.subject) @@ -104,20 +104,25 @@ describe DigestMailer, type: :mailer do .to eql recipient.name end - it 'includes the notifications grouped by project and work package' do + it 'includes the notifications grouped by work package' do expect(mail_body) - .to have_selector('body section h1', text: project1.name) + .to have_text("Hey #{recipient.firstname}!") - expected = "#{work_package.type.name} ##{work_package.id} #{work_package.status.name}: #{work_package.subject}" + expected_notification_subject = "#{work_package.type.name.upcase} #{work_package.subject}" expect(mail_body) - .to have_selector('body section section h2', text: expected) + .to have_text(expected_notification_subject, normalize_ws: true) + expected_notification_header = "#{work_package.status.name} ##{work_package.id} - #{work_package.project}" expect(mail_body) - .to have_selector('body section section p.op-uc-p', text: journal.notes) + .to have_text(expected_notification_header, normalize_ws: true) + expected_journal_text = "Comment added less than a minute ago by #{recipient.name}" expect(mail_body) - .to have_selector('body section section li', - text: "Subject changed from old subject to new subject") + .to have_text(expected_journal_text, normalize_ws: true) + + expected_details_text = "Subject changed from old subject to new subject less than a minute ago by #{recipient.name}" + expect(mail_body) + .to have_text(expected_details_text, normalize_ws: true) end context 'with only a deleted work package for the digest' do From 4017151d25879b79cd8603eb47aa9d12427a3d16 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Wed, 1 Sep 2021 15:25:08 +0200 Subject: [PATCH 12/23] Differentiate between normal comments and @mentioned comments --- app/helpers/mail_digest_helper.rb | 12 ++++++++++-- app/views/digest_mailer/work_packages.html.erb | 13 ++++++++----- config/locales/en.yml | 1 + 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index 428e8d9489..2ac0d97522 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -49,6 +49,14 @@ module MailDigestHelper timestamp_text(user, journal, extended_text) end + def digest_comment_text(notification) + if notification.journal.notes.match(//) + I18n.t(:'mail.digests.work_packages.mentioned').html_safe + else + I18n.t(:'mail.digests.work_packages.comment_added').html_safe + end + end + def email_image_tag(image, **options) attachments[image] = File.read(Rails.root.join("app/assets/images/#{image}")) image_tag attachments[image].url, **options @@ -64,9 +72,9 @@ module MailDigestHelper notifications_center_url(['details', id, 'activity']) end - def type_color(type) + def type_color(type, default_fallback) color_id = selected_color(type) - Color.find(color_id).hexcode if color_id + color_id ? Color.find(color_id).hexcode : default_fallback end def status_colors(status) diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 61a71aa7d1..1397c7fa21 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -40,7 +40,9 @@ target="_blank">
+ <%= I18n.t(:'mail.digests.work_packages.comment_added').html_safe %> + <%= digest_notification_timestamp_text(notification) %> +
diff --git a/app/views/digest_mailer/work_packages.text.erb b/app/views/digest_mailer/work_packages.text.erb index e6388d2058..46090e8d8a 100644 --- a/app/views/digest_mailer/work_packages.text.erb +++ b/app/views/digest_mailer/work_packages.text.erb @@ -15,6 +15,7 @@ <% journal = notification.journal %> <% if journal.notes.present? %> + <%= I18n.t(:label_comment_added) %>: <%= journal.notes %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 7baca0c65c..e987f8c726 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1972,6 +1972,7 @@ en: settings: 'Change email settings' time_frame: "At %{time} on %{weekday}, %{date}, you have %{number_unread} unread notifications in %{number_work_packages} work packages" work_packages: + comment_added: 'Comment added' created: 'Created' created_at: 'by %{user} %{timestamp} ago' login_to_see_all: 'Log in to see all notifications.' From 9447285404e31a1bf764cdbd2fb5d8af4177244a Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Mon, 30 Aug 2021 14:44:39 +0200 Subject: [PATCH 09/23] Add logo to email header block --- app/assets/images/logo_openproject_narrow.svg | 9 +++++ app/helpers/mail_digest_helper.rb | 5 +++ .../digest_mailer/work_packages.html.erb | 39 ++++++++++++------- config/locales/en.yml | 1 + 4 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 app/assets/images/logo_openproject_narrow.svg diff --git a/app/assets/images/logo_openproject_narrow.svg b/app/assets/images/logo_openproject_narrow.svg new file mode 100644 index 0000000000..69d321df5c --- /dev/null +++ b/app/assets/images/logo_openproject_narrow.svg @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index bbf1691c81..d93bc7ae08 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -49,6 +49,11 @@ module MailDigestHelper timestamp_text(user, journal, extended_text) end + def email_image_tag(image, **options) + attachments[image] = File.read(Rails.root.join("app/assets/images/#{image}")) + image_tag attachments[image].url, **options + end + def unique_reasons_of_notifications(notifications) notifications .map(&:reason_mail_digest) diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 3efed52cbe..c156ac6d80 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -1,21 +1,30 @@ - - - - - - -
- <%= I18n.t(:'mail.digests.salutation', user: @user.firstname) %> + + <%= email_image_tag("logo_openproject_narrow.svg", alt: I18n.t(:'mail.logo_alt_text')) %>
- <%= digest_timespan_text(@notification_ids.length, @aggregated_notifications.length) %> -
- - <%= I18n.t(:'mail.digests.center') %> - + + + + + + + + + + + +
+ <%= I18n.t(:'mail.digests.salutation', user: @user.firstname) %> +
+ <%= digest_timespan_text(@notification_ids.length, @aggregated_notifications.length) %> +
+ + <%= I18n.t(:'mail.digests.center') %> + +
diff --git a/config/locales/en.yml b/config/locales/en.yml index e987f8c726..77057af0cf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1988,6 +1988,7 @@ en: subject: 'Daily project summary for %{date} - %{number} updates' updated: 'Updated' updated_at: '%{timestamp} ago by %{user}' + logo_alt_text: 'OpenProject Logo' mail_body_account_activation_request: "A new user (%{value}) has registered. The account is pending your approval:" mail_body_account_information: "Your account information" From 1dcea8cbace6071bcab506199ad33eb09e034ff7 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Tue, 31 Aug 2021 15:02:23 +0200 Subject: [PATCH 10/23] Stabilise email for the case that status or type don't have a defined color --- app/helpers/mail_digest_helper.rb | 12 ++++++------ app/mailers/digest_mailer.rb | 2 +- app/views/digest_mailer/work_packages.html.erb | 2 +- app/views/digest_mailer/work_packages.text.erb | 4 ++++ 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index d93bc7ae08..428e8d9489 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -31,10 +31,10 @@ module MailDigestHelper include ::ColorsHelper - def digest_timespan_text(notification_count, wp_count) + def digest_summary_text(notification_count, wp_count) date = Time.parse(Setting.notification_email_digest_time) - I18n.t(:"mail.digests.time_frame", + I18n.t(:'mail.digests.time_frame', time: Setting.notification_email_digest_time, weekday: day_name(date.wday), date: ::I18n.l(date.to_date, format: :long), @@ -66,12 +66,12 @@ module MailDigestHelper def type_color(type) color_id = selected_color(type) - Color.find(color_id).hexcode + Color.find(color_id).hexcode if color_id end - def status_colors(object) - color_id = selected_color(object) - Color.find(color_id).color_styles.map { |k, v| "#{k}:#{v};" }.join(' ') + def status_colors(status) + color_id = selected_color(status) + Color.find(color_id).color_styles.map { |k, v| "#{k}:#{v};" }.join(' ') if color_id end private diff --git a/app/mailers/digest_mailer.rb b/app/mailers/digest_mailer.rb index 7a585f9452..f11dcc4413 100644 --- a/app/mailers/digest_mailer.rb +++ b/app/mailers/digest_mailer.rb @@ -39,7 +39,7 @@ class DigestMailer < ApplicationMailer helper :mail_digest - MAX_SHOWN_WORK_PACKAGES = 15.freeze + MAX_SHOWN_WORK_PACKAGES = 15 class << self def generate_message_id(_, user) diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index c156ac6d80..61a71aa7d1 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -12,7 +12,7 @@
- <%= digest_timespan_text(@notification_ids.length, @aggregated_notifications.length) %> + <%= digest_summary_text(@notification_ids.length, @aggregated_notifications.length) %>
-
white-space: nowrap; padding: 0 5px;"> <%= work_package.status %> @@ -52,16 +54,17 @@ - - <%= work_package.project %> - + - <%= work_package.project %> <% unique_reasons = unique_reasons_of_notifications(notifications_by_work_package) %> + <%= ' - ' unless unique_reasons.length === 1 && unique_reasons.first.nil? %> <% unique_reasons.each_with_index do |reason, index| %> <%= I18n.t( :"mail.digests.work_packages.reason.#{reason || :unknown}", - default: '-') %><%= ', ' unless unique_reasons.size-1 == index %> + default: '') %><%= ', ' unless unique_reasons.size-1 == index %> <% end %> @@ -77,7 +80,7 @@
- diff --git a/config/locales/en.yml b/config/locales/en.yml index 6794035aaf..e9d9abd7f7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1976,6 +1976,7 @@ en: created: 'Created' created_at: '%{timestamp} ago by %{user} ' login_to_see_all: 'Log in to see all notifications.' + mentioned: 'You have been mentioned in a comment' more_to_see_singular: 'There is 1 more work package with notifications.' more_to_see_plural: 'There are %{number} more work packages with notifications.' reason: From 51ec3c91c64591301cfdd0303fc9897260461b57 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Thu, 2 Sep 2021 11:03:04 +0200 Subject: [PATCH 13/23] Extract shared email parts into partials to make it easier to reuse them later --- app/helpers/mail_digest_helper.rb | 27 ----- app/helpers/mail_notification_helper.rb | 58 ++++++++++ app/mailers/digest_mailer.rb | 3 +- .../digest_mailer/work_packages.html.erb | 105 ++---------------- .../_notification_mailer_header.html.erb | 30 +++++ app/views/mailer/_notification_row.html.erb | 61 ++++++++++ .../_notification_settings_button.html.erb | 5 + config/locales/en.yml | 7 +- 8 files changed, 171 insertions(+), 125 deletions(-) create mode 100644 app/helpers/mail_notification_helper.rb create mode 100644 app/views/mailer/_notification_mailer_header.html.erb create mode 100644 app/views/mailer/_notification_row.html.erb create mode 100644 app/views/mailer/_notification_settings_button.html.erb diff --git a/app/helpers/mail_digest_helper.rb b/app/helpers/mail_digest_helper.rb index 2ac0d97522..1dc389bd5c 100644 --- a/app/helpers/mail_digest_helper.rb +++ b/app/helpers/mail_digest_helper.rb @@ -29,8 +29,6 @@ #++ module MailDigestHelper - include ::ColorsHelper - def digest_summary_text(notification_count, wp_count) date = Time.parse(Setting.notification_email_digest_time) @@ -57,31 +55,6 @@ module MailDigestHelper end end - def email_image_tag(image, **options) - attachments[image] = File.read(Rails.root.join("app/assets/images/#{image}")) - image_tag attachments[image].url, **options - end - - def unique_reasons_of_notifications(notifications) - notifications - .map(&:reason_mail_digest) - .uniq - end - - def notifications_path(id) - notifications_center_url(['details', id, 'activity']) - end - - def type_color(type, default_fallback) - color_id = selected_color(type) - color_id ? Color.find(color_id).hexcode : default_fallback - end - - def status_colors(status) - color_id = selected_color(status) - Color.find(color_id).color_styles.map { |k, v| "#{k}:#{v};" }.join(' ') if color_id - end - private def timestamp_text(user, journal, extended) diff --git a/app/helpers/mail_notification_helper.rb b/app/helpers/mail_notification_helper.rb new file mode 100644 index 0000000000..b32bf04eae --- /dev/null +++ b/app/helpers/mail_notification_helper.rb @@ -0,0 +1,58 @@ +#-- 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 docs/COPYRIGHT.rdoc for more details. +#++ + +module MailNotificationHelper + include ::ColorsHelper + + def email_image_tag(image, **options) + attachments[image] = File.read(Rails.root.join("app/assets/images/#{image}")) + image_tag attachments[image].url, **options + end + + def unique_reasons_of_notifications(notifications) + notifications + .map(&:reason_mail_digest) + .uniq + end + + def notifications_path(id) + notifications_center_url(['details', id, 'activity']) + end + + def type_color(type, default_fallback) + color_id = selected_color(type) + color_id ? Color.find(color_id).hexcode : default_fallback + end + + def status_colors(status) + color_id = selected_color(status) + Color.find(color_id).color_styles.map { |k, v| "#{k}:#{v};" }.join(' ') if color_id + end +end diff --git a/app/mailers/digest_mailer.rb b/app/mailers/digest_mailer.rb index f11dcc4413..6648f6a3be 100644 --- a/app/mailers/digest_mailer.rb +++ b/app/mailers/digest_mailer.rb @@ -37,7 +37,8 @@ class DigestMailer < ApplicationMailer include OpenProject::TextFormatting include Redmine::I18n - helper :mail_digest + helper :mail_digest, + :mail_notification MAX_SHOWN_WORK_PACKAGES = 15 diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index 1397c7fa21..c5ea948feb 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -1,93 +1,14 @@ -
+ <%= work_package.type.to_s.upcase %> @@ -90,7 +93,7 @@ <% if notification.journal.notes.present? %>
- <%= I18n.t(:'mail.digests.work_packages.comment_added').html_safe %> + <%= digest_comment_text(notification) %> <%= digest_notification_timestamp_text(notification) %>
- - - - -
- <%= email_image_tag("logo_openproject_narrow.svg", alt: I18n.t(:'mail.logo_alt_text')) %> - - - - - - - - - - - -
- <%= I18n.t(:'mail.digests.salutation', user: @user.firstname) %> -
- <%= digest_summary_text(@notification_ids.length, @aggregated_notifications.length) %> -
- - <%= I18n.t(:'mail.digests.center') %> - -
-
+<%= render partial: 'mailer/notification_mailer_header', + locals: { + summary: digest_summary_text(@notification_ids.length, @aggregated_notifications.length) + } %> <% @aggregated_notifications.first(DigestMailer::MAX_SHOWN_WORK_PACKAGES).each do | work_package, notifications_by_work_package| %> - - - - - - - - - -
- <%= work_package.status %> - - #<%= work_package.id %> - - - <%= work_package.project %> - - <% unique_reasons = unique_reasons_of_notifications(notifications_by_work_package) %> - <%= ' - ' unless unique_reasons.length === 1 && unique_reasons.first.nil? %> - <% unique_reasons.each_with_index do |reason, index| %> - <%= I18n.t( - :"mail.digests.work_packages.reason.#{reason || :unknown}", - default: '') %><%= ', ' unless unique_reasons.size-1 == index %> - <% end %> - - - <%= notifications_by_work_package.length %> - -
- - - - - -
- <%= work_package.type.to_s.upcase %> - - <%= work_package.subject %> -
+ <%= render layout: 'mailer/notification_row', + locals: { + work_package: work_package, + notifications_by_work_package: notifications_by_work_package + } do %> <% notifications_by_work_package.each do | notification | %> <% if notification.journal.notes.present? %> @@ -108,7 +29,7 @@ <% end %> <% end %>
- + <% end %> <% end %> @@ -131,11 +52,7 @@ <% end %>
- - <%= I18n.t(:'mail.digests.settings') %> - + <%= render partial: 'mailer/notification_settings_button' %>
diff --git a/app/views/mailer/_notification_mailer_header.html.erb b/app/views/mailer/_notification_mailer_header.html.erb new file mode 100644 index 0000000000..459951821c --- /dev/null +++ b/app/views/mailer/_notification_mailer_header.html.erb @@ -0,0 +1,30 @@ + + + + + +
+ <%= email_image_tag("logo_openproject_narrow.svg", alt: I18n.t(:'mail.logo_alt_text')) %> + + + + + + + + + + + +
+ <%= I18n.t(:'mail.salutation', user: @user.firstname) %> +
+ <%= summary %> +
+ + <%= I18n.t(:'mail.notification.center') %> + +
+
\ No newline at end of file diff --git a/app/views/mailer/_notification_row.html.erb b/app/views/mailer/_notification_row.html.erb new file mode 100644 index 0000000000..8db6e76d7d --- /dev/null +++ b/app/views/mailer/_notification_row.html.erb @@ -0,0 +1,61 @@ + + + + + + + + + +
+ <%= work_package.status %> + + #<%= work_package.id %> + + - <%= work_package.project %> + + <% unique_reasons = unique_reasons_of_notifications(notifications_by_work_package) %> + <%= ' - ' unless unique_reasons.length === 1 && unique_reasons.first.nil? %> + <% unique_reasons.each_with_index do |reason, index| %> + <%= I18n.t( + :"mail.digests.work_packages.reason.#{reason || :unknown}", + default: '') %><%= ', ' unless unique_reasons.size-1 == index %> + <% end %> + + + <%= notifications_by_work_package.length %> + +
+ + + + + +
+ <%= work_package.type.to_s.upcase %> + + <%= work_package.subject %> +
+ + <%= yield %> + \ No newline at end of file diff --git a/app/views/mailer/_notification_settings_button.html.erb b/app/views/mailer/_notification_settings_button.html.erb new file mode 100644 index 0000000000..a3d01ab38a --- /dev/null +++ b/app/views/mailer/_notification_settings_button.html.erb @@ -0,0 +1,5 @@ + + <%= I18n.t(:'mail.notification.settings') %> + \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index e9d9abd7f7..9248ddd8f5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1967,9 +1967,6 @@ en: mail: actions: 'Actions' digests: - center: 'Notification center' - salutation: 'Hey %{user}!' - settings: 'Change email settings' time_frame: "At %{time} on %{weekday}, %{date}, you have %{number_unread} unread notifications in %{number_work_packages} work packages" work_packages: comment_added: 'Comment added' @@ -1990,6 +1987,10 @@ en: updated: 'Updated' updated_at: '%{timestamp} ago by %{user}' logo_alt_text: 'OpenProject Logo' + notification: + center: 'Notification center' + settings: 'Change email settings' + salutation: 'Hey %{user}!' mail_body_account_activation_request: "A new user (%{value}) has registered. The account is pending your approval:" mail_body_account_information: "Your account information" From 3dede40fe5b6ad137cbab4c09c98ddffa8f83216 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Mon, 6 Sep 2021 14:06:29 +0200 Subject: [PATCH 14/23] Adjust spacings and alignment in daily email --- .../digest_mailer/work_packages.html.erb | 6 +++--- .../digest_mailer/work_packages.text.erb | 2 +- app/views/layouts/mailer.html.erb | 4 ++-- .../_notification_mailer_header.html.erb | 6 +++--- app/views/mailer/_notification_row.html.erb | 21 ++++++++++++------- 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/app/views/digest_mailer/work_packages.html.erb b/app/views/digest_mailer/work_packages.html.erb index c5ea948feb..1ad0b2bbab 100644 --- a/app/views/digest_mailer/work_packages.html.erb +++ b/app/views/digest_mailer/work_packages.html.erb @@ -12,7 +12,7 @@ <% notifications_by_work_package.each do | notification | %> <% if notification.journal.notes.present? %> - + <% end %> <% notification.journal.details.each do |detail| %> - +
<%= digest_comment_text(notification) %> <%= digest_notification_timestamp_text(notification) %> @@ -20,7 +20,7 @@
<%= notification.journal.render_detail(detail, only_path: false) %> <%= digest_notification_timestamp_text(notification) %> @@ -36,7 +36,7 @@
<% if @aggregated_notifications.length > DigestMailer::MAX_SHOWN_WORK_PACKAGES %> - + <% number_of_overflowing_work_packages = @aggregated_notifications.length - DigestMailer::MAX_SHOWN_WORK_PACKAGES %> <% if number_of_overflowing_work_packages === 1 %> <%= I18n.t(:'mail.digests.work_packages.more_to_see_singular') %> diff --git a/app/views/digest_mailer/work_packages.text.erb b/app/views/digest_mailer/work_packages.text.erb index 4f60246ba4..089a332c9f 100644 --- a/app/views/digest_mailer/work_packages.text.erb +++ b/app/views/digest_mailer/work_packages.text.erb @@ -1,4 +1,4 @@ -<%= I18n.t(:'mail.digests.salutation', user: @user.firstname) %> +<%= I18n.t(:'mail.salutation', user: @user.firstname) %> <%= digest_summary_text(@notification_ids.length, @aggregated_notifications.length) %> <%= "-" * 100 %> diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index 117a987cf1..213fb1d369 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -30,8 +30,8 @@ See COPYRIGHT and LICENSE files for more details.