Fix-missing-id-handling (#603)

## Status

Ready for review

## What's changed?

- Ensures import_id is set on import, and prevent the edge case of a nil
import_id being passed and a random record being import
- Improve readability of list_student method
- Add the SSO student to the seeds

## Steps to perform after deploying to production

N/A

Author: danhalson

app/models/school_class.rb

@@ -19,9 +19,9 @@ class SchoolClass < ApplicationRecord
   enum :import_origin, { google_classroom: 0 }, validate: { allow_nil: true }
 
   validates :import_origin, presence: true, on: :import
+  validates :import_id, presence: true, on: :import
 
   validates :import_id, uniqueness: { scope: %i[school_id import_origin] }, if: -> { import_id.present? }
-  validates :import_id, presence: true, if: -> { import_origin.present? }
 
   has_paper_trail(
     meta: {

lib/concepts/school_member/list.rb

@@ -37,6 +37,7 @@ def call(school:, token:)
 
       def fetch_students(school:, token:)
         student_roles = Role.student.where(school:)
+
         students_response = student_roles.any? ? SchoolStudent::List.call(school:, token:).fetch(:school_students, []) : []
 
         students_response.map do |student|

lib/concepts/school_student/list.rb

@@ -17,10 +17,9 @@ def call(school:, token:, student_ids: nil)
 
       def list_students(school, token, student_ids)
         student_ids ||= Role.student.where(school:).map(&:user_id)
-        ProfileApiClient.list_school_students(token:, school_id: school.id, student_ids:).map do |student|
-          User.new(student.to_h.slice(:id, :username, :name, :email).merge(
-                     sso_providers: student.ssoProviders
-                   ))
+        students = ProfileApiClient.list_school_students(token:, school_id: school.id, student_ids:)
+        students.map do |student|
+          User.new(student.to_h.slice(:id, :username, :name, :email).merge(sso_providers: student.ssoProviders))
         end
       end
     end

lib/tasks/for_education.rake

@@ -21,7 +21,7 @@ namespace :for_education do
       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]]
+      student_ids = [TEST_USERS[:jane_smith], TEST_USERS[:john_smith], TEST_USERS[:emily_ssouser]]
       school_id = TEST_SCHOOL
 
       # Remove the roles first

lib/tasks/seeds_helper.rb

@@ -5,7 +5,8 @@ module SeedsHelper
     jane_doe: '583ba872-b16e-46e1-9f7d-df89d267550d', # jane.doe@example.com
     john_doe: 'bbb9b8fd-f357-4238-983d-6f87b99bdbb2', # john.doe@example.com
     jane_smith: 'e52de409-9210-4e94-b08c-dd11439e07d9', # student
-    john_smith: '0d488bec-b10d-46d3-b6f3-4cddf5d90c71' # student
+    john_smith: '0d488bec-b10d-46d3-b6f3-4cddf5d90c71', # student
+    emily_ssouser: '88e0aed6-8f20-4e40-98f9-610a0ab1cfcc' # sso student
   }.freeze
 
   # Match the school in profile...
@@ -64,7 +65,7 @@ def assign_a_teacher(user_id, school)
   end
 
   def assign_students(school_class, school)
-    [TEST_USERS[:jane_smith], TEST_USERS[:john_smith]].map do |student_id|
+    [TEST_USERS[:jane_smith], TEST_USERS[:john_smith], TEST_USERS[:emily_ssouser]].map do |student_id|
       Rails.logger.info 'Assigning student role...'
       Role.student.find_or_create_by!(user_id: student_id, school:)
 

lib/tasks/test_seeds.rake

@@ -14,7 +14,7 @@ namespace :test_seeds do
       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]]
+      student_ids = [TEST_USERS[:jane_smith], TEST_USERS[:john_smith], TEST_USERS[:emily_ssouser]]
       school_id = TEST_SCHOOL
 
       # Remove the roles first

spec/models/school_class_spec.rb

@@ -112,13 +112,6 @@
       expect(school_class.errors[:import_origin]).to include('is not included in the list')
     end
 
-    it 'requires import_id when import_origin is set' do
-      school_class.import_origin = :google_classroom
-      school_class.import_id = nil
-      expect(school_class).not_to be_valid
-      expect(school_class.errors[:import_id]).to include("can't be blank")
-    end
-
     it 'allows import_id to be nil when import_origin is nil' do
       school_class.import_origin = nil
       school_class.import_id = nil