894: Complete ECT admin search page and set it as root (#927)

Extend search ECT admin page and implement search with filters for
future usage.

<img width="816" alt="Screenshot 2024-12-13 at 12 27 56"
src="https://github.com/user-attachments/assets/5680e30d-5fd4-4cb8-8acd-db7d4d959a91"
/>

app/controllers/admin/teachers_controller.rb

@@ -3,8 +3,12 @@ class TeachersController < AdminController
     layout "full"
 
     def index
-      @pagy, teachers = pagy(Teachers::Search.new(params[:q]).search.order(:last_name, :first_name, :id))
-      @teachers = TeacherPresenter.wrap(teachers)
+      @appropriate_bodies = AppropriateBody.order(:name)
+      @pagy, @teachers = pagy(
+        Teachers::Search.new(
+          query_string: params[:q]
+        ).search
+      )
     end
 
     def show

app/controllers/appropriate_bodies/teachers_controller.rb

@@ -3,7 +3,10 @@ class TeachersController < AppropriateBodiesController
     layout "full", only: :index
 
     def index
-      @teachers = ::Teachers::Search.new(params[:q], appropriate_body: @appropriate_body).search
+      @teachers = ::Teachers::Search.new(
+        query_string: params[:q],
+        appropriate_body: @appropriate_body
+      ).search
     end
 
     def show

app/services/admin/current_teachers.rb

@@ -0,0 +1,19 @@
+module Admin
+  class CurrentTeachers
+    attr_reader :appropriate_body_ids
+
+    def initialize(appropriate_body_ids)
+      @appropriate_body_ids = Array(appropriate_body_ids)
+    end
+
+    def current
+      scope = Teacher.joins(:induction_periods).merge(InductionPeriod.ongoing)
+
+      if appropriate_body_ids.any?
+        scope = scope.merge(InductionPeriod.for_appropriate_body(appropriate_body_ids))
+      end
+
+      scope.distinct
+    end
+  end
+end

app/services/teachers/search.rb

@@ -1,31 +1,59 @@
 module Teachers
   class Search
-    def initialize(query_string, appropriate_body: nil, filters: {})
-      @scope = if appropriate_body.present?
-                 AppropriateBodies::CurrentTeachers.new(appropriate_body).current
-               else
-                 Teacher
-               end
+    attr_reader :scope
 
+    def initialize(query_string: :ignore, appropriate_body: :ignore, appropriate_body_ids: :ignore)
+      @scope = Teacher.distinct
       @query_string = query_string
 
-      # TODO: no filters yet
-      @_filters = filters
+      where_appropriate_body_is(appropriate_body)
+      where_appropriate_body_ids_in(appropriate_body_ids)
+      where_query_matches(query_string)
     end
 
     def search
-      case
-      when @query_string.blank?
-        @scope.all
-      when trns.any?
-        @scope.where(trn: trns)
-      else
-        @scope.search(@query_string)
-      end
+      scope.order(:last_name, :first_name, :id)
+    end
+
+  private
+
+    def ignore?(filter:)
+      filter == :ignore
+    end
+
+    def where_appropriate_body_is(appropriate_body)
+      return if ignore?(filter: appropriate_body)
+
+      # If no appropriate body provided in regular context, return nothing
+      @scope = if appropriate_body.nil?
+                 Teacher.none
+               else
+                 AppropriateBodies::CurrentTeachers.new(appropriate_body).current
+               end
+    end
+
+    def where_appropriate_body_ids_in(appropriate_body_ids)
+      return if ignore?(filter: appropriate_body_ids)
+
+      # In admin context, if no IDs provided, show all current teachers
+      @scope = if @scope == Teacher.none
+                 Teacher.none
+               else
+                 Admin::CurrentTeachers.new(appropriate_body_ids).current
+               end
     end
 
-    def trns
-      @trns ||= @query_string.scan(%r(\d{7}))
+    def where_query_matches(query_string)
+      return if ignore?(filter: query_string)
+      return if query_string.blank?
+      return if @scope == Teacher.none
+
+      trns = query_string.scan(%r(\d{7}))
+      @scope = if trns.any?
+                 @scope.where(trn: trns)
+               else
+                 @scope.search(query_string)
+               end
     end
   end
 end

app/views/admin/teachers/index.html.erb

@@ -1,48 +1,51 @@
-<% page_data(title: "Teachers") %>
+<% page_data(title: "Early career teachers") %>
 
-<%= form_with method: :get do |f| %>
-  <%=
-    f.govuk_text_field(
-      "q",
-      value: params[:q],
-      label: { text: "Search for teacher", size: "s" },
-      hint: { text: "Name or TRN" },
-    )
-  %>
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-one-third">
+    <%= form_with method: :get, data: { turbo: false } do |f| %>
 
-  <%= f.govuk_submit "Search" %>
-<% end %>
+      <%= f.govuk_text_field(
+            "q",
+            value: params[:q],
+            label: { text: "Search by name or TRN", size: "s" },
+          )
+      %>
 
-<p class="govuk-body"><strong><%= @pagy.count %></strong> teachers found</p>
+      <div class="govuk-button-group">
+        <%= f.govuk_submit "Search" %>
+        <button type="reset" class="govuk-button govuk-button--secondary">Reset</button>
+      </div>
+    <% end %>
+  </div>
 
-<%=
-  govuk_table do |table|
-    table.with_head do |head|
-      head.with_row do |row|
-        row.with_cell(text: "Name")
-        row.with_cell(text: "Role")
-        row.with_cell(text: "TRN")
-        row.with_cell { govuk_visually_hidden("Has migration errors") }
-      end
-    end
+  <div class="govuk-grid-column-two-thirds">
+    <ul class="govuk-list">
+      <% @teachers.each do |teacher| %>
+        <%= govuk_summary_card(title: Teachers::Name.new(teacher).full_name) do |card|
+          card.with_action { govuk_link_to("Show", admin_teacher_path(teacher)) }
+          card.with_summary_list(
+            actions: false,
+            rows: [
+              { key: { text: "TRN" }, value: { text: teacher.trn } },
+              {
+                key: { text: "Appropriate body" },
+                value: { text: Teachers::InductionPeriod.new(teacher).active_induction_period&.appropriate_body&.name },
+              },
+              {
+                key: { text: "Induction periods recorded" },
+                value: { text: teacher.induction_periods.count },
+              },
+              {
+                key: { text: "Status" },
+                # FIXME: this is a placeholder as we cannot display a real status yet
+                value: { text: govuk_tag(text: "placeholder", colour: %w[grey green red purple orange yellow].sample) },
+              },
+            ]
+          )
+        end %>
+      <% end %>
+    </ul>
 
-    table.with_body do |body|
-      @teachers.each_with_index do |teacher, index|
-        body.with_row do |row|
-          row.with_cell do
-            govuk_link_to "#{teacher.first_name} #{teacher.last_name}", admin_teacher_path(teacher, page: @pagy.page)
-          end
-          row.with_cell(text: Teachers::Role.new(teacher:))
-          row.with_cell(text: teacher.trn)
-          if teacher.migration_failures.any?
-            row.with_cell { govuk_tag(text: "Error", colour: "red") }
-          else
-            row.with_cell(text: "")
-          end
-        end
-      end
-    end
-  end
-%>
-
-<%= govuk_pagination pagy: @pagy %>
+    <%= govuk_pagination pagy: @pagy %>
+  </div>
+</div>

config/routes.rb

@@ -30,7 +30,7 @@
 
   post 'auth/:provider/callback', to: 'sessions#create'
 
-  get '/admin', to: 'admin#index'
+  get '/admin', to: 'admin/teachers#index'
 
   namespace :admin do
     constraints -> { Rails.application.config.enable_blazer } do

spec/features/admin_spec.rb

@@ -1,22 +1,20 @@
-RSpec.describe "Admin" do
+RSpec.describe "Admin root" do
   include UserHelper
-  # TODO: This test should be replaced with something meaningful.
-  # Just here to prove Playwright is working.
   scenario "visiting the admin placeholder page" do
     given_i_am_logged_in_as_an_admin
-    when_i_visit_the_admin_placeholder_page
-    then_i_should_see_the_admin_placeholder_page
+    when_i_visit_the_admin_root_page
+    then_i_should_see_the_admin_teacher_search_page
   end
 
   def given_i_am_logged_in_as_an_admin
     sign_in_as_admin
   end
 
-  def when_i_visit_the_admin_placeholder_page
+  def when_i_visit_the_admin_root_page
     page.goto(admin_path)
   end
 
-  def then_i_should_see_the_admin_placeholder_page
-    expect(page.get_by_text("Placeholder page for the admin site")).to be_visible
+  def then_i_should_see_the_admin_teacher_search_page
+    expect(page.get_by_text("Search by name or TRN")).to be_visible
   end
 end

spec/requests/admin/root_spec.rb

@@ -0,0 +1,31 @@
+require "rails_helper"
+
+RSpec.describe "Admin root", type: :request do
+  describe "GET /admin" do
+    it "redirects to sign-in" do
+      get "/admin"
+      expect(response).to redirect_to(sign_in_path)
+    end
+
+    context "with an authenticated non-DfE user" do
+      include_context 'fake session manager for non-DfE user'
+
+      it "requires authorisation" do
+        get "/admin"
+        expect(response.status).to eq(401)
+      end
+    end
+
+    context "with an authenticated DfE user" do
+      include_context 'fake session manager for DfE user'
+
+      it "shows the teachers search page" do
+        get "/admin"
+        expect(response.status).to eq(200)
+
+        expect(response.body).to include("Early career teachers")
+        expect(response.body).to include("Search by name or TRN")
+      end
+    end
+  end
+end

spec/requests/admin/teachers/index_spec.rb

@@ -0,0 +1,77 @@
+require "rails_helper"
+
+RSpec.describe "Admin teachers index", type: :request do
+  describe "GET /admin/teachers" do
+    it "redirects to sign-in" do
+      get "/admin/teachers"
+      expect(response).to redirect_to(sign_in_path)
+    end
+
+    context "with an authenticated non-DfE user" do
+      include_context 'fake session manager for non-DfE user'
+
+      it "requires authorisation" do
+        get "/admin/teachers"
+        expect(response.status).to eq(401)
+      end
+    end
+
+    context "with an authenticated DfE user" do
+      include_context 'fake session manager for DfE user'
+
+      context "with search query" do
+        it "filters teachers by name" do
+          teacher = FactoryBot.create(:teacher, first_name: "John", last_name: "Smith")
+          other_teacher = FactoryBot.create(:teacher, first_name: "Jane", last_name: "Doe")
+
+          FactoryBot.create(
+            :induction_period,
+            teacher: teacher,
+            started_on: 1.month.ago.to_date,
+            finished_on: nil,
+            induction_programme: 'fip'
+          )
+          FactoryBot.create(
+            :induction_period,
+            teacher: other_teacher,
+            started_on: 1.month.ago.to_date,
+            finished_on: nil,
+            induction_programme: 'fip'
+          )
+
+          get "/admin/teachers", params: { q: "John Smith" }
+
+          expect(response.status).to eq(200)
+          expect(response.body).to include("John Smith")
+          expect(response.body).not_to include("Jane Doe")
+        end
+
+        it "filters teachers by TRN" do
+          teacher = FactoryBot.create(:teacher, trn: "1234567")
+          other_teacher = FactoryBot.create(:teacher, trn: "7654321")
+
+          FactoryBot.create(
+            :induction_period,
+            teacher: teacher,
+            started_on: 1.month.ago.to_date,
+            finished_on: nil,
+            induction_programme: 'fip'
+          )
+          FactoryBot.create(
+            :induction_period,
+            teacher: other_teacher,
+            started_on: 1.month.ago.to_date,
+            finished_on: nil,
+            induction_programme: 'fip'
+          )
+
+          get "/admin/teachers", params: { q: "1234567" }
+
+          expect(response.status).to eq(200)
+          expect(response.body).to include("1234567")
+          expect(response.body).not_to include("7654321")
+        end
+      end
+    end
+  end
+end