Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CST-1207: rectify mentor pools when a participant is rectified #2913

Conversation

ltello
Copy link
Contributor

@ltello ltello commented Jan 18, 2023

Context

When a school gets closed their participants are moved to its successor but the mentor pools are left unchanged.

Changes proposed in this pull request

Rectify the mentor pools of the school and its successor so the entries are moved from one to the other.

Guidance to review

@ltello ltello force-pushed the CST-1207-ensure-that-mentor-pools-transfer-over-when-a-schools-gias-record-updates branch from 19bbd7e to d471f8c Compare January 18, 2023 18:16
@github-actions
Copy link

Created review app at https://ecf-review-pr-2913.london.cloudapps.digital

@github-actions
Copy link

Smoke tests passed against the review app.

@ltello ltello marked this pull request as ready for review January 18, 2023 19:21
Copy link
Contributor

@tonyheadford tonyheadford left a comment

Choose a reason for hiding this comment

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

Looks good, just one comment about the by scope.

@@ -5,5 +5,6 @@ class SchoolMentor < ApplicationRecord
belongs_to :participant_profile, class_name: "ParticipantProfile::Mentor"
belongs_to :preferred_identity, class_name: "ParticipantIdentity"

scope :by, ->(participant_profile) { where(participant_profile:) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure of the value of this as it is only a simple where(participant_profile:) and is a bit ambiguous, what if I want want to look up by school? I get it's adding a bit of sugar but it then also becomes something else to remember.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, code rewritten without the scope.

@ltello ltello force-pushed the CST-1207-ensure-that-mentor-pools-transfer-over-when-a-schools-gias-record-updates branch from d471f8c to ead966b Compare January 19, 2023 13:50
Copy link
Contributor

@tonyheadford tonyheadford left a comment

Choose a reason for hiding this comment

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

Nice!

@ltello ltello merged commit 1dfde2a into develop Jan 19, 2023
@ltello ltello deleted the CST-1207-ensure-that-mentor-pools-transfer-over-when-a-schools-gias-record-updates branch January 19, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants