From 16ff2a4715061d006ef608fc89d342adf66b34bd Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Thu, 9 Nov 2017 15:33:26 -0500 Subject: [PATCH] FIX: topic counts after converting topic to/from public and private --- app/models/topic_converter.rb | 19 +++++++++++++-- app/services/user_action_creator.rb | 4 ++++ spec/models/topic_converter_spec.rb | 36 ++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/app/models/topic_converter.rb b/app/models/topic_converter.rb index f46eaba6ce9..5ebe6516250 100644 --- a/app/models/topic_converter.rb +++ b/app/models/topic_converter.rb @@ -24,6 +24,21 @@ class TopicConverter @topic.archetype = Archetype.default @topic.save update_user_stats + + # TODO: Every post in a PRIVATE MESSAGE looks the same: each is a UserAction::NEW_PRIVATE_MESSAGE. + # So we need to remove all those user actions and re-log all the posts. + # Post counting depends on the correct UserActions (NEW_TOPIC, REPLY), so once a private topic + # becomes a public topic, post counts are wrong. The reverse is not so bad because + # we don't count NEW_PRIVATE_MESSAGE in any public stats. + # TBD: why do so many specs fail with this change? + + # UserAction.where(target_topic_id: @topic.id, action_type: [UserAction::GOT_PRIVATE_MESSAGE, UserAction::NEW_PRIVATE_MESSAGE]).find_each do |ua| + # UserAction.remove_action!(ua.attributes.symbolize_keys.slice(:action_type, :user_id, :acting_user_id, :target_topic_id, :target_post_id)) + # end + # @topic.posts.each do |post| + # UserActionCreator.log_post(post) unless post.post_number == 1 + # end + watch_topic(topic) end @topic @@ -45,7 +60,7 @@ class TopicConverter def update_user_stats @topic.posts.where(deleted_at: nil).each do |p| user = User.find(p.user_id) - # update posts count + # update posts count. NOTE that DirectoryItem.refresh will overwrite this by counting UserAction records. user.user_stat.post_count += 1 user.user_stat.save! end @@ -58,7 +73,7 @@ class TopicConverter @topic.posts.where(deleted_at: nil).each do |p| user = User.find(p.user_id) @topic.topic_allowed_users.build(user_id: user.id) unless @topic.topic_allowed_users.where(user_id: user.id).exists? - # update posts count + # update posts count. NOTE that DirectoryItem.refresh will overwrite this by counting UserAction records. user.user_stat.post_count -= 1 user.user_stat.save! end diff --git a/app/services/user_action_creator.rb b/app/services/user_action_creator.rb index 68a5d0847d2..7b1c92562bb 100644 --- a/app/services/user_action_creator.rb +++ b/app/services/user_action_creator.rb @@ -93,6 +93,10 @@ class UserActionCreator created_at: model.created_at } + UserAction.remove_action!(row.merge( + action_type: model.archetype == Archetype.private_message ? UserAction::NEW_TOPIC : UserAction::NEW_PRIVATE_MESSAGE + )) + rows = [row] if model.private_message? diff --git a/spec/models/topic_converter_spec.rb b/spec/models/topic_converter_spec.rb index 2e76cf9bba8..05d28b10e62 100644 --- a/spec/models/topic_converter_spec.rb +++ b/spec/models/topic_converter_spec.rb @@ -6,7 +6,9 @@ describe TopicConverter do let(:admin) { Fabricate(:admin) } let(:author) { Fabricate(:user) } let(:category) { Fabricate(:category) } - let(:private_message) { Fabricate(:private_message_topic, user: author) } + let(:private_message) { Fabricate(:private_message_topic, user: author) } # creates a topic without a first post + let(:first_post) { Fabricate(:post, topic: private_message, user: author) } + let(:other_user) { private_message.topic_allowed_users.find { |u| u.user != author }.user } context 'success' do it "converts private message to regular topic" do @@ -48,12 +50,36 @@ describe TopicConverter do end it "updates user stats" do + first_post topic_user = TopicUser.create!(user_id: author.id, topic_id: private_message.id, posted: true) expect(private_message.user.user_stat.topic_count).to eq(0) private_message.convert_to_public_topic(admin) expect(private_message.reload.user.user_stat.topic_count).to eq(1) expect(topic_user.reload.notification_level).to eq(TopicUser.notification_levels[:watching]) end + + context "with a reply" do + before do + UserActionCreator.enable + first_post + create_post(topic: private_message, user: other_user) + private_message.reload + end + + after do + UserActionCreator.disable + end + + it "updates UserActions" do + TopicConverter.new(private_message, admin).convert_to_public_topic + expect(author.user_actions.where(action_type: UserAction::NEW_PRIVATE_MESSAGE).count).to eq(0) + expect(author.user_actions.where(action_type: UserAction::NEW_TOPIC).count).to eq(1) + # TODO: + # expect(other_user.user_actions.where(action_type: UserAction::NEW_PRIVATE_MESSAGE).count).to eq(0) + # expect(other_user.user_actions.where(action_type: UserAction::GOT_PRIVATE_MESSAGE).count).to eq(0) + # expect(other_user.user_actions.where(action_type: UserAction::REPLY).count).to eq(1) + end + end end end @@ -81,6 +107,14 @@ describe TopicConverter do expect(topic.reload.user.user_stat.topic_count).to eq(0) expect(topic_user.reload.notification_level).to eq(TopicUser.notification_levels[:watching]) end + + it "changes user_action type" do + UserActionCreator.enable + topic.convert_to_private_message(admin) + expect(author.user_actions.where(action_type: UserAction::NEW_TOPIC).count).to eq(0) + expect(author.user_actions.where(action_type: UserAction::NEW_PRIVATE_MESSAGE).count).to eq(1) + UserActionCreator.disable + end end context 'topic has replies' do