Bulk student create (Backend) (#443)

closes #442

---------

Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com>
Co-authored-by: Dan Halson <[email protected]>

Author: create-issue-branch[bot]

.devcontainer/devcontainer.json

@@ -46,7 +46,6 @@
     "vscode": {
       "extensions": [
         "ms-azuretools.vscode-docker",
-        "ninoseki.vscode-gem-lens",
         "eamodio.gitlens",
         "github.vscode-pull-request-github",
         "wmaurer.change-case",
@@ -57,7 +56,6 @@
         "hashicorp.terraform",
         "yzhang.markdown-all-in-one",
         "mikestead.dotenv",
-        "wingrunr21.vscode-ruby",
         "ms-vscode.remote-repositories",
         "github.remotehub",
         "circleci.circleci",
@@ -68,7 +66,8 @@
         "codezombiech.gitignore",
         "shopify.ruby-lsp",
         "koichisasada.vscode-rdbg",
-        "rangav.vscode-thunder-client"
+        "rangav.vscode-thunder-client",
+        "ninoseki.vscode-mogami"
       ],
       "settings": {
         "terminal.integrated.defaultProfile.linux": "zsh"

.rubocop.yml

@@ -39,7 +39,7 @@ Metrics/BlockLength:
 Metrics/AbcSize:
   Enabled: false
 
-Metrics/LineLength:
+Layout/LineLength:
   Enabled: false
 
 Naming/VariableNumber:

Gemfile

@@ -15,7 +15,7 @@ gem 'email_validator'
 gem 'faraday'
 gem 'github_webhook', '~> 1.4'
 gem 'globalid'
-gem 'good_job', '~> 3.12'
+gem 'good_job', '~> 4.3'
 gem 'graphql'
 gem 'graphql-client'
 gem 'image_processing'

Gemfile.lock

@@ -162,7 +162,7 @@ GEM
     email_validator (2.2.4)
       activemodel
     erubi (1.13.0)
-    et-orbi (1.2.7)
+    et-orbi (1.2.11)
       tzinfo
     factory_bot (6.2.1)
       activesupport (>= 5.0.0)
@@ -176,22 +176,22 @@ GEM
       ruby2_keywords (>= 0.0.4)
     faraday-net_http (3.0.2)
     ffi (1.16.3)
-    fugit (1.8.1)
-      et-orbi (~> 1, >= 1.2.7)
+    fugit (1.11.1)
+      et-orbi (~> 1, >= 1.2.11)
       raabro (~> 1.4)
     github_webhook (1.4.2)
       activesupport (>= 4)
       rack (>= 1.3)
       railties (>= 4)
     globalid (1.1.0)
       activesupport (>= 5.0)
-    good_job (3.29.3)
-      activejob (>= 6.0.0)
-      activerecord (>= 6.0.0)
-      concurrent-ruby (>= 1.0.2)
-      fugit (>= 1.1)
-      railties (>= 6.0.0)
-      thor (>= 0.14.1)
+    good_job (4.3.0)
+      activejob (>= 6.1.0)
+      activerecord (>= 6.1.0)
+      concurrent-ruby (>= 1.3.1)
+      fugit (>= 1.11.0)
+      railties (>= 6.1.0)
+      thor (>= 1.0.0)
     graphiql-rails (1.9.0)
       railties
       sprockets-rails
@@ -533,7 +533,7 @@ DEPENDENCIES
   faraday
   github_webhook (~> 1.4)
   globalid
-  good_job (~> 3.12)
+  good_job (~> 4.3)
   graphiql-rails
   graphql
   graphql-client

app/controllers/api/school_students_controller.rb

@@ -20,7 +20,9 @@ def index
     end
 
     def create
-      result = SchoolStudent::Create.call(school: @school, school_student_params:, token: current_user.token)
+      result = SchoolStudent::Create.call(
+        school: @school, school_student_params:, token: current_user.token
+      )
 
       if result.success?
         head :no_content
@@ -30,7 +32,9 @@ def create
     end
 
     def create_batch
-      result = SchoolStudent::CreateBatch.call(school: @school, uploaded_file: params[:file], token: current_user.token)
+      result = SchoolStudent::CreateBatch.call(
+        school: @school, school_students_params:, token: current_user.token, user_id: current_user.id
+      )
 
       if result.success?
         head :no_content
@@ -67,6 +71,16 @@ def school_student_params
       params.require(:school_student).permit(:username, :password, :name)
     end
 
+    def school_students_params
+      school_students = params.require(:school_students)
+
+      school_students.map do |student|
+        next if student.blank?
+
+        student.permit(:username, :password, :name).to_h.with_indifferent_access
+      end
+    end
+
     def create_safeguarding_flags
       create_teacher_safeguarding_flag
       create_owner_safeguarding_flag

app/controllers/api/user_jobs_controller.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module Api
+  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
+      else
+        render json: { error: 'No jobs found' }, status: :not_found
+      end
+    end
+
+    def show
+      user_job = UserJob.find_by(job_id: params[:id], teacher_id: current_user.id)
+      job = job_attributes(user_job.good_job)
+      if job
+        render json: { job: }, status: :ok
+      else
+        render json: { error: 'Job not found' }, status: :not_found
+      end
+    end
+
+    private
+
+    def job_attributes(job)
+      {
+        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
+      }
+    end
+  end
+end

app/jobs/create_students_job.rb

@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+class ConcurrencyExceededForSchool < StandardError; end
+
+class CreateStudentsJob < ApplicationJob
+  include GoodJob::ActiveJobExtensions::Concurrency
+
+  queue_as :default
+
+  # 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
+  end
+
+  def perform(school_id:, students:, token:)
+    students = Array(students)
+
+    responses = ProfileApiClient.create_school_students(token:, students:, school_id:)
+    return if responses[:created].blank?
+
+    responses[:created].each do |user_id|
+      Role.student.create!(school_id:, user_id:)
+    end
+  end
+
+  # Don't retry...
+  rescue_from ConcurrencyExceededForSchool do |e|
+    Rails.logger.error "Only one job per school can be enqueued at a time: #{school_id}"
+    Sentry.capture_exception(e)
+    raise e
+  end
+
+  # Don't retry...
+  rescue_from ActiveRecord::RecordInvalid do |e|
+    Rails.logger.error "Failed to create student role: #{e.record.errors.full_messages.join(', ')}"
+    Sentry.capture_exception(e)
+    raise e
+  end
+
+  retry_on StandardError, attempts: 3 do |_job, e|
+    Sentry.capture_exception(e)
+    raise e
+  end
+end

app/models/user_job.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class UserJob < ApplicationRecord
+  belongs_to :good_job, class_name: 'GoodJob::Job'
+
+  attr_accessor :user
+end

config/environments/development.rb

@@ -70,6 +70,9 @@
   # Highlight code that enqueued background job in logs.
   config.active_job.verbose_enqueue_logs = true
 
+  # Use the async adapter for Active Job in development
+  config.active_job.queue_adapter = :good_job
+
   # Suppress logger output for asset requests.
   config.assets.quiet = true
 

config/initializers/awesome_print.rb

@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+require 'awesome_print' if Rails.env.development?

config/routes.rb

@@ -63,6 +63,8 @@
     resources :teacher_invitations, param: :token, only: :show do
       put :accept, on: :member
     end
+
+    resources :user_jobs, only: %i[index show]
   end
 
   resource :github_webhooks, only: :create, defaults: { formats: :json }

db/migrate/20241008171501_add_jobs_finished_at_to_good_job_batches.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class AddJobsFinishedAtToGoodJobBatches < ActiveRecord::Migration[7.1]
+  def change
+    reversible do |dir|
+      dir.up do
+        # Ensure this incremental update migration is idempotent
+        # with monolithic install migration.
+        return if connection.column_exists?(:good_job_batches, :jobs_finished_at)
+      end
+    end
+
+    change_table :good_job_batches do |t|
+      t.datetime :jobs_finished_at
+    end
+  end
+end

db/migrate/20241008171600_create_good_job_execution_duration.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class CreateGoodJobExecutionDuration < ActiveRecord::Migration[7.1]
+  def change
+    reversible do |dir|
+      dir.up do
+        # Ensure this incremental update migration is idempotent
+        # with monolithic install migration.
+        return if connection.column_exists?(:good_job_executions, :duration)
+      end
+    end
+
+    add_column :good_job_executions, :duration, :interval
+  end
+end

db/migrate/20241009082132_add_concurrency_key_to_good_job_executions.rb

@@ -0,0 +1,6 @@
+class AddConcurrencyKeyToGoodJobExecutions < ActiveRecord::Migration[6.0]
+  def change
+    add_column :good_job_executions, :concurrency_key, :string
+    add_index :good_job_executions, :concurrency_key
+  end
+end

db/migrate/20241014110435_create_user_jobs.rb

@@ -0,0 +1,13 @@
+class CreateUserJobs < ActiveRecord::Migration[7.1]
+  def change
+    create_table :user_jobs, id: :uuid do |t|
+      t.uuid :user_id, null: false, type: :uuid
+      t.uuid :good_job_id, null: false, type: :uuid
+
+      t.timestamps
+    end
+
+    add_foreign_key :user_jobs, :good_jobs, column: :good_job_id
+    add_index :user_jobs, [:user_id, :good_job_id], unique: true
+  end
+end