Update ownership transfer and mailer

app/mailers/school_ownership_mailer.rb

@@ -4,12 +4,26 @@ class SchoolOwnershipMailer < ApplicationMailer
   default from: email_address_with_name('web@raspberrypi.org', 'Raspberry Pi Foundation')
 
   def request_ownership_transfer
-    ownership_transfer = params[: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
+
     mail(to: ownership_transfer.email_address,
          subject: "You've been nominated to be an owner of #{@school.name}",
          track_opens: 'true',
          message_stream: 'outbound')
   end
+
+  private
+
+  def ownership_transfer
+    params[:ownership_transfer]
+  end
+
+  def users_by_id
+    @users_by_id ||= User.from_userinfo(
+      ids: [ownership_transfer.nominated_user_id, ownership_transfer.requested_by_user_id]
+    ).index_by(&:id)
+  end
 end

app/models/ownership_transfer.rb

@@ -4,13 +4,31 @@ class OwnershipTransfer < ApplicationRecord
   delegate :name, to: :school, prefix: true
 
   belongs_to :school
+
+  enum :status, {
+    pending: 'pending', completed: 'completed', rejected: 'rejected', cancelled: 'cancelled'
+  }, default: :pending, validate: true
+
+  validates :nominated_user_id, presence: true
+  validates :requested_by_user_id, presence: true
   validates :email_address,
             format: { with: EmailValidator.regexp, message: I18n.t('validations.invitation.email_address') }
+  validate :nominee_has_the_school_owner_or_school_teacher_role_for_the_school
+
   after_create_commit :send_ownership_transfer_request_email
   encrypts :email_address
 
   private
 
+  def nominee_has_the_school_owner_or_school_teacher_role_for_the_school
+    return unless nominated_user_id_changed? && errors.blank? && school
+
+    return if school.roles.exists?(user_id: nominated_user_id, role: %i[owner teacher])
+
+    msg = "'#{nominated_user_id}' does not have the 'owner' or 'teacher' role for school '#{school.id}'"
+    errors.add(:nominated_user_id, msg)
+  end
+
   def send_ownership_transfer_request_email
     SchoolOwnershipMailer.with(ownership_transfer: self).request_ownership_transfer.deliver_later
   end

app/models/school.rb

@@ -7,6 +7,7 @@ class School < ApplicationRecord
   has_many :roles, dependent: :nullify
   has_many :school_projects, dependent: :nullify
   has_many :school_email_domains, dependent: :destroy
+  has_many :ownership_transfers, dependent: :destroy
 
   VALID_URL_REGEX = %r{\A(?:https?://)?(?:www.)?[a-z0-9]+([-.]{1}[a-z0-9]+)*\.[a-z]{2,63}(\.[a-z]{2,63})*(/.*)?\z}ix
 

app/views/school_ownership_mailer/request_ownership_transfer.text.erb

@@ -1,6 +1,6 @@
-Hi there,
+Hi <%= @nominee_name %>,
 
-Current school owner has nominated you to become the new owner of the Code Classroom account for <%= @school.name %>.
+<%= @requested_owner_name %> has nominated you to become the new owner of the Code Classroom account for <%= @school.name %>.
 
 If you accept, you’ll take over ownership of the school account and be able to:
 

db/migrate/20260915093824_add_nominee_and_status_to_ownership_transfers.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddNomineeAndStatusToOwnershipTransfers < ActiveRecord::Migration[8.1]
+  def change
+    add_column :ownership_transfers, :nominated_user_id, :uuid
+    add_column :ownership_transfers, :requested_by_user_id, :uuid
+    add_column :ownership_transfers, :status, :string, null: false, default: 'pending'
+  end
+end

spec/factories/ownership_transfer.rb

@@ -2,7 +2,20 @@
 
 FactoryBot.define do
   factory :ownership_transfer do
-    email_address { 'new-owner@example.com' }
-    school factory: :verified_school
+    school
+    email_address { Faker::Internet.email }
+    status { 'pending' }
+
+    # rubocop:disable FactoryBot/FactoryAssociationWithStrategy
+    # Must be persisted even when building an ownership_transfer,
+    # since the nominee role check queries the roles table directly
+    transient do
+      nominee { create(:teacher, school:) }
+      requester { create(:owner, school:) }
+    end
+    # rubocop:enable FactoryBot/FactoryAssociationWithStrategy
+
+    nominated_user_id { nominee.id }
+    requested_by_user_id { requester.id }
   end
 end

spec/mailers/previews/school_ownership_mailer_preview.rb

@@ -2,9 +2,32 @@
 
 # Preview all emails at http://localhost:3009/rails/mailers/school_ownership_mailer
 class SchoolOwnershipMailerPreview < ActionMailer::Preview
+  NOMINEE = { id: SecureRandom.uuid, name: 'Eliseo Ortiz' }.freeze
+  REQUESTED_OWNER = { id: SecureRandom.uuid, name: 'Oaklynn Duran' }.freeze
+
   def request_ownership_transfer
     school = School.new(name: 'Elmwood Secondary School')
-    ownership_transfer = OwnershipTransfer.new(email_address: 'teacher@example.com', school:)
-    SchoolOwnershipMailer.with(ownership_transfer:).request_ownership_transfer
+    ownership_transfer = OwnershipTransfer.new(
+      email_address: 'teacher@example.com',
+      school:,
+      nominated_user_id: NOMINEE[:id],
+      requested_by_user_id: REQUESTED_OWNER[:id]
+    )
+
+    with_stubbed_user_info_api { SchoolOwnershipMailer.with(ownership_transfer:).request_ownership_transfer.message }
+  end
+
+  private
+
+  # fake the user info response, but only for the duration of
+  # this call, so other previews/requests in the same dev server aren't affected
+  def with_stubbed_user_info_api
+    users = [NOMINEE, REQUESTED_OWNER]
+    original_fetch_by_ids = UserInfoApiClient.method(:fetch_by_ids)
+
+    UserInfoApiClient.define_singleton_method(:fetch_by_ids) { |ids| users.select { |u| ids.include?(u[:id]) } }
+    yield
+  ensure
+    UserInfoApiClient.define_singleton_method(:fetch_by_ids, original_fetch_by_ids)
   end
 end

spec/mailers/school_ownership_mailer_spec.rb

@@ -6,12 +6,29 @@
   describe 'request_ownership_transfer' do
     subject(:email) { described_class.with(ownership_transfer:).request_ownership_transfer }
 
-    let(:ownership_transfer) { create(: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)
+    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 }]
+      )
       allow(ENV).to receive(:fetch).with('EDITOR_PUBLIC_URL').and_return('http://example.com')
     end
 
+    it 'includes the nominee name in the body' do
+      expect(email.body.to_s).to include(nominee.name)
+    end
+
+    it 'includes the name of requested owner in the body' do
+      expect(email.body.to_s).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

spec/models/ownership_transfer_spec.rb

@@ -5,37 +5,134 @@
 RSpec.describe OwnershipTransfer do
   include ActionMailer::TestHelper
 
-  it 'has a valid factory' do
-    ownership_transfer = build(:ownership_transfer)
+  subject(:ownership_transfer) { build(:ownership_transfer, school:, nominated_user_id: nominee.id) }
 
-    expect(ownership_transfer).to be_valid
-  end
+  let(:school) { create(:verified_school) }
+  let(:nominee) { create(:teacher, school:) }
+
+  describe 'validations' do
+    it 'has a valid factory' do
+      expect(ownership_transfer).to be_valid
+    end
+
+    it 'requires a school' do
+      ownership_transfer.school = nil
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'requires a nominated_user_id' do
+      ownership_transfer.nominated_user_id = nil
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'requires a requested_by_user_id' do
+      ownership_transfer.requested_by_user_id = nil
 
-  it 'is invalid with an incorrectly formatted email address' do
-    ownership_transfer = build(:ownership_transfer, email_address: 'not-an-email-address')
+      expect(ownership_transfer).not_to be_valid
+    end
 
-    expect(ownership_transfer).not_to be_valid
+    it 'requires an email_address' do
+      ownership_transfer.email_address = nil
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'is invalid with an incorrectly formatted email address' do
+      ownership_transfer.email_address = 'not-an-email-address'
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'non-deterministically encrypts the email_address' do
+      ownership_transfer.save!
+
+      expect(described_class.find_by(email_address: ownership_transfer.email_address)).to be_nil
+    end
   end
 
-  it 'sends an ownership transfer request email after create' do
-    school = create(:verified_school)
+  describe 'status' do
+    it 'defaults to pending on a new record' do
+      expect(ownership_transfer.status).to eq('pending')
+    end
+
+    it 'is valid for every declared status' do
+      described_class.statuses.each_key do |status|
+        ownership_transfer.status = status
 
-    ownership_transfer = described_class.create!(email_address: 'new-owner@example.com', school:)
+        expect(ownership_transfer).to be_valid
+      end
+    end
 
-    assert_enqueued_email_with SchoolOwnershipMailer, :request_ownership_transfer, params: { ownership_transfer: }
+    it 'is invalid when set to a status outside the enum' do
+      ownership_transfer.status = 'made-up-status'
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'exposes a predicate for the current status' do
+      described_class.statuses.each_key do |status|
+        ownership_transfer.status = status
+
+        expect(ownership_transfer.public_send("#{status}?")).to be true
+      end
+    end
+
+    it 'exposes a scope per status' do
+      described_class.statuses.each_key do |status|
+        ownership_transfer.status = status
+        ownership_transfer.save!
+
+        expect(described_class.public_send(status)).to include(ownership_transfer)
+      end
+    end
   end
 
-  it 'delegates #school_name to School#name' do
-    school = build(:school, name: 'school-name')
-    ownership_transfer = build(:ownership_transfer, school:)
+  describe 'nominee role validation' do
+    it 'is valid when the nominee has the teacher role for the school' do
+      expect(ownership_transfer).to be_valid
+    end
+
+    it 'is valid when the nominee has the owner role for the school' do
+      owner = create(:owner, school:)
+      ownership_transfer.nominated_user_id = owner.id
+
+      expect(ownership_transfer).to be_valid
+    end
+
+    it 'is invalid when the nominee has only the student role for the school' do
+      student = create(:student, school:)
+      ownership_transfer.nominated_user_id = student.id
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'is invalid when the nominee has a teacher role for a different school' do
+      other_school = create(:verified_school)
+      other_teacher = create(:teacher, school: other_school)
+      ownership_transfer.nominated_user_id = other_teacher.id
+
+      expect(ownership_transfer).not_to be_valid
+    end
+
+    it 'adds an error naming the nominated_user_id and the school id' do
+      student = create(:student, school:)
+      ownership_transfer.nominated_user_id = student.id
+      ownership_transfer.valid?
 
-    expect(ownership_transfer.school_name).to eq('school-name')
+      expect(ownership_transfer.errors[:nominated_user_id].first).to include(student.id)
+      expect(ownership_transfer.errors[:nominated_user_id].first).to include(school.id)
+    end
   end
 
-  it 'non-deterministically encrypts the email_address' do
-    school = create(:verified_school)
-    described_class.create!(email_address: 'new-owner@example.com', school:)
+  describe 'the request email' do
+    it 'is enqueued with the transfer as the mailer param' do
+      ownership_transfer.save!
 
-    expect(described_class.find_by(email_address: 'new-owner@example.com')).to be_nil
+      assert_enqueued_email_with(
+        SchoolOwnershipMailer, :request_ownership_transfer, params: { ownership_transfer: }
+      )
+    end
   end
 end

spec/models/school_spec.rb

@@ -41,6 +41,15 @@
       expect(school.school_email_domains.size).to eq(2)
     end
 
+    it 'has many ownership transfers' do
+      owner_one = create(:owner_role, school:)
+      owner_two = create(:owner_role, school:)
+      create(:ownership_transfer, school:, requested_by_user_id: owner_one.user_id, nominated_user_id: owner_one.user_id)
+      create(:ownership_transfer, school:, requested_by_user_id: owner_two.user_id, nominated_user_id: owner_two.user_id)
+
+      expect(school.ownership_transfers.size).to eq(2)
+    end
+
     context 'when a school is destroyed' do
       let!(:school_class) { create(:school_class, school:, teacher_ids: [teacher.id]) }
       let!(:lesson_1) { create(:lesson, user_id: teacher.id, school_class:) }