Merge pull request #14964 from krauselukas/align_is_moderator_method

Align `is_moderator?` method and report pundit policies

src/api/app/controllers/webui/users/notifications_controller.rb

@@ -39,7 +39,7 @@ def update
           notifications: paginated_notifications,
           selected_filter: selected_filter,
           show_read_all_button: show_read_all_button?,
-          show_reports: Flipper.enabled?(:content_moderation, User.session) && User.session.is_moderator?
+          show_reports: ReportPolicy.new(User.session, Report).notify?
         }
       end
     end

src/api/app/jobs/send_event_emails_job.rb

@@ -32,7 +32,7 @@ def send_email(subscribers, event)
 
   def event_subscribers(event:)
     if event.is_a?(Event::CreateReport)
-      event.subscribers.filter_map { |subscriber| subscriber if Flipper.enabled?(:content_moderation, subscriber) && subscriber.is_moderator? }
+      event.subscribers.filter_map { |subscriber| subscriber if ReportPolicy.new(subscriber, Report).notify? }
     else
       event.subscribers
     end

src/api/app/models/event_subscription/form.rb

@@ -45,8 +45,9 @@ def find_or_initialize_subscription(eventtype, receiver_role, channel)
 
     def show_form_for_create_report_event?(event_class:, subscriber:)
       if event_class.name == 'Event::CreateReport'
-        return false unless subscriber.try(:is_moderator?)
-        return false unless Flipper.enabled?(:content_moderation, subscriber)
+        # There is no subscriber for the global subscription configuration
+        return false if subscriber.blank?
+        return false unless ReportPolicy.new(subscriber, Report).notify?
       end
 
       true

src/api/app/models/user.rb

@@ -409,7 +409,7 @@ def is_nobody?
   end
 
   def is_moderator?
-    is_admin? || is_staff?
+    roles.exists?(title: 'Moderator')
   end
 
   def is_active?

src/api/app/policies/comment_policy.rb

@@ -32,10 +32,11 @@ def reply?
     !(user.blank? || user.is_nobody? || record.user.is_nobody?)
   end
 
-  # Only logged-in Admins of Staff members can moderate comments
+  # Only logged-in Admins/Staff members or user with moderator role can moderate comments
   def moderate?
     return false if record.user.is_nobody? # soft-deleted comments
-    return true if user.try(:is_admin?) || user.try(:is_staff?)
+    return false if user == record.user
+    return true if user.try(:is_moderator?) || user.try(:is_admin?) || user.try(:is_staff?)
 
     false
   end

src/api/app/policies/decision_policy.rb

@@ -2,6 +2,6 @@ class DecisionPolicy < ApplicationPolicy
   def create?
     return false unless Flipper.enabled?(:content_moderation, user)
 
-    user.is_admin? || user.is_moderator?
+    user.is_moderator? || user.is_admin? || user.is_staff?
   end
 end

src/api/app/policies/report_policy.rb

@@ -21,4 +21,12 @@ def create?
       !UserPolicy.new(user, record.reportable).update?
     end
   end
+
+  def notify?
+    return false unless Flipper.enabled?(:content_moderation, user)
+    return true if User.moderators.blank? && (user.is_admin? || user.is_staff?)
+    return true if user.is_moderator?
+
+    false
+  end
 end

src/api/app/services/notification_service/notifier.rb

@@ -61,6 +61,12 @@ def create_notification?(subscriber, channel)
       true
     end
 
+    def create_report_notification?(event:, subscriber:)
+      return false if event.is_a?(Event::CreateReport) && !ReportPolicy.new(subscriber, Report).notify?
+
+      true
+    end
+
     def notifiable_exists?
       # We need this check because the notification is created in a delayed job.
       # So the notifiable object could have been removed in the meantime.
@@ -70,14 +76,5 @@ def notifiable_exists?
       notifiable_id = @event.parameters_for_notification[:notifiable_id]
       notifiable_type.exists?(notifiable_id)
     end
-
-    def create_report_notification?(event:, subscriber:)
-      if event.is_a?(Event::CreateReport)
-        return false unless Flipper.enabled?(:content_moderation, subscriber)
-        return false unless subscriber.is_moderator?
-      end
-
-      true
-    end
   end
 end

src/api/spec/factories/users.rb

@@ -45,6 +45,10 @@
         roles { [Role.find_by_title('Staff')] }
       end
 
+      factory :moderator do
+        roles { [Role.find_by_title('Moderator')] }
+      end
+
       factory :user_with_groups do
         after(:create) do |user|
           create(:group, users: [user])

src/api/spec/policies/comment_policy_spec.rb

@@ -176,6 +176,14 @@
         expect(subject).to permit(staff_user, comment)
       end
     end
+
+    context 'when the user has the moderator role assigned' do
+      let(:user_with_moderator_role) { create(:moderator) }
+
+      it 'can moderate comments' do
+        expect(subject).to permit(user_with_moderator_role, comment)
+      end
+    end
   end
   # rubocop:enable RSpec/RepeatedExample
 end

src/api/spec/policies/report_policy_spec.rb

@@ -5,6 +5,10 @@
 
   let(:user) { create(:confirmed_user) }
 
+  before do
+    Flipper.enable(:content_moderation)
+  end
+
   permissions :show? do
     context 'when the current user is the owner of the report' do
       let(:report) { create(:report, user: user) }
@@ -27,10 +31,6 @@
   end
 
   permissions :create? do
-    before do
-      Flipper.enable(:content_moderation, user)
-    end
-
     context 'when the current user has already reported it' do
       let(:reported_comment) { create(:comment_package) }
       let(:report) { build(:report, user: user, reportable: reported_comment) }
@@ -101,4 +101,35 @@
       end
     end
   end
+
+  permissions :notify? do
+    let(:staff_user) { create(:staff_user) }
+    let(:admin_user) { create(:admin_user) }
+
+    context 'when there is no user with moderator role' do
+      it 'notifies admin users' do
+        expect(subject).to permit(admin_user, Report)
+      end
+
+      it 'notifies staff users' do
+        expect(subject).to permit(staff_user, Report)
+      end
+    end
+
+    context 'when there is a user with moderator role' do
+      let!(:moderator_user) { create(:moderator) }
+
+      it 'does not notify admin users' do
+        expect(subject).not_to(permit(admin_user, Report))
+      end
+
+      it 'does not notify staff users' do
+        expect(subject).not_to(permit(staff_user, Report))
+      end
+
+      it 'notifies the moderator' do
+        expect(subject).to permit(moderator_user, Report)
+      end
+    end
+  end
 end