diff --git a/app/mailers/school_ownership_mailer.rb b/app/mailers/school_ownership_mailer.rb index 850343275..b4e235da2 100644 --- a/app/mailers/school_ownership_mailer.rb +++ b/app/mailers/school_ownership_mailer.rb @@ -6,10 +6,18 @@ class SchoolOwnershipMailer < ApplicationMailer def request_ownership_transfer ownership_transfer = params[:ownership_transfer] @school = ownership_transfer.school + @nominee_name = user_name(ownership_transfer.nominated_user_id) + @requested_owner_name = user_name(ownership_transfer.requested_by_user_id) 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 user_name(user_id) + User.from_userinfo(ids: [user_id]).first&.name + end end diff --git a/app/models/ownership_transfer.rb b/app/models/ownership_transfer.rb index 1b65ca227..d364b8234 100644 --- a/app/models/ownership_transfer.rb +++ b/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 diff --git a/app/models/school.rb b/app/models/school.rb index b224422c6..479d9bd96 100644 --- a/app/models/school.rb +++ b/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 diff --git a/app/views/school_ownership_mailer/request_ownership_transfer.text.erb b/app/views/school_ownership_mailer/request_ownership_transfer.text.erb index 418a5270a..451dd9268 100644 --- a/app/views/school_ownership_mailer/request_ownership_transfer.text.erb +++ b/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: diff --git a/db/migrate/20260915093824_add_nominee_and_status_to_ownership_transfers.rb b/db/migrate/20260915093824_add_nominee_and_status_to_ownership_transfers.rb new file mode 100644 index 000000000..6db23a243 --- /dev/null +++ b/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, null: false + add_column :ownership_transfers, :requested_by_user_id, :uuid, null: false + add_column :ownership_transfers, :status, :string, null: false, default: 'pending' + end +end diff --git a/db/schema.rb b/db/schema.rb index b05ab35a8..fa1861ef6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_09_11_104254) do +ActiveRecord::Schema[8.1].define(version: 2026_09_15_093824) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" enable_extension "pgcrypto" @@ -226,7 +226,10 @@ t.datetime "accepted_at" t.datetime "created_at", null: false t.string "email_address" + t.uuid "nominated_user_id", null: false + t.uuid "requested_by_user_id", null: false t.uuid "school_id", null: false + t.string "status", default: "pending", null: false t.datetime "updated_at", null: false t.index ["school_id"], name: "index_ownership_transfers_on_school_id" end diff --git a/spec/factories/ownership_transfer.rb b/spec/factories/ownership_transfer.rb index 33c89a03a..3e630476b 100644 --- a/spec/factories/ownership_transfer.rb +++ b/spec/factories/ownership_transfer.rb @@ -2,7 +2,10 @@ FactoryBot.define do factory :ownership_transfer do - email_address { 'new-owner@example.com' } - school factory: :verified_school + school + email_address { Faker::Internet.email } + nominated_user_id { SecureRandom.uuid } + requested_by_user_id { SecureRandom.uuid } + status { 'pending' } end end diff --git a/spec/mailers/previews/school_ownership_mailer_preview.rb b/spec/mailers/previews/school_ownership_mailer_preview.rb index a30188d9c..2407e8f35 100644 --- a/spec/mailers/previews/school_ownership_mailer_preview.rb +++ b/spec/mailers/previews/school_ownership_mailer_preview.rb @@ -2,9 +2,26 @@ # 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:) + stub_user_info_api + + ownership_transfer = OwnershipTransfer.new( + email_address: 'teacher@example.com', + school:, + nominated_user_id: NOMINEE[:id], + requested_by_user_id: REQUESTED_OWNER[:id] + ) SchoolOwnershipMailer.with(ownership_transfer:).request_ownership_transfer end + + private + + def stub_user_info_api + users = [NOMINEE, REQUESTED_OWNER] + UserInfoApiClient.define_singleton_method(:fetch_by_ids) { |ids| users.select { |u| ids.include?(u[:id]) } } + end end diff --git a/spec/mailers/school_ownership_mailer_spec.rb b/spec/mailers/school_ownership_mailer_spec.rb index e910221be..a4f6e368b 100644 --- a/spec/mailers/school_ownership_mailer_spec.rb +++ b/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], users: [{ id: nominee.id, name: nominee.name }]) + stub_user_info_api_fetch_by_ids( + user_ids: [requested_owner.id], users: [{ 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 diff --git a/spec/models/ownership_transfer_spec.rb b/spec/models/ownership_transfer_spec.rb index 618ef0057..e9dede049 100644 --- a/spec/models/ownership_transfer_spec.rb +++ b/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 diff --git a/spec/models/school_spec.rb b/spec/models/school_spec.rb index 20cffcc64..0d1382d33 100644 --- a/spec/models/school_spec.rb +++ b/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:) }