Merge pull request #1332 from locks/user_email_observer

User email observer (Code Climate improvements)

app/models/user_email_observer.rb

@@ -1,78 +1,78 @@
 class UserEmailObserver < ActiveRecord::Observer
   observe :notification
 
-  def after_commit(notification)
+  class EmailUser
-    if rails4?
+    attr_reader :notification
-      if notification.send(:transaction_include_any_action?, [:create])
-        notification_type = Notification.types[notification.notification_type]
 
-        # Delegate to email_user_{{NOTIFICATION_TYPE}} if exists
+    def initialize(notification)
-        email_method = :"email_user_#{notification_type.to_s}"
+      @notification = notification
-        send(email_method, notification) if respond_to?(email_method)
-      end
-    else
-      if notification.send(:transaction_include_action?, :create)
-        notification_type = Notification.types[notification.notification_type]
-
-        # Delegate to email_user_{{NOTIFICATION_TYPE}} if exists
-        email_method = :"email_user_#{notification_type.to_s}"
-
-        send(email_method, notification) if respond_to?(email_method)
-      end
-    end
     end
 
-  def email_user_mentioned(notification)
+    def mentioned
+      enqueue :user_mentioned
+    end
+
+    def posted
+      enqueue :user_posted
+    end
+
+    def quoted
+      enqueue :user_quoted
+    end
+
+    def replied
+      enqueue :user_replied
+    end
+
+    def private_message
+      enqueue_private :user_private_message
+    end
+
+    def invited_to_private_message
+      enqueue :user_invited_to_private_message
+    end
+
+    private
+
+    def enqueue(type)
       return unless notification.user.email_direct?
       Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
                      :user_email,
-                   type: :user_mentioned,
+                     type: type,
                      user_id: notification.user_id,
                      notification_id: notification.id)
     end
 
-  def email_user_posted(notification)
+    def enqueue_private(type)
-    return unless notification.user.email_direct?
-    Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
-                   :user_email,
-                   type: :user_posted,
-                   user_id: notification.user_id,
-                   notification_id: notification.id)
-  end
-
-  def email_user_quoted(notification)
-    return unless notification.user.email_direct?
-    Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
-                   :user_email,
-                   type: :user_quoted,
-                   user_id: notification.user_id,
-                   notification_id: notification.id)
-  end
-
-  def email_user_replied(notification)
-    return unless notification.user.email_direct?
-    Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
-                    :user_email,
-                    type: :user_replied,
-                    user_id: notification.user_id,
-                    notification_id: notification.id)
-  end
-
-  def email_user_private_message(notification)
       return unless (notification.user.email_direct? && notification.user.email_private_messages?)
       Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
                       :user_email,
-                    type: :user_private_message,
+                      type: type,
                       user_id: notification.user_id,
                       notification_id: notification.id)
     end
+  end
 
-  def email_user_invited_to_private_message(notification)
+  def after_commit(notification)
-    return unless notification.user.email_direct?
+    transaction_includes_action = if rails4?
-    Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
+      notification.send(:transaction_include_any_action?, [:create])
-                    :user_email,
+    else
-                    type: :user_invited_to_private_message,
+      notification.send(:transaction_include_action?, :create)
-                    user_id: notification.user_id,
+    end
-                    notification_id: notification.id)
+
+    delegate_to_email_user notification if transaction_includes_action
+  end
+
+  private
+
+  def extract_notification_type(notification)
+    Notification.types[notification.notification_type]
+  end
+
+  def delegate_to_email_user(notification)
+    email_user   = EmailUser.new(notification)
+    email_method = extract_notification_type notification
+
+    email_user.send(email_method) if email_user.respond_to? email_method
   end
 end

spec/models/notification_spec.rb

@@ -109,7 +109,7 @@ describe Notification do
   describe '@mention' do
 
     it "calls email_user_mentioned on creating a notification" do
-      UserEmailObserver.any_instance.expects(:email_user_mentioned).with(instance_of(Notification))
+      UserEmailObserver.any_instance.expects(:after_commit).with(instance_of(Notification))
       Fabricate(:notification)
     end
 
@@ -117,7 +117,7 @@ describe Notification do
 
   describe '@mention' do
     it "calls email_user_quoted on creating a quote notification" do
-      UserEmailObserver.any_instance.expects(:email_user_quoted).with(instance_of(Notification))
+      UserEmailObserver.any_instance.expects(:after_commit).with(instance_of(Notification))
       Fabricate(:quote_notification)
     end
   end

spec/models/user_email_observer_spec.rb

@@ -9,13 +9,13 @@ describe UserEmailObserver do
 
     it "enqueues a job for the email" do
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_mentioned, user_id: notification.user_id, notification_id: notification.id)
-      UserEmailObserver.send(:new).email_user_mentioned(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
     it "doesn't enqueue an email if the user has mention emails disabled" do
       user.expects(:email_direct?).returns(false)
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_mentioned)).never
-      UserEmailObserver.send(:new).email_user_mentioned(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
   end
@@ -23,17 +23,17 @@ describe UserEmailObserver do
   context 'posted' do
 
     let(:user) { Fabricate(:user) }
-    let!(:notification) { Fabricate(:notification, user: user) }
+    let!(:notification) { Fabricate(:notification, user: user, notification_type: 9) }
 
     it "enqueues a job for the email" do
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_posted, user_id: notification.user_id, notification_id: notification.id)
-      UserEmailObserver.send(:new).email_user_posted(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
     it "doesn't enqueue an email if the user has mention emails disabled" do
       user.expects(:email_direct?).returns(false)
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_posted)).never
-      UserEmailObserver.send(:new).email_user_posted(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
   end
@@ -41,17 +41,17 @@ describe UserEmailObserver do
   context 'user_replied' do
 
     let(:user) { Fabricate(:user) }
-    let!(:notification) { Fabricate(:notification, user: user) }
+    let!(:notification) { Fabricate(:notification, user: user, notification_type: 2) }
 
     it "enqueues a job for the email" do
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_replied, user_id: notification.user_id, notification_id: notification.id)
-      UserEmailObserver.send(:new).email_user_replied(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
     it "doesn't enqueue an email if the user has mention emails disabled" do
       user.expects(:email_direct?).returns(false)
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_replied)).never
-      UserEmailObserver.send(:new).email_user_replied(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
   end
@@ -59,17 +59,17 @@ describe UserEmailObserver do
   context 'user_quoted' do
 
     let(:user) { Fabricate(:user) }
-    let!(:notification) { Fabricate(:notification, user: user) }
+    let!(:notification) { Fabricate(:notification, user: user, notification_type: 3) }
 
     it "enqueues a job for the email" do
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_quoted, user_id: notification.user_id, notification_id: notification.id)
-      UserEmailObserver.send(:new).email_user_quoted(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
     it "doesn't enqueue an email if the user has mention emails disabled" do
       user.expects(:email_direct?).returns(false)
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_quoted)).never
-      UserEmailObserver.send(:new).email_user_quoted(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
   end
@@ -77,17 +77,17 @@ describe UserEmailObserver do
   context 'email_user_invited_to_private_message' do
 
     let(:user) { Fabricate(:user) }
-    let!(:notification) { Fabricate(:notification, user: user) }
+    let!(:notification) { Fabricate(:notification, user: user, notification_type: 7) }
 
     it "enqueues a job for the email" do
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, type: :user_invited_to_private_message, user_id: notification.user_id, notification_id: notification.id)
-      UserEmailObserver.send(:new).email_user_invited_to_private_message(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
     it "doesn't enqueue an email if the user has mention emails disabled" do
       user.expects(:email_direct?).returns(false)
       Jobs.expects(:enqueue_in).with(SiteSetting.email_time_window_mins.minutes, :user_email, has_entry(type: :user_invited_to_private_message)).never
-      UserEmailObserver.send(:new).email_user_invited_to_private_message(notification)
+      UserEmailObserver.send(:new).after_commit(notification)
     end
 
   end