Resolve merge conflict

Author: mwtrew

.clabot

@@ -24,12 +24,14 @@
     "magdalenajadach",
     "MarcScott",
     "Mohamed-RPF",
+    "mwtrew",
     "patch0",
     "peconia",
     "PetarSimonovic",
     "pjbRPF",
     "sra405",
-    "tuzz"
+    "tuzz",
+    "zetter-rpf"
   ],
   "message": "We require contributors to sign our Contributor License Agreement, and we don't have you on file. In order for us to review and merge your code, please complete [this form](https://form.raspberrypi.org/4873530) and we'll get you added and review your contribution as soon as possible."
 }

.env.example

@@ -54,3 +54,6 @@ GOOGLE_CLIENT_SECRET=changeme
 
 # E2E tests can supply this to enable test utility endpoints
 RESEED_API_KEY=changeme
+
+# Enable immediate onboarding for schools
+ENABLE_IMMEDIATE_SCHOOL_ONBOARDING=true

Gemfile.lock

@@ -558,6 +558,7 @@ GEM
 PLATFORMS
   aarch64-linux
   arm64-darwin-24
+  arm64-darwin-25
   x86_64-linux
 
 DEPENDENCIES

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/controllers/api/schools_controller.rb

@@ -16,13 +16,16 @@ def show
     end
 
     def create
-      result = School::Create.call(school_params:, creator_id: current_user.id)
+      result = School::Create.call(school_params:, creator_id: current_user.id, token: current_user.token)
 
       if result.success?
         @school = result[:school]
         render :show, formats: [:json], status: :created
       else
-        render json: { error: result[:error] }, status: :unprocessable_entity
+        render json: {
+          error: result[:error],
+          error_types: result[:error_types]
+        }, status: :unprocessable_entity
       end
     end
 

app/jobs/school_import_job.rb

@@ -71,7 +71,8 @@ def import_school(school_data)
     School.transaction do
       result = School::Create.call(
         school_params: school_params,
-        creator_id: proposed_owner[:id]
+        creator_id: proposed_owner[:id],
+        token: @token
       )
 
       if result.success?
@@ -128,7 +129,10 @@ def build_school_params(school_data)
       creator_agree_authority: true,
       creator_agree_terms_and_conditions: true,
       creator_agree_responsible_safeguarding: true,
-      user_origin: 'experience_cs'
+      user_origin: 'experience_cs',
+      district_name: school_data[:district_name],
+      district_nces_id: school_data[:district_nces_id],
+      school_roll_number: school_data[:school_roll_number]
     }.compact
   end
 

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

@@ -15,13 +15,22 @@ class School < ApplicationRecord
   validates :website, presence: true, format: { with: VALID_URL_REGEX, message: I18n.t('validations.school.website') }
   validates :address_line_1, presence: true
   validates :municipality, presence: true
+  validates :administrative_area, presence: true
+  validates :postal_code, presence: true
   validates :country_code, presence: true, inclusion: { in: ISO3166::Country.codes }
-  validates :reference, uniqueness: { case_sensitive: false, allow_nil: true }, presence: false
-  validates :district_nces_id, uniqueness: { case_sensitive: false, allow_nil: true }, presence: false
+  validates :reference,
+            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.reference_urn_exists') },
+            format: { with: /\A\d{5,6}\z/, allow_nil: true, message: I18n.t('validations.school.reference') },
+            if: :united_kingdom?
+  validates :district_nces_id,
+            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,
-            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_nil: true },
-            presence: { if: :ireland? },
-            format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') }
+            uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.school_roll_number_exists') },
+            format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') },
+            presence: true,
+            if: :ireland?
   validates :creator_id, presence: true, uniqueness: true
   validates :creator_agree_authority, presence: true, acceptance: true
   validates :creator_agree_terms_and_conditions, presence: true, acceptance: true
@@ -30,8 +39,6 @@ class School < ApplicationRecord
   validates :verified_at, absence: { if: proc { |school| school.rejected? } }
   validates :code,
             uniqueness: { allow_nil: true },
-            presence: { if: proc { |school| school.verified? } },
-            absence: { unless: proc { |school| school.verified? } },
             format: { with: /\d\d-\d\d-\d\d/, allow_nil: true }
   validate :verified_at_cannot_be_changed
   validate :code_cannot_be_changed
@@ -42,6 +49,9 @@ class School < ApplicationRecord
 
   before_save :format_uk_postal_code, if: :should_format_uk_postal_code?
 
+  # TODO: Remove the conditional once the feature flag is retired
+  after_create :generate_code!, if: -> { FeatureFlags.immediate_school_onboarding? }
+
   def self.find_for_user!(user)
     school = Role.find_by(user_id: user.id)&.school || find_by(creator_id: user.id)
     raise ActiveRecord::RecordNotFound unless school
@@ -62,9 +72,19 @@ def rejected?
   end
 
   def verify!
+    # TODO: Remove this line once the feature flag is retired
+    generate_code! unless FeatureFlags.immediate_school_onboarding?
+
+    update!(verified_at: Time.zone.now)
+  end
+
+  def generate_code!
+    return code if code.present?
+
     attempts = 0
     begin
-      update!(verified_at: Time.zone.now, code: ForEducationCodeGenerator.generate)
+      new_code = ForEducationCodeGenerator.generate
+      update!(code: new_code)
     rescue ActiveRecord::RecordInvalid => e
       raise unless e.record.errors[:code].include?('has already been taken') && attempts <= 5
 
@@ -78,6 +98,8 @@ def reject
   end
 
   def reopen
+    return false unless rejected?
+
     update(rejected_at: nil)
   end
 
@@ -122,13 +144,21 @@ def rejected_at_cannot_be_changed
   end
 
   def code_cannot_be_changed
-    errors.add(:code, 'cannot be changed after verification') if code_was.present? && code_changed?
+    errors.add(:code, 'cannot be changed after onboarding') if code_was.present? && code_changed?
   end
 
   def should_format_uk_postal_code?
     country_code == 'GB' && postal_code.to_s.length >= 5
   end
 
+  def united_kingdom?
+    country_code == 'GB'
+  end
+
+  def united_states?
+    country_code == 'US'
+  end
+
   def ireland?
     country_code == 'IE'
   end

app/services/school_onboarding_service.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+class SchoolOnboardingService
+  attr_reader :school
+
+  def initialize(school)
+    @school = school
+  end
+
+  def onboard(token:)
+    School.transaction do
+      Role.owner.create!(user_id: school.creator_id, school:)
+      Role.teacher.create!(user_id: school.creator_id, school:)
+
+      ProfileApiClient.create_school(token:, id: school.id, code: school.code)
+    end
+  rescue StandardError => e
+    Sentry.capture_exception(e)
+    Rails.logger.error { "Failed to onboard school #{@school.id}: #{e.message}" }
+    false
+  else
+    true
+  end
+end