Skip to content

Commit 1f96b90

Browse files
authored
Implement API Endpoints for bulk-import of Schools (#622)
## Status - Related to RaspberryPiFoundation/experience-cs#1309 ## Points for consideration: - 🧠 Review - Reviewers may find it easier to review commit-by-commit - ❓ Exclude - do we want to keep the wrapper scripts and test data? - 📖 Documentation - should we migrate the documentation for CSMs out of here? - 🔒 Security - calling these endpoints should only be possible for users with `editor-admin` or `experience-cs-admin` roles. All other roles and none should return `401`. - 🐌 Performance - The schools are created asynchronously through GoodJob, so immediate performance of the endpoints should not be an issue, however this code does make an N+1 query to `userinfo` to look up school owners by email - there is no bulk option for this. ## What's changed? - Introduction of two new endpoints to allow import and validation of schools via CSV files: - `/api/schools/import` - for POSTing CSV - `/api/school_import_jobs` - for tracking job status - Both endpoints require a user to have the `editor-admin` or `experience-cs-admin` roles. - An `ImportSchoolsJob` GoodJob job type that will execute the import asynchronously - A `SchoolImportResult` record type that tracks the results of `ImportSchoolsJob`s. ## Steps to perform after deploying to production There is a database migration involved in this to add the table that tracks `SchoolImportResult`. ## Tasks Outstanding - [x] Improve consistency of naming (I have used `import_schools` in some places and `school_import` in others). - [x] Check that we validate all of the data correctly - [x] Stress-test with some very large imports to get a sense of upper bounds. - [x] Write a tool to make it easy to get an `editor-api` OAuth token - [x] Merge the branch that adds UI in the `/admin` path - [x] Remove the wrapper scripts in favour of web-based UI - [x] Remove test data commit. - [x] Rewrite the CSM guide to refer to the new Admin UI. ## Follow-On Work ~This is not the final form of this feature. In a follow-up task in the next sprint we intend to add a UI under the `/admin` route to make it easier for non-developers to access this endpoint. The goal in the first instance is to develop and test the endpoints separately from the UI that accesses them.~ This PR now contains all the work necessary to make a complete feature, including web-based UI. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds CSV-based bulk school import with admin UI, async GoodJob processing, job status API, and persisted results (with CSV export), restricted to editor/experience CS admins. > > - **API/Services**: > - `POST /api/schools/import` to start imports; `GET /api/school_import_jobs/:id` to track status. > - CSV parsing/validation via `School::ImportBatch` with structured `SchoolImportError` codes. > - **Background Processing**: > - `SchoolImportJob` (queue: `import_schools_job`) creates/verifies schools, assigns owner role, and saves results to `SchoolImportResult`. > - GoodJob queues updated in `config/initializers/good_job.rb`. > - **Admin UI**: > - New `admin/school_import_results` pages (index/show/new) for upload, status, results, and CSV download. > - Custom Administrate fields: `StatusField`, `ResultsSummaryField`, `UserInfoField` with batched user lookups. > - **Data Model**: > - New `SchoolImportResult` model/table (+ migration) and dashboard. > - **Authorization**: > - CanCan abilities grant `School#import` and `:school_import_job` read to editor/experience CS admins. > - **Utilities/Config**: > - `UserInfoApiClient` adds `find_user_by_email` and improved transforms. > - Locale for missing CSV error; routes wired for admin/API. > - **Docs/Tests**: > - CSM guide and CSV template added under `docs/import/`. > - Specs for import service, job, API endpoints, and admin pages. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6eacf9a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 2a6ab27 commit 1f96b90

38 files changed

+2018
-22
lines changed
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# frozen_string_literal: true
2+
3+
require 'csv'
4+
5+
module Admin
6+
class SchoolImportResultsController < Admin::ApplicationController
7+
def index
8+
search_term = params[:search].to_s.strip
9+
resources = Administrate::Search.new(
10+
SchoolImportResult.all,
11+
dashboard_class,
12+
search_term
13+
).run
14+
resources = apply_collection_includes(resources)
15+
resources = order.apply(resources)
16+
resources = resources.page(params[:_page]).per(records_per_page)
17+
18+
# Batch load user info to avoid N+1 queries
19+
user_ids = resources.filter_map(&:user_id).uniq
20+
RequestStore.store[:user_info_cache] = fetch_users_batch(user_ids)
21+
22+
page = Administrate::Page::Collection.new(dashboard, order: order)
23+
24+
render locals: {
25+
resources: resources,
26+
search_term: search_term,
27+
page: page,
28+
show_search_bar: show_search_bar?
29+
}
30+
end
31+
32+
def show
33+
respond_to do |format|
34+
format.html do
35+
render locals: {
36+
page: Administrate::Page::Show.new(dashboard, requested_resource)
37+
}
38+
end
39+
format.csv do
40+
send_data generate_csv(requested_resource),
41+
filename: "school_import_#{requested_resource.job_id}_#{Date.current.strftime('%Y-%m-%d')}.csv",
42+
type: 'text/csv'
43+
end
44+
end
45+
end
46+
47+
def new
48+
@error_details = flash[:error_details]
49+
render locals: {
50+
page: Administrate::Page::Form.new(dashboard, SchoolImportResult.new)
51+
}
52+
end
53+
54+
def create
55+
if params[:csv_file].blank?
56+
flash[:error] = I18n.t('errors.admin.csv_file_required')
57+
redirect_to new_admin_school_import_result_path
58+
return
59+
end
60+
61+
# Call the same service that the API endpoint uses, ensuring all validation is applied
62+
result = School::ImportBatch.call(
63+
csv_file: params[:csv_file],
64+
current_user: current_user
65+
)
66+
67+
if result.success?
68+
flash[:notice] = "Import job started successfully. Job ID: #{result[:job_id]}"
69+
redirect_to admin_school_import_results_path
70+
else
71+
# Display error inline on the page
72+
flash.now[:error] = format_error_message(result[:error])
73+
@error_details = extract_error_details(result[:error])
74+
render :new, locals: {
75+
page: Administrate::Page::Form.new(dashboard, SchoolImportResult.new)
76+
}
77+
end
78+
end
79+
80+
private
81+
82+
def default_sorting_attribute
83+
:created_at
84+
end
85+
86+
def default_sorting_direction
87+
:desc
88+
end
89+
90+
def format_error_message(error)
91+
return error.to_s unless error.is_a?(Hash)
92+
93+
error[:message] || error['message'] || 'Import failed'
94+
end
95+
96+
def extract_error_details(error)
97+
return nil unless error.is_a?(Hash)
98+
99+
error[:details] || error['details']
100+
end
101+
102+
def generate_csv(import_result)
103+
CSV.generate(headers: true) do |csv|
104+
# Header row
105+
csv << ['Status', 'School Name', 'School Code', 'School ID', 'Owner Email', 'Error Code', 'Error Message']
106+
107+
results = import_result.results
108+
successful = results['successful'] || []
109+
failed = results['failed'] || []
110+
111+
# Successful schools
112+
successful.each do |school|
113+
csv << [
114+
'Success',
115+
school['name'],
116+
school['code'],
117+
school['id'],
118+
school['owner_email'],
119+
'',
120+
''
121+
]
122+
end
123+
124+
# Failed schools
125+
failed.each do |school|
126+
csv << [
127+
'Failed',
128+
school['name'],
129+
'',
130+
'',
131+
school['owner_email'],
132+
school['error_code'],
133+
school['error']
134+
]
135+
end
136+
end
137+
end
138+
139+
def fetch_users_batch(user_ids)
140+
return {} if user_ids.empty?
141+
142+
users = UserInfoApiClient.fetch_by_ids(user_ids)
143+
return {} if users.nil?
144+
145+
users.index_by { |user| user[:id] }
146+
rescue StandardError => e
147+
Rails.logger.error("Failed to batch fetch user info: #{e.message}")
148+
{}
149+
end
150+
end
151+
end
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# frozen_string_literal: true
2+
3+
module Api
4+
class SchoolImportJobsController < ApiController
5+
before_action :authorize_user
6+
7+
def show
8+
authorize! :read, :school_import_job
9+
@job = find_job
10+
11+
if @job.nil?
12+
render json: SchoolImportError.format_error(:job_not_found, 'Job not found'), status: :not_found
13+
return
14+
end
15+
16+
@status = job_status(@job)
17+
@result = SchoolImportResult.find_by(job_id: @job.active_job_id) if @job.succeeded?
18+
render :show, formats: [:json], status: :ok
19+
end
20+
21+
private
22+
23+
def find_job
24+
job = GoodJob::Job.find_by(active_job_id: params[:id])
25+
26+
# Verify this is an import job (security check)
27+
return nil unless job && job.job_class == SchoolImportJob.name
28+
29+
job
30+
end
31+
32+
def job_status(job)
33+
return 'discarded' if job.discarded?
34+
return 'succeeded' if job.succeeded?
35+
return 'failed' if job_failed?(job)
36+
return 'running' if job.running?
37+
return 'scheduled' if job_scheduled?(job)
38+
39+
'queued'
40+
end
41+
42+
def job_failed?(job)
43+
job.finished? && job.error.present?
44+
end
45+
46+
def job_scheduled?(job)
47+
job.scheduled_at.present? && job.scheduled_at > Time.current
48+
end
49+
end
50+
end

app/controllers/api/schools_controller.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module Api
44
class SchoolsController < ApiController
55
before_action :authorize_user
66
load_and_authorize_resource
7+
skip_load_and_authorize_resource only: :import
78

89
def index
910
@schools = School.accessible_by(current_ability)
@@ -47,6 +48,29 @@ def destroy
4748
end
4849
end
4950

51+
def import
52+
authorize! :import, School
53+
54+
if params[:csv_file].blank?
55+
render json: { error: SchoolImportError.format_error(:csv_file_required, 'CSV file is required') },
56+
status: :unprocessable_entity
57+
return
58+
end
59+
60+
result = School::ImportBatch.call(
61+
csv_file: params[:csv_file],
62+
current_user: current_user
63+
)
64+
65+
if result.success?
66+
@job_id = result[:job_id]
67+
@total_schools = result[:total_schools]
68+
render :import, formats: [:json], status: :accepted
69+
else
70+
render json: { error: result[:error] }, status: :unprocessable_entity
71+
end
72+
end
73+
5074
private
5175

5276
def school_params
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# frozen_string_literal: true
2+
3+
require 'administrate/base_dashboard'
4+
5+
class SchoolImportResultDashboard < Administrate::BaseDashboard
6+
ATTRIBUTE_TYPES = {
7+
id: Field::String,
8+
job_id: StatusField,
9+
user_id: UserInfoField,
10+
results: ResultsSummaryField,
11+
created_at: Field::DateTime,
12+
updated_at: Field::DateTime
13+
}.freeze
14+
15+
COLLECTION_ATTRIBUTES = %i[
16+
job_id
17+
user_id
18+
results
19+
created_at
20+
].freeze
21+
22+
SHOW_PAGE_ATTRIBUTES = %i[
23+
id
24+
job_id
25+
user_id
26+
results
27+
created_at
28+
updated_at
29+
].freeze
30+
31+
FORM_ATTRIBUTES = [].freeze
32+
33+
COLLECTION_FILTERS = {}.freeze
34+
35+
def display_resource(school_import_result)
36+
"Import Job #{school_import_result.job_id}"
37+
end
38+
end
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# frozen_string_literal: true
2+
3+
require 'administrate/field/base'
4+
5+
class ResultsSummaryField < Administrate::Field::Base
6+
def to_s
7+
"#{successful_count} successful, #{failed_count} failed"
8+
end
9+
10+
def successful_count
11+
data['successful']&.count || 0
12+
end
13+
14+
def failed_count
15+
data['failed']&.count || 0
16+
end
17+
18+
def total_count
19+
successful_count + failed_count
20+
end
21+
end

app/fields/status_field.rb

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# frozen_string_literal: true
2+
3+
require 'administrate/field/base'
4+
5+
class StatusField < Administrate::Field::Base
6+
def to_s
7+
job_status
8+
end
9+
10+
def job_status
11+
return 'unknown' if data.blank?
12+
13+
job = GoodJob::Job.find_by(active_job_id: data)
14+
return 'not_found' unless job
15+
16+
determine_job_status(job)
17+
end
18+
19+
def status_class
20+
case job_status
21+
when 'succeeded', 'completed' then 'status-completed'
22+
when 'failed', 'discarded' then 'status-failed'
23+
when 'running' then 'status-running'
24+
when 'queued', 'scheduled' then 'status-queued'
25+
else 'status-unknown'
26+
end
27+
end
28+
29+
private
30+
31+
def determine_job_status(job)
32+
return 'discarded' if job.discarded?
33+
return 'succeeded' if job.succeeded?
34+
return 'failed' if job.finished? && job.error.present?
35+
return 'running' if job.running?
36+
return 'scheduled' if scheduled?(job)
37+
38+
'queued'
39+
end
40+
41+
def scheduled?(job)
42+
job.scheduled_at.present? && job.scheduled_at > Time.current
43+
end
44+
end

0 commit comments

Comments
 (0)