diff --git a/app/models/post.rb b/app/models/post.rb index 5fe4606f78b..417788a3499 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -52,6 +52,7 @@ class Post < ActiveRecord::Base validates_with ::Validators::PostValidator after_save :index_search + after_save :create_user_action # We can pass several creating options to a post via attributes attr_accessor :image_sizes, :quoted_post_numbers, :no_bump, :invalidate_oneboxes, :cooking_options, :skip_unique_check @@ -642,6 +643,10 @@ class Post < ActiveRecord::Base SearchIndexer.index(self) end + def create_user_action + UserActionCreator.log_post(self) + end + private def parse_quote_into_arguments(quote) diff --git a/app/models/post_action.rb b/app/models/post_action.rb index 5bc031ccae5..fb63edbf0c4 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -22,6 +22,7 @@ class PostAction < ActiveRecord::Base after_save :update_counters after_save :enforce_rules + after_save :create_user_action after_commit :notify_subscribers def disposed_by_id @@ -453,6 +454,12 @@ SQL SpamRulesEnforcer.enforce!(post.user) end + def create_user_action + if is_bookmark? || is_like? + UserActionCreator.log_post_action(self) + end + end + def notify_subscribers if (is_like? || is_flag?) && post post.publish_change_to_clients! :acted diff --git a/app/models/topic.rb b/app/models/topic.rb index 7ce360bc93f..86674dfbb77 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -200,6 +200,7 @@ class Topic < ActiveRecord::Base end SearchIndexer.index(self) + UserActionCreator.log_topic(self) end def initialize_default_values diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index f4369f56988..49c43e7b4e0 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -132,8 +132,6 @@ SQL if rows == 0 create_missing_record(user_id, topic_id, attrs) - else - observe_after_save_callbacks_for topic_id, user_id end end @@ -203,8 +201,6 @@ SQL if rows == 0 change(user_id, topic_id, last_visited_at: now, first_visited_at: now) - else - observe_after_save_callbacks_for(topic_id, user_id) end end @@ -323,11 +319,6 @@ SQL end end - def observe_after_save_callbacks_for(topic_id, user_id) - TopicUser.where(topic_id: topic_id, user_id: user_id).each do |topic_user| - UserActionObserver.instance.after_save topic_user - end - end end def self.update_post_action_cache(opts={}) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 00e27ab3727..4f2d9026a72 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -343,7 +343,7 @@ class PostAlerter end end - UserActionObserver.log_notification(original_post, user, type, opts[:acting_user_id]) + UserActionCreator.log_notification(original_post, user, type, opts[:acting_user_id]) topic_title = post.topic.title # when sending a private message email, keep the original title diff --git a/app/models/user_action_observer.rb b/app/services/user_action_creator.rb similarity index 88% rename from app/models/user_action_observer.rb rename to app/services/user_action_creator.rb index 932e8254ed4..bbf21f9fb9d 100644 --- a/app/models/user_action_observer.rb +++ b/app/services/user_action_creator.rb @@ -1,18 +1,15 @@ -class UserActionObserver < ActiveRecord::Observer - observe :post_action, :topic, :post, :notification, :topic_user +class UserActionCreator + def self.disable + @disabled = true + end - def after_save(model) - case - when (model.is_a?(PostAction) && (model.is_bookmark? || model.is_like?)) - log_post_action(model) - when (model.is_a?(Topic)) - log_topic(model) - when (model.is_a?(Post)) - UserActionObserver.log_post(model) - end + def self.enable + @disabled = false end def self.log_notification(post, user, notification_type, acting_user_id=nil) + return if @disabled + action = case notification_type when Notification.types[:quoted] @@ -44,6 +41,8 @@ class UserActionObserver < ActiveRecord::Observer end def self.log_post(model) + return if @disabled + # first post gets nada return if model.is_first_post? return if model.topic.blank? @@ -78,7 +77,8 @@ class UserActionObserver < ActiveRecord::Observer end end - def log_topic(model) + def self.log_topic(model) + return if @disabled # no action to log here, this can happen if a user is deleted # then topic has no user_id @@ -113,7 +113,9 @@ class UserActionObserver < ActiveRecord::Observer end end - def log_post_action(model) + def self.log_post_action(model) + return if @disabled + action = UserAction::BOOKMARK if model.is_bookmark? action = UserAction::LIKE if model.is_like? diff --git a/config/application.rb b/config/application.rb index 249f4e90d9a..e8189eed1b7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -83,7 +83,6 @@ module Discourse # Activate observers that should always be running. config.active_record.observers = [ :user_email_observer, - :user_action_observer, :post_alert_observer, ] diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index fbb9bff2f5f..ec4047bce18 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -187,7 +187,7 @@ class PostDestroyer def recover_user_actions # TODO: Use a trash concept for `user_actions` to avoid churn and simplify this? - UserActionObserver.log_post(@post) + UserActionCreator.log_post(@post) end def remove_associated_replies diff --git a/lib/post_revisor.rb b/lib/post_revisor.rb index 4fea530847f..bf12588e709 100644 --- a/lib/post_revisor.rb +++ b/lib/post_revisor.rb @@ -250,7 +250,7 @@ class PostRevisor prev_owner = User.find(@post.user_id) new_owner = User.find(@fields["user_id"]) - # UserActionObserver will create new UserAction records for the new owner + # UserActionCreator will create new UserAction records for the new owner UserAction.where(target_post_id: @post.id) .where(user_id: prev_owner.id) diff --git a/lib/tasks/user_actions.rake b/lib/tasks/user_actions.rake index b9ced27e4e5..ffba1f2b228 100644 --- a/lib/tasks/user_actions.rake +++ b/lib/tasks/user_actions.rake @@ -1,13 +1,15 @@ desc "rebuild the user_actions table" task "user_actions:rebuild" => :environment do - o = UserActionObserver.send :new MessageBus.off UserAction.delete_all - PostAction.all.each{|i| o.after_save(i)} - Topic.all.each {|i| o.after_save(i)} - Post.all.each {|i| o.after_save(i)} - Notification.all.each {|i| o.after_save(i)} - # not really needed but who knows - MessageBus.on + PostAction.all.each{|i| UserActionCreator.log_post_action(i)} + Topic.all.each {|i| UserActionCreator.log_topic(i)} + Post.all.each {|i| UserActionCreator.log_post(i)} + Notification.all.each do |notification| + UserActionCreator.log_notification(notification.post, + notification.user, + notification.notification_type, + notification.user) + end end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index a0ba06c8818..c4eb2261367 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -107,6 +107,8 @@ describe PostCreator do it "generates the correct messages for a secure topic" do + UserActionCreator.enable + admin = Fabricate(:admin) cat = Fabricate(:category) @@ -140,6 +142,8 @@ describe PostCreator do it 'generates the correct messages for a normal topic' do + UserActionCreator.enable + p = nil messages = MessageBus.track_publish do p = creator.create diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 6a506fd6a00..05690326301 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -5,6 +5,7 @@ describe PostDestroyer do before do ActiveRecord::Base.observers.enable :all + UserActionCreator.enable end let(:moderator) { Fabricate(:moderator) } diff --git a/spec/controllers/user_actions_controller_spec.rb b/spec/controllers/user_actions_controller_spec.rb index 52583c7f86a..c4efbfbcdaf 100644 --- a/spec/controllers/user_actions_controller_spec.rb +++ b/spec/controllers/user_actions_controller_spec.rb @@ -10,6 +10,7 @@ describe UserActionsController do it 'renders list correctly' do ActiveRecord::Base.observers.enable :all + UserActionCreator.enable post = Fabricate(:post) xhr :get, :index, username: post.user.username diff --git a/spec/models/directory_item_spec.rb b/spec/models/directory_item_spec.rb index 6612686e3f7..c7a0fa6efbe 100644 --- a/spec/models/directory_item_spec.rb +++ b/spec/models/directory_item_spec.rb @@ -21,6 +21,7 @@ describe DirectoryItem do context 'refresh' do before do ActiveRecord::Base.observers.enable :all + UserActionCreator.enable end let!(:post) { create_post } diff --git a/spec/models/post_mover_spec.rb b/spec/models/post_mover_spec.rb index 1a8077804ca..f6bd9bc7699 100644 --- a/spec/models/post_mover_spec.rb +++ b/spec/models/post_mover_spec.rb @@ -33,6 +33,7 @@ describe PostMover do p2.replies << p4 # add a like to a post, enable observers so we get user actions ActiveRecord::Base.observers.enable :all + UserActionCreator.enable @like = PostAction.act(another_user, p4, PostActionType.types[:like]) end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 71c7731818a..edeafa5a393 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -430,6 +430,7 @@ describe Topic do it "should set up actions correctly" do ActiveRecord::Base.observers.enable :all + UserActionCreator.enable expect(actions.map{|a| a.action_type}).not_to include(UserAction::NEW_TOPIC) expect(actions.map{|a| a.action_type}).to include(UserAction::NEW_PRIVATE_MESSAGE) diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb index 3259316c22c..e92e694306b 100644 --- a/spec/models/topic_user_spec.rb +++ b/spec/models/topic_user_spec.rb @@ -174,11 +174,6 @@ describe TopicUser do expect(topic_user.last_visited_at.to_i).to eq(today.to_i) end end - - it 'triggers the observer callbacks when updating' do - UserActionObserver.instance.expects(:after_save).twice - 2.times { TopicUser.track_visit!(topic.id, user.id) } - end end describe 'read tracking' do diff --git a/spec/models/trust_level3_requirements_spec.rb b/spec/models/trust_level3_requirements_spec.rb index 46e3d899929..cb0a2b4e445 100644 --- a/spec/models/trust_level3_requirements_spec.rb +++ b/spec/models/trust_level3_requirements_spec.rb @@ -221,7 +221,7 @@ describe TrustLevel3Requirements do describe "num_likes_given" do it "counts likes given in the last 100 days" do - ActiveRecord::Base.observers.enable :user_action_observer + UserActionCreator.enable recent_post1 = create_post(created_at: 1.hour.ago) recent_post2 = create_post(created_at: 10.days.ago) @@ -237,7 +237,7 @@ describe TrustLevel3Requirements do describe "num_likes_received" do it "counts likes received in the last 100 days" do - ActiveRecord::Base.observers.enable :user_action_observer + UserActionCreator.enable t = Fabricate(:topic, user: user, created_at: 102.days.ago) old_post = create_post(topic: t, user: user, created_at: 102.days.ago) diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 79b7b4e5c89..6f0c37df7be 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -4,6 +4,7 @@ describe UserAction do before do ActiveRecord::Base.observers.enable :all + UserActionCreator.enable end it { is_expected.to validate_presence_of :action_type } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 614cf17697e..de661098940 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -922,6 +922,7 @@ describe User do before do # To make testing easier, say 1 reply is too much SiteSetting.stubs(:newuser_max_replies_per_topic).returns(1) + UserActionCreator.enable end context "for a user who didn't create the topic" do @@ -938,7 +939,10 @@ describe User do context "with a reply" do before do - PostCreator.new(Fabricate(:user), raw: 'whatever this is a raw post', topic_id: topic.id, reply_to_post_number: post.post_number).create + PostCreator.new(Fabricate(:user), + raw: 'whatever this is a raw post', + topic_id: topic.id, + reply_to_post_number: post.post_number).create end it "resets the `posted_too_much` threshold" do diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 24efd0106ff..7bfdeb29bcb 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -108,6 +108,7 @@ Spork.prefork do # ActiveRecord::Base.observers.disable :all SearchIndexer.disable + UserActionCreator.disable SiteSetting.provider.all.each do |setting| SiteSetting.remove_override!(setting.name) diff --git a/spec/services/post_owner_changer_spec.rb b/spec/services/post_owner_changer_spec.rb index b1452d35c45..bdbb36541a8 100644 --- a/spec/services/post_owner_changer_spec.rb +++ b/spec/services/post_owner_changer_spec.rb @@ -74,7 +74,8 @@ describe PostOwnerChanger do UserAction.create!( action_type: UserAction::REPLY, user_id: p2user.id, acting_user_id: p2user.id, target_post_id: p2.id, target_topic_id: p2.topic_id, created_at: p2.created_at ) - ActiveRecord::Base.observers.enable :user_action_observer + + UserActionCreator.enable end subject(:change_owners) { described_class.new(post_ids: [p1.id, p2.id], topic_id: topic.id, new_owner: user_a, acting_user: editor).change_owner! }