Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

db consistency check for mismatching topic_ids in user_actions 

fix up post mover so it remaps user actions as well
move specs for post mover into post_mover_spec
This commit is contained in:
Sam 2013-07-17 16:40:15 +10:00
parent c2be81a76e
commit 81616a46ba
6 changed files with 226 additions and 175 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/models/post_mover.rb
View File

@ -37,14 +37,12 @@ class PostMover
Guardian.new(user).ensure_can_see! topic
@destination_topic = topic
move_posts_to_destination_topic
destination_topic
end
def move_posts_to_destination_topic
move_each_post
notify_users_that_posts_have_moved
update_statistics
update_user_actions
destination_topic
end
def move_each_post
@ -87,6 +85,10 @@ class PostMover
original_topic.update_statistics
end
def update_user_actions
UserAction.synchronize_target_topic_ids(posts.map(&:id))
end
def notify_users_that_posts_have_moved
enqueue_notification_job
create_moderator_post_in_original_topic

18
app/models/user_action.rb
View File

@ -187,6 +187,7 @@ ORDER BY p.created_at desc
action.id,
user_ids: [user_id],
group_ids: group_ids )
action
rescue ActiveRecord::RecordNotUnique
# can happen, don't care already logged
@ -205,6 +206,23 @@ ORDER BY p.created_at desc
update_like_count(hash[:user_id], hash[:action_type], -1)
end
def self.synchronize_target_topic_ids(post_ids = nil)
builder = SqlBuilder.new("UPDATE user_actions
SET target_topic_id = (select topic_id from posts where posts.id = target_post_id)
/*where*/")
builder.where("target_topic_id <> (select topic_id from posts where posts.id = target_post_id)")
if post_ids
builder.where("target_post_id in (:post_ids)", post_ids: post_ids)
end
builder.exec
end
def self.ensure_consistency!
self.synchronize_target_topic_ids
end
protected
def self.update_like_count(user_id, action_type, delta)

1
lib/jobs/ensure_db_consistency.rb
View File

@ -6,6 +6,7 @@ module Jobs
UserVisit.ensure_consistency!
Group.refresh_automatic_groups!
Notification.ensure_consistency!
UserAction.ensure_consistency!
end
end
end

178
spec/models/post_mover_spec.rb Normal file
View File

@ -0,0 +1,178 @@
require 'spec_helper'
describe PostMover do
context 'move_posts' do
let(:user) { Fabricate(:user) }
let(:another_user) { Fabricate(:evil_trout) }
let(:category) { Fabricate(:category, user: user) }
let!(:topic) { Fabricate(:topic, user: user, category: category) }
let!(:p1) { Fabricate(:post, topic: topic, user: user) }
let!(:p2) { Fabricate(:post, topic: topic, user: another_user, raw: "Has a link to [evil trout](http://eviltrout.com) which is a cool site.")}
let!(:p3) { Fabricate(:post, topic: topic, user: user)}
let!(:p4) { Fabricate(:post, topic: topic, user: user)}
before do
# add a like to a post, enable observers so we get user actions
ActiveRecord::Base.observers.enable :all
@like = PostAction.act(another_user, p4, PostActionType.types[:like])
end
context 'success' do
it "enqueues a job to notify users" do
topic.stubs(:add_moderator_post)
Jobs.expects(:enqueue).with(:notify_moved_posts, post_ids: [p2.id, p4.id], moved_by_id: user.id)
topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name")
end
it "adds a moderator post at the location of the first moved post" do
topic.expects(:add_moderator_post).with(user, instance_of(String), has_entries(post_number: 2))
topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name")
end
end
context "errors" do
it "raises an error when one of the posts doesn't exist" do
lambda { topic.move_posts(user, [1003], title: "new testing topic name") }.should raise_error(Discourse::InvalidParameters)
end
it "raises an error and does not create a topic if no posts were moved" do
Topic.count.tap do |original_topic_count|
lambda {
topic.move_posts(user, [], title: "new testing topic name")
}.should raise_error(Discourse::InvalidParameters)
expect(Topic.count).to eq original_topic_count
end
end
end
context "successfully moved" do
before do
topic.expects(:add_moderator_post)
TopicUser.update_last_read(user, topic.id, p4.post_number, 0)
TopicLink.extract_from(p2)
end
context "to a new topic" do
let!(:new_topic) { topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name") }
it "works correctly" do
TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number
new_topic.should be_present
new_topic.featured_user1_id.should == another_user.id
new_topic.like_count.should == 1
new_topic.category.should == category
topic.featured_user1_id.should be_blank
new_topic.posts.should =~ [p2, p4]
new_topic.reload
new_topic.posts_count.should == 2
new_topic.highest_post_number.should == 2
expect(new_topic.last_posted_at).to be_present
p2.reload
p2.sort_order.should == 1
p2.post_number.should == 1
p2.topic_links.first.topic_id.should == new_topic.id
p4.reload
p4.post_number.should == 2
p4.sort_order.should == 2
topic.reload
topic.featured_user1_id.should be_blank
topic.like_count.should == 0
topic.posts_count.should == 2
topic.posts.should =~ [p1, p3]
topic.highest_post_number.should == p3.post_number
# both the like and was_liked user actions should be correct
action = UserAction.where(user_id: another_user.id).first
action.target_topic_id.should == new_topic.id
end
end
context "to an existing topic" do
let!(:destination_topic) { Fabricate(:topic, user: user ) }
let!(:destination_op) { Fabricate(:post, topic: destination_topic, user: user) }
let!(:moved_to) { topic.move_posts(user, [p2.id, p4.id], destination_topic_id: destination_topic.id )}
it "works correctly" do
moved_to.should == destination_topic
# Check out new topic
moved_to.reload
moved_to.posts_count.should == 3
moved_to.highest_post_number.should == 3
moved_to.featured_user1_id.should == another_user.id
moved_to.like_count.should == 1
moved_to.category.should be_blank
# Posts should be re-ordered
p2.reload
p2.sort_order.should == 2
p2.post_number.should == 2
p2.topic_id.should == moved_to.id
p4.reload
p4.post_number.should == 3
p4.sort_order.should == 3
p4.topic_id.should == moved_to.id
# Check out the original topic
topic.reload
topic.posts_count.should == 2
topic.highest_post_number.should == 3
topic.featured_user1_id.should be_blank
topic.like_count.should == 0
topic.posts_count.should == 2
topic.posts.should =~ [p1, p3]
topic.highest_post_number.should == p3.post_number
# Should update last reads
TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number
end
end
context "moving the first post" do
let!(:new_topic) { topic.move_posts(user, [p1.id, p2.id], title: "new testing topic name") }
it "copies the OP, doesn't delete it" do
new_topic.should be_present
new_topic.posts.first.raw.should == p1.raw
new_topic.reload
new_topic.posts_count.should == 2
new_topic.highest_post_number.should == 2
# First post didn't move
p1.reload
p1.sort_order.should == 1
p1.post_number.should == 1
p1.topic_id == topic.id
# Second post is in a new topic
p2.reload
p2.post_number.should == 2
p2.sort_order.should == 2
p2.topic_id == new_topic.id
topic.reload
topic.posts.should =~ [p1, p3, p4]
topic.highest_post_number.should == p4.post_number
end
end
end
end
end

169
spec/models/topic_spec.rb
View File

@ -228,175 +228,6 @@ describe Topic do
end
context 'move_posts' do
let(:user) { Fabricate(:user) }
let(:another_user) { Fabricate(:evil_trout) }
let(:category) { Fabricate(:category, user: user) }
let!(:topic) { Fabricate(:topic, user: user, category: category) }
let!(:p1) { Fabricate(:post, topic: topic, user: user) }
let!(:p2) { Fabricate(:post, topic: topic, user: another_user, raw: "Has a link to [evil trout](http://eviltrout.com) which is a cool site.")}
let!(:p3) { Fabricate(:post, topic: topic, user: user)}
let!(:p4) { Fabricate(:post, topic: topic, user: user)}
before do
# add a like to a post
PostAction.act(another_user, p4, PostActionType.types[:like])
end
context 'success' do
it "enqueues a job to notify users" do
topic.stubs(:add_moderator_post)
Jobs.expects(:enqueue).with(:notify_moved_posts, post_ids: [p2.id, p4.id], moved_by_id: user.id)
topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name")
end
it "adds a moderator post at the location of the first moved post" do
topic.expects(:add_moderator_post).with(user, instance_of(String), has_entries(post_number: 2))
topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name")
end
end
context "errors" do
it "raises an error when one of the posts doesn't exist" do
lambda { topic.move_posts(user, [1003], title: "new testing topic name") }.should raise_error(Discourse::InvalidParameters)
end
it "raises an error and does not create a topic if no posts were moved" do
Topic.count.tap do |original_topic_count|
lambda {
topic.move_posts(user, [], title: "new testing topic name")
}.should raise_error(Discourse::InvalidParameters)
expect(Topic.count).to eq original_topic_count
end
end
end
context "successfully moved" do
before do
topic.expects(:add_moderator_post)
TopicUser.update_last_read(user, topic.id, p4.post_number, 0)
TopicLink.extract_from(p2)
end
context "to a new topic" do
let!(:new_topic) { topic.move_posts(user, [p2.id, p4.id], title: "new testing topic name") }
it "works correctly" do
TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number
new_topic.should be_present
new_topic.featured_user1_id.should == another_user.id
new_topic.like_count.should == 1
new_topic.category.should == category
topic.featured_user1_id.should be_blank
new_topic.posts.should =~ [p2, p4]
new_topic.reload
new_topic.posts_count.should == 2
new_topic.highest_post_number.should == 2
expect(new_topic.last_posted_at).to be_present
p2.reload
p2.sort_order.should == 1
p2.post_number.should == 1
p2.topic_links.first.topic_id.should == new_topic.id
p4.reload
p4.post_number.should == 2
p4.sort_order.should == 2
topic.reload
topic.featured_user1_id.should be_blank
topic.like_count.should == 0
topic.posts_count.should == 2
topic.posts.should =~ [p1, p3]
topic.highest_post_number.should == p3.post_number
end
end
context "to an existing topic" do
let!(:destination_topic) { Fabricate(:topic, user: user ) }
let!(:destination_op) { Fabricate(:post, topic: destination_topic, user: user) }
let!(:moved_to) { topic.move_posts(user, [p2.id, p4.id], destination_topic_id: destination_topic.id )}
it "works correctly" do
moved_to.should == destination_topic
# Check out new topic
moved_to.reload
moved_to.posts_count.should == 3
moved_to.highest_post_number.should == 3
moved_to.featured_user1_id.should == another_user.id
moved_to.like_count.should == 1
moved_to.category.should be_blank
# Posts should be re-ordered
p2.reload
p2.sort_order.should == 2
p2.post_number.should == 2
p2.topic_id.should == moved_to.id
p4.reload
p4.post_number.should == 3
p4.sort_order.should == 3
p4.topic_id.should == moved_to.id
# Check out the original topic
topic.reload
topic.posts_count.should == 2
topic.highest_post_number.should == 3
topic.featured_user1_id.should be_blank
topic.like_count.should == 0
topic.posts_count.should == 2
topic.posts.should =~ [p1, p3]
topic.highest_post_number.should == p3.post_number
# Should update last reads
TopicUser.where(user_id: user.id, topic_id: topic.id).first.last_read_post_number.should == p3.post_number
end
end
context "moving the first post" do
let!(:new_topic) { topic.move_posts(user, [p1.id, p2.id], title: "new testing topic name") }
it "copies the OP, doesn't delete it" do
new_topic.should be_present
new_topic.posts.first.raw.should == p1.raw
new_topic.reload
new_topic.posts_count.should == 2
new_topic.highest_post_number.should == 2
# First post didn't move
p1.reload
p1.sort_order.should == 1
p1.post_number.should == 1
p1.topic_id == topic.id
# Second post is in a new topic
p2.reload
p2.post_number.should == 2
p2.sort_order.should == 2
p2.topic_id == new_topic.id
topic.reload
topic.posts.should =~ [p1, p3, p4]
topic.highest_post_number.should == p4.post_number
end
end
end
end
context 'private message' do
let(:coding_horror) { User.where(username: 'CodingHorror').first }

23
spec/models/user_action_spec.rb
View File

@ -9,7 +9,6 @@ describe UserAction do
it { should validate_presence_of :action_type }
it { should validate_presence_of :user_id }
describe 'lists' do
let(:public_post) { Fabricate(:post) }
@ -257,4 +256,26 @@ describe UserAction do
end
end
describe 'ensure_consistency!' do
it 'correct target_topic_id' do
post = Fabricate(:post)
action = UserAction.log_action!(
action_type: UserAction::NEW_PRIVATE_MESSAGE,
user_id: post.user.id,
acting_user_id: post.user.id,
target_topic_id: -1,
target_post_id: post.id,
)
action.reload
action.target_topic_id.should == -1
UserAction.ensure_consistency!
action.reload
action.target_topic_id.should == post.topic_id
end
end
end