From 284d5232effb83a2e6aa7e8e3b33e6bcdbffe719 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 24 Mar 2026 18:59:47 +0000 Subject: [PATCH] Deliver diary comment notifications using Noticed --- app/controllers/diary_comments_controller.rb | 4 +--- app/mailers/user_mailer.rb | 2 +- app/models/diary_comment.rb | 4 ++++ app/models/diary_entry.rb | 4 ++++ app/notifiers/diary_comment_notifier.rb | 12 ++++++++++++ test/mailers/previews/user_mailer_preview.rb | 2 +- test/mailers/user_mailer_test.rb | 2 +- test/models/diary_comment_test.rb | 15 +++++++++++++++ test/models/diary_entry_test.rb | 12 ++++++++++++ 9 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 app/notifiers/diary_comment_notifier.rb diff --git a/app/controllers/diary_comments_controller.rb b/app/controllers/diary_comments_controller.rb index 1b4b4c370..c707035dc 100644 --- a/app/controllers/diary_comments_controller.rb +++ b/app/controllers/diary_comments_controller.rb @@ -21,9 +21,7 @@ class DiaryCommentsController < ApplicationController if @diary_comment.save # Notify current subscribers of the new comment - @diary_entry.subscribers.visible.each do |user| - UserMailer.with(:comment => @diary_comment, :recipient => user).diary_comment_notification.deliver_later if current_user != user - end + DiaryCommentNotifier.with(:record => @diary_comment).deliver_later # Add the commenter to the subscribers if necessary @diary_entry.subscriptions.create(:user => current_user) unless @diary_entry.subscribers.exists?(current_user.id) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 567cbe459..96d027af7 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -105,7 +105,7 @@ class UserMailer < ApplicationMailer end def diary_comment_notification - comment, recipient = params.fetch_values(:comment, :recipient) + comment, recipient = params.fetch_values(:record, :recipient) with_recipient_locale recipient do @to_user = recipient.display_name diff --git a/app/models/diary_comment.rb b/app/models/diary_comment.rb index 5a7ddeb22..1422a87ec 100644 --- a/app/models/diary_comment.rb +++ b/app/models/diary_comment.rb @@ -48,6 +48,10 @@ class DiaryComment < ApplicationRecord Base64.urlsafe_encode64(sha256.digest)[0, 8] end + def notifiable_subscribers + diary_entry.visible_subscribers.where.not(:id => user_id) + end + private def spam_check diff --git a/app/models/diary_entry.rb b/app/models/diary_entry.rb index ae0f8e6c3..924ef41f8 100644 --- a/app/models/diary_entry.rb +++ b/app/models/diary_entry.rb @@ -55,6 +55,10 @@ class DiaryEntry < ApplicationRecord @body ||= RichText.new(self[:body_format], self[:body]) end + def visible_subscribers + subscribers.visible + end + private def spam_check diff --git a/app/notifiers/diary_comment_notifier.rb b/app/notifiers/diary_comment_notifier.rb new file mode 100644 index 000000000..afe992ab6 --- /dev/null +++ b/app/notifiers/diary_comment_notifier.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class DiaryCommentNotifier < ApplicationNotifier + recipients -> { record.notifiable_subscribers } + + validates :record, :presence => true + + deliver_by :email do |config| + config.mailer = "UserMailer" + config.method = "diary_comment_notification" + end +end diff --git a/test/mailers/previews/user_mailer_preview.rb b/test/mailers/previews/user_mailer_preview.rb index 0fa9db335..bf4023e4f 100644 --- a/test/mailers/previews/user_mailer_preview.rb +++ b/test/mailers/previews/user_mailer_preview.rb @@ -63,7 +63,7 @@ class UserMailerPreview < ActionMailer::Preview recipient = create(:user, :languages => [I18n.locale]) diary_entry = create(:diary_entry) diary_comment = create(:diary_comment, :diary_entry => diary_entry) - UserMailer.with(:comment => diary_comment, :recipient => recipient).diary_comment_notification + UserMailer.with(:record => diary_comment, :recipient => recipient).diary_comment_notification end def follow_notification diff --git a/test/mailers/user_mailer_test.rb b/test/mailers/user_mailer_test.rb index 13152a0dc..15e64f935 100644 --- a/test/mailers/user_mailer_test.rb +++ b/test/mailers/user_mailer_test.rb @@ -103,7 +103,7 @@ class UserMailerTest < ActionMailer::TestCase other_user = create(:user) diary_entry = create(:diary_entry, :user => user) diary_comment = create(:diary_comment, :diary_entry => diary_entry) - email = UserMailer.with(:comment => diary_comment, :recipient => other_user).diary_comment_notification + email = UserMailer.with(:record => diary_comment, :recipient => other_user).diary_comment_notification body = parse_html_body(email) url = url_helpers.diary_entry_url(user, diary_entry) diff --git a/test/models/diary_comment_test.rb b/test/models/diary_comment_test.rb index 2e2cc39a8..e00274e97 100644 --- a/test/models/diary_comment_test.rb +++ b/test/models/diary_comment_test.rb @@ -22,4 +22,19 @@ class DiaryCommentTest < ActiveSupport::TestCase assert_not_predicate comment, :valid? assert_not_nil comment.errors[:body], "no validation error for body too long" end + + test "the correct subscribers are notified" do + commenter1 = create(:user) + commenter2 = create(:user, :suspended) + commenter3 = create(:user) + commenter4 = create(:user) + diary_entry = create(:diary_entry) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter1) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter2) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter3) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter4) + comment = create(:diary_comment, :diary_entry => diary_entry, :user => commenter4) + + assert_equal comment.notifiable_subscribers.sort, [commenter1, commenter3].sort + end end diff --git a/test/models/diary_entry_test.rb b/test/models/diary_entry_test.rb index c87dab8a8..e130589ee 100644 --- a/test/models/diary_entry_test.rb +++ b/test/models/diary_entry_test.rb @@ -51,6 +51,18 @@ class DiaryEntryTest < ActiveSupport::TestCase assert_equal 2, diary.comments.count end + def test_visible_subscribers + commenter1 = create(:user) + commenter2 = create(:user, :suspended) + commenter3 = create(:user) + diary_entry = create(:diary_entry) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter1) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter2) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => commenter3) + + assert_equal diary_entry.visible_subscribers.sort, [commenter1, commenter3].sort + end + private def diary_entry_valid(attrs, valid: true) -- 2.39.5