feat: let teachers and owners use class join links

Teachers and owners of the class's school can now use the join link:

- A teacher already in the class is treated as already_member and
  redirected into the class.
- A teacher of the school not yet in the class gets joinable_as_teacher;
  POST /api/join adds them as a ClassTeacher, then redirects.
- An owner of the school gets the owner status and is redirected into
  the class without being added to it.

Refactors compute_action_status into existing_user_join_status /
new_user_join_status, split on whether the user already has a role in
the school, and renames add_user_to_school_and_class to
add_student_to_school_and_class.

Author: fspeirs

app/controllers/api/join_controller.rb

@@ -14,10 +14,13 @@ def create
       case action_status
       when :wrong_school, :domain_mismatch, :not_a_student
         render json: { error: action_status.to_s }, status: :forbidden
-      when :already_member
+      when :already_member, :owner
+        render json: { redirect_url: class_redirect_path }, status: :ok
+      when :joinable_as_teacher
+        add_user_to_class_as_teacher
         render json: { redirect_url: class_redirect_path }, status: :ok
       else
-        add_user_to_school_and_class
+        add_student_to_school_and_class
         render json: { redirect_url: class_redirect_path }, status: :ok
       end
     end
@@ -41,7 +44,21 @@ def action_status
 
     def compute_action_status
       return :already_member if user_is_member_of_class?
-      return :joinable if user_is_student_of_school?
+      return existing_user_join_status if user_has_role_in_school?
+
+      new_user_join_status
+    end
+
+    # The user already has a role in this school: which one decides the status.
+    def existing_user_join_status
+      return :owner if user_is_owner_of_school?
+      return :joinable_as_teacher if user_is_teacher_of_school?
+
+      :joinable # student is the only remaining role for this school
+    end
+
+    # The user has no role in this school yet: may they join as a new student?
+    def new_user_join_status
       return :not_a_student if user_has_non_student_role?
       return :wrong_school if user_in_different_school?
       return :domain_mismatch unless @school.valid_email?(current_user.email)
@@ -54,11 +71,20 @@ def class_redirect_path
     end
 
     def user_is_member_of_class?
-      ClassStudent.exists?(school_class: @school_class, student_id: current_user.id)
+      ClassStudent.exists?(school_class: @school_class, student_id: current_user.id) ||
+        ClassTeacher.exists?(school_class: @school_class, teacher_id: current_user.id)
     end
 
-    def user_is_student_of_school?
-      Role.exists?(school: @school, user_id: current_user.id, role: Role.roles[:student])
+    def user_is_owner_of_school?
+      Role.exists?(school: @school, user_id: current_user.id, role: Role.roles[:owner])
+    end
+
+    def user_is_teacher_of_school?
+      Role.exists?(school: @school, user_id: current_user.id, role: Role.roles[:teacher])
+    end
+
+    def user_has_role_in_school?
+      Role.exists?(school: @school, user_id: current_user.id)
     end
 
     def user_has_non_student_role?
@@ -69,9 +95,15 @@ def user_in_different_school?
       Role.where(user_id: current_user.id).where.not(school_id: @school.id).exists?
     end
 
-    def add_user_to_school_and_class
+    def add_student_to_school_and_class
       Role.create!(school: @school, user_id: current_user.id, role: :student) unless Role.exists?(school: @school, user_id: current_user.id)
       ClassStudent.create!(school_class: @school_class, student_id: current_user.id)
     end
+
+    def add_user_to_class_as_teacher
+      class_teacher = @school_class.teachers.build(teacher_id: current_user.id)
+      class_teacher.teacher = current_user
+      class_teacher.save!
+    end
   end
 end

spec/requests/join_controller_spec.rb

@@ -70,22 +70,32 @@
         expect(data[:status]).to eq('wrong_school')
       end
 
-      it 'returns status: not_a_student when the user is a teacher of this school' do
+      it 'returns status: joinable_as_teacher when the user is a teacher of this school not yet in the class' do
         create(:teacher_role, school:, user_id: student.id)
 
         get "/api/join/#{school_class.join_code}", headers: headers
 
         data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:status]).to eq('not_a_student')
+        expect(data[:status]).to eq('joinable_as_teacher')
+      end
+
+      it 'returns status: already_member when the user is already a teacher in the class' do
+        create(:teacher_role, school:, user_id: student.id)
+        ClassTeacher.create!(school_class:, teacher_id: student.id)
+
+        get "/api/join/#{school_class.join_code}", headers: headers
+
+        data = JSON.parse(response.body, symbolize_names: true)
+        expect(data[:status]).to eq('already_member')
       end
 
-      it 'returns status: not_a_student when the user is an owner of this school' do
+      it 'returns status: owner when the user is an owner of this school' do
         create(:owner_role, school:, user_id: student.id)
 
         get "/api/join/#{school_class.join_code}", headers: headers
 
         data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:status]).to eq('not_a_student')
+        expect(data[:status]).to eq('owner')
       end
 
       it 'returns status: not_a_student for a teacher of a different school (not wrong_school)' do
@@ -204,26 +214,47 @@
         end
       end
 
-      it 'responds with 403 not_a_student when the user is a teacher of the school' do
+      it 'adds the user to the class as a teacher and returns a redirect URL' do
         create(:teacher_role, school:, user_id: student.id)
+        school_class # force creation before the request
 
         expect do
           post "/api/join/#{school_class.join_code}", headers: headers
-        end.not_to change(ClassStudent, :count)
+        end.to change(ClassTeacher, :count).by(1)
 
-        expect(response).to have_http_status(:forbidden)
+        expect(response).to have_http_status(:ok)
         data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:error]).to eq('not_a_student')
+        expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
+        expect(ClassTeacher.exists?(school_class:, teacher_id: student.id)).to be(true)
+        expect(ClassStudent.exists?(school_class:, student_id: student.id)).to be(false)
+        expect(Role.where(user_id: student.id, school:).pluck(:role)).to eq(['teacher'])
       end
 
-      it 'responds with 403 not_a_student when the user is an owner of the school' do
+      it 'is idempotent when the user is already a teacher in the class' do
+        create(:teacher_role, school:, user_id: student.id)
+        ClassTeacher.create!(school_class:, teacher_id: student.id)
+
+        expect do
+          post "/api/join/#{school_class.join_code}", headers: headers
+        end.not_to change(ClassTeacher, :count)
+
+        expect(response).to have_http_status(:ok)
+        data = JSON.parse(response.body, symbolize_names: true)
+        expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
+      end
+
+      it 'redirects an owner into the class without adding them to it' do
         create(:owner_role, school:, user_id: student.id)
 
-        post "/api/join/#{school_class.join_code}", headers: headers
+        expect do
+          post "/api/join/#{school_class.join_code}", headers: headers
+        end.not_to change(ClassStudent, :count)
 
-        expect(response).to have_http_status(:forbidden)
+        expect(response).to have_http_status(:ok)
         data = JSON.parse(response.body, symbolize_names: true)
-        expect(data[:error]).to eq('not_a_student')
+        expect(data[:redirect_url]).to eq("/school/#{school.code}/class/#{school_class.code}")
+        expect(ClassTeacher.exists?(school_class:, teacher_id: student.id)).to be(false)
+        expect(Role.where(user_id: student.id, school:).pluck(:role)).to eq(['owner'])
       end
 
       it 'responds with 404 when the join code does not exist' do