Skip to content

Add jim dun seed data#686

Merged
mwtrew merged 3 commits intomainfrom
update-test-seed-data
Feb 25, 2026
Merged

Add jim dun seed data#686
mwtrew merged 3 commits intomainfrom
update-test-seed-data

Conversation

@jamiebenstead
Copy link
Contributor

@jamiebenstead jamiebenstead commented Feb 17, 2026

What's changed?

Copilot AI review requested due to automatic review settings February 17, 2026 15:14
@cla-bot cla-bot bot added the cla-signed label Feb 17, 2026
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 adds a new test user jim_dun to the seed data system and updates the test_seeds:destroy rake task to clean up data associated with this user. The PR also includes minor improvements to array syntax in the cleanup logic.

Changes:

  • Added jim_dun test user UUID to TEST_USERS constant in seeds_helper.rb
  • Updated test_seeds:destroy task to clean up roles and optionally schools created by the jim_dun user
  • Refactored array syntax to remove unnecessary nested array wrapping

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
lib/tasks/seeds_helper.rb Adds jim_dun UUID to TEST_USERS hash with comment indicating "user with no school"
lib/tasks/test_seeds.rake Adds cleanup logic for jim_dun's roles and school; refactors array syntax in where clauses

Lesson.where(id: lesson_ids).delete_all

# Destroy the class members and then the class itself
school_class_ids = SchoolClass.where(school_id:).pluck(:id)
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The SchoolClass cleanup only handles the primary school_id, not the teacher_signup_school_id. If jim_dun has created a school with classes and class members, those won't be cleaned up before the school is destroyed at line 40.

Consider extending the cleanup to include both schools:

school_class_ids = SchoolClass.where(school_id: [school_id, teacher_signup_school_id].compact).pluck(:id)
Suggested change
school_class_ids = SchoolClass.where(school_id:).pluck(:id)
school_class_ids = SchoolClass.where(school_id: [school_id, teacher_signup_school_id].compact).pluck(:id)

Copilot uses AI. Check for mistakes.
@mwtrew mwtrew self-assigned this Feb 17, 2026
Copy link
Contributor

@mwtrew mwtrew left a comment

Choose a reason for hiding this comment

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

Could you take a look at these Copilot suggestions before we merge this in?

@jamiebenstead
Copy link
Contributor Author

Could you take a look at these Copilot suggestions before we merge this in?

Just looking at this now. The tests are not running again properly so I'll try fix these

@mwtrew mwtrew merged commit 062a116 into main Feb 25, 2026
6 checks passed
@mwtrew mwtrew deleted the update-test-seed-data branch February 25, 2026 16:46
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