Merge pull request #2335 from openstax/restore_deleted_teachers_with_…

…join_url

Allow deleted teachers to be restored using the teacher join URL

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)

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

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