From 47e466ee53cb4bca46b4393d96acb9f3805bdb76 Mon Sep 17 00:00:00 2001
From: Chris Lowis <chris.lowis@digital.cabinet-office.gov.uk>
Date: Tue, 10 Oct 2023 17:59:32 +0100
Subject: [PATCH] Improve Organisation select on /users/invitation/new

Rosa got in touch to say that this Organisation select box is not very
usable, particularly in integration. The organisations are sorted in a
seemingly random order and they include closed organisations that we
never need to assign to new users.

This commit fixes both of those issues.

The previous tests stubbed `policy_scope` which is a Pundit-provided
convenience for `Pundit.policy_scope` automatically passing in
`current_user` as the first argument. I wanted to actually call the
scope, so I could test that the `not_closed` and `order` scopes I've
added were working correctly. To get `policy_scope` to work in the
isolated tests I've explicitly included `Pundit::Authorization` in
`UsersHelper` and stubbed `current_user` instead.
---
 app/helpers/users_helper.rb       |  4 +++-
 test/helpers/users_helper_test.rb | 16 +++++++++-------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
index 1aa626a0a..f86b8623c 100644
--- a/app/helpers/users_helper.rb
+++ b/app/helpers/users_helper.rb
@@ -1,4 +1,6 @@
 module UsersHelper
+  include Pundit::Authorization
+
   def two_step_status(user)
     user.two_step_status.humanize.capitalize
   end
@@ -66,7 +68,7 @@ def options_for_role_select(selected: nil)
   end
 
   def options_for_organisation_select(selected: nil)
-    [{ text: "None", value: nil }] + policy_scope(Organisation).map do |organisation|
+    [{ text: "None", value: nil }] + policy_scope(Organisation).not_closed.order(:name).map do |organisation|
       { text: organisation.name_with_abbreviation, value: organisation.id }.tap do |option|
         option[:selected] = true if option[:value] == selected
       end
diff --git a/test/helpers/users_helper_test.rb b/test/helpers/users_helper_test.rb
index dca168376..fdaee2d2e 100644
--- a/test/helpers/users_helper_test.rb
+++ b/test/helpers/users_helper_test.rb
@@ -57,18 +57,20 @@ class UsersHelperTest < ActionView::TestCase
   end
 
   context "#options_for_organisation_select" do
-    should "return organisation options suitable for select component" do
-      organisation1 = create(:organisation)
-      organisation2 = create(:organisation)
-      organisations = [organisation1, organisation2]
-      stubs(:policy_scope).with(Organisation).returns(organisations)
+    should "return organisation options suitable for select component, sorted alphabetically and exluding closed organisations" do
+      user = create(:admin_user)
+      stubs(:current_user).returns(user)
+
+      organisation1 = create(:organisation, name: "B Organisation")
+      organisation2 = create(:organisation, name: "A Organisation")
+      create(:organisation, name: "Closed Organisation", closed: true)
 
       options = options_for_organisation_select(selected: organisation2.id)
 
       expected_options = [
         { text: "None", value: nil },
-        { text: organisation1.name, value: organisation1.id },
-        { text: organisation2.name, value: organisation2.id, selected: true },
+        { text: "A Organisation", value: organisation2.id, selected: true },
+        { text: "B Organisation", value: organisation1.id },
       ]
       assert_equal expected_options, options
     end