FIX: missing edit notifications in some rare cases

Due to a refactor in e90f9e5cc4 we stopped notifying on edits if
a user liked a post and then edited.

The like could have happened a long time ago so this gets extra
confusing.

This change makes the suppression more deliberate. We only want
to suppress quite/link/mention if the user already got a reply
notification.

We can expand this suppression if it is not enough.
This commit is contained in:
Sam Saffron 2020-05-04 17:40:09 +10:00
parent 5e9c96dfed
commit 3d0ccf8642
No known key found for this signature in database
GPG Key ID: B9606168D2FFD9F5
2 changed files with 25 additions and 7 deletions

View File

@ -8,6 +8,11 @@ class PostAlerter
post
end
def self.post_edited(post, opts = {})
PostAlerter.new(opts).after_save_post(post, false)
post
end
def initialize(default_opts = {})
@default_opts = default_opts
end
@ -261,7 +266,6 @@ class PostAlerter
end
def should_notify_previous?(user, post, notification, opts)
return false unless notification
case notification.notification_type
when Notification.types[:edited] then should_notify_edit?(notification, post, opts)
when Notification.types[:liked] then should_notify_like?(user, notification)
@ -331,7 +335,14 @@ class PostAlerter
# Don't notify the same user about the same type of notification on the same post
existing_notification_of_same_type = existing_notifications.find { |n| n.notification_type == type }
return if existing_notifications.present? && !should_notify_previous?(user, post, existing_notification_of_same_type, opts)
if existing_notification_of_same_type && !should_notify_previous?(user, post, existing_notification_of_same_type, opts)
return
end
# linked, quoted, mentioned may be suppressed if you already have a reply notification
if type == Notification.types[:quoted] || type == Notification.types[:linked] || type == Notification.types[:mentioned]
return if existing_notifications.find { |n| n.notification_type == Notification.types[:replied] }
end
notification_data = {}

View File

@ -110,7 +110,15 @@ describe PostAlerter do
admin = Fabricate(:admin)
post.revise(admin, raw: 'I made a revision')
# skip this notification cause we already notified on a similar edit
# lets also like this post which should trigger a notification
PostActionCreator.new(
admin,
post,
PostActionType.types[:like]
).perform
# skip this notification cause we already notified on an edit by the same user
# in the previous edit
freeze_time 2.hours.from_now
post.revise(admin, raw: 'I made another revision')
@ -119,7 +127,7 @@ describe PostAlerter do
freeze_time 2.hours.from_now
post.revise(admin, raw: 'I made another revision')
expect(Notification.where(post_number: 1, topic_id: post.topic_id).count).to eq(3)
expect(Notification.where(post_number: 1, topic_id: post.topic_id).count).to eq(4)
end
it 'notifies flaggers when flagged post gets unhidden by edit' do
@ -195,14 +203,13 @@ describe PostAlerter do
user: evil_trout,
data: { topic_title: "test topic" }.to_json
)
expect {
PostAlerter.post_created(post)
PostAlerter.post_edited(post)
}.to change(evil_trout.notifications, :count).by(0)
notification.destroy
expect {
PostAlerter.post_created(post)
PostAlerter.post_edited(post)
}.to change(evil_trout.notifications, :count).by(1)
end