diff --git a/app/models/post_timing.rb b/app/models/post_timing.rb index 44d683db50d..6bfc5b7f2bd 100644 --- a/app/models/post_timing.rb +++ b/app/models/post_timing.rb @@ -148,6 +148,16 @@ class PostTiming < ActiveRecord::Base MAX_READ_TIME_PER_BATCH = 60 * 1000.0 def self.process_timings(current_user, topic_id, topic_time, timings, opts = {}) + lookup_column = current_user.whisperer? ? "highest_staff_post_number" : "highest_post_number" + highest_post_number = DB.query_single(<<~SQL, topic_id: topic_id).first + SELECT #{lookup_column} + FROM topics + WHERE id = :topic_id + SQL + + # does not exist log nothing + return if highest_post_number.nil? + UserStat.update_time_read!(current_user.id) max_time_per_post = ((Time.now - current_user.created_at) * 1000.0) @@ -163,6 +173,7 @@ class PostTiming < ActiveRecord::Base i -= 1 timings[i][1] = max_time_per_post if timings[i][1] > max_time_per_post timings.delete_at(i) if timings[i][0] < 1 + timings.delete_at(i) if timings[i][0] > highest_post_number end timings.each_with_index do |(post_number, time), index| diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index e8c15fd6d51..cc9ce74b5c5 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -308,7 +308,14 @@ class TopicUser < ActiveRecord::Base UPDATE_TOPIC_USER_SQL = <<~SQL UPDATE topic_users SET - last_read_post_number = GREATEST(:post_number, tu.last_read_post_number), + last_read_post_number = + LEAST( + CASE WHEN :whisperer + THEN highest_staff_post_number + ELSE highest_post_number END + , + GREATEST(:post_number, tu.last_read_post_number) + ), total_msecs_viewed = LEAST(tu.total_msecs_viewed + :msecs,86400000), notification_level = case when tu.notifications_reason_id is null and (tu.total_msecs_viewed + :msecs) > @@ -357,6 +364,7 @@ class TopicUser < ActiveRecord::Base msecs: msecs, tracking: notification_levels[:tracking], threshold: SiteSetting.default_other_auto_track_topics_after_msecs, + whisperer: user.whisperer?, } rows = DB.query(UPDATE_TOPIC_USER_SQL, args) diff --git a/spec/models/post_timing_spec.rb b/spec/models/post_timing_spec.rb index eb39cc40c0e..341e7385f59 100644 --- a/spec/models/post_timing_spec.rb +++ b/spec/models/post_timing_spec.rb @@ -83,6 +83,7 @@ RSpec.describe PostTiming do (2..5).each { |i| Fabricate(:post, topic: post.topic, post_number: i) } user2 = Fabricate(:coding_horror, created_at: 1.day.ago) + Topic.reset_highest(post.topic.id) PostActionCreator.like(user2, post) diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb index 5e027aa3284..6954c17964c 100644 --- a/spec/models/topic_user_spec.rb +++ b/spec/models/topic_user_spec.rb @@ -228,6 +228,12 @@ RSpec.describe TopicUser do today = Time.zone.now freeze_time Time.zone.now + # ensure data model is correct for the test + # logging an update to a row that does not exist + # is not supported + _post1 = Fabricate(:post, topic: topic) + _post2 = Fabricate(:post, topic: topic) + TopicUser.update_last_read(user, topic.id, 1, 1, 0) tomorrow = 1.day.from_now diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 9463d33151b..27e92128b73 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -4626,6 +4626,62 @@ RSpec.describe TopicsController do describe "#timings" do fab!(:post_1) { Fabricate(:post, user: post_author1, topic: topic) } + before do + # admins + SiteSetting.whispers_allowed_groups = "1" + end + + let(:whisper) do + Fabricate(:post, user: post_author1, topic: topic, post_type: Post.types[:whisper]) + end + + it "should gracefully handle invalid timings sent in" do + sign_in(user) + params = { + topic_id: topic.id, + topic_time: 5, + timings: { + post_1.post_number => 2, + whisper.post_number => 2, + 1000 => 100, + }, + } + + post "/topics/timings.json", params: params + expect(response.status).to eq(200) + + tu = TopicUser.find_by(user: user, topic: topic) + expect(tu.last_read_post_number).to eq(post_1.post_number) + + # lets also test timing recovery here + tu.update!(last_read_post_number: 999) + + post "/topics/timings.json", params: params + + tu = TopicUser.find_by(user: user, topic: topic) + expect(tu.last_read_post_number).to eq(post_1.post_number) + end + + it "should gracefully handle invalid timings sent in from staff" do + sign_in(admin) + + post "/topics/timings.json", + params: { + topic_id: topic.id, + topic_time: 5, + timings: { + post_1.post_number => 2, + whisper.post_number => 2, + 1000 => 100, + }, + } + + expect(response.status).to eq(200) + + tu = TopicUser.find_by(user: admin, topic: topic) + expect(tu.last_read_post_number).to eq(whisper.post_number) + end + it "should record the timing" do sign_in(user) @@ -5382,7 +5438,7 @@ RSpec.describe TopicsController do it "resets bump correctly" do post1 = Fabricate(:post, user: post_author1, topic: topic, created_at: 2.days.ago) - post2 = Fabricate(:post, user: post_author1, topic: topic, created_at: 1.day.ago) + _post2 = Fabricate(:post, user: post_author1, topic: topic, created_at: 1.day.ago) put "/t/#{topic.id}/reset-bump-date/#{post1.id}.json" expect(response.status).to eq(200)