Enable Bulk User Creation for Larger Batches of Users

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
@@ -32,16 +36,65 @@ 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 = StudentHelpers.normalise_nil_values_to_empty_strings(school_students_params)
+
+      # 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
       )
 
-      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 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)
+      # 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
+        students.each_slice(MAX_BATCH_CREATION_SIZE) do |student_batch|
+          SchoolStudent::CreateBatch.call(
+            school: @school, school_students_params: student_batch, token: current_user.token
+          )
+        end
+      end
+
+      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
@@ -75,7 +128,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

app/controllers/api/user_jobs_controller.rb

@@ -5,37 +5,76 @@ 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
     end
 
     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

app/jobs/create_students_job.rb

@@ -24,37 +24,14 @@ 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
-  )
-
-  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
+  def self.attempt_perform_later(school_id:, students:, token:)
+    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?
 

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

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

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

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?

config/initializers/good_job.rb

@@ -13,3 +13,10 @@ 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.
+  # 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

db/migrate/20250925135238_change_user_jobs_to_batch_id.rb

@@ -0,0 +1,5 @@
+class ChangeUserJobsToBatchId < ActiveRecord::Migration[7.2]
+  def change
+    add_column :user_jobs, :good_job_batch_id, :uuid
+  end
+end

lib/concepts/school_student/create_batch.rb

@@ -3,26 +3,13 @@
 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
     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
-      rescue ValidationError => e
-        response[:error] = e.errors
-        response[:error_type] = :validation_error
+        response[:job_id] = create_batch(school, school_students_params, token)
         response
       rescue ConcurrencyExceededForSchool => e
         response[:error] = e
@@ -37,44 +24,10 @@ 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:)
+      def create_batch(school, students, token)
+        job = CreateStudentsJob.attempt_perform_later(school_id: school.id, students:, token:)
         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