From d60dfaf1b46bb1cb6866be15590e7ac8e6c2c2fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 7 May 2026 10:00:49 +0200 Subject: [PATCH 1/5] Allow journals to be aggregated with different times --- app/services/journals/create_service.rb | 4 +++- modules/meeting/app/models/meeting.rb | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/services/journals/create_service.rb b/app/services/journals/create_service.rb index 7f9650795aff..7e73dc95095a 100644 --- a/app/services/journals/create_service.rb +++ b/app/services/journals/create_service.rb @@ -566,7 +566,9 @@ def aggregation_active? end def within_aggregation_time?(predecessor) - predecessor.updated_at >= (Time.zone.now - Setting.journal_aggregation_time_minutes.to_i.minutes) + minutes = journable.class.try(:journal_aggregation_time_minutes) || + Setting.journal_aggregation_time_minutes.to_i + predecessor.updated_at >= (Time.zone.now - minutes.minutes) end def same_user?(predecessor) diff --git a/modules/meeting/app/models/meeting.rb b/modules/meeting/app/models/meeting.rb index 9c0517755e16..e63dbf0f02d7 100644 --- a/modules/meeting/app/models/meeting.rb +++ b/modules/meeting/app/models/meeting.rb @@ -152,6 +152,13 @@ class Meeting < ApplicationRecord system: "system" }, prefix: :sharing, validate: { allow_nil: true } + # Journals for meetings are aggregated only within the email debounce window so that + # the debounce job always has a populated since_journal to diff against. + # Returns nil when the setting is not yet defined, falling back to the global default. + def self.journal_aggregation_time_minutes + Setting.meeting_email_debounce_minutes.to_i + end + def self.templates_visible_in_project(project, user = User.current) accessible_ids = Project.allowed_to(user, :view_meetings).select(:id) From 04edc4f7533a0a917a2b62c6d39d3c93f6275d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Wed, 6 May 2026 21:22:58 +0200 Subject: [PATCH 2/5] Debounce meeting participant additions/removals --- app/models/journal.rb | 13 +- .../meeting_participants_controller.rb | 4 +- modules/meeting/app/mailers/meeting_mailer.rb | 24 +- .../app/mailers/meeting_series_mailer.rb | 24 +- .../models/activities/meeting_event_mapper.rb | 2 +- .../journal/meeting_participant_journal.rb | 38 +++ modules/meeting/app/models/meeting.rb | 25 +- .../create_service/agenda_itemable.rb | 0 .../journals/create_service/participatable.rb | 97 ++++++ .../meeting_participants/create_service.rb | 51 +-- .../meeting_participants/delete_service.rb | 57 +--- .../app/services/meetings/delete_service.rb | 1 + ...spatch_aggregated_notifications_service.rb | 177 +++++++++++ .../recurring_meetings/delete_service.rb | 1 + .../meeting_mailer/participant_added.html.erb | 2 +- .../meeting_mailer/participant_added.text.erb | 2 +- .../participant_removed.html.erb | 10 +- .../participant_removed.text.erb | 2 +- .../participants_changed.html.erb | 117 +++++++ .../participants_changed.text.erb | 41 +++ .../participant_added.html.erb | 2 +- .../participant_added.text.erb | 2 +- .../participant_removed.html.erb | 10 +- .../participant_removed.text.erb | 2 +- .../participants_changed.html.erb | 115 +++++++ .../participants_changed.text.erb | 41 +++ .../meetings/notification_debounce_job.rb | 106 +++++++ modules/meeting/config/locales/en.yml | 8 + ...001_create_meeting_participant_journals.rb | 18 ++ .../tables/meeting_participant_journals.rb | 48 +++ .../lib/open_project/meeting/engine.rb | 1 + .../spec/mailers/meeting_mailer_spec.rb | 52 +++- .../mailers/meeting_series_mailer_spec.rb | 52 +++- .../meeting_acts_as_journalized_spec.rb | 87 ++++++ .../requests/meeting_participants_spec.rb | 42 +-- .../create_service_spec.rb | 27 ++ .../delete_service_spec.rb | 30 ++ ...h_aggregated_notifications_service_spec.rb | 292 ++++++++++++++++++ .../notification_debounce_job_spec.rb | 171 ++++++++++ 39 files changed, 1629 insertions(+), 165 deletions(-) create mode 100644 modules/meeting/app/models/journal/meeting_participant_journal.rb rename {app => modules/meeting/app}/services/journals/create_service/agenda_itemable.rb (100%) create mode 100644 modules/meeting/app/services/journals/create_service/participatable.rb create mode 100644 modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb create mode 100644 modules/meeting/app/views/meeting_mailer/participants_changed.html.erb create mode 100644 modules/meeting/app/views/meeting_mailer/participants_changed.text.erb create mode 100644 modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb create mode 100644 modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb create mode 100644 modules/meeting/app/workers/meetings/notification_debounce_job.rb create mode 100644 modules/meeting/db/migrate/20260506000001_create_meeting_participant_journals.rb create mode 100644 modules/meeting/db/migrate/tables/meeting_participant_journals.rb create mode 100644 modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb create mode 100644 modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb diff --git a/app/models/journal.rb b/app/models/journal.rb index 89986f8337cd..03f86bbe4be5 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -110,6 +110,7 @@ class Journal < ApplicationRecord belongs_to :data, polymorphic: true, dependent: :destroy has_many :agenda_item_journals, class_name: "Journal::MeetingAgendaItemJournal", dependent: :delete_all + has_many :participant_journals, class_name: "Journal::MeetingParticipantJournal", dependent: :delete_all has_many :attachable_journals, class_name: "Journal::AttachableJournal", dependent: :delete_all has_many :customizable_journals, class_name: "Journal::CustomizableJournal", dependent: :delete_all has_many :custom_comment_journals, class_name: "Journal::CustomCommentJournal", dependent: :delete_all @@ -223,12 +224,6 @@ def has_unread_notifications_for_user?(user) end end - private - - def has_file_links? - journable.respond_to?(:file_links) - end - def predecessor return @predecessor if defined?(@predecessor) @@ -242,4 +237,10 @@ def predecessor .first end end + + private + + def has_file_links? + journable.respond_to?(:file_links) + end end diff --git a/modules/meeting/app/controllers/meeting_participants_controller.rb b/modules/meeting/app/controllers/meeting_participants_controller.rb index f62795210abd..a17ecfb93fe7 100644 --- a/modules/meeting/app/controllers/meeting_participants_controller.rb +++ b/modules/meeting/app/controllers/meeting_participants_controller.rb @@ -125,7 +125,7 @@ def remove_from_upcoming_occurrences(user_id) next unless participant MeetingParticipants::DeleteService - .new(user: User.current, model: participant) + .new(user: User.current, model: participant, notify: false) .call end end @@ -136,7 +136,7 @@ def add_to_upcoming_occurrences(user_ids) next if meeting.participants.exists?(user_id:) MeetingParticipants::CreateService - .new(user: User.current) + .new(user: User.current, notify: false) .call(meeting:, user_id:, invited: true, attended: false) end end diff --git a/modules/meeting/app/mailers/meeting_mailer.rb b/modules/meeting/app/mailers/meeting_mailer.rb index 415cea5406ea..48757e02643d 100644 --- a/modules/meeting/app/mailers/meeting_mailer.rb +++ b/modules/meeting/app/mailers/meeting_mailer.rb @@ -117,11 +117,11 @@ def icalendar_notification(meeting, user, _actor, **) end end - def participant_added(meeting, user, actor, added_participant:) + def participant_added(meeting, user, actor, added_participants:) @actor = actor @meeting = meeting @user = user - @added_participant = added_participant + @added_participants = Array(added_participants) open_project_headers "Project" => @meeting.project.identifier, "Meeting-Id" => @meeting.id @@ -132,11 +132,11 @@ def participant_added(meeting, user, actor, added_participant:) end end - def participant_removed(meeting, user, actor, removed_participant:) + def participant_removed(meeting, user, actor, removed_participants:) @actor = actor @meeting = meeting @user = user - @removed_participant = removed_participant + @removed_participants = Array(removed_participants) open_project_headers "Project" => @meeting.project.identifier, "Meeting-Id" => @meeting.id @@ -147,6 +147,22 @@ def participant_removed(meeting, user, actor, removed_participant:) end end + def participants_changed(meeting, user, actor, added_participants:, removed_participants:) + @actor = actor + @meeting = meeting + @user = user + @added_participants = Array(added_participants) + @removed_participants = Array(removed_participants) + + open_project_headers "Project" => @meeting.project.identifier, + "Meeting-Id" => @meeting.id + + with_attached_ics(meeting, user) do + subject = I18n.t("meeting.email.participants_changed.header", title: @meeting.title) + mail(to: user, subject: "[#{@meeting.project.name}] #{subject}") + end + end + private def with_attached_ics(meeting, user, **args) diff --git a/modules/meeting/app/mailers/meeting_series_mailer.rb b/modules/meeting/app/mailers/meeting_series_mailer.rb index 8c3e0a324e7c..294074751c5e 100644 --- a/modules/meeting/app/mailers/meeting_series_mailer.rb +++ b/modules/meeting/app/mailers/meeting_series_mailer.rb @@ -60,12 +60,12 @@ def updated(series, user, actor, changes:) end end - def participant_added(series, user, actor, added_participant:) + def participant_added(series, user, actor, added_participants:) @actor = actor @series = series @template = series.template @user = user - @added_participant = added_participant + @added_participants = Array(added_participants) set_headers(series) @@ -75,12 +75,12 @@ def participant_added(series, user, actor, added_participant:) end end - def participant_removed(series, user, actor, removed_participant:) + def participant_removed(series, user, actor, removed_participants:) @actor = actor @series = series @template = series.template @user = user - @removed_participant = removed_participant + @removed_participants = Array(removed_participants) set_headers(series) @@ -90,6 +90,22 @@ def participant_removed(series, user, actor, removed_participant:) end end + def participants_changed(series, user, actor, added_participants:, removed_participants:) + @actor = actor + @series = series + @template = series.template + @user = user + @added_participants = Array(added_participants) + @removed_participants = Array(removed_participants) + + set_headers(series) + + with_attached_ics(series, user) do + subject = I18n.t("meeting.email.participants_changed.header_series", title: series.title) + mail(to: user, subject: "[#{@series.project.name}] #{subject}") + end + end + private def with_attached_ics(series, user, cancelled: false) diff --git a/modules/meeting/app/models/activities/meeting_event_mapper.rb b/modules/meeting/app/models/activities/meeting_event_mapper.rb index 79e84fd08d8d..34c90c96d69a 100644 --- a/modules/meeting/app/models/activities/meeting_event_mapper.rb +++ b/modules/meeting/app/models/activities/meeting_event_mapper.rb @@ -212,7 +212,7 @@ def event_title(_journal, data) end def journals_includes - super + %i[agenda_item_journals] + super + %i[agenda_item_journals participant_journals] end def url_helpers diff --git a/modules/meeting/app/models/journal/meeting_participant_journal.rb b/modules/meeting/app/models/journal/meeting_participant_journal.rb new file mode 100644 index 000000000000..91e3daf07bf5 --- /dev/null +++ b/modules/meeting/app/models/journal/meeting_participant_journal.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class Journal::MeetingParticipantJournal < ApplicationRecord + self.table_name = "meeting_participant_journals" + + belongs_to :journal + belongs_to :user + + enum :participation_status, MeetingParticipant.participation_statuses, allow_nil: true +end diff --git a/modules/meeting/app/models/meeting.rb b/modules/meeting/app/models/meeting.rb index e63dbf0f02d7..afbfb8733b92 100644 --- a/modules/meeting/app/models/meeting.rb +++ b/modules/meeting/app/models/meeting.rb @@ -152,11 +152,10 @@ class Meeting < ApplicationRecord system: "system" }, prefix: :sharing, validate: { allow_nil: true } - # Journals for meetings are aggregated only within the email debounce window so that - # the debounce job always has a populated since_journal to diff against. - # Returns nil when the setting is not yet defined, falling back to the global default. + # Debounce meeting emails by one minute + # this is currently hard coded def self.journal_aggregation_time_minutes - Setting.meeting_email_debounce_minutes.to_i + 1 end def self.templates_visible_in_project(project, user = User.current) @@ -348,22 +347,6 @@ def add_new_participants_as_watcher def send_updated_mail return unless send_emails? - MeetingNotificationService - .new(self) - .call :updated, - changes: updated_mail_changes - end - - def updated_mail_changes # rubocop:disable Metrics/AbcSize - { - old_start: saved_change_to_start_time? ? saved_change_to_start_time.first : start_time, - new_start: start_time, - old_duration: saved_change_to_duration? ? saved_change_to_duration.first : duration, - new_duration: duration, - old_location: saved_change_to_location? ? saved_change_to_location.first : location, - new_location: location, - old_title: saved_change_to_title? ? saved_change_to_title.first : title, - new_title: title - } + Meetings::NotificationDebounceJob.debounce(self, since_journal_id: last_journal&.id) end end diff --git a/app/services/journals/create_service/agenda_itemable.rb b/modules/meeting/app/services/journals/create_service/agenda_itemable.rb similarity index 100% rename from app/services/journals/create_service/agenda_itemable.rb rename to modules/meeting/app/services/journals/create_service/agenda_itemable.rb diff --git a/modules/meeting/app/services/journals/create_service/participatable.rb b/modules/meeting/app/services/journals/create_service/participatable.rb new file mode 100644 index 000000000000..34b687335eab --- /dev/null +++ b/modules/meeting/app/services/journals/create_service/participatable.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +class Journals::CreateService + class Participatable < Association + def associated? + journable.respond_to?(:participants) + end + + def cleanup_predecessor(predecessor, notes, cause) + cleanup_predecessor_for(predecessor, + notes, + cause, + "meeting_participant_journals", + :journal_id, + :id) + end + + def insert_sql + sanitize(<<~SQL.squish, journable_id:) + INSERT INTO + meeting_participant_journals ( + journal_id, + user_id, + invited, + attended, + participation_status + ) + SELECT + #{id_from_inserted_journal_sql}, + participants.user_id, + COALESCE(participants.invited, false), + COALESCE(participants.attended, false), + participants.participation_status + FROM meeting_participants participants + WHERE + #{only_if_created_sql} + AND participants.meeting_id = :journable_id + ON CONFLICT (journal_id, user_id) DO UPDATE SET + invited = EXCLUDED.invited, + attended = EXCLUDED.attended, + participation_status = EXCLUDED.participation_status + SQL + end + + def changes_sql + sanitize(<<~SQL.squish, journable_id:) + SELECT + max_journals.journable_id + FROM + max_journals + LEFT OUTER JOIN + meeting_participant_journals + ON + meeting_participant_journals.journal_id = max_journals.id + FULL JOIN + (SELECT * + FROM meeting_participants + WHERE meeting_participants.meeting_id = :journable_id) participants + ON + participants.user_id = meeting_participant_journals.user_id + WHERE + (participants.user_id IS DISTINCT FROM meeting_participant_journals.user_id) + OR (participants.invited IS DISTINCT FROM meeting_participant_journals.invited) + OR (participants.attended IS DISTINCT FROM meeting_participant_journals.attended) + OR (participants.participation_status IS DISTINCT FROM meeting_participant_journals.participation_status) + SQL + end + end +end diff --git a/modules/meeting/app/services/meeting_participants/create_service.rb b/modules/meeting/app/services/meeting_participants/create_service.rb index eab08c3de753..012ef722f560 100644 --- a/modules/meeting/app/services/meeting_participants/create_service.rb +++ b/modules/meeting/app/services/meeting_participants/create_service.rb @@ -29,52 +29,23 @@ #++ module MeetingParticipants class CreateService < BaseServices::Create - protected - - def after_perform(call) - send_notification call.result - - call - end - - def send_notification(meeting_participant) - meeting = meeting_participant.meeting - - if Journal::NotificationConfiguration.active? && meeting.send_emails? - send_meeting_invite(meeting, meeting_participant) - notify_other_participants(meeting, meeting_participant) - end + def initialize(user:, model: nil, contract_class: nil, contract_options: {}, notify: true) + @notify = notify + super(user:, model:, contract_class:, contract_options:) end - def send_meeting_invite(meeting, participant) - if meeting.template? - MeetingSeriesMailer.invited(meeting.recurring_meeting, participant.user, user).deliver_later - else - MeetingMailer.invited(meeting, participant.user, user).deliver_later - end - end + protected - def notify_other_participants(meeting, new_participant) - added_participant_name = new_participant.user.name + def after_perform(call) + meeting = call.result.meeting + meeting.touch_and_save_journals - meeting - .participants - .invited - .where.not(id: new_participant.id) - .includes(:user) - .find_each do |participant| - send_participant_added_notification(meeting, participant.user, added_participant_name) + if @notify + since_invited_ids = meeting.participants.where(invited: true).where.not(id: call.result.id).pluck(:user_id) + Meetings::NotificationDebounceJob.debounce(meeting, since_invited_ids:) end - end - def send_participant_added_notification(meeting, recipient, added_participant_name) - if meeting.template? - MeetingSeriesMailer.participant_added(meeting.recurring_meeting, recipient, user, - added_participant: added_participant_name).deliver_later - else - MeetingMailer.participant_added(meeting, recipient, user, - added_participant: added_participant_name).deliver_later - end + call end end end diff --git a/modules/meeting/app/services/meeting_participants/delete_service.rb b/modules/meeting/app/services/meeting_participants/delete_service.rb index 7198cb591f1c..c0dfd00987de 100644 --- a/modules/meeting/app/services/meeting_participants/delete_service.rb +++ b/modules/meeting/app/services/meeting_participants/delete_service.rb @@ -30,57 +30,24 @@ module MeetingParticipants class DeleteService < BaseServices::Delete - protected - - def after_validate(call) - send_notifications if should_send_notification? - - call - end - - def send_notifications - remaining_participants = fetch_remaining_participants - - send_cancellation_notification(model) - notify_remaining_participants(model.meeting, remaining_participants, model.user) - end - - def fetch_remaining_participants - model.meeting.participants.invited - .where.not(id: model.id) - .includes(:user).to_a + def initialize(user:, model:, contract_class: nil, contract_options: {}, notify: true) + @notify = notify + super(user:, model:, contract_class:, contract_options:) end - def send_cancellation_notification(participant) - meeting = participant.meeting - - if meeting.template? - MeetingMailer.cancelled_series(meeting.recurring_meeting, participant.user, user).deliver_later - else - MeetingMailer.cancelled(meeting, participant.user, user).deliver_later - end - end + protected - def notify_remaining_participants(meeting, remaining_participants, removed_user) - removed_participant_name = removed_user.name + def after_perform(call) + meeting = model.meeting + meeting.touch_and_save_journals - remaining_participants.each do |participant| - send_participant_removed_notification(meeting, participant.user, removed_participant_name) + if @notify + since_invited_ids = meeting.participants.where(invited: true).pluck(:user_id) + since_invited_ids |= [model.user_id] if model.invited? + Meetings::NotificationDebounceJob.debounce(meeting, since_invited_ids:) end - end - def send_participant_removed_notification(meeting, recipient, removed_participant_name) - if meeting.template? - MeetingSeriesMailer.participant_removed(meeting.recurring_meeting, recipient, user, - removed_participant: removed_participant_name).deliver_later - else - MeetingMailer.participant_removed(meeting, recipient, user, - removed_participant: removed_participant_name).deliver_later - end - end - - def should_send_notification? - Journal::NotificationConfiguration.active? && model.meeting.send_emails? + call end end end diff --git a/modules/meeting/app/services/meetings/delete_service.rb b/modules/meeting/app/services/meetings/delete_service.rb index 633712ae3a32..9973aff4b385 100644 --- a/modules/meeting/app/services/meetings/delete_service.rb +++ b/modules/meeting/app/services/meetings/delete_service.rb @@ -33,6 +33,7 @@ class DeleteService < ::BaseServices::Delete protected def after_validate(call) + Meetings::NotificationDebounceJob.cancel_pending(model) send_cancellation_mail(model) if model.notify? call diff --git a/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb b/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb new file mode 100644 index 000000000000..6b028e40b508 --- /dev/null +++ b/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb @@ -0,0 +1,177 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +module Meetings + # Compute the diff between two meeting journals and sends + # the appropriate set of emails. + # + # For series templates (meeting.template? == true), series schedule updates are + # handled by RecurringMeetings::UpdateService directly (it has the full context only then). + # This service therefore only dispatches participant-change emails for series templates and one-time meetings + class DispatchAggregatedNotificationsService + attr_reader :meeting, :since_journal, :latest_journal + + def initialize(meeting:, since_journal:, latest_journal:, since_invited_ids: nil) + @meeting = meeting + @since_journal = since_journal + @latest_journal = latest_journal + @since_invited_ids_override = since_invited_ids + end + + def call + return unless Journal::NotificationConfiguration.active? && meeting.send_emails? + + actor = latest_journal.user + + since_invited_ids = @since_invited_ids_override || invited_user_ids_from(since_journal) + latest_invited_ids = invited_user_ids_from(latest_journal) + + added_user_ids = latest_invited_ids - since_invited_ids + removed_user_ids = since_invited_ids - latest_invited_ids + still_invited_ids = latest_invited_ids & since_invited_ids + + all_ids = (added_user_ids + removed_user_ids + still_invited_ids).uniq + users_by_id = User.where(id: all_ids).index_by(&:id) + + added_names = added_user_ids.filter_map { users_by_id[it]&.name } + removed_names = removed_user_ids.filter_map { users_by_id[it]&.name } + + attribute_changes = meeting.template? ? {} : compute_attribute_changes + + added_user_ids.each { |uid| send_invite(users_by_id[uid], actor) } + removed_user_ids.each { |uid| send_cancellation(users_by_id[uid], actor) } + + still_invited_ids.each do |uid| + recipient = users_by_id[uid] + next unless recipient + + send_updated(recipient, actor, attribute_changes) if attribute_changes.any? + + if added_names.any? && removed_names.any? + send_participants_changed(recipient, actor, added_names, removed_names) + elsif added_names.any? + send_participant_added(recipient, actor, added_names) + elsif removed_names.any? + send_participant_removed(recipient, actor, removed_names) + end + end + end + + private + + def invited_user_ids_from(journal) + return [] unless journal + + journal.participant_journals.where(invited: true).pluck(:user_id) + end + + def compute_attribute_changes + return {} unless since_journal + + since_data = since_journal.data + latest_data = latest_journal.data + return {} unless since_data && latest_data + + changes = {} + %i[title location start_time duration].each do |attr| + next unless since_data.respond_to?(attr) && latest_data.respond_to?(attr) + + old_val = since_data.send(attr) + new_val = latest_data.send(attr) + changes[attr] = [old_val, new_val] if old_val != new_val + end + changes + end + + def send_invite(recipient, actor) + return unless recipient + + if meeting.template? + MeetingSeriesMailer.invited(meeting.recurring_meeting, recipient, actor).deliver_later + else + MeetingMailer.invited(meeting, recipient, actor).deliver_later + end + end + + def send_cancellation(recipient, actor) + return unless recipient + + if meeting.template? + MeetingMailer.cancelled_series(meeting.recurring_meeting, recipient, actor).deliver_later + else + MeetingMailer.cancelled(meeting, recipient, actor).deliver_later + end + end + + def send_updated(recipient, actor, changes) + MeetingMailer.updated(meeting, recipient, actor, + changes: { + old_start: changes.dig(:start_time, 0) || meeting.start_time, + new_start: changes.dig(:start_time, 1) || meeting.start_time, + old_duration: changes.dig(:duration, 0) || meeting.duration, + new_duration: changes.dig(:duration, 1) || meeting.duration, + old_location: changes.dig(:location, 0) || meeting.location, + new_location: changes.dig(:location, 1) || meeting.location + }).deliver_later + end + + def send_participant_added(recipient, actor, added_names) + if meeting.template? + MeetingSeriesMailer.participant_added(meeting.recurring_meeting, recipient, actor, + added_participants: added_names).deliver_later + else + MeetingMailer.participant_added(meeting, recipient, actor, + added_participants: added_names).deliver_later + end + end + + def send_participant_removed(recipient, actor, removed_names) + if meeting.template? + MeetingSeriesMailer.participant_removed(meeting.recurring_meeting, recipient, actor, + removed_participants: removed_names).deliver_later + else + MeetingMailer.participant_removed(meeting, recipient, actor, + removed_participants: removed_names).deliver_later + end + end + + def send_participants_changed(recipient, actor, added_names, removed_names) + if meeting.template? + MeetingSeriesMailer.participants_changed(meeting.recurring_meeting, recipient, actor, + added_participants: added_names, + removed_participants: removed_names).deliver_later + else + MeetingMailer.participants_changed(meeting, recipient, actor, + added_participants: added_names, + removed_participants: removed_names).deliver_later + end + end + end +end diff --git a/modules/meeting/app/services/recurring_meetings/delete_service.rb b/modules/meeting/app/services/recurring_meetings/delete_service.rb index 3dad8ed5a563..7f5086ca4af4 100644 --- a/modules/meeting/app/services/recurring_meetings/delete_service.rb +++ b/modules/meeting/app/services/recurring_meetings/delete_service.rb @@ -37,6 +37,7 @@ def default_contract_class end def after_validate(call) + Meetings::NotificationDebounceJob.cancel_pending(model.template) send_cancellation_mail(model) if model.notify? call diff --git a/modules/meeting/app/views/meeting_mailer/participant_added.html.erb b/modules/meeting/app/views/meeting_mailer/participant_added.html.erb index 086fc4c2708d..c987893950a3 100644 --- a/modules/meeting/app/views/meeting_mailer/participant_added.html.erb +++ b/modules/meeting/app/views/meeting_mailer/participant_added.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <%= render partial: "mailer/mailer_header", locals: { user: @user, - summary: I18n.t("meeting.email.participant_added.summary", title: @meeting.title, actor: @actor, participant: @added_participant), + summary: I18n.t("meeting.email.participant_added.summary", title: @meeting.title, actor: @actor, participant: @added_participants.join(", ")), bottom_spacing: false } %> diff --git a/modules/meeting/app/views/meeting_mailer/participant_added.text.erb b/modules/meeting/app/views/meeting_mailer/participant_added.text.erb index b8046ed6d90e..8c12b6004dc3 100644 --- a/modules/meeting/app/views/meeting_mailer/participant_added.text.erb +++ b/modules/meeting/app/views/meeting_mailer/participant_added.text.erb @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<%= I18n.t("meeting.email.participant_added.summary", title: @meeting.title, actor: @actor, participant: @added_participant) %> +<%= I18n.t("meeting.email.participant_added.summary", title: @meeting.title, actor: @actor, participant: @added_participants.join(", ")) %> <%= @meeting.project.name %>: <%= @meeting.title %> (<%= meeting_url(@meeting) %>) diff --git a/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb b/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb index f3373054fb1c..0f3b9950d535 100644 --- a/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb +++ b/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <%= render partial: "mailer/mailer_header", locals: { user: @user, - summary: I18n.t("meeting.email.participant_removed.summary", title: @meeting.title, actor: @actor, participant: @removed_participant), + summary: I18n.t("meeting.email.participant_removed.summary", title: @meeting.title, actor: @actor, participant: @removed_participants.join(", ")), bottom_spacing: false } %> @@ -76,6 +76,14 @@ See COPYRIGHT and LICENSE files for more details. <%= @meeting.author %> + + "> + <%= I18n.t("meeting.email.participant_removed.label") %> + + + <%= @removed_participants.join(", ") %> + + <% if @meeting.participants.exists? %> "> diff --git a/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb b/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb index e4a21ee1de53..1eba29596cc1 100644 --- a/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb +++ b/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<%= I18n.t("meeting.email.participant_removed.summary", title: @meeting.title, actor: @actor, participant: @removed_participant) %> +<%= I18n.t("meeting.email.participant_removed.summary", title: @meeting.title, actor: @actor, participant: @removed_participants.join(", ")) %> <%= @meeting.project.name %>: <%= @meeting.title %> (<%= meeting_url(@meeting) %>) diff --git a/modules/meeting/app/views/meeting_mailer/participants_changed.html.erb b/modules/meeting/app/views/meeting_mailer/participants_changed.html.erb new file mode 100644 index 000000000000..124154f975a8 --- /dev/null +++ b/modules/meeting/app/views/meeting_mailer/participants_changed.html.erb @@ -0,0 +1,117 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 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. + +++#%> + +<%= render layout: "mailer/spacer_table" do %> + <%= render partial: "mailer/mailer_header", + locals: { + user: @user, + summary: I18n.t("meeting.email.participants_changed.summary", title: @meeting.title, actor: @actor), + bottom_spacing: false + } %> + + <%= render layout: "mailer/border_table" do %> + + <%= placeholder_cell("24px", vertical: true) %> + + + + + + + <% if @meeting.location.present? %> + + + + + <% end %> + + + + + + + + + <% if @removed_participants.any? %> + + + + + <% end %> + <% if @added_participants.any? %> + + + + + <% end %> +
"> + <%= I18n.t(:label_meeting_date_time) %> + + <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %> + - + <%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) +
"> + <%= Meeting.human_attribute_name(:location) %> + + <%= auto_link @meeting.location %> +
"> + <%= Meeting.human_attribute_name(:project) %> + + <%= link_to @meeting.project.name, project_url(@meeting.project) %> +
"> + <%= Meeting.human_attribute_name(:author) %> + + <%= @meeting.author %> +
"> + <%= I18n.t("meeting.email.participants_changed.removed") %> + + <%= @removed_participants.join(", ") %> +
"> + <%= I18n.t("meeting.email.participants_changed.added") %> + "> + <%= @added_participants.join(", ") %> +
+ + + <% end %> + + + + <%= placeholder_cell("20px", vertical: false) %> + +
+ + <%= action_button do %> + <%= link_to I18n.t(:"meeting.email.open_meeting_link"), + meeting_url(@meeting), + target: "_blank", + rel: "noopener", + style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> + <% end %> +<% end %> diff --git a/modules/meeting/app/views/meeting_mailer/participants_changed.text.erb b/modules/meeting/app/views/meeting_mailer/participants_changed.text.erb new file mode 100644 index 000000000000..7e9583ced544 --- /dev/null +++ b/modules/meeting/app/views/meeting_mailer/participants_changed.text.erb @@ -0,0 +1,41 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 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. + +++#%> + +<%= I18n.t("meeting.email.participants_changed.summary", title: @meeting.title, actor: @actor) %> + +<%= @meeting.project.name %>: <%= @meeting.title %> (<%= meeting_url(@meeting) %>) + +<%= t :label_meeting_date_time %>: <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %>-<%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) +<%= Meeting.human_attribute_name(:location) %>: <%= @meeting.location %> +<% if @added_participants.any? %> +<%= I18n.t("meeting.email.participants_changed.added") %>: <%= @added_participants.join(", ") %> +<% end %> +<% if @removed_participants.any? %> +<%= I18n.t("meeting.email.participants_changed.removed") %>: <%= @removed_participants.join(", ") %> +<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb b/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb index 2fd31fb0f039..4c5d0522fd1e 100644 --- a/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb +++ b/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <%= render partial: "mailer/mailer_header", locals: { user: @user, - summary: I18n.t("meeting.email.participant_added.summary_series", title: @series.title, actor: @actor, participant: @added_participant), + summary: I18n.t("meeting.email.participant_added.summary_series", title: @series.title, actor: @actor, participant: @added_participants.join(", ")), bottom_spacing: false } %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb b/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb index 4bc98e5bae58..0a76066721b0 100644 --- a/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb +++ b/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<%= I18n.t("meeting.email.participant_added.summary_series", title: @series.title, actor: @actor, participant: @added_participant) %> +<%= I18n.t("meeting.email.participant_added.summary_series", title: @series.title, actor: @actor, participant: @added_participants.join(", ")) %> <%= @series.project.name %>: <%= @series.title %> (<%= recurring_meeting_url(@series) %>) diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb b/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb index 63c4f48d78aa..4680f2aba9a0 100644 --- a/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb +++ b/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb @@ -31,7 +31,7 @@ See COPYRIGHT and LICENSE files for more details. <%= render partial: "mailer/mailer_header", locals: { user: @user, - summary: I18n.t("meeting.email.participant_removed.summary_series", title: @series.title, actor: @actor, participant: @removed_participant), + summary: I18n.t("meeting.email.participant_removed.summary_series", title: @series.title, actor: @actor, participant: @removed_participants.join(", ")), bottom_spacing: false } %> @@ -74,6 +74,14 @@ See COPYRIGHT and LICENSE files for more details. <%= @series.author %> + + "> + <%= I18n.t("meeting.email.participant_removed.label") %> + + + <%= @removed_participants.join(", ") %> + + <% if @template.participants.exists? %> "> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb b/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb index 0b24512f322e..c40ac7ad18c0 100644 --- a/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb +++ b/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<%= I18n.t("meeting.email.participant_removed.summary_series", title: @series.title, actor: @actor, participant: @removed_participant) %> +<%= I18n.t("meeting.email.participant_removed.summary_series", title: @series.title, actor: @actor, participant: @removed_participants.join(", ")) %> <%= @series.project.name %>: <%= @series.title %> (<%= recurring_meeting_url(@series) %>) diff --git a/modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb b/modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb new file mode 100644 index 000000000000..0bb5cf1e6a60 --- /dev/null +++ b/modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb @@ -0,0 +1,115 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 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. + +++#%> + +<%= render layout: "mailer/spacer_table" do %> + <%= render partial: "mailer/mailer_header", + locals: { + user: @user, + summary: I18n.t("meeting.email.participants_changed.summary_series", title: @series.title, actor: @actor), + bottom_spacing: false + } %> + + <%= render layout: "mailer/border_table" do %> + + <%= placeholder_cell("24px", vertical: true) %> + + + + + + + <% if @template.location.present? %> + + + + + <% end %> + + + + + + + + + <% if @removed_participants.any? %> + + + + + <% end %> + <% if @added_participants.any? %> + + + + + <% end %> +
"> + <%= I18n.t(:label_recurring_meeting_schedule) %> + + <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) +
"> + <%= Meeting.human_attribute_name(:location) %> + + <%= auto_link @template.location %> +
"> + <%= Meeting.human_attribute_name(:project) %> + + <%= link_to @series.project.name, project_url(@series.project) %> +
"> + <%= Meeting.human_attribute_name(:author) %> + + <%= @series.author %> +
"> + <%= I18n.t("meeting.email.participants_changed.removed") %> + + <%= @removed_participants.join(", ") %> +
"> + <%= I18n.t("meeting.email.participants_changed.added") %> + "> + <%= @added_participants.join(", ") %> +
+ + + <% end %> + + + + <%= placeholder_cell("20px", vertical: false) %> + +
+ + <%= action_button do %> + <%= link_to I18n.t(:label_view_meeting_series), + recurring_meeting_url(@series), + target: "_blank", + rel: "noopener", + style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> + <% end %> +<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb b/modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb new file mode 100644 index 000000000000..c6724de6c919 --- /dev/null +++ b/modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb @@ -0,0 +1,41 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 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. + +++#%> + +<%= I18n.t("meeting.email.participants_changed.summary_series", title: @series.title, actor: @actor) %> + +<%= @series.project.name %>: <%= @series.title %> (<%= recurring_meeting_url(@series) %>) + +<%= t :label_recurring_meeting_schedule %>: <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) +<%= Meeting.human_attribute_name(:location) %>: <%= @template.location %> +<% if @added_participants.any? %> +<%= I18n.t("meeting.email.participants_changed.added") %>: <%= @added_participants.join(", ") %> +<% end %> +<% if @removed_participants.any? %> +<%= I18n.t("meeting.email.participants_changed.removed") %>: <%= @removed_participants.join(", ") %> +<% end %> diff --git a/modules/meeting/app/workers/meetings/notification_debounce_job.rb b/modules/meeting/app/workers/meetings/notification_debounce_job.rb new file mode 100644 index 000000000000..8e1174f48a75 --- /dev/null +++ b/modules/meeting/app/workers/meetings/notification_debounce_job.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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. +#++ + +module Meetings + # Debounces email notifications for meeting changes. Instead of sending emails + # immediately for every participant addition/removal or attribute update, this + # job waits for 1 minute of inactivity. + # It then sends a set of emails based on the net diff between the + # journal snapshot at the start of the debounce window and the latest journal. + # + # Callers use `.debounce(meeting)` to: + # 1. Cancels any pending job for this meeting (preserving its since_journal_id) + # 2. Schedules a new job with a fresh wait period + # + # When debounce_minutes == 0 the service is called synchronously (previous behavior). + class NotificationDebounceJob < ApplicationJob + include GoodJob::ActiveJobExtensions::Concurrency + + queue_with_priority :notification + + good_job_control_concurrency_with( + total_limit: 1, + key: -> { "Meetings::NotificationDebounceJob-#{arguments.first}" } + ) + + def self.unique_key_for(meeting_id) + "Meetings::NotificationDebounceJob-#{meeting_id}" + end + + # +since_invited_ids+ is an explicit array of invited user IDs representing the + # participant list BEFORE the current debounce window started. Callers that + # know them (CreateService, DeleteService) pass it here so the + # dispatch service can correctly diff participants even when journal aggregation + # has overwritten the predecessor journal in-place. + def self.debounce(meeting, since_journal_id: nil, since_invited_ids: nil) + concurrency_key = unique_key_for(meeting.id) + existing = GoodJob::Job.where(finished_at: nil, concurrency_key:).first + args = preserved_job_args(existing, meeting, since_journal_id, since_invited_ids) + + GoodJob::Job.where(finished_at: nil, concurrency_key:).delete_all + set(wait: 1.minute).perform_later(meeting.id, *args) + end + + def perform(meeting_id, since_journal_id, since_invited_ids = nil) + meeting = Meeting.find_by(id: meeting_id) + return unless meeting + return unless meeting.send_emails? + + since_journal = Journal.find_by(id: since_journal_id) + latest_journal = meeting.last_journal + + return if latest_journal.nil? + return if latest_journal.id == since_journal_id + + Meetings::DispatchAggregatedNotificationsService + .new(meeting:, since_journal:, latest_journal:, since_invited_ids:) + .call + end + + def self.cancel_pending(meeting) + GoodJob::Job.where(finished_at: nil, concurrency_key: unique_key_for(meeting.id)).delete_all + end + + # Extracts since_journal_id, since_invited_ids from the existing pending job + # Uses the following order: previous job args > explicit caller arg > journal predecessor. + def self.preserved_job_args(existing, meeting, since_journal_id, since_invited_ids) + existing_journal_id, existing_invited_ids = serialized_job_args(existing) + [ + existing_journal_id || since_journal_id || meeting.last_journal&.predecessor&.id, + existing_invited_ids || since_invited_ids + ] + end + + def self.serialized_job_args(existing) + args = existing&.serialized_params&.dig("arguments") || [] + [args[1], args[2]] + end + end +end diff --git a/modules/meeting/config/locales/en.yml b/modules/meeting/config/locales/en.yml index 57ae377fd63f..4b6c5ee3704b 100644 --- a/modules/meeting/config/locales/en.yml +++ b/modules/meeting/config/locales/en.yml @@ -301,6 +301,14 @@ en: header_series: "Meeting series '%{title}' - Participant removed" summary: "%{actor} removed %{participant} from the meeting '%{title}'" summary_series: "%{actor} removed %{participant} from the meeting series '%{title}'" + label: "Removed" + participants_changed: + header: "Meeting '%{title}' - Participants updated" + header_series: "Meeting series '%{title}' - Participants updated" + summary: "%{actor} updated the participants of the meeting '%{title}'" + summary_series: "%{actor} updated the participants of the meeting series '%{title}'" + added: "Added" + removed: "Removed" ended: header_series: "Ended: Meeting series '%{title}'" summary_series: "Meeting series '%{title}' has been ended by %{actor}" diff --git a/modules/meeting/db/migrate/20260506000001_create_meeting_participant_journals.rb b/modules/meeting/db/migrate/20260506000001_create_meeting_participant_journals.rb new file mode 100644 index 000000000000..8cbe92708bf6 --- /dev/null +++ b/modules/meeting/db/migrate/20260506000001_create_meeting_participant_journals.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateMeetingParticipantJournals < ActiveRecord::Migration[8.0] + def change + create_table :meeting_participant_journals do |t| + t.integer :journal_id, null: false + t.integer :user_id, null: false + t.boolean :invited, default: false, null: false + t.boolean :attended, default: false, null: false + t.string :participation_status + end + + add_index :meeting_participant_journals, + %i[journal_id user_id], + unique: true, + name: :idx_meeting_participant_journals_journal_user + end +end diff --git a/modules/meeting/db/migrate/tables/meeting_participant_journals.rb b/modules/meeting/db/migrate/tables/meeting_participant_journals.rb new file mode 100644 index 000000000000..2bd206628065 --- /dev/null +++ b/modules/meeting/db/migrate/tables/meeting_participant_journals.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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 Rails.root.join("db/migrate/tables/base").to_s + +class Tables::MeetingParticipantJournals < Tables::Base + def self.table(migration) + create_table migration do |t| + t.integer :journal_id, null: false + t.integer :user_id, null: false + t.boolean :invited, default: false, null: false + t.boolean :attended, default: false, null: false + t.string :participation_status + end + + migration.add_index :meeting_participant_journals, + %i[journal_id user_id], + unique: true, + name: :index_meeting_participant_journals_on_journal_id_and_user_id + end +end diff --git a/modules/meeting/lib/open_project/meeting/engine.rb b/modules/meeting/lib/open_project/meeting/engine.rb index a8df2aa424f9..9d3cb5ccb1a0 100644 --- a/modules/meeting/lib/open_project/meeting/engine.rb +++ b/modules/meeting/lib/open_project/meeting/engine.rb @@ -197,6 +197,7 @@ class Engine < ::Rails::Engine end Journals::CreateService::Association.register(:AgendaItemable) + Journals::CreateService::Association.register(:Participatable) end add_api_path :meetings do diff --git a/modules/meeting/spec/mailers/meeting_mailer_spec.rb b/modules/meeting/spec/mailers/meeting_mailer_spec.rb index 88af39611893..273f76db798d 100644 --- a/modules/meeting/spec/mailers/meeting_mailer_spec.rb +++ b/modules/meeting/spec/mailers/meeting_mailer_spec.rb @@ -370,8 +370,8 @@ def find_calendar_part(message) end describe "participant_added" do - let(:added_participant_name) { "New Participant" } - let(:mail) { described_class.participant_added(meeting, watcher1, author, added_participant: added_participant_name) } + let(:added_participant_names) { ["New Participant", "Another Participant"] } + let(:mail) { described_class.participant_added(meeting, watcher1, author, added_participants: added_participant_names) } it "renders the headers" do expect(mail.subject).to include(meeting.project.name) @@ -384,7 +384,7 @@ def find_calendar_part(message) User.execute_as(watcher1) do expect(mail.text_part.body).to include(meeting.project.name) expect(mail.text_part.body).to include(meeting.title) - expect(mail.text_part.body).to include(added_participant_name) + expect(mail.text_part.body).to include(added_participant_names.first) expect(mail.text_part.body).to include(author.name) end end @@ -393,15 +393,15 @@ def find_calendar_part(message) User.execute_as(watcher1) do expect(mail.html_part.body).to include(meeting.project.name) expect(mail.html_part.body).to include(meeting.title) - expect(mail.html_part.body).to include(added_participant_name) + expect(mail.html_part.body).to include(added_participant_names.first) expect(mail.html_part.body).to include(author.name) end end end describe "participant_removed" do - let(:removed_participant_name) { "Removed Participant" } - let(:mail) { described_class.participant_removed(meeting, watcher1, author, removed_participant: removed_participant_name) } + let(:removed_participant_names) { ["Removed Participant"] } + let(:mail) { described_class.participant_removed(meeting, watcher1, author, removed_participants: removed_participant_names) } it "renders the headers" do expect(mail.subject).to include(meeting.project.name) @@ -414,7 +414,7 @@ def find_calendar_part(message) User.execute_as(watcher1) do expect(mail.text_part.body).to include(meeting.project.name) expect(mail.text_part.body).to include(meeting.title) - expect(mail.text_part.body).to include(removed_participant_name) + expect(mail.text_part.body).to include(removed_participant_names.first) expect(mail.text_part.body).to include(author.name) end end @@ -423,12 +423,48 @@ def find_calendar_part(message) User.execute_as(watcher1) do expect(mail.html_part.body).to include(meeting.project.name) expect(mail.html_part.body).to include(meeting.title) - expect(mail.html_part.body).to include(removed_participant_name) + expect(mail.html_part.body).to include(removed_participant_names.first) expect(mail.html_part.body).to include(author.name) end end end + describe "participants_changed" do + let(:added_names) { ["Added Person"] } + let(:removed_names) { ["Removed Person"] } + let(:mail) do + described_class.participants_changed(meeting, watcher1, author, + added_participants: added_names, + removed_participants: removed_names) + end + + it "renders the headers" do + expect(mail.subject).to include(meeting.project.name) + expect(mail.to).to contain_exactly(watcher1.mail) + expect(mail.from).to eq([ApplicationMailer.reply_to_address]) + end + + it "renders added and removed participants in the text body" do + User.execute_as(watcher1) do + expect(mail.text_part.body).to include(added_names.first) + expect(mail.text_part.body).to include(removed_names.first) + expect(mail.text_part.body).to include(author.name) + end + end + + it "renders added participants in the html body" do + User.execute_as(watcher1) do + expect(mail.html_part.body).to include(added_names.first) + end + end + + it "renders removed participants with strikethrough in the html body" do + User.execute_as(watcher1) do + expect(mail.html_part.body).to include("#{removed_names.first}") + end + end + end + def check_meeting_mail_content(body) expect(body).to include(meeting.project.name) expect(body).to include(meeting.title) diff --git a/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb b/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb index 0261384d5361..e8d9f8a0a5db 100644 --- a/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb +++ b/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb @@ -156,8 +156,8 @@ end describe "participant_added" do - let(:added_participant_name) { "New Participant" } - let(:mail) { described_class.participant_added(series, recipient, author, added_participant: added_participant_name) } + let(:added_participant_names) { ["New Participant"] } + let(:mail) { described_class.participant_added(series, recipient, author, added_participants: added_participant_names) } it "renders the headers" do expect(mail.subject).to include(series.project.name) @@ -170,7 +170,7 @@ User.execute_as(recipient) do expect(mail.text_part.body).to include(series.project.name) expect(mail.text_part.body).to include(series.title) - expect(mail.text_part.body).to include(added_participant_name) + expect(mail.text_part.body).to include(added_participant_names.first) expect(mail.text_part.body).to include(author.name) end end @@ -179,15 +179,15 @@ User.execute_as(recipient) do expect(mail.html_part.body).to include(series.project.name) expect(mail.html_part.body).to include(series.title) - expect(mail.html_part.body).to include(added_participant_name) + expect(mail.html_part.body).to include(added_participant_names.first) expect(mail.html_part.body).to include(author.name) end end end describe "participant_removed" do - let(:removed_participant_name) { "Removed Participant" } - let(:mail) { described_class.participant_removed(series, recipient, author, removed_participant: removed_participant_name) } + let(:removed_participant_names) { ["Removed Participant"] } + let(:mail) { described_class.participant_removed(series, recipient, author, removed_participants: removed_participant_names) } it "renders the headers" do expect(mail.subject).to include(series.project.name) @@ -200,7 +200,7 @@ User.execute_as(recipient) do expect(mail.text_part.body).to include(series.project.name) expect(mail.text_part.body).to include(series.title) - expect(mail.text_part.body).to include(removed_participant_name) + expect(mail.text_part.body).to include(removed_participant_names.first) expect(mail.text_part.body).to include(author.name) end end @@ -209,12 +209,48 @@ User.execute_as(recipient) do expect(mail.html_part.body).to include(series.project.name) expect(mail.html_part.body).to include(series.title) - expect(mail.html_part.body).to include(removed_participant_name) + expect(mail.html_part.body).to include(removed_participant_names.first) expect(mail.html_part.body).to include(author.name) end end end + describe "participants_changed" do + let(:added_names) { ["Added Person"] } + let(:removed_names) { ["Removed Person"] } + let(:mail) do + described_class.participants_changed(series, recipient, author, + added_participants: added_names, + removed_participants: removed_names) + end + + it "renders the headers" do + expect(mail.subject).to include(series.project.name) + expect(mail.to).to contain_exactly(recipient.mail) + expect(mail.from).to eq([ApplicationMailer.reply_to_address]) + end + + it "renders added and removed participants in the text body" do + User.execute_as(recipient) do + expect(mail.text_part.body).to include(added_names.first) + expect(mail.text_part.body).to include(removed_names.first) + expect(mail.text_part.body).to include(author.name) + end + end + + it "renders added participants in the html body" do + User.execute_as(recipient) do + expect(mail.html_part.body).to include(added_names.first) + end + end + + it "renders removed participants with strikethrough in the html body" do + User.execute_as(recipient) do + expect(mail.html_part.body).to include("#{removed_names.first}") + end + end + end + def check_series_mail_content(body) expect(body).to include(series.project.name) expect(body).to include(series.title) diff --git a/modules/meeting/spec/models/meeting_acts_as_journalized_spec.rb b/modules/meeting/spec/models/meeting_acts_as_journalized_spec.rb index b502bace4668..867a9a7dcffa 100644 --- a/modules/meeting/spec/models/meeting_acts_as_journalized_spec.rb +++ b/modules/meeting/spec/models/meeting_acts_as_journalized_spec.rb @@ -280,6 +280,93 @@ end end + describe "participants" do + shared_let(:participant_user) { create(:user) } + + let(:participant) { meeting.participants.find_by(user: participant_user) } + let(:participant_journals) { meeting.journals.last.participant_journals } + + before do + meeting.participants << MeetingParticipant.new(user: participant_user, invited: true, attended: false) + meeting.touch_and_save_journals + end + + context "when a participant is added" do + it "creates a participant journal entry" do + expect(participant_journals.count).to eq(1) + end + + it "records the correct user, invited, and attended values" do + expect(participant_journals.last).to have_attributes( + user_id: participant_user.id, + invited: true, + attended: false + ) + end + end + + context "when a participant is removed", with_settings: { journal_aggregation_time_minutes: 0 } do + subject(:remove_participant) do + participant.destroy + meeting.touch_and_save_journals + end + + it "creates a new journal" do + expect { remove_participant }.to change { meeting.journals.count }.by(1) + end + + it "removes the participant journal from the new journal" do + remove_participant + expect(meeting.journals.last.participant_journals).to be_empty + end + + it "keeps the participant journal in the previous journal" do + previous_participant_journals = participant_journals.to_a + remove_participant + expect(previous_participant_journals.count).to eq(1) + end + end + + context "when a participant's invited status changes", with_settings: { journal_aggregation_time_minutes: 0 } do + subject(:update_participant) do + participant.update!(invited: false) + meeting.touch_and_save_journals + end + + it "creates a new journal" do + expect { update_participant }.to change { meeting.journals.count }.by(1) + end + + it "reflects the updated invited value in the new journal" do + update_participant + expect(meeting.journals.last.participant_journals.last).to have_attributes(invited: false) + end + end + + context "when no participant changes occur" do + it "does not create a new journal" do + expect do + meeting.save_journals + end.not_to change(Journal, :count) + end + end + + context "when save_journals is called again within the aggregation window (regression: PG::UniqueViolation)" do + it "does not raise a unique violation" do + # Simulates save_journals being called a second time while the first journal is still + # the aggregation predecessor (e.g., template_completed updates the meeting right after creation) + meeting.update_column(:title, "Updated title") + expect { meeting.save_journals }.not_to raise_error + end + + it "participant journals reflect the current state" do + meeting.update_column(:title, "Updated title") + meeting.save_journals + expect(meeting.journals.last.participant_journals.count).to eq(1) + end + end + end + describe "#destroy" do let(:meeting_agenda_item) { create(:meeting_agenda_item, meeting:) } diff --git a/modules/meeting/spec/requests/meeting_participants_spec.rb b/modules/meeting/spec/requests/meeting_participants_spec.rb index b311b38b4558..c29ce3946260 100644 --- a/modules/meeting/spec/requests/meeting_participants_spec.rb +++ b/modules/meeting/spec/requests/meeting_participants_spec.rb @@ -76,8 +76,9 @@ it "sends notification email" do expect do - post project_meeting_participants_path(project, meeting), params: params, as: :turbo_stream - perform_enqueued_jobs + perform_enqueued_jobs do + post project_meeting_participants_path(project, meeting), params: params, as: :turbo_stream + end end.to change { ActionMailer::Base.deliveries.size }.by(1) end end @@ -327,8 +328,9 @@ end it "only sends series invitation emails" do - post project_meeting_participants_path(project, template), params: add_params, as: :turbo_stream - perform_enqueued_jobs + perform_enqueued_jobs do + post project_meeting_participants_path(project, template), params: add_params, as: :turbo_stream + end # 1 series invite to new participant + 1 participant added email to existing participant expect(ActionMailer::Base.deliveries.size).to eq(2) @@ -369,12 +371,14 @@ expect(recurring_meeting.meetings.not_templated.find_by(recurrence_start_time: future_occurrence_time)).to be_nil end - it "sends emails for series and open occurrences, but not closed" do - post project_meeting_participants_path(project, template), params:, as: :turbo_stream - perform_enqueued_jobs + it "only sends series emails for the template, not for propagated occurrences" do + perform_enqueued_jobs do + post project_meeting_participants_path(project, template), params:, as: :turbo_stream + end - # 1 series invite to new participant + 1 participant added email to existing participant + 1 occurrence invite - expect(ActionMailer::Base.deliveries.size).to eq(3) + # 1 series invite to new participant + 1 participant added email to existing participant + # occurrence propagation runs with notify: false — no occurrence emails + expect(ActionMailer::Base.deliveries.size).to eq(2) expect(ActionMailer::Base.deliveries.map(&:to).flatten) .to include(user_with_meeting_permissions.mail, user.mail) end @@ -402,8 +406,9 @@ end it "only sends template cancellation emails" do - delete project_meeting_participant_path(project, template, template_participant), as: :turbo_stream - perform_enqueued_jobs + perform_enqueued_jobs do + delete project_meeting_participant_path(project, template, template_participant), as: :turbo_stream + end # 1 cancelled series to removed participant + 1 participant removed to remaining template participant expect(ActionMailer::Base.deliveries.size).to eq(2) @@ -448,14 +453,15 @@ expect(recurring_meeting.meetings.not_templated.find_by(recurrence_start_time: future_occurrence_time)).to be_nil end - it "sends cancellation emails for template and open occurrences, but not closed" do - delete project_meeting_participant_path(project, template, template_participant), - params: delete_params, as: :turbo_stream - perform_enqueued_jobs + it "only sends template cancellation emails, not occurrence emails" do + perform_enqueued_jobs do + delete project_meeting_participant_path(project, template, template_participant), + params: delete_params, as: :turbo_stream + end - # 1 cancelled series to removed participant + 1 participant removed to remaining participant + - # 1 occurrence cancelled - expect(ActionMailer::Base.deliveries.size).to eq(3) + # 1 cancelled series to removed participant + 1 participant removed to remaining participant + # occurrence propagation runs with notify: false — no occurrence emails + expect(ActionMailer::Base.deliveries.size).to eq(2) expect(ActionMailer::Base.deliveries.map(&:to).flatten).to include(user_with_meeting_permissions.mail) end end diff --git a/modules/meeting/spec/services/meeting_participants/create_service_spec.rb b/modules/meeting/spec/services/meeting_participants/create_service_spec.rb index 565adc65d124..b32ed3b06025 100644 --- a/modules/meeting/spec/services/meeting_participants/create_service_spec.rb +++ b/modules/meeting/spec/services/meeting_participants/create_service_spec.rb @@ -53,6 +53,33 @@ expect(subject.result.invited).to be true expect(subject.result.attended).to be false end + + it "triggers the notification debounce job with since_invited_ids" do + allow(Meetings::NotificationDebounceJob).to receive(:debounce) + subject + expect(Meetings::NotificationDebounceJob).to have_received(:debounce).with(meeting, since_invited_ids: anything) + end + + context "when notify: false" do + subject do + described_class.new(user: current_user, notify: false).call(meeting:, user_id:, invited: true, attended: false) + end + + it "creates the participant" do + expect { subject }.to change { meeting.participants.count }.by(1) + end + + it "does not trigger the notification debounce job" do + allow(Meetings::NotificationDebounceJob).to receive(:debounce) + subject + expect(Meetings::NotificationDebounceJob).not_to have_received(:debounce) + end + end + + it "saves a new journal for the meeting", + with_settings: { journal_aggregation_time_minutes: 0 } do + expect { subject }.to change { meeting.journals.count } + end end context "when user does not have meeting permissions" do diff --git a/modules/meeting/spec/services/meeting_participants/delete_service_spec.rb b/modules/meeting/spec/services/meeting_participants/delete_service_spec.rb index 555068f0bd39..45dc5a446f6d 100644 --- a/modules/meeting/spec/services/meeting_participants/delete_service_spec.rb +++ b/modules/meeting/spec/services/meeting_participants/delete_service_spec.rb @@ -49,6 +49,36 @@ expect(subject).to be_success end + + it "triggers the notification debounce job with since_invited_ids" do + participant + allow(Meetings::NotificationDebounceJob).to receive(:debounce) + subject + expect(Meetings::NotificationDebounceJob).to have_received(:debounce).with(meeting, since_invited_ids: anything) + end + + context "when notify: false" do + subject { described_class.new(user: current_user, model: participant, notify: false).call } + + it "deletes the participant" do + participant + expect { subject }.to change { meeting.participants.count }.by(-1) + end + + it "does not trigger the notification debounce job" do + participant + allow(Meetings::NotificationDebounceJob).to receive(:debounce) + subject + expect(Meetings::NotificationDebounceJob).not_to have_received(:debounce) + end + end + + it "saves a new journal for the meeting reflecting the participant removal", + with_settings: { journal_aggregation_time_minutes: 0 } do + participant + meeting.touch_and_save_journals # snapshot participant into journal baseline + expect { subject }.to change { meeting.journals.count } + end end context "when user does not have edit permissions" do diff --git a/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb b/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb new file mode 100644 index 000000000000..78f6433f214f --- /dev/null +++ b/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb @@ -0,0 +1,292 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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" + +RSpec.describe Meetings::DispatchAggregatedNotificationsService do + shared_let(:project) { create(:project, enabled_module_names: %i[meetings]) } + shared_let(:actor) { create(:user, member_with_permissions: { project => %i[view_meetings edit_meetings] }) } + shared_let(:existing_user) { create(:user, member_with_permissions: { project => %i[view_meetings] }) } + shared_let(:new_user) { create(:user, member_with_permissions: { project => %i[view_meetings] }) } + shared_let(:removed_user) { create(:user, member_with_permissions: { project => %i[view_meetings] }) } + + let(:fixed_start_time) { 1.day.from_now.change(usec: 0) } + let(:since_journal) { instance_double(Journal, user: actor, data: since_data) } + let(:latest_journal) { instance_double(Journal, user: actor, data: latest_data) } + let(:since_data) { instance_double(Journal::MeetingJournal, title: "Old Title", location: nil, start_time: fixed_start_time, duration: 1.0) } + let(:latest_data) { instance_double(Journal::MeetingJournal, title: "Old Title", location: nil, start_time: fixed_start_time, duration: 1.0) } + + let(:meeting) do + create(:meeting, project:, author: actor, notify: true) + end + + let(:since_participant_journals) { class_double(Journal::MeetingParticipantJournal) } + let(:latest_participant_journals) { class_double(Journal::MeetingParticipantJournal) } + + before do + allow(since_journal).to receive(:participant_journals).and_return(since_participant_journals) + allow(latest_journal).to receive(:participant_journals).and_return(latest_participant_journals) + allow(since_participant_journals).to receive(:where).with(invited: true).and_return(since_participant_journals) + allow(latest_participant_journals).to receive(:where).with(invited: true).and_return(latest_participant_journals) + + allow(Journal::NotificationConfiguration).to receive(:active?).and_return(true) + end + + subject(:service) do + described_class.new(meeting:, since_journal:, latest_journal:).call + end + + def stub_invited_ids(journal_double, user_ids) + allow(journal_double).to receive(:pluck).with(:user_id).and_return(user_ids) + end + + context "when a user is added (not previously invited)" do + before do + stub_invited_ids(since_participant_journals, [existing_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id, new_user.id]) + end + + it "sends invite to the newly added user" do + expect(MeetingMailer).to receive(:invited).with(meeting, new_user, actor).and_return(double(deliver_later: nil)) + service + end + + it "sends participant_added to the existing user" do + allow(MeetingMailer).to receive(:invited).and_return(double(deliver_later: nil)) + expect(MeetingMailer) + .to receive(:participant_added) + .with(meeting, existing_user, actor, added_participants: ["#{new_user.firstname} #{new_user.lastname}"]) + .and_return(double(deliver_later: nil)) + service + end + + it "does not send participant_added to the newly added user" do + allow(MeetingMailer).to receive_messages(invited: double(deliver_later: nil), participant_added: double(deliver_later: nil)) + expect(MeetingMailer).not_to receive(:participant_added).with(meeting, new_user, anything, anything) + service + end + end + + context "when a user is removed (was previously invited)" do + before do + stub_invited_ids(since_participant_journals, [existing_user.id, removed_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id]) + end + + it "sends cancellation to the removed user" do + expect(MeetingMailer).to receive(:cancelled).with(meeting, removed_user, actor).and_return(double(deliver_later: nil)) + service + end + + it "sends participant_removed to the still-invited user" do + allow(MeetingMailer).to receive(:cancelled).and_return(double(deliver_later: nil)) + expect(MeetingMailer) + .to receive(:participant_removed) + .with(meeting, existing_user, actor, removed_participants: ["#{removed_user.firstname} #{removed_user.lastname}"]) + .and_return(double(deliver_later: nil)) + service + end + end + + context "when a user is added and another is removed in the same window" do + before do + stub_invited_ids(since_participant_journals, [existing_user.id, removed_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id, new_user.id]) + allow(MeetingMailer).to receive_messages(invited: double(deliver_later: nil), + cancelled: double(deliver_later: nil), + participant_added: double(deliver_later: nil), + participant_removed: double(deliver_later: nil), + participants_changed: double(deliver_later: nil)) + end + + it "sends a single participants_changed email to the still-invited user" do + service + expect(MeetingMailer).to have_received(:participants_changed) + .with(meeting, existing_user, actor, + added_participants: ["#{new_user.firstname} #{new_user.lastname}"], + removed_participants: ["#{removed_user.firstname} #{removed_user.lastname}"]) + end + + it "does not send separate participant_added and participant_removed emails" do + service + expect(MeetingMailer).not_to have_received(:participant_added) + expect(MeetingMailer).not_to have_received(:participant_removed) + end + end + + context "when the same user is added and removed within the window (net zero)" do + before do + stub_invited_ids(since_participant_journals, [existing_user.id, removed_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id, removed_user.id]) + end + + it "sends no emails" do + expect(MeetingMailer).not_to receive(:invited) + expect(MeetingMailer).not_to receive(:cancelled) + expect(MeetingMailer).not_to receive(:participant_added) + expect(MeetingMailer).not_to receive(:participant_removed) + service + end + end + + context "when meeting attributes change" do + let(:new_start_time) { 2.days.from_now } + + before do + stub_invited_ids(since_participant_journals, [existing_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id]) + allow(since_data).to receive(:start_time).and_return(1.day.from_now) + allow(latest_data).to receive(:start_time).and_return(new_start_time) + end + + it "sends updated email to the still-invited user" do + expect(MeetingMailer) + .to receive(:updated) + .with(meeting, existing_user, actor, hash_including(changes: hash_including(:old_start, :new_start))) + .and_return(double(deliver_later: nil)) + service + end + end + + context "when nothing changes" do + before do + stub_invited_ids(since_participant_journals, [existing_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id]) + end + + it "sends no emails" do + expect(MeetingMailer).not_to receive(:invited) + expect(MeetingMailer).not_to receive(:cancelled) + expect(MeetingMailer).not_to receive(:updated) + expect(MeetingMailer).not_to receive(:participant_added) + expect(MeetingMailer).not_to receive(:participant_removed) + service + end + end + + context "when since_journal is nil" do + let(:since_journal) { nil } + + before do + stub_invited_ids(latest_participant_journals, [new_user.id]) + end + + it "treats all latest participants as newly added" do + expect(MeetingMailer).to receive(:invited).with(meeting, new_user, actor).and_return(double(deliver_later: nil)) + service + end + end + + context "when since_invited_ids is provided explicitly (journal aggregation override)" do + subject(:service) do + described_class.new( + meeting:, + since_journal: nil, + latest_journal:, + since_invited_ids: [existing_user.id] + ).call + end + + before do + stub_invited_ids(latest_participant_journals, [existing_user.id, new_user.id]) + allow(MeetingMailer).to receive_messages(invited: double(deliver_later: nil), participant_added: double(deliver_later: nil)) + end + + it "sends invite only to the newly added user (not the existing one)" do + service + expect(MeetingMailer).to have_received(:invited).with(meeting, new_user, actor) + expect(MeetingMailer).not_to have_received(:invited).with(meeting, existing_user, anything) + end + + it "sends participant_added to the existing user" do + service + expect(MeetingMailer) + .to have_received(:participant_added) + .with(meeting, existing_user, actor, added_participants: ["#{new_user.firstname} #{new_user.lastname}"]) + end + end + + context "when meeting is a series template" do + let(:recurring_meeting) { create(:recurring_meeting, project:, author: actor) } + let(:meeting) { recurring_meeting.template } + + before do + allow(meeting).to receive(:send_emails?).and_return(true) + stub_invited_ids(since_participant_journals, [existing_user.id]) + stub_invited_ids(latest_participant_journals, [existing_user.id, new_user.id]) + end + + it "sends series invite via MeetingSeriesMailer" do + expect(MeetingSeriesMailer) + .to receive(:invited) + .with(recurring_meeting, new_user, actor) + .and_return(double(deliver_later: nil)) + service + end + + it "sends participant_added via MeetingSeriesMailer" do + allow(MeetingSeriesMailer).to receive(:invited).and_return(double(deliver_later: nil)) + expect(MeetingSeriesMailer) + .to receive(:participant_added) + .with(recurring_meeting, existing_user, actor, added_participants: anything) + .and_return(double(deliver_later: nil)) + service + end + + it "does not send updated even when attributes changed" do + allow(MeetingSeriesMailer).to receive_messages(invited: double(deliver_later: nil), + participant_added: double(deliver_later: nil)) + expect(MeetingSeriesMailer).not_to receive(:updated) + service + end + end + + context "when Journal::NotificationConfiguration is inactive" do + before do + allow(Journal::NotificationConfiguration).to receive(:active?).and_return(false) + end + + it "sends no emails" do + expect(MeetingMailer).not_to receive(:invited) + service + end + end + + context "when meeting cannot send emails" do + before do + allow(meeting).to receive(:send_emails?).and_return(false) + end + + it "sends no emails" do + expect(MeetingMailer).not_to receive(:invited) + service + end + end +end diff --git a/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb b/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb new file mode 100644 index 000000000000..fc63035c5587 --- /dev/null +++ b/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb @@ -0,0 +1,171 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 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" + +RSpec.describe Meetings::NotificationDebounceJob do + shared_let(:project) { create(:project, enabled_module_names: %i[meetings]) } + shared_let(:user) { create(:user, member_with_permissions: { project => %i[view_meetings edit_meetings] }) } + shared_let(:meeting) { create(:meeting, project:, author: user, notify: true) } + + let(:dispatch_service) { instance_double(Meetings::DispatchAggregatedNotificationsService) } + + before do + allow(Meetings::DispatchAggregatedNotificationsService) + .to receive(:new) + .and_return(dispatch_service) + allow(dispatch_service).to receive(:call) + end + + describe ".debounce" do + it "enqueues a background job" do + expect { described_class.debounce(meeting) } + .to have_enqueued_job(described_class) + .with(meeting.id, anything, anything) + end + + it "does not call the dispatch service synchronously" do + described_class.debounce(meeting) + expect(dispatch_service).not_to have_received(:call) + end + + context "when querying GoodJob directly", + with_good_job: described_class do + it "writes the job to the GoodJob::Job table with the correct concurrency key" do + described_class.debounce(meeting) + + job = GoodJob::Job.where(concurrency_key: described_class.unique_key_for(meeting.id)).first + expect(job).not_to be_nil + expect(job.serialized_params.dig("arguments", 0)).to eq(meeting.id) + end + + it "passes nil since_journal_id when meeting journal has no predecessor" do + described_class.debounce(meeting) + + job = GoodJob::Job.where(concurrency_key: described_class.unique_key_for(meeting.id)).first + expect(job.serialized_params.dig("arguments", 1)).to be_nil + end + + context "when called multiple times (debounce reset)" do + before { described_class.debounce(meeting) } + + it "leaves only one pending job" do + described_class.debounce(meeting) + + count = GoodJob::Job.where(finished_at: nil, concurrency_key: described_class.unique_key_for(meeting.id)).count + expect(count).to eq(1) + end + + it "preserves the since_journal_id from the first call" do + first_since_id = GoodJob::Job.where(concurrency_key: described_class.unique_key_for(meeting.id)) + .first + .serialized_params + .dig("arguments", 1) + + described_class.debounce(meeting) + + job = GoodJob::Job.where(finished_at: nil, concurrency_key: described_class.unique_key_for(meeting.id)).first + expect(job.serialized_params.dig("arguments", 1)).to eq(first_since_id) + end + end + end + end + + describe ".cancel_pending", with_good_job: described_class do + before do + described_class.debounce(meeting) + end + + it "removes pending jobs for the meeting" do + expect do + described_class.cancel_pending(meeting) + end.to change { + GoodJob::Job.where(finished_at: nil, concurrency_key: described_class.unique_key_for(meeting.id)).count + }.from(1).to(0) + end + end + + describe "#perform" do + let(:since_journal) { meeting.last_journal } + let(:latest_journal) { instance_double(Journal, id: since_journal.id + 1, user: user) } + + before do + allow(meeting.class).to receive(:find_by).with(id: meeting.id).and_return(meeting) + allow(Journal).to receive(:find_by).with(id: since_journal.id).and_return(since_journal) + allow(meeting).to receive(:last_journal).and_return(latest_journal) + end + + subject { described_class.new.perform(meeting.id, since_journal.id) } + + it "calls the dispatch service with the correct journals" do + subject + expect(Meetings::DispatchAggregatedNotificationsService) + .to have_received(:new) + .with(meeting:, since_journal:, latest_journal:, since_invited_ids: nil) + expect(dispatch_service).to have_received(:call) + end + + context "when meeting does not exist" do + before { allow(meeting.class).to receive(:find_by).with(id: meeting.id).and_return(nil) } + + it "does nothing" do + subject + expect(dispatch_service).not_to have_received(:call) + end + end + + context "when meeting cannot send emails" do + before { allow(meeting).to receive(:send_emails?).and_return(false) } + + it "does nothing" do + subject + expect(dispatch_service).not_to have_received(:call) + end + end + + context "when latest journal equals since_journal_id (no change)" do + before { allow(meeting).to receive(:last_journal).and_return(since_journal) } + + it "does nothing" do + subject + expect(dispatch_service).not_to have_received(:call) + end + end + + context "when there is no latest journal" do + before { allow(meeting).to receive(:last_journal).and_return(nil) } + + it "does nothing" do + subject + expect(dispatch_service).not_to have_received(:call) + end + end + end +end From 0983cf794aaa7b052741f4f627c56640899fb6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 7 May 2026 16:39:05 +0200 Subject: [PATCH 3/5] Consolidate all user_added/removed into one "updated" template --- modules/meeting/app/mailers/meeting_mailer.rb | 50 +------ .../app/mailers/meeting_series_mailer.rb | 50 +------ modules/meeting/app/models/meeting.rb | 4 +- ...spatch_aggregated_notifications_service.rb | 72 ++++------ .../meeting_mailer/participant_added.html.erb | 115 ---------------- .../meeting_mailer/participant_added.text.erb | 37 ------ .../participant_removed.html.erb | 123 ------------------ .../participant_removed.text.erb | 37 ------ .../participants_changed.html.erb | 117 ----------------- .../participants_changed.text.erb | 41 ------ .../app/views/meeting_mailer/updated.html.erb | 21 +++ .../app/views/meeting_mailer/updated.text.erb | 6 + .../participant_added.html.erb | 105 --------------- .../participant_added.text.erb | 36 ----- .../participant_removed.html.erb | 113 ---------------- .../participant_removed.text.erb | 36 ----- .../participants_changed.html.erb | 115 ---------------- .../participants_changed.text.erb | 41 ------ .../meeting_series_mailer/updated.html.erb | 21 +++ .../meeting_series_mailer/updated.text.erb | 6 + modules/meeting/config/locales/en.yml | 20 +-- .../spec/mailers/meeting_mailer_spec.rb | 100 ++++---------- .../mailers/meeting_series_mailer_spec.rb | 91 ++----------- 23 files changed, 129 insertions(+), 1228 deletions(-) delete mode 100644 modules/meeting/app/views/meeting_mailer/participant_added.html.erb delete mode 100644 modules/meeting/app/views/meeting_mailer/participant_added.text.erb delete mode 100644 modules/meeting/app/views/meeting_mailer/participant_removed.html.erb delete mode 100644 modules/meeting/app/views/meeting_mailer/participant_removed.text.erb delete mode 100644 modules/meeting/app/views/meeting_mailer/participants_changed.html.erb delete mode 100644 modules/meeting/app/views/meeting_mailer/participants_changed.text.erb delete mode 100644 modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb delete mode 100644 modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb delete mode 100644 modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb delete mode 100644 modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb delete mode 100644 modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb delete mode 100644 modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb diff --git a/modules/meeting/app/mailers/meeting_mailer.rb b/modules/meeting/app/mailers/meeting_mailer.rb index 48757e02643d..0fbdba7e0cfe 100644 --- a/modules/meeting/app/mailers/meeting_mailer.rb +++ b/modules/meeting/app/mailers/meeting_mailer.rb @@ -45,11 +45,13 @@ def invited(meeting, user, actor) end end - def updated(meeting, user, actor, changes:) + def updated(meeting, user, actor, changes:, added_participants: [], removed_participants: []) @actor = actor @user = user @meeting = meeting @changes = changes + @added_participants = Array(added_participants) + @removed_participants = Array(removed_participants) open_project_headers "Project" => @meeting.project.identifier, "Meeting-Id" => @meeting.id @@ -117,52 +119,6 @@ def icalendar_notification(meeting, user, _actor, **) end end - def participant_added(meeting, user, actor, added_participants:) - @actor = actor - @meeting = meeting - @user = user - @added_participants = Array(added_participants) - - open_project_headers "Project" => @meeting.project.identifier, - "Meeting-Id" => @meeting.id - - with_attached_ics(meeting, user) do - subject = I18n.t("meeting.email.participant_added.header", title: @meeting.title) - mail(to: user, subject: "[#{@meeting.project.name}] #{subject}") - end - end - - def participant_removed(meeting, user, actor, removed_participants:) - @actor = actor - @meeting = meeting - @user = user - @removed_participants = Array(removed_participants) - - open_project_headers "Project" => @meeting.project.identifier, - "Meeting-Id" => @meeting.id - - with_attached_ics(meeting, user) do - subject = I18n.t("meeting.email.participant_removed.header", title: @meeting.title) - mail(to: user, subject: "[#{@meeting.project.name}] #{subject}") - end - end - - def participants_changed(meeting, user, actor, added_participants:, removed_participants:) - @actor = actor - @meeting = meeting - @user = user - @added_participants = Array(added_participants) - @removed_participants = Array(removed_participants) - - open_project_headers "Project" => @meeting.project.identifier, - "Meeting-Id" => @meeting.id - - with_attached_ics(meeting, user) do - subject = I18n.t("meeting.email.participants_changed.header", title: @meeting.title) - mail(to: user, subject: "[#{@meeting.project.name}] #{subject}") - end - end - private def with_attached_ics(meeting, user, **args) diff --git a/modules/meeting/app/mailers/meeting_series_mailer.rb b/modules/meeting/app/mailers/meeting_series_mailer.rb index 294074751c5e..8563718d119a 100644 --- a/modules/meeting/app/mailers/meeting_series_mailer.rb +++ b/modules/meeting/app/mailers/meeting_series_mailer.rb @@ -46,63 +46,19 @@ def invited(series, user, actor) end end - def updated(series, user, actor, changes:) + def updated(series, user, actor, changes:, added_participants: [], removed_participants: []) @actor = actor @series = series @user = user @changes = changes - - set_headers(series) - - with_attached_ics(series, user) do - subject = I18n.t("meeting.email.series_updated.title", title: series.title, project_name: series.project.name) - mail(to: user, subject:) - end - end - - def participant_added(series, user, actor, added_participants:) - @actor = actor - @series = series - @template = series.template - @user = user @added_participants = Array(added_participants) - - set_headers(series) - - with_attached_ics(series, user) do - subject = I18n.t("meeting.email.participant_added.header_series", title: series.title) - mail(to: user, subject: "[#{@series.project.name}] #{subject}") - end - end - - def participant_removed(series, user, actor, removed_participants:) - @actor = actor - @series = series - @template = series.template - @user = user @removed_participants = Array(removed_participants) set_headers(series) with_attached_ics(series, user) do - subject = I18n.t("meeting.email.participant_removed.header_series", title: series.title) - mail(to: user, subject: "[#{@series.project.name}] #{subject}") - end - end - - def participants_changed(series, user, actor, added_participants:, removed_participants:) - @actor = actor - @series = series - @template = series.template - @user = user - @added_participants = Array(added_participants) - @removed_participants = Array(removed_participants) - - set_headers(series) - - with_attached_ics(series, user) do - subject = I18n.t("meeting.email.participants_changed.header_series", title: series.title) - mail(to: user, subject: "[#{@series.project.name}] #{subject}") + subject = I18n.t("meeting.email.series_updated.title", title: series.title, project_name: series.project.name) + mail(to: user, subject:) end end diff --git a/modules/meeting/app/models/meeting.rb b/modules/meeting/app/models/meeting.rb index afbfb8733b92..c2ef60f69771 100644 --- a/modules/meeting/app/models/meeting.rb +++ b/modules/meeting/app/models/meeting.rb @@ -133,7 +133,7 @@ class Meeting < ApplicationRecord before_save :add_new_participants_as_watcher - after_update :send_updated_mail, if: -> { + after_commit :send_updated_mail, on: :update, if: -> { !template? && (saved_change_to_start_time? || saved_change_to_duration? || saved_change_to_location? || saved_change_to_title?) } @@ -347,6 +347,6 @@ def add_new_participants_as_watcher def send_updated_mail return unless send_emails? - Meetings::NotificationDebounceJob.debounce(self, since_journal_id: last_journal&.id) + Meetings::NotificationDebounceJob.debounce(self, since_journal_id: last_journal&.predecessor&.id) end end diff --git a/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb b/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb index 6b028e40b508..ea3f56dbf95d 100644 --- a/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb +++ b/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb @@ -68,19 +68,13 @@ def call added_user_ids.each { |uid| send_invite(users_by_id[uid], actor) } removed_user_ids.each { |uid| send_cancellation(users_by_id[uid], actor) } + return if attribute_changes.empty? && added_names.empty? && removed_names.empty? + still_invited_ids.each do |uid| recipient = users_by_id[uid] next unless recipient - send_updated(recipient, actor, attribute_changes) if attribute_changes.any? - - if added_names.any? && removed_names.any? - send_participants_changed(recipient, actor, added_names, removed_names) - elsif added_names.any? - send_participant_added(recipient, actor, added_names) - elsif removed_names.any? - send_participant_removed(recipient, actor, removed_names) - end + send_updated(recipient, actor, attribute_changes, added_names:, removed_names:) end end @@ -130,48 +124,38 @@ def send_cancellation(recipient, actor) end end - def send_updated(recipient, actor, changes) - MeetingMailer.updated(meeting, recipient, actor, - changes: { - old_start: changes.dig(:start_time, 0) || meeting.start_time, - new_start: changes.dig(:start_time, 1) || meeting.start_time, - old_duration: changes.dig(:duration, 0) || meeting.duration, - new_duration: changes.dig(:duration, 1) || meeting.duration, - old_location: changes.dig(:location, 0) || meeting.location, - new_location: changes.dig(:location, 1) || meeting.location - }).deliver_later - end - - def send_participant_added(recipient, actor, added_names) + def send_updated(recipient, actor, attribute_changes, added_names: [], removed_names: []) if meeting.template? - MeetingSeriesMailer.participant_added(meeting.recurring_meeting, recipient, actor, - added_participants: added_names).deliver_later + send_series_updated(recipient, actor, added_names:, removed_names:) else - MeetingMailer.participant_added(meeting, recipient, actor, - added_participants: added_names).deliver_later + send_meeting_updated(recipient, actor, attribute_changes, added_names:, removed_names:) end end - def send_participant_removed(recipient, actor, removed_names) - if meeting.template? - MeetingSeriesMailer.participant_removed(meeting.recurring_meeting, recipient, actor, - removed_participants: removed_names).deliver_later - else - MeetingMailer.participant_removed(meeting, recipient, actor, - removed_participants: removed_names).deliver_later - end + def send_series_updated(recipient, actor, added_names:, removed_names:) + series = meeting.recurring_meeting + MeetingSeriesMailer.updated(series, recipient, actor, + changes: { old_schedule: series.full_schedule_in_words, + old_location: series.location }, + added_participants: added_names, + removed_participants: removed_names).deliver_later end - def send_participants_changed(recipient, actor, added_names, removed_names) - if meeting.template? - MeetingSeriesMailer.participants_changed(meeting.recurring_meeting, recipient, actor, - added_participants: added_names, - removed_participants: removed_names).deliver_later - else - MeetingMailer.participants_changed(meeting, recipient, actor, - added_participants: added_names, - removed_participants: removed_names).deliver_later - end + def send_meeting_updated(recipient, actor, attribute_changes, added_names:, removed_names:) + MeetingMailer.updated(meeting, recipient, actor, + changes: meeting_changes(attribute_changes), + added_participants: added_names, + removed_participants: removed_names).deliver_later + end + + def meeting_changes(attribute_changes) # rubocop:disable Metrics/AbcSize + start_time = attribute_changes[:start_time] || [meeting.start_time, meeting.start_time] + duration = attribute_changes[:duration] || [meeting.duration, meeting.duration] + location = attribute_changes[:location] || [meeting.location, meeting.location] + + { old_start: start_time[0], new_start: start_time[1], + old_duration: duration[0], new_duration: duration[1], + old_location: location[0], new_location: location[1] } end end end diff --git a/modules/meeting/app/views/meeting_mailer/participant_added.html.erb b/modules/meeting/app/views/meeting_mailer/participant_added.html.erb deleted file mode 100644 index c987893950a3..000000000000 --- a/modules/meeting/app/views/meeting_mailer/participant_added.html.erb +++ /dev/null @@ -1,115 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= render layout: "mailer/spacer_table" do %> - <%= render partial: "mailer/mailer_header", - locals: { - user: @user, - summary: I18n.t("meeting.email.participant_added.summary", title: @meeting.title, actor: @actor, participant: @added_participants.join(", ")), - bottom_spacing: false - } %> - - <%= render layout: "mailer/border_table" do %> - - <%= placeholder_cell("24px", vertical: true) %> - - - - - - - <% if @meeting.location.present? %> - - - - - <% end %> - - - - - - - - - <% if @meeting.participants.exists? %> - - - - - - - - - <% end %> -
"> - <%= I18n.t(:label_meeting_date_time) %> - - <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %> - - - <%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) -
"> - <%= Meeting.human_attribute_name(:location) %> - - <%= auto_link @meeting.location %> -
"> - <%= Meeting.human_attribute_name(:project) %> - - <%= link_to @meeting.project.name, project_url(@meeting.project) %> -
"> - <%= Meeting.human_attribute_name(:author) %> - - <%= @meeting.author %> -
"> - <%= Meeting.human_attribute_name(:participants_invited) %> - - <%= @meeting.participants.invited.sort.join("; ") %> -
"> - <%= Meeting.human_attribute_name(:participants_attended) %> - - <%= @meeting.participants.attended.sort.join("; ") %> -
- - - <% end %> - - - - <%= placeholder_cell("20px", vertical: false) %> - -
- - <%= action_button do %> - <%= link_to I18n.t(:"meeting.email.open_meeting_link"), - meeting_url(@meeting), - target: "_blank", - rel: "noopener", - style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> - <% end %> -<% end %> diff --git a/modules/meeting/app/views/meeting_mailer/participant_added.text.erb b/modules/meeting/app/views/meeting_mailer/participant_added.text.erb deleted file mode 100644 index 8c12b6004dc3..000000000000 --- a/modules/meeting/app/views/meeting_mailer/participant_added.text.erb +++ /dev/null @@ -1,37 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= I18n.t("meeting.email.participant_added.summary", title: @meeting.title, actor: @actor, participant: @added_participants.join(", ")) %> - -<%= @meeting.project.name %>: <%= @meeting.title %> (<%= meeting_url(@meeting) %>) - -<%= t :label_meeting_date_time %>: <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %>-<%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) -<%= Meeting.human_attribute_name(:location) %>: <%= @meeting.location %> -<%= Meeting.human_attribute_name(:participants_invited) %>: <%= @meeting.participants.invited.sort.join("; ") %> -<%= Meeting.human_attribute_name(:participants_attended) %>: <%= @meeting.participants.attended.sort.join("; ") %> diff --git a/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb b/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb deleted file mode 100644 index 0f3b9950d535..000000000000 --- a/modules/meeting/app/views/meeting_mailer/participant_removed.html.erb +++ /dev/null @@ -1,123 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= render layout: "mailer/spacer_table" do %> - <%= render partial: "mailer/mailer_header", - locals: { - user: @user, - summary: I18n.t("meeting.email.participant_removed.summary", title: @meeting.title, actor: @actor, participant: @removed_participants.join(", ")), - bottom_spacing: false - } %> - - <%= render layout: "mailer/border_table" do %> - - <%= placeholder_cell("24px", vertical: true) %> - - - - - - - <% if @meeting.location.present? %> - - - - - <% end %> - - - - - - - - - - - - - <% if @meeting.participants.exists? %> - - - - - - - - - <% end %> -
"> - <%= I18n.t(:label_meeting_date_time) %> - - <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %> - - - <%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) -
"> - <%= Meeting.human_attribute_name(:location) %> - - <%= auto_link @meeting.location %> -
"> - <%= Meeting.human_attribute_name(:project) %> - - <%= link_to @meeting.project.name, project_url(@meeting.project) %> -
"> - <%= Meeting.human_attribute_name(:author) %> - - <%= @meeting.author %> -
"> - <%= I18n.t("meeting.email.participant_removed.label") %> - - <%= @removed_participants.join(", ") %> -
"> - <%= Meeting.human_attribute_name(:participants_invited) %> - - <%= @meeting.participants.invited.sort.join("; ") %> -
"> - <%= Meeting.human_attribute_name(:participants_attended) %> - - <%= @meeting.participants.attended.sort.join("; ") %> -
- - - <% end %> - - - - <%= placeholder_cell("20px", vertical: false) %> - -
- - <%= action_button do %> - <%= link_to I18n.t(:"meeting.email.open_meeting_link"), - meeting_url(@meeting), - target: "_blank", - rel: "noopener", - style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> - <% end %> -<% end %> diff --git a/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb b/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb deleted file mode 100644 index 1eba29596cc1..000000000000 --- a/modules/meeting/app/views/meeting_mailer/participant_removed.text.erb +++ /dev/null @@ -1,37 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= I18n.t("meeting.email.participant_removed.summary", title: @meeting.title, actor: @actor, participant: @removed_participants.join(", ")) %> - -<%= @meeting.project.name %>: <%= @meeting.title %> (<%= meeting_url(@meeting) %>) - -<%= t :label_meeting_date_time %>: <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %>-<%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) -<%= Meeting.human_attribute_name(:location) %>: <%= @meeting.location %> -<%= Meeting.human_attribute_name(:participants_invited) %>: <%= @meeting.participants.invited.sort.join("; ") %> -<%= Meeting.human_attribute_name(:participants_attended) %>: <%= @meeting.participants.attended.sort.join("; ") %> diff --git a/modules/meeting/app/views/meeting_mailer/participants_changed.html.erb b/modules/meeting/app/views/meeting_mailer/participants_changed.html.erb deleted file mode 100644 index 124154f975a8..000000000000 --- a/modules/meeting/app/views/meeting_mailer/participants_changed.html.erb +++ /dev/null @@ -1,117 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= render layout: "mailer/spacer_table" do %> - <%= render partial: "mailer/mailer_header", - locals: { - user: @user, - summary: I18n.t("meeting.email.participants_changed.summary", title: @meeting.title, actor: @actor), - bottom_spacing: false - } %> - - <%= render layout: "mailer/border_table" do %> - - <%= placeholder_cell("24px", vertical: true) %> - - - - - - - <% if @meeting.location.present? %> - - - - - <% end %> - - - - - - - - - <% if @removed_participants.any? %> - - - - - <% end %> - <% if @added_participants.any? %> - - - - - <% end %> -
"> - <%= I18n.t(:label_meeting_date_time) %> - - <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %> - - - <%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) -
"> - <%= Meeting.human_attribute_name(:location) %> - - <%= auto_link @meeting.location %> -
"> - <%= Meeting.human_attribute_name(:project) %> - - <%= link_to @meeting.project.name, project_url(@meeting.project) %> -
"> - <%= Meeting.human_attribute_name(:author) %> - - <%= @meeting.author %> -
"> - <%= I18n.t("meeting.email.participants_changed.removed") %> - - <%= @removed_participants.join(", ") %> -
"> - <%= I18n.t("meeting.email.participants_changed.added") %> - "> - <%= @added_participants.join(", ") %> -
- - - <% end %> - - - - <%= placeholder_cell("20px", vertical: false) %> - -
- - <%= action_button do %> - <%= link_to I18n.t(:"meeting.email.open_meeting_link"), - meeting_url(@meeting), - target: "_blank", - rel: "noopener", - style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> - <% end %> -<% end %> diff --git a/modules/meeting/app/views/meeting_mailer/participants_changed.text.erb b/modules/meeting/app/views/meeting_mailer/participants_changed.text.erb deleted file mode 100644 index 7e9583ced544..000000000000 --- a/modules/meeting/app/views/meeting_mailer/participants_changed.text.erb +++ /dev/null @@ -1,41 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= I18n.t("meeting.email.participants_changed.summary", title: @meeting.title, actor: @actor) %> - -<%= @meeting.project.name %>: <%= @meeting.title %> (<%= meeting_url(@meeting) %>) - -<%= t :label_meeting_date_time %>: <%= format_date @meeting.start_time %> <%= format_time @meeting.start_time, include_date: false %>-<%= format_time @meeting.end_time, include_date: false %> (<%= formatted_time_zone_offset %>) -<%= Meeting.human_attribute_name(:location) %>: <%= @meeting.location %> -<% if @added_participants.any? %> -<%= I18n.t("meeting.email.participants_changed.added") %>: <%= @added_participants.join(", ") %> -<% end %> -<% if @removed_participants.any? %> -<%= I18n.t("meeting.email.participants_changed.removed") %>: <%= @removed_participants.join(", ") %> -<% end %> diff --git a/modules/meeting/app/views/meeting_mailer/updated.html.erb b/modules/meeting/app/views/meeting_mailer/updated.html.erb index e065198ffef2..99eca2a3ea93 100644 --- a/modules/meeting/app/views/meeting_mailer/updated.html.erb +++ b/modules/meeting/app/views/meeting_mailer/updated.html.erb @@ -146,6 +146,27 @@ See COPYRIGHT and LICENSE files for more details. + <% if @added_participants.any? %> + + "> + <%= I18n.t("meeting.email.updated.added_participants") %> + + "> + <%= @added_participants.join(", ") %> + + + <% end %> + <% if @removed_participants.any? %> + + "> + <%= I18n.t("meeting.email.updated.removed_participants") %> + + + <%= @removed_participants.join(", ") %> + + + <% end %> + <% if @meeting.participants.exists? %> "> diff --git a/modules/meeting/app/views/meeting_mailer/updated.text.erb b/modules/meeting/app/views/meeting_mailer/updated.text.erb index b524f43362f0..a1473f3c0402 100644 --- a/modules/meeting/app/views/meeting_mailer/updated.text.erb +++ b/modules/meeting/app/views/meeting_mailer/updated.text.erb @@ -69,3 +69,9 @@ See COPYRIGHT and LICENSE files for more details. <%= auto_link @changes[:new_location] %> <% end %> <% end %> +<% if @added_participants.any? %> +<%= I18n.t("meeting.email.updated.added_participants") %>: <%= @added_participants.join(", ") %> +<% end %> +<% if @removed_participants.any? %> +<%= I18n.t("meeting.email.updated.removed_participants") %>: <%= @removed_participants.join(", ") %> +<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb b/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb deleted file mode 100644 index 4c5d0522fd1e..000000000000 --- a/modules/meeting/app/views/meeting_series_mailer/participant_added.html.erb +++ /dev/null @@ -1,105 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= render layout: "mailer/spacer_table" do %> - <%= render partial: "mailer/mailer_header", - locals: { - user: @user, - summary: I18n.t("meeting.email.participant_added.summary_series", title: @series.title, actor: @actor, participant: @added_participants.join(", ")), - bottom_spacing: false - } %> - - <%= render layout: "mailer/border_table" do %> - - <%= placeholder_cell("24px", vertical: true) %> - - - - - - - <% if @template.location.present? %> - - - - - <% end %> - - - - - - - - - <% if @template.participants.exists? %> - - - - - <% end %> -
"> - <%= I18n.t(:label_recurring_meeting_schedule) %> - - <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) -
"> - <%= Meeting.human_attribute_name(:location) %> - - <%= auto_link @template.location %> -
"> - <%= Meeting.human_attribute_name(:project) %> - - <%= link_to @series.project.name, project_url(@series.project) %> -
"> - <%= Meeting.human_attribute_name(:author) %> - - <%= @series.author %> -
"> - <%= Meeting.human_attribute_name(:participants_invited) %> - - <%= @template.participants.invited.sort.join("; ") %> -
- - - <% end %> - - - - <%= placeholder_cell("20px", vertical: false) %> - -
- - <%= action_button do %> - <%= link_to I18n.t(:label_view_meeting_series), - recurring_meeting_url(@series), - target: "_blank", - rel: "noopener", - style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> - <% end %> -<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb b/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb deleted file mode 100644 index 0a76066721b0..000000000000 --- a/modules/meeting/app/views/meeting_series_mailer/participant_added.text.erb +++ /dev/null @@ -1,36 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= I18n.t("meeting.email.participant_added.summary_series", title: @series.title, actor: @actor, participant: @added_participants.join(", ")) %> - -<%= @series.project.name %>: <%= @series.title %> (<%= recurring_meeting_url(@series) %>) - -<%= t :label_recurring_meeting_schedule %>: <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) -<%= Meeting.human_attribute_name(:location) %>: <%= @template.location %> -<%= Meeting.human_attribute_name(:participants_invited) %>: <%= @template.participants.invited.sort.join("; ") %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb b/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb deleted file mode 100644 index 4680f2aba9a0..000000000000 --- a/modules/meeting/app/views/meeting_series_mailer/participant_removed.html.erb +++ /dev/null @@ -1,113 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= render layout: "mailer/spacer_table" do %> - <%= render partial: "mailer/mailer_header", - locals: { - user: @user, - summary: I18n.t("meeting.email.participant_removed.summary_series", title: @series.title, actor: @actor, participant: @removed_participants.join(", ")), - bottom_spacing: false - } %> - - <%= render layout: "mailer/border_table" do %> - - <%= placeholder_cell("24px", vertical: true) %> - - - - - - - <% if @template.location.present? %> - - - - - <% end %> - - - - - - - - - - - - - <% if @template.participants.exists? %> - - - - - <% end %> -
"> - <%= I18n.t(:label_recurring_meeting_schedule) %> - - <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) -
"> - <%= Meeting.human_attribute_name(:location) %> - - <%= auto_link @template.location %> -
"> - <%= Meeting.human_attribute_name(:project) %> - - <%= link_to @series.project.name, project_url(@series.project) %> -
"> - <%= Meeting.human_attribute_name(:author) %> - - <%= @series.author %> -
"> - <%= I18n.t("meeting.email.participant_removed.label") %> - - <%= @removed_participants.join(", ") %> -
"> - <%= Meeting.human_attribute_name(:participants_invited) %> - - <%= @template.participants.invited.sort.join("; ") %> -
- - - <% end %> - - - - <%= placeholder_cell("20px", vertical: false) %> - -
- - <%= action_button do %> - <%= link_to I18n.t(:label_view_meeting_series), - recurring_meeting_url(@series), - target: "_blank", - rel: "noopener", - style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> - <% end %> -<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb b/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb deleted file mode 100644 index c40ac7ad18c0..000000000000 --- a/modules/meeting/app/views/meeting_series_mailer/participant_removed.text.erb +++ /dev/null @@ -1,36 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= I18n.t("meeting.email.participant_removed.summary_series", title: @series.title, actor: @actor, participant: @removed_participants.join(", ")) %> - -<%= @series.project.name %>: <%= @series.title %> (<%= recurring_meeting_url(@series) %>) - -<%= t :label_recurring_meeting_schedule %>: <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) -<%= Meeting.human_attribute_name(:location) %>: <%= @template.location %> -<%= Meeting.human_attribute_name(:participants_invited) %>: <%= @template.participants.invited.sort.join("; ") %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb b/modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb deleted file mode 100644 index 0bb5cf1e6a60..000000000000 --- a/modules/meeting/app/views/meeting_series_mailer/participants_changed.html.erb +++ /dev/null @@ -1,115 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= render layout: "mailer/spacer_table" do %> - <%= render partial: "mailer/mailer_header", - locals: { - user: @user, - summary: I18n.t("meeting.email.participants_changed.summary_series", title: @series.title, actor: @actor), - bottom_spacing: false - } %> - - <%= render layout: "mailer/border_table" do %> - - <%= placeholder_cell("24px", vertical: true) %> - - - - - - - <% if @template.location.present? %> - - - - - <% end %> - - - - - - - - - <% if @removed_participants.any? %> - - - - - <% end %> - <% if @added_participants.any? %> - - - - - <% end %> -
"> - <%= I18n.t(:label_recurring_meeting_schedule) %> - - <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) -
"> - <%= Meeting.human_attribute_name(:location) %> - - <%= auto_link @template.location %> -
"> - <%= Meeting.human_attribute_name(:project) %> - - <%= link_to @series.project.name, project_url(@series.project) %> -
"> - <%= Meeting.human_attribute_name(:author) %> - - <%= @series.author %> -
"> - <%= I18n.t("meeting.email.participants_changed.removed") %> - - <%= @removed_participants.join(", ") %> -
"> - <%= I18n.t("meeting.email.participants_changed.added") %> - "> - <%= @added_participants.join(", ") %> -
- - - <% end %> - - - - <%= placeholder_cell("20px", vertical: false) %> - -
- - <%= action_button do %> - <%= link_to I18n.t(:label_view_meeting_series), - recurring_meeting_url(@series), - target: "_blank", - rel: "noopener", - style: "color: #333333; text-decoration: none; font-size: 14px;white-space: nowrap;" %> - <% end %> -<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb b/modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb deleted file mode 100644 index c6724de6c919..000000000000 --- a/modules/meeting/app/views/meeting_series_mailer/participants_changed.text.erb +++ /dev/null @@ -1,41 +0,0 @@ -<%#-- copyright -OpenProject is an open source project management software. -Copyright (C) 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. - -++#%> - -<%= I18n.t("meeting.email.participants_changed.summary_series", title: @series.title, actor: @actor) %> - -<%= @series.project.name %>: <%= @series.title %> (<%= recurring_meeting_url(@series) %>) - -<%= t :label_recurring_meeting_schedule %>: <%= @series.full_schedule_in_words %> (<%= formatted_time_zone_offset %>) -<%= Meeting.human_attribute_name(:location) %>: <%= @template.location %> -<% if @added_participants.any? %> -<%= I18n.t("meeting.email.participants_changed.added") %>: <%= @added_participants.join(", ") %> -<% end %> -<% if @removed_participants.any? %> -<%= I18n.t("meeting.email.participants_changed.removed") %>: <%= @removed_participants.join(", ") %> -<% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/updated.html.erb b/modules/meeting/app/views/meeting_series_mailer/updated.html.erb index 29c2f639d174..dfaac0692071 100644 --- a/modules/meeting/app/views/meeting_series_mailer/updated.html.erb +++ b/modules/meeting/app/views/meeting_series_mailer/updated.html.erb @@ -119,6 +119,27 @@ See COPYRIGHT and LICENSE files for more details. <%= @series.author %> + + <% if @added_participants.any? %> + + "> + <%= I18n.t("meeting.email.updated.added_participants") %> + + "> + <%= @added_participants.join(", ") %> + + + <% end %> + <% if @removed_participants.any? %> + + "> + <%= I18n.t("meeting.email.updated.removed_participants") %> + + + <%= @removed_participants.join(", ") %> + + + <% end %> diff --git a/modules/meeting/app/views/meeting_series_mailer/updated.text.erb b/modules/meeting/app/views/meeting_series_mailer/updated.text.erb index d6691b2414a4..398c2e918998 100644 --- a/modules/meeting/app/views/meeting_series_mailer/updated.text.erb +++ b/modules/meeting/app/views/meeting_series_mailer/updated.text.erb @@ -59,3 +59,9 @@ See COPYRIGHT and LICENSE files for more details. <%= auto_link @series.location %> <% end %> <% end %> +<% if @added_participants.any? %> +<%= I18n.t("meeting.email.updated.added_participants") %>: <%= @added_participants.join(", ") %> +<% end %> +<% if @removed_participants.any? %> +<%= I18n.t("meeting.email.updated.removed_participants") %>: <%= @removed_participants.join(", ") %> +<% end %> diff --git a/modules/meeting/config/locales/en.yml b/modules/meeting/config/locales/en.yml index 4b6c5ee3704b..19e14b5715da 100644 --- a/modules/meeting/config/locales/en.yml +++ b/modules/meeting/config/locales/en.yml @@ -291,24 +291,6 @@ en: summary_series: "Meeting series '%{title}' has been cancelled by %{actor}, or you have been removed as a participant" summary: "'%{title}' has been cancelled by %{actor}, or you have been removed as a participant" date_time: "Scheduled date/time" - participant_added: - header: "Meeting '%{title}' - Participant added" - header_series: "Meeting series '%{title}' - Participant added" - summary: "%{actor} added %{participant} to the meeting '%{title}'" - summary_series: "%{actor} added %{participant} to the meeting series '%{title}'" - participant_removed: - header: "Meeting '%{title}' - Participant removed" - header_series: "Meeting series '%{title}' - Participant removed" - summary: "%{actor} removed %{participant} from the meeting '%{title}'" - summary_series: "%{actor} removed %{participant} from the meeting series '%{title}'" - label: "Removed" - participants_changed: - header: "Meeting '%{title}' - Participants updated" - header_series: "Meeting series '%{title}' - Participants updated" - summary: "%{actor} updated the participants of the meeting '%{title}'" - summary_series: "%{actor} updated the participants of the meeting series '%{title}'" - added: "Added" - removed: "Removed" ended: header_series: "Ended: Meeting series '%{title}'" summary_series: "Meeting series '%{title}' has been ended by %{actor}" @@ -322,6 +304,8 @@ en: new_date_time: "New date/time" old_location: "Old location" new_location: "New location" + added_participants: "Added" + removed_participants: "Removed" label_mail_all_participants: "Send email invite to participants" types: one_time: "One-time" diff --git a/modules/meeting/spec/mailers/meeting_mailer_spec.rb b/modules/meeting/spec/mailers/meeting_mailer_spec.rb index 273f76db798d..108d9484a334 100644 --- a/modules/meeting/spec/mailers/meeting_mailer_spec.rb +++ b/modules/meeting/spec/mailers/meeting_mailer_spec.rb @@ -369,98 +369,46 @@ def find_calendar_part(message) end end - describe "participant_added" do - let(:added_participant_names) { ["New Participant", "Another Participant"] } - let(:mail) { described_class.participant_added(meeting, watcher1, author, added_participants: added_participant_names) } - - it "renders the headers" do - expect(mail.subject).to include(meeting.project.name) - expect(mail.subject).to include("Participant added") - expect(mail.to).to contain_exactly(watcher1.mail) - expect(mail.from).to eq([ApplicationMailer.reply_to_address]) - end - - it "renders the text body with participant info" do - User.execute_as(watcher1) do - expect(mail.text_part.body).to include(meeting.project.name) - expect(mail.text_part.body).to include(meeting.title) - expect(mail.text_part.body).to include(added_participant_names.first) - expect(mail.text_part.body).to include(author.name) - end - end - - it "renders the html body with participant info" do - User.execute_as(watcher1) do - expect(mail.html_part.body).to include(meeting.project.name) - expect(mail.html_part.body).to include(meeting.title) - expect(mail.html_part.body).to include(added_participant_names.first) - expect(mail.html_part.body).to include(author.name) - end - end - end - - describe "participant_removed" do - let(:removed_participant_names) { ["Removed Participant"] } - let(:mail) { described_class.participant_removed(meeting, watcher1, author, removed_participants: removed_participant_names) } - - it "renders the headers" do - expect(mail.subject).to include(meeting.project.name) - expect(mail.subject).to include("Participant removed") - expect(mail.to).to contain_exactly(watcher1.mail) - expect(mail.from).to eq([ApplicationMailer.reply_to_address]) - end - - it "renders the text body with participant info" do - User.execute_as(watcher1) do - expect(mail.text_part.body).to include(meeting.project.name) - expect(mail.text_part.body).to include(meeting.title) - expect(mail.text_part.body).to include(removed_participant_names.first) - expect(mail.text_part.body).to include(author.name) - end + describe "updated with participant changes" do + let(:meeting) do + create(:meeting, + author:, + project:, + start_time: "2021-11-09T23:00:00 +0100".to_datetime.utc) end - - it "renders the html body with participant info" do - User.execute_as(watcher1) do - expect(mail.html_part.body).to include(meeting.project.name) - expect(mail.html_part.body).to include(meeting.title) - expect(mail.html_part.body).to include(removed_participant_names.first) - expect(mail.html_part.body).to include(author.name) - end + let(:changes) do + { old_start: meeting.start_time, + new_start: meeting.start_time, + old_duration: 1, + new_duration: 1, + old_location: nil, + new_location: nil } end - end - - describe "participants_changed" do let(:added_names) { ["Added Person"] } let(:removed_names) { ["Removed Person"] } let(:mail) do - described_class.participants_changed(meeting, watcher1, author, - added_participants: added_names, - removed_participants: removed_names) + described_class.updated(meeting, watcher1, author, + changes:, + added_participants: added_names, + removed_participants: removed_names) end - it "renders the headers" do - expect(mail.subject).to include(meeting.project.name) - expect(mail.to).to contain_exactly(watcher1.mail) - expect(mail.from).to eq([ApplicationMailer.reply_to_address]) - end - - it "renders added and removed participants in the text body" do + it "renders added participants bold in the html body" do User.execute_as(watcher1) do - expect(mail.text_part.body).to include(added_names.first) - expect(mail.text_part.body).to include(removed_names.first) - expect(mail.text_part.body).to include(author.name) + expect(mail.html_part.body).to include(added_names.first) end end - it "renders added participants in the html body" do + it "renders removed participants with strikethrough in the html body" do User.execute_as(watcher1) do - expect(mail.html_part.body).to include(added_names.first) + expect(mail.html_part.body).to include("#{removed_names.first}") end end - it "renders removed participants with strikethrough in the html body" do + it "renders added and removed participants in the text body" do User.execute_as(watcher1) do - expect(mail.html_part.body).to include("#{removed_names.first}") + expect(mail.text_part.body).to include(added_names.first) + expect(mail.text_part.body).to include(removed_names.first) end end end diff --git a/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb b/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb index e8d9f8a0a5db..f3c834650ca5 100644 --- a/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb +++ b/modules/meeting/spec/mailers/meeting_series_mailer_spec.rb @@ -155,98 +155,33 @@ end end - describe "participant_added" do - let(:added_participant_names) { ["New Participant"] } - let(:mail) { described_class.participant_added(series, recipient, author, added_participants: added_participant_names) } - - it "renders the headers" do - expect(mail.subject).to include(series.project.name) - expect(mail.subject).to include("Participant added") - expect(mail.to).to contain_exactly(recipient.mail) - expect(mail.from).to eq([ApplicationMailer.reply_to_address]) - end - - it "renders the text body with participant info" do - User.execute_as(recipient) do - expect(mail.text_part.body).to include(series.project.name) - expect(mail.text_part.body).to include(series.title) - expect(mail.text_part.body).to include(added_participant_names.first) - expect(mail.text_part.body).to include(author.name) - end - end - - it "renders the html body with participant info" do - User.execute_as(recipient) do - expect(mail.html_part.body).to include(series.project.name) - expect(mail.html_part.body).to include(series.title) - expect(mail.html_part.body).to include(added_participant_names.first) - expect(mail.html_part.body).to include(author.name) - end - end - end - - describe "participant_removed" do - let(:removed_participant_names) { ["Removed Participant"] } - let(:mail) { described_class.participant_removed(series, recipient, author, removed_participants: removed_participant_names) } - - it "renders the headers" do - expect(mail.subject).to include(series.project.name) - expect(mail.subject).to include("Participant removed") - expect(mail.to).to contain_exactly(recipient.mail) - expect(mail.from).to eq([ApplicationMailer.reply_to_address]) - end - - it "renders the text body with participant info" do - User.execute_as(recipient) do - expect(mail.text_part.body).to include(series.project.name) - expect(mail.text_part.body).to include(series.title) - expect(mail.text_part.body).to include(removed_participant_names.first) - expect(mail.text_part.body).to include(author.name) - end - end - - it "renders the html body with participant info" do - User.execute_as(recipient) do - expect(mail.html_part.body).to include(series.project.name) - expect(mail.html_part.body).to include(series.title) - expect(mail.html_part.body).to include(removed_participant_names.first) - expect(mail.html_part.body).to include(author.name) - end - end - end - - describe "participants_changed" do + describe "updated with participant changes" do + let(:changes) { { old_schedule: "some old schedule", old_location: "some old location" } } let(:added_names) { ["Added Person"] } let(:removed_names) { ["Removed Person"] } let(:mail) do - described_class.participants_changed(series, recipient, author, - added_participants: added_names, - removed_participants: removed_names) - end - - it "renders the headers" do - expect(mail.subject).to include(series.project.name) - expect(mail.to).to contain_exactly(recipient.mail) - expect(mail.from).to eq([ApplicationMailer.reply_to_address]) + described_class.updated(series, recipient, author, + changes:, + added_participants: added_names, + removed_participants: removed_names) end - it "renders added and removed participants in the text body" do + it "renders added participants bold in the html body" do User.execute_as(recipient) do - expect(mail.text_part.body).to include(added_names.first) - expect(mail.text_part.body).to include(removed_names.first) - expect(mail.text_part.body).to include(author.name) + expect(mail.html_part.body).to include(added_names.first) end end - it "renders added participants in the html body" do + it "renders removed participants with strikethrough in the html body" do User.execute_as(recipient) do - expect(mail.html_part.body).to include(added_names.first) + expect(mail.html_part.body).to include("#{removed_names.first}") end end - it "renders removed participants with strikethrough in the html body" do + it "renders added and removed participants in the text body" do User.execute_as(recipient) do - expect(mail.html_part.body).to include("#{removed_names.first}") + expect(mail.text_part.body).to include(added_names.first) + expect(mail.text_part.body).to include(removed_names.first) end end end From cc73ab68eccf134b68e1b7f380c6f014549038de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 7 May 2026 16:48:00 +0200 Subject: [PATCH 4/5] Also pass previous attribute changes so that we can correctly diff them --- modules/meeting/app/models/meeting.rb | 14 +- ...spatch_aggregated_notifications_service.rb | 89 +++++++--- .../meetings/notification_debounce_job.rb | 31 ++-- ...h_aggregated_notifications_service_spec.rb | 154 ++++++++++-------- .../notification_debounce_job_spec.rb | 16 +- 5 files changed, 194 insertions(+), 110 deletions(-) diff --git a/modules/meeting/app/models/meeting.rb b/modules/meeting/app/models/meeting.rb index c2ef60f69771..81a32364bc98 100644 --- a/modules/meeting/app/models/meeting.rb +++ b/modules/meeting/app/models/meeting.rb @@ -347,6 +347,18 @@ def add_new_participants_as_watcher def send_updated_mail return unless send_emails? - Meetings::NotificationDebounceJob.debounce(self, since_journal_id: last_journal&.predecessor&.id) + Meetings::NotificationDebounceJob.debounce( + self, + since_journal_id: last_journal&.predecessor&.id, + since_invited_ids: participants.invited.pluck(:user_id), + since_attributes: updated_mail_since_attributes + ) + end + + def updated_mail_since_attributes + %w[title location start_time duration].index_with do |attribute| + value = saved_change_to_attribute(attribute)&.first || public_send(attribute) + value.respond_to?(:iso8601) ? value.iso8601 : value + end end end diff --git a/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb b/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb index ea3f56dbf95d..0c07604d107d 100644 --- a/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb +++ b/modules/meeting/app/services/meetings/dispatch_aggregated_notifications_service.rb @@ -36,49 +36,67 @@ module Meetings # handled by RecurringMeetings::UpdateService directly (it has the full context only then). # This service therefore only dispatches participant-change emails for series templates and one-time meetings class DispatchAggregatedNotificationsService - attr_reader :meeting, :since_journal, :latest_journal + attr_reader :meeting, :since_journal, :latest_journal, :actor, :added_user_ids, + :removed_user_ids, :still_invited_ids, :users_by_id, :added_names, + :removed_names, :attribute_changes - def initialize(meeting:, since_journal:, latest_journal:, since_invited_ids: nil) + def initialize(meeting:, since_journal:, latest_journal:, since_invited_ids: nil, since_attributes: nil) @meeting = meeting @since_journal = since_journal @latest_journal = latest_journal @since_invited_ids_override = since_invited_ids + @since_attributes_override = normalize_since_attributes(since_attributes) end def call return unless Journal::NotificationConfiguration.active? && meeting.send_emails? - actor = latest_journal.user + prepare_changes - since_invited_ids = @since_invited_ids_override || invited_user_ids_from(since_journal) - latest_invited_ids = invited_user_ids_from(latest_journal) + send_direct_notifications + send_update_notifications if update_notifications? + end - added_user_ids = latest_invited_ids - since_invited_ids - removed_user_ids = since_invited_ids - latest_invited_ids - still_invited_ids = latest_invited_ids & since_invited_ids + private - all_ids = (added_user_ids + removed_user_ids + still_invited_ids).uniq - users_by_id = User.where(id: all_ids).index_by(&:id) + def prepare_changes # rubocop:disable Metrics/AbcSize + @actor = latest_journal.user - added_names = added_user_ids.filter_map { users_by_id[it]&.name } - removed_names = removed_user_ids.filter_map { users_by_id[it]&.name } + prepare_participant_changes + @users_by_id = User.where(id: changed_user_ids).index_by(&:id) - attribute_changes = meeting.template? ? {} : compute_attribute_changes + @added_names = added_user_ids.filter_map { users_by_id[it]&.name } + @removed_names = removed_user_ids.filter_map { users_by_id[it]&.name } + @attribute_changes = meeting.template? ? {} : compute_attribute_changes + end - added_user_ids.each { |uid| send_invite(users_by_id[uid], actor) } - removed_user_ids.each { |uid| send_cancellation(users_by_id[uid], actor) } + def prepare_participant_changes + since_invited_ids = @since_invited_ids_override || invited_user_ids_from(since_journal) + latest_invited_ids = invited_user_ids_from(latest_journal) - return if attribute_changes.empty? && added_names.empty? && removed_names.empty? + @added_user_ids = latest_invited_ids - since_invited_ids + @removed_user_ids = since_invited_ids - latest_invited_ids + @still_invited_ids = latest_invited_ids & since_invited_ids + end - still_invited_ids.each do |uid| - recipient = users_by_id[uid] - next unless recipient + def changed_user_ids + (added_user_ids + removed_user_ids + still_invited_ids).uniq + end + def send_direct_notifications + added_user_ids.each { |uid| send_invite(users_by_id[uid], actor) } + removed_user_ids.each { |uid| send_cancellation(users_by_id[uid], actor) } + end + + def send_update_notifications + still_invited_ids.filter_map { |uid| users_by_id[uid] }.each do |recipient| send_updated(recipient, actor, attribute_changes, added_names:, removed_names:) end end - private + def update_notifications? + attribute_changes.any? || added_names.any? || removed_names.any? + end def invited_user_ids_from(journal) return [] unless journal @@ -87,23 +105,38 @@ def invited_user_ids_from(journal) end def compute_attribute_changes - return {} unless since_journal - - since_data = since_journal.data latest_data = latest_journal.data - return {} unless since_data && latest_data + return {} unless latest_data + + since_attributes = @since_attributes_override || attributes_from_journal(since_journal) + return {} unless since_attributes changes = {} %i[title location start_time duration].each do |attr| - next unless since_data.respond_to?(attr) && latest_data.respond_to?(attr) + next unless since_attributes.key?(attr.to_s) && latest_data.respond_to?(attr) - old_val = since_data.send(attr) + old_val = since_attributes[attr.to_s] new_val = latest_data.send(attr) changes[attr] = [old_val, new_val] if old_val != new_val end changes end + def attributes_from_journal(journal) + data = journal&.data + return unless data + + %w[title location start_time duration].index_with { |attr| data.public_send(attr) } + end + + def normalize_since_attributes(attributes) + attributes = attributes&.stringify_keys + return unless attributes + + attributes["start_time"] = Time.zone.parse(attributes["start_time"]) if attributes["start_time"].is_a?(String) + attributes + end + def send_invite(recipient, actor) return unless recipient @@ -149,11 +182,13 @@ def send_meeting_updated(recipient, actor, attribute_changes, added_names:, remo end def meeting_changes(attribute_changes) # rubocop:disable Metrics/AbcSize + title = attribute_changes[:title] || [meeting.title, meeting.title] start_time = attribute_changes[:start_time] || [meeting.start_time, meeting.start_time] duration = attribute_changes[:duration] || [meeting.duration, meeting.duration] location = attribute_changes[:location] || [meeting.location, meeting.location] - { old_start: start_time[0], new_start: start_time[1], + { old_title: title[0], new_title: title[1], + old_start: start_time[0], new_start: start_time[1], old_duration: duration[0], new_duration: duration[1], old_location: location[0], new_location: location[1] } end diff --git a/modules/meeting/app/workers/meetings/notification_debounce_job.rb b/modules/meeting/app/workers/meetings/notification_debounce_job.rb index 8e1174f48a75..2c8180ee0f46 100644 --- a/modules/meeting/app/workers/meetings/notification_debounce_job.rb +++ b/modules/meeting/app/workers/meetings/notification_debounce_job.rb @@ -54,21 +54,19 @@ def self.unique_key_for(meeting_id) "Meetings::NotificationDebounceJob-#{meeting_id}" end - # +since_invited_ids+ is an explicit array of invited user IDs representing the - # participant list BEFORE the current debounce window started. Callers that - # know them (CreateService, DeleteService) pass it here so the - # dispatch service can correctly diff participants even when journal aggregation - # has overwritten the predecessor journal in-place. - def self.debounce(meeting, since_journal_id: nil, since_invited_ids: nil) + # Since any changes after the since_journal_id might CHANGE this journal, + # we pass in the known values at the current time for the initial call. + # They are needed so that we can know the true "previous" values at the end of the debounce window. + def self.debounce(meeting, since_journal_id: nil, since_invited_ids: nil, since_attributes: nil) concurrency_key = unique_key_for(meeting.id) existing = GoodJob::Job.where(finished_at: nil, concurrency_key:).first - args = preserved_job_args(existing, meeting, since_journal_id, since_invited_ids) + args = preserved_job_args(existing, meeting, since_journal_id, since_invited_ids, since_attributes) GoodJob::Job.where(finished_at: nil, concurrency_key:).delete_all set(wait: 1.minute).perform_later(meeting.id, *args) end - def perform(meeting_id, since_journal_id, since_invited_ids = nil) + def perform(meeting_id, since_journal_id, since_invited_ids = nil, since_attributes = nil) meeting = Meeting.find_by(id: meeting_id) return unless meeting return unless meeting.send_emails? @@ -77,10 +75,10 @@ def perform(meeting_id, since_journal_id, since_invited_ids = nil) latest_journal = meeting.last_journal return if latest_journal.nil? - return if latest_journal.id == since_journal_id + return if latest_journal.id == since_journal_id && since_invited_ids.nil? && since_attributes.nil? Meetings::DispatchAggregatedNotificationsService - .new(meeting:, since_journal:, latest_journal:, since_invited_ids:) + .new(meeting:, since_journal:, latest_journal:, since_invited_ids:, since_attributes:) .call end @@ -88,19 +86,20 @@ def self.cancel_pending(meeting) GoodJob::Job.where(finished_at: nil, concurrency_key: unique_key_for(meeting.id)).delete_all end - # Extracts since_journal_id, since_invited_ids from the existing pending job + # Extracts baseline arguments from the existing pending job # Uses the following order: previous job args > explicit caller arg > journal predecessor. - def self.preserved_job_args(existing, meeting, since_journal_id, since_invited_ids) - existing_journal_id, existing_invited_ids = serialized_job_args(existing) + def self.preserved_job_args(existing, meeting, since_journal_id, since_invited_ids, since_attributes) + existing_journal_id, existing_invited_ids, existing_attributes = serialized_job_args(existing) [ existing_journal_id || since_journal_id || meeting.last_journal&.predecessor&.id, - existing_invited_ids || since_invited_ids + existing_invited_ids || since_invited_ids, + existing_attributes || since_attributes ] end def self.serialized_job_args(existing) - args = existing&.serialized_params&.dig("arguments") || [] - [args[1], args[2]] + args = ActiveJob::Arguments.deserialize(existing&.serialized_params&.dig("arguments") || []) + [args[1], args[2], args[3]] end end end diff --git a/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb b/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb index 78f6433f214f..4a2e5f4aa23d 100644 --- a/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb +++ b/modules/meeting/spec/services/meetings/dispatch_aggregated_notifications_service_spec.rb @@ -49,14 +49,24 @@ let(:since_participant_journals) { class_double(Journal::MeetingParticipantJournal) } let(:latest_participant_journals) { class_double(Journal::MeetingParticipantJournal) } + let(:mail_delivery) { double(deliver_later: nil) } before do - allow(since_journal).to receive(:participant_journals).and_return(since_participant_journals) + allow(since_journal).to receive(:participant_journals).and_return(since_participant_journals) if since_journal allow(latest_journal).to receive(:participant_journals).and_return(latest_participant_journals) allow(since_participant_journals).to receive(:where).with(invited: true).and_return(since_participant_journals) allow(latest_participant_journals).to receive(:where).with(invited: true).and_return(latest_participant_journals) allow(Journal::NotificationConfiguration).to receive(:active?).and_return(true) + allow(MeetingMailer).to receive_messages( + invited: mail_delivery, + cancelled: mail_delivery, + updated: mail_delivery + ) + allow(MeetingSeriesMailer).to receive_messages( + invited: mail_delivery, + updated: mail_delivery + ) end subject(:service) do @@ -74,23 +84,22 @@ def stub_invited_ids(journal_double, user_ids) end it "sends invite to the newly added user" do - expect(MeetingMailer).to receive(:invited).with(meeting, new_user, actor).and_return(double(deliver_later: nil)) service + expect(MeetingMailer).to have_received(:invited).with(meeting, new_user, actor) end - it "sends participant_added to the existing user" do - allow(MeetingMailer).to receive(:invited).and_return(double(deliver_later: nil)) - expect(MeetingMailer) - .to receive(:participant_added) - .with(meeting, existing_user, actor, added_participants: ["#{new_user.firstname} #{new_user.lastname}"]) - .and_return(double(deliver_later: nil)) + it "sends an updated email with the added participant to the existing user" do service + expect(MeetingMailer) + .to have_received(:updated) + .with(meeting, existing_user, actor, + hash_including(added_participants: [new_user.name], + removed_participants: [])) end - it "does not send participant_added to the newly added user" do - allow(MeetingMailer).to receive_messages(invited: double(deliver_later: nil), participant_added: double(deliver_later: nil)) - expect(MeetingMailer).not_to receive(:participant_added).with(meeting, new_user, anything, anything) + it "does not send an updated email to the newly added user" do service + expect(MeetingMailer).not_to have_received(:updated).with(meeting, new_user, anything, anything) end end @@ -101,17 +110,19 @@ def stub_invited_ids(journal_double, user_ids) end it "sends cancellation to the removed user" do - expect(MeetingMailer).to receive(:cancelled).with(meeting, removed_user, actor).and_return(double(deliver_later: nil)) service + expect(MeetingMailer) + .to have_received(:cancelled) + .with(meeting, removed_user, actor) end - it "sends participant_removed to the still-invited user" do - allow(MeetingMailer).to receive(:cancelled).and_return(double(deliver_later: nil)) - expect(MeetingMailer) - .to receive(:participant_removed) - .with(meeting, existing_user, actor, removed_participants: ["#{removed_user.firstname} #{removed_user.lastname}"]) - .and_return(double(deliver_later: nil)) + it "sends an updated email with the removed participant to the still-invited user" do service + expect(MeetingMailer) + .to have_received(:updated) + .with(meeting, existing_user, actor, + hash_including(added_participants: [], + removed_participants: [removed_user.name])) end end @@ -119,25 +130,14 @@ def stub_invited_ids(journal_double, user_ids) before do stub_invited_ids(since_participant_journals, [existing_user.id, removed_user.id]) stub_invited_ids(latest_participant_journals, [existing_user.id, new_user.id]) - allow(MeetingMailer).to receive_messages(invited: double(deliver_later: nil), - cancelled: double(deliver_later: nil), - participant_added: double(deliver_later: nil), - participant_removed: double(deliver_later: nil), - participants_changed: double(deliver_later: nil)) end - it "sends a single participants_changed email to the still-invited user" do + it "sends a single updated email with participant changes to the still-invited user" do service - expect(MeetingMailer).to have_received(:participants_changed) + expect(MeetingMailer).to have_received(:updated) .with(meeting, existing_user, actor, - added_participants: ["#{new_user.firstname} #{new_user.lastname}"], - removed_participants: ["#{removed_user.firstname} #{removed_user.lastname}"]) - end - - it "does not send separate participant_added and participant_removed emails" do - service - expect(MeetingMailer).not_to have_received(:participant_added) - expect(MeetingMailer).not_to have_received(:participant_removed) + hash_including(added_participants: [new_user.name], + removed_participants: [removed_user.name])) end end @@ -148,11 +148,10 @@ def stub_invited_ids(journal_double, user_ids) end it "sends no emails" do - expect(MeetingMailer).not_to receive(:invited) - expect(MeetingMailer).not_to receive(:cancelled) - expect(MeetingMailer).not_to receive(:participant_added) - expect(MeetingMailer).not_to receive(:participant_removed) service + expect(MeetingMailer).not_to have_received(:invited) + expect(MeetingMailer).not_to have_received(:cancelled) + expect(MeetingMailer).not_to have_received(:updated) end end @@ -167,11 +166,36 @@ def stub_invited_ids(journal_double, user_ids) end it "sends updated email to the still-invited user" do + service expect(MeetingMailer) - .to receive(:updated) + .to have_received(:updated) .with(meeting, existing_user, actor, hash_including(changes: hash_including(:old_start, :new_start))) - .and_return(double(deliver_later: nil)) + end + end + + context "when explicit since_attributes are provided" do + subject(:service) do + described_class.new( + meeting:, + since_journal: nil, + latest_journal:, + since_invited_ids: [existing_user.id], + since_attributes: { "title" => "Old Title", "start_time" => fixed_start_time.iso8601 } + ).call + end + + before do + stub_invited_ids(latest_participant_journals, [existing_user.id]) + allow(latest_data).to receive(:title).and_return("New Title") + end + + it "uses them as the baseline for the updated email" do service + expect(MeetingMailer) + .to have_received(:updated) + .with(meeting, existing_user, actor, + hash_including(changes: hash_including(old_title: "Old Title", + new_title: "New Title"))) end end @@ -182,12 +206,10 @@ def stub_invited_ids(journal_double, user_ids) end it "sends no emails" do - expect(MeetingMailer).not_to receive(:invited) - expect(MeetingMailer).not_to receive(:cancelled) - expect(MeetingMailer).not_to receive(:updated) - expect(MeetingMailer).not_to receive(:participant_added) - expect(MeetingMailer).not_to receive(:participant_removed) service + expect(MeetingMailer).not_to have_received(:invited) + expect(MeetingMailer).not_to have_received(:cancelled) + expect(MeetingMailer).not_to have_received(:updated) end end @@ -199,8 +221,10 @@ def stub_invited_ids(journal_double, user_ids) end it "treats all latest participants as newly added" do - expect(MeetingMailer).to receive(:invited).with(meeting, new_user, actor).and_return(double(deliver_later: nil)) service + expect(MeetingMailer) + .to have_received(:invited) + .with(meeting, new_user, actor) end end @@ -216,7 +240,6 @@ def stub_invited_ids(journal_double, user_ids) before do stub_invited_ids(latest_participant_journals, [existing_user.id, new_user.id]) - allow(MeetingMailer).to receive_messages(invited: double(deliver_later: nil), participant_added: double(deliver_later: nil)) end it "sends invite only to the newly added user (not the existing one)" do @@ -225,11 +248,13 @@ def stub_invited_ids(journal_double, user_ids) expect(MeetingMailer).not_to have_received(:invited).with(meeting, existing_user, anything) end - it "sends participant_added to the existing user" do + it "sends an updated email with the added participant to the existing user" do service expect(MeetingMailer) - .to have_received(:participant_added) - .with(meeting, existing_user, actor, added_participants: ["#{new_user.firstname} #{new_user.lastname}"]) + .to have_received(:updated) + .with(meeting, existing_user, actor, + hash_including(added_participants: [new_user.name], + removed_participants: [])) end end @@ -244,27 +269,28 @@ def stub_invited_ids(journal_double, user_ids) end it "sends series invite via MeetingSeriesMailer" do + service expect(MeetingSeriesMailer) - .to receive(:invited) + .to have_received(:invited) .with(recurring_meeting, new_user, actor) - .and_return(double(deliver_later: nil)) - service end - it "sends participant_added via MeetingSeriesMailer" do - allow(MeetingSeriesMailer).to receive(:invited).and_return(double(deliver_later: nil)) - expect(MeetingSeriesMailer) - .to receive(:participant_added) - .with(recurring_meeting, existing_user, actor, added_participants: anything) - .and_return(double(deliver_later: nil)) + it "sends updated via MeetingSeriesMailer with the added participant" do service + expect(MeetingSeriesMailer) + .to have_received(:updated) + .with(recurring_meeting, existing_user, actor, + hash_including(added_participants: [new_user.name], + removed_participants: [])) end - it "does not send updated even when attributes changed" do - allow(MeetingSeriesMailer).to receive_messages(invited: double(deliver_later: nil), - participant_added: double(deliver_later: nil)) - expect(MeetingSeriesMailer).not_to receive(:updated) + it "does not send meeting attribute changes via MeetingSeriesMailer" do service + expect(MeetingSeriesMailer) + .to have_received(:updated) + .with(recurring_meeting, existing_user, actor, + hash_including(changes: { old_schedule: recurring_meeting.full_schedule_in_words, + old_location: nil })) end end @@ -274,8 +300,8 @@ def stub_invited_ids(journal_double, user_ids) end it "sends no emails" do - expect(MeetingMailer).not_to receive(:invited) service + expect(MeetingMailer).not_to have_received(:invited) end end @@ -285,8 +311,8 @@ def stub_invited_ids(journal_double, user_ids) end it "sends no emails" do - expect(MeetingMailer).not_to receive(:invited) service + expect(MeetingMailer).not_to have_received(:invited) end end end diff --git a/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb b/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb index fc63035c5587..fcbc39e426c8 100644 --- a/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb +++ b/modules/meeting/spec/workers/meetings/notification_debounce_job_spec.rb @@ -48,7 +48,7 @@ it "enqueues a background job" do expect { described_class.debounce(meeting) } .to have_enqueued_job(described_class) - .with(meeting.id, anything, anything) + .with(meeting.id, anything, anything, anything) end it "does not call the dispatch service synchronously" do @@ -94,6 +94,14 @@ job = GoodJob::Job.where(finished_at: nil, concurrency_key: described_class.unique_key_for(meeting.id)).first expect(job.serialized_params.dig("arguments", 1)).to eq(first_since_id) end + + it "preserves explicit since_attributes from the first call" do + described_class.debounce(meeting, since_attributes: { "title" => "Initial title" }) + described_class.debounce(meeting, since_attributes: { "title" => "Later title" }) + + job = GoodJob::Job.where(finished_at: nil, concurrency_key: described_class.unique_key_for(meeting.id)).first + expect(job.serialized_params.dig("arguments", 3, "title")).to eq("Initial title") + end end end end @@ -128,7 +136,11 @@ subject expect(Meetings::DispatchAggregatedNotificationsService) .to have_received(:new) - .with(meeting:, since_journal:, latest_journal:, since_invited_ids: nil) + .with(meeting:, + since_journal:, + latest_journal:, + since_invited_ids: nil, + since_attributes: nil) expect(dispatch_service).to have_received(:call) end From 18d78a7fe50d136adaf26bd8673b5fe51c52b338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Fri, 8 May 2026 10:14:33 +0200 Subject: [PATCH 5/5] Adapt feature specs for debounce job --- .../features/meeting_notifications_spec.rb | 34 +++++++++++-------- .../recurring_meeting_create_spec.rb | 10 ++++-- .../structured_meeting_participant_spec.rb | 8 +++-- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/modules/meeting/spec/features/meeting_notifications_spec.rb b/modules/meeting/spec/features/meeting_notifications_spec.rb index 10d989aa1dab..8c5fb13f8cb8 100644 --- a/modules/meeting/spec/features/meeting_notifications_spec.rb +++ b/modules/meeting/spec/features/meeting_notifications_spec.rb @@ -46,6 +46,10 @@ login_as(user) end + def perform_debounced_meeting_notification_jobs + perform_enqueued_jobs(at: 2.minutes.from_now) + end + shared_examples "notification checkbox behaviour" do it "shows checkbox checked initially" do page.find_by_id("open-meeting-button").click @@ -104,7 +108,7 @@ wait_for_network_idle # check if mail is sent on opening meeting (Bug #70109) - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -130,7 +134,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -183,7 +187,7 @@ expect_flash(message: "Email calendar update sent to all participants") - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -199,7 +203,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 0 show_page.trigger_dropdown_menu_item "Delete meeting" @@ -211,7 +215,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 0 end end @@ -263,7 +267,7 @@ wait_for_network_idle # check if mail is sent on opening first meeting - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -293,7 +297,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -324,7 +328,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -369,7 +373,7 @@ wait_for_network_idle expect_flash(type: :success, message: "Successful deletion.") - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 0 end @@ -380,7 +384,7 @@ wait_for_network_idle # check for initial invitation mail - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 ActionMailer::Base.deliveries.clear @@ -414,7 +418,7 @@ expect(meeting.template.reload.notify).to be true # check for invitation mail on re-enabling notifications - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 1 end end @@ -498,7 +502,7 @@ show_page.expect_participant(third_user) end - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs # 1 to the invited user + 2 to the existing participants expect(ActionMailer::Base.deliveries.size).to eq 3 @@ -520,7 +524,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs # 1 to the removed user + 2 to the existing participants expect(ActionMailer::Base.deliveries.size).to eq 3 @@ -570,7 +574,7 @@ show_page.expect_participant(third_user, editable: false) end - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs # apply_to_upcoming is enabled by default on templates. # 3 mails for template (invite + 2 participant_added) and @@ -599,7 +603,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs # 1 to the removed user + 2 to the existing participants expect(ActionMailer::Base.deliveries.size).to eq 3 diff --git a/modules/meeting/spec/features/recurring_meetings/recurring_meeting_create_spec.rb b/modules/meeting/spec/features/recurring_meetings/recurring_meeting_create_spec.rb index f729b87b8bb7..706c05b31e92 100644 --- a/modules/meeting/spec/features/recurring_meetings/recurring_meeting_create_spec.rb +++ b/modules/meeting/spec/features/recurring_meetings/recurring_meeting_create_spec.rb @@ -78,6 +78,10 @@ travel_back end + def perform_debounced_meeting_notification_jobs + perform_enqueued_jobs(at: 2.minutes.from_now) + end + context "with a user with permissions" do it "can create a recurring meeting" do login_as current_user @@ -125,7 +129,7 @@ expect(page).to have_css("#meetings-side-panel-participants-component", text: 2) - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 0 # Before exiting draft mode, user is always redirected to the template @@ -154,7 +158,7 @@ show_page.expect_planned_meeting date: "01/07/2025 01:30 PM" show_page.expect_planned_meeting date: "01/14/2025 01:30 PM" - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 2 expect(ActionMailer::Base.deliveries.map(&:to).flatten) .to contain_exactly user.mail, other_user.mail @@ -182,7 +186,7 @@ expect(page).to have_css("#meetings-side-panel-participants-component", text: 3) - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 3 expect(ActionMailer::Base.deliveries.first.to).to contain_exactly(third_user.mail) end diff --git a/modules/meeting/spec/features/structured_meetings/structured_meeting_participant_spec.rb b/modules/meeting/spec/features/structured_meetings/structured_meeting_participant_spec.rb index a6a8d5492098..21cc7b902ab3 100644 --- a/modules/meeting/spec/features/structured_meetings/structured_meeting_participant_spec.rb +++ b/modules/meeting/spec/features/structured_meetings/structured_meeting_participant_spec.rb @@ -36,6 +36,10 @@ :js do include Components::Autocompleter::NgSelectAutocompleteHelpers + def perform_debounced_meeting_notification_jobs + perform_enqueued_jobs(at: 2.minutes.from_now) + end + shared_let(:project) { create(:project, enabled_module_names: %w[meetings work_package_tracking]) } shared_let(:user) do create(:user, @@ -114,7 +118,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 2 ActionMailer::Base.deliveries.clear @@ -124,7 +128,7 @@ wait_for_network_idle - perform_enqueued_jobs + perform_debounced_meeting_notification_jobs expect(ActionMailer::Base.deliveries.size).to eq 2 end