Remove creator_id

AGENTS.md

@@ -40,7 +40,7 @@ docker compose up
 ## Salesforce / Heroku Connect
 - Sync writes to the `salesforce_connect` DB (not a Salesforce API). Pattern from editor-api PR #677.
 - Feature flag: `SALESFORCE_ENABLED=true`.
-- After deploy, backfill: `rails salesforce_sync:school`, `salesforce_sync:role`, `salesforce_sync:contact`, `salesforce_sync:school_class`, `salesforce_sync:class_teacher`, `salesforce_sync:lesson`.
+- After deploy, backfill: `rails salesforce_sync:school`, `salesforce_sync:role`, `salesforce_sync:school_class`, `salesforce_sync:class_teacher`, `salesforce_sync:lesson`.
 - **Parent-sync race guard (required for any job using `__r__` external-ID lookups).** Heroku Connect rejects an INSERT permanently with `Foreign key external ID … not found` if the parent record isn't yet in Salesforce — the mirror row stays `FAILED` forever (no auto-retry). Call `ensure_parent_synced!(model, external_id_field, external_id, label)` on `Salesforce::SalesforceSyncJob` (the base class) before saving a child record; it checks the parent has a non-nil `sfid` in its Heroku Connect mirror and raises `SalesforceRecordNotFound` if not. The base job declares `retry_on SalesforceRecordNotFound, wait: :polynomially_longer, attempts: 10` so the job self-heals once parents land. See `Salesforce::RoleSyncJob` and `Salesforce::ClassTeacherSyncJob` for call-site examples.
 
 ## Where to Look First

app/controllers/api/schools_controller.rb

@@ -26,7 +26,7 @@ def show
     end
 
     def create
-      result = School::Create.call(school_params: create_params, creator_id: current_user.id, token: current_user.token)
+      result = School::Create.call(school_params: create_params, owner_id: current_user.id, token: current_user.token)
 
       if result.success?
         @school = result[:school]

app/dashboards/school_dashboard.rb

@@ -12,7 +12,6 @@ class SchoolDashboard < Administrate::BaseDashboard
   ATTRIBUTE_TYPES = {
     id: Field::String,
     code: Field::String,
-    creator: Field::BelongsTo.with_options(class_name: 'User'),
     postal_code: Field::String,
     creator_role: Field::String,
     creator_department: Field::String,
@@ -64,7 +63,6 @@ class SchoolDashboard < Administrate::BaseDashboard
     name
     code
     user_origin
-    creator
     roles
     student_count
     creator_role

app/jobs/salesforce/role_sync_job.rb

@@ -32,10 +32,18 @@ def perform(role_id:)
       sf_role.editor_type__c = role.school&.user_origin || ::School.new.user_origin
 
       sf_role.save!
+
+      sync_ux_contact_consent(role:) if role.owner?
     end
 
     private
 
+    def sync_ux_contact_consent(role:)
+      sf_contact = Salesforce::Contact.find_by!(pi_accounts_unique_id__c: role.user_id)
+      sf_contact.editor_consent_to_ux_contact__c = role.school.creator_agree_to_ux_contact
+      sf_contact.save!
+    end
+
     def sf_role_attributes(role:)
       mapped_attributes(role:).to_h do |sf_field, value|
         value = truncate_value(sf_field:, value:) if value.is_a?(String)

app/jobs/school_import_job.rb

@@ -71,7 +71,7 @@ def import_school(school_data)
     School.transaction do
       result = School::Create.call(
         school_params: school_params,
-        creator_id: proposed_owner[:id],
+        owner_id: proposed_owner[:id],
         token: @token
       )
 

app/models/ability.rb

@@ -47,9 +47,6 @@ def define_authenticated_non_student_abilities(user)
     # Any authenticated user can create a school. They agree to become the school-owner.
     can :create, School
 
-    # An unverified school owner can read their own school.
-    can :read, School, creator_id: user.id, verified_at: nil
-
     # Any authenticated user can create a lesson, to support a RPF library of public lessons.
     can :create, Lesson, school_id: nil, school_class_id: nil
 

app/models/school.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class School < ApplicationRecord
+  self.ignored_columns += [:creator_id]
+
   has_many :classes, class_name: :SchoolClass, inverse_of: :school, dependent: :destroy
   has_many :lessons, dependent: :nullify
   has_many :projects, dependent: :nullify
@@ -34,11 +36,6 @@ class School < ApplicationRecord
             uniqueness: { conditions: -> { active }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.school_roll_number_exists') },
             format: { with: /\A[0-9]+[A-Z]+\z/, allow_nil: true, message: I18n.t('validations.school.school_roll_number') },
             presence: true, on: :create, if: :ireland?, unless: :hidden?
-  validates :creator_id,
-            presence: true,
-            uniqueness: {
-              conditions: -> { active }
-            }, unless: :hidden?
   validates :creator_agree_authority, presence: true, acceptance: true
   validates :creator_agree_terms_and_conditions, presence: true, acceptance: true
   validates :creator_agree_responsible_safeguarding, presence: true, acceptance: true
@@ -63,16 +60,12 @@ class School < ApplicationRecord
   after_commit -> { do_salesforce_sync(is_create: false) }, on: :update, if: -> { FeatureFlags.salesforce_sync? }
 
   def self.find_for_user!(user)
-    school = Role.find_by(user_id: user.id)&.school || active.find_by(creator_id: user.id)
+    school = Role.find_by(user_id: user.id)&.school
     raise ActiveRecord::RecordNotFound unless school
 
     school
   end
 
-  def creator
-    User.from_userinfo(ids: creator_id).first
-  end
-
   def verified?
     verified_at.present?
   end
@@ -221,6 +214,5 @@ def format_uk_postal_code
 
   def do_salesforce_sync(is_create:)
     Salesforce::SchoolSyncJob.perform_later(school_id: id, is_create:)
-    Salesforce::ContactSyncJob.perform_later(school_id: id)
   end
 end

app/services/school_onboarding_service.rb

@@ -7,10 +7,10 @@ def initialize(school)
     @school = school
   end
 
-  def onboard(token:)
+  def onboard(owner_id:, token:)
     School.transaction do
-      Role.owner.create!(user_id: school.creator_id, school:)
-      Role.teacher.create!(user_id: school.creator_id, school:)
+      Role.owner.create!(user_id: owner_id, school:)
+      Role.teacher.create!(user_id: owner_id, school:)
 
       ProfileApiClient.create_school(token:, id: school.id, code: school.code)
     end

lib/concepts/school/operations/create.rb

@@ -3,32 +3,51 @@
 class School
   class Create
     class << self
-      def call(school_params:, creator_id:, token:)
+      def call(school_params:, owner_id:, token:)
         response = OperationResponse.new
-        response[:school] = build_school(school_params.merge!(creator_id:))
 
-        School.transaction do
-          response[:school].save!
+        begin
+          response[:school] = nil
+          response[:school] = build_school(school_params)
 
-          SchoolOnboardingService.new(response[:school]).onboard(token:)
-        end
+          # Savepoint so failures roll back the school even when called inside an outer transaction (e.g. SchoolImportJob)
+          School.transaction(requires_new: true) do
+            acquire_advisory_lock_for_owner(owner_id)
+            response[:school].save!
 
-        response
-      rescue ProfileApiClient::UnauthorizedError => e
-        # Do not log noise to sentry.
-        # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422
-        Rails.logger.warn { "Failed to onboard school #{response[:school].id}: user is unauthorized" }
-        failure(response, e)
-      rescue StandardError => e
-        Sentry.capture_exception(e)
-        failure(response, e)
+            SchoolOnboardingService.new(response[:school]).onboard(owner_id:, token:)
+          end
+
+          response
+        rescue ProfileApiClient::UnauthorizedError => e
+          # Do not log noise to sentry.
+          # TODO: consider returning a separate error here to distinguish from other errors and return 401 from the API, not 422
+          Rails.logger.warn { "Failed to onboard school #{response[:school].id}: user is unauthorized" }
+          failure(response, e)
+        rescue ActiveRecord::RecordInvalid => e
+          if e.record.is_a?(Role)
+            response[:school].errors.merge!(e.record.errors)
+          else
+            Sentry.capture_exception(e)
+          end
+          failure(response, e)
+        rescue StandardError => e
+          Sentry.capture_exception(e)
+          failure(response, e)
+        end
       end
 
       private
 
+      def acquire_advisory_lock_for_owner(owner_id)
+        lock_key = Zlib.crc32("#{owner_id}:#{name}")
+        School.connection.execute("SELECT pg_advisory_xact_lock(#{lock_key})")
+      end
+
       def failure(response, error)
-        response[:error] = response[:school].errors.presence || [error.message]
-        response[:error_types] = response[:school].errors.details
+        school = response[:school]
+        response[:error] = school&.errors.presence || [error.message]
+        response[:error_types] = school&.errors&.details || {}
         response
       end
 

lib/tasks/for_education.rake

@@ -17,15 +17,15 @@ namespace :for_education do
   task destroy_seed_data: :environment do
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Destroying existing seeds...'
-      creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
+      owner_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
       teacher_id = ENV.fetch('SEEDING_TEACHER_ID', TEST_USERS[:john_doe])
 
       # Hard coded as the student's school needs to match
       student_ids = [TEST_USERS[:jane_smith], TEST_USERS[:john_smith], TEST_USERS[:emily_ssouser]]
       school_id = TEST_SCHOOL
 
       # Remove the roles first
-      Role.where(user_id: [creator_id, teacher_id] + student_ids).destroy_all
+      Role.where(user_id: [owner_id, teacher_id] + student_ids).destroy_all
 
       # Destroy the project and then the lesson itself (The lesson's `before_destroy` prevents us using destroy)
       lesson_ids = Lesson.where(school_id:).pluck(:id)
@@ -53,8 +53,8 @@ namespace :for_education do
 
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Attempting to seed data...'
-      creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
-      create_school(creator_id, TEST_SCHOOL)
+      owner_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
+      create_school(owner_id, TEST_SCHOOL)
 
       Rails.logger.info 'Done...'
     end
@@ -69,9 +69,9 @@ namespace :for_education do
 
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Attempting to seed data...'
-      creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
+      owner_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
 
-      school = create_school(creator_id, TEST_SCHOOL)
+      school = create_school(owner_id, TEST_SCHOOL)
       verify_school(school)
       Rails.logger.info 'Done...'
     end
@@ -86,18 +86,18 @@ namespace :for_education do
 
     ActiveRecord::Base.transaction do
       Rails.logger.info 'Attempting to seed data...'
-      creator_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
+      owner_id = ENV.fetch('SEEDING_CREATOR_ID', TEST_USERS[:jane_doe])
       teacher_id = ENV.fetch('SEEDING_TEACHER_ID', TEST_USERS[:john_doe])
 
-      school = create_school(creator_id, TEST_SCHOOL)
+      school = create_school(owner_id, TEST_SCHOOL)
       verify_school(school)
       school.update!(scratch_enabled: true)
       assign_a_teacher(teacher_id, school)
 
-      school_class = create_school_class(creator_id, school)
+      school_class = create_school_class(owner_id, school)
       assign_students(school_class, school)
 
-      create_lessons(creator_id, school, school_class)
+      create_lessons(owner_id, school, school_class)
       Rails.logger.info 'Done...'
     end
   end

lib/tasks/salesforce_sync.rake

@@ -15,13 +15,6 @@ namespace :salesforce_sync do
     end
   end
 
-  desc 'Sync creator_agree_to_ux_contact for all Schools to Salesforce Contact'
-  task contact: :environment do
-    School.find_each do |school|
-      Salesforce::ContactSyncJob.perform_later(school_id: school.id)
-    end
-  end
-
   desc 'Sync all SchoolClasses to Salesforce'
   task school_class: :environment do
     SchoolClass.find_each do |school_class|

lib/tasks/school_management.rake

@@ -21,18 +21,13 @@ namespace :school_management do
       next
     end
 
-    if School.exists?(creator_id: new_owner[:id])
-      Rails.logger.error("User #{new_owner[:id]} is already the creator of a school")
-      next
-    end
-
     school = Role.find_by(roles: { user_id: old_owner[:id], role: 'owner' }).school
 
     school.transaction do
       remove_old_owner(school, old_owner[:id], args[:keep_old_owner_as_teacher])
       assign_roles_to_new_owner(school, new_owner[:id])
 
-      school.update!(creator_id: new_owner[:id], creator_agree_to_ux_contact: false)
+      school.update!(creator_agree_to_ux_contact: false)
     end
 
     Rails.logger.info "Ownership transfered to #{new_owner[:email]} successfully."

lib/tasks/seeds_helper.rb

@@ -20,8 +20,8 @@ module SeedsHelper
   TEST_SCHOOL = 'e52de409-9210-4e94-b08c-dd11439e07d9' # e52de409-9210-4e94-b08c-dd11439e07d9
   SCHOOL_CODE = '12-34-56'
 
-  def create_school(creator_id, school_id = nil)
-    School.find_or_create_by!(creator_id:, id: school_id) do |school|
+  def create_school(owner_id, school_id = nil)
+    seeded_school = School.find_or_create_by!(id: school_id) do |school|
       Rails.logger.info 'Seeding a school...'
       country_code = Faker::Address.country_code
       school.name = Faker::Educator.secondary_school
@@ -31,7 +31,6 @@ def create_school(creator_id, school_id = nil)
       school.municipality = Faker::Address.city
       school.postal_code = Faker::Address.postcode
       school.country_code = country_code
-      school.creator_id = creator_id
       school.creator_agree_authority = true
       school.creator_agree_terms_and_conditions = true
       school.creator_agree_to_ux_contact = true
@@ -43,6 +42,12 @@ def create_school(creator_id, school_id = nil)
       end
       school.school_roll_number = "#{rand(10_000..99_999)}#{('A'..'Z').to_a.sample}" if country_code == 'IE'
     end
+
+    # Mirror SchoolOnboardingService: the owner gets their roles when the school is created
+    Role.owner.find_or_create_by!(user_id: owner_id, school: seeded_school)
+    Role.teacher.find_or_create_by!(user_id: owner_id, school: seeded_school)
+
+    seeded_school
   end
 
   def verify_school(school)
@@ -53,11 +58,7 @@ def verify_school(school)
 
     Rails.logger.info 'Verifying the school...'
 
-    School.transaction do
-      school.verify!
-      Role.owner.create!(user_id: school.creator_id, school:)
-      Role.teacher.create!(user_id: school.creator_id, school:)
-    end
+    school.verify!
 
     # rubocop:disable-next Rails/SkipsModelValidations
     school.update_column(:code, SCHOOL_CODE) # The code needs to match the one in the profile

lib/tasks/test_seeds.rake

@@ -20,7 +20,7 @@ namespace :test_seeds do
       student_ids = [TEST_USERS[:jane_smith], TEST_USERS[:john_smith], TEST_USERS[:emily_ssouser]]
       school_id = TEST_SCHOOL
       teacher_signup_school_id =
-        School.find_by(creator_id: teacher_signup_id)&.id
+        Role.owner.find_by(user_id: teacher_signup_id)&.school_id
 
       # Remove the roles first
       Role.where(user_id: teacher_signup_id).destroy_all