Allow class members create route to take teachers as well as students (#516)

## What's Changed?

- Updated class member `create` and `create_batch` routes to accept both
students and teachers
- `ClassMembers::Create` operation now deals with both students and
teachers

closes #464

---------

Co-authored-by: Jamie Benstead <[email protected]>

Author: loiswells97

app/controllers/api/class_members_controller.rb

@@ -20,9 +20,16 @@ def index
     end
 
     def create
-      student_ids = [class_member_params[:student_id]]
-      students = SchoolStudent::List.call(school: @school, token: current_user.token, student_ids:)
-      result = ClassMember::Create.call(school_class: @school_class, students: students[:school_students])
+      user_ids = [class_member_params[:user_id]]
+      user_type = class_member_params[:type]
+      if user_type == 'teacher'
+        teachers = SchoolTeacher::List.call(school: @school, teacher_ids: user_ids)
+        students = { school_students: [] }
+      else
+        teachers = { school_teachers: [] }
+        students = SchoolStudent::List.call(school: @school, token: current_user.token, student_ids: user_ids)
+      end
+      result = ClassMember::Create.call(school_class: @school_class, students: students[:school_students], teachers: teachers[:school_teachers])
 
       if result.success?
         @class_member = result[:class_members].first
@@ -33,8 +40,16 @@ def create
     end
 
     def create_batch
-      students = SchoolStudent::List.call(school: @school, token: current_user.token, student_ids: create_batch_params)
-      result = ClassMember::Create.call(school_class: @school_class, students: students[:school_students])
+      # Teacher objects needs to be the compliment of student objects so that every user creation is attempted and validated.
+      student_objects = create_batch_params.select { |user| user[:type] == 'student' }
+      teacher_objects = create_batch_params.select { |user| student_objects.pluck(:user_id).exclude?(user[:user_id]) }
+      student_ids = student_objects.pluck(:user_id)
+      teacher_ids = teacher_objects.pluck(:user_id)
+
+      students = list_students(@school, current_user.token, student_ids)
+      teachers = list_teachers(@school, teacher_ids)
+
+      result = ClassMember::Create.call(school_class: @school_class, students: students[:school_students], teachers: teachers[:school_teachers])
 
       if result.success?
         @class_members = result[:class_members]
@@ -57,11 +72,33 @@ def destroy
     private
 
     def class_member_params
-      params.require(:class_member).permit(:student_id)
+      params.require(:class_member).permit(:user_id, :type)
     end
 
     def create_batch_params
-      params.permit(student_ids: [])
+      class_members = params.require(:class_members)
+
+      class_members.map do |class_member|
+        next if class_member.blank?
+
+        class_member.permit(:user_id, :type).to_h.with_indifferent_access
+      end
+    end
+
+    def list_students(school, _token, student_ids)
+      if student_ids.present?
+        SchoolStudent::List.call(school:, token: current_user.token, student_ids:)
+      else
+        { school_students: [] }
+      end
+    end
+
+    def list_teachers(school, teacher_ids)
+      if teacher_ids.present?
+        SchoolTeacher::List.call(school:, teacher_ids:)
+      else
+        { school_teachers: [] }
+      end
     end
   end
 end

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

@@ -4,13 +4,18 @@ json.call(
   class_member,
   :id,
   :school_class_id,
-  :student_id,
   :created_at,
   :updated_at
 )
 
-if class_member.student.present?
+if class_member.respond_to?(:student)
+  json.student_id(class_member.student_id)
   json.set! :student do
     json.partial! '/api/school_students/school_student', student: class_member.student
   end
+elsif class_member.respond_to?(:teacher)
+  json.teacher_id(class_member.teacher_id)
+  json.set! :teacher do
+    json.partial! '/api/school_teachers/school_teacher', teacher: class_member.teacher
+  end
 end

lib/concepts/class_member/operations/create.rb

@@ -3,13 +3,14 @@
 module ClassMember
   class Create
     class << self
-      def call(school_class:, students:)
+      def call(school_class:, students: [], teachers: [])
         response = OperationResponse.new
         response[:class_members] = []
         response[:errors] = {}
-        raise ArgumentError, 'No valid students provided' if students.blank?
+        raise ArgumentError, 'No valid school members provided' if students.blank? && teachers.blank?
 
-        create_class_members(school_class:, students:, response:)
+        create_class_teachers(school_class:, teachers:, response:)
+        create_class_students(school_class:, students:, response:)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -19,21 +20,39 @@ def call(school_class:, students:)
 
       private
 
-      def create_class_members(school_class:, students:, response:)
+      def create_class_teachers(school_class:, teachers:, response:)
+        teachers.each do |teacher|
+          class_teacher = school_class.teachers.build({ teacher_id: teacher.id })
+          class_teacher.teacher = teacher
+          class_teacher.save!
+          response[:class_members] << class_teacher
+        rescue StandardError => e
+          handle_class_teacher_error(e, class_teacher, teacher, response)
+          response
+        end
+      end
+
+      def create_class_students(school_class:, students:, response:)
         students.each do |student|
           class_student = school_class.students.build({ student_id: student.id })
           class_student.student = student
           class_student.save!
           response[:class_members] << class_student
         rescue StandardError => e
-          handle_class_member_error(e, class_student, student, response)
+          handle_class_student_error(e, class_student, student, response)
           response
         end
       end
 
-      def handle_class_member_error(exception, class_member, student, response)
+      def handle_class_teacher_error(exception, class_teacher, teacher, response)
+        Sentry.capture_exception(exception)
+        errors = class_teacher.errors.full_messages.join(',')
+        response[:errors][teacher.id] = "Error creating class member for teacher_id #{teacher.id}: #{errors}"
+      end
+
+      def handle_class_student_error(exception, class_student, student, response)
         Sentry.capture_exception(exception)
-        errors = class_member.errors.full_messages.join(',')
+        errors = class_student.errors.full_messages.join(',')
         response[:errors][student.id] = "Error creating class member for student_id #{student.id}: #{errors}"
       end
     end

spec/concepts/class_member/create_spec.rb

@@ -7,26 +7,38 @@
   let(:school) { create(:school) }
   let(:students) { create_list(:student, 3, school:) }
   let(:teacher) { create(:teacher, school:) }
+  let(:teachers) { [create(:teacher, school:), create(:teacher, school:)] }
 
   let(:student_ids) { students.map(&:id) }
 
   it 'returns a successful operation response' do
-    response = described_class.call(school_class:, students:)
+    response = described_class.call(school_class:, students:, teachers:)
     expect(response.success?).to be(true)
   end
 
   it 'creates class students' do
-    expect { described_class.call(school_class:, students:) }.to change(ClassStudent, :count).by(3)
+    expect { described_class.call(school_class:, students:, teachers:) }.to change(ClassStudent, :count).by(3)
+  end
+
+  it 'creates class teachers' do
+    expect { described_class.call(school_class:, students:, teachers:) }.to change(ClassTeacher, :count).by(2)
   end
 
   it 'returns a class members JSON array' do
-    response = described_class.call(school_class:, students:)
-    expect(response[:class_members].size).to eq(3)
+    response = described_class.call(school_class:, students:, teachers:)
+    expect(response[:class_members].size).to eq(5)
   end
 
   it 'returns class students in the operation response' do
-    response = described_class.call(school_class:, students:)
-    expect(response[:class_members]).to all(be_a(ClassStudent))
+    response = described_class.call(school_class:, students:, teachers:)
+    class_students_count = response[:class_members].count { |member| member.is_a?(ClassStudent) }
+    expect(class_students_count).to eq(3)
+  end
+
+  it 'returns class teachers in the operation response' do
+    response = described_class.call(school_class:, students:, teachers:)
+    class_teachers_count = response[:class_members].count { |member| member.is_a?(ClassTeacher) }
+    expect(class_teachers_count).to eq(2)
   end
 
   it 'assigns the school_class' do
@@ -35,41 +47,53 @@
   end
 
   it 'assigns the student_id' do
-    response = described_class.call(school_class:, students:)
-    expect(response[:class_members].map(&:student_id)).to match_array(student_ids)
+    response = described_class.call(school_class:, students:, teachers:)
+    response_students = response[:class_members].select { |member| member.is_a?(ClassStudent) }
+    expect(response_students.map(&:student_id)).to match_array(student_ids)
+  end
+
+  it 'assigns the teacher_id' do
+    teacher_ids = teachers.map(&:id)
+    response = described_class.call(school_class:, students:, teachers:)
+    response_teachers = response[:class_members].select { |member| member.is_a?(ClassTeacher) }
+    expect(response_teachers.map(&:teacher_id)).to match_array(teacher_ids)
   end
 
   context 'when creations fail' do
     before do
       allow(Sentry).to receive(:capture_exception)
     end
 
-    context 'with malformed students' do
+    context 'with malformed members' do
       let(:students) { nil }
+      let(:teachers) { nil }
+
+      it 'does not create a class student' do
+        expect { described_class.call(school_class:, students:, teachers:) }.not_to change(ClassStudent, :count)
+      end
 
-      it 'does not create a class member' do
-        expect { described_class.call(school_class:, students:) }.not_to change(ClassStudent, :count)
+      it 'does not create a class teacher' do
+        expect { described_class.call(school_class:, students:, teachers:) }.not_to change(ClassTeacher, :count)
       end
 
       it 'returns a failed operation response' do
-        response = described_class.call(school_class:, students:)
+        response = described_class.call(school_class:, students:, teachers:)
         expect(response.failure?).to be(true)
       end
 
       it 'returns the error message in the operation response' do
-        response = described_class.call(school_class:, students:)
-        expect(response[:error]).to match(/No valid students provided/)
+        response = described_class.call(school_class:, students:, teachers:)
+        expect(response[:error]).to match(/No valid school members provided/)
       end
 
       it 'sent the exception to Sentry' do
-        described_class.call(school_class:, students:)
+        described_class.call(school_class:, students:, teachers:)
         expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
       end
     end
 
     context 'with a student from a different school' do
-      let(:different_school) { create(:school) }
-      let(:different_school_student) { create(:student, school: different_school) }
+      let(:different_school_student) { create(:student, school: create(:school)) }
 
       context 'with non existent students' do
         let(:students) { [different_school_student] }
@@ -103,37 +127,52 @@
         let(:new_students) { students + [different_school_student] }
 
         it 'returns a successful operation response' do
-          response = described_class.call(school_class:, students: new_students)
+          response = described_class.call(school_class:, students: new_students, teachers:)
           expect(response.success?).to be(true)
         end
 
         it 'returns a class members JSON array' do
-          response = described_class.call(school_class:, students: new_students)
-          expect(response[:class_members].size).to eq(3)
+          response = described_class.call(school_class:, students: new_students, teachers:)
+          expect(response[:class_members].size).to eq(5)
         end
 
-        it 'returns class members in the operation response' do
-          response = described_class.call(school_class:, students: new_students)
-          expect(response[:class_members]).to all(be_a(ClassStudent))
+        it 'returns class students in the operation response' do
+          response = described_class.call(school_class:, students: new_students, teachers:)
+          class_students_count = response[:class_members].count { |member| member.is_a?(ClassStudent) }
+          expect(class_students_count).to eq(3)
+        end
+
+        it 'returns class teachers in the operation response' do
+          response = described_class.call(school_class:, students: new_students, teachers:)
+          class_teachers_count = response[:class_members].count { |member| member.is_a?(ClassTeacher) }
+          expect(class_teachers_count).to eq(2)
         end
 
         it 'assigns the school_class' do
-          response = described_class.call(school_class:, students: new_students)
+          response = described_class.call(school_class:, students: new_students, teachers:)
           expect(response[:class_members]).to all(have_attributes(school_class:))
         end
 
         it 'assigns the successful students' do
-          response = described_class.call(school_class:, students: new_students)
-          expect(response[:class_members].map(&:student_id)).to match_array(student_ids)
+          response = described_class.call(school_class:, students: new_students, teachers:)
+          response_students = response[:class_members].select { |member| member.is_a?(ClassStudent) }
+          expect(response_students.map(&:student_id)).to match_array(student_ids)
+        end
+
+        it 'assigns the successful teachers' do
+          teacher_ids = teachers.map(&:id)
+          response = described_class.call(school_class:, students: new_students, teachers:)
+          response_teachers = response[:class_members].select { |member| member.is_a?(ClassTeacher) }
+          expect(response_teachers.map(&:teacher_id)).to match_array(teacher_ids)
         end
 
         it 'returns the error messages in the operation response' do
-          response = described_class.call(school_class:, students: new_students)
+          response = described_class.call(school_class:, students: new_students, teachers:)
           expect(response[:errors][different_school_student.id]).to eq("Error creating class member for student_id #{different_school_student.id}: Student '#{different_school_student.id}' does not have the 'school-student' role for organisation '#{school.id}'")
         end
 
         it 'sent the exception to Sentry' do
-          described_class.call(school_class:, students: new_students)
+          described_class.call(school_class:, students: new_students, teachers:)
           expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
         end
       end