remove UserActionObserver, replace with after_save and service

interestingly there was some left over dead code from when stars
existed in the topic_users table

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)

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

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

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={})

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

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?
 

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,
     ]
 

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

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)

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
 

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

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) }

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

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 }

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
 

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)

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

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)

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 }

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

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)

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! }