Merge pull request #2884 from DFE-Digital/AQTS-1131-reorder-list

Reorder staff list

Author: syed87

app/models/staff.rb

@@ -74,7 +74,7 @@ class Staff < ApplicationRecord
 
   validates :name, presence: true
 
-  scope :assessors, -> { where(assess_permission: true) }
+  scope :assessors_by_name, -> { where(assess_permission: true).order(:name) }
   scope :not_archived, -> { where(archived: false) }
   scope :archived, -> { where(archived: true) }
 

app/views/assessor_interface/assessor_assignments/new.html.erb

@@ -7,7 +7,7 @@
   <%= f.govuk_error_summary %>
 
   <%= f.govuk_collection_radio_buttons :assessor_id,
-                                       [OpenStruct.new(id: "", name: t("application_form.summary.unassigned"))] + Staff.assessors.not_archived,
+                                       [OpenStruct.new(id: "", name: t("application_form.summary.unassigned"))] + Staff.assessors_by_name.not_archived,
                                        :id,
                                        :name,
                                        legend: nil %>

app/views/assessor_interface/reviewer_assignments/new.html.erb

@@ -7,7 +7,7 @@
   <%= f.govuk_error_summary %>
 
   <%= f.govuk_collection_radio_buttons :reviewer_id,
-                                       [OpenStruct.new(id: "", name: t("application_form.summary.unassigned"))] + Staff.assessors.not_archived,
+                                       [OpenStruct.new(id: "", name: t("application_form.summary.unassigned"))] + Staff.assessors_by_name.not_archived,
                                        :id,
                                        :name,
                                        legend: nil %>

spec/models/staff_spec.rb

@@ -79,8 +79,8 @@
   end
 
   describe "scopes" do
-    describe ".assessors" do
-      subject { described_class.assessors }
+    describe ".assessors_by_name" do
+      subject { described_class.assessors_by_name }
 
       context "when assess_permission == true" do
         let(:with_assess_permission) { create(:staff, :with_assess_permission) }

spec/requests/assessor/applications/assign-assessor/new_spec.rb

@@ -9,28 +9,54 @@
   end
 
   let(:application_form) { create :application_form, :submitted }
+  let(:signed_in_staff) do
+    create(:staff, :with_assess_permission, name: "Charlie Assessor")
+  end
 
   before do
-    create(:staff, :with_assess_permission, name: "Active Assessor")
+    create(:staff, :with_assess_permission, name: "Zachary Assessor")
+    create(:staff, :with_assess_permission, name: "Aaron Assessor")
+    create(:staff, :with_assess_permission, name: "Bernard Assessor")
     create(
       :staff,
       :with_assess_permission,
       :archived,
       name: "Archived Assessor",
     )
-    sign_in(create(:staff, :with_assess_permission))
+    sign_in(signed_in_staff)
     assign_assessor
   end
 
   it "displays the page with the 'Select an assessor' title" do
     expect(response.body).to include("Select an assessor")
   end
 
-  it "displays the active assessor as an option" do
-    expect(response.body).to include("Active Assessor")
+  it "displays the active assessors as options" do
+    expect(response.body).to include("Aaron Assessor")
+    expect(response.body).to include("Bernard Assessor")
+    expect(response.body).to include("Zachary Assessor")
   end
 
   it "does not display the archived assessor as an option" do
     expect(response.body).not_to include("Archived Assessor")
   end
+
+  it "displays the assessors in alphabetical order" do
+    assessor_names =
+      Nokogiri
+        .HTML(response.body)
+        .css(".govuk-radios__label")
+        .map(&:text)
+        .map(&:strip)
+        .reject { |name| name == "Not assigned" }
+
+    expected_names = [
+      "Aaron Assessor",
+      "Bernard Assessor",
+      "Charlie Assessor",
+      "Zachary Assessor",
+    ]
+
+    expect(assessor_names).to eq(expected_names)
+  end
 end

spec/requests/assessor/applications/assign-reviewer/new_spec.rb

@@ -4,33 +4,59 @@
 
 RSpec.describe "GET /assessor/applications/:reference/assign-reviewer",
                type: :request do
-  subject(:assign_assessor) do
+  subject(:assign_reviewer) do
     get "/assessor/applications/#{application_form.reference}/assign-reviewer"
   end
 
   let(:application_form) { create :application_form, :submitted }
+  let(:signed_in_staff) do
+    create(:staff, :with_assess_permission, name: "Charlie Reviewer")
+  end
 
   before do
-    create(:staff, :with_assess_permission, name: "Active Assessor")
+    create(:staff, :with_assess_permission, name: "Zachary Reviewer")
+    create(:staff, :with_assess_permission, name: "Aaron Reviewer")
+    create(:staff, :with_assess_permission, name: "Bernard Reviewer")
     create(
       :staff,
       :with_assess_permission,
       :archived,
-      name: "Archived Assessor",
+      name: "Archived Reviewer",
     )
-    sign_in(create(:staff, :with_assess_permission))
-    assign_assessor
+    sign_in(signed_in_staff)
+    assign_reviewer
   end
 
-  it "displays the page with the 'Select an reviewer' title" do
+  it "displays the page with the 'Select a reviewer' title" do
     expect(response.body).to include("Select a reviewer")
   end
 
-  it "displays the active assessor as an option" do
-    expect(response.body).to include("Active Assessor")
+  it "displays the active reviewers as options" do
+    expect(response.body).to include("Aaron Reviewer")
+    expect(response.body).to include("Bernard Reviewer")
+    expect(response.body).to include("Zachary Reviewer")
+  end
+
+  it "does not display the archived reviewer as an option" do
+    expect(response.body).not_to include("Archived Reviewer")
   end
 
-  it "does not display the archived assessor as an option" do
-    expect(response.body).not_to include("Archived Assessor")
+  it "displays the reviewers in alphabetical order" do
+    reviewer_names =
+      Nokogiri
+        .HTML(response.body)
+        .css(".govuk-radios__label")
+        .map(&:text)
+        .map(&:strip)
+        .reject { |name| name == "Not assigned" }
+
+    expected_names = [
+      "Aaron Reviewer",
+      "Bernard Reviewer",
+      "Charlie Reviewer",
+      "Zachary Reviewer",
+    ]
+
+    expect(reviewer_names).to eq(expected_names)
   end
 end