Archive 21-22 applications and make 22-23 the current application year - #42708
Conversation
724d5fe to
531de77
Compare
… shared teacher strings
we want to be able to instantiate this class with year set to something other than application current year there's some sticky stuff here with regard to static class methods
088f07a to
d8bdc84
Compare
|
requesting a re-review for this PR, I've added a bunch to refactor how we handle the year in the application classes, because I realized that we'll still need these classes to instantiate correctly for older years. This required moving some of the constants around and changing how the year and next_year methods work/get inherited. The applications now default to being created for the current year, but if the db already has a year set, they'll defer to that year (or it can be set directly). I only changed how this worked for teacher and principal applications, as the facilitator applications are unused and should be removed. I'd prefer there to be less static stuff still overall, but I got it to work workable place. |
|
|
||
| def self.next_year | ||
| YEAR_22_23 | ||
| def self.next_year(year) |
There was a problem hiding this comment.
Ooh, I like that this removes another place we would have had to manually update!
| # - two scripts (teacher_applications_to_gdrive and scheduled_pd_application_emails) use the constant | ||
| # - used for regional partner workshop enrollment counts | ||
| APPLICATION_CURRENT_YEAR = YEAR_21_22 | ||
| APPLICATION_CURRENT_YEAR_SHORT = YEAR_21_22_SHORT |
There was a problem hiding this comment.
without these, can we remove include ApplicationConstants?
There was a problem hiding this comment.
Yes! I'll clean that up
davidsbailey
left a comment
davidsbailey
left a comment
There was a problem hiding this comment.
Nice cleanup!
One request for next time if you want me to look at something I've already approved is to @ mention me or click the button to re-request review. Otherwise, if it looks like you're just pushing commits to fix tests and stuff I may unsubscribe from the PR and miss follow-up messages which do not mention me explicitly.
| ].freeze | ||
|
|
||
| APPLICATION_CURRENT_YEAR = YEAR_22_23 | ||
|
|
There was a problem hiding this comment.
it could be helpful to add a quick comment indicating what belongs here vs dashboard/app/models/pd/application/application_constants.rb. are you moving these constants here because you plan to start accessing them from js? are application_constants.rb still shared across multiple application types, just not across ruby + js?
There was a problem hiding this comment.
Mostly this move was for structural reasons. Year was used in some of the strings that were already shared to js, but it was hard coded separately from APPLICATION_CURRENT_YEAR, so I wanted to consolidate them, which required moving it all to shared, because the shared file couldn't reference the other constants file.
3 participants
Flip the active application model current application year constant from 2021-2022 to 2022-2023. This affects the year displayed in the application and the applications that are retrieved for the application dashboard.
Links
Testing story
Tested manually in local environment