List class member students (#391)

## What's changed?

Updated class members controller to utilise `SchoolStudents:List` and
return the student details in a nested `student` attribute in the class
members response.

Previous Profile API mocks were for a single student so I expanded this
to allow and test for a filtered subset using the `student_ids` array

app/controllers/api/class_members_controller.rb

@@ -8,8 +8,17 @@ class ClassMembersController < ApiController
     load_and_authorize_resource :class_member, through: :school_class, through_association: :members
 
     def index
-      @class_members_with_students = @school_class.members.accessible_by(current_ability).with_students
-      render :index, formats: [:json], status: :ok
+      @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:)
+
+      if result.success?
+        @school_students = result[:school_students]
+        render :index, formats: [:json], status: :ok
+      else
+        render json: { error: result[:error] }, status: :unprocessable_entity
+      end
     end
 
     def create
@@ -36,7 +45,7 @@ def destroy
     private
 
     def class_member_params
-      params.require(:class_member).permit(:student_id)
+      params.require(:class_member).permit(:student_id, student_ids: [])
     end
   end
 end

app/views/api/class_members/index.json.jbuilder

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-json.array!(@class_members_with_students) do |class_member, student|
+json.array!(@class_members, @school_students) do |class_member|
   json.call(
     class_member,
     :id,
@@ -10,6 +10,16 @@ json.array!(@class_members_with_students) do |class_member, student|
     :updated_at
   )
 
-  json.student_username(student&.username)
-  json.student_name(student&.name)
+  school_student = @school_students.find { |student| student.id == class_member.student_id }
+
+  if school_student.present?
+    json.set! :student do
+      json.call(
+        school_student,
+        :id,
+        :username,
+        :name
+      )
+    end
+  end
 end

lib/concepts/school_student/list.rb

@@ -3,9 +3,9 @@
 module SchoolStudent
   class List
     class << self
-      def call(school:, token:)
+      def call(school:, token:, student_ids: nil)
         response = OperationResponse.new
-        response[:school_students] = list_students(school, token)
+        response[:school_students] = list_students(school, token, student_ids)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -15,8 +15,8 @@ def call(school:, token:)
 
       private
 
-      def list_students(school, token)
-        student_ids = Role.student.where(school:).map(&:user_id)
+      def list_students(school, token, student_ids)
+        student_ids ||= Role.student.where(school:).map(&:user_id)
         ProfileApiClient.list_school_students(token:, school_id: school.id, student_ids:).map do |student|
           User.new(student.to_h.slice(:id, :username, :name))
         end

spec/concepts/school_student/list_spec.rb

@@ -5,49 +5,99 @@
 RSpec.describe SchoolStudent::List, type: :unit do
   let(:token) { UserProfileMock::TOKEN }
   let(:school) { create(:school) }
-  let(:student) { create(:student, school:) }
+  let(:students) { create_list(:student, 3, school:) }
 
-  before do
-    stub_profile_api_list_school_students(school:, id: student.id, name: 'name', username: 'username')
-    stub_user_info_api_for(student)
-  end
+  context 'without student_ids' do
+    before do
+      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:)
+    end
 
-  it 'returns a successful operation response' do
-    response = described_class.call(school:, token:)
-    expect(response.success?).to be(true)
-  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:)
+    it 'makes a profile API call' do
+      described_class.call(school:, token:)
 
-    # TODO: Replace with WebMock assertion once the profile API has been built.
-    expect(ProfileApiClient).to have_received(:list_school_students).with(token:, school_id: school.id, student_ids: [student.id])
+      # TODO: Replace with WebMock assertion once the profile API has been built.
+      expect(ProfileApiClient).to have_received(:list_school_students).with(token:, school_id: school.id, student_ids: students.map(&:id))
+    end
+
+    it 'returns a school students JSON array' do
+      response = described_class.call(school:, token:)
+      expect(response[:school_students].size).to eq(3)
+    end
+
+    it 'returns the school students in the operation response' do
+      response = described_class.call(school:, token:)
+      students.each do |student|
+        expected_user = User.new(id: student.id, name: student.name, username: student.username)
+        expect(response[:school_students]).to include(expected_user)
+      end
+    end
   end
 
-  it 'returns the school students in the operation response' do
-    response = described_class.call(school:, token:)
-    expected_user = User.new(id: student.id, name: 'name', username: 'username')
-    expect(response[:school_students].first).to eq(expected_user)
+  context 'with student_ids' do
+    let(:student_ids) { students.map(&:id).take(2) }
+    let(:filtered_students) { students.select { |student| student_ids.include?(student.id) } }
+
+    before do
+      student_attributes = filtered_students.map do |student|
+        { id: student.id, name: student.name, username: student.username }
+      end
+      stub_profile_api_list_school_students(school:, student_attributes:)
+    end
+
+    it 'returns a successful operation response' do
+      response = described_class.call(school:, token:, student_ids:)
+      expect(response.success?).to be(true)
+    end
+
+    it 'makes a profile API call' do
+      described_class.call(school:, token:, student_ids:)
+
+      # TODO: Replace with WebMock assertion once the profile API has been built.
+      expect(ProfileApiClient).to have_received(:list_school_students).with(token:, school_id: school.id, student_ids:)
+    end
+
+    it 'returns a filtered school students JSON array' do
+      response = described_class.call(school:, token:, student_ids:)
+      expect(response[:school_students].size).to eq(2)
+    end
+
+    it 'returns the filtered school students in the operation response' do
+      response = described_class.call(school:, token:, student_ids:)
+      filtered_students.each do |student|
+        expected_user = User.new(id: student.id, name: student.name, username: student.username)
+        expect(response[:school_students]).to include(expected_user)
+      end
+    end
   end
 
   context 'when listing fails' do
+    let(:student_ids) { [123] }
+
     before do
       allow(ProfileApiClient).to receive(:list_school_students).and_raise('Some API error')
       allow(Sentry).to receive(:capture_exception)
     end
 
     it 'returns a failed operation response' do
-      response = described_class.call(school:, token:)
+      response = described_class.call(school:, token:, student_ids:)
       expect(response.failure?).to be(true)
     end
 
     it 'returns the error message in the operation response' do
-      response = described_class.call(school:, token:)
+      response = described_class.call(school:, token:, student_ids:)
       expect(response[:error]).to match(/Some API error/)
     end
 
     it 'sent the exception to Sentry' do
-      described_class.call(school:, token:)
+      described_class.call(school:, token:, student_ids:)
       expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
     end
   end

spec/factories/user.rb

@@ -11,6 +11,8 @@
     end
 
     factory :student do
+      email { nil }
+      username { Faker::Internet.username }
       transient do
         school { nil }
       end

spec/features/class_member/listing_class_members_spec.rb

@@ -5,14 +5,19 @@
 RSpec.describe 'Listing class members', type: :request do
   before do
     authenticated_in_hydra_as(owner)
-    stub_user_info_api_for(student)
-    create(:class_member, student_id: student.id, school_class:)
+    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(:student) { create(:student, school:, name: 'School Student') }
+  let(:students) { create_list(:student, 3, school:) }
   let(:teacher) { create(:teacher, school:) }
   let(:owner) { create(:owner, school:) }
 
@@ -21,22 +26,45 @@
     expect(response).to have_http_status(:ok)
   end
 
-  it 'responds with the class members JSON' do
+  it 'responds with the class members JSON array' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(data.first[:student_id]).to eq(student.id)
+    expect(data.size).to eq(3)
+  end
+
+  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_from_response = data.pluck(:student_id)
+    expected_student_ids = students.map(&:id)
+
+    expect(student_ids_from_response).to match_array(expected_student_ids)
+  end
+
+  it 'responds with the correct nested student ids' 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.map { |member| member[:student][:id] }
+    expected_student_ids = students.map(&:id)
+
+    expect(student_ids_from_response).to match_array(expected_student_ids)
   end
 
   it 'responds with the students JSON' do
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(data.first[:student_name]).to eq('School Student')
+    student_names_from_response = data.map { |member| member[:student][:name] }
+    expected_student_names = students.map(&:name)
+
+    expect(student_names_from_response).to match_array(expected_student_names)
   end
 
   it "responds with nil attributes for students if the user profile doesn't exist" do
-    stub_user_info_api_for_unknown_users(user_id: student.id)
+    stub_user_info_api_for_unknown_users(user_id: students.first.id)
 
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
@@ -53,7 +81,7 @@
     get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(data.size).to eq(1)
+    expect(data.size).to eq(3)
   end
   # rubocop:enable RSpec/ExampleLength
 

spec/features/school_student/listing_school_students_spec.rb

@@ -5,7 +5,8 @@
 RSpec.describe 'Listing school students', type: :request do
   before do
     authenticated_in_hydra_as(owner)
-    stub_profile_api_list_school_students(school:, id: student.id, name: 'School Student')
+    student_attributes = [{ id: student.id, name: 'School Student' }]
+    stub_profile_api_list_school_students(school:, student_attributes:)
     stub_profile_api_create_safeguarding_flag
   end
 

spec/support/profile_api_mock.rb

@@ -23,14 +23,20 @@ def stub_profile_api_remove_school_teacher
     allow(ProfileApiClient).to receive(:remove_school_teacher)
   end
 
-  def stub_profile_api_list_school_students(school:, id:, username: '', name: '')
+  def stub_profile_api_list_school_students(school:, student_attributes:)
     now = Time.current.to_fs(:iso8601) # rubocop:disable Naming/VariableNumber
-    student = ProfileApiClient::Student.new(
-      schoolId: school.id,
-      id:, username:, name:,
-      createdAt: now, updatedAt: now, discardedAt: nil
-    )
-    allow(ProfileApiClient).to receive(:list_school_students).and_return([student])
+
+    students = student_attributes.map do |student_attrs|
+      ProfileApiClient::Student.new(
+        schoolId: school.id,
+        id: student_attrs[:id],
+        username: student_attrs[:username],
+        name: student_attrs[:name],
+        createdAt: now, updatedAt: now, discardedAt: nil
+      )
+    end
+
+    allow(ProfileApiClient).to receive(:list_school_students).and_return(students)
   end
 
   def stub_profile_api_create_school_student(user_id: SecureRandom.uuid)