fix owners being added to class (#528)

## What's changed?

- Fixed ability to add owners to a class
- Fixed owner appearing with teacher icon when first added to a class

Author: loiswells97

app/controllers/api/class_members_controller.rb

@@ -24,12 +24,14 @@ def index
     def create
       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
+      owners = SchoolOwner::List.call(school: @school).fetch(:school_owners, [])
+      @school_owner_ids = owners.map(&:id)
+      if user_type == 'student'
         teachers = { school_teachers: [] }
         students = SchoolStudent::List.call(school: @school, token: current_user.token, student_ids: user_ids)
+      else
+        teachers = SchoolTeacher::List.call(school: @school, teacher_ids: user_ids)
+        students = { school_students: [] }
       end
       result = ClassMember::Create.call(school_class: @school_class, students: students[:school_students], teachers: teachers[:school_teachers])
 
@@ -42,6 +44,9 @@ def create
     end
 
     def create_batch
+      owners = SchoolOwner::List.call(school: @school).fetch(:school_owners, [])
+      @school_owner_ids = owners.map(&:id)
+
       # 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]) }

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

@@ -15,7 +15,13 @@ if class_member.respond_to?(: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
+  if @school_owner_ids.include?(class_member.teacher_id)
+    json.set! :owner do
+      json.partial! '/api/school_owners/school_owner', owner: class_member.teacher
+    end
+  else
+    json.set! :teacher do
+      json.partial! '/api/school_teachers/school_teacher', teacher: class_member.teacher
+    end
   end
 end

spec/features/class_member/creating_a_batch_of_class_members_spec.rb

@@ -8,13 +8,6 @@
   let(:school) { create(:school) }
   let(:students) { create_list(:student, 3, school:) }
   let(:teacher) { create(:teacher, school:) }
-  let(:another_teacher) { create(:teacher, school:) }
-
-  let(:params) do
-    {
-      class_members: [{ user_id: another_teacher.id, type: 'teacher' }] + students.map { |student| { user_id: student.id, type: 'student' } }
-    }
-  end
 
   context 'with valid params' do
     let(:student_attributes) do
@@ -23,45 +16,105 @@
       end
     end
 
-    before do
-      authenticated_in_hydra_as(teacher)
-      stub_profile_api_list_school_students(school:, student_attributes:)
-      stub_user_info_api_for(another_teacher)
-    end
+    context 'when adding another teacher' do
+      let(:another_teacher) { create(:teacher, school:) }
+      let(:params) do
+        {
+          class_members: [{ user_id: another_teacher.id, type: 'teacher' }] + students.map { |student| { user_id: student.id, type: 'student' } }
+        }
+      end
 
-    it 'responds 201 Created' do
-      post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
-      expect(response).to have_http_status(:created)
-    end
+      before do
+        authenticated_in_hydra_as(teacher)
+        stub_profile_api_list_school_students(school:, student_attributes:)
+        stub_user_info_api_for(another_teacher)
+      end
 
-    it 'responds 201 Created when the user is a school-teacher' do
-      authenticated_in_hydra_as(teacher)
+      it 'responds 201 Created' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        expect(response).to have_http_status(:created)
+      end
 
-      post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
-      expect(response).to have_http_status(:created)
-    end
+      it 'responds 201 Created when the user is a school-teacher' do
+        authenticated_in_hydra_as(teacher)
 
-    it 'responds with the class members JSON array' do
-      post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
-      data = JSON.parse(response.body, symbolize_names: true)
-      expect(data.size).to eq(4)
-    end
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        expect(response).to have_http_status(:created)
+      end
 
-    it 'responds with the class member JSON' do
-      post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
-      data = JSON.parse(response.body, symbolize_names: true)
+      it 'responds with the class members JSON array' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        data = JSON.parse(response.body, symbolize_names: true)
+        expect(data.size).to eq(4)
+      end
+
+      it 'responds with the class member JSON' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        data = JSON.parse(response.body, symbolize_names: true)
+
+        class_member_ids = data.map { |member| member[:student_id] || member[:teacher_id] }
+        expect(class_member_ids).to eq(params[:class_members].pluck(:user_id))
+      end
+
+      it 'responds with the teacher/student JSON' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        data = JSON.parse(response.body, symbolize_names: true)
 
-      class_member_ids = data.map { |member| member[:student_id] || member[:teacher_id] }
-      expect(class_member_ids).to eq(params[:class_members].pluck(:user_id))
+        response_members = data.map { |member| member[:student] || member[:teacher] }
+        teacher_attributes = [{ id: another_teacher.id, name: another_teacher.name, email: another_teacher.email, type: 'teacher' }]
+        expect(response_members).to eq(teacher_attributes + student_attributes)
+      end
     end
 
-    it 'responds with the teacher/student JSON' do
-      post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
-      data = JSON.parse(response.body, symbolize_names: true)
+    context 'when adding an owner as another teacher' do
+      let(:owner_teacher) { create(:teacher, school:, id: create(:owner, school:).id) }
+
+      let(:params) do
+        {
+          class_members: [{ user_id: owner_teacher.id, type: 'owner' }] + students.map { |student| { user_id: student.id, type: 'student' } }
+        }
+      end
+
+      before do
+        authenticated_in_hydra_as(teacher)
+        stub_profile_api_list_school_students(school:, student_attributes:)
+        stub_user_info_api_for(owner_teacher)
+      end
+
+      it 'responds 201 Created' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        expect(response).to have_http_status(:created)
+      end
+
+      it 'responds 201 Created when the user is a school-teacher' do
+        authenticated_in_hydra_as(teacher)
+
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        expect(response).to have_http_status(:created)
+      end
+
+      it 'responds with the class members JSON array' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        data = JSON.parse(response.body, symbolize_names: true)
+        expect(data.size).to eq(4)
+      end
+
+      it 'responds with the class member JSON' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        data = JSON.parse(response.body, symbolize_names: true)
+
+        class_member_ids = data.map { |member| member[:student_id] || member[:teacher_id] }
+        expect(class_member_ids).to eq(params[:class_members].pluck(:user_id))
+      end
+
+      it 'responds with the teacher/student JSON' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", headers:, params:)
+        data = JSON.parse(response.body, symbolize_names: true)
 
-      response_members = data.map { |member| member[:student] || member[:teacher] }
-      teacher_attributes = [{ id: another_teacher.id, name: another_teacher.name, email: another_teacher.email, type: 'teacher' }]
-      expect(response_members).to eq(teacher_attributes + student_attributes)
+        response_members = data.map { |member| member[:student] || member[:teacher] || member[:owner] }
+        teacher_attributes = [{ id: owner_teacher.id, name: owner_teacher.name, email: owner_teacher.email, type: 'owner' }]
+        expect(response_members).to eq(teacher_attributes + student_attributes)
+      end
     end
   end
 
@@ -95,13 +148,23 @@
 
       expect(data[:error]).to match(/No valid school members provided/)
     end
+  end
+
+  context 'when the user is not authorized' do
+    let(:another_teacher) { create(:teacher, school:) }
+    let(:params) do
+      {
+        class_members: [{ user_id: another_teacher.id, type: 'teacher' }] + students.map { |student| { user_id: student.id, type: 'student' } }
+      }
+    end
 
     it 'responds 401 Unauthorized when no token is given' do
       post("/api/schools/#{school.id}/classes/#{school_class.id}/members/batch", params:)
       expect(response).to have_http_status(:unauthorized)
     end
 
-    it 'responds 403 Forbidden when the user is a school-owner for a different school' do
+    it 'responds 403 Forbidden when the user is a teacher from a different school' do
+      authenticated_in_hydra_as(teacher)
       school = create(:school, id: SecureRandom.uuid)
       school_class.update!(school_id: school.id)
 

spec/features/class_member/creating_a_class_member_spec.rb

@@ -8,16 +8,6 @@
   let(:school) { create(:school) }
   let(:student) { create(:student, school:, name: 'School Student', username: 'school-student') }
   let(:teacher) { create(:teacher, school:) }
-  let(:another_teacher) { create(:teacher, school:) }
-
-  let(:student_params) do
-    {
-      class_member: {
-        user_id: student.id,
-        type: 'student'
-      }
-    }
-  end
 
   before do
     owner = create(:owner, school:)
@@ -27,6 +17,14 @@
 
   context 'with valid params' do
     context 'when new class member is a student' do
+      let(:student_params) do
+        {
+          class_member: {
+            user_id: student.id,
+            type: 'student'
+          }
+        }
+      end
       let(:student_attributes) { { id: student.id, name: student.name, username: student.username, type: 'student' } }
 
       before do
@@ -63,6 +61,7 @@
     end
 
     context 'when new class member is a teacher' do
+      let(:another_teacher) { create(:teacher, school:) }
       let(:teacher_params) do
         {
           class_member: {
@@ -104,13 +103,59 @@
         expect(response_teacher).to eq(teacher_attributes)
       end
     end
+
+    context 'when new class member is an owner' do
+      let(:owner) { create(:owner, school:) }
+      let(:owner_teacher) { create(:teacher, school:, id: owner.id, name: owner.name, email: owner.email) }
+
+      let(:owner_params) do
+        {
+          class_member: {
+            user_id: owner.id,
+            type: 'owner'
+          }
+        }
+      end
+
+      before do
+        stub_user_info_api_for(owner_teacher)
+      end
+
+      it 'responds 201 Created' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: owner_params)
+        expect(response).to have_http_status(:created)
+      end
+
+      it 'responds 201 Created when the user is a school-teacher' do
+        authenticated_in_hydra_as(teacher)
+
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: owner_params)
+        expect(response).to have_http_status(:created)
+      end
+
+      it 'responds with the class member JSON' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: owner_params)
+        data = JSON.parse(response.body, symbolize_names: true)
+
+        expect(data[:class_member][:teacher_id]).to eq(owner.id)
+      end
+
+      it 'responds with the teacher JSON' do
+        post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: owner_params)
+        data = JSON.parse(response.body, symbolize_names: true)
+
+        response_teacher = data[:class_member][:owner]
+        owner_attributes = { id: owner.id, name: owner.name, email: owner.email, type: 'owner' }
+        expect(response_teacher).to eq(owner_attributes)
+      end
+    end
   end
 
   context 'with invalid params' do
     let(:invalid_params) { { class_member: { user_id: SecureRandom.uuid } } }
 
     before do
-      stub_profile_api_list_school_students(school:, student_attributes: [])
+      stub_user_info_api_for_unknown_users(user_id: invalid_params[:class_member][:user_id])
     end
 
     it 'responds 400 Bad Request when params are missing' do
@@ -129,34 +174,45 @@
 
       expect(data[:error]).to match(/No valid school members provided/)
     end
+  end
+
+  context 'when the user is not authorized' do
+    let(:student_params) do
+      {
+        class_member: {
+          user_id: student.id,
+          type: 'student'
+        }
+      }
+    end
 
     it 'responds 401 Unauthorized when no token is given' do
       post("/api/schools/#{school.id}/classes/#{school_class.id}/members", params: student_params)
       expect(response).to have_http_status(:unauthorized)
     end
-  end
 
-  it 'responds 403 Forbidden when the user is a school-owner for a different school' do
-    school = create(:school, id: SecureRandom.uuid)
-    school_class.update!(school_id: school.id)
+    it 'responds 403 Forbidden when the user is a school-owner for a different school' do
+      school = create(:school, id: SecureRandom.uuid)
+      school_class.update!(school_id: school.id)
 
-    post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: student_params)
-    expect(response).to have_http_status(:forbidden)
-  end
+      post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: student_params)
+      expect(response).to have_http_status(:forbidden)
+    end
 
-  it 'responds 403 Forbidden when the user is not the school-teacher for the class' do
-    teacher = create(:teacher, school:)
-    authenticated_in_hydra_as(teacher)
+    it 'responds 403 Forbidden when the user is not the school-teacher for the class' do
+      teacher = create(:teacher, school:)
+      authenticated_in_hydra_as(teacher)
 
-    post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: student_params)
-    expect(response).to have_http_status(:forbidden)
-  end
+      post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: student_params)
+      expect(response).to have_http_status(:forbidden)
+    end
 
-  it 'responds 403 Forbidden when the user is a school-student' do
-    student = create(:student, school:)
-    authenticated_in_hydra_as(student)
+    it 'responds 403 Forbidden when the user is a school-student' do
+      student = create(:student, school:)
+      authenticated_in_hydra_as(student)
 
-    post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: student_params)
-    expect(response).to have_http_status(:forbidden)
+      post("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:, params: student_params)
+      expect(response).to have_http_status(:forbidden)
+    end
   end
 end