Fix/active enrollment flag enhancement#248
Open
smckee-tnedu wants to merge 3 commits into
Open
Conversation
…ts continue to stay active even when future data starts loading.
This file contains hidden or 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
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.
Description & motivation
Tennessee Districts start loading data for the next school year into a new ODS in the March/April timeframe. They are doing this to get their Calendars ready. Stadium_Tennessee has a suite of business rules which will alert them when their submitted Calendars don't follow TNDOE rules. We use our the front end to EDU that we've coded to show them these errors.
BUT BECAUSE they start loading in data in the March/April timeframe, the current code for fct_student_school_association starts setting active_enrollment to false. There is a portion of the code that checks to see what the latest school year of loaded data is and, for us, since it's future, things get set to false.
I tried to make this fit in with the new "buffer_days" code for 0.6.3, but I admit that I don't totally get the reasoning behind the implementation. So I'm not positive that what I've done doesn't somehow break the intention of that impl.
Also, I'm sorry if this is showing as a merge conflict because of the way I did it. We are on 0.6.2 and I made these changes against what exists in main by just copying the code in your main and applying my logic overtop. I probably could (should?) have cherry picked the proper commit from your branch so that there was no conflict but I'm short on time. You'll have to manually diff your main branch and this change to everything I changed properly. Maybe. I don't know.
Breaking changes introduced by this PR:
The active_enrollment flag might not be set to what people are expecting. But I have no idea. It is working for us, but we don't use buffer_days.
PR Merge Priority:
Changes to existing files:
fct_student_school_association: Modified the logic for is_active_enrollment so that the future school year data doesn't matter. Whether or not an enrollment is active is entirely determined by if the current_date() is both within the enrollment window and the school year window. I didn't change the buffer_days logic, but if I understand it correctly, it should still work as intended.New files created:
None
Tests and QC done:
I have done unit tests on my side and this is working as expected. It is about to go through our QA process though. It has not yet.
edu_wh PR Review Checklist:
Make sure the following have been completed before approving this PR:
## New Featuresfor features, etc.