Skip to content

Commit

Permalink
FIX: Reply notifications should not appear as edited (#9965)
Browse files Browse the repository at this point in the history
  • Loading branch information
nbianca committed Jun 8, 2020
1 parent cb13152 commit 052c917
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
6 changes: 3 additions & 3 deletions app/services/post_alerter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def after_save_post(post, new_record = false)
if post.topic.private_message?
notify_pm_users(post, reply_to_user, notified)
elsif notify_about_reply?(post)
notify_post_users(post, notified)
notify_post_users(post, notified, new_record: new_record)
end
end

Expand Down Expand Up @@ -566,7 +566,7 @@ def notify_pm_users(post, reply_to_user, notified)
end
end

def notify_post_users(post, notified, include_category_watchers: true, include_tag_watchers: true)
def notify_post_users(post, notified, include_category_watchers: true, include_tag_watchers: true, new_record: false)
return unless post.topic

warn_if_not_sidekiq
Expand Down Expand Up @@ -627,7 +627,7 @@ def notify_post_users(post, notified, include_category_watchers: true, include_t
already_seen_user_ids = Set.new TopicUser.where(topic_id: post.topic.id).where("highest_seen_post_number >= ?", post.post_number).pluck(:user_id)

each_user_in_batches(notify) do |user|
notification_type = already_seen_user_ids.include?(user.id) ? Notification.types[:edited] : Notification.types[:posted]
notification_type = !new_record && already_seen_user_ids.include?(user.id) ? Notification.types[:edited] : Notification.types[:posted]
opts = {}
opts[:display_username] = post.last_editor.username if notification_type == Notification.types[:edited]
create_notification(user, notification_type, post, opts)
Expand Down
22 changes: 22 additions & 0 deletions spec/services/post_alerter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,7 @@ def set_topic_notification_level(user, topic, level_name)
fab!(:post) { Fabricate(:post, topic: topic) }
fab!(:last_editor) { Fabricate(:user) }
fab!(:tag) { Fabricate(:tag) }
fab!(:category) { Fabricate(:category) }

it 'creates single edit notification when post is modified' do
TopicUser.create!(user_id: user.id, topic_id: topic.id, notification_level: TopicUser.notification_levels[:watching], highest_seen_post_number: post.post_number)
Expand All @@ -1142,5 +1143,26 @@ def set_topic_notification_level(user, topic, level_name)
PostAlerter.new.notify_post_users(post, [])
expect(Notification.count).to eq(1)
end

it 'creates posted notification when Sidekiq is slow' do
CategoryUser.set_notification_level_for_category(user, CategoryUser.notification_levels[:watching], category.id)

post = PostCreator.create!(
Fabricate(:user),
title: "one of my first topics",
raw: "one of my first posts",
category: category.id
)

TopicUser.change(user, post.topic_id, highest_seen_post_number: post.post_number)

# Manually run job after the user read the topic to simulate a slow
# Sidekiq.
job_args = Jobs::PostAlert.jobs[0]['args'][0]
expect { Jobs::PostAlert.new.execute(job_args.with_indifferent_access) }
.to change { Notification.count }.by(1)

expect(Notification.last.notification_type).to eq(Notification.types[:posted])
end
end
end

0 comments on commit 052c917

Please sign in to comment.