Skip to content

Commit

Permalink
[57251] Fix mail_alert_sent not being set correctly on update
Browse files Browse the repository at this point in the history
https://community.openproject.org/wp/57251

When a notification exists for a work package journal for a reason other
than :mentioned, then its `mail_alert_sent` value is `nil`.

On a subsequent update of the work package which mentions the user, the
existing notification is updated. In this update, the `mail_alert_sent`
value must be changed from `nil` to `false` if the reason is :mentioned.
Without it, the immediate notification email for the mention would not
be sent.
  • Loading branch information
cbliard authored and oliverguenther committed Oct 15, 2024
1 parent b0ad254 commit 1ad3b4e
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
1 change: 1 addition & 0 deletions app/services/notifications/create_from_model_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def update_notification(recipient_id, reason)
Notifications::UpdateService
.new(model: existing_notification, user:, contract_class: EmptyContract)
.call(read_ian: strategy.supports_ian?(reason) ? false : nil,
mail_alert_sent: existing_notification.mail_alert_sent || (strategy.supports_mail?(reason) ? false : nil),
reason:)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@
context "when there is already a notification for the journal (because it was aggregated)" do
let(:note) { "Hello user:\"#{recipient_login}\"" }
let!(:existing_notification) do
create(:notification, resource:, journal:, recipient:, reason: :mentioned, read_ian: true)
create(:notification, resource:, journal:, recipient:, reason: :mentioned, read_ian: true, mail_alert_sent: nil)
end

it_behaves_like "creates no notification"
Expand All @@ -1006,7 +1006,27 @@
call

expect(existing_notification.reload.read_ian)
.to be_falsey
.to be(false)
end

it "changes the mail_alert_sent of the existing notification from nil to false" do
call

expect(existing_notification.reload.mail_alert_sent)
.to be(false)
end

context "and the mail alert has already been sent" do
before do
existing_notification.update(mail_alert_sent: true)
end

it "keeps the mail_alert_sent of the existing notification to true" do
call

expect(existing_notification.reload.mail_alert_sent)
.to be(true)
end
end
end

Expand Down

0 comments on commit 1ad3b4e

Please sign in to comment.