1036: Refactor sso batch to only create new roles where necessary, avoiding an n+1, also refactors to smaller methods to avoid a cyclomatic depdency issue

Author: danhalson

lib/concepts/school_student/create_batch_sso.rb

@@ -32,17 +32,36 @@ def call(school:, school_students_params:, current_user:)
       private
 
       def create_batch_sso(school, students, token)
+        students = normalize_student_params(students)
+        responses = ProfileApiClient.create_school_students_sso(token:, students:, school_id: school.id)
+
+        create_student_roles(school, responses)
+        format_student_responses(responses)
+      rescue ProfileApiClient::Student422Error => e
+        handle_validations(e.errors)
+      end
+
+      def normalize_student_params(students)
         # Ensure that nil values are empty strings, else Profile will swallow validations
-        students = students.map do |student|
+        students.map do |student|
           student.transform_values { |value| value.nil? ? '' : value }
         end
+      end
 
-        responses = ProfileApiClient.create_school_students_sso(token:, students:, school_id: school.id)
+      def create_student_roles(school, responses)
+        # Bulk check which roles already exist to avoid N+1 queries
+        user_ids = responses.pluck(:id)
+        existing_roles = Role.student.where(school_id: school.id, user_id: user_ids).index_by(&:user_id)
+
+        # Create only the missing roles
+        user_ids.each do |user_id|
+          next if existing_roles[user_id]
 
-        responses.each do |student|
-          Role.student.find_or_create_by(school_id: school.id, user_id: student[:id])
+          Role.create!(role: :student, school_id: school.id, user_id: user_id)
         end
+      end
 
+      def format_student_responses(responses)
         # Convert hash responses to User objects with separate metadata
         # This separates student data from metadata (success, error, created flags)
         responses.map do |student_data|
@@ -53,8 +72,6 @@ def create_batch_sso(school, students, token)
             created: student_data[:created]
           }
         end
-      rescue ProfileApiClient::Student422Error => e
-        handle_validations(e.errors)
       end
 
       def handle_validations(errors)

spec/concepts/school_student/create_batch_sso_spec.rb

@@ -23,6 +23,8 @@
     let(:user_ids) { [SecureRandom.uuid, SecureRandom.uuid] }
 
     before do
+      # Force memoization of user_ids before stub so they're consistent
+      user_ids
       stub_profile_api_create_school_students_sso(user_ids:)
     end
 
@@ -83,6 +85,39 @@
       expect(second_student_item[:student].name).to eq('SSO Test Student 2')
       expect(second_student_item[:success]).to be(true)
     end
+
+    context 'when roles already exist for some students' do
+      let(:user_ids) { [SecureRandom.uuid, SecureRandom.uuid] }
+
+      before do
+        # Pre-create a role for the first student
+        Role.create!(role: :student, school_id: school.id, user_id: user_ids[0])
+      end
+
+      it 'does not duplicate existing roles' do
+        roles_before_call = Role.student.where(school_id: school.id).to_a
+
+        described_class.call(school:, school_students_params:, current_user:)
+
+        roles_after_call = Role.student.where(school_id: school.id).to_a
+        new_student_roles = roles_after_call - roles_before_call
+
+        # Should only create 1 new student role (for second student) since first already exists
+        expect(new_student_roles.length).to eq(1)
+        expect(new_student_roles.first.user_id).to eq(user_ids[1])
+      end
+
+      it 'does not raise an error' do
+        expect do
+          described_class.call(school:, school_students_params:, current_user:)
+        end.not_to raise_error
+      end
+
+      it 'still returns all students in the response' do
+        response = described_class.call(school:, school_students_params:, current_user:)
+        expect(response[:school_students].length).to eq(2)
+      end
+    end
   end
 
   context 'when validation errors occur' do