Moved emailable logic to user model. Adjust AREL clause to only load …

…necessary users.

app/models/polymorphic/comment.rb

@@ -52,11 +52,9 @@ def subscribe_user_to_entity(u = user)
 
   # Notify subscribed users when a comment is added, unless user created this comment
   def notify_subscribers
-    commentable.subscribed_users.reject { |user_id| user_id == user.id }.each do |subscriber_id|
-      subscriber = User.find_by_id(subscriber_id)
-      if subscriber&.confirmed? && !subscriber.awaits_approval? && !subscriber.suspended? && subscriber.email.present?
-        SubscriptionMailer.comment_notification(subscriber, self).deliver_later
-      end
+    users_to_notify = User.where(id: commentable.subscribed_users.reject { |user_id| user_id == user.id })
+    users_to_notify.select(&:emailable?).each do |subscriber|
+      SubscriptionMailer.comment_notification(subscriber, self).deliver_later
     end
   end
 

app/models/users/user.rb

@@ -134,6 +134,12 @@ def inactive_message
     end
   end
 
+  # Send emails to active users only
+  #----------------------------------------------------------------------------
+  def emailable?
+    confirmed? && !awaits_approval? && !suspended? && email.present?
+  end
+
   #----------------------------------------------------------------------------
   def preference
     @preference ||= preferences.build

spec/models/users/user_spec.rb

@@ -235,4 +235,24 @@
       expect(search.first).to eql(user)
     end
   end
+
+  describe "emailable?" do
+    let(:user) { create(:user) }
+    it "should return true for standard user" do
+      expect(user.emailable?).to eql(true)
+    end
+    it "should return false for unconfirmed user" do
+      user.update(confirmed_at: nil)
+      expect(user.emailable?).to eql(false)
+    end
+    it "should return false for user awaiting approval" do
+      user.update(sign_in_count: 0, suspended_at: Time.now)
+      allow(Setting).to receive(:user_signup).and_return(:not_allowed)
+      expect(user.emailable?).to eql(false)
+    end
+    it "should return false for suspended user" do
+      user.update(suspended_at: Time.now)
+      expect(user.emailable?).to eql(false)
+    end
+  end
 end