diff --git a/app/routines/add_user_as_course_teacher.rb b/app/routines/add_user_as_course_teacher.rb index 13fa0d92bf..1562573ba2 100644 --- a/app/routines/add_user_as_course_teacher.rb +++ b/app/routines/add_user_as_course_teacher.rb @@ -8,11 +8,19 @@ class AddUserAsCourseTeacher protected def exec(user:, course:) - if run(:is_teacher, user: user, course: course, include_deleted_teachers: true) - .outputs.is_course_teacher - fatal_error(code: :user_is_already_a_course_teacher, - message: 'You are already a teacher of this course.', - offending_inputs: [user, course]) + outs = run(:is_teacher, user: user, course: course, include_deleted_teachers: true).outputs + if outs.is_course_teacher + teachers = outs.teachers + if teachers.any? { |teacher| !teacher.deleted? } + fatal_error(code: :user_is_already_a_course_teacher, + message: 'You are already a teacher of this course.', + offending_inputs: [user, course]) + else + outputs.teacher = teachers.sort_by(&:created_at).last + outputs.role = outputs.teacher.role + outputs.teacher.restore + transfer_errors_from outputs.teacher, { type: :verbatim }, true + end else run(Role::CreateUserRole, user, :teacher) run(CourseMembership::AddTeacher, course: course, role: outputs.role) diff --git a/app/subsystems/course_membership/is_course_teacher.rb b/app/subsystems/course_membership/is_course_teacher.rb index 7bbbe5ea46..18faac5a64 100644 --- a/app/subsystems/course_membership/is_course_teacher.rb +++ b/app/subsystems/course_membership/is_course_teacher.rb @@ -4,11 +4,11 @@ class CourseMembership::IsCourseTeacher protected def exec(course:, roles:, include_deleted_teachers: false) - teachers = course.teachers.where(entity_role_id: roles) - teachers = teachers.without_deleted unless include_deleted_teachers + outputs.teachers = course.teachers.where(entity_role_id: roles) + outputs.teachers = outputs.teachers.without_deleted unless include_deleted_teachers - outputs.teacher = teachers.to_a.find { |teacher| !teacher.deleted? } - outputs.is_deleted = outputs.teacher.nil? && !teachers.empty? + outputs.teacher = outputs.teachers.to_a.find { |teacher| !teacher.deleted? } + outputs.is_deleted = outputs.teacher.nil? && !outputs.teachers.empty? outputs.is_course_teacher = outputs.teacher.present? || outputs.is_deleted end end diff --git a/spec/routines/add_user_as_course_teacher_spec.rb b/spec/routines/add_user_as_course_teacher_spec.rb index 46c9207e21..7b86b97070 100644 --- a/spec/routines/add_user_as_course_teacher_spec.rb +++ b/spec/routines/add_user_as_course_teacher_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe AddUserAsCourseTeacher, type: :routine do - context "when the given user is not a teacher in the given course" do + context 'when the user is not a teacher in the course' do it "returns the user's new teacher role" do user = FactoryBot.create :user_profile course = FactoryBot.create :course_profile_course @@ -11,17 +11,38 @@ expect(result.outputs.role).to_not be_nil end end - context "when the given user is a teacher in the given course" do - it "has errors" do - user = FactoryBot.create :user_profile - course = FactoryBot.create :course_profile_course - result = AddUserAsCourseTeacher.call(user: user, course: course) - expect(result.errors).to be_empty - expect(result.outputs.role).to_not be_nil + context 'when the user is already a teacher in the course' do + context 'when the teacher is not deleted' do + it 'errors' do + user = FactoryBot.create :user_profile + course = FactoryBot.create :course_profile_course - result = AddUserAsCourseTeacher.call(user: user, course: course) - expect(result.errors).to_not be_empty + result = AddUserAsCourseTeacher.call(user: user, course: course) + expect(result.errors).to be_empty + expect(result.outputs.role).to_not be_nil + + result = AddUserAsCourseTeacher.call(user: user, course: course) + expect(result.errors).to_not be_empty + end + end + + context 'when the teacher is deleted' do + it 'restores the teacher' do + user = FactoryBot.create :user_profile + course = FactoryBot.create :course_profile_course + + result = AddUserAsCourseTeacher.call(user: user, course: course) + expect(result.errors).to be_empty + + teacher = result.outputs.teacher + teacher.destroy! + + result = AddUserAsCourseTeacher.call(user: user, course: course) + expect(result.errors).to be_empty + expect(result.outputs.teacher).to eq teacher + expect(teacher.reload).not_to be_deleted + end end end end