From 9a00eb55b331a3200241aee9f68f07b48ec885b5 Mon Sep 17 00:00:00 2001
From: "create-issue-branch[bot]"
 <53036503+create-issue-branch[bot]@users.noreply.github.com>
Date: Thu, 12 Sep 2024 15:46:31 +0100
Subject: [PATCH] Update class members endpoint to include teachers (#426)

closes #424

---------

Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com>
Co-authored-by: Dan Halson <dan.halson@raspberrypi.org>
Co-authored-by: Dan Halson <danhalson@users.noreply.github.com>
---
 .rubocop.yml                                  | 12 ++-
 .../api/class_members_controller.rb           |  6 +-
 .../api/school_teachers_controller.rb         |  3 +-
 app/graphql/mutations/create_component.rb     |  4 +-
 app/graphql/mutations/delete_project.rb       |  4 +-
 app/graphql/mutations/remix_project.rb        |  2 -
 app/models/ability.rb                         |  6 --
 app/models/filesystem_project.rb              |  2 +-
 app/services/school_verification_service.rb   |  2 -
 .../api/class_members/index.json.jbuilder     | 26 ++----
 config/routes.rb                              |  2 -
 lib/concepts/class_member/operations/list.rb  | 34 +++++++
 lib/concepts/school_teacher/list.rb           | 11 +--
 lib/concepts/school_teacher/remove.rb         |  1 +
 lib/profile_api_client.rb                     |  6 --
 lib/tasks/classroom_management.rake           |  2 -
 lib/tasks/classroom_management_helper.rb      |  2 -
 spec/concepts/class_member/list_spec.rb       | 82 +++++++++++++++++
 spec/concepts/school_teacher/list_spec.rb     | 59 +++++-------
 .../listing_class_members_spec.rb             | 90 ++++++++++++++-----
 spec/features/lesson/listing_lessons_spec.rb  |  6 --
 spec/features/lesson/showing_a_lesson_spec.rb |  2 -
 .../features/lesson/updating_a_lesson_spec.rb |  2 -
 .../listing_school_classes_spec.rb            |  4 -
 .../listing_school_teachers_spec.rb           | 45 +++++-----
 spec/lib/classroom_management_spec.rb         |  8 +-
 spec/lib/profile_api_client_spec.rb           |  6 --
 spec/support/profile_api_mock.rb              |  4 -
 spec/support/user_profile_mock.rb             |  7 ++
 29 files changed, 273 insertions(+), 167 deletions(-)
 create mode 100644 lib/concepts/class_member/operations/list.rb
 create mode 100644 spec/concepts/class_member/list_spec.rb

diff --git a/.rubocop.yml b/.rubocop.yml
index b8ca7ed8..ce780ec6 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -30,9 +30,17 @@ RSpec/DescribeClass:
 RSpec/MultipleMemoizedHelpers:
   Max: 8
 
+RSpec/ExampleLength:
+  Enabled: false
+
 Metrics/BlockLength:
-  Exclude:
-    - "config/routes.rb"
+  Enabled: false
+
+Metrics/AbcSize:
+  Enabled: false
+
+Metrics/LineLength:
+  Enabled: false
 
 Naming/VariableNumber:
   EnforcedStyle: snake_case
diff --git a/app/controllers/api/class_members_controller.rb b/app/controllers/api/class_members_controller.rb
index 34454b2b..1a93b9cc 100644
--- a/app/controllers/api/class_members_controller.rb
+++ b/app/controllers/api/class_members_controller.rb
@@ -9,12 +9,10 @@ class ClassMembersController < ApiController
 
     def index
       @class_members = @school_class.members.accessible_by(current_ability)
-      student_ids = @class_members.pluck(:student_id)
-
-      result = SchoolStudent::List.call(school: @school, token: current_user.token, student_ids:)
+      result = ClassMember::List.call(school_class: @school_class, class_members: @class_members, token: current_user.token)
 
       if result.success?
-        @school_students = result[:school_students]
+        @class_members = result[:class_members]
         render :index, formats: [:json], status: :ok
       else
         render json: { error: result[:error] }, status: :unprocessable_entity
diff --git a/app/controllers/api/school_teachers_controller.rb b/app/controllers/api/school_teachers_controller.rb
index d7994495..e9f086e0 100644
--- a/app/controllers/api/school_teachers_controller.rb
+++ b/app/controllers/api/school_teachers_controller.rb
@@ -7,7 +7,8 @@ class SchoolTeachersController < ApiController
     authorize_resource :school_teacher, class: false
 
     def index
-      result = SchoolTeacher::List.call(school: @school, token: current_user.token)
+      teacher_ids = @school.roles.where(role: :teacher).pluck(:user_id)
+      result = SchoolTeacher::List.call(teacher_ids:)
 
       if result.success?
         @school_teachers = result[:school_teachers]
diff --git a/app/graphql/mutations/create_component.rb b/app/graphql/mutations/create_component.rb
index 8a3ca486..e1896fd9 100644
--- a/app/graphql/mutations/create_component.rb
+++ b/app/graphql/mutations/create_component.rb
@@ -26,9 +26,7 @@ def resolve(**input)
     end
 
     def ready?(**_args)
-      if context[:current_ability]&.can?(:create, Component, Project.new(user_id: context[:current_user]&.id))
-        return true
-      end
+      return true if context[:current_ability]&.can?(:create, Component, Project.new(user_id: context[:current_user]&.id))
 
       raise GraphQL::ExecutionError, 'You are not permitted to create a component'
     end
diff --git a/app/graphql/mutations/delete_project.rb b/app/graphql/mutations/delete_project.rb
index abc14154..c371219f 100644
--- a/app/graphql/mutations/delete_project.rb
+++ b/app/graphql/mutations/delete_project.rb
@@ -13,9 +13,7 @@ def resolve(**input)
 
       raise GraphQL::ExecutionError, 'Project not found' unless project
 
-      unless context[:current_ability].can?(:destroy, project)
-        raise GraphQL::ExecutionError, 'You are not permitted to delete that project'
-      end
+      raise GraphQL::ExecutionError, 'You are not permitted to delete that project' unless context[:current_ability].can?(:destroy, project)
 
       return { id: project.id } if project.destroy
 
diff --git a/app/graphql/mutations/remix_project.rb b/app/graphql/mutations/remix_project.rb
index 33a0ff41..e95c728f 100644
--- a/app/graphql/mutations/remix_project.rb
+++ b/app/graphql/mutations/remix_project.rb
@@ -7,7 +7,6 @@ class RemixProject < BaseMutation
 
     field :project, Types::ProjectType, description: 'The project that has been created'
 
-    # rubocop:disable Metrics/AbcSize
     def resolve(**input)
       original_project = GlobalID.find(input[:id])
       raise GraphQL::ExecutionError, 'Project not found' unless original_project
@@ -30,7 +29,6 @@ def resolve(**input)
 
       { project: response[:project] }
     end
-    # rubocop:enable Metrics/AbcSize
 
     def ready?(**_args)
       return true if can_create_project?
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 321629fb..e84ccbc6 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -3,7 +3,6 @@
 class Ability
   include CanCan::Ability
 
-  # rubocop:disable Metrics/AbcSize
   def initialize(user)
     # Anyone can view projects not owner by a user or a school.
     can :show, Project, user_id: nil, school_id: nil
@@ -43,7 +42,6 @@ def initialize(user)
       define_school_owner_abilities(school:) if user.school_owner?(school)
     end
   end
-  # rubocop:enable Metrics/AbcSize
 
   private
 
@@ -60,7 +58,6 @@ def define_school_owner_abilities(school:)
     can(%i[create], Project, school_id: school.id)
   end
 
-  # rubocop:disable Metrics/AbcSize
   def define_school_teacher_abilities(user:, school:)
     can(%i[read], School, id: school.id)
     can(%i[create], SchoolClass, school: { id: school.id })
@@ -81,9 +78,7 @@ def define_school_teacher_abilities(user:, school:)
     can(%i[read], Project,
         remixed_from_id: Project.where(user_id: user.id, school_id: school.id, remixed_from_id: nil).pluck(:id))
   end
-  # rubocop:enable Metrics/AbcSize
 
-  # rubocop:disable Layout/LineLength
   def define_school_student_abilities(user:, school:)
     can(%i[read], School, id: school.id)
     can(%i[read], SchoolClass, school: { id: school.id }, members: { student_id: user.id })
@@ -94,7 +89,6 @@ def define_school_student_abilities(user:, school:)
       school_student_can_toggle_finished?(user:, school:, project:)
     end
   end
-  # rubocop:enable Layout/LineLength
 
   def school_student_can_toggle_finished?(user:, school:, project:)
     is_my_project = project.user_id == user.id && project.school_id == school.id
diff --git a/app/models/filesystem_project.rb b/app/models/filesystem_project.rb
index 1d056ea9..c2622367 100644
--- a/app/models/filesystem_project.rb
+++ b/app/models/filesystem_project.rb
@@ -7,7 +7,7 @@ class FilesystemProject
   IMAGE_FORMATS = ['.png', '.jpg', '.jpeg', '.webp'].freeze
   PROJECTS_ROOT = Rails.root.join('lib/tasks/project_components')
 
-  def self.import_all! # rubocop:disable Metrics/AbcSize
+  def self.import_all!
     PROJECTS_ROOT.each_child do |dir|
       proj_config = YAML.safe_load_file(dir.join('project_config.yml').to_s)
       files = dir.children
diff --git a/app/services/school_verification_service.rb b/app/services/school_verification_service.rb
index 38b6f4ff..f9cb5b5a 100644
--- a/app/services/school_verification_service.rb
+++ b/app/services/school_verification_service.rb
@@ -7,7 +7,6 @@ def initialize(school)
     @school = school
   end
 
-  # rubocop:disable Metrics/AbcSize
   def verify(token:)
     School.transaction do
       school.verify!
@@ -22,7 +21,6 @@ def verify(token:)
   else
     true
   end
-  # rubocop:enable Metrics/AbcSize
 
   delegate :reject, to: :school
 end
diff --git a/app/views/api/class_members/index.json.jbuilder b/app/views/api/class_members/index.json.jbuilder
index 9dd65498..3e04d4cc 100644
--- a/app/views/api/class_members/index.json.jbuilder
+++ b/app/views/api/class_members/index.json.jbuilder
@@ -1,24 +1,16 @@
 # frozen_string_literal: true
 
-json.array!(@class_members, @school_students) do |class_member|
-  json.call(
-    class_member,
-    :id,
-    :school_class_id,
-    :student_id,
-    :created_at,
-    :updated_at
-  )
-
-  school_student = @school_students.find { |student| student.id == class_member.student_id }
-
-  if school_student.present?
-    json.set! :student do
+json.array!(@class_members) do |class_member|
+  if class_member.respond_to?(:student_id)
+    json.partial! 'class_member', class_member:
+  else
+    # Teachers are not modelled as ClassMembers
+    json.set! :teacher do
       json.call(
-        school_student,
+        class_member,
         :id,
-        :username,
-        :name
+        :name,
+        :email
       )
     end
   end
diff --git a/config/routes.rb b/config/routes.rb
index dd3ff9d6..9b07d3db 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop:disable Metrics/BlockLength
 Rails.application.routes.draw do
   namespace :admin do
     mount GoodJob::Engine => 'good_job'
@@ -73,4 +72,3 @@
   get '/auth/callback', to: 'auth#callback', as: 'callback'
   get '/logout', to: 'auth#destroy', as: 'logout'
 end
-# rubocop:enable Metrics/BlockLength
diff --git a/lib/concepts/class_member/operations/list.rb b/lib/concepts/class_member/operations/list.rb
new file mode 100644
index 00000000..e18997e1
--- /dev/null
+++ b/lib/concepts/class_member/operations/list.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class ClassMember
+  class List
+    class << self
+      def call(school_class:, class_members:, token:)
+        response = OperationResponse.new
+        response[:class_members] = []
+
+        begin
+          school = school_class.school
+          student_ids = class_members.pluck(:student_id)
+          students = SchoolStudent::List.call(school:, token:, student_ids:).fetch(:school_students, [])
+          class_members.each do |member|
+            member.student = students.find { |student| student.id == member.student_id }
+          end
+
+          teacher_ids = [school_class.teacher_id]
+          teachers = SchoolTeacher::List.call(teacher_ids:).fetch(:school_teachers, [])
+        rescue StandardError => e
+          Sentry.capture_exception(e)
+          response[:error] = "Error listing class members: #{e}"
+          return response
+        end
+
+        response[:class_members] = teachers + class_members.sort do |a, b|
+          a.student.name <=> b.student.name
+        end
+
+        response
+      end
+    end
+  end
+end
diff --git a/lib/concepts/school_teacher/list.rb b/lib/concepts/school_teacher/list.rb
index dfefc553..f8cfede1 100644
--- a/lib/concepts/school_teacher/list.rb
+++ b/lib/concepts/school_teacher/list.rb
@@ -3,9 +3,9 @@
 module SchoolTeacher
   class List
     class << self
-      def call(school:, token:)
+      def call(teacher_ids:)
         response = OperationResponse.new
-        response[:school_teachers] = list_teachers(school, token)
+        response[:school_teachers] = list_teachers(teacher_ids)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -15,11 +15,8 @@ def call(school:, token:)
 
       private
 
-      def list_teachers(school, token)
-        response = ProfileApiClient.list_school_teachers(token:, organisation_id: school.id)
-        user_ids = response.fetch(:ids)
-
-        User.from_userinfo(ids: user_ids)
+      def list_teachers(ids)
+        User.from_userinfo(ids:)
       end
     end
   end
diff --git a/lib/concepts/school_teacher/remove.rb b/lib/concepts/school_teacher/remove.rb
index 50561b4e..505f331e 100644
--- a/lib/concepts/school_teacher/remove.rb
+++ b/lib/concepts/school_teacher/remove.rb
@@ -16,6 +16,7 @@ def call(school:, teacher_id:, token:)
       private
 
       def remove_teacher(school, teacher_id, token)
+        # TODO: This has not been implemented yet
         ProfileApiClient.remove_school_teacher(token:, teacher_id:, organisation_id: school.id)
       end
     end
diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb
index 98344582..6bcdc466 100644
--- a/lib/profile_api_client.rb
+++ b/lib/profile_api_client.rb
@@ -71,10 +71,6 @@ def remove_school_owner(*)
       {}
     end
 
-    def list_school_teachers(*)
-      {}
-    end
-
     def remove_school_teacher(*)
       {}
     end
@@ -117,7 +113,6 @@ def create_school_student(token:, username:, password:, name:, school_id:)
       raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
 
-    # rubocop:disable Metrics/AbcSize
     def update_school_student(token:, school_id:, student_id:, name: nil, username: nil, password: nil) # rubocop:disable Metrics/ParameterLists
       return nil if token.blank?
 
@@ -135,7 +130,6 @@ def update_school_student(token:, school_id:, student_id:, name: nil, username:
     rescue Faraday::UnprocessableEntityError => e
       raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
-    # rubocop:enable Metrics/AbcSize
 
     def delete_school_student(token:, school_id:, student_id:)
       return nil if token.blank?
diff --git a/lib/tasks/classroom_management.rake b/lib/tasks/classroom_management.rake
index 1b4032f0..add61a6d 100644
--- a/lib/tasks/classroom_management.rake
+++ b/lib/tasks/classroom_management.rake
@@ -10,7 +10,6 @@ Rails.logger = Logger.new($stdout) unless Rails.env.test?
 
 # For students to match up the school needs to match with the school defined in profile (hard coded in the helper)
 
-# rubocop:disable Metrics/BlockLength
 namespace :classroom_management do
   include ClassroomManagementHelper
 
@@ -117,4 +116,3 @@ namespace :classroom_management do
     end
   end
 end
-# rubocop:enable Metrics/BlockLength
diff --git a/lib/tasks/classroom_management_helper.rb b/lib/tasks/classroom_management_helper.rb
index 3cd281fc..e296e817 100644
--- a/lib/tasks/classroom_management_helper.rb
+++ b/lib/tasks/classroom_management_helper.rb
@@ -72,7 +72,6 @@ def assign_students(school_class, school)
     end
   end
 
-  # rubocop:disable Metrics/AbcSize
   def create_lessons(user_id, school, school_class, visibility = 'students')
     2.times.map do |i|
       Lesson.find_or_create_by!(school:, school_class:,
@@ -87,7 +86,6 @@ def create_lessons(user_id, school, school_class, visibility = 'students')
       end
     end
   end
-  # rubocop:enable Metrics/AbcSize
 
   def create_project(user_id, school, lesson)
     Project.find_or_create_by!(user_id:, school:, lesson:) do |project|
diff --git a/spec/concepts/class_member/list_spec.rb b/spec/concepts/class_member/list_spec.rb
new file mode 100644
index 00000000..e5b2233a
--- /dev/null
+++ b/spec/concepts/class_member/list_spec.rb
@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ClassMember::List, type: :unit do
+  let(:token) { UserProfileMock::TOKEN }
+  let(:school) { create(:school) }
+  let(:students) { create_list(:student, 3, school:) }
+  let(:teacher) { create(:teacher, school:) }
+  let(:school_class) { create(:school_class, teacher_id: teacher.id, school:) }
+  let(:class_members) { school_class.members }
+
+  let(:student_ids) { students.map(&:id) }
+  let(:teacher_ids) { [teacher.id] }
+
+  context 'with students and a teacher' do
+    before do
+      student_ids.each do |student_id|
+        create(:class_member, school_class:, student_id:)
+      end
+
+      student_attributes = students.map do |student|
+        { id: student.id, name: student.name, username: student.username }
+      end
+      stub_profile_api_list_school_students(school:, student_attributes:)
+      stub_user_info_api_for(teacher)
+    end
+
+    it 'returns a successful operation response' do
+      response = described_class.call(school_class:, class_members:, token:)
+      expect(response.success?).to be(true)
+    end
+
+    it 'returns class members in the operation response' do
+      response = described_class.call(school_class:, class_members:, token:)
+      expect(response[:class_members].count { |member| member.is_a?(ClassMember) }).to eq(3)
+    end
+
+    it 'returns the teacher in the operation response' do
+      response = described_class.call(school_class:, class_members:, token:)
+      expect(response[:class_members].count { |member| member.is_a?(User) }).to eq(1)
+    end
+
+    it 'contains the expected students' do
+      response = described_class.call(school_class:, class_members:, token:)
+      class_members.each do |class_member|
+        expect(response[:class_members].map(&:id)).to include(class_member.id)
+      end
+    end
+
+    it 'contains the expected teacher' do
+      response = described_class.call(school_class:, class_members:, token:)
+      expect(response[:class_members].map(&:id)).to include(teacher.id)
+    end
+  end
+
+  context 'when errors occur' do
+    before do
+      allow(Sentry).to receive(:capture_exception)
+    end
+
+    # rubocop:disable RSpec/MultipleExpectations
+    it 'captures and handles errors' do
+      allow(SchoolStudent::List).to receive(:call).and_raise(StandardError.new('forced error'))
+
+      response = described_class.call(school_class:, class_members:, token:)
+
+      expect(response[:error]).to eq('Error listing class members: forced error')
+      expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError))
+    end
+    # rubocop:enable RSpec/MultipleExpectations
+
+    it 'returns an empty array when no ids match' do
+      allow(SchoolStudent::List).to receive(:call).and_return({ school_students: [] })
+      allow(SchoolTeacher::List).to receive(:call).and_return({ school_teachers: [] })
+
+      response = described_class.call(school_class:, class_members:, token:)
+
+      expect(response[:class_members]).to eq([])
+    end
+  end
+end
diff --git a/spec/concepts/school_teacher/list_spec.rb b/spec/concepts/school_teacher/list_spec.rb
index 407df33b..4e403b56 100644
--- a/spec/concepts/school_teacher/list_spec.rb
+++ b/spec/concepts/school_teacher/list_spec.rb
@@ -3,51 +3,40 @@
 require 'rails_helper'
 
 RSpec.describe SchoolTeacher::List, type: :unit do
-  let(:token) { UserProfileMock::TOKEN }
   let(:school) { create(:school) }
-  let(:teacher) { create(:teacher, school:) }
+  let(:teachers) { create_list(:teacher, 3, school:) }
+  let(:teacher_ids) { teachers.map(&:id) }
+  let(:response) { described_class.call(teacher_ids:) }
 
-  before do
-    stub_profile_api_list_school_teachers(user_id: teacher.id)
-    stub_user_info_api_for(teacher)
-  end
-
-  it 'returns a successful operation response' do
-    response = described_class.call(school:, token:)
-    expect(response.success?).to be(true)
-  end
-
-  it 'makes a profile API call' do
-    described_class.call(school:, token:)
+  context 'when successful' do
+    before do
+      allow(User).to receive(:from_userinfo).with(ids: teacher_ids).and_return(teachers)
+    end
 
-    # TODO: Replace with WebMock assertion once the profile API has been built.
-    expect(ProfileApiClient).to have_received(:list_school_teachers).with(token:, organisation_id: school.id)
+    # rubocop:disable RSpec/MultipleExpectations
+    it 'returns a successful response with school teachers' do
+      expect(response[:school_teachers]).to eq(teachers)
+      expect(response[:error]).to be_nil
+    end
+    # rubocop:enable RSpec/MultipleExpectations
   end
 
-  it 'returns the school teachers in the operation response' do
-    response = described_class.call(school:, token:)
-    expect(response[:school_teachers].first).to be_a(User)
-  end
+  context 'when an error occurs' do
+    let(:error_message) { 'Error listing school teachers: some error' }
 
-  context 'when listing fails' do
     before do
-      allow(ProfileApiClient).to receive(:list_school_teachers).and_raise('Some API error')
+      allow(User).to receive(:from_userinfo).with(ids: teacher_ids).and_raise(StandardError.new('some error'))
       allow(Sentry).to receive(:capture_exception)
     end
 
-    it 'returns a failed operation response' do
-      response = described_class.call(school:, token:)
-      expect(response.failure?).to be(true)
-    end
-
-    it 'returns the error message in the operation response' do
-      response = described_class.call(school:, token:)
-      expect(response[:error]).to match(/Some API error/)
-    end
-
-    it 'sent the exception to Sentry' do
-      described_class.call(school:, token:)
-      expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+    # rubocop:disable RSpec/MultipleExpectations
+    it 'captures the exception and returns an error response' do
+      # Call the method to ensure the error is raised and captured
+      response
+      expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError))
+      expect(response[:school_teachers]).to be_nil
+      expect(response[:error]).to eq(error_message)
     end
+    # rubocop:enable RSpec/MultipleExpectations
   end
 end
diff --git a/spec/features/class_member/listing_class_members_spec.rb b/spec/features/class_member/listing_class_members_spec.rb
index 03701ffb..73fb0d6c 100644
--- a/spec/features/class_member/listing_class_members_spec.rb
+++ b/spec/features/class_member/listing_class_members_spec.rb
@@ -3,23 +3,27 @@
 require 'rails_helper'
 
 RSpec.describe 'Listing class members', type: :request do
+  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+  let(:school) { create(:school) }
+  let(:owner) { create(:owner, school:) }
+  let(:teacher) { create(:teacher, school:) }
+  let(:students) { create_list(:student, 3, school:) }
+  let(:school_class) { build(:school_class, teacher_id: teacher.id, school:) }
+
   before do
     authenticated_in_hydra_as(owner)
+
     student_attributes = students.map do |student|
       { id: student.id, name: student.name, username: student.username }
     end
     stub_profile_api_list_school_students(school:, student_attributes:)
+
     students.each do |student|
       create(:class_member, student_id: student.id, school_class:)
     end
-  end
 
-  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
-  let(:school_class) { build(:school_class, teacher_id: teacher.id, school:) }
-  let(:school) { create(:school) }
-  let(:students) { create_list(:student, 3, school:) }
-  let(:teacher) { create(:teacher, school:) }
-  let(:owner) { create(:owner, school:) }
+    stub_user_info_api_for(teacher)
+  end
 
   it 'responds 200 OK' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
@@ -30,37 +34,79 @@
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(data.size).to eq(3)
+    expect(data.size).to eq(4)
   end
 
-  it 'responds with the correct student_ids' do
+  it 'responds with the correct member ids, where applicable' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    student_ids_from_response = data.pluck(:student_id)
-    expected_student_ids = students.map(&:id)
+    school_class.members.each do |class_member|
+      expect(data.pluck(:id)).to include(class_member.id)
+    end
+  end
 
-    expect(student_ids_from_response).to match_array(expected_student_ids)
+  it 'responds with the correct student ids' do
+    get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
+    data = JSON.parse(response.body, symbolize_names: true)
+    student_ids = data.pluck(:student).compact.pluck(:id)
+
+    school_class.members.each do |class_member|
+      expect(student_ids).to include(class_member.student_id)
+    end
+  end
+
+  it 'responds with the expected student parameters' do
+    get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
+    data = JSON.parse(response.body, symbolize_names: true)
+    student_data = data.pluck(:student).compact.find { |member| member[:id] == students[0].id }
+
+    expect(student_data).to eq(
+      {
+        id: students[0].id,
+        username: students[0].username,
+        name: students[0].name
+      }
+    )
   end
 
-  it 'responds with the correct nested student ids' do
+  it 'responds with the correct teacher id' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
+    teacher_id = data.pluck(:teacher).compact.pick(:id)
 
-    student_ids_from_response = data.map { |member| member[:student][:id] }
-    expected_student_ids = students.map(&:id)
+    expect(teacher_id).to eq(teacher.id)
+  end
+
+  it 'responds with the expected teacher parameters' do
+    get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
+    data = JSON.parse(response.body, symbolize_names: true)
+    teacher_data = data.pluck(:teacher).compact
+
+    expect(teacher_data.first).to eq(
+      {
+        id: teacher.id,
+        name: teacher.name,
+        email: teacher.email
+      }
+    )
+  end
+
+  it 'responds with teachers at the top' do
+    get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
+    data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(student_ids_from_response).to match_array(expected_student_ids)
+    expect(data[0][:teacher]).to be_truthy
   end
 
-  it 'responds with the students JSON' do
+  it 'responds with students in alphabetical order by name ascending' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    student_names_from_response = data.map { |member| member[:student][:name] }
-    expected_student_names = students.map(&:name)
+    student_names = data.pluck(:student).compact.pluck(:name)
+    sorted_student_names = student_names.sort
 
-    expect(student_names_from_response).to match_array(expected_student_names)
+    expect(student_names).to eq(sorted_student_names)
   end
 
   it "responds with nil attributes for students if the user profile doesn't exist" do
@@ -72,7 +118,6 @@
     expect(data.first[:student_name]).to be_nil
   end
 
-  # rubocop:disable RSpec/ExampleLength
   it 'does not include class members that belong to a different class' do
     student = create(:student, school:)
     different_class = create(:school_class, school:, teacher_id: teacher.id)
@@ -81,9 +126,8 @@
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(data.size).to eq(3)
+    expect(data.size).to eq(4)
   end
-  # rubocop:enable RSpec/ExampleLength
 
   it 'responds 401 Unauthorized when no token is given' do
     get "/api/schools/#{school.id}/classes/#{school_class.id}/members"
diff --git a/spec/features/lesson/listing_lessons_spec.rb b/spec/features/lesson/listing_lessons_spec.rb
index 93e6c34d..df501817 100644
--- a/spec/features/lesson/listing_lessons_spec.rb
+++ b/spec/features/lesson/listing_lessons_spec.rb
@@ -48,7 +48,6 @@
     expect(data.first[:project]).to eq(expected_project)
   end
 
-  # rubocop:disable RSpec/ExampleLength
   it "responds with nil attributes for the user if their user profile doesn't exist" do
     user_id = SecureRandom.uuid
     stub_user_info_api_for_unknown_users(user_id:)
@@ -59,7 +58,6 @@
 
     expect(data.first[:user_name]).to be_nil
   end
-  # rubocop:enable RSpec/ExampleLength
 
   it 'does not include archived lessons' do
     lesson.archive!
@@ -190,7 +188,6 @@
     let!(:lesson) { create(:lesson, school_class:, name: 'Test Lesson', visibility: 'students', user_id: teacher.id) }
     let(:teacher) { create(:teacher, school:) }
 
-    # rubocop:disable RSpec/ExampleLength
     it 'includes the lesson when the user owns the lesson' do
       another_teacher = create(:teacher, school:)
       authenticated_in_hydra_as(another_teacher)
@@ -201,9 +198,7 @@
 
       expect(data.size).to eq(1)
     end
-    # rubocop:enable RSpec/ExampleLength
 
-    # rubocop:disable RSpec/ExampleLength
     it "includes the lesson when the user is a school-student within the lesson's class" do
       student = create(:student, school:)
       authenticated_in_hydra_as(student)
@@ -214,7 +209,6 @@
 
       expect(data.size).to eq(1)
     end
-    # rubocop:enable RSpec/ExampleLength
 
     it "does not include the lesson when the user is not a school-student within the lesson's class" do
       student = create(:student, school:)
diff --git a/spec/features/lesson/showing_a_lesson_spec.rb b/spec/features/lesson/showing_a_lesson_spec.rb
index 4856da1e..674b0481 100644
--- a/spec/features/lesson/showing_a_lesson_spec.rb
+++ b/spec/features/lesson/showing_a_lesson_spec.rb
@@ -38,7 +38,6 @@
     expect(data[:user_name]).to eq('School Teacher')
   end
 
-  # rubocop:disable RSpec/ExampleLength
   it "responds with nil attributes for the user if their user profile doesn't exist" do
     user_id = SecureRandom.uuid
     stub_user_info_api_for_unknown_users(user_id:)
@@ -49,7 +48,6 @@
 
     expect(data[:user_name]).to be_nil
   end
-  # rubocop:enable RSpec/ExampleLength
 
   it 'responds 404 Not Found when no lesson exists' do
     get('/api/lessons/not-a-real-id', headers:)
diff --git a/spec/features/lesson/updating_a_lesson_spec.rb b/spec/features/lesson/updating_a_lesson_spec.rb
index 97c61cd6..b43f04e5 100644
--- a/spec/features/lesson/updating_a_lesson_spec.rb
+++ b/spec/features/lesson/updating_a_lesson_spec.rb
@@ -115,7 +115,6 @@
       expect(response).to have_http_status(:ok)
     end
 
-    # rubocop:disable RSpec/ExampleLength
     it 'responds 422 Unprocessable Entity when trying to re-assign the lesson to a different class' do
       school = create(:school, id: SecureRandom.uuid)
       teacher = create(:teacher, school:)
@@ -126,6 +125,5 @@
 
       expect(response).to have_http_status(:unprocessable_entity)
     end
-    # rubocop:enable RSpec/ExampleLength
   end
 end
diff --git a/spec/features/school_class/listing_school_classes_spec.rb b/spec/features/school_class/listing_school_classes_spec.rb
index a1f11395..99c15f47 100644
--- a/spec/features/school_class/listing_school_classes_spec.rb
+++ b/spec/features/school_class/listing_school_classes_spec.rb
@@ -45,7 +45,6 @@
     expect(data.first[:teacher_name]).to be_nil
   end
 
-  # rubocop:disable RSpec/ExampleLength
   it "does not include school classes that the school-teacher doesn't teach" do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)
@@ -56,9 +55,7 @@
 
     expect(data.size).to eq(1)
   end
-  # rubocop:enable RSpec/ExampleLength
 
-  # rubocop:disable RSpec/ExampleLength
   it "does not include school classes that the school-student isn't a member of" do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(student)
@@ -69,7 +66,6 @@
 
     expect(data.size).to eq(1)
   end
-  # rubocop:enable RSpec/ExampleLength
 
   it 'responds 401 Unauthorized when no token is given' do
     get "/api/schools/#{school.id}/classes"
diff --git a/spec/features/school_teacher/listing_school_teachers_spec.rb b/spec/features/school_teacher/listing_school_teachers_spec.rb
index f5b379ea..02fed958 100644
--- a/spec/features/school_teacher/listing_school_teachers_spec.rb
+++ b/spec/features/school_teacher/listing_school_teachers_spec.rb
@@ -3,37 +3,22 @@
 require 'rails_helper'
 
 RSpec.describe 'Listing school teachers', type: :request do
-  before do
-    authenticated_in_hydra_as(owner)
-    stub_profile_api_list_school_teachers(user_id: teacher.id)
-    stub_user_info_api_for(teacher)
-  end
-
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
   let(:school) { create(:school) }
-  let(:teacher) { create(:teacher, school:, name: 'School Teacher') }
   let(:owner) { create(:owner, school:) }
+  let(:teacher) { create(:teacher, school:, name: 'School Teacher') }
+  let(:teacher_2) { create(:teacher, school:) }
 
-  it 'responds 200 OK' do
-    get("/api/schools/#{school.id}/teachers", headers:)
-    expect(response).to have_http_status(:ok)
+  before do
+    authenticated_in_hydra_as(owner)
+    stub_user_info_api_for(teacher)
   end
 
-  it 'responds 200 OK when the user is a school-teacher' do
-    teacher = create(:teacher, school:)
-    authenticated_in_hydra_as(teacher)
-
+  it 'responds 200 OK' do
     get("/api/schools/#{school.id}/teachers", headers:)
     expect(response).to have_http_status(:ok)
   end
 
-  it 'responds with the school teachers JSON' do
-    get("/api/schools/#{school.id}/teachers", headers:)
-    data = JSON.parse(response.body, symbolize_names: true)
-
-    expect(data.first[:name]).to eq('School Teacher')
-  end
-
   it 'responds 401 Unauthorized when no token is given' do
     get "/api/schools/#{school.id}/teachers"
     expect(response).to have_http_status(:unauthorized)
@@ -55,4 +40,22 @@
     get("/api/schools/#{school.id}/teachers", headers:)
     expect(response).to have_http_status(:forbidden)
   end
+
+  it 'responds 200 OK when the user is a school-teacher' do
+    stub_user_info_api_for_users([teacher.id, teacher_2.id], users: [teacher, teacher_2])
+    authenticated_in_hydra_as(teacher_2)
+
+    get("/api/schools/#{school.id}/teachers", headers:)
+    expect(response).to have_http_status(:ok)
+  end
+
+  it 'responds with the school teachers JSON' do
+    stub_user_info_api_for_users([teacher.id, teacher_2.id], users: [teacher, teacher_2])
+    authenticated_in_hydra_as(teacher_2)
+
+    get("/api/schools/#{school.id}/teachers", headers:)
+    data = JSON.parse(response.body, symbolize_names: true)
+
+    expect(data.pluck(:name)).to include(teacher_2.name)
+  end
 end
diff --git a/spec/lib/classroom_management_spec.rb b/spec/lib/classroom_management_spec.rb
index 72c0ffca..6e787cb8 100644
--- a/spec/lib/classroom_management_spec.rb
+++ b/spec/lib/classroom_management_spec.rb
@@ -23,7 +23,7 @@
       create(:lesson, school_id: school.id, user_id: creator_id)
     end
 
-    # rubocop:disable RSpec/ExampleLength, RSpec/MultipleExpectations
+    # rubocop:disable RSpec/MultipleExpectations
     it 'destroys all seed data' do
       task.invoke
       expect(Role.where(user_id: [creator_id, teacher_id, student_1, student_2])).not_to exist
@@ -33,7 +33,7 @@
       expect(Lesson.where(school_id: school.id)).not_to exist
       expect(Project.where(school_id: school.id)).not_to exist
     end
-    # rubocop:enable RSpec/ExampleLength, RSpec/MultipleExpectations
+    # rubocop:enable RSpec/MultipleExpectations
   end
 
   describe ':seed_an_unverified_school' do
@@ -80,7 +80,7 @@
       expect(Role.teacher.where(user_id: teacher_id, school_id: school.id)).to exist
     end
 
-    # rubocop:disable RSpec/MultipleExpectations, RSpec/ExampleLength
+    # rubocop:disable RSpec/MultipleExpectations
     it 'assigns students' do
       school_id = School.find_by(creator_id:).id
       school_class_id = SchoolClass.find_by(school_id:).id
@@ -89,6 +89,6 @@
       expect(Role.student.where(user_id: student_2, school_id:)).to exist
       expect(ClassMember.where(student_id: student_2, school_class_id:)).to exist
     end
-    # rubocop:enable RSpec/MultipleExpectations, RSpec/ExampleLength
+    # rubocop:enable RSpec/MultipleExpectations
   end
 end
diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb
index 1663fb2a..48ed7ce3 100644
--- a/spec/lib/profile_api_client_spec.rb
+++ b/spec/lib/profile_api_client_spec.rb
@@ -172,7 +172,6 @@ def create_school
       expect(WebMock).to have_requested(:get, list_safeguarding_flags_url).with(headers: { 'accept' => 'application/json' })
     end
 
-    # rubocop:disable RSpec/ExampleLength
     it 'returns list of safeguarding flags if successful' do
       flag = {
         id: '7ac79585-e187-4d2f-bf0c-a1cbe72ecc9a',
@@ -188,7 +187,6 @@ def create_school
         .to_return(status: 200, body: [flag].to_json, headers: { 'content-type' => 'application/json' })
       expect(list_safeguarding_flags).to eq([expected])
     end
-    # rubocop:enable RSpec/ExampleLength
 
     it 'raises exception if anything other than a 200 status code is returned' do
       stub_request(:get, list_safeguarding_flags_url)
@@ -459,7 +457,6 @@ def create_school_student
       expect(WebMock).to have_requested(:post, list_students_url).with(body: student_ids)
     end
 
-    # rubocop:disable RSpec/ExampleLength
     it 'returns the student(s) if successful' do
       student = {
         id: '549e4674-6ffd-4ac6-9a97-b4d7e5c0e5c5',
@@ -475,7 +472,6 @@ def create_school_student
         .to_return(status: 200, body: [student].to_json, headers: { 'content-type' => 'application/json' })
       expect(list_school_students).to eq([expected])
     end
-    # rubocop:enable RSpec/ExampleLength
 
     it 'raises exception if anything other that 200 status code is returned' do
       stub_request(:post, list_students_url)
@@ -637,7 +633,6 @@ def update_school_student
       expect(WebMock).to have_requested(:get, student_url).with(headers: { 'accept' => 'application/json' })
     end
 
-    # rubocop:disable RSpec/ExampleLength
     it 'returns the student(s) if successful' do
       student = {
         id: '549e4674-6ffd-4ac6-9a97-b4d7e5c0e5c5',
@@ -653,7 +648,6 @@ def update_school_student
         .to_return(status: 200, body: student.to_json, headers: { 'content-type' => 'application/json' })
       expect(school_student).to eq(expected)
     end
-    # rubocop:enable RSpec/ExampleLength
 
     it 'raises exception if anything other than a 200 status code is returned' do
       stub_request(:get, student_url)
diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb
index dc43ff60..c5c1db29 100644
--- a/spec/support/profile_api_mock.rb
+++ b/spec/support/profile_api_mock.rb
@@ -15,10 +15,6 @@ def stub_profile_api_remove_school_owner
     allow(ProfileApiClient).to receive(:remove_school_owner)
   end
 
-  def stub_profile_api_list_school_teachers(user_id:)
-    allow(ProfileApiClient).to receive(:list_school_teachers).and_return(ids: [user_id])
-  end
-
   def stub_profile_api_remove_school_teacher
     allow(ProfileApiClient).to receive(:remove_school_teacher)
   end
diff --git a/spec/support/user_profile_mock.rb b/spec/support/user_profile_mock.rb
index bdcee714..4f56e353 100644
--- a/spec/support/user_profile_mock.rb
+++ b/spec/support/user_profile_mock.rb
@@ -51,4 +51,11 @@ def stub_user_info_api(user_id:, users:)
       .with(headers: { Authorization: "Bearer #{UserInfoApiClient::API_KEY}" }, body: /#{user_id}/)
       .to_return({ body: { users: }.to_json, headers: { 'Content-Type' => 'application/json' } })
   end
+
+  # Stubs the api to accept multiple user ids and return multiple users
+  def stub_user_info_api_for_users(user_ids, users:)
+    stub_request(:get, "#{UserInfoApiClient::API_URL}/users")
+      .with(headers: { Authorization: "Bearer #{UserInfoApiClient::API_KEY}" }, body: { userIds: user_ids }.to_json)
+      .to_return({ body: { users: }.to_json, headers: { 'Content-Type' => 'application/json' } })
+  end
 end