UserEmailObserver is now removed

no big surprises here was pretty straightforward

after_commit semantics sure are weird though

app/models/notification.rb

@@ -15,6 +15,8 @@ class Notification < ActiveRecord::Base
   after_save :refresh_notification_count
   after_destroy :refresh_notification_count
 
+  after_commit :send_email
+
   def self.ensure_consistency!
     Notification.exec_sql("
     DELETE FROM Notifications n WHERE notification_type = :id AND
@@ -196,6 +198,11 @@ class Notification < ActiveRecord::Base
     user.publish_notifications_state
   end
 
+  def send_email
+    transaction_includes_action = self.send(:transaction_include_any_action?, [:create])
+    NotificationEmailer.process_notification(self) if transaction_includes_action
+  end
+
 end
 
 # == Schema Information

app/services/notification_emailer.rb

@@ -1,5 +1,4 @@
-class UserEmailObserver < ActiveRecord::Observer
+class NotificationEmailer
-  observe :notification
 
   class EmailUser
     attr_reader :notification
@@ -105,12 +104,17 @@ class UserEmailObserver < ActiveRecord::Observer
 
   end
 
-  def after_commit(notification)
+  def self.disable
-    transaction_includes_action = notification.send(:transaction_include_any_action?, [:create])
+    @disabled = true
-    self.class.process_notification(notification) if transaction_includes_action
+  end
+
+  def self.enable
+    @disabled = false
   end
 
   def self.process_notification(notification)
+    return if @disabled
+
     email_user   = EmailUser.new(notification)
     email_method = Notification.types[notification.notification_type]
 

config/application.rb

@@ -82,7 +82,6 @@ module Discourse
 
     # Activate observers that should always be running.
     config.active_record.observers = [
-        :user_email_observer,
         :post_alert_observer,
     ]
 

spec/models/category_user_spec.rb

@@ -69,6 +69,7 @@ describe CategoryUser do
   context 'integration' do
     before do
       ActiveRecord::Base.observers.enable :all
+      NotificationEmailer.enable
     end
 
     it 'should operate correctly' do

spec/models/notification_spec.rb

@@ -3,6 +3,7 @@ require 'rails_helper'
 describe Notification do
   before do
     ActiveRecord::Base.observers.enable :all
+    NotificationEmailer.enable
   end
 
   it { is_expected.to validate_presence_of :notification_type }
@@ -139,21 +140,6 @@ describe Notification do
     end
   end
 
-  describe '@mention' do
-
-    it "calls email_user_mentioned on creating a notification" do
-      UserEmailObserver.any_instance.expects(:after_commit).with(instance_of(Notification))
-      Fabricate(:notification)
-    end
-
-  end
-
-  describe '@mention' do
-    it "calls email_user_quoted on creating a quote notification" do
-      UserEmailObserver.any_instance.expects(:after_commit).with(instance_of(Notification))
-      Fabricate(:quote_notification)
-    end
-  end
 
   describe 'private message' do
     before do
@@ -250,6 +236,8 @@ describe Notification do
     it 'deletes notifications if post is missing or deleted' do
 
       ActiveRecord::Base.observers.disable :all
+      NotificationEmailer.disable
+
       p = Fabricate(:post)
       p2 = Fabricate(:post)
 

spec/rails_helper.rb

@@ -109,6 +109,7 @@ Spork.prefork do
       ActiveRecord::Base.observers.disable :all
       SearchIndexer.disable
       UserActionCreator.disable
+      NotificationEmailer.disable
 
       SiteSetting.provider.all.each do |setting|
         SiteSetting.remove_override!(setting.name)

spec/services/notification_emailer_spec.rb

@@ -1,6 +1,10 @@
 require 'rails_helper'
 
-describe UserEmailObserver do
+describe NotificationEmailer do
+
+  before do
+    NotificationEmailer.enable
+  end
 
   let(:topic) { Fabricate(:topic) }
   let(:post) { Fabricate(:post, topic: topic) }
@@ -18,8 +22,8 @@ describe UserEmailObserver do
   shared_examples "enqueue" do
 
     it "enqueues a job for the email" do
-      Jobs.expects(:enqueue_in).with(delay, :user_email, UserEmailObserver::EmailUser.notification_params(notification,type))
+      Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification,type))
-      UserEmailObserver.process_notification(notification)
+      NotificationEmailer.process_notification(notification)
     end
 
     context "inactive user" do
@@ -27,13 +31,13 @@ describe UserEmailObserver do
 
       it "doesn't enqueue a job" do
         Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
-        UserEmailObserver.process_notification(notification)
+        NotificationEmailer.process_notification(notification)
       end
 
       it "enqueues a job if the user is staged" do
         notification.user.staged = true
-        Jobs.expects(:enqueue_in).with(delay, :user_email, UserEmailObserver::EmailUser.notification_params(notification,type))
+        Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification,type))
-        UserEmailObserver.process_notification(notification)
+        NotificationEmailer.process_notification(notification)
       end
     end
 
@@ -46,7 +50,7 @@ describe UserEmailObserver do
 
       it "doesn't enqueue a job" do
         Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
-        UserEmailObserver.process_notification(notification)
+        NotificationEmailer.process_notification(notification)
       end
     end
 
@@ -55,7 +59,7 @@ describe UserEmailObserver do
       it "doesn't enqueue a job" do
         Post.any_instance.expects(:post_type).returns(Post.types[:small_action])
         Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
-        UserEmailObserver.process_notification(notification)
+        NotificationEmailer.process_notification(notification)
       end
 
     end
@@ -68,7 +72,7 @@ describe UserEmailObserver do
     it "doesn't enqueue a job if the user has mention emails disabled" do
       notification.user.user_option.update_columns(email_direct: false)
       Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
-      UserEmailObserver.process_notification(notification)
+      NotificationEmailer.process_notification(notification)
     end
   end
 
@@ -78,7 +82,7 @@ describe UserEmailObserver do
     it "doesn't enqueue a job if the user has private message emails disabled" do
       notification.user.user_option.update_columns(email_private_messages: false)
       Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
-      UserEmailObserver.process_notification(notification)
+      NotificationEmailer.process_notification(notification)
     end
 
   end
@@ -92,8 +96,8 @@ describe UserEmailObserver do
 
     it "enqueue a delayed job for users that are online" do
       notification.user.last_seen_at = 1.minute.ago
-      Jobs.expects(:enqueue_in).with(delay, :user_email, UserEmailObserver::EmailUser.notification_params(notification,type))
+      Jobs.expects(:enqueue_in).with(delay, :user_email, NotificationEmailer::EmailUser.notification_params(notification,type))
-      UserEmailObserver.process_notification(notification)
+      NotificationEmailer.process_notification(notification)
     end
 
   end
@@ -140,7 +144,7 @@ describe UserEmailObserver do
     it "doesn't enqueue a job for a small action" do
       notification.data_hash["original_post_type"] = Post.types[:small_action]
       Jobs.expects(:enqueue_in).with(delay, :user_email, has_entry(type: type)).never
-      UserEmailObserver.process_notification(notification)
+      NotificationEmailer.process_notification(notification)
     end
 
   end