refactor check for active user

app/models/user.rb

@@ -618,23 +618,7 @@ class User < Principal
   # Utility method to help check if a user should be notified about an
   # event.
   def notify_about?(object)
-    case mail_notification
-    when 'all'
-      true
-    when 'selected'
-      # user receives notifications for created/assigned issues on unselected projects
-      object.is_a?(WorkPackage) && (object.author == self || is_or_belongs_to?(object.assigned_to))
-    when 'none'
-      false
-    when 'only_my_events'
-      object.is_a?(WorkPackage) && (object.author == self || is_or_belongs_to?(object.assigned_to))
-    when 'only_assigned'
-      object.is_a?(WorkPackage) && is_or_belongs_to?(object.assigned_to)
-    when 'only_owner'
-      object.is_a?(WorkPackage) && object.author == self
-    else
-      false
-    end
+    active? && (mail_notification == 'all' || (object.is_a?(WorkPackage) && object.notify?(self)))
   end
 
   def reported_work_package_count

app/models/work_package.rb

@@ -435,12 +435,12 @@ class WorkPackage < ActiveRecord::Base
     notified = project.notified_users
     # Author and assignee are always notified unless they have been
     # locked or don't want to be notified
-    notified << author if author && author.active? && author.notify_about?(self)
+    notified << author if author && author.notify_about?(self)
     if assigned_to
       if assigned_to.is_a?(Group)
-        notified += assigned_to.users.select { |u| u.active? && u.notify_about?(self) }
+        notified += assigned_to.users.select { |u| u.notify_about?(self) }
       else
-        notified << assigned_to if assigned_to.active? && assigned_to.notify_about?(self)
+        notified << assigned_to if assigned_to.notify_about?(self)
       end
     end
     notified.uniq!
@@ -448,6 +448,21 @@ class WorkPackage < ActiveRecord::Base
     notified.select { |user| visible?(user) }
   end
 
+  def notify?(user)
+    case user.mail_notification
+    when 'selected', 'only_my_events'
+      author == user || user.is_or_belongs_to?(assigned_to)
+    when 'none'
+      false
+    when 'only_assigned'
+      user.is_or_belongs_to?(assigned_to)
+    when 'only_owner'
+      author == user
+    else
+      false
+    end
+  end
+
   def done_ratio
     if WorkPackage.use_status_for_done_ratio? && status && status.default_done_ratio
       status.default_done_ratio

spec/models/user_spec.rb

@@ -554,6 +554,12 @@ describe User, type: :model do
       FactoryGirl.build_stubbed(:role)
     end
 
+    it 'is false for an inactive user' do
+      user.status = User::STATUSES[:locked]
+      user.mail_notification = 'all'
+      expect(user.notify_about?({})).to be_falsey
+    end
+
     context 'Work package' do
       it 'is true for a user with :all' do
         author.mail_notification = 'all'