Consolidate organisation options methods

These are essentially doing the same thing, so there's no need to have
multiple methods for them

I've deleted a test from a controller spec that duplicates a helper test
added here

I had considered removing the `Organisation` policy scope. The scope
gives GOV.UK admins access to all organisations and everyone else access
to none, but is only used in places that are accessible only to GOV.UK
admins (also based on the organisation policy), so it felt like a bit of
a double-authorisation situation. However, George and I felt a bit
uneasy about the potential for the `options_for_organisation_select`
method being used elsewhere in future, and the authorisation therefore
being skipped. As a result, we've retained the double authorisation, and
I've included a scope-based context in the helper tests

Another thing I spotted is that the `User` model has a
`manageable_organisations` method, in turn relying on e.g.
`Roles::Admin.manageable_organisations_for` or
`Roles::SuperOrganisationAdmin.manageable_organisations_for`. This is
used for filtering users by organisation in the users page/table, rather
than populating a select field when creating/editing users, but the name
makes the remits a little hard to differentiate without close inspection

app/helpers/organisation_helper.rb

@@ -0,0 +1,9 @@
+module OrganisationHelper
+  def options_for_organisation_select(selected_id: nil)
+    [{ text: Organisation::NONE, value: nil }] + policy_scope(Organisation).not_closed.map do |organisation|
+      { text: organisation.name_with_abbreviation, value: organisation.id }.tap do |option|
+        option[:selected] = true if option[:value] == selected_id
+      end
+    end
+  end
+end

app/helpers/users_helper.rb

@@ -70,14 +70,6 @@ def options_for_role_select(selected: nil)
     end
   end
 
-  def options_for_organisation_select(selected: nil)
-    [{ text: "None", value: nil }] + policy_scope(Organisation).not_closed.map do |organisation|
-      { text: organisation.name_with_abbreviation, value: organisation.id }.tap do |option|
-        option[:selected] = true if option[:value] == selected
-      end
-    end
-  end
-
   def options_for_permission_option_select(application:, user: nil)
     application.sorted_supported_permissions_grantable_from_ui.map do |permission|
       {

app/views/account/organisations/edit.html.erb

@@ -25,7 +25,7 @@
             id: "user_organisation_id",
             name: "user[organisation_id]",
             label: "Organisation",
-            options: options_for_your_organisation_select(current_user)
+            options: options_for_organisation_select(selected_id: current_user.organisation_id)
           } %>
 
           <div class="govuk-button-group">

app/views/batch_invitations/new.html.erb

@@ -71,7 +71,7 @@
           id: "batch_invitation_organisation_id",
           name: "batch_invitation[organisation_id]",
           label: "Organisation",
-          options: policy_scope(Organisation).not_closed.map { |organisation| { text: organisation.name_with_abbreviation, value: organisation.id } }
+          options: options_for_organisation_select
         } %>
 
         <div class="govuk-button-group">

app/views/devise/invitations/new.html.erb

@@ -54,7 +54,7 @@
           id: "user_organisation_id",
           name: "user[organisation_id]",
           label: "Organisation",
-          options: options_for_organisation_select(selected: f.object.organisation_id)
+          options: options_for_organisation_select(selected_id: f.object.organisation_id)
         } %>
 
         <div class="govuk-button-group">

app/views/users/organisations/edit.html.erb

@@ -43,7 +43,7 @@
           id: "user_organisation_id",
           name: "user[organisation_id]",
           label: "Organisation",
-          options: options_for_organisation_select(selected: @user.organisation_id),
+          options: options_for_organisation_select(selected_id: @user.organisation_id),
           error_message: @user.errors[:organisation_id].any? ? @user.errors.full_messages_for(:organisation_id).to_sentence : nil
         } %>
 

test/controllers/batch_invitations_controller_test.rb

@@ -25,14 +25,6 @@ def users_csv(filename = "users.csv")
 
       assert_select "#batch_invitation_organisation_id option", organisation.name
     end
-
-    should "exclude closed organisations from select" do
-      create(:organisation)
-      create(:organisation, closed: true)
-      get :new
-
-      assert_select "#batch_invitation_organisation_id option", 1
-    end
   end
 
   context "POST create" do

test/helpers/organisation_helper_test.rb

@@ -0,0 +1,54 @@
+require "test_helper"
+
+class OrganisationHelperTest < ActionView::TestCase
+  setup do
+    @gds = create(:organisation, name: "Government Digital Service", abbreviation: "GDS")
+    @gas = create(:organisation, name: "Government Analogue Service", abbreviation: "GAS")
+    @unabbreviated = create(:organisation, name: "unAbbreviaTed")
+
+    stubs(:policy_scope).with(Organisation).returns(Organisation.all)
+  end
+
+  should "return a select option for each organisation sorted alphabetically, preceded by a 'None' option" do
+    expected = [
+      { text: "None", value: nil },
+      { text: "Government Analogue Service - GAS", value: @gas.id },
+      { text: "Government Digital Service - GDS", value: @gds.id },
+      { text: "unAbbreviaTed", value: @unabbreviated.id },
+    ]
+
+    assert_equal expected, options_for_organisation_select
+  end
+
+  context "when a closed organisation exists" do
+    setup do
+      @closed = create(:organisation, name: "Prehistoric Government Service", closed: true)
+    end
+
+    should "exclude the closed organisation" do
+      unexpected_closed_option = { text: "Prehistoric Government Service", value: @closed.id }
+
+      assert_not_includes options_for_organisation_select, unexpected_closed_option
+    end
+  end
+
+  context "when a selection is provided" do
+    should "mark the selection as selected" do
+      selected_option = { text: "Government Analogue Service - GAS", value: @gas.id, selected: true }
+
+      assert_includes options_for_organisation_select(selected_id: @gas.id), selected_option
+    end
+  end
+
+  context "when there are no organisations the current user can select" do
+    setup do
+      stubs(:policy_scope).with(Organisation).returns(Organisation.where("false"))
+    end
+
+    should "return only a 'None' option" do
+      expected = [{ text: "None", value: nil }]
+
+      assert_equal expected, options_for_organisation_select
+    end
+  end
+end

test/helpers/users_helper_test.rb

@@ -78,26 +78,6 @@ class UsersHelperTest < ActionView::TestCase
     end
   end
 
-  context "#options_for_organisation_select" do
-    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: "A Organisation", value: organisation2.id, selected: true },
-        { text: "B Organisation", value: organisation1.id },
-      ]
-      assert_equal expected_options, options
-    end
-  end
-
   context "#options_for_permission_option_select" do
     should "return permission options suitable for option-select component" do
       application = create(:application)