Merge branch 'main' into 1069-add-immediate-onboarding

Author: danhalson

app/controllers/api/lessons_controller.rb

@@ -7,12 +7,11 @@ class LessonsController < ApiController
     load_and_authorize_resource :lesson
 
     def index
-      archive_scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived
-      scope = params[:school_class_id] ? archive_scope.where(school_class_id: params[:school_class_id]) : archive_scope
-      ordered_scope = scope.order(created_at: :asc)
-
-      accessible_lessons = ordered_scope.accessible_by(current_ability).includes(project: :remixes)
+      accessible_lessons = filtered_lessons_scope
+                           .accessible_by(current_ability)
+                           .includes(project: :remixes)
       @lessons_with_users = accessible_lessons.with_users
+
       if current_user&.school_teacher?(school) || current_user&.school_owner?(school)
         render :teacher_index, formats: [:json], status: :ok
       else
@@ -75,6 +74,13 @@ def destroy
 
     private
 
+    def filtered_lessons_scope
+      archive_scope = params[:include_archived] == 'true' ? Lesson : Lesson.unarchived
+      scope = params[:school_class_id] ? archive_scope.where(school_class_id: params[:school_class_id]) : archive_scope
+      scope = scope.joins(:project).where(projects: { identifier: params[:project_identifier] }) if params[:project_identifier].present?
+      scope.order(created_at: :asc)
+    end
+
     def verify_school_class_belongs_to_school
       return if base_params[:school_class_id].blank?
       return if school&.classes&.pluck(:id)&.include?(base_params[:school_class_id])

app/controllers/api/school_students_controller.rb

@@ -110,17 +110,62 @@ def update
     end
 
     def destroy
-      result = SchoolStudent::Delete.call(school: @school, student_id: params[:id], token: current_user.token)
+      remove_students([params[:id]])
+    end
 
-      if result.success?
-        head :no_content
-      else
-        render json: { error: result[:error] }, status: :unprocessable_entity
+    def destroy_batch
+      # DELETE /api/schools/:school_id/students/batch
+      # Params: { student_ids: ["uuid1", "uuid2", ...] }
+      #
+      # Returns 200 OK with one of:
+      # - Success: { results: [{ user_id: "..." }, ...] }
+      # - Partial failure: { results: [...], error: "N student(s) failed to be removed" }
+      #
+      # Each result may contain:
+      # - { user_id:, error: } - deletion failed
+      # - { user_id:, skipped:, reason: } - student skipped (e.g., not in this school)
+      # - { user_id: } - deletion succeeded
+
+      student_ids = student_ids_params
+
+      if student_ids.blank?
+        render json: {
+                 error: 'No student IDs provided',
+                 error_type: :unprocessable_entity
+               },
+               status: :unprocessable_entity
+        return
       end
+
+      # Remove duplicates to avoid redundant processing
+      unique_student_ids = student_ids.uniq
+      remove_students(unique_student_ids)
     end
 
     private
 
+    def remove_students(student_ids)
+      service = StudentRemovalService.new(
+        students: student_ids,
+        school: @school,
+        remove_from_profile: true,
+        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: :ok
+      else
+        render json: { results: results }, status: :ok
+      end
+    end
+
     def school_student_params
       params.require(:school_student).permit(:username, :password, :name)
     end
@@ -135,6 +180,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

@@ -67,7 +67,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[read destroy], Feedback, school_project: { school_id: school.id })

app/models/school.rb

@@ -24,8 +24,7 @@ class School < ApplicationRecord
             if: :united_kingdom?
   validates :district_nces_id,
             uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.district_nces_id_exists') },
-            format: { with: /\A\d{12}\z/, allow_nil: true, message: I18n.t('validations.school.district_nces_id') },
-            presence: true,
+            format: { with: /\A\d{7}\z/, allow_nil: true, message: I18n.t('validations.school.district_nces_id') },
             if: :united_states?
   validates :district_name, presence: true, if: :united_states?
   validates :school_roll_number,

app/services/student_removal_service.rb

@@ -11,25 +11,29 @@ def initialize(students:, school:, remove_from_profile: false, token: nil)
   # Returns an array of hashes, one per student, with details of what was removed
   def remove_students
     results = []
+
     @students.each do |user_id|
+      student_roles = Role.student.where(user_id:, school_id: @school.id)
+      if student_roles.empty?
+        results << { user_id:, skipped: true, reason: 'no_role_in_school' }
+        next
+      end
+
       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 for this user
+          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.joins(:school_class).where(student_id: user_id, school_class: { school_id: @school.id })
+          class_assignments.destroy_all
 
-            # Remove roles
-            roles = Role.student.where(user_id: user_id)
-            roles.destroy_all
-          end
+          # Remove roles
+          student_roles.destroy_all
 
-          # Remove from profile if requested
+          # Remove from profile if requested - inside transaction so it can be rolled back
           ProfileApiClient.delete_school_student(token: @token, school_id: @school.id, student_id: user_id) if @remove_from_profile && @token.present?
         end
       rescue StandardError => e

config/locales/en.yml

@@ -17,7 +17,7 @@ en:
       website: "must be a valid URL"
       reference: "must be 5-6 digits (e.g., 100000)"
       reference_urn_exists: "URN number already exists"
-      district_nces_id: "must be 12 digits (e.g., 010000000001)"
+      district_nces_id: "must be 7 digits (e.g., 0100000)"
       district_nces_id_exists: "NCES ID already exists"
       school_roll_number: "must be numbers followed by letters (e.g., 01572D)"
       school_roll_number_exists: "School roll number already exists"

config/routes.rb

@@ -71,6 +71,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