From dc7f648518ea5073caf5bae7f443fef74e059606 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 19 Sep 2025 10:45:32 +0100
Subject: [PATCH 01/19] Introduce validate_school_students to ProfileApiClient.

This commit allows ProfileApiClient to call the new fast validation
endpoint in Profile.
---
 lib/profile_api_client.rb                       | 15 +++++++++++++++
 .../creating_a_batch_of_school_students_spec.rb | 12 +++++++++++-
 spec/support/profile_api_mock.rb                | 17 +++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb
index 9983e42d3..5f7cb76e0 100644
--- a/lib/profile_api_client.rb
+++ b/lib/profile_api_client.rb
@@ -93,6 +93,21 @@ def create_school_student(token:, username:, password:, name:, school_id:)
       raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
 
+    def validate_school_students(token:, students:, school_id:)
+      return nil if token.blank?
+
+      students = Array(students)
+      endpoint = "/api/v1/schools/#{school_id}/students/preflight-student-upload"
+      response = connection(token).post(endpoint) do |request|
+        request.body = { students: students, school_id: school_id }.to_json
+        request.headers['Content-Type'] = 'application/json'
+      end
+
+      raise UnexpectedResponse, response unless response.status == 200
+    rescue Faraday::UnprocessableEntityError => e
+      raise Student422Error, JSON.parse(e.response_body)
+    end
+
     def create_school_students(token:, students:, school_id:, preflight: false)
       return nil if token.blank?
 
diff --git a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
index 536187f6c..24d5b3fcd 100644
--- a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
+++ b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
@@ -6,6 +6,7 @@
   before do
     authenticated_in_hydra_as(owner)
     stub_profile_api_create_school_students
+    stub_profile_api_validate_school_students
     stub_profile_api_create_safeguarding_flag
 
     # UserJob will fail validation as it won't find our test job, so we need to double it
@@ -105,9 +106,18 @@
 
   it 'responds 422 Unprocessable Entity with a JSON array of validation errors' do
     stub_profile_api_create_school_students_validation_error
+    stub_profile_api_validate_students_with_validation_error
     post("/api/schools/#{school.id}/students/batch", headers:, params:)
     expect(response).to have_http_status(:unprocessable_entity)
-    expect(response.body).to eq('{"error":{"student-to-create":["isUniqueInBatch","isComplex","notEmpty"],"another-student-to-create-2":["minLength","notEmpty"]},"error_type":"validation_error"}')
+    expect(response.body).to eq(
+      {
+        error: {
+          'student-to-create' => %w[isUniqueInBatch isComplex notEmpty],
+          'another-student-to-create-2' => %w[minLength notEmpty]
+        },
+        error_type: 'validation_error'
+      }.to_json
+    )
   end
 
   it 'responds 401 Unauthorized when no token is given' do
diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb
index d27f97bc2..61ddb5e1b 100644
--- a/spec/support/profile_api_mock.rb
+++ b/spec/support/profile_api_mock.rb
@@ -108,4 +108,21 @@ def stub_profile_api_create_school_students_sso_validation_error
       )
     )
   end
+
+  def stub_profile_api_validate_school_students
+    allow(ProfileApiClient).to receive(:validate_school_students)
+  end
+
+  def stub_profile_api_validate_students_with_validation_error
+    allow(ProfileApiClient).to receive(:validate_school_students).and_raise(
+      ProfileApiClient::Student422Error.new(
+        [{ 'path' => '0.username', 'errorCode' => 'isUniqueInBatch', 'message' => 'Username must be unique in the batch data', 'location' => 'body', 'username' => 'student-to-create' },
+         { 'path' => '0.password', 'errorCode' => 'isComplex', 'message' => 'Password is too simple (it should not be easily guessable, <a href="https://my.raspberrypi.org/password-help">need password help?</a>)', 'location' => 'body', 'username' => 'student-to-create' },
+         { 'path' => '0.name', 'errorCode' => 'notEmpty', 'message' => 'Validation notEmpty on name failed', 'location' => 'body', 'username' => 'student-to-create' },
+         { 'path' => '1.username', 'errorCode' => 'isUniqueInBatch', 'message' => 'Username must be unique in the batch data', 'location' => 'body', 'username' => 'student-to-create' },
+         { 'path' => '2.password', 'errorCode' => 'minLength', 'message' => 'Password must be at least 8 characters', 'location' => 'body', 'username' => 'another-student-to-create-2' },
+         { 'path' => '2.name', 'errorCode' => 'notEmpty', 'message' => 'Validation notEmpty on name failed', 'location' => 'body', 'username' => 'another-student-to-create-2' }]
+      )
+    )
+  end
 end

From 9a676568c0cf7beb9d121b673fef5e21c6870dec Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 19 Sep 2025 10:46:40 +0100
Subject: [PATCH 02/19] Introduce a new GoodJob queue just for creating
 students.

This is deliberately a serial queue.
---
 config/initializers/good_job.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/config/initializers/good_job.rb b/config/initializers/good_job.rb
index 54a245bea..063611605 100644
--- a/config/initializers/good_job.rb
+++ b/config/initializers/good_job.rb
@@ -13,3 +13,8 @@ def authenticate_admin
     end
   end
 end
+
+Rails.application.configure do
+  # The create_students_job queue is a serial queue that allows only one job at a time.
+  config.good_job.queues = 'create_students_job:1;default:5'
+end

From d118e325bffa0bf2555e4ce3d38585a4b945708a Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 19 Sep 2025 10:47:46 +0100
Subject: [PATCH 03/19] Refactor SchoolStudent::CreateBatch into
 ::ValidateBatch

This commit creates a new operation that validates a batch. It moves
all of the validation error handling out of ::CreateBatch, leaving that
operation quite simple.
---
 lib/concepts/school_student/create_batch.rb   | 47 -------------
 lib/concepts/school_student/validate_batch.rb | 68 +++++++++++++++++++
 2 files changed, 68 insertions(+), 47 deletions(-)
 create mode 100644 lib/concepts/school_student/validate_batch.rb

diff --git a/lib/concepts/school_student/create_batch.rb b/lib/concepts/school_student/create_batch.rb
index a1048e75f..e33837ff2 100644
--- a/lib/concepts/school_student/create_batch.rb
+++ b/lib/concepts/school_student/create_batch.rb
@@ -3,15 +3,6 @@
 module SchoolStudent
   class Error < StandardError; end
 
-  class ValidationError < StandardError
-    attr_reader :errors
-
-    def initialize(errors)
-      @errors = errors
-      super()
-    end
-  end
-
   class ConcurrencyExceededForSchool < StandardError; end
 
   class CreateBatch
@@ -20,10 +11,6 @@ def call(school:, school_students_params:, token:, user_id:)
         response = OperationResponse.new
         response[:job_id] = create_batch(school, school_students_params, token, user_id)
         response
-      rescue ValidationError => e
-        response[:error] = e.errors
-        response[:error_type] = :validation_error
-        response
       rescue ConcurrencyExceededForSchool => e
         response[:error] = e
         response[:error_type] = :job_concurrency_error
@@ -38,43 +25,9 @@ def call(school:, school_students_params:, token:, user_id:)
       private
 
       def create_batch(school, students, token, user_id)
-        # Ensure that nil values are empty strings, else Profile will swallow validations
-        students = students.map do |student|
-          student.transform_values { |value| value.nil? ? '' : value }
-        end
-
-        validate(school:, students:, token:)
-
         job = CreateStudentsJob.attempt_perform_later(school_id: school.id, students:, token:, user_id:)
         job&.job_id
       end
-
-      def validate(school:, students:, token:)
-        decrypted_students = decrypt_students(students)
-        ProfileApiClient.create_school_students(token:, students: decrypted_students, school_id: school.id, preflight: true)
-      rescue ProfileApiClient::Student422Error => e
-        handle_student422_error(e.errors)
-      end
-
-      def decrypt_students(students)
-        students.deep_dup.each do |student|
-          student[:password] = DecryptionHelpers.decrypt_password(student[:password]) if student[:password].present?
-        end
-      end
-
-      def handle_student422_error(errors)
-        formatted_errors = errors.each_with_object({}) do |error, hash|
-          username = error['username'] || error['path']
-
-          hash[username] ||= []
-          hash[username] << (error['errorCode'] || error['message'])
-
-          # Ensure uniqueness to avoid repeat errors with duplicate usernames
-          hash[username] = hash[username].uniq
-        end
-
-        raise ValidationError, formatted_errors unless formatted_errors.nil? || formatted_errors.blank?
-      end
     end
   end
 end
diff --git a/lib/concepts/school_student/validate_batch.rb b/lib/concepts/school_student/validate_batch.rb
new file mode 100644
index 000000000..ba22c34e2
--- /dev/null
+++ b/lib/concepts/school_student/validate_batch.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+module SchoolStudent
+  class Error < StandardError; end
+
+  class ValidationError < StandardError
+    attr_reader :errors
+
+    def initialize(errors)
+      @errors = errors
+      super()
+    end
+  end
+
+  class ValidateBatch
+    class << self
+      def call(school:, students:, token:)
+        response = OperationResponse.new
+        validate_batch(school:, students:, token:)
+        response
+      rescue ValidationError => e
+        response[:error] = e.errors
+        response[:error_type] = :validation_error
+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response[:error] = "Error creating school students: #{e}"
+        response[:error_type] = :standard_error
+        response
+      end
+
+      private
+
+      def validate_batch(school:, students:, token:)
+        decrypted_students = decrypt_students(students)
+        ProfileApiClient.validate_school_students(token:, students: decrypted_students, school_id: school.id)
+      rescue ProfileApiClient::Student422Error => e
+        handle_student422_error(e.errors)
+      end
+
+      def decrypt_students(students)
+        students.deep_dup.each do |student|
+          student[:password] = DecryptionHelpers.decrypt_password(student[:password]) if student[:password].present?
+        end
+      end
+
+      # This method converts the error structure returned by Profile (an array of error objects) to
+      # the structure expected by the React front-end, which is a hash with the structure:
+      #
+      # username => [array of error codes]
+      #
+      # The front end will translate the error codes into user-readable error messages.
+      def handle_student422_error(errors)
+        formatted_errors = errors.each_with_object({}) do |error, hash|
+          username = error['username'] || error['path']
+
+          hash[username] ||= []
+          hash[username] << error['errorCode']
+
+          # Ensure uniqueness to avoid repeat errors with duplicate usernames
+          hash[username] = hash[username].uniq
+        end
+
+        raise ValidationError, formatted_errors unless formatted_errors.nil? || formatted_errors.blank?
+      end
+    end
+  end
+end

From a09a9f7222291148abcad3981eb8330861f323ca Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 19 Sep 2025 10:50:39 +0100
Subject: [PATCH 04/19] Raise batch validation to the SchoolStudentsController
 level.

This commmit takes the validation and concurrency control out of create_students_job
and puts it in school_students_controller.

The idea is that instead of validating then committing one job of 50
students, we:

1. validate the entire batch of N students quickly by calling
   SchoolStudents::ValidateBatch
2. Split the batch into chunks of 50.
3. Enqueue them in the context of a GoodJob::Batch, ensuring atomic
   enqueue of all `N/50` jobs.
4. Control concurrency by not allowing creation of another Batch whose
   description field matches the school ID (this makes up for the fact
   that GoodJob::Batch does not have a concurrency key like Jobs do).
---
 .../api/school_students_controller.rb         | 82 +++++++++++++++++--
 app/jobs/create_students_job.rb               | 18 ++--
 ...reating_a_batch_of_school_students_spec.rb | 22 +++++
 3 files changed, 102 insertions(+), 20 deletions(-)

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index 5eba8a5c2..f27789938 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -32,15 +32,83 @@ def create
     end
 
     def create_batch
-      result = SchoolStudent::CreateBatch.call(
-        school: @school, school_students_params:, token: current_user.token, user_id: current_user.id
+      if school_students_params.blank?
+        render json: {
+                 error: StandardError,
+                 error_type: :unprocessable_entity
+               },
+               status: :unprocessable_entity
+        return
+      end
+
+      students = normalise_student_nil_values_to_empty_strings(school_students_params)
+
+      # We validate the entire batch here in one go and then, if the validation succeds,
+      # feed the batch to Profile in chunks of 50.
+      validation_result = SchoolStudent::ValidateBatch.call(
+        school: @school, students: students, token: current_user.token
       )
 
-      if result.success?
-        @job_id = result[:job_id]
-        render :create_batch, formats: [:json], status: :accepted
-      else
-        render json: { error: result[:error], error_type: result[:error_type] }, status: :unprocessable_entity
+      if validation_result.failure?
+        render json: {
+          error: validation_result[:error],
+          error_type: validation_result[:error_type]
+        }, status: :unprocessable_entity
+        return
+      end
+
+      # If we get this far, validation of the entire batch succeeded, so we enqueue it in chunks
+      begin
+        enqueue_batches(students)
+      rescue StandardError => e
+        Rails.logger.error "Failed to enqueue GoodJob Batch: #{e}"
+        render json: { error: e, error_type: :batch_error }, status: :unprocessable_entity
+        return
+      end
+
+      # We enqueued everything! Yay!
+      render :create_batch, formats: [:json], status: :accepted
+    end
+
+    # This method returns true if there is an existing, unfinished, batch whose description
+    # matches the current school ID. This prevents two users enqueueing a batch for
+    # the same school, since GoodJob::Batch doesn't support a concurrency key.
+    def batch_in_progress?(identifier:)
+      GoodJob::BatchRecord.where(finished_at: nil)
+                          .where(discarded_at: nil)
+                          .exists?(description: identifier)
+    end
+
+    # This method takes a large list of students to insert and enqueues a GoodJob
+    # Batch to insert them, 50 at a time. We use a GoodJob::Batch to enqueue the
+    # set of jobs atomically.
+    #
+    # This method will throw an error if any batch fails to enqueue, so callers
+    # should assume the entire student import has failed.
+    def enqueue_batches(students)
+      # Set the maximum batch size to the limit imposed by Profile
+      max_batch_size = 50
+      batch_identifier = "school_id:#{@school.id}"
+
+      # Raise if a batch is already in progress for this school.
+      raise ConcurrencyExceededForSchool if batch_in_progress?(identifier: batch_identifier)
+
+      batch = GoodJob::Batch.new(description: batch_identifier)
+      batch.enqueue do
+        students.each_slice(max_batch_size) do |student_batch|
+          SchoolStudent::CreateBatch.call(
+            school: @school, school_students_params: student_batch, token: current_user.token, user_id: current_user.id
+          )
+        end
+      end
+
+      Rails.logger.info("Batch #{batch.id} enqueued successfully with identifier #{batch_identifier}!")
+    end
+
+    def normalise_student_nil_values_to_empty_strings(students)
+      # Ensure that nil values are empty strings, else Profile will swallow validations
+      students.map do |student|
+        student.transform_values { |value| value.nil? ? '' : value }
       end
     end
 
diff --git a/app/jobs/create_students_job.rb b/app/jobs/create_students_job.rb
index 446724ae0..ccf13af85 100644
--- a/app/jobs/create_students_job.rb
+++ b/app/jobs/create_students_job.rb
@@ -24,26 +24,18 @@ class CreateStudentsJob < ApplicationJob
 
   include GoodJob::ActiveJobExtensions::Concurrency
 
-  queue_as :default
+  queue_as :create_students_job
 
   # Restrict to one job per school to avoid duplicates
   good_job_control_concurrency_with(
     key: -> { "create_students_job_#{arguments.first[:school_id]}" },
-    total_limit: 1
+    perform_limit: 1
   )
 
   def self.attempt_perform_later(school_id:, students:, token:, user_id:)
-    concurrency_key = "create_students_job_#{school_id}"
-    existing_jobs = GoodJob::Job.where(concurrency_key:, finished_at: nil)
-
-    raise ConcurrencyExceededForSchool, 'Only one job per school can be enqueued at a time.' if existing_jobs.exists?
-
-    ActiveRecord::Base.transaction do
-      job = perform_later(school_id:, students:, token:)
-      UserJob.create!(user_id:, good_job_id: job.job_id) unless job.nil?
-
-      job
-    end
+    job = perform_later(school_id:, students:, token:)
+    UserJob.create!(user_id:, good_job_id: job.job_id) unless job.nil?
+    job
   end
 
   def perform(school_id:, students:, token:)
diff --git a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
index 24d5b3fcd..c770bb1ea 100644
--- a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
+++ b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
@@ -69,6 +69,28 @@
     expect(response).to have_http_status(:accepted)
   end
 
+  it 'responds 422 when a batch already exists for this school' do
+    # Create a fake batch for the school.
+    batch_identifier = "school_id:#{school.id}"
+    GoodJob::BatchRecord.create!(
+      description: batch_identifier,
+      finished_at: nil,
+      discarded_at: nil
+    )
+
+    expect do
+      post("/api/schools/#{school.id}/students/batch", headers:, params:)
+    end.not_to change(GoodJob::BatchRecord, :count)
+    expect(response).to have_http_status(:unprocessable_entity)
+
+    active_batches = GoodJob::BatchRecord.where(
+      description: batch_identifier,
+      finished_at: nil,
+      discarded_at: nil
+    )
+    expect(active_batches.count).to eq(1)
+  end
+
   it 'responds 202 Accepted when the user is a school-teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)

From bfd91d0ebc9c68f38b71d335b1f6dc1315dfb06d Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 19 Sep 2025 10:55:25 +0100
Subject: [PATCH 05/19] Fix handling of `students: []` parameters in tests.

Switching to `filter_map` ensures that a parameter list of `[]` gets
through as `[]` and not `[nil]`.
---
 app/controllers/api/school_students_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index f27789938..7bf7b1f37 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -143,7 +143,7 @@ def school_student_params
     def school_students_params
       school_students = params.require(:school_students)
 
-      school_students.map do |student|
+      school_students.filter_map do |student|
         next if student.blank?
 
         student.permit(:username, :password, :name).to_h.with_indifferent_access

From 2f92544d9f98016d762140f8ddfc6286c8f35c9f Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 19 Sep 2025 11:36:42 +0100
Subject: [PATCH 06/19] This commit removes SchoolStudent::CreateBatch tests
 that are obsolete.

The validation functionality has been refactored and raised to the
controller level that calls CreateBatch, so the tests are at a higher
level now.
---
 .../school_student/create_batch_spec.rb       | 57 -------------------
 spec/jobs/create_students_job_spec.rb         |  9 ---
 2 files changed, 66 deletions(-)

diff --git a/spec/concepts/school_student/create_batch_spec.rb b/spec/concepts/school_student/create_batch_spec.rb
index 116a0125f..46ab91b13 100644
--- a/spec/concepts/school_student/create_batch_spec.rb
+++ b/spec/concepts/school_student/create_batch_spec.rb
@@ -52,61 +52,4 @@
       expect(response[:job_id]).to be_truthy
     end
   end
-
-  context 'when a normal error occurs' do
-    let(:school_students_params) do
-      [
-        {
-          username: 'a-student',
-          password: 'Password',
-          name: 'School Student'
-        },
-        {
-          username: 'second-student-to-create',
-          password: 'Password',
-          name: 'School Student 2'
-        }
-      ]
-    end
-
-    before do
-      stub_profile_api_create_school_students(user_ids: [SecureRandom.uuid, SecureRandom.uuid])
-      allow(Sentry).to receive(:capture_exception)
-    end
-
-    it 'does not queue a new job' do
-      expect do
-        described_class.call(school:, school_students_params:, token:, user_id:)
-      end.not_to have_enqueued_job(CreateStudentsJob)
-    end
-
-    it 'returns a failed operation response' do
-      response = described_class.call(school:, school_students_params:, token:, user_id:)
-      expect(response.failure?).to be(true)
-    end
-
-    it 'returns the error message in the operation response' do
-      response = described_class.call(school:, school_students_params:, token:, user_id:)
-      error_message = response[:error]
-      expect(error_message).to match(/Decryption failed: iv must be 16 bytes/)
-    end
-
-    it 'sent the exception to Sentry' do
-      described_class.call(school:, school_students_params:, token:, user_id:)
-      expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
-    end
-  end
-
-  context 'when a validation error occurs' do
-    before do
-      stub_profile_api_create_school_students_validation_error
-    end
-
-    it 'returns the expected error codes' do
-      response = described_class.call(school:, school_students_params:, token:, user_id:)
-      expect(response[:error]).to eq(
-        { 'student-to-create' => %w[isUniqueInBatch isComplex notEmpty], 'another-student-to-create-2' => %w[minLength notEmpty] }
-      )
-    end
-  end
 end
diff --git a/spec/jobs/create_students_job_spec.rb b/spec/jobs/create_students_job_spec.rb
index 91b65ebf9..bc4a76193 100644
--- a/spec/jobs/create_students_job_spec.rb
+++ b/spec/jobs/create_students_job_spec.rb
@@ -45,13 +45,4 @@
 
     expect(Role.student.where(school:, user_id:)).to exist
   end
-
-  it 'does not enqueue a job if one is already running for that school' do
-    # Enqueue the job
-    GoodJob::Job.enqueue(described_class.new(school_id: school.id, students:, token:))
-
-    expect do
-      described_class.attempt_perform_later(school_id: school.id, students:, token:, user_id:)
-    end.to raise_error(ConcurrencyExceededForSchool)
-  end
 end

From fddf780cff60427ad45017a3472c576b9419cbc8 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Wed, 24 Sep 2025 11:36:21 +0100
Subject: [PATCH 07/19] Update ProfileApiClient to match params and errors in
 Profile.

Recent updates in profile made a small number of changes to the
structure of errors and parameters in profile. This commit brings
editor-api up to match it.
---
 lib/profile_api_client.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb
index 5f7cb76e0..bd70fe796 100644
--- a/lib/profile_api_client.rb
+++ b/lib/profile_api_client.rb
@@ -99,13 +99,13 @@ def validate_school_students(token:, students:, school_id:)
       students = Array(students)
       endpoint = "/api/v1/schools/#{school_id}/students/preflight-student-upload"
       response = connection(token).post(endpoint) do |request|
-        request.body = { students: students, school_id: school_id }.to_json
+        request.body = students.to_json
         request.headers['Content-Type'] = 'application/json'
       end
 
       raise UnexpectedResponse, response unless response.status == 200
     rescue Faraday::UnprocessableEntityError => e
-      raise Student422Error, JSON.parse(e.response_body)
+      raise Student422Error, JSON.parse(e.response_body)['errors']
     end
 
     def create_school_students(token:, students:, school_id:, preflight: false)

From 78ec24d0bb018a5bc9f20f7db34377cddc558bf5 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Wed, 24 Sep 2025 11:47:54 +0100
Subject: [PATCH 08/19] Make MAX_BATCH_CREATION_SIZE a constant in
 SchoolStudentsController.

---
 app/controllers/api/school_students_controller.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index 7bf7b1f37..ec45b30b2 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -2,6 +2,10 @@
 
 module Api
   class SchoolStudentsController < ApiController
+    # This constant is the maximum batch size we can post to
+    # profile in the create step. We can validate larger batches.
+    MAX_BATCH_CREATION_SIZE = 50
+
     before_action :authorize_user
     load_and_authorize_resource :school
     authorize_resource :school_student, class: false
@@ -87,7 +91,6 @@ def batch_in_progress?(identifier:)
     # should assume the entire student import has failed.
     def enqueue_batches(students)
       # Set the maximum batch size to the limit imposed by Profile
-      max_batch_size = 50
       batch_identifier = "school_id:#{@school.id}"
 
       # Raise if a batch is already in progress for this school.
@@ -95,7 +98,7 @@ def enqueue_batches(students)
 
       batch = GoodJob::Batch.new(description: batch_identifier)
       batch.enqueue do
-        students.each_slice(max_batch_size) do |student_batch|
+        students.each_slice(MAX_BATCH_CREATION_SIZE) do |student_batch|
           SchoolStudent::CreateBatch.call(
             school: @school, school_students_params: student_batch, token: current_user.token, user_id: current_user.id
           )

From 71a790623449bd7d6bdc35ee1af0a561a583c17f Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Wed, 24 Sep 2025 12:00:24 +0100
Subject: [PATCH 09/19] Refactor the nil-to-empty-string function into a new
 StudentHelpers.

---
 app/controllers/api/school_students_controller.rb |  9 +--------
 lib/helpers/student_helpers.rb                    | 10 ++++++++++
 2 files changed, 11 insertions(+), 8 deletions(-)
 create mode 100644 lib/helpers/student_helpers.rb

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index ec45b30b2..34c23a69e 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -45,7 +45,7 @@ def create_batch
         return
       end
 
-      students = normalise_student_nil_values_to_empty_strings(school_students_params)
+      students = StudentHelpers.normalise_nil_values_to_empty_strings(school_students_params)
 
       # We validate the entire batch here in one go and then, if the validation succeds,
       # feed the batch to Profile in chunks of 50.
@@ -108,13 +108,6 @@ def enqueue_batches(students)
       Rails.logger.info("Batch #{batch.id} enqueued successfully with identifier #{batch_identifier}!")
     end
 
-    def normalise_student_nil_values_to_empty_strings(students)
-      # Ensure that nil values are empty strings, else Profile will swallow validations
-      students.map do |student|
-        student.transform_values { |value| value.nil? ? '' : value }
-      end
-    end
-
     def update
       result = SchoolStudent::Update.call(
         school: @school, student_id: params[:id], school_student_params:, token: current_user.token
diff --git a/lib/helpers/student_helpers.rb b/lib/helpers/student_helpers.rb
new file mode 100644
index 000000000..982b98bee
--- /dev/null
+++ b/lib/helpers/student_helpers.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class StudentHelpers
+  # Ensure that nil values are empty strings, else Profile will swallow validations
+  def self.normalise_nil_values_to_empty_strings(students)
+    students.map do |student|
+      student.transform_values { |value| value.nil? ? '' : value }
+    end
+  end
+end

From c206c71f15b11d6083cc2907b9a81790424a5e2b Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Wed, 24 Sep 2025 13:45:29 +0100
Subject: [PATCH 10/19] Test that batches of students are correctly split into
 batches of <=50.

---
 .../creating_a_batch_of_school_students_spec.rb        | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
index c770bb1ea..b03f648e0 100644
--- a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
+++ b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
@@ -99,6 +99,16 @@
     expect(response).to have_http_status(:accepted)
   end
 
+  it 'splits students into jobs of 50 each' do
+    total_students = 169
+    students = Array.new(total_students) do |i|
+      { username: "student-#{i}", password: 'SaoXlDBAyiAFoMH3VsddhdA7JWnM8P8by1wOjBUWH2g=', name: "Student #{i}" }
+    end
+
+    post("/api/schools/#{school.id}/students/batch", headers:, params: { school_students: students })
+    expect(CreateStudentsJob).to have_received(:attempt_perform_later).exactly((total_students.to_f / 50).ceil).times
+  end
+
   it 'does not create the school owner safeguarding flag when the user is a school-teacher' do
     teacher = create(:teacher, school:)
     authenticated_in_hydra_as(teacher)

From 4c4ae0132c81efa4953e8f3420dd918a43713ef5 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Thu, 25 Sep 2025 09:24:45 +0100
Subject: [PATCH 11/19] Move batch_in_progress? from SchoolStudentsController
 to School

This commit moves the method SchoolStudentsController#batch_in_progress?
to School#import_in_progress? and changes the batch identifier from
"school_id:#{school.id}" to simply "#{school.id}".

This allows us to expose the import_in_progress field through the API so
that we can enable/disable front-end UI components to prevent user
frustration.
---
 .../api/school_students_controller.rb         | 19 +++----------------
 app/models/school.rb                          |  9 +++++++++
 ...reating_a_batch_of_school_students_spec.rb |  5 ++---
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index 34c23a69e..90c2a6159 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -74,15 +74,6 @@ def create_batch
       render :create_batch, formats: [:json], status: :accepted
     end
 
-    # This method returns true if there is an existing, unfinished, batch whose description
-    # matches the current school ID. This prevents two users enqueueing a batch for
-    # the same school, since GoodJob::Batch doesn't support a concurrency key.
-    def batch_in_progress?(identifier:)
-      GoodJob::BatchRecord.where(finished_at: nil)
-                          .where(discarded_at: nil)
-                          .exists?(description: identifier)
-    end
-
     # This method takes a large list of students to insert and enqueues a GoodJob
     # Batch to insert them, 50 at a time. We use a GoodJob::Batch to enqueue the
     # set of jobs atomically.
@@ -90,13 +81,10 @@ def batch_in_progress?(identifier:)
     # This method will throw an error if any batch fails to enqueue, so callers
     # should assume the entire student import has failed.
     def enqueue_batches(students)
-      # Set the maximum batch size to the limit imposed by Profile
-      batch_identifier = "school_id:#{@school.id}"
-
       # Raise if a batch is already in progress for this school.
-      raise ConcurrencyExceededForSchool if batch_in_progress?(identifier: batch_identifier)
+      raise ConcurrencyExceededForSchool if @school.import_in_progress?
 
-      batch = GoodJob::Batch.new(description: batch_identifier)
+      batch = GoodJob::Batch.new(description: @school.id)
       batch.enqueue do
         students.each_slice(MAX_BATCH_CREATION_SIZE) do |student_batch|
           SchoolStudent::CreateBatch.call(
@@ -104,8 +92,7 @@ def enqueue_batches(students)
           )
         end
       end
-
-      Rails.logger.info("Batch #{batch.id} enqueued successfully with identifier #{batch_identifier}!")
+      Rails.logger.info("Batch #{batch.id} enqueued successfully with identifier #{@school.id}!")
     end
 
     def update
diff --git a/app/models/school.rb b/app/models/school.rb
index e2dd475e7..776419c23 100644
--- a/app/models/school.rb
+++ b/app/models/school.rb
@@ -78,6 +78,15 @@ def postal_code=(str)
     super(str.to_s.upcase)
   end
 
+  # This method returns true if there is an existing, unfinished, batch whose description
+  # matches the current school ID. This prevents two users enqueueing a batch for
+  # the same school, since GoodJob::Batch doesn't support a concurrency key.
+  def import_in_progress?
+    GoodJob::BatchRecord.where(finished_at: nil)
+                        .where(discarded_at: nil)
+                        .exists?(description: id)
+  end
+
   private
 
   # Ensure the reference is nil, not an empty string
diff --git a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
index b03f648e0..3310870fb 100644
--- a/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
+++ b/spec/features/school_student/creating_a_batch_of_school_students_spec.rb
@@ -71,9 +71,8 @@
 
   it 'responds 422 when a batch already exists for this school' do
     # Create a fake batch for the school.
-    batch_identifier = "school_id:#{school.id}"
     GoodJob::BatchRecord.create!(
-      description: batch_identifier,
+      description: school.id,
       finished_at: nil,
       discarded_at: nil
     )
@@ -84,7 +83,7 @@
     expect(response).to have_http_status(:unprocessable_entity)
 
     active_batches = GoodJob::BatchRecord.where(
-      description: batch_identifier,
+      description: school.id,
       finished_at: nil,
       discarded_at: nil
     )

From 90745358455a3e3ee4568c597503b2bd6522da3b Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Thu, 25 Sep 2025 09:36:57 +0100
Subject: [PATCH 12/19] Expose School#import_in_progress? through API.

This change includes the state of current imports for a school in the
API response.
---
 app/views/api/schools/_school.json.jbuilder       | 2 ++
 spec/features/my_school/showing_my_school_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/views/api/schools/_school.json.jbuilder b/app/views/api/schools/_school.json.jbuilder
index 7dea9024d..a453b8bb5 100644
--- a/app/views/api/schools/_school.json.jbuilder
+++ b/app/views/api/schools/_school.json.jbuilder
@@ -25,3 +25,5 @@ json.code(school.code) if include_code
 
 include_user_origin = local_assigns.fetch(:user_origin, false)
 json.user_origin(school.user_origin) if include_user_origin
+
+json.import_in_progress school.import_in_progress?
diff --git a/spec/features/my_school/showing_my_school_spec.rb b/spec/features/my_school/showing_my_school_spec.rb
index eb3a2d4c2..db7c1ceef 100644
--- a/spec/features/my_school/showing_my_school_spec.rb
+++ b/spec/features/my_school/showing_my_school_spec.rb
@@ -18,7 +18,7 @@
 
   it "includes the school details and user's roles in the JSON" do
     school_json = school.to_json(only: %i[id name website reference address_line_1 address_line_2 municipality administrative_area postal_code country_code code verified_at created_at updated_at])
-    expected_data = JSON.parse(school_json, symbolize_names: true).merge(roles: ['owner'])
+    expected_data = JSON.parse(school_json, symbolize_names: true).merge(roles: ['owner'], import_in_progress: school.import_in_progress?)
 
     get('/api/school', headers:)
     data = JSON.parse(response.body, symbolize_names: true)

From 67f21fe1ecc17d20fc0554d3e31a7299a64fe71c Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 26 Sep 2025 14:26:57 +0100
Subject: [PATCH 13/19] Make UserJob track a Batch instead of a Job.

This commit changes UserJob from having a 1:1 relationship with
GoodJob::Job to tracking the ID of a GoodJob::Batch.

This means that the UserJobsController had to change to track the state
of a batch and emulate the status messages that a Job used to provide
(e.g. runnning, queued, etc.).

The front end can now poll for the status of this job.
---
 .../api/school_students_controller.rb         | 10 +--
 app/controllers/api/user_jobs_controller.rb   | 71 ++++++++++++++-----
 app/jobs/create_students_job.rb               | 12 +---
 app/models/user_job.rb                        |  3 +-
 .../create_batch.json.jbuilder                |  2 +-
 lib/concepts/school_student/create_batch.rb   |  8 +--
 .../school_student/create_batch_spec.rb       |  6 +-
 spec/factories/user_job.rb                    |  2 +-
 .../user_job/showing_user_jobs_spec.rb        |  8 +--
 spec/models/user_job_spec.rb                  |  2 +-
 10 files changed, 78 insertions(+), 46 deletions(-)

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index 90c2a6159..6f9d3295e 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -84,15 +84,17 @@ def enqueue_batches(students)
       # Raise if a batch is already in progress for this school.
       raise ConcurrencyExceededForSchool if @school.import_in_progress?
 
-      batch = GoodJob::Batch.new(description: @school.id)
-      batch.enqueue do
+      @batch = GoodJob::Batch.new(description: @school.id)
+      @batch.enqueue do
         students.each_slice(MAX_BATCH_CREATION_SIZE) do |student_batch|
           SchoolStudent::CreateBatch.call(
-            school: @school, school_students_params: student_batch, token: current_user.token, user_id: current_user.id
+            school: @school, school_students_params: student_batch, token: current_user.token
           )
         end
       end
-      Rails.logger.info("Batch #{batch.id} enqueued successfully with identifier #{@school.id}!")
+
+      UserJob.create!(user_id: current_user.id, good_job_batch_id: @batch.id)
+      Rails.logger.info("Batch #{@batch.id} enqueued successfully with school identifier #{@school.id}!")
     end
 
     def update
diff --git a/app/controllers/api/user_jobs_controller.rb b/app/controllers/api/user_jobs_controller.rb
index a51dbb016..b064d79e6 100644
--- a/app/controllers/api/user_jobs_controller.rb
+++ b/app/controllers/api/user_jobs_controller.rb
@@ -5,20 +5,28 @@ class UserJobsController < ApiController
     before_action :authorize_user
 
     def index
-      user_jobs = UserJob.where(user_id: current_user.id).includes(:good_job)
-      jobs = user_jobs&.map { |user_job| job_attributes(user_job&.good_job) }
-      if jobs.any?
-        render json: { jobs: }, status: :ok
+      user_jobs = UserJob.where(user_id: current_user.id)
+
+      batches = user_jobs.filter_map do |user_job|
+        batch_attributes_for(user_job)
+      end
+
+      if batches.any?
+        render json: { jobs: batches }, status: :ok
       else
         render json: { error: 'No jobs found' }, status: :not_found
       end
     end
 
     def show
-      user_job = UserJob.find_by(good_job_id: params[:id], user_id: current_user.id)
-      job = job_attributes(user_job&.good_job) unless user_job.nil?
-      if job.present?
-        render json: { job: }, status: :ok
+      user_job = UserJob.find_by(
+        good_job_batch_id: params[:id],
+        user_id: current_user.id
+      )
+
+      batch = batch_attributes_for(user_job) if user_job
+      if batch.present?
+        render json: { job: batch }, status: :ok
       else
         render json: { error: 'Job not found' }, status: :not_found
       end
@@ -26,16 +34,47 @@ def show
 
     private
 
-    def job_attributes(job)
+    def batch_attributes_for(user_job)
+      return nil unless user_job
+
+      begin
+        batch = GoodJob::Batch.find(user_job.good_job_batch_id)
+        batch_attributes(batch)
+      rescue GoodJob::Batch::NotFoundError
+        nil
+      end
+    end
+
+    def batch_attributes(batch)
       {
-        id: job.id,
-        concurrency_key: job.concurrency_key,
-        status: job.status,
-        scheduled_at: job.scheduled_at,
-        performed_at: job.performed_at,
-        finished_at: job.finished_at,
-        error: job.error
+        id: batch.id,
+        concurrency_key: batch.description,
+        status: batch_status(batch),
+        finished_at: batch.finished_at
       }
     end
+
+    # Try to emulate a Job's .status field for a batch.
+    def batch_status(batch)
+      # If the batch is finished or discarded, report that.
+      return 'discarded' if batch.discarded?
+      return 'finished'  if batch.finished?
+
+      # If the batch is in progress, try and summarise the state of the jobs
+      job_summary_status(GoodJob::Job.where(batch: batch.id))
+    end
+
+    # Summarise the status of a list of jobs.
+    def job_summary_status(jobs)
+      if jobs.any? { |j| j.status == :running }
+        'running'
+      elsif jobs.any? { |j| j.status == :retried }
+        'retrying'
+      elsif jobs.any? { |j| j.status == :scheduled }
+        'scheduled'
+      else
+        'queued'
+      end
+    end
   end
 end
diff --git a/app/jobs/create_students_job.rb b/app/jobs/create_students_job.rb
index ccf13af85..2d7e3f551 100644
--- a/app/jobs/create_students_job.rb
+++ b/app/jobs/create_students_job.rb
@@ -26,16 +26,8 @@ class CreateStudentsJob < ApplicationJob
 
   queue_as :create_students_job
 
-  # Restrict to one job per school to avoid duplicates
-  good_job_control_concurrency_with(
-    key: -> { "create_students_job_#{arguments.first[:school_id]}" },
-    perform_limit: 1
-  )
-
-  def self.attempt_perform_later(school_id:, students:, token:, user_id:)
-    job = perform_later(school_id:, students:, token:)
-    UserJob.create!(user_id:, good_job_id: job.job_id) unless job.nil?
-    job
+  def self.attempt_perform_later(school_id:, students:, token:)
+    perform_later(school_id:, students:, token:)
   end
 
   def perform(school_id:, students:, token:)
diff --git a/app/models/user_job.rb b/app/models/user_job.rb
index 5a0058669..512d60228 100644
--- a/app/models/user_job.rb
+++ b/app/models/user_job.rb
@@ -1,8 +1,7 @@
 # frozen_string_literal: true
 
 class UserJob < ApplicationRecord
-  belongs_to :good_job, class_name: 'GoodJob::Job'
-
+  validates :good_job_batch_id, presence: true
   validates :user_id, presence: true
 
   attr_accessor :user
diff --git a/app/views/api/school_students/create_batch.json.jbuilder b/app/views/api/school_students/create_batch.json.jbuilder
index 7c4447b23..318fa2808 100644
--- a/app/views/api/school_students/create_batch.json.jbuilder
+++ b/app/views/api/school_students/create_batch.json.jbuilder
@@ -1,3 +1,3 @@
 # frozen_string_literal: true
 
-json.job_id @job_id
+json.job_id @batch.id
diff --git a/lib/concepts/school_student/create_batch.rb b/lib/concepts/school_student/create_batch.rb
index e33837ff2..a7c0d6ef3 100644
--- a/lib/concepts/school_student/create_batch.rb
+++ b/lib/concepts/school_student/create_batch.rb
@@ -7,9 +7,9 @@ class ConcurrencyExceededForSchool < StandardError; end
 
   class CreateBatch
     class << self
-      def call(school:, school_students_params:, token:, user_id:)
+      def call(school:, school_students_params:, token:)
         response = OperationResponse.new
-        response[:job_id] = create_batch(school, school_students_params, token, user_id)
+        response[:job_id] = create_batch(school, school_students_params, token)
         response
       rescue ConcurrencyExceededForSchool => e
         response[:error] = e
@@ -24,8 +24,8 @@ def call(school:, school_students_params:, token:, user_id:)
 
       private
 
-      def create_batch(school, students, token, user_id)
-        job = CreateStudentsJob.attempt_perform_later(school_id: school.id, students:, token:, user_id:)
+      def create_batch(school, students, token)
+        job = CreateStudentsJob.attempt_perform_later(school_id: school.id, students:, token:)
         job&.job_id
       end
     end
diff --git a/spec/concepts/school_student/create_batch_spec.rb b/spec/concepts/school_student/create_batch_spec.rb
index 46ab91b13..776e0a9f5 100644
--- a/spec/concepts/school_student/create_batch_spec.rb
+++ b/spec/concepts/school_student/create_batch_spec.rb
@@ -29,7 +29,7 @@
 
     it 'queues CreateStudentsJob' do
       expect do
-        described_class.call(school:, school_students_params:, token:, user_id:)
+        described_class.call(school:, school_students_params:, token:)
       end.to have_enqueued_job(CreateStudentsJob).with(school_id: school.id, students: school_students_params, token:)
     end
   end
@@ -43,12 +43,12 @@
     end
 
     it 'returns a successful operation response' do
-      response = described_class.call(school:, school_students_params:, token:, user_id:)
+      response = described_class.call(school:, school_students_params:, token:)
       expect(response.success?).to be(true)
     end
 
     it 'returns the job id' do
-      response = described_class.call(school:, school_students_params:, token:, user_id:)
+      response = described_class.call(school:, school_students_params:, token:)
       expect(response[:job_id]).to be_truthy
     end
   end
diff --git a/spec/factories/user_job.rb b/spec/factories/user_job.rb
index 23bf0266d..551a69d40 100644
--- a/spec/factories/user_job.rb
+++ b/spec/factories/user_job.rb
@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :user_job do
-    good_job
+    good_job_batch_id { nil }
     user_id { SecureRandom.uuid }
   end
 end
diff --git a/spec/features/user_job/showing_user_jobs_spec.rb b/spec/features/user_job/showing_user_jobs_spec.rb
index 54eac2112..fd25bfccd 100644
--- a/spec/features/user_job/showing_user_jobs_spec.rb
+++ b/spec/features/user_job/showing_user_jobs_spec.rb
@@ -7,8 +7,8 @@
   let(:school) { create(:school) }
   let(:owner) { create(:owner, school:) }
 
-  let(:good_jobs) { create_list(:good_job, 2) }
-  let!(:user_jobs) { good_jobs.map { |job| create(:user_job, good_job: job, user_id: owner.id) } }
+  let!(:batch) { GoodJob::BatchRecord.create!(description: school.id) }
+  let!(:user_job) { create(:user_job, good_job_batch_id: batch.id, user_id: owner.id) }
 
   before do
     authenticated_in_hydra_as(owner)
@@ -28,11 +28,11 @@
     get('/api/user_jobs', headers:)
     data = JSON.parse(response.body, symbolize_names: true)
 
-    expect(data[:jobs].pluck(:id)).to eq(user_jobs.pluck(:good_job_id))
+    expect(data[:jobs].first[:id]).to eq(user_job.good_job_batch_id)
   end
 
   it 'responds with the expected job' do
-    job_id = user_jobs.first.good_job_id
+    job_id = user_job.good_job_batch_id
 
     get("/api/user_jobs/#{job_id}", headers:)
     data = JSON.parse(response.body, symbolize_names: true)
diff --git a/spec/models/user_job_spec.rb b/spec/models/user_job_spec.rb
index eb4ecd60f..3ae2de1fe 100644
--- a/spec/models/user_job_spec.rb
+++ b/spec/models/user_job_spec.rb
@@ -3,6 +3,6 @@
 require 'rails_helper'
 
 RSpec.describe UserJob do
-  it { is_expected.to belong_to(:good_job) }
+  it { is_expected.to validate_presence_of(:good_job_batch_id) }
   it { is_expected.to validate_presence_of(:user_id) }
 end

From b77a0d7cdfaf1803b4a01f5d442e46105ba6b22a Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Fri, 26 Sep 2025 14:29:25 +0100
Subject: [PATCH 14/19] Database migration and schema change to support UserJob
 change.

---
 db/migrate/20250925135238_change_user_jobs_to_batch_id.rb | 6 ++++++
 db/schema.rb                                              | 6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20250925135238_change_user_jobs_to_batch_id.rb

diff --git a/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb b/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb
new file mode 100644
index 000000000..691fc5157
--- /dev/null
+++ b/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb
@@ -0,0 +1,6 @@
+class ChangeUserJobsToBatchId < ActiveRecord::Migration[7.2]
+  def change
+    add_column :user_jobs, :good_job_batch_id, :uuid
+    remove_column :user_jobs, :good_job_id, :uuid
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index be56697f3..e3df5c1a0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2025_08_25_102042) do
+ActiveRecord::Schema[7.2].define(version: 2025_09_25_135238) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -289,10 +289,9 @@
 
   create_table "user_jobs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
     t.uuid "user_id", null: false
-    t.uuid "good_job_id", null: false
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.index ["user_id", "good_job_id"], name: "index_user_jobs_on_user_id_and_good_job_id", unique: true
+    t.uuid "good_job_batch_id"
   end
 
   create_table "versions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
@@ -324,5 +323,4 @@
   add_foreign_key "school_projects", "projects"
   add_foreign_key "school_projects", "schools"
   add_foreign_key "teacher_invitations", "schools"
-  add_foreign_key "user_jobs", "good_jobs"
 end

From c83d8401b06727c02c525b17fc537771618c7718 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Mon, 29 Sep 2025 16:14:34 +0100
Subject: [PATCH 15/19] Refactor decrypt_students and use it for validation and
 creation.

We were only decrypting students at validation and not creation. Fix.
---
 app/jobs/create_students_job.rb               | 9 +--------
 lib/concepts/school_student/validate_batch.rb | 8 +-------
 lib/helpers/student_helpers.rb                | 6 ++++++
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/app/jobs/create_students_job.rb b/app/jobs/create_students_job.rb
index 2d7e3f551..18f14c386 100644
--- a/app/jobs/create_students_job.rb
+++ b/app/jobs/create_students_job.rb
@@ -31,14 +31,7 @@ def self.attempt_perform_later(school_id:, students:, token:)
   end
 
   def perform(school_id:, students:, token:)
-    decrypted_students = students.map do |student|
-      {
-        name: student[:name],
-        username: student[:username],
-        password: DecryptionHelpers.decrypt_password(student[:password])
-      }
-    end
-
+    decrypted_students = StudentHelpers.decrypt_students(students)
     responses = ProfileApiClient.create_school_students(token:, students: decrypted_students, school_id:)
     return if responses[:created].blank?
 
diff --git a/lib/concepts/school_student/validate_batch.rb b/lib/concepts/school_student/validate_batch.rb
index ba22c34e2..bb768c3cf 100644
--- a/lib/concepts/school_student/validate_batch.rb
+++ b/lib/concepts/school_student/validate_batch.rb
@@ -32,18 +32,12 @@ def call(school:, students:, token:)
       private
 
       def validate_batch(school:, students:, token:)
-        decrypted_students = decrypt_students(students)
+        decrypted_students = StudentHelpers.decrypt_students(students)
         ProfileApiClient.validate_school_students(token:, students: decrypted_students, school_id: school.id)
       rescue ProfileApiClient::Student422Error => e
         handle_student422_error(e.errors)
       end
 
-      def decrypt_students(students)
-        students.deep_dup.each do |student|
-          student[:password] = DecryptionHelpers.decrypt_password(student[:password]) if student[:password].present?
-        end
-      end
-
       # This method converts the error structure returned by Profile (an array of error objects) to
       # the structure expected by the React front-end, which is a hash with the structure:
       #
diff --git a/lib/helpers/student_helpers.rb b/lib/helpers/student_helpers.rb
index 982b98bee..62004ef3f 100644
--- a/lib/helpers/student_helpers.rb
+++ b/lib/helpers/student_helpers.rb
@@ -7,4 +7,10 @@ def self.normalise_nil_values_to_empty_strings(students)
       student.transform_values { |value| value.nil? ? '' : value }
     end
   end
+
+  def self.decrypt_students(students)
+    students.deep_dup.each do |student|
+      student[:password] = DecryptionHelpers.decrypt_password(student[:password]) if student[:password].present?
+    end
+  end
 end

From 5204b95275c094fc974d6673d7523cae9882954b Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Mon, 29 Sep 2025 16:32:52 +0100
Subject: [PATCH 16/19] Test that we are only using 1 worker for GoodJob user
 creation.

---
 config/initializers/good_job.rb         |  2 ++
 spec/configuration/rails_config_spec.rb | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 spec/configuration/rails_config_spec.rb

diff --git a/config/initializers/good_job.rb b/config/initializers/good_job.rb
index 063611605..a1f84cbe4 100644
--- a/config/initializers/good_job.rb
+++ b/config/initializers/good_job.rb
@@ -16,5 +16,7 @@ def authenticate_admin
 
 Rails.application.configure do
   # The create_students_job queue is a serial queue that allows only one job at a time.
+  # DO NOT change the the value of create_students_job:1 without understanding the implications
+  # of processing more than one user creation job at once.:
   config.good_job.queues = 'create_students_job:1;default:5'
 end
diff --git a/spec/configuration/rails_config_spec.rb b/spec/configuration/rails_config_spec.rb
new file mode 100644
index 000000000..f77bb2c27
--- /dev/null
+++ b/spec/configuration/rails_config_spec.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Rails do
+  it 'ensures the create_students_job queue only has one worker' do
+    queues_config = described_class.application.config.good_job.queues
+    # queues_config is a string like "create_students_job:1;default:5"
+
+    # Parse the queues into a hash
+    parsed_queues = queues_config.split(';').to_h do |q|
+      name, concurrency = q.split(':')
+      [name, concurrency.to_i]
+    end
+
+    expect(parsed_queues['create_students_job']).to eq(1)
+  end
+end

From e4f2cabaffff2b5e3b0bde3033f9e8022dc5dfe7 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Tue, 7 Oct 2025 13:34:34 +0100
Subject: [PATCH 17/19] Correct spellings and wording in comments and error
 messages.

---
 app/controllers/api/school_students_controller.rb | 2 +-
 config/initializers/good_job.rb                   | 4 ++--
 lib/concepts/school_student/validate_batch.rb     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/controllers/api/school_students_controller.rb b/app/controllers/api/school_students_controller.rb
index 6f9d3295e..7045c1493 100644
--- a/app/controllers/api/school_students_controller.rb
+++ b/app/controllers/api/school_students_controller.rb
@@ -47,7 +47,7 @@ def create_batch
 
       students = StudentHelpers.normalise_nil_values_to_empty_strings(school_students_params)
 
-      # We validate the entire batch here in one go and then, if the validation succeds,
+      # We validate the entire batch here in one go and then, if the validation succeeds,
       # feed the batch to Profile in chunks of 50.
       validation_result = SchoolStudent::ValidateBatch.call(
         school: @school, students: students, token: current_user.token
diff --git a/config/initializers/good_job.rb b/config/initializers/good_job.rb
index a1f84cbe4..65d1b301e 100644
--- a/config/initializers/good_job.rb
+++ b/config/initializers/good_job.rb
@@ -16,7 +16,7 @@ def authenticate_admin
 
 Rails.application.configure do
   # The create_students_job queue is a serial queue that allows only one job at a time.
-  # DO NOT change the the value of create_students_job:1 without understanding the implications
-  # of processing more than one user creation job at once.:
+  # DO NOT change the value of create_students_job:1 without understanding the implications
+  # of processing more than one user creation job at once.
   config.good_job.queues = 'create_students_job:1;default:5'
 end
diff --git a/lib/concepts/school_student/validate_batch.rb b/lib/concepts/school_student/validate_batch.rb
index bb768c3cf..09b51c928 100644
--- a/lib/concepts/school_student/validate_batch.rb
+++ b/lib/concepts/school_student/validate_batch.rb
@@ -24,7 +24,7 @@ def call(school:, students:, token:)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
-        response[:error] = "Error creating school students: #{e}"
+        response[:error] = "Error validating school students: #{e}"
         response[:error_type] = :standard_error
         response
       end

From 7d8d74541a38b328946569420fd66408a8adba14 Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Tue, 7 Oct 2025 14:37:31 +0100
Subject: [PATCH 18/19] Provide tests for ValidateBatch class.

This test suite checks that the ValidateBatch class correctly handles
valid students, invalid batches and other unprocessable types of
uploads, such as passwords that are not correctly encrypted.
---
 .../school_student/validate_batch_spec.rb     | 144 ++++++++++++++++++
 1 file changed, 144 insertions(+)
 create mode 100644 spec/concepts/school_student/validate_batch_spec.rb

diff --git a/spec/concepts/school_student/validate_batch_spec.rb b/spec/concepts/school_student/validate_batch_spec.rb
new file mode 100644
index 000000000..a45af5978
--- /dev/null
+++ b/spec/concepts/school_student/validate_batch_spec.rb
@@ -0,0 +1,144 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe SchoolStudent::ValidateBatch do
+  let(:school) { create(:verified_school) }
+  let(:token) { UserProfileMock::TOKEN }
+
+  context 'when all students are valid' do
+    let(:valid_students) do
+      [
+        {
+          username: 'student1',
+          password: 'SaoXlDBAyiAFoMH3VsddhdA7JWnM8P8by1wOjBUWH2g=',
+          name: 'Student One'
+        },
+        {
+          username: 'student2',
+          password: 'SaoXlDBAyiAFoMH3VsddhdA7JWnM8P8by1wOjBUWH2g=',
+          name: 'Student Two'
+        }
+      ]
+    end
+
+    before do
+      allow(ProfileApiClient).to receive(:validate_school_students).and_return(OperationResponse.new)
+    end
+
+    it 'does not raise an error' do
+      expect do
+        described_class.call(school: school, students: valid_students, token: token)
+      end.not_to raise_error
+    end
+
+    it 'returns a successful operation response' do
+      result = described_class.call(school: school, students: valid_students, token: token)
+      expect(result).to be_success
+    end
+
+    it 'does not include any errors in the response' do
+      result = described_class.call(school: school, students: valid_students, token: token)
+      expect(result[:error]).to be_nil
+      expect(result[:error_type]).to be_nil
+    end
+  end
+
+  context 'when some students are invalid' do
+    let(:invalid_students) do
+      [
+        {
+          username: 'johndoe',
+          password: 'SaoXlDBAyiAFoMH3VsddhdA7JWnM8P8by1wOjBUWH2g=',
+          name: 'John Doe'
+        },
+        {
+          username: 'janedoe',
+          password: 'SaoXlDBAyiAFoMH3VsddhdA7JWnM8P8by1wOjBUWH2g=',
+          name: 'jane doe'
+        }
+      ]
+    end
+
+    let(:expected_errors) do
+      {
+        'johndoe' => ['doesNotEqualUsername'],
+        'janedoe' => ['isRequired']
+      }
+    end
+
+    let(:error_response) do
+      ProfileApiClient::Student422Error.new([{
+                                              'username' => 'johndoe',
+                                              'errorCode' => 'doesNotEqualUsername',
+                                              'message' => 'Password canot match username',
+                                              'location' => 'body'
+                                            },
+                                             {
+                                               'username' => 'janedoe',
+                                               'errorCode' => 'isRequired',
+                                               'message' => 'Username is required',
+                                               'location' => 'body'
+                                             }])
+    end
+
+    before do
+      allow(ProfileApiClient).to receive(:validate_school_students).and_raise(error_response)
+    end
+
+    it 'does not raise an error' do
+      expect do
+        described_class.call(school: school, students: invalid_students, token: token)
+      end.not_to raise_error
+    end
+
+    it 'returns an operation response without success' do
+      result = described_class.call(school: school, students: invalid_students, token: token)
+      expect(result).to be_failure
+    end
+
+    it 'returns an error type of :validation_error' do
+      result = described_class.call(school: school, students: invalid_students, token: token)
+      expect(result[:error_type]).to eq(:validation_error)
+    end
+
+    it 'includes the validation errors in the response' do
+      result = described_class.call(school: school, students: invalid_students, token: token)
+      expect(result[:error]).to be_present
+      expect(result[:error]).to eq(expected_errors)
+    end
+  end
+
+  context 'when passwords are not encrypted' do
+    let(:unencrypted_students) do
+      [
+        {
+          username: 'student1',
+          password: 'PlainTextPassword',
+          name: 'Student One'
+        }
+      ]
+    end
+
+    it 'does not raise an error' do
+      expect do
+        described_class.call(school: school, students: unencrypted_students, token: token)
+      end.not_to raise_error
+    end
+
+    it 'returns an operation response without success' do
+      result = described_class.call(school: school, students: unencrypted_students, token: token)
+      expect(result).to be_failure
+    end
+
+    it 'returns an error type of :standard_error' do
+      result = described_class.call(school: school, students: unencrypted_students, token: token)
+      expect(result[:error_type]).to eq(:standard_error)
+    end
+
+    it 'includes a suitable error message in the response' do
+      result = described_class.call(school: school, students: unencrypted_students, token: token)
+      expect(result[:error]).to include('Decryption failed')
+    end
+  end
+end

From 0eb4a08c725da262d4965c633dd24a3c8134321f Mon Sep 17 00:00:00 2001
From: Fraser Speirs <fraser@speirs.org>
Date: Tue, 7 Oct 2025 14:39:47 +0100
Subject: [PATCH 19/19] Reverse the removal of the good_job_id column from the
 user_jobs table.

This should allow existing jobs to continue working until completed.
---
 db/migrate/20250925135238_change_user_jobs_to_batch_id.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb b/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb
index 691fc5157..eafa6d5f1 100644
--- a/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb
+++ b/db/migrate/20250925135238_change_user_jobs_to_batch_id.rb
@@ -1,6 +1,5 @@
 class ChangeUserJobsToBatchId < ActiveRecord::Migration[7.2]
   def change
     add_column :user_jobs, :good_job_batch_id, :uuid
-    remove_column :user_jobs, :good_job_id, :uuid
   end
 end