Don't send comment email to suspended or inactive users.

app/models/polymorphic/comment.rb

@@ -52,10 +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|
-      if subscriber = User.find_by_id(subscriber_id)
-        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/factories/user_factories.rb

@@ -31,6 +31,7 @@
     deleted_at          { nil }
     updated_at          { FactoryBot.generate(:time) }
     created_at          { FactoryBot.generate(:time) }
+    confirmed_at        { Time.now.utc }
     suspended_at        { nil }
     password            { "password" }
     password_confirmation { "password" }

spec/features/devise/sign_in_spec.rb

@@ -15,7 +15,8 @@
                    password: 'password',
                    password_confirmation: 'password',
                    email: '[email protected]',
-                   sign_in_count: 0
+                   sign_in_count: 0,
+                   confirmed_at: nil
   end
 
   scenario 'without confirmation' do

spec/models/polymorphic/comment_spec.rb

@@ -42,4 +42,36 @@
       expect(entity.subscribed_users).to include(user.id)
     end
   end
+
+  describe "notify_subscribers" do
+    let(:subscriber) { create(:user) }
+    let(:entity) { create(:lead, subscribed_users: [subscriber.id]) }
+    before(:each) do
+      allow(SubscriptionMailer).to receive_message_chain(:comment_notification, :deliver_later)
+    end
+
+    it "should notify subscribers when a comment is added" do
+      Comment.create!(comment: "Hello", user: create(:user), commentable: entity)
+      expect(SubscriptionMailer).to have_received(:comment_notification).with(subscriber, instance_of(Comment))
+    end
+
+    it "should not notify the user who created the comment" do
+      user = create(:user, confirmed_at: Time.now, email: "[email protected]")
+      Comment.create!(comment: "Hello", user: user, commentable: entity)
+      expect(SubscriptionMailer).not_to have_received(:comment_notification).with(user, instance_of(Comment))
+    end
+
+    it "should not notify suspended users" do
+      subscriber.update(suspended_at: Time.now)
+      Comment.create!(comment: "Hello", user: create(:user), commentable: entity)
+      expect(SubscriptionMailer).not_to have_received(:comment_notification).with(subscriber, instance_of(Comment))
+    end
+
+    it "should not notify users awaiting approval" do
+      subscriber.update(sign_in_count: 0, suspended_at: Time.now)
+      allow(Setting).to receive(:user_signup).and_return(:not_allowed)
+      Comment.create!(comment: "Hello", user: create(:user), commentable: entity)
+      expect(SubscriptionMailer).not_to have_received(:comment_notification).with(subscriber, instance_of(Comment))
+    end
+  end
 end

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