504: Update plan_504 method on student record to read EdPlan for Somerville #2315
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
selfie |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #2287.
Who is this PR for?
Somerville educators
What problem does this PR fix?
The
plan_504value coming from the SIS export isn't accurate; it overcounts students who have an ed plan set to "previous" status.What does this PR do?
To work around this, this PR adds a method to the
Studentmodel to patch this for Somerville. We could update the student export to solve this upstream, but this is a faster change and self-contained. Aside from the complexity downside, this also means doing another query for each read ofplan_504, which could be expensive for calls likeStudent.active.as_jsonwith an eager include.This also updates the JS function that decides about showing "504 plan" so that it is stricter, which isn't directly related to the server-side change (since the server-side change for Somerville keeps the same API with the UI).
Checklists
Which features or pages does this PR touch?
Does this PR use tests to help verify we can deploy these changes quickly and confidently?