From 64fb019a0341e8a2363b71f59329d667eee0d4f7 Mon Sep 17 00:00:00 2001
From: Nathan Richards <nathan.richards@irysan.com>
Date: Tue, 22 Sep 2026 13:54:04 +0200
Subject: [PATCH 1/6] feat: add endpoint for the owner to cancel a pending
 ownership transfer

Any current owner can cancel a pending transfer, not just the one
who started it - cancel is just a third status transition alongside
accept/decline, so it slots into the existing resolve! helper with
no new resource-loading logic, which now wraps the read-and-update
in a transaction so its row lock is held across both steps (stopping
a concurrent accept/decline/cancel from also finding the transfer
pending) and responds only after commit, so a commit-time callback
failure surfaces as a clean 500 instead of racing an already-rendered
response.

The read ability for a pending transfer is likewise open to any
owner of the school, not just the one who started it, since any of
them may need to see it in order to decide whether to cancel it -
the status endpoint now identifies the viewer by their owner role
rather than by whether they happened to be the requester.
---
 ...nership_transfer_resolutions_controller.rb |  34 +++--
 .../api/ownership_transfers_controller.rb     |   6 +-
 app/models/ability.rb                         |   7 +-
 config/routes.rb                              |   1 +
 .../cancelling_an_ownership_transfer_spec.rb  | 131 ++++++++++++++++++
 .../viewing_ownership_transfer_status_spec.rb |  35 +++++
 6 files changed, 197 insertions(+), 17 deletions(-)
 create mode 100644 spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb

diff --git a/app/controllers/api/ownership_transfer_resolutions_controller.rb b/app/controllers/api/ownership_transfer_resolutions_controller.rb
index 2e483c5b1..67bd32feb 100644
--- a/app/controllers/api/ownership_transfer_resolutions_controller.rb
+++ b/app/controllers/api/ownership_transfer_resolutions_controller.rb
@@ -14,26 +14,38 @@ def decline
       resolve!(:rejected)
     end
 
+    def cancel
+      resolve!(:cancelled)
+    end
+
     private
 
     # Wrapped in a transaction so the row lock below is held across the
-    # read-and-update, preventing a concurrent accept/decline on the same
-    # transfer from also finding it pending. Authorizing against the loaded
+    # read-and-update, preventing a concurrent accept/decline/cancel on the
+    # same transfer from also finding it pending. Authorizing against the loaded
     # transfer (rather than checking cannot? by hand) means an unauthorized
     # attempt raises and is rescued below into the same 404 a nonexistent
     # transfer gets, instead of leaking that a pending transfer exists.
+    #
+    # head/render happen after the transaction returns, not inside it, so a
+    # commit-time callback failure (e.g. enqueuing a notification) surfaces as
+    # a clean 500 instead of racing an already-performed response.
     def resolve!(status)
-      OwnershipTransfer.transaction do
-        transfer = pending_ownership_transfer
-        return head(:not_found) if transfer.blank?
+      transfer = OwnershipTransfer.transaction do
+        loaded = pending_ownership_transfer
+        next if loaded.blank?
 
-        authorize!(action_name.to_sym, transfer)
+        authorize!(action_name.to_sym, loaded)
+        loaded.update(status:)
+        loaded
+      end
 
-        if transfer.update(status:)
-          head :ok
-        else
-          render json: { error: transfer.errors }, status: :unprocessable_content
-        end
+      if transfer.nil?
+        head :not_found
+      elsif transfer.errors.empty?
+        head :ok
+      else
+        render json: { error: transfer.errors }, status: :unprocessable_content
       end
     rescue CanCan::AccessDenied
       head :not_found
diff --git a/app/controllers/api/ownership_transfers_controller.rb b/app/controllers/api/ownership_transfers_controller.rb
index 004d59841..b6de6c55f 100644
--- a/app/controllers/api/ownership_transfers_controller.rb
+++ b/app/controllers/api/ownership_transfers_controller.rb
@@ -11,7 +11,7 @@ def show
 
       if @ownership_transfer.blank? || cannot?(:read, @ownership_transfer)
         head :not_found
-      elsif current_user_is_requester?
+      elsif current_user_is_an_owner?
         render json: { status: @ownership_transfer.status, you_are: 'owner', nominee_name:, nominee_email: }, status: :ok
       else
         render json: { status: @ownership_transfer.status, you_are: 'nominee' }, status: :ok
@@ -42,8 +42,8 @@ def most_recent_ownership_transfer
       @school.ownership_transfers.order(created_at: :desc).first
     end
 
-    def current_user_is_requester?
-      @ownership_transfer.requested_by_user_id == current_user.id
+    def current_user_is_an_owner?
+      current_user.school_owner?(@school)
     end
 
     def nominee_name
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ffa3b7081..15f03622f 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -13,7 +13,7 @@ def initialize(user)
     user.schools.active.each do |school|
       define_school_student_abilities(user:, school:) if user.school_student?(school)
       define_school_teacher_abilities(user:, school:) if user.school_teacher?(school)
-      define_school_owner_abilities(user:, school:) if user.school_owner?(school)
+      define_school_owner_abilities(school:) if user.school_owner?(school)
     end
 
     define_editor_admin_abilities(user)
@@ -68,7 +68,7 @@ def define_authenticated_non_student_abilities(user)
     can %i[read update destroy], Component, project: { user_id: user.id }
   end
 
-  def define_school_owner_abilities(user:, school:)
+  def define_school_owner_abilities(school:)
     can(%i[read update], School, id: school.id)
     can(%i[read], :school_member)
     can(%i[read create import update destroy regenerate_join_code], SchoolClass, school: { id: school.id })
@@ -83,7 +83,8 @@ def define_school_owner_abilities(user:, school:)
     can(%i[read create destroy], :school_owner)
     can(%i[read create destroy], :school_teacher)
     can(%i[read create], :ownership_transfer)
-    can(:read, OwnershipTransfer, school_id: school.id, requested_by_user_id: user.id)
+    can(:read, OwnershipTransfer, school_id: school.id)
+    can(:cancel, OwnershipTransfer, school_id: school.id, status: 'pending')
     can(%i[read create create_batch update destroy destroy_batch], :school_student)
     can(%i[create create_copy], Lesson, school_id: school.id)
     can(%i[read update destroy], Lesson, school_id: school.id, visibility: %w[teachers students public])
diff --git a/config/routes.rb b/config/routes.rb
index 86278b9ce..4da976888 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -90,6 +90,7 @@
       resource :ownership_transfer, only: %i[show create], controller: 'ownership_transfers' do
         put :accept, to: 'ownership_transfer_resolutions#accept'
         put :decline, to: 'ownership_transfer_resolutions#decline'
+        put :cancel, to: 'ownership_transfer_resolutions#cancel'
       end
       resources :students, only: %i[index create update destroy], controller: 'school_students' do
         post :batch, on: :collection, to: 'school_students#create_batch'
diff --git a/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb b/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb
new file mode 100644
index 000000000..c26008d53
--- /dev/null
+++ b/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb
@@ -0,0 +1,131 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Cancelling an ownership transfer', type: :request do
+  include_context 'with a school owner and nominated teacher'
+
+  it 'responds 401 Unauthorized when no token is given' do
+    put("/api/schools/#{school.id}/ownership_transfer/cancel")
+    expect(response).to have_http_status(:unauthorized)
+  end
+
+  it 'responds 403 Forbidden when the user is a school-student' do
+    student = create(:student, school:)
+    authenticated_in_hydra_as(student)
+
+    put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+    expect(response).to have_http_status(:forbidden)
+  end
+
+  it 'responds 403 Forbidden when the user is the owner of a different school' do
+    other_owner = create(:owner, school: create(:verified_school))
+    authenticated_in_hydra_as(other_owner)
+
+    put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+    expect(response).to have_http_status(:forbidden)
+  end
+
+  context 'when the school has never had an ownership transfer' do
+    before { authenticated_in_hydra_as(owner) }
+
+    it 'responds 404 Not Found' do
+      put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+      expect(response).to have_http_status(:not_found)
+    end
+  end
+
+  context 'when there is a pending transfer for the school' do
+    let!(:ownership_transfer) do
+      create(
+        :ownership_transfer,
+        school:,
+        nominated_user_id: nominee.id,
+        requested_by_user_id: owner.id,
+        email_address: nominee.email
+      )
+    end
+
+    context 'when the current user is the owner who requested the transfer' do
+      before { authenticated_in_hydra_as(owner) }
+
+      it 'responds 200 OK' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'marks the transfer as cancelled' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(ownership_transfer.reload.status).to eq('cancelled')
+      end
+    end
+
+    context 'when the current user is a different owner of the same school' do
+      let(:other_owner) { create(:owner, school:) }
+
+      before { authenticated_in_hydra_as(other_owner) }
+
+      it 'responds 200 OK, since any current owner can cancel, not only the one who requested it' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    context 'when the current user is the nominee' do
+      before { authenticated_in_hydra_as(nominee) }
+
+      it 'responds 404 Not Found, since only an owner of the school can cancel' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(response).to have_http_status(:not_found)
+      end
+
+      it 'does not change the transfer status' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(ownership_transfer.reload.status).to eq('pending')
+      end
+    end
+
+    context 'when the current user is a different teacher at the school' do
+      let(:other_teacher) { create(:teacher, school:) }
+
+      before { authenticated_in_hydra_as(other_teacher) }
+
+      it 'responds 404 Not Found' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'when the transfer is no longer pending' do
+      before do
+        ownership_transfer.update!(status: :completed)
+        authenticated_in_hydra_as(owner)
+      end
+
+      it 'responds 404 Not Found' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'when the transfer has already been cancelled' do
+      before do
+        authenticated_in_hydra_as(owner)
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+      end
+
+      it 'allows the owner to start a new transfer' do
+        other_teacher = create(:teacher, school:)
+        stub_user_info_api_for(other_teacher)
+
+        post(
+          "/api/schools/#{school.id}/ownership_transfer",
+          params: { ownership_transfer: { nominated_user_id: other_teacher.id } },
+          headers:
+        )
+
+        expect(response).to have_http_status(:created)
+      end
+    end
+  end
+end
diff --git a/spec/features/ownership_transfer/viewing_ownership_transfer_status_spec.rb b/spec/features/ownership_transfer/viewing_ownership_transfer_status_spec.rb
index cf03b3f22..54b8b035f 100644
--- a/spec/features/ownership_transfer/viewing_ownership_transfer_status_spec.rb
+++ b/spec/features/ownership_transfer/viewing_ownership_transfer_status_spec.rb
@@ -78,6 +78,25 @@
       end
     end
 
+    context 'when the current user is a different owner of the same school' do
+      let(:other_owner) { create(:owner, school:) }
+
+      before do
+        stub_user_info_api_for(nominee)
+        authenticated_in_hydra_as(other_owner)
+      end
+
+      it 'responds 200 OK, identifying them as an owner even though they did not request the transfer' do
+        get("/api/schools/#{school.id}/ownership_transfer", headers:)
+
+        expect(response).to have_http_status(:ok)
+        json = JSON.parse(response.body)
+        expect(json).to include(
+          'status' => 'pending', 'you_are' => 'owner', 'nominee_name' => nominee.name, 'nominee_email' => nominee.email
+        )
+      end
+    end
+
     context 'when the current user is the nominee' do
       before { authenticated_in_hydra_as(nominee) }
 
@@ -155,6 +174,22 @@
       end
     end
 
+    context 'when the transfer was cancelled by the owner' do
+      before do
+        ownership_transfer.update!(status: :cancelled)
+        stub_user_info_api_for(nominee)
+        authenticated_in_hydra_as(owner)
+      end
+
+      it 'responds 200 OK, still visible to the owner who cancelled it' do
+        get("/api/schools/#{school.id}/ownership_transfer", headers:)
+
+        expect(response).to have_http_status(:ok)
+        json = JSON.parse(response.body)
+        expect(json).to include('status' => 'cancelled', 'you_are' => 'owner', 'nominee_name' => nominee.name)
+      end
+    end
+
     context 'when a resolved transfer is viewed by someone who was never involved' do
       let(:other_teacher) { create(:teacher, school:) }
 

From 25be4cd0dd71a44cc3bd42af8006b8309497e557 Mon Sep 17 00:00:00 2001
From: Nathan Richards <nathan.richards@irysan.com>
Date: Tue, 22 Sep 2026 13:59:28 +0200
Subject: [PATCH 2/6] feat: email the nominee when their ownership nomination
 is cancelled

Works like the existing request email - sent from a callback that
only fires when a transfer actually moves from pending to cancelled,
so fixing up an old transfer to cancelled some other way won't
trigger it. Since any owner can now cancel a transfer, not just the
one who started it, the email attributes the cancellation to "the
school owner" generically rather than naming a specific person - it
has no reliable way to know which owner actually clicked cancel.
---
 app/mailers/school_ownership_mailer.rb        | 10 ++++
 app/models/ownership_transfer.rb              |  6 +++
 .../cancel_ownership_transfer.text.erb        | 10 ++++
 .../cancelling_an_ownership_transfer_spec.rb  | 10 ++++
 .../school_ownership_mailer_preview.rb        | 13 +++++
 spec/mailers/school_ownership_mailer_spec.rb  | 54 +++++++++++++++++++
 spec/models/ownership_transfer_spec.rb        | 34 ++++++++++++
 7 files changed, 137 insertions(+)
 create mode 100644 app/views/school_ownership_mailer/cancel_ownership_transfer.text.erb

diff --git a/app/mailers/school_ownership_mailer.rb b/app/mailers/school_ownership_mailer.rb
index 1a7820ef4..95bf75b9b 100644
--- a/app/mailers/school_ownership_mailer.rb
+++ b/app/mailers/school_ownership_mailer.rb
@@ -15,6 +15,16 @@ def request_ownership_transfer
          message_stream: 'outbound')
   end
 
+  def cancel_ownership_transfer
+    @school = ownership_transfer.school
+    @nominee_name = users_by_id[ownership_transfer.nominated_user_id]&.name.presence || 'there'
+
+    mail(to: ownership_transfer.email_address,
+         subject: "The ownership nomination for #{@school.name} has been cancelled",
+         track_opens: 'true',
+         message_stream: 'outbound')
+  end
+
   private
 
   def ownership_transfer
diff --git a/app/models/ownership_transfer.rb b/app/models/ownership_transfer.rb
index 9556a9c98..53d33574e 100644
--- a/app/models/ownership_transfer.rb
+++ b/app/models/ownership_transfer.rb
@@ -19,6 +19,8 @@ class OwnershipTransfer < ApplicationRecord
   validate :nominee_has_the_school_teacher_role_for_the_school
 
   after_create_commit :send_ownership_transfer_request_email
+  after_update_commit :send_ownership_transfer_cancelled_email,
+                      if: -> { saved_change_to_status?(from: 'pending', to: 'cancelled') }
   encrypts :email_address
 
   private
@@ -35,4 +37,8 @@ def nominee_has_the_school_teacher_role_for_the_school
   def send_ownership_transfer_request_email
     SchoolOwnershipMailer.with(ownership_transfer: self).request_ownership_transfer.deliver_later
   end
+
+  def send_ownership_transfer_cancelled_email
+    SchoolOwnershipMailer.with(ownership_transfer: self).cancel_ownership_transfer.deliver_later
+  end
 end
diff --git a/app/views/school_ownership_mailer/cancel_ownership_transfer.text.erb b/app/views/school_ownership_mailer/cancel_ownership_transfer.text.erb
new file mode 100644
index 000000000..88a0a9c59
--- /dev/null
+++ b/app/views/school_ownership_mailer/cancel_ownership_transfer.text.erb
@@ -0,0 +1,10 @@
+Hi <%= @nominee_name %>,
+
+The nomination for you to become the owner of the Code Classroom account for <%= @school.name %> has been cancelled by the school owner.
+
+No action is needed from you, and ownership remains unchanged. If you were expecting to take over this Code Classroom, please reach out to the school owner directly.
+
+If you have any questions, please contact us at websupport@raspberrypi.org.
+
+Kind Regards,
+The Code Editor team
diff --git a/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb b/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb
index c26008d53..d00db9588 100644
--- a/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb
+++ b/spec/features/ownership_transfer/cancelling_an_ownership_transfer_spec.rb
@@ -3,6 +3,8 @@
 require 'rails_helper'
 
 RSpec.describe 'Cancelling an ownership transfer', type: :request do
+  include ActionMailer::TestHelper
+
   include_context 'with a school owner and nominated teacher'
 
   it 'responds 401 Unauthorized when no token is given' do
@@ -58,6 +60,14 @@
         put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
         expect(ownership_transfer.reload.status).to eq('cancelled')
       end
+
+      it 'sends the cancellation email' do
+        put("/api/schools/#{school.id}/ownership_transfer/cancel", headers:)
+
+        assert_enqueued_email_with(
+          SchoolOwnershipMailer, :cancel_ownership_transfer, params: { ownership_transfer: }
+        )
+      end
     end
 
     context 'when the current user is a different owner of the same school' do
diff --git a/spec/mailers/previews/school_ownership_mailer_preview.rb b/spec/mailers/previews/school_ownership_mailer_preview.rb
index cfa4de6d3..b5a215cc8 100644
--- a/spec/mailers/previews/school_ownership_mailer_preview.rb
+++ b/spec/mailers/previews/school_ownership_mailer_preview.rb
@@ -17,6 +17,19 @@ def request_ownership_transfer
     with_stubbed_user_info_api { SchoolOwnershipMailer.with(ownership_transfer:).request_ownership_transfer.message }
   end
 
+  def cancel_ownership_transfer
+    school = School.new(name: 'Elmwood Secondary School')
+    ownership_transfer = OwnershipTransfer.new(
+      email_address: 'teacher@example.com',
+      school:,
+      nominated_user_id: NOMINEE[:id],
+      requested_by_user_id: REQUESTED_OWNER[:id],
+      status: :cancelled
+    )
+
+    with_stubbed_user_info_api { SchoolOwnershipMailer.with(ownership_transfer:).cancel_ownership_transfer.message }
+  end
+
   private
 
   # fake the user info response, but only for the duration of
diff --git a/spec/mailers/school_ownership_mailer_spec.rb b/spec/mailers/school_ownership_mailer_spec.rb
index 7451a1f59..36be6993b 100644
--- a/spec/mailers/school_ownership_mailer_spec.rb
+++ b/spec/mailers/school_ownership_mailer_spec.rb
@@ -41,4 +41,58 @@
       expect(email.subject).to include(ownership_transfer.school.name)
     end
   end
+
+  describe 'cancel_ownership_transfer' do
+    subject(:email) { described_class.with(ownership_transfer:).cancel_ownership_transfer }
+
+    let(:school) { create(:verified_school) }
+    let(:nominee) { create(:teacher, school:) }
+    let(:requested_owner) { create(:owner, school:) }
+    let(:ownership_transfer) do
+      create(
+        :ownership_transfer,
+        school:,
+        nominated_user_id: nominee.id,
+        requested_by_user_id: requested_owner.id,
+        status: :cancelled
+      )
+    end
+
+    before do
+      stub_user_info_api_fetch_by_ids(
+        user_ids: [nominee.id, requested_owner.id],
+        users: [{ id: nominee.id, name: nominee.name }, { id: requested_owner.id, name: requested_owner.name }]
+      )
+    end
+
+    it 'includes the nominee name in the body' do
+      expect(email.body.to_s).to include(nominee.name)
+    end
+
+    it 'attributes the cancellation to the school owner generically, not by name' do
+      expect(email.body.to_s).to include('the school owner')
+      expect(email.body.to_s).not_to include(requested_owner.name)
+    end
+
+    it 'includes the school name in the body' do
+      expect(email.body.to_s).to include(ownership_transfer.school.name)
+    end
+
+    it 'includes the school name in the subject' do
+      expect(email.subject).to include(ownership_transfer.school.name)
+    end
+
+    context 'when the nominee is missing from the user-info response' do
+      before do
+        stub_user_info_api_fetch_by_ids(
+          user_ids: [nominee.id, requested_owner.id],
+          users: [{ id: requested_owner.id, name: requested_owner.name }]
+        )
+      end
+
+      it 'greets them generically instead of leaving the greeting blank' do
+        expect(email.body.to_s).to include('Hi there,')
+      end
+    end
+  end
 end
diff --git a/spec/models/ownership_transfer_spec.rb b/spec/models/ownership_transfer_spec.rb
index 678d700f7..694d0438f 100644
--- a/spec/models/ownership_transfer_spec.rb
+++ b/spec/models/ownership_transfer_spec.rb
@@ -173,4 +173,38 @@
       )
     end
   end
+
+  describe 'the cancellation email' do
+    before { ownership_transfer.save! }
+
+    it 'is enqueued with the transfer as the mailer param when the transfer is cancelled' do
+      ownership_transfer.update!(status: :cancelled)
+
+      assert_enqueued_email_with(
+        SchoolOwnershipMailer, :cancel_ownership_transfer, params: { ownership_transfer: }
+      )
+    end
+
+    it 'is not enqueued when the transfer resolves to a different status' do
+      assert_no_enqueued_emails do
+        ownership_transfer.update!(status: :completed)
+      end
+    end
+
+    it 'is not enqueued when an already-cancelled transfer is saved again unchanged' do
+      ownership_transfer.update!(status: :cancelled)
+
+      assert_no_enqueued_emails do
+        ownership_transfer.update!(status: :cancelled)
+      end
+    end
+
+    it 'is not enqueued when a non-pending transfer is corrected to cancelled' do
+      ownership_transfer.update!(status: :completed)
+
+      assert_no_enqueued_emails do
+        ownership_transfer.update!(status: :cancelled)
+      end
+    end
+  end
 end

From 7acbcd9d95fefaa731b29aa4dad6c2a9301aa58f Mon Sep 17 00:00:00 2001
From: Nathan Richards <nathan.richards@irysan.com>
Date: Tue, 22 Sep 2026 17:35:06 +0200
Subject: [PATCH 3/6] fix: guard against a blank user-info API response
 crashing mailer jobs

If the user-info API ever returns an empty body, the user-lookup
code would crash instead of treating it as "no users found". The
fix lives in the client itself, not just the caller that happened to
hit it, so every caller is protected - including the job that sends
the new cancellation email.
---
 lib/user_info_api_client.rb           |  2 +-
 spec/lib/user_info_api_client_spec.rb | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 spec/lib/user_info_api_client_spec.rb

diff --git a/lib/user_info_api_client.rb b/lib/user_info_api_client.rb
index 74f96370b..ed415d21e 100644
--- a/lib/user_info_api_client.rb
+++ b/lib/user_info_api_client.rb
@@ -13,7 +13,7 @@ def fetch_by_ids(user_ids)
         r.url '/users'
         r.body = { userIds: user_ids }
       end
-      return if response.body.blank?
+      return [] if response.body.blank?
 
       transform_result(response.body.fetch('users', []))
     end
diff --git a/spec/lib/user_info_api_client_spec.rb b/spec/lib/user_info_api_client_spec.rb
new file mode 100644
index 000000000..b4b993898
--- /dev/null
+++ b/spec/lib/user_info_api_client_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe UserInfoApiClient do
+  describe '.fetch_by_ids' do
+    subject(:result) { described_class.fetch_by_ids(user_ids) }
+
+    let(:user_ids) { [SecureRandom.uuid] }
+
+    it 'returns an empty Array when the ids are blank' do
+      expect(described_class.fetch_by_ids([])).to eq []
+    end
+
+    context 'when the API responds with a blank body' do
+      before do
+        stub_request(:get, "#{described_class::API_URL}/users")
+          .with(headers: { Authorization: "Bearer #{described_class::API_KEY}" })
+          .to_return(status: 200, body: '')
+      end
+
+      it 'returns an empty Array rather than nil' do
+        expect(result).to eq []
+      end
+    end
+  end
+end

From 768f7e369b0211b10a69ebd507e5e92db292297b Mon Sep 17 00:00:00 2001
From: Nathan Richards <nathan.richards@irysan.com>
Date: Tue, 22 Sep 2026 18:40:22 +0200
Subject: [PATCH 4/6] fix: guard against blank names in the ownership transfer
 request email

request_ownership_transfer interpolated the nominee's and requested
owner's names directly with no fallback, so a missing name from the
user-info lookup would render as a blank greeting or a blank sentence
instead of degrading gracefully. Brings it in line with the same
hardening the cancellation email already has: a generic label when a
name isn't available, never a blank.
---
 app/mailers/school_ownership_mailer.rb       |  4 +--
 spec/mailers/school_ownership_mailer_spec.rb | 26 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/app/mailers/school_ownership_mailer.rb b/app/mailers/school_ownership_mailer.rb
index 95bf75b9b..730bbeac3 100644
--- a/app/mailers/school_ownership_mailer.rb
+++ b/app/mailers/school_ownership_mailer.rb
@@ -6,8 +6,8 @@ class SchoolOwnershipMailer < ApplicationMailer
   def request_ownership_transfer
     @school = ownership_transfer.school
 
-    @nominee_name = users_by_id[ownership_transfer.nominated_user_id]&.name
-    @requested_owner_name = users_by_id[ownership_transfer.requested_by_user_id]&.name
+    @nominee_name = users_by_id[ownership_transfer.nominated_user_id]&.name.presence || 'there'
+    @requested_owner_name = users_by_id[ownership_transfer.requested_by_user_id]&.name.presence || 'The school owner'
 
     mail(to: ownership_transfer.email_address,
          subject: "You've been nominated to be an owner of #{@school.name}",
diff --git a/spec/mailers/school_ownership_mailer_spec.rb b/spec/mailers/school_ownership_mailer_spec.rb
index 36be6993b..4f0c08f18 100644
--- a/spec/mailers/school_ownership_mailer_spec.rb
+++ b/spec/mailers/school_ownership_mailer_spec.rb
@@ -40,6 +40,32 @@
     it 'includes the school name in the subject' do
       expect(email.subject).to include(ownership_transfer.school.name)
     end
+
+    context 'when the nominee is missing from the user-info response' do
+      before do
+        stub_user_info_api_fetch_by_ids(
+          user_ids: [nominee.id, requested_owner.id],
+          users: [{ id: requested_owner.id, name: requested_owner.name }]
+        )
+      end
+
+      it 'greets them generically instead of leaving the greeting blank' do
+        expect(email.body.to_s).to include('Hi there,')
+      end
+    end
+
+    context 'when the requested owner is missing from the user-info response' do
+      before do
+        stub_user_info_api_fetch_by_ids(
+          user_ids: [nominee.id, requested_owner.id],
+          users: [{ id: nominee.id, name: nominee.name }]
+        )
+      end
+
+      it 'falls back to a generic label instead of leaving it blank' do
+        expect(email.body.to_s).to include('The school owner')
+      end
+    end
   end
 
   describe 'cancel_ownership_transfer' do

From fe1e75a1910d04236c99c4b0dc4f59e5b489931e Mon Sep 17 00:00:00 2001
From: Nathan Richards <nathan.richards@irysan.com>
Date: Fri, 25 Sep 2026 13:39:51 +0200
Subject: [PATCH 5/6] docs: reword comment to be less jargony

---
 .../api/ownership_transfer_resolutions_controller.rb        | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/api/ownership_transfer_resolutions_controller.rb b/app/controllers/api/ownership_transfer_resolutions_controller.rb
index 67bd32feb..2a893ca85 100644
--- a/app/controllers/api/ownership_transfer_resolutions_controller.rb
+++ b/app/controllers/api/ownership_transfer_resolutions_controller.rb
@@ -27,9 +27,9 @@ def cancel
     # attempt raises and is rescued below into the same 404 a nonexistent
     # transfer gets, instead of leaking that a pending transfer exists.
     #
-    # head/render happen after the transaction returns, not inside it, so a
-    # commit-time callback failure (e.g. enqueuing a notification) surfaces as
-    # a clean 500 instead of racing an already-performed response.
+    # head/render stay outside the transaction block: the transfer's
+    # after_update_commit callback (sends the cancellation email) runs inside
+    # it, and we want a failure there to be the only thing we respond with.
     def resolve!(status)
       transfer = OwnershipTransfer.transaction do
         loaded = pending_ownership_transfer

From 7166cd97a372a2c44d243943b997d1ef0f4ac0a1 Mon Sep 17 00:00:00 2001
From: Nathan Richards <nathan.richards@irysan.com>
Date: Fri, 25 Sep 2026 13:52:23 +0200
Subject: [PATCH 6/6] refactor: use first! so not-found and unauthorized share
 one rescue path

---
 ...nership_transfer_resolutions_controller.rb | 28 ++++++++-----------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/app/controllers/api/ownership_transfer_resolutions_controller.rb b/app/controllers/api/ownership_transfer_resolutions_controller.rb
index 2a893ca85..ecc11d270 100644
--- a/app/controllers/api/ownership_transfer_resolutions_controller.rb
+++ b/app/controllers/api/ownership_transfer_resolutions_controller.rb
@@ -20,39 +20,35 @@ def cancel
 
     private
 
-    # Wrapped in a transaction so the row lock below is held across the
-    # read-and-update, preventing a concurrent accept/decline/cancel on the
-    # same transfer from also finding it pending. Authorizing against the loaded
-    # transfer (rather than checking cannot? by hand) means an unauthorized
-    # attempt raises and is rescued below into the same 404 a nonexistent
-    # transfer gets, instead of leaking that a pending transfer exists.
+    # The lock is held until update finishes, so two requests can't both act
+    # on the same pending transfer at once.
     #
-    # head/render stay outside the transaction block: the transfer's
-    # after_update_commit callback (sends the cancellation email) runs inside
-    # it, and we want a failure there to be the only thing we respond with.
+    # Not found and not authorized both raise into the same head :not_found
+    # below, so the response can't be used to tell whether a transfer exists
+    # that the user just isn't allowed to touch.
+    #
+    # head/render happen after the transaction, not inside it, so a failure
+    # sending the cancellation email (its after_commit callback) is the only
+    # thing we respond with.
     def resolve!(status)
       transfer = OwnershipTransfer.transaction do
         loaded = pending_ownership_transfer
-        next if loaded.blank?
-
         authorize!(action_name.to_sym, loaded)
         loaded.update(status:)
         loaded
       end
 
-      if transfer.nil?
-        head :not_found
-      elsif transfer.errors.empty?
+      if transfer.errors.empty?
         head :ok
       else
         render json: { error: transfer.errors }, status: :unprocessable_content
       end
-    rescue CanCan::AccessDenied
+    rescue CanCan::AccessDenied, ActiveRecord::RecordNotFound
       head :not_found
     end
 
     def pending_ownership_transfer
-      @school.ownership_transfers.lock.pending.first
+      @school.ownership_transfers.lock.pending.first!
     end
   end
 end