Skip to content

Fix/active enrollment flag enhancement#248

Open
smckee-tnedu wants to merge 3 commits into
edanalytics:mainfrom
tnedu:fix/active_enrollment_flag_enhancement
Open

Fix/active enrollment flag enhancement#248
smckee-tnedu wants to merge 3 commits into
edanalytics:mainfrom
tnedu:fix/active_enrollment_flag_enhancement

Conversation

@smckee-tnedu
Copy link
Copy Markdown
Contributor

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:

  • Low
  • Medium
  • High

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:

  • Description of changes has been added to Unreleased section of CHANGELOG.md. Add under ## New Features for features, etc.
  • Code has been tested/checked for Databricks and Snowflake compatibility - EA engineers see Databricks checklist here
  • Reviewer confirms the grain of all tables are unchanged, OR any changes are expected, communicated, and this PR is flagged as a breaking change (not for patch release)
  • If a new configuration xwalk was added:
    • The code is written such that the xwalk is optional (preferred), and this behavior was tested, OR
    • The code is written such that the xwalk is required, and the required xwalk is added to edu_project_template, and this PR is flagged as breaking change (not for patch release)
    • A description for the new xwalk has been added to EDU documentation site here
  • If a new configuration variable was added:
    • The code is written such that the variable is optional (preferred), and this behavior was tested, OR
    • The code is written such that the variable is required, and a default value was added to edu_project_template, and this PR is flagged as breaking change (not for patch release)
    • A description for the new variable has been added to EDU documentation site here

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.

1 participant