Add endpoint for mass user deletion.

This code:

1. Modifies StudentRemovalService to mass-delete users and their
   projects instead of skipping users who have projects.
2. Adds an endpoint for school owners to mass-delete students.

Author: fspeirs

app/controllers/api/school_students_controller.rb

@@ -119,6 +119,40 @@ def destroy
       end
     end
 
+    def destroy_batch
+      student_ids = student_ids_params
+
+      if student_ids.blank? || student_ids.empty?
+        render json: {
+                 error: 'No student IDs provided',
+                 error_type: :unprocessable_entity
+               },
+               status: :unprocessable_entity
+        return
+      end
+
+      # Invoke StudentRemovalService
+      service = StudentRemovalService.new(
+        students: student_ids,
+        school: @school,
+        remove_from_profile: false,
+        token: current_user.token
+      )
+
+      results = service.remove_students
+
+      # Check if any errors occurred
+      errors = results.select { |r| r[:error] }
+      if errors.any?
+        render json: {
+          results: results,
+          error: "#{errors.size} student(s) failed to be removed"
+        }, status: :partial_content
+      else
+        render json: { results: results }, status: :ok
+      end
+    end
+
     private
 
     def school_student_params
@@ -135,6 +169,10 @@ def school_students_params
       end
     end
 
+    def student_ids_params
+      params.fetch(:student_ids, [])
+    end
+
     def create_safeguarding_flags
       create_teacher_safeguarding_flag
       create_owner_safeguarding_flag

app/models/ability.rb

@@ -66,7 +66,7 @@ def define_school_owner_abilities(school:)
     can(%i[read create create_batch destroy], ClassStudent, school_class: { school: { id: school.id } })
     can(%i[read create destroy], :school_owner)
     can(%i[read create destroy], :school_teacher)
-    can(%i[read create create_batch update destroy], :school_student)
+    can(%i[read create create_batch update destroy destroy_batch], :school_student)
     can(%i[create create_copy], Lesson, school_id: school.id)
     can(%i[read update destroy], Lesson, school_id: school.id, visibility: %w[teachers students public])
     can(%i[exchange_code], :google_auth)

app/services/student_removal_service.rb

@@ -14,24 +14,22 @@ def remove_students
     @students.each do |user_id|
       result = { user_id: }
       begin
-        # Skip if student has projects
-        projects = Project.where(user_id: user_id)
-        result[:skipped] = true if projects.length.positive?
+        ActiveRecord::Base.transaction do
+          # Delete all projects
+          projects = Project.where(user_id: user_id)
+          projects.destroy_all
 
-        unless result[:skipped]
-          ActiveRecord::Base.transaction do
-            # Remove from classes
-            class_assignments = ClassStudent.where(student_id: user_id)
-            class_assignments.destroy_all
+          # Remove from classes
+          class_assignments = ClassStudent.where(student_id: user_id)
+          class_assignments.destroy_all
 
-            # Remove roles
-            roles = Role.student.where(user_id: user_id)
-            roles.destroy_all
-          end
-
-          # Remove from profile if requested
-          ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id) if @remove_from_profile && @token.present?
+          # Remove roles
+          roles = Role.student.where(user_id: user_id)
+          roles.destroy_all
         end
+
+        # Remove from profile if requested
+        ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id) if @remove_from_profile && @token.present?
       rescue StandardError => e
         result[:error] = "#{e.class}: #{e.message}"
       end

config/routes.rb

@@ -69,6 +69,7 @@
       resources :teachers, only: %i[index create], controller: 'school_teachers'
       resources :students, only: %i[index create update destroy], controller: 'school_students' do
         post :batch, on: :collection, to: 'school_students#create_batch'
+        delete :batch, on: :collection, to: 'school_students#destroy_batch'
       end
     end
 

lib/tasks/remove_students.rake

@@ -39,6 +39,7 @@ namespace :remove_students do
     puts "REMOVE_FROM_PROFILE: #{remove_from_profile}"
     puts "Students to remove: #{students.size}"
     puts "====================\n\n"
+    puts "WARNING: All student projects will be deleted permanently and this operation is not reversible.\n"
     puts "Please confirm deletion of #{students.size} user(s), and that recent Postgres backups have been captured for all services affected (https://devcenter.heroku.com/articles/heroku-postgres-backups#manual-backups)"
     print 'Are you sure you want to continue? (yes/no): '
     confirmation = $stdin.gets.strip.downcase

spec/features/school_student/batch_deleting_school_students_spec.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Batch deleting school students', type: :request do
+  let(:school) { create(:school) }
+  let(:owner) { create(:owner, school: school) }
+  let(:teacher) { create(:teacher, school: school) }
+  let(:school_class) { create(:school_class, school: school, teacher_ids: [teacher.id]) }
+  let(:student_1) { create(:student, school: school) }
+  let(:student_2) { create(:student, school: school) }
+  let(:headers) { { Authorization: UserProfileMock::TOKEN } }
+
+  before do
+    authenticated_in_hydra_as(owner)
+    stub_profile_api_create_safeguarding_flag
+    create(:class_student, student_id: student_1.id, school_class: school_class)
+    create(:class_student, student_id: student_2.id, school_class: school_class)
+  end
+
+  describe 'DELETE /api/schools/:school_id/students/batch' do
+    it 'deletes all students and their projects' do
+      project_1 = create(:project, user_id: student_1.id)
+      project_2 = create(:project, user_id: student_2.id)
+
+      expect(ClassStudent.where(student_id: student_1.id)).to exist
+      expect(ClassStudent.where(student_id: student_2.id)).to exist
+      expect(Project.where(id: project_1.id)).to exist
+      expect(Project.where(id: project_2.id)).to exist
+
+      delete "/api/schools/#{school.id}/students/batch",
+             params: { student_ids: [student_1.id, student_2.id] },
+             headers: headers
+
+      expect(response).to have_http_status(:ok)
+
+      json = JSON.parse(response.body)
+      expect(json['results'].size).to eq(2)
+      expect(json['results'].all? { |r| r['error'].nil? }).to be true
+
+      # Verify students removed from classes
+      expect(ClassStudent.where(student_id: student_1.id)).not_to exist
+      expect(ClassStudent.where(student_id: student_2.id)).not_to exist
+
+      # Verify projects deleted
+      expect(Project.where(id: project_1.id)).not_to exist
+      expect(Project.where(id: project_2.id)).not_to exist
+    end
+
+    it 'returns error when no student IDs provided' do
+      delete "/api/schools/#{school.id}/students/batch",
+             headers: headers
+
+      expect(response).to have_http_status(:unprocessable_entity)
+      json = JSON.parse(response.body)
+      expect(json['error']).to eq('No student IDs provided')
+    end
+  end
+end

spec/services/student_removal_service_spec.rb

@@ -11,25 +11,24 @@
   let(:service) { described_class.new(students: [student.id], school: school) }
 
   before do
-    allow(Project).to receive(:where).and_return([])
     allow(ProfileApiClient).to receive(:delete_school_student)
     create(:class_student, student_id: student.id, school_class: school_class)
   end
 
-  it 'removes student from classes and roles' do
+  it 'removes student from classes, roles, and deletes all projects' do
+    create(:project, user_id: student.id)
+
     expect(Role.where(user_id: student.id, role: :student)).to exist
     expect(ClassStudent.where(student_id: student.id)).to exist
+    expect(Project.where(user_id: student.id)).to exist
+
     results = service.remove_students
+
     expect(results.first[:user_id]).to eq(student.id)
-    expect(results.first[:skipped]).to be_nil
+    expect(results.first[:error]).to be_nil
     expect(ClassStudent.where(student_id: student.id)).not_to exist
     expect(Role.where(user_id: student.id, role: :student)).not_to exist
-  end
-
-  it 'skips removal if student has projects' do
-    allow(Project).to receive(:where).and_return([instance_double(Project)])
-    results = service.remove_students
-    expect(results.first[:skipped]).to be true
+    expect(Project.where(user_id: student.id)).not_to exist
   end
 
   it 'calls ProfileApiClient if remove_from_profile is true and token is present' do