From eff161ba58b290eec730aecf06680854c6e8918f Mon Sep 17 00:00:00 2001
From: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Date: Wed, 2 Mar 2022 11:02:04 -0500
Subject: [PATCH] Adding ArticlePolicy.scope_users_authorized_to_action
 (#16732)

As I'm looking at limiting feed fetching only for users who are able to
create articles, I needed a means of querying "Who are the users who can
create articles?"  This bit of work was the smallest chunk that I could
think of.

These kinds of questsions are going to propogate (e.g. give me a list of
all the people who can comment? who can create articles in this space?)
For now, this pattern should suffice.

Yes, there's duplication of knowledge between the policy's instance
method and class method.  And there's a bit of knowledge bleed between a
user's method (e.g. `user.any_admin?` and
`*Authorizer::RoleBasedQueries::ANY_ADMIN`) but that's a small "evil".

Related to #16486
---
 app/policies/article_policy.rb       | 38 ++++++++++++++++++++++++++++
 spec/policies/article_policy_spec.rb | 25 ++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb
index f8f0a98d4..8a8ca7a4d 100644
--- a/app/policies/article_policy.rb
+++ b/app/policies/article_policy.rb
@@ -12,6 +12,43 @@ class ArticlePolicy < ApplicationPolicy
     FeatureFlag.enabled?(:limit_post_creation_to_admins)
   end
 
+  # Helps filter a `:user_scope` to those authorized to the `:action`.  I want a list of all users
+  # who can create an Article.  This policy method can help with that.
+  #
+  # @param user_scope [ActiveRecord::Relation] a scope for querying user objects
+  # @param action [Symbol] the name of one of the ArticlePolicy action predicates (e.g. :create?,
+  #        :new?) though as a convenience, we will also accept :new, and :create.
+  #
+  # @return [ActiveRecord::Relation]
+  #
+  # @see https://api.rubyonrails.org/classes/ActiveRecord/Scoping/Named/ClassMethods.html#method-i-scope
+  #
+  # @note With this duplication it would be feasible to alter the instance method logics to use the
+  #       class method (e.g. `ArticlePolicy.scope_authorized(user_scope: User, action:
+  #       :create?).find_by(user.id)`) but that's a future consideration.
+  #
+  # @note This is not a Pundit scope (see https://github.com/varvet/pundit#scopes), as those methods
+  #       are for answering "What articles can I see?"  This method is for answering "Who all can
+  #       <action> on Articles?"
+  #
+  # @note Why isn't this a User.scope method?  Because the logic of who can take an action on the
+  #       resource is the problem domain of the policy.
+  def self.scope_users_authorized_to_action(user_scope:, action:)
+    case action
+    when :create?, :new?, :create, :new
+      # Note the delicate dance to duplicate logic in a general sense.  [I hope that] this is a
+      # stop-gap solution.
+      user_scope = user_scope.without_role(:suspended)
+      return user_scope unless limit_post_creation_to_admins?
+
+      # NOTE: Not a fan of reaching over to the constant of another class, but I digress.
+      user_scope.with_any_role(*Authorizer::RoleBasedQueries::ANY_ADMIN_ROLES)
+    else
+      # Not going to implement all of the use cases.
+      raise "Unhandled predicate: #{action} for #{self}.#{__method__}"
+    end
+  end
+
   # @note [@jeremyf] I am re-implemnenting the initialize method, but removing the Pundit
   #       authorization.  There's an assumption that all policy questions will require a user,
   #       unless you know specifically that they don't.
@@ -40,6 +77,7 @@ class ArticlePolicy < ApplicationPolicy
     true
   end
 
+  # @see {ArticlePolicy.scope_users_authorized_to_action} for "mirrored" details.
   def create?
     require_user_in_good_standing!
     return true unless self.class.limit_post_creation_to_admins?
diff --git a/spec/policies/article_policy_spec.rb b/spec/policies/article_policy_spec.rb
index 3d2e8536e..3df1ecb7b 100644
--- a/spec/policies/article_policy_spec.rb
+++ b/spec/policies/article_policy_spec.rb
@@ -84,6 +84,31 @@ RSpec.describe ArticlePolicy do
   let(:resource) { build(:article, user: author, organization: organization) }
   let(:policy) { described_class.new(user, resource) }
 
+  describe ".scope_users_authorized_to_action" do
+    let!(:regular_user) { create(:user) }
+    let!(:super_admin_user) { create(:user, :super_admin) }
+
+    before { create(:user, :suspended) }
+
+    context "when limit_post_creation_to_admins is true" do
+      before { allow(described_class).to receive(:limit_post_creation_to_admins?).and_return(true) }
+
+      it "omits suspended and regular users" do
+        results = described_class.scope_users_authorized_to_action(user_scope: User, action: :create?).to_a
+        expect(results).to match_array([super_admin_user])
+      end
+    end
+
+    context "when limit_post_creation_to_admins is false" do
+      before { allow(described_class).to receive(:limit_post_creation_to_admins?).and_return(false) }
+
+      it "omits only suspended users" do
+        results = described_class.scope_users_authorized_to_action(user_scope: User, action: :create?).to_a
+        expect(results).to match_array([regular_user, super_admin_user])
+      end
+    end
+  end
+
   describe "#feed?" do
     let(:policy_method) { :feed? }