Implement API Endpoints for bulk-import of Schools #622

fspeirs · 2025-11-20T16:27:05Z

Status

Related to https://github.com/RaspberryPiFoundation/experience-cs/issues/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 ImportSchoolsJobs.

Steps to perform after deploying to production

There is a database migration involved in this to add the table that tracks SchoolImportResult.

Tasks Outstanding

Improve consistency of naming (I have used import_schools in some places and school_import in others).
Check that we validate all of the data correctly
Stress-test with some very large imports to get a sense of upper bounds.
Write a tool to make it easy to get an editor-api OAuth token
Merge the branch that adds UI in the /admin path
Remove the wrapper scripts in favour of web-based UI
Remove test data commit.
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.

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.

^{Written by Cursor Bugbot for commit 6eacf9a. This will update automatically on new commits. Configure here.}

lib/concepts/school/operations/import_batch.rb

cursor

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on December 11

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

app/jobs/school_import_job.rb

cursor · 2025-11-24T12:07:19Z

app/jobs/school_import_job.rb

+        school.verify!
+
+        # Create owner role
+        Role.owner.create!(school_id: school.id, user_id: owner[:id])


Bug: Owner role creation fails for cross-school users

Creating an owner role will fail if the user already has any role in a different school due to the users_can_only_have_roles_in_one_school validation in the Role model. The import job doesn't check for existing roles in other schools before attempting to create the owner role, causing legitimate imports to fail when users are teachers or students elsewhere.

This is supposed to be a failure condition, but all of this is wrapped in a transaction, so it's atomic.

docs/import/large_import_emails.txt

db/schema.rb

lib/user_info_api_client.rb

app/controllers/api/school_import_jobs_controller.rb

app/jobs/school_import_job.rb

app/controllers/admin/school_import_results_controller.rb

app/fields/status_field.rb

danhalson

Looks great, but there are a few things I've spotted that are worth addressing and/or discussing

danhalson · 2025-12-02T16:33:58Z

app/views/admin/school_import_results/new.html.erb

+      <%= f.file_field :csv_file, accept: ".csv", required: true, class: "field-unit__field" %>
+      <p class="field-unit__hint">
+        Select a CSV file containing school data to import. 
+        Download the <%= link_to "CSV template", "/docs/import/school_import_template.csv" %> to see the required format.


is this the correct path? I tried it out and it wasn't working

Fixed in c05a9d1

danhalson · 2025-12-02T17:41:53Z

app/controllers/api/school_import_jobs_controller.rb

+        return
+      end
+
+      render json: build_job_response(job), status: :ok


we typically use jbuilder for json responses, was there a reason not to use it here?

Skill issue on my part - thanks for pointing out. Have fixed.

Fixed in ec078bf

danhalson · 2025-12-03T08:29:40Z

app/models/ability.rb

+  def define_editor_admin_abilities(user)
+    return unless user&.admin?
+
+    can :import, School


I'd be interested to know your opinion, but I'm thinking it could be beneficial to add a method define_school_import_abilities or maybe can_peform_a_school_import, with the school_import abilities grouped and then reference it from both methods here, just to help keep related abilities together

Yeah, I agree with you there. define_editor_admin_abilities is new in this PR but I've kept it as a possible future extension point and makes it clearer that the two kinds of admin are defined separately.

Addressed in 58b7bad

danhalson · 2025-12-03T09:24:20Z

app/jobs/school_import_job.rb

+    end
+
+    # Check if this owner already has a school as creator
+    existing_school = School.find_by(creator_id: owner[:id])


I think this may also need to take into account if they're a teacher at a different school too, so I'd be inclined to just check for any roles here...the only permissible situation would be granting them an owner role at the same school they're already a teacher

And if they're already a teacher in that school, then the school must already exist....

danhalson · 2025-12-03T09:27:38Z

app/jobs/school_import_job.rb

+        school.verify!
+
+        # Create owner role
+        Role.owner.create!(school_id: school.id, user_id: owner[:id])


Just in case you're not aware, we assign both owner and teacher roles during verification as the owner role alone is quite restrictive and doesn't provide access to many basic things, it's not a super user with access to all classes etc. If you've already looked at this and are happy with those restrictions, that's fine, I just want to make sure you're aware if not.

Role definitions have been discussed quite a bit, and we will be working on them further down the line.

So I actually realised that I was duplicating SchoolVerificationService in SchoolImportJob, so I refactored SchoolImportJob to use the verification service directly instead of calling school.verify! and setting roles on the owner.

I addressed this in 26b48f4

danhalson · 2025-12-03T10:14:16Z

app/jobs/school_import_job.rb

+    Sentry.capture_exception(e)
+  end
+
+  def find_owner(email)


could we rename this to find_user_account (or something like that) since it's really just the user_account we're looking for, but this confused me a bit with the check we're doing on the role

Addressed in 6ea9cfc

danhalson · 2025-12-03T10:24:31Z

app/controllers/api/schools_controller.rb

+      )
+
+      if result.success?
+        render json: {


it would be good to use jbuilder here

danhalson · 2025-12-03T11:40:18Z

lib/concepts/school/operations/import_batch.rb

+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response ||= OperationResponse.new


is instantiating OperationResponse again here just to be defensive? maybe I'm missing something, but it seems unlikely to be necessary

Honestly, I did this because either bugbot or Copilot (I forget which, but suspect bugbot) kept insisting that it might constitute a use-before-initialise. I'll pull that out.

Addressed in fd38d22

danhalson · 2025-12-03T11:45:28Z

lib/concepts/school/operations/import_batch.rb

+      end
+
+      def valid_headers?(headers)
+        required = %i[name website address_line_1 municipality country_code owner_email]


We could define this as a constant since it's referenced multiple times, and could also use it in the csv_invalid_format error above

danhalson · 2025-12-03T12:05:57Z

lib/user_info_api_client.rb

      end
    end
+
+    def stubbed_user_by_email(email)


I realise this is following prior art, but I don't feel like it was right in the first place to be stubbing here...would be better to put these into spec/support or consider using FactoryBot, though I realise the latter has wider implications in terms of env scoping (we don't currently have a staging env_type)

Addressed in 2b1e126 (some tests changed or were deleted as a result of this change).

We allow `editor-admin` and `experience-cs-admin` users to import schools and monitor the state of the resulting jobs.

This change adds a search_by_email method to allow lookup of users by email address to acquire the ID of proposed school owners.

This commit introduces a SchoolImportJob type of GoodJob job to allow for enqueueing imports and tracking their asynchronous completion. The controller will return to the caller a Job ID which can be used to track the progress of these jobs and retrieve their results when done. The SchoolImportResult class tracks the job and provides access to the results. There are structured error codes for possible issues in SchoolImportError.

This operation parses and validates the CSV and reports on errors or enqueues the job.

This commit modifies SchoolsController to add the endpoint for importing schools. It introduces SchoolImportJobsController as the controller for the endpoint though which clients can track job status.

These tests cover the core functionality and some additional complex edge cases.

This commit adds new UI in /admin to allow users to upload CSVs of schools and track the results of the upload jobs in the UI. They can also download CSV files of the resulting school structures. Add index, show and new views for Administrate Add SchoolImportResultDashboard Add routes for admin interface for school_import_results Add SchoolImportResultsController to show results in the UI. This commit also adds code to new.html.erb to show validation errors if the uploaded CSV can't be enqueued as a batch.

Only one Job is created per job, but an upload which is failed or retried could result in more than one execution.

…stead of duplicating.

fspeirs · 2025-12-04T10:25:08Z

lib/concepts/school/operations/import_batch.rb

+        response
+      rescue StandardError => e
+        Sentry.capture_exception(e)
+        response[:error] = SchoolImportError.format_error(:unknown_error, e.message)


@danhalson Here's where I was being pestered about the reinitialisation of response - I agree that it seems unlikely to be a problem.

… tests.

fspeirs · 2025-12-04T10:26:03Z

@danhalson Thanks for the comprehensive and careful review. I have left your comments unresolved but added links to the commits where I addressed your points. Hopefully that makes it easy to track in re-review?

cla-bot bot added the cla-signed label Nov 20, 2025

github-code-quality bot found potential problems Nov 20, 2025

View reviewed changes

lib/concepts/school/operations/import_batch.rb Fixed Show fixed Hide fixed

fspeirs force-pushed the fs-implement-school-import-endpoint branch 2 times, most recently from 3de1b35 to 68a1e78 Compare November 21, 2025 08:13

fspeirs self-assigned this Nov 22, 2025

fspeirs force-pushed the fs-implement-school-import-endpoint branch 2 times, most recently from 2f964a2 to b613fb8 Compare November 24, 2025 11:12

fspeirs marked this pull request as ready for review November 24, 2025 11:12

fspeirs force-pushed the fs-implement-school-import-endpoint branch from b613fb8 to c9e400a Compare November 24, 2025 11:14

raspberrypiherokubot temporarily deployed to editor-api-p-fs-impleme-nqxegq November 24, 2025 11:14 Inactive

cursor bot reviewed Nov 24, 2025

View reviewed changes

app/jobs/school_import_job.rb Outdated Show resolved Hide resolved

fspeirs force-pushed the fs-implement-school-import-endpoint branch from c9e400a to 1cbaf58 Compare November 24, 2025 11:40

cursor bot reviewed Nov 24, 2025

View reviewed changes

app/jobs/school_import_job.rb Outdated Show resolved Hide resolved

fspeirs force-pushed the fs-implement-school-import-endpoint branch 2 times, most recently from d0a84c3 to 90e1a35 Compare November 24, 2025 12:02

cursor bot reviewed Nov 24, 2025

View reviewed changes

adrian-rpf requested review from danhalson and jamiebenstead November 24, 2025 13:50

adrian-rpf added the editor-api label Nov 27, 2025

fspeirs force-pushed the fs-implement-school-import-endpoint branch from 90e1a35 to 48f5d00 Compare December 1, 2025 10:06

cursor bot reviewed Dec 1, 2025

View reviewed changes

db/schema.rb Outdated Show resolved Hide resolved

lib/user_info_api_client.rb Outdated Show resolved Hide resolved

fspeirs force-pushed the fs-implement-school-import-endpoint branch 2 times, most recently from 4096e1d to 3681a33 Compare December 2, 2025 11:11

cursor bot reviewed Dec 2, 2025

View reviewed changes

lib/user_info_api_client.rb Outdated Show resolved Hide resolved

app/controllers/api/school_import_jobs_controller.rb Outdated Show resolved Hide resolved

fspeirs force-pushed the fs-implement-school-import-endpoint branch 2 times, most recently from 62bb9ea to 166509b Compare December 2, 2025 13:52

cursor bot reviewed Dec 2, 2025

View reviewed changes

app/jobs/school_import_job.rb Outdated Show resolved Hide resolved

fspeirs force-pushed the fs-implement-school-import-endpoint branch 2 times, most recently from 36f73e2 to 3604789 Compare December 2, 2025 14:16

cursor bot reviewed Dec 2, 2025

View reviewed changes

app/controllers/admin/school_import_results_controller.rb Outdated Show resolved Hide resolved

cursor bot reviewed Dec 2, 2025

View reviewed changes

app/fields/status_field.rb Outdated Show resolved Hide resolved

fspeirs force-pushed the fs-implement-school-import-endpoint branch 3 times, most recently from 86d1fda to 6eacf9a Compare December 2, 2025 15:24

danhalson requested changes Dec 3, 2025

View reviewed changes

fspeirs force-pushed the fs-implement-school-import-endpoint branch from f3f67bc to 848ab06 Compare December 4, 2025 10:08

raspberrypiherokubot temporarily deployed to editor-api-p-fs-impleme-whr8un December 4, 2025 10:08 Inactive

fspeirs added 18 commits December 4, 2025 10:08

Add CanCan ability and routes for import schools endpoints.

df7d6a6

We allow `editor-admin` and `experience-cs-admin` users to import schools and monitor the state of the resulting jobs.

Update UserInfoApiClient to allow user lookup by email.

bfebcc5

This change adds a search_by_email method to allow lookup of users by email address to acquire the ID of proposed school owners.

Add School::ImportBatch operation to import a CSV of schools.

4bd694a

This operation parses and validates the CSV and reports on errors or enqueues the job.

Update/Add controllers for school import and job tracking.

0ee161e

This commit modifies SchoolsController to add the endpoint for importing schools. It introduces SchoolImportJobsController as the controller for the endpoint though which clients can track job status.

Test coverage for CSV school import.

eada195

These tests cover the core functionality and some additional complex edge cases.

Add documentation - CSM guide and example CSV template

4276a45

Change from GoodJob::Execution to GoodJob::Job for upload tracking.

2755e18

Only one Job is created per job, but an upload which is failed or retried could result in more than one execution.

Number of small Rubocop fixings and refactorings.

e0e7f26

Modify SchoolImportJobsController to use jbuilder for JSON response.

ec078bf

Modify SchoolsController to use jbuilder

654fa25

Remove redundant reinitialisation of OperationResponse.

fd38d22

Make the array of required school CSV fields a constant.

d43b7c3

Modify check on owner to fail if owner has any role in any other school.

d767187

Consolidate school import abilities into one method

58b7bad

Refactor SchoolImportJob to use SchoolVerificationService directly in…

26b48f4

…stead of duplicating.

Rename find_owner method to find_user_account_for_proposed_owner

6ea9cfc

github-code-quality bot found potential problems Dec 4, 2025

View reviewed changes

fspeirs added 2 commits December 4, 2025 10:16

Refactor the stubbed_users methods out of UserInfoApiClient; refactor…

2b1e126

… tests.

Update path to CSV template file.

c05a9d1

fspeirs force-pushed the fs-implement-school-import-endpoint branch from 848ab06 to c05a9d1 Compare December 4, 2025 10:16

Implement API Endpoints for bulk-import of Schools #622

Are you sure you want to change the base?

Implement API Endpoints for bulk-import of Schools #622

Uh oh!

Conversation

fspeirs commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status

Points for consideration:

What's changed?

Steps to perform after deploying to production

Tasks Outstanding

Follow-On Work

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This is the final PR Bugbot will review for you during this billing cycle

Uh oh!

Uh oh!

Uh oh!

cursor bot Nov 24, 2025

Choose a reason for hiding this comment

Bug: Owner role creation fails for cross-school users

Uh oh!

fspeirs Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danhalson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fspeirs commented Nov 20, 2025 •

edited

Loading

fspeirs Nov 24, 2025 •

edited

Loading