Refactor the stubbed_users methods out of UserInfoApiClient; refactor tests.

fspeirs · fspeirs · commit 2b1e12630aa9 · 2025-12-04T10:16:40.000Z
diff --git a/lib/user_info_api_client.rb b/lib/user_info_api_client.rb
@@ -7,7 +7,6 @@ class UserInfoApiClient
   class << self
     def fetch_by_ids(user_ids)
       return [] if user_ids.blank?
-      return stubbed_users(user_ids) if bypass_oauth?
 
       response = conn.get do |r|
         r.url '/users'
@@ -20,7 +19,6 @@ def fetch_by_ids(user_ids)
 
     def find_user_by_email(email)
       return nil if email.blank?
-      return stubbed_user_by_email(email) if bypass_oauth?
 
       response = conn.get do |r|
         r.url "/users/#{CGI.escape(email)}"
@@ -36,10 +34,6 @@ def find_user_by_email(email)
 
     private
 
-    def bypass_oauth?
-      ENV.fetch('BYPASS_OAUTH', nil) == 'true'
-    end
-
     def transform_user(user)
       user.transform_keys { |k| k.to_s.underscore.to_sym }
     end
@@ -59,49 +53,5 @@ def conn
         f.response :json # decode response bodies as JSON
       end
     end
-
-    def stubbed_users(user_ids)
-      user_ids.map do |user_id|
-        {
-          id: user_id,
-          email: "user-#{user_id}@example.com",
-          username: nil,
-          parentalEmail: nil,
-          name: 'School Owner',
-          nickname: 'Owner',
-          country: 'United Kingdom',
-          country_code: 'GB',
-          postcode: nil,
-          dateOfBirth: nil,
-          verifiedAt: '2024-01-01T12:00:00.000Z',
-          createdAt: '2024-01-01T12:00:00.000Z',
-          updatedAt: '2024-01-01T12:00:00.000Z',
-          discardedAt: nil,
-          lastLoggedInAt: '2024-01-01T12:00:00.000Z',
-          roles: ''
-        }
-      end
-    end
-
-    def stubbed_user_by_email(email)
-      {
-        id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE, email),
-        email: email,
-        username: nil,
-        parentalEmail: nil,
-        name: 'School Owner',
-        nickname: 'Owner',
-        country: 'United Kingdom',
-        country_code: 'GB',
-        postcode: nil,
-        dateOfBirth: nil,
-        verifiedAt: '2024-01-01T12:00:00.000Z',
-        createdAt: '2024-01-01T12:00:00.000Z',
-        updatedAt: '2024-01-01T12:00:00.000Z',
-        discardedAt: nil,
-        lastLoggedInAt: '2024-01-01T12:00:00.000Z',
-        roles: ''
-      }
-    end
   end
 end
diff --git a/spec/features/school/importing_schools_spec.rb b/spec/features/school/importing_schools_spec.rb
@@ -26,7 +26,10 @@
 
       before do
         authenticated_in_hydra_as(admin_user)
-        allow(UserInfoApiClient).to receive(:find_user_by_email).and_return({ id: SecureRandom.uuid, email: 'owner@example.com' })
+        stub_user_info_api_find_by_email(
+          email: 'owner@example.com',
+          user: { id: SecureRandom.uuid, email: 'owner@example.com' }
+        )
         allow(SchoolImportJob).to receive(:perform_later).and_return(instance_double(SchoolImportJob, job_id: 'test-job-id'))
       end
 
diff --git a/spec/jobs/school_import_job_spec.rb b/spec/jobs/school_import_job_spec.rb
@@ -31,18 +31,22 @@
     end
 
     before do
-      allow(UserInfoApiClient).to receive(:find_user_by_email)
-        .with('owner1@example.com')
-        .and_return({ id: owner1_id, email: 'owner1@example.com' })
-
-      allow(UserInfoApiClient).to receive(:find_user_by_email)
-        .with('owner2@example.com')
-        .and_return({ id: owner2_id, email: 'owner2@example.com' })
-
       allow(ProfileApiClient).to receive(:create_school).and_return(true)
     end
 
     context 'when all schools can be created successfully' do
+      before do
+        stub_user_info_api_find_by_email(
+          email: 'owner1@example.com',
+          user: { id: owner1_id, email: 'owner1@example.com' }
+        )
+
+        stub_user_info_api_find_by_email(
+          email: 'owner2@example.com',
+          user: { id: owner2_id, email: 'owner2@example.com' }
+        )
+      end
+
       it 'creates schools and returns successful results' do
         results = described_class.new.perform(
           schools_data: schools_data,
@@ -67,9 +71,7 @@
 
     context 'when owner email is not found' do
       before do
-        allow(UserInfoApiClient).to receive(:find_user_by_email)
-          .with('owner1@example.com')
-          .and_return(nil)
+        stub_user_info_api_find_by_email(email: 'owner1@example.com', user: nil)
       end
 
       it 'adds failed result for that school' do
@@ -91,6 +93,11 @@
 
       before do
         Role.owner.create!(school_id: existing_school.id, user_id: owner1_id)
+
+        stub_user_info_api_find_by_email(
+          email: 'owner1@example.com',
+          user: { id: owner1_id, email: 'owner1@example.com' }
+        )
       end
 
       it 'adds failed result for that school' do
@@ -123,9 +130,10 @@
       end
 
       before do
-        allow(UserInfoApiClient).to receive(:find_user_by_email)
-          .with('owner1@example.com')
-          .and_return({ id: owner1_id, email: 'owner1@example.com' })
+        stub_user_info_api_find_by_email(
+          email: 'owner1@example.com',
+          user: { id: owner1_id, email: 'owner1@example.com' }
+        )
       end
 
       it 'handles string keys correctly' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
@@ -73,23 +73,6 @@
       it 'returns a user without a username' do
         expect(user.username).to be_nil
       end
-
-      context 'when BYPASS_OAUTH is true' do
-        around do |example|
-          ClimateControl.modify(BYPASS_OAUTH: 'true') do
-            example.run
-          end
-        end
-
-        it 'does not call the API' do
-          user
-          expect(WebMock).not_to have_requested(:get, /.*/)
-        end
-
-        it 'returns a stubbed user' do
-          expect(user.name).to eq('School Owner')
-        end
-      end
     end
 
     context 'when logged into a student account' do
@@ -370,25 +353,6 @@
     it 'returns a user with the correct email' do
       expect(user.email).to eq 'school-owner@example.com'
     end
-
-    context 'when BYPASS_OAUTH is true' do
-      around do |example|
-        ClimateControl.modify(BYPASS_OAUTH: 'true') do
-          example.run
-        end
-      end
-
-      let(:owner) { create(:owner, school:, id: '00000000-0000-0000-0000-000000000000') }
-
-      it 'does not call the API' do
-        user
-        expect(WebMock).not_to have_requested(:get, /.*/)
-      end
-
-      it 'returns a stubbed user' do
-        expect(user.name).to eq('School Owner')
-      end
-    end
   end
 
   describe '#schools' do
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
@@ -85,6 +85,7 @@
   config.include PhraseIdentifierMock
   config.include ProfileApiMock
   config.include UserProfileMock
+  config.include UserInfoApiMock
 
   config.include SignInStubs, type: :request
   config.include SignInStubs, type: :system
diff --git a/spec/requests/admin/school_import_results_spec.rb b/spec/requests/admin/school_import_results_spec.rb
@@ -9,13 +9,14 @@
     sign_in_as(admin_user)
 
     # Stub UserInfoApiClient to avoid external API calls
-    allow(UserInfoApiClient).to receive(:fetch_by_ids).and_return([
-                                                                    {
-                                                                      id: admin_user.id,
-                                                                      name: 'Test Admin',
-                                                                      email: 'admin@test.com'
-                                                                    }
-                                                                  ])
+    stub_user_info_api_fetch_by_ids(
+      user_ids: [admin_user.id],
+      users: [{
+        id: admin_user.id,
+        name: 'Test Admin',
+        email: 'admin@test.com'
+      }]
+    )
   end
 
   describe 'GET /admin/school_import_results' do
diff --git a/spec/support/user_info_api_mock.rb b/spec/support/user_info_api_mock.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module UserInfoApiMock
+  def stub_user_info_api_fetch_by_ids(user_ids:, users: [])
+    users = default_stubbed_users(user_ids) if users.empty?
+
+    allow(UserInfoApiClient).to receive(:fetch_by_ids)
+      .with(user_ids)
+      .and_return(users)
+  end
+
+  def stub_user_info_api_find_by_email(email:, user: :not_provided)
+    user = default_stubbed_user_by_email(email) if user == :not_provided
+
+    allow(UserInfoApiClient).to receive(:find_user_by_email)
+      .with(email)
+      .and_return(user)
+  end
+
+  private
+
+  def default_stubbed_users(user_ids)
+    user_ids.map { |user_id| default_stubbed_user(id: user_id, email: "user-#{user_id}@example.com") }
+  end
+
+  def default_stubbed_user_by_email(email)
+    default_stubbed_user(
+      id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE, email),
+      email: email
+    )
+  end
+
+  def default_stubbed_user(id:, email:)
+    {
+      id: id,
+      email: email,
+      username: nil,
+      parental_email: nil,
+      name: 'School Owner',
+      nickname: 'Owner',
+      country: 'United Kingdom',
+      country_code: 'GB',
+      postcode: nil,
+      date_of_birth: nil,
+      verified_at: '2024-01-01T12:00:00.000Z',
+      created_at: '2024-01-01T12:00:00.000Z',
+      updated_at: '2024-01-01T12:00:00.000Z',
+      discarded_at: nil,
+      last_logged_in_at: '2024-01-01T12:00:00.000Z',
+      roles: ''
+    }
+  end
+end

-Original file line number
+Diff line change
       before do
         authenticated_in_hydra_as(admin_user)
 -        allow(UserInfoApiClient).to receive(:find_user_by_email).and_return({ id: SecureRandom.uuid, email: '[email protected]' })
 +        stub_user_info_api_find_by_email(
 +          email: '[email protected]',
 +          user: { id: SecureRandom.uuid, email: '[email protected]' }
 +        )
         allow(SchoolImportJob).to receive(:perform_later).and_return(instance_double(SchoolImportJob, job_id: 'test-job-id'))
       end