GibbonEdu / core

Gibbon is a flexible, open source school management platform designed to make life better for teachers, students, parents and leaders.
https://gibbonedu.org
GNU General Public License v3.0
479 stars 304 forks source link

System: move school year session setup to SessionFactory #1688

Closed SKuipers closed 2 years ago

SKuipers commented 2 years ago

After merging #1669 I got looking at the code and there appeared to be some school year setup code that ended up scattered throughout CLI scripts and other locations, rather than in one initialization step.

@yookoala I've moved the setCurrentSchoolYear method out of the SchoolYearGateway into the SessionFactory, as this prevents the Gateway class from needing session dependencies, and puts more of the session initialization code in the SessionFactory, which is then called during the Gibbon initializeCore method. This enabled using a single try-catch for exception handling, instead of needing several throughout the system.

This also has fixed an esoteric bug where the current school year wasn't available directly after logging out unless setCurrentSchoolYear was called manually, so I've been able to remove the code from the sidebar.

Motivation and Context Enabled us to delete even more old code and centralize the session initialization.

How Has This Been Tested? Locally.

yookoala commented 2 years ago

Totally cool change. If fact it's more reasonable than my changes.

You might want to mention this in the phpdocs of the deprecated setCurrentSchoolYear() function. https://github.com/GibbonEdu/core/blob/4a0472077bff1687fd9da87556a1218d6f9d76c4/functions.php#L1311-L1326

SKuipers commented 2 years ago

Thanks! Well caught, I've updated the docs in functions.php 👍

Edit: Oops, the dismissed message is automatic! I wish it would say "SKuipers appreciated yookoala’s review and updated the PR accordingly" 😆