DEV: UserCommScreener fine-grained actor improvements (#17737)

This commit introduces several fine-grained methods
to UserCommScreener which can be used to show the actor
who they are ignoring/muting/blocking DMs from in order
to prevent them initiating conversation with those users
or to display relevant information in the UI to the
actor.

This will be used in a companion PR in discourse-chat,
and is a follow up to 74584ff3ca

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Co-authored-by: Osama Sayegh <asooomaasoooma90@gmail.com>

lib/user_comm_screener.rb

@@ -26,6 +26,11 @@
 #
 # An important note is that **all of these settings do not apply when the actor
 # is a staff member**. So admins and moderators can PM and notify anyone they please.
+#
+# The secondary usage of this class is to determine which users the actor themselves
+# are muting, ignoring, or preventing private messages from. This is useful when
+# wanting to alert the actor to these users in the UI in various ways, or prevent
+# the actor from communicating with users they prefer not to talk with.
 class UserCommScreener
   attr_reader :acting_user, :preferences
 
@@ -124,6 +129,7 @@ class UserCommScreener
   # Whether the user is ignoring or muting the actor, meaning the actor cannot
   # PM or send notifications to this target user.
   def ignoring_or_muting_actor?(user_id)
+    validate_user_id!(user_id)
     preferences.ignoring_or_muting?(user_id)
   end
 
@@ -133,9 +139,47 @@ class UserCommScreener
   # implicitly disallows PMs, so we need to take into account those preferences
   # here too.
   def disallowing_pms_from_actor?(user_id)
+    validate_user_id!(user_id)
     preferences.disallowing_pms?(user_id) || ignoring_or_muting_actor?(user_id)
   end
 
+  def actor_allowing_communication
+    @target_users.keys - actor_preventing_communication
+  end
+
+  def actor_preventing_communication
+    (actor_preferences[:ignoring] + actor_preferences[:muting] + actor_preferences[:disallowed_pms_from]).uniq
+  end
+
+  ##
+  # The actor methods below are more fine-grained than the user ones,
+  # since we may want to display more detailed messages to the actor about
+  # their preferences than we do when we are informing the actor that
+  # they cannot communicate with certain users.
+  #
+  # In this spirit, actor_disallowing_pms? is intentionally different from
+  # disallowing_pms_from_actor? above.
+
+  def actor_ignoring?(user_id)
+    validate_user_id!(user_id)
+    actor_preferences[:ignoring].include?(user_id)
+  end
+
+  def actor_muting?(user_id)
+    validate_user_id!(user_id)
+    actor_preferences[:muting].include?(user_id)
+  end
+
+  def actor_disallowing_pms?(user_id)
+    validate_user_id!(user_id)
+    return false if !acting_user.user_option.enable_allowed_pm_users
+    actor_preferences[:disallowed_pms_from].include?(user_id)
+  end
+
+  def actor_disallowing_all_pms?
+    !acting_user.user_option.allow_private_messages
+  end
+
   private
 
   def users_with_no_preference
@@ -193,6 +237,21 @@ class UserCommScreener
     UserCommPrefs.new(acting_user, resolved_user_communication_preferences)
   end
 
+  def actor_preferences
+    @actor_preferences ||= begin
+      user_ids_by_preference_type = actor_communication_preferences.reduce({}) do |hash, pref|
+        hash[pref.preference_type] ||= []
+        hash[pref.preference_type] << pref.target_user_id
+        hash
+      end
+      {
+        muting: user_ids_by_preference_type["muted"],
+        ignoring: user_ids_by_preference_type["ignored"],
+        disallowed_pms_from: user_ids_by_preference_type["disallowed_pm"]
+      }
+    end
+  end
+
   def user_communication_preferences
     @user_communication_preferences ||= DB.query(<<~SQL, acting_user_id: acting_user.id, target_user_ids: @target_users.keys)
       SELECT users.id,
@@ -213,4 +272,27 @@ class UserCommScreener
       )
     SQL
   end
+
+  def actor_communication_preferences
+    @actor_communication_preferences ||= DB.query(<<~SQL, acting_user_id: acting_user.id, target_user_ids: @target_users.keys)
+      SELECT users.id AS target_user_id, 'disallowed_pm' AS preference_type FROM users
+      LEFT JOIN allowed_pm_users ON allowed_pm_users.allowed_pm_user_id = users.id
+      WHERE users.id IN (:target_user_ids)
+      AND (allowed_pm_users.user_id = :acting_user_id OR allowed_pm_users.user_id IS NULL)
+      AND allowed_pm_users.allowed_pm_user_id IS NULL
+      UNION
+      SELECT ignored_user_id AS target_user_id, 'ignored' AS preference_type
+      FROM ignored_users
+      WHERE user_id = :acting_user_id AND ignored_user_id IN (:target_user_ids)
+      UNION
+      SELECT muted_user_id AS target_user_id, 'muted' AS preference_type
+      FROM muted_users
+      WHERE user_id = :acting_user_id AND muted_user_id IN (:target_user_ids)
+    SQL
+  end
+
+  def validate_user_id!(user_id)
+    return if user_id == acting_user.id
+    raise Discourse::NotFound if !@target_users.keys.include?(user_id)
+  end
 end

spec/lib/user_comm_screener_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe UserCommScreener do
   end
   fab!(:target_user4) { Fabricate(:user, username: "janescreen") }
   fab!(:target_user5) { Fabricate(:user, username: "maryscreen") }
+  fab!(:other_user) { Fabricate(:user) }
 
   subject do
     described_class.new(
@@ -70,6 +71,10 @@ RSpec.describe UserCommScreener do
       it "returns false for a user neither ignoring or muting the actor" do
         expect(subject.ignoring_or_muting_actor?(target_user3.id)).to eq(false)
       end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.ignoring_or_muting_actor?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
     end
 
     describe "#disallowing_pms_from_actor?" do
@@ -99,6 +104,10 @@ RSpec.describe UserCommScreener do
       it "returns true for a user not disallowing PMs but still muting" do
         expect(subject.disallowing_pms_from_actor?(target_user2.id)).to eq(true)
       end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.disallowing_pms_from_actor?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
     end
   end
 
@@ -137,6 +146,10 @@ RSpec.describe UserCommScreener do
       it "returns false for a user neither ignoring or muting the actor" do
         expect(subject.ignoring_or_muting_actor?(target_user3.id)).to eq(false)
       end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.ignoring_or_muting_actor?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
     end
 
     describe "#disallowing_pms_from_actor?" do
@@ -160,6 +173,77 @@ RSpec.describe UserCommScreener do
       it "returns false for a user not disallowing PMs but still muting" do
         expect(subject.disallowing_pms_from_actor?(target_user2.id)).to eq(false)
       end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.disallowing_pms_from_actor?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
+    end
+  end
+
+  describe "actor preferences" do
+    fab!(:acting_user) { Fabricate(:user) }
+    fab!(:muted_user) { Fabricate(:muted_user, user: acting_user, muted_user: target_user1) }
+    fab!(:ignored_user) { Fabricate(:ignored_user, user: acting_user, ignored_user: target_user2, expiring_at: 2.days.from_now) }
+    fab!(:allowed_pm_user1) { AllowedPmUser.create!(user: acting_user, allowed_pm_user: target_user1) }
+    fab!(:allowed_pm_user2) { AllowedPmUser.create!(user: acting_user, allowed_pm_user: target_user2) }
+    fab!(:allowed_pm_user3) { AllowedPmUser.create!(user: acting_user, allowed_pm_user: target_user4) }
+
+    describe "#actor_preventing_communication" do
+      it "returns the user_ids of the users the actor is ignoring, muting, or disallowing PMs from" do
+        acting_user.user_option.update!(enable_allowed_pm_users: true)
+        expect(subject.actor_preventing_communication).to match_array([
+          target_user1.id, target_user2.id, target_user3.id, target_user5.id
+        ])
+      end
+    end
+
+    describe "#actor_allowing_communication" do
+      it "returns the user_ids of the users who the actor is not ignoring, muting, or disallowing PMs from" do
+        acting_user.user_option.update!(enable_allowed_pm_users: true)
+        expect(subject.actor_allowing_communication).to match_array([target_user4.id])
+      end
+    end
+
+    describe "#actor_ignoring?" do
+      it "returns true for user ids that the actor is ignoring" do
+        expect(subject.actor_ignoring?(target_user2.id)).to eq(true)
+        expect(subject.actor_ignoring?(target_user4.id)).to eq(false)
+      end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.actor_ignoring?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
+    end
+
+    describe "#actor_muting?" do
+      it "returns true for user ids that the actor is muting" do
+        expect(subject.actor_muting?(target_user1.id)).to eq(true)
+        expect(subject.actor_muting?(target_user2.id)).to eq(false)
+      end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.actor_muting?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
+    end
+
+    describe "#actor_disallowing_pms?" do
+      it "returns true for user ids that the actor is not explicitly allowing PMs from" do
+        acting_user.user_option.update!(enable_allowed_pm_users: true)
+        expect(subject.actor_disallowing_pms?(target_user3.id)).to eq(true)
+        expect(subject.actor_disallowing_pms?(target_user1.id)).to eq(false)
+      end
+
+      it "raises a NotFound error if the user_id passed in is not part of the target users" do
+        expect { subject.actor_disallowing_pms?(other_user.id) }.to raise_error(Discourse::NotFound)
+      end
+    end
+
+    describe "#actor_disallowing_all_pms?" do
+      it "returns true if the acting user has disabled private messages altogether" do
+        expect(subject.actor_disallowing_all_pms?).to eq(false)
+        acting_user.user_option.update!(allow_private_messages: false)
+        expect(subject.actor_disallowing_all_pms?).to eq(true)
+      end
     end
   end
 end