If a user read to the end of an auto closing topic, when it is closes just pretend they read the close message.

This commit is contained in:
Sam 2013-07-04 11:47:12 +10:00
parent 7504da13e3
commit b662cb6c02
5 changed files with 139 additions and 10 deletions

View file

@ -6,6 +6,29 @@ class PostTiming < ActiveRecord::Base
validates_presence_of :msecs validates_presence_of :msecs
def self.pretend_read(topic_id, actual_read_post_number, pretend_read_post_number)
# This is done in SQL cause the logic is quite tricky and we want to do this in one db hit
#
exec_sql("INSERT INTO post_timings(topic_id, user_id, post_number, msecs)
SELECT :topic_id, user_id, :pretend_read_post_number, 1
FROM post_timings pt
WHERE topic_id = :topic_id AND
post_number = :actual_read_post_number AND
NOT EXISTS (
SELECT 1 FROM post_timings pt1
WHERE pt1.topic_id = pt.topic_id AND
pt1.post_number = :pretend_read_post_number AND
pt1.user_id = pt.user_id
)
",
pretend_read_post_number: pretend_read_post_number,
topic_id: topic_id,
actual_read_post_number: actual_read_post_number
)
TopicUser.ensure_consistency!(topic_id)
end
# Increases a timer if a row exists, otherwise create it # Increases a timer if a row exists, otherwise create it
def self.record_timing(args) def self.record_timing(args)
rows = exec_sql_row_count("UPDATE post_timings rows = exec_sql_row_count("UPDATE post_timings

View file

@ -3,8 +3,11 @@ TopicStatusUpdate = Struct.new(:topic, :user) do
status = Status.new(status, enabled) status = Status.new(status, enabled)
Topic.transaction do Topic.transaction do
change status change(status)
create_moderator_post_for status highest_post_number = topic.highest_post_number
create_moderator_post_for(status)
update_read_state_for(status, highest_post_number)
end end
end end
@ -26,6 +29,15 @@ TopicStatusUpdate = Struct.new(:topic, :user) do
def create_moderator_post_for(status) def create_moderator_post_for(status)
topic.add_moderator_post(user, message_for(status), options_for(status)) topic.add_moderator_post(user, message_for(status), options_for(status))
topic.reload
end
def update_read_state_for(status, old_highest_read)
if status.autoclosed?
# let's pretend all the people that read up to the autoclose message
# actually read the topic
PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number)
end
end end
def message_for(status) def message_for(status)

View file

@ -204,8 +204,8 @@ class TopicUser < ActiveRecord::Base
end end
end end
def self.ensure_consistency! def self.ensure_consistency!(topic_id=nil)
exec_sql <<SQL builder = SqlBuilder.new <<SQL
UPDATE topic_users t UPDATE topic_users t
SET SET
last_read_post_number = last_read, last_read_post_number = last_read,
@ -219,13 +219,23 @@ JOIN (
SELECT p.topic_id, MAX(p.post_number) max_post_number from posts p SELECT p.topic_id, MAX(p.post_number) max_post_number from posts p
GROUP BY p.topic_id GROUP BY p.topic_id
) as Y on Y.topic_id = X.topic_id ) as Y on Y.topic_id = X.topic_id
WHERE X.topic_id = t.topic_id AND /*where*/
X.user_id = t.user_id AND
(
last_read_post_number <> last_read OR
seen_post_count <> LEAST(max_post_number,GREATEST(t.seen_post_count, last_read))
)
SQL SQL
builder.where <<SQL
X.topic_id = t.topic_id AND
X.user_id = t.user_id AND
(
last_read_post_number <> last_read OR
seen_post_count <> LEAST(max_post_number,GREATEST(t.seen_post_count, last_read))
)
SQL
if topic_id
builder.where("t.topic_id = :topic_id", topic_id: topic_id)
end
builder.exec
end end
end end

View file

@ -8,6 +8,61 @@ describe PostTiming do
it { should validate_presence_of :post_number } it { should validate_presence_of :post_number }
it { should validate_presence_of :msecs } it { should validate_presence_of :msecs }
describe 'pretend_read' do
let!(:p1) { Fabricate(:post) }
let!(:p2) { Fabricate(:post, topic: p1.topic, user: p1.user) }
let!(:p3) { Fabricate(:post, topic: p1.topic, user: p1.user) }
let :topic_id do
p1.topic_id
end
def timing(user_id, post_number)
PostTiming.create!(topic_id: topic_id, user_id: user_id, post_number: post_number, msecs: 0)
end
def topic_user(user_id, last_read_post_number, seen_post_count)
TopicUser.create!(
topic_id: topic_id,
user_id: user_id,
last_read_post_number: last_read_post_number,
seen_post_count: seen_post_count
)
end
it 'works correctly' do
timing(1,1)
timing(2,1)
timing(2,2)
timing(3,1)
timing(3,2)
timing(3,3)
tu_one = topic_user(1,1,1)
tu_two = topic_user(2,2,2)
tu_three = topic_user(3,3,3)
PostTiming.pretend_read(topic_id, 2, 3)
PostTiming.where(topic_id: topic_id, user_id: 1, post_number: 3).count.should == 0
PostTiming.where(topic_id: topic_id, user_id: 2, post_number: 3).count.should == 1
PostTiming.where(topic_id: topic_id, user_id: 3, post_number: 3).count.should == 1
tu = TopicUser.where(topic_id: topic_id, user_id: 1).first
tu.last_read_post_number.should == 1
tu.seen_post_count.should == 1
tu = TopicUser.where(topic_id: topic_id, user_id: 2).first
tu.last_read_post_number.should == 3
tu.seen_post_count.should == 3
tu = TopicUser.where(topic_id: topic_id, user_id: 3).first
tu.last_read_post_number.should == 3
tu.seen_post_count.should == 3
end
end
describe 'process_timings' do describe 'process_timings' do
# integration test # integration test

View file

@ -0,0 +1,29 @@
# encoding: UTF-8
require 'spec_helper'
require_dependency 'post_destroyer'
describe TopicStatusUpdate do
it "avoids notifying on automatically closed topics" do
# TODO: TopicStatusUpdate should supress message bus updates from the users it "pretends to read"
user = Fabricate(:user)
post = PostCreator.create(user,
raw: "this is a test post 123 this is a test post",
title: "hello world title",
)
# TODO needed so counts sync up,
# PostCreator really should not give back out-of-date Topic
post.topic.reload
# TODO: also annoying PostTiming is not logged
PostTiming.create!(topic_id: post.topic_id, user_id: user.id, post_number: 1, msecs: 0)
admin = Fabricate(:admin)
TopicStatusUpdate.new(post.topic, admin).update!("autoclosed", true)
post.topic.posts.count.should == 2
tu = TopicUser.where(user_id: user.id).first
tu.last_read_post_number.should == 2
end
end