codeninjasllc/discourse
1
0
Fork 0
mirror of https://github.com/codeninjasllc/discourse.git synced 2024-11-27 09:36:19 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

FIX: "Dismiss Posts" corrupting read state

Browse source 
REFACTOR: seen_post_count was a bad name, renamed to highest_seen_post_number
This commit is contained in:
Sam 2014-10-31 09:40:35 +11:00
parent fb69690a4f
commit 2251877332
12 changed files with 46 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
app/models/topic.rb
View file

@ -421,9 +421,9 @@ class Topic < ActiveRecord::Base
WHEN last_read_post_number > :highest THEN :highest
ELSE last_read_post_number
END,
seen_post_count = CASE
WHEN seen_post_count > :highest THEN :highest
ELSE seen_post_count
highest_seen_post_number = CASE
WHEN highest_seen_post_number > :highest THEN :highest
ELSE highest_seen_post_number
END
WHERE topic_id = :topic_id",
highest: highest_post_number,

14
app/models/topic_user.rb
View file

@ -152,8 +152,8 @@ class TopicUser < ActiveRecord::Base
threshold: SiteSetting.auto_track_topics_after
}
# In case anyone seens "seen_post_count" and gets confused, like I do.
# seen_post_count represents the highest_post_number of the topic when
# In case anyone seens "highest_seen_post_number" and gets confused, like I do.
# highest_seen_post_number represents the highest_post_number of the topic when
# the user visited it. It may be out of alignment with last_read, meaning
# ... user visited the topic but did not read the posts
#
@ -161,7 +161,7 @@ class TopicUser < ActiveRecord::Base
rows = exec_sql("UPDATE topic_users
SET
last_read_post_number = GREATEST(:post_number, tu.last_read_post_number),
seen_post_count = t.highest_post_number,
highest_seen_post_number = t.highest_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) >
@ -210,7 +210,7 @@ class TopicUser < ActiveRecord::Base
TopicTrackingState.publish_read(topic_id, post_number, user.id, args[:new_status])
user.update_posts_read!(post_number)
exec_sql("INSERT INTO topic_users (user_id, topic_id, last_read_post_number, seen_post_count, last_visited_at, first_visited_at, notification_level)
exec_sql("INSERT INTO topic_users (user_id, topic_id, last_read_post_number, highest_seen_post_number, last_visited_at, first_visited_at, notification_level)
SELECT :user_id, :topic_id, :post_number, ft.highest_post_number, :now, :now, :new_status
FROM topics AS ft
JOIN users u on u.id = :user_id
@ -241,7 +241,7 @@ class TopicUser < ActiveRecord::Base
UPDATE topic_users t
SET
last_read_post_number = LEAST(GREATEST(last_read, last_read_post_number), max_post_number),
seen_post_count = LEAST(max_post_number,GREATEST(t.seen_post_count, last_read))
highest_seen_post_number = LEAST(max_post_number,GREATEST(t.highest_seen_post_number, last_read))
FROM (
SELECT topic_id, user_id, MAX(post_number) last_read
FROM post_timings
@ -259,7 +259,7 @@ X.topic_id = t.topic_id AND
X.user_id = t.user_id AND
(
last_read_post_number <> LEAST(GREATEST(last_read, last_read_post_number), max_post_number) OR
seen_post_count <> LEAST(max_post_number,GREATEST(t.seen_post_count, last_read))
highest_seen_post_number <> LEAST(max_post_number,GREATEST(t.highest_seen_post_number, last_read))
)
SQL
@ -281,7 +281,7 @@ end
# starred :boolean default(FALSE), not null
# posted :boolean default(FALSE), not null
# last_read_post_number :integer
# seen_post_count :integer
# highest_seen_post_number :integer
# starred_at :datetime
# last_visited_at :datetime
# first_visited_at :datetime

5
db/migrate/20141030222425_rename_seen_post_count.rb Normal file
View file

@ -0,0 +1,5 @@
class RenameSeenPostCount < ActiveRecord::Migration
def change
rename_column :topic_users, :seen_post_count, :highest_seen_post_number
end
end

2
lib/post_creator.rb
View file

@ -298,7 +298,7 @@ class PostCreator
@topic.id,
posted: true,
last_read_post_number: @post.post_number,
seen_post_count: @post.post_number)
highest_seen_post_number: @post.post_number)
# assume it took us 5 seconds of reading time to make a post

2
lib/topics_bulk_action.rb
View file

@ -22,7 +22,7 @@ class TopicsBulkAction
def dismiss_posts
sql = "
UPDATE topic_users tu
SET seen_post_count = t.posts_count , last_read_post_number = highest_post_number
SET highest_seen_post_number = t.highest_post_number , last_read_post_number = highest_post_number
FROM topics t
WHERE t.id = tu.topic_id AND tu.user_id = :user_id AND t.id IN (:topic_ids)
"

6
lib/unread.rb
View file

@ -10,17 +10,17 @@ class Unread
def unread_posts
return 0 if do_not_notify?(@topic_user.notification_level)
result = ((@topic_user.seen_post_count||0) - (@topic_user.last_read_post_number||0))
result = ((@topic_user.highest_seen_post_number||0) - (@topic_user.last_read_post_number||0))
result = 0 if result < 0
result
end
def new_posts
return 0 if @topic_user.seen_post_count.blank?
return 0 if @topic_user.highest_seen_post_number.blank?
return 0 if do_not_notify?(@topic_user.notification_level)
return 0 if (@topic_user.last_read_post_number||0) > @topic.highest_post_number
new_posts = (@topic.highest_post_number - @topic_user.seen_post_count)
new_posts = (@topic.highest_post_number - @topic_user.highest_seen_post_number)
new_posts = 0 if new_posts < 0
return new_posts
end

2
spec/components/post_creator_spec.rb
View file

@ -178,7 +178,7 @@ describe PostCreator do
topic_user.should be_present
topic_user.should be_posted
topic_user.last_read_post_number.should == first_post.post_number
topic_user.seen_post_count.should == first_post.post_number
topic_user.highest_seen_post_number.should == first_post.post_number
user2 = Fabricate(:coding_horror)
user2.user_stat.topic_reply_count.should == 0

2
spec/components/post_destroyer_spec.rb
View file

@ -241,7 +241,7 @@ describe PostDestroyer do
end
it "sets the second user's last_read_post_number back to 1" do
topic_user.seen_post_count.should == 1
topic_user.highest_seen_post_number.should == 1
end
end

8
spec/components/topics_bulk_action_spec.rb
View file

@ -6,13 +6,17 @@ describe TopicsBulkAction do
describe "dismiss_posts" do
it "dismisses posts" do
post1 = create_post
p = create_post(topic_id: post1.topic_id)
create_post(topic_id: post1.topic_id)
PostDestroyer.new(Fabricate(:admin), p).destroy
TopicsBulkAction.new(post1.user, [post1.topic_id], type: 'dismiss_posts').perform!
tu = TopicUser.find_by(user_id: post1.user_id, topic_id: post1.topic_id)
tu.last_read_post_number.should == 2
tu.seen_post_count = 2
tu.last_read_post_number.should == 3
tu.highest_seen_post_number.should == 3
end
end

14
spec/components/unread_spec.rb
View file

@ -16,19 +16,19 @@ describe Unread do
describe 'unread_posts' do
it 'should have 0 unread posts if the user has seen all posts' do
@topic_user.stubs(:last_read_post_number).returns(13)
@topic_user.stubs(:seen_post_count).returns(13)
@topic_user.stubs(:highest_seen_post_number).returns(13)
@unread.unread_posts.should == 0
end
it 'should have 6 unread posts if the user has seen all but 6 posts' do
@topic_user.stubs(:last_read_post_number).returns(5)
@topic_user.stubs(:seen_post_count).returns(11)
@topic_user.stubs(:highest_seen_post_number).returns(11)
@unread.unread_posts.should == 6
end
it 'should have 0 unread posts if the user has seen more posts than exist (deleted)' do
@topic_user.stubs(:last_read_post_number).returns(100)
@topic_user.stubs(:seen_post_count).returns(13)
@topic_user.stubs(:highest_seen_post_number).returns(13)
@unread.unread_posts.should == 0
end
end
@ -40,23 +40,23 @@ describe Unread do
end
it 'returns 0 when the topic is the same length as when you last saw it' do
@topic_user.stubs(:seen_post_count).returns(13)
@topic_user.stubs(:highest_seen_post_number).returns(13)
@unread.new_posts.should == 0
end
it 'has 3 new posts if the user has read 10 posts' do
@topic_user.stubs(:seen_post_count).returns(10)
@topic_user.stubs(:highest_seen_post_number).returns(10)
@unread.new_posts.should == 3
end
it 'has 0 new posts if the user has read 10 posts but is not tracking' do
@topic_user.stubs(:seen_post_count).returns(10)
@topic_user.stubs(:highest_seen_post_number).returns(10)
@topic_user.stubs(:notification_level).returns(TopicUser.notification_levels[:regular])
@unread.new_posts.should == 0
end
it 'has 0 new posts if the user read more posts than exist (deleted)' do
@topic_user.stubs(:seen_post_count).returns(16)
@topic_user.stubs(:highest_seen_post_number).returns(16)
@unread.new_posts.should == 0
end

16
spec/models/post_timing_spec.rb
View file

@ -18,12 +18,12 @@ describe PostTiming do
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)
def topic_user(user_id, last_read_post_number, highest_seen_post_number)
TopicUser.create!(
topic_id: topic_id,
user_id: user_id,
last_read_post_number: last_read_post_number,
seen_post_count: seen_post_count
highest_seen_post_number: highest_seen_post_number
)
end
@ -35,9 +35,9 @@ describe PostTiming do
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)
_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)
@ -47,15 +47,15 @@ describe PostTiming do
tu = TopicUser.find_by(topic_id: topic_id, user_id: 1)
tu.last_read_post_number.should == 1
tu.seen_post_count.should == 1
tu.highest_seen_post_number.should == 1
tu = TopicUser.find_by(topic_id: topic_id, user_id: 2)
tu.last_read_post_number.should == 3
tu.seen_post_count.should == 3
tu.highest_seen_post_number.should == 3
tu = TopicUser.find_by(topic_id: topic_id, user_id: 3)
tu.last_read_post_number.should == 3
tu.seen_post_count.should == 3
tu.highest_seen_post_number.should == 3
end
end

5
spec/models/topic_user_spec.rb
View file

@ -254,7 +254,7 @@ describe TopicUser do
p2 = Fabricate(:post, user: p1.user, topic: p1.topic, post_number: 2)
p1.topic.notifier.watch_topic!(p1.user_id)
TopicUser.exec_sql("UPDATE topic_users set seen_post_count=100, last_read_post_number=0
TopicUser.exec_sql("UPDATE topic_users set highest_seen_post_number=1, last_read_post_number=0
WHERE topic_id = :topic_id AND user_id = :user_id", topic_id: p1.topic_id, user_id: p1.user_id)
[p1,p2].each do |p|
@ -265,7 +265,8 @@ describe TopicUser do
tu = TopicUser.find_by(user_id: p1.user_id, topic_id: p1.topic_id)
tu.last_read_post_number.should == p2.post_number
tu.seen_post_count.should == 2
tu.highest_seen_post_number.should == 2
end
describe "mailing_list_mode" do