Lean into namespaced policies per controller

Demodulize model policy class names as string and rebuild the classes by following the guidance in Pundit's README: https://github.com/varvet/pundit?tab=readme-ov-file#policy-namespacing.

We have service namespaces and then the "Support" namespace within each service. To avoid duplicate service namespaces - as by default `authorize [:claims, :support, Claims::Claim]` would attempt to lookup a `Claims::Support::Claims::ClaimPolicy`. As both the model and controller both have the service namespaces, we need to de-dup them.

By demodulizing policy class names from models, The `Claims::Claim` model will return a `ClaimPolicy`. This policy does not exist by itself, but we override each of Pundit's controller methods by injecting namespaces.

app/controllers/application_controller.rb

@@ -9,7 +9,7 @@ class ApplicationController < ActionController::Base
 
   default_form_builder(GOVUKDesignSystemFormBuilder::FormBuilder)
 
-  helper_method :current_user, :support_controller?
+  helper_method :current_user, :support_controller?, :policy_scope
 
   rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized
 
@@ -74,4 +74,8 @@ def user_not_authorized
       redirect_back(fallback_location: root_path)
     end
   end
+
+  def unwrap_pundit_scope(scope)
+    scope.is_a?(Array) ? scope : [scope]
+  end
 end

app/controllers/claims/application_controller.rb

@@ -1,8 +1,20 @@
 class Claims::ApplicationController < ApplicationController
   after_action :verify_authorized
 
+  def authorize(record, query = nil, policy_class: nil)
+    super([:claims, *unwrap_pundit_scope(record)], query, policy_class:)
+  end
+
+  def policy(record)
+    super([:claims, *unwrap_pundit_scope(record)])
+  end
+
   private
 
+  def pundit_policy_scope(scope)
+    super([:claims, *unwrap_pundit_scope(scope)])
+  end
+
   def has_school_accepted_grant_conditions?
     return true if current_user.support_user?
 

app/controllers/claims/schools/claims/mentor_trainings_controller.rb

@@ -1,5 +1,6 @@
 class Claims::Schools::Claims::MentorTrainingsController < Claims::ApplicationController
   include Claims::BelongsToSchool
+
   before_action :authorize_claim
   helper_method :mentor_training_form
 

app/controllers/claims/support/application_controller.rb

@@ -1,8 +1,20 @@
 class Claims::Support::ApplicationController < Claims::ApplicationController
   before_action :authorize_user!
 
+  def authorize(record, query = nil, policy_class: nil)
+    super([:support, *unwrap_pundit_scope(record)], query, policy_class:)
+  end
+
+  def policy(record)
+    super([:support, *unwrap_pundit_scope(record)])
+  end
+
   private
 
+  def pundit_policy_scope(scope)
+    super([:support, *unwrap_pundit_scope(scope)])
+  end
+
   def authorize_user!
     return if current_user.support_user?
 

app/controllers/claims/support/schools/add_school_controller.rb

@@ -27,7 +27,7 @@ def set_wizard
   end
 
   def authorize_school
-    authorize Claims::School, policy_class: Claims::SchoolPolicy
+    authorize Claims::School
   end
 
   def step_path(step)

app/controllers/claims/support/schools/claims/mentor_trainings_controller.rb

@@ -1,5 +1,6 @@
-class Claims::Support::Schools::Claims::MentorTrainingsController < Claims::ApplicationController
+class Claims::Support::Schools::Claims::MentorTrainingsController < Claims::Support::ApplicationController
   include Claims::BelongsToSchool
+
   before_action :authorize_claim
   helper_method :mentor_training_form
 

app/controllers/claims/support/schools/claims/mentors_controller.rb

@@ -1,20 +1,24 @@
-class Claims::Support::Schools::Claims::MentorsController < Claims::ApplicationController
+class Claims::Support::Schools::Claims::MentorsController < Claims::Support::ApplicationController
   include Claims::BelongsToSchool
-  before_action :authorize_claim
 
+  before_action :authorize_claim
   helper_method :claim_mentors_form
 
   def new; end
 
   def create
     if claim_mentors_form.save
-      redirect_to(
-        edit_claims_support_school_claim_mentor_training_path(
-          @school,
-          claim,
-          claim.mentor_trainings.without_hours.first,
-        ),
-      )
+      if claim.mentor_trainings.without_hours.any?
+        redirect_to(
+          edit_claims_support_school_claim_mentor_training_path(
+            @school,
+            claim,
+            claim.mentor_trainings.without_hours.first,
+          ),
+        )
+      else
+        redirect_to check_claims_support_school_claim_path(@school, claim)
+      end
     else
       render :new
     end

app/controllers/claims/support/schools_controller.rb

@@ -42,6 +42,6 @@ def set_school
   end
 
   def authorize_school
-    authorize @school || Claims::School, policy_class: Claims::SchoolPolicy
+    authorize @school || Claims::School
   end
 end

app/controllers/placements/application_controller.rb

@@ -2,8 +2,20 @@ class Placements::ApplicationController < ApplicationController
   after_action :verify_policy_scoped, if: ->(c) { c.action_name == "index" }
   before_action :authorize_support_user!
 
+  def authorize(record, query = nil, policy_class: nil)
+    super([:placements, *unwrap_pundit_scope(record)], query, policy_class:)
+  end
+
+  def policy(record)
+    super([:placements, *unwrap_pundit_scope(record)])
+  end
+
   private
 
+  def pundit_policy_scope(scope)
+    super([:placements, *unwrap_pundit_scope(scope)])
+  end
+
   def set_school
     @school = policy_scope(Placements::School).find(params.require(:school_id))
   end

app/controllers/placements/organisations/users_controller.rb

@@ -33,6 +33,6 @@ def set_user_membership
   end
 
   def authorize_user
-    authorize [:placements, @user_membership]
+    authorize @user_membership
   end
 end

app/controllers/placements/providers/partner_schools/placements_controller.rb

@@ -20,6 +20,6 @@ def set_partner_school
   end
 
   def placements
-    policy_scope(@partner_school.placements, policy_scope_class: Placements::Provider::PlacementPolicy::Scope)
+    policy_scope([:provider, @partner_school.placements])
   end
 end

app/controllers/placements/providers/partner_schools_controller.rb

@@ -25,10 +25,7 @@ def set_decorated_partner_organisation
   end
 
   def partner_schools
-    policy_scope(
-      @provider.partner_schools,
-      policy_scope_class: Placements::Partnership::SchoolPolicy::Scope,
-    )
+    policy_scope([:partnership, @provider.partner_schools])
   end
 
   def set_partnership

app/controllers/placements/providers/placements_controller.rb

@@ -25,10 +25,7 @@ def show
   private
 
   def placements
-    policy_scope(
-      Placement.all,
-      policy_scope_class: Placements::Provider::PlacementPolicy::Scope,
-    )
+    policy_scope([:provider, Placement.all])
   end
 
   def schools_scope

app/controllers/placements/schools/partner_providers_controller.rb

@@ -25,10 +25,7 @@ def set_decorated_partner_organisation
   end
 
   def partner_providers
-    policy_scope(
-      @school.partner_providers,
-      policy_scope_class: Placements::Partnership::ProviderPolicy::Scope,
-    )
+    policy_scope([:partnership, @school.partner_providers])
   end
 
   def set_partnership

app/models/application_record.rb

@@ -1,6 +1,8 @@
 class ApplicationRecord < ActiveRecord::Base
   primary_abstract_class
 
+  include DemodulizesPolicyClass
+
   def valid_attributes?(*attributes)
     attributes.each do |attribute|
       self.class.validators_on(attribute).each do |validator|

app/models/concerns/demodulizes_policy_class.rb

@@ -0,0 +1,9 @@
+module DemodulizesPolicyClass
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def policy_class
+      "#{name.demodulize}Policy"
+    end
+  end
+end

app/policies/claims/claim_policy.rb

@@ -16,34 +16,25 @@ def create_revision?
   end
 
   def submit?
-    current_claim_window? && !user.support_user? && !record.submitted?
+    current_claim_window? && !record.submitted?
   end
 
   def rejected?
-    submit? || draft?
+    submit?
   end
 
   def destroy?
     record.draft?
   end
 
   def confirmation?
-    !user.support_user? && record.submitted?
-  end
-
-  # TODO: Remove record.draft? and not create drafts for existing drafts
-  def draft?
-    current_claim_window? && user.support_user? && (record.internal_draft? || record.draft?)
+    record.submitted?
   end
 
   def check?
     record.draft? || record.internal_draft?
   end
 
-  def download_csv?
-    user.support_user?
-  end
-
   private
 
   def current_claim_window?

app/policies/grant_conditions/claims/school_policy.rb → app/policies/claims/grant_conditions/school_policy.rb

@@ -1,4 +1,4 @@
-class GrantConditions::Claims::SchoolPolicy < Claims::ApplicationPolicy
+class Claims::GrantConditions::SchoolPolicy < Claims::ApplicationPolicy
   def show?
     record.users.include?(user)
   end

app/policies/claims/school_policy.rb

@@ -1,31 +1,7 @@
 class Claims::SchoolPolicy < Claims::ApplicationPolicy
-  def update?
-    true
-  end
-
-  def school_options?
-    true
-  end
-
-  def check_school_option?
-    true
-  end
-
-  def remove_grant_conditions_acceptance_check?
-    user != record && user.support_user?
-  end
-
-  def remove_grant_conditions_acceptance?
-    remove_grant_conditions_acceptance_check?
-  end
-
   class Scope < ApplicationPolicy::Scope
     def resolve
-      if user.support_user?
-        scope.all
-      else
-        scope.joins(:users).where(users: { id: user })
-      end
+      scope.joins(:users).where(users: { id: user })
     end
   end
 end

app/policies/claims/support/claim_policy.rb

@@ -0,0 +1,52 @@
+class Claims::Support::ClaimPolicy < Claims::ApplicationPolicy
+  def create?
+    current_claim_window?
+  end
+
+  def edit?
+    claim_claim_window_current? && !record.submitted?
+  end
+
+  def update?
+    edit?
+  end
+
+  def create_revision?
+    edit?
+  end
+
+  def submit?
+    current_claim_window? && !user.support_user? && !record.submitted?
+  end
+
+  def rejected?
+    submit? || draft?
+  end
+
+  def destroy?
+    record.draft?
+  end
+
+  # TODO: Remove record.draft? and not create drafts for existing drafts
+  def draft?
+    current_claim_window? && user.support_user? && (record.internal_draft? || record.draft?)
+  end
+
+  def check?
+    record.draft? || record.internal_draft?
+  end
+
+  def download_csv?
+    user.support_user?
+  end
+
+  private
+
+  def current_claim_window?
+    Claims::ClaimWindow.current.present?
+  end
+
+  def claim_claim_window_current?
+    record.claim_window.current?
+  end
+end

app/policies/claims/claim_window_policy.rb → app/policies/claims/support/claim_window_policy.rb

@@ -1,4 +1,4 @@
-class Claims::ClaimWindowPolicy < Claims::ApplicationPolicy
+class Claims::Support::ClaimWindowPolicy < Claims::ApplicationPolicy
   def read?
     user.support_user?
   end

app/policies/claims/support/mentor_membership_policy.rb

@@ -0,0 +1,13 @@
+class Claims::Support::MentorMembershipPolicy < Claims::ApplicationPolicy
+  def update?
+    true
+  end
+
+  def destroy?
+    !Claims::Claim.active.joins(:mentor_trainings).where(school_id: record.school_id, mentor_trainings: { mentor_id: record.mentor_id }).exists?
+  end
+
+  def remove?
+    true
+  end
+end

app/policies/claims/support/mentor_policy.rb

@@ -0,0 +1,9 @@
+class Claims::Support::MentorPolicy < Claims::ApplicationPolicy
+  def update?
+    true
+  end
+
+  def destroy?
+    true
+  end
+end

app/policies/claims/support/school_policy.rb

@@ -0,0 +1,19 @@
+class Claims::Support::SchoolPolicy < Claims::ApplicationPolicy
+  def update?
+    true
+  end
+
+  def remove_grant_conditions_acceptance_check?
+    user != record && user.support_user?
+  end
+
+  def remove_grant_conditions_acceptance?
+    remove_grant_conditions_acceptance_check?
+  end
+
+  class Scope < ApplicationPolicy::Scope
+    def resolve
+      scope.all
+    end
+  end
+end