Skip to content

Conversation

@danhalson
Copy link
Contributor

@danhalson danhalson commented Oct 21, 2025

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

@cla-bot cla-bot bot added the cla-signed label Oct 21, 2025
@danhalson danhalson self-assigned this Oct 21, 2025
@danhalson danhalson requested a review from Copilot October 22, 2025 10:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the security and reliability of the import process by ensuring import_id is always set during imports, preventing a potential edge case where a nil import_id could result in importing a random record. The changes also improve code readability and expand test coverage by adding an SSO student to the seed data.

  • Validation logic updated to enforce import_id presence during import context
  • Refactored list_students method for better readability
  • Added SSO student to seed data for more comprehensive testing scenarios

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
app/models/school_class.rb Updated validation to require import_id during import context instead of when import_origin is present
spec/models/school_class_spec.rb Removed test case that validated import_id presence when import_origin was set
lib/concepts/school_student/list.rb Refactored to extract students list to intermediate variable for clarity
lib/concepts/school_member/list.rb Added blank line for formatting consistency
lib/tasks/seeds_helper.rb Added emily_ssouser to TEST_USERS hash and included in student assignments
lib/tasks/for_education.rake Added emily_ssouser to student_ids array
lib/tasks/test_seeds.rake Added emily_ssouser to student_ids array

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@jamiebenstead jamiebenstead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danhalson danhalson merged commit 1876049 into main Oct 22, 2025
3 checks passed
@danhalson danhalson deleted the fix-missing-id-handling branch October 22, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants