1069-add-immediate-onboarding

.env.example

@@ -51,3 +51,6 @@ EDITOR_ENCRYPTION_KEY=a1b2c3d4e5f67890123456789abcdef0123456789abcdef0123456789a
 # The sandbox creds can be found in 1password under "Google Cloud Console: CEfE Sandbox"
 GOOGLE_CLIENT_ID=changeme.apps.googleusercontent.com
 GOOGLE_CLIENT_SECRET=changeme
+
+# Enable immediate onboarding for schools
+ENABLE_IMMEDIATE_SCHOOL_ONBOARDING=true

app/controllers/api/schools_controller.rb

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

app/jobs/school_import_job.rb

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

app/models/school.rb

@@ -15,6 +15,8 @@ class School < ApplicationRecord
   validates :website, presence: true, format: { with: VALID_URL_REGEX, message: I18n.t('validations.school.website') }
   validates :address_line_1, presence: true
   validates :municipality, presence: true
+  validates :administrative_area, presence: true
+  validates :postal_code, presence: true
   validates :country_code, presence: true, inclusion: { in: ISO3166::Country.codes }
   validates :reference,
             uniqueness: { conditions: -> { where(rejected_at: nil) }, case_sensitive: false, allow_blank: true, message: I18n.t('validations.school.reference_urn_exists') },
@@ -39,8 +41,6 @@ class School < ApplicationRecord
   validates :verified_at, absence: { if: proc { |school| school.rejected? } }
   validates :code,
             uniqueness: { allow_nil: true },
-            presence: { if: proc { |school| school.verified? } },
-            absence: { unless: proc { |school| school.verified? } },
             format: { with: /\d\d-\d\d-\d\d/, allow_nil: true }
   validate :verified_at_cannot_be_changed
   validate :code_cannot_be_changed
@@ -51,6 +51,9 @@ class School < ApplicationRecord
 
   before_save :format_uk_postal_code, if: :should_format_uk_postal_code?
 
+  # TODO: Remove the conditional once the feature flag is retired
+  after_create :generate_code!, if: -> { FeatureFlags.immediate_school_onboarding? }
+
   def self.find_for_user!(user)
     school = Role.find_by(user_id: user.id)&.school || find_by(creator_id: user.id)
     raise ActiveRecord::RecordNotFound unless school
@@ -71,9 +74,19 @@ def rejected?
   end
 
   def verify!
+    # TODO: Remove this line once the feature flag is retired
+    generate_code! unless FeatureFlags.immediate_school_onboarding?
+
+    update!(verified_at: Time.zone.now)
+  end
+
+  def generate_code!
+    return code if code.present?
+
     attempts = 0
     begin
-      update!(verified_at: Time.zone.now, code: ForEducationCodeGenerator.generate)
+      new_code = ForEducationCodeGenerator.generate
+      update!(code: new_code)
     rescue ActiveRecord::RecordInvalid => e
       raise unless e.record.errors[:code].include?('has already been taken') && attempts <= 5
 
@@ -87,6 +100,8 @@ def reject
   end
 
   def reopen
+    return false unless rejected?
+
     update(rejected_at: nil)
   end
 
@@ -131,7 +146,7 @@ def rejected_at_cannot_be_changed
   end
 
   def code_cannot_be_changed
-    errors.add(:code, 'cannot be changed after verification') if code_was.present? && code_changed?
+    errors.add(:code, 'cannot be changed after onboarding') if code_was.present? && code_changed?
   end
 
   def should_format_uk_postal_code?

app/services/school_onboarding_service.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+class SchoolOnboardingService
+  attr_reader :school
+
+  def initialize(school)
+    @school = school
+  end
+
+  def onboard(token:)
+    School.transaction do
+      Role.owner.create!(user_id: school.creator_id, school:)
+      Role.teacher.create!(user_id: school.creator_id, school:)
+
+      ProfileApiClient.create_school(token:, id: school.id, code: school.code)
+    end
+  rescue StandardError => e
+    Sentry.capture_exception(e)
+    Rails.logger.error { "Failed to onboard school #{@school.id}: #{e.message}" }
+    false
+  else
+    true
+  end
+end

app/services/school_verification_service.rb

@@ -7,19 +7,24 @@ def initialize(school)
     @school = school
   end
 
-  def verify(token:)
+  def verify(token: nil)
+    success = false
     School.transaction do
       school.verify!
-      Role.owner.create!(user_id: school.creator_id, school:)
-      Role.teacher.create!(user_id: school.creator_id, school:)
-      ProfileApiClient.create_school(token:, id: school.id, code: school.code)
+
+      # TODO: Remove this line, once the feature flag is retired
+      success = FeatureFlags.immediate_school_onboarding? || SchoolOnboardingService.new(school).onboard(token: token)
+
+      # TODO: Remove this line, once the feature flag is retired
+      raise ActiveRecord::Rollback unless success
     end
   rescue StandardError => e
     Sentry.capture_exception(e)
     Rails.logger.error { "Failed to verify school #{@school.id}: #{e.message}" }
     false
   else
-    true
+    # TODO: Return 'true', once the feature flag is retired
+    success
   end
 
   delegate :reject, to: :school

lib/concepts/school/operations/create.rb

@@ -3,14 +3,24 @@
 class School
   class Create
     class << self
-      def call(school_params:, creator_id:)
+      def call(school_params:, creator_id:, token:)
         response = OperationResponse.new
         response[:school] = build_school(school_params.merge!(creator_id:))
-        response[:school].save!
+
+        School.transaction do
+          response[:school].save!
+
+          # TODO: Remove this conditional once the feature flag is retired
+          if FeatureFlags.immediate_school_onboarding?
+            onboarded = SchoolOnboardingService.new(response[:school]).onboard(token:)
+            raise 'School onboarding failed' unless onboarded
+          end
+        end
+
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
-        response[:error] = response[:school].errors
+        response[:error] = response[:school].errors.presence || [e.message]
         response[:error_types] = response[:school].errors.details
 
         response

lib/feature_flags.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module FeatureFlags
+  def self.immediate_school_onboarding?
+    ENV['ENABLE_IMMEDIATE_SCHOOL_ONBOARDING'] == 'true'
+  end
+end

lib/tasks/seeds_helper.rb

@@ -20,7 +20,9 @@ def create_school(creator_id, school_id = nil)
       school.name = Faker::Educator.secondary_school
       school.website = Faker::Internet.url(scheme: 'https')
       school.address_line_1 = Faker::Address.street_address
+      school.administrative_area = "#{Faker::Address.city}shire"
       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

spec/concepts/school/create_spec.rb

@@ -8,7 +8,9 @@
       name: 'Test School',
       website: 'http://www.example.com',
       address_line_1: 'Address Line 1',
+      administrative_area: 'Greater London',
       municipality: 'Greater London',
+      postal_code: 'SW1A 1AA',
       country_code: 'GB',
       reference: '100000',
       creator_agree_authority: true,
@@ -21,27 +23,31 @@
   let(:token) { UserProfileMock::TOKEN }
   let(:creator_id) { SecureRandom.uuid }
 
+  before do
+    allow(ProfileApiClient).to receive(:create_school).and_return(true)
+  end
+
   it 'returns a successful operation response' do
-    response = described_class.call(school_params:, creator_id:)
+    response = described_class.call(school_params:, creator_id:, token:)
     expect(response.success?).to be(true)
   end
 
   it 'creates a school' do
-    expect { described_class.call(school_params:, creator_id:) }.to change(School, :count).by(1)
+    expect { described_class.call(school_params:, creator_id:, token:) }.to change(School, :count).by(1)
   end
 
   it 'returns the school in the operation response' do
-    response = described_class.call(school_params:, creator_id:)
+    response = described_class.call(school_params:, creator_id:, token:)
     expect(response[:school]).to be_a(School)
   end
 
   it 'assigns the name' do
-    response = described_class.call(school_params:, creator_id:)
+    response = described_class.call(school_params:, creator_id:, token:)
     expect(response[:school].name).to eq('Test School')
   end
 
   it 'assigns the creator_id' do
-    response = described_class.call(school_params:, creator_id:)
+    response = described_class.call(school_params:, creator_id:, token:)
     expect(response[:school].creator_id).to eq(creator_id)
   end
 
@@ -53,27 +59,67 @@
     end
 
     it 'does not create a school' do
-      expect { described_class.call(school_params:, creator_id:) }.not_to change(School, :count)
+      expect { described_class.call(school_params:, creator_id:, token:) }.not_to change(School, :count)
     end
 
     it 'returns a failed operation response' do
-      response = described_class.call(school_params:, creator_id:)
+      response = described_class.call(school_params:, creator_id:, token:)
       expect(response.failure?).to be(true)
     end
 
     it 'returns the correct number of objects in the operation response' do
-      response = described_class.call(school_params:, creator_id:)
-      expect(response[:error].count).to eq(9)
+      response = described_class.call(school_params:, creator_id:, token:)
+      expect(response[:error].count).to eq(11)
     end
 
     it 'returns the correct type of object in the operation response' do
-      response = described_class.call(school_params:, creator_id:)
+      response = described_class.call(school_params:, creator_id:, token:)
       expect(response[:error].first).to be_a(ActiveModel::Error)
     end
 
     it 'sent the exception to Sentry' do
-      described_class.call(school_params:, creator_id:)
+      described_class.call(school_params:, creator_id:, token:)
       expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
     end
   end
+
+  describe 'when immediate onboarding is enabled' do
+    # TODO: Remove this block once the feature flag is retired
+    around do |example|
+      ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: 'true') do
+        example.run
+      end
+    end
+
+    let(:onboarding_service) { instance_spy(SchoolOnboardingService, onboard: true) }
+
+    before do
+      allow(SchoolOnboardingService).to receive(:new).and_return(onboarding_service)
+    end
+
+    it 'calls the onboarding service' do
+      described_class.call(school_params:, creator_id:, token:)
+      expect(onboarding_service).to have_received(:onboard).with(token:)
+    end
+  end
+
+  # TODO: Remove these examples once the feature flag is retired
+  describe 'when immediate onboarding is disabled' do
+    around do |example|
+      ClimateControl.modify(ENABLE_IMMEDIATE_SCHOOL_ONBOARDING: nil) do
+        example.run
+      end
+    end
+
+    let(:onboarding_service) { instance_spy(SchoolOnboardingService) }
+
+    before do
+      allow(SchoolOnboardingService).to receive(:new).and_return(onboarding_service)
+    end
+
+    it 'does not call the onboarding service' do
+      described_class.call(school_params:, creator_id:, token:)
+      expect(onboarding_service).not_to have_received(:onboard)
+    end
+  end
 end

spec/factories/school.rb

@@ -2,10 +2,12 @@
 
 FactoryBot.define do
   factory :school do
-    sequence(:name) { |n| "School #{n}" }
-    website { 'http://www.example.com' }
-    address_line_1 { 'Address Line 1' }
-    municipality { 'Greater London' }
+    name { "#{Faker::Educator.primary_school} #{Faker::Address.city}" }
+    website { Faker::Internet.url }
+    address_line_1 { Faker::Address.street_address }
+    administrative_area { "#{Faker::Address.city}shire" }
+    municipality { Faker::Address.city }
+    postal_code { Faker::Address.postcode }
     country_code { 'GB' }
     sequence(:reference) { |n| format('%06d', 100_000 + n) }
     school_roll_number { nil }

spec/features/school/creating_a_school_spec.rb

@@ -17,7 +17,9 @@
         name: 'Test School',
         website: 'http://www.example.com',
         address_line_1: 'Address Line 1',
+        administrative_area: 'Greater London',
         municipality: 'Greater London',
+        postal_code: 'SW1A 1AA',
         country_code: 'GB',
         reference: '100000',
         creator_agree_authority: true,

spec/features/school/showing_a_school_spec.rb

@@ -34,10 +34,9 @@
   end
 
   it 'responds 403 Forbidden when the user belongs to a different school' do
-    Role.owner.find_by(user_id: owner.id, school:).delete
-    school.update!(id: SecureRandom.uuid)
+    other_school = create(:school)
 
-    get("/api/schools/#{school.id}", headers:)
+    get("/api/schools/#{other_school.id}", headers:)
     expect(response).to have_http_status(:forbidden)
   end
 end

spec/jobs/school_import_job_spec.rb

@@ -15,7 +15,9 @@
           name: 'Test School 1',
           website: 'https://test1.example.com',
           address_line_1: '123 Main St',
+          administrative_area: 'Massachusetts',
           municipality: 'Springfield',
+          postal_code: '01101',
           country_code: 'US',
           owner_email: 'owner1@example.com',
           district_name: 'Some District',
@@ -25,7 +27,9 @@
           name: 'Test School 2',
           website: 'https://test2.example.com',
           address_line_1: '456 Oak Ave',
+          administrative_area: 'Massachusetts',
           municipality: 'Boston',
+          postal_code: '02101',
           country_code: 'US',
           owner_email: 'owner2@example.com',
           district_name: 'Other District',
@@ -126,7 +130,9 @@
             'name' => 'Test School 1',
             'website' => 'https://test1.example.com',
             'address_line_1' => '123 Main St',
+            'administrative_area' => 'Massachusetts',
             'municipality' => 'Springfield',
+            'postal_code' => '01101',
             'country_code' => 'us',
             'owner_email' => 'owner1@example.com',
             'district_name' => 'Some District',