SDSM:Historical Changes: Difference between revisions
(Rearrange entries by splitting off a slow performance fixes main section from the broken behaviour main section.) |
(Add item about decoupling the main menu builder and the report writer screen.) |
||
(7 intermediate revisions by the same user not shown) | |||
Line 21: | Line 21: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
== | == Fixes to Broken Behavior == | ||
=== 2024 Apr 18: User Preferences Screen Fails to Display === | === 2024 Apr 18: SDS Laravel: User Preferences Screen Fails to Display === | ||
'''Context:''' User <code>darren.duncan</code> attempted to visit their | '''Context:''' User <code>darren.duncan</code> attempted to visit their | ||
User Preferences screen, example urls | ''SDS Laravel'' User Preferences screen, example urls | ||
<code>https://api.smus.ca/User/Preferences</code> (production) and | <code>https://api.smus.ca/User/Preferences</code> (production) and | ||
<code>https://api-latest.smus.ca/User/Preferences</code> (preview). | <code>https://api-latest.smus.ca/User/Preferences</code> (preview). | ||
Line 58: | Line 58: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
=== 2024 May 21: Timeout Feature Broken In Sub-Directory Hosted Apps === | === 2024 May 21: SDS Laravel: Timeout Feature Broken In Sub-Directory Hosted Apps === | ||
''SDS Laravel'' has a security feature such that if a user is inactive for | ''SDS Laravel'' has a security feature such that if a user is inactive for | ||
Line 113: | Line 113: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
=== 2024 Jun 12: Form Meister Screen Fails to Display === | === 2024 Jun 12: SDS Laravel: Form Meister Screen Fails to Display === | ||
This task made 2 distinct changes. | This task made 2 distinct changes. | ||
Line 149: | Line 149: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
=== 2024 Jul 4: Functional Screens Not Distinguished in Main Menu Bar === | === 2024 Jul 4: SDS Laravel: Functional Screens Not Distinguished in Main Menu Bar === | ||
The main navigation menus of ''SDS Laravel'' declare a hierarchical list of | The main navigation menus of ''SDS Laravel'' declare a hierarchical list of | ||
Line 197: | Line 197: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
== SDS Laravel: | === 2024 Jul 23: SDS Laravel: Form Campaign Add Target Students Uncheck All Button Fails === | ||
=== 2024 May 14: Main Menu Bar Heavy Database Use Very Slow Page Loads === | The ''SDS Laravel'' screen Admin Menu -> Online Forms -> Form Campaign | ||
-> Consolidated Health Form -> Manage has broken behavior. | |||
The "Add New Targets" screen section presents a list of students with a | |||
checkbox for each student. When visiting the screen, the checkboxes all | |||
default to a checked state. Often this is what the user wants but sometimes | |||
they want to just pick a few students instead, and to assist with this is | |||
an "Uncheck All" button. | |||
Clicking the "Uncheck All" button as the user's first action on this screen | |||
succeeds. But if any boxes are manually checked afterwards, clicking | |||
"Uncheck All" again does nothing, and those boxes are still checked. | |||
Furthermore, if ''SDS Laravel'' is updated to use Vue 3 from its current | |||
Vue 2, the same functionality manifests additional broken behavior such | |||
that the "Uncheck All" button has no effect even on initially visiting the | |||
page, so it doesn't even work a single time to uncheck all the boxes. | |||
This task fixes the broken behaviour by just making a few small updates in | |||
the 1 PHP source file | |||
<code>resources/views/admin/online_forms/form_campaigns/show.blade.php</code>. | |||
Primarily there was this 1 substitution in the "Uncheck All" button definition: | |||
onclick="$('#student_list input:checkbox').attr('checked', false);" | |||
onclick="Array.from(document.getElementsByClassName('checkbox_student_list_add_target')).forEach(function(checkbox) { checkbox.checked = false; });" | |||
In support of that, there was also this 1 addition to each student checkbox: | |||
class="checkbox_student_list_add_target" | |||
Those 2 line changes are all that was needed to fix the broken behavior. | |||
The underlying problem appears to be that jQuery's mechanism to enumerate | |||
the set of checkbox DOM nodes fails to find them when any environmental | |||
change occurs such as when users click the checkboxes, so the JavaScript | |||
code to uncheck those boxes acts like the checkboxes don't exist. | |||
Note that built-ins like <code>querySelectorAll()</code> have the same problem. | |||
Whereas, <code>getElementsByClassName()</code> is resilient to such | |||
environmental changes and can still find the checkboxes after user actions. | |||
This task also makes a user interface change to enhance users' experience, | |||
which is to add a complementing "Check All" button. | |||
[[#top|RETURN]] | |||
=== 2024 Aug 15: SDS Gavintech: Input Absences Screen Campus Filter Not Sticky and Other Bugs === | |||
The ''SDS Gavintech'' screen Staff Menu -> Attendance -> Input Absences has | |||
several bugs or otherwise sub-optimal behaviors, and this task makes | |||
7+ distinct changes to respectively fix or improve those. | |||
All of the changes are isolated to the single PHP source file | |||
<code>pages/attendance/input_absence.php</code> and so should not impact | |||
any part of SDS besides this one screen. | |||
The first/primary change... | |||
The filter toggle "Show all campuses"/"Restrict to campus only" had a bug | |||
such that the setting of this toggle was not sticky; as soon as the user | |||
took any action which reloaded the screen, such as adding/editing/deleting | |||
an input absence record, or such as flipping the filter toggle "Day | |||
students only"/"All students", the campus toggle would reset to its default | |||
of showing the current campus only. Besides this going against user | |||
expectations and making more work for them, it is also inconsistent with | |||
the behavior of the day students toggle which generally did persist through | |||
any corresponding user actions. | |||
This task normalized the behavior of the screen so that both of these | |||
filter toggles behave the same and persist through any actions the user | |||
takes on the screen; they would only reset to defaults if the user leaves | |||
the screen entirely and returns. | |||
In particular, this task's change makes things better because if one uses | |||
the screen to add an absence for a student in another campus, then they can | |||
actually see the record they just added on the screen after they submitted | |||
the form, while otherwise it would look more like their action had no | |||
effect, besides the transient status message of "Success: Added entries". | |||
The second change... | |||
This task also made a behavior change such that the current setting of the | |||
campus filter toggle is now reflected in the title of the screen with a | |||
"(campus only)" or "(all campuses)" suffix, so its more clear to users what | |||
the setting is. This also makes the screen more consistent with the other | |||
filters, such that the day students filter had a corresponding title suffix | |||
already, "(day students)" and "(all students)" respectively, and the date | |||
filter which shows the date its filtered to. | |||
The third change... | |||
This task also made a behavior change such that the value of the "Set date" | |||
form/button now defaults to the same date the screen is currently | |||
displaying, rather than the real-world current date, which is effectively | |||
like the screen not persisting their choice to an extent. | |||
I feel that behavior is more useful and in line with user expectations or | |||
less surprising to them. It also means that this form is more thoroughly a | |||
localized version of the application global "Date" setting. That is, | |||
changing the date globally also affects this screen, but changing it on | |||
this screen only affects this screen, until the user leaves this screen and | |||
it resets to match the global setting. | |||
The fourth change... | |||
This task also made a behavior change such that the link to "Print list of | |||
absent day students" has been renamed to "Print list of absent students", | |||
and it now displays unconditionally instead of only when the screen is | |||
filtered to just day students. | |||
This new behavior was deemed more useful such as if we have an emergency on | |||
campus and we have to evacuate, a full list would indicate day students | |||
with absences entered by Ally/Bonita and boarding student absences entered | |||
by the Health Centre. | |||
However, there was no behavior change with respect to what actually | |||
happened when that link was clicked on; because of a prior bug, the link | |||
always led to a list of all students anyway, rather than filtering to show | |||
just day students. | |||
The fifth change... | |||
This task also fixed a bug in the "Existing absences" -> "Entered by" | |||
column such that the HTML for the link to email the person was malformed | |||
and the url did not go anywhere / got a 404, but with the fix it correctly | |||
goes to the email screen. | |||
The sixth change... | |||
This task also removed the link "Email Advisors & Students", which was | |||
never implemented and did nothing but reload the page when clicked on. | |||
The seventh+ changes... | |||
This task also further normalizes how urls are generated that point back to | |||
the current screen, reducing duplication of repeated details, and also | |||
normalizing that arguments for filters only appear in urls if they did | |||
before or are not defaults. | |||
This task also normalizes some other implementation code, such as | |||
specifying that all the "Dropdown" calls return their results rather than | |||
printing them. | |||
[[#top|RETURN]] | |||
== Fixes to Slow Performance == | |||
=== 2024 May 14: SDS Laravel: Main Menu Bar Heavy Database Use Very Slow Page Loads === | |||
''SDS Laravel'' currently relies on the "pages" database table to | ''SDS Laravel'' currently relies on the "pages" database table to | ||
Line 335: | Line 483: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
== SDS | === 2024 Aug 20: SDS Gavintech: Input Absences Screen Heavy Database Use Very Slow === | ||
The ''SDS Gavintech'' app has widespread structural inefficiencies in the | |||
way that it uses the database, and this manifests as the primary cause of | |||
very slow app performance for users. The app may make 1000s or 10000s of | |||
distinct database calls every time a user visits any screen or performs any | |||
action on the screen that involves reloading the page, when it should only | |||
be making on the order of 1-20 database calls total each such time. As a | |||
result, each user action that loads/reloads a screen may carry a delay of | |||
1-2 seconds at the bare minimum and often 10-20 seconds or several minutes, | |||
which cumulatively is a very poor user experience. | |||
To be specific, that was the experience of the production SDS instance in | |||
early June of 2024. A pending migration as of this writing to newer | |||
production hardware would yield performance improvements of 2-5X alone, | |||
which helps significantly, but is still very noticeably slow. In contrast, | |||
changes to the app source code to correct the structural inefficiencies | |||
would instead yield further performance improvements in the 15X+ range, such | |||
that user actions then actually may seem instant. Meanwhile, any specific | |||
metrics given below for this task are all using the same newer hardware, to | |||
be apples to apples comparisons, and also each version is refreshed several | |||
times and the shortest generation time is used. | |||
This task makes several distinct changes specific/isolated to the 1 screen | |||
Staff Menu -> Attendance -> Input Absences such that the logic specific to | |||
that screen is optimized in how it uses the database. | |||
As a result of this task's changes, in order to display the screen, a total | |||
of exactly 8-9 database calls are made the screen-specific elements (4 | |||
trivial ones plus 4-5 main ones), rather than about 9594 calls. | |||
Further, as a result of this task's changes, in order to bulk-add new | |||
absence records, a total in typical cases of exactly 4 database calls are | |||
made for the database updating logic (2 trivial ones plus 2 main ones), or | |||
a typical extreme maximum of about 24 database calls if adding around 1100 | |||
students, rather than a minimum of 6 calls and a maximum of about 4000-5000 | |||
respectively. | |||
As a result of this task's changes, all user actions on the Input Absences | |||
screen that involve a screen load/reload now take about 0.1 seconds, which | |||
is essentially immediately, including displaying and bulk adding, as far as | |||
the work specific to the screen itself is concerned. Unlike the old logic, | |||
the new logic should scale and have essentially this same speed no matter | |||
how many students there are, rather than slowing down with more students. | |||
This should be very noticeable to users, as every single individual action | |||
should feel a lot faster, which is even more significant given a typical | |||
user workflow of performing dozens to a hundred actions here each day. | |||
As a result of this task's changes, the amount of server memory used also | |||
goes down by orders of magnitude, from about 168MB to about 4MB usually | |||
(2MB in some cases), a 42X savings, as far as the work specific to the | |||
screen itself is concerned. If widely applied, such changes can also mean | |||
SDS can scale to handle orders of magnitude more concurrent users on the | |||
same hardware without users seeing further slowdowns due to competition for | |||
resources with other concurrent users. | |||
All of the changes are isolated to the single PHP source file | |||
<code>pages/attendance/input_absence.php</code> and so should not impact | |||
any part of SDS besides this one screen. | |||
Prior to the changes of this task... | |||
This is the overhead for every screen minus that specific to the main menus; | |||
it comprises 16 generally distinct simple database calls: | |||
Page generation time: 0.011 | |||
Total SQL Queries: 16 | |||
Total SQL Cache Hits: 15 | |||
Total Local Cache Hits: 2 | |||
Total Memcache Hits: | |||
Memory Usage: 2048K | |||
Realpath Usage: 87K | |||
This is the overhead for every screen inclusive of the main menus, | |||
when using SDS with the effective date of 2024 May 6 and | |||
as a test/staff user "Darren Duncan" that is not a teacher/parent/student; | |||
the main menus alone add 1567 database calls: | |||
=== 2024 Apr 19: Consolidate Eloquent model classes to /app/Models === | Page generation time: 0.231 | ||
Total SQL Queries: 1583 | |||
Total SQL Cache Hits: 7078 | |||
Total Local Cache Hits: 10 | |||
Total Memcache Hits: | |||
Memory Usage: 32768K | |||
Realpath Usage: 125K | |||
This is the overhead for every screen inclusive of the main menus, | |||
when using SDS with the effective date of 2024 May 6 and | |||
as a teacher user "Carol Adamson", | |||
who also has a "My Courses" main menu but lacks some other staff menu items; | |||
the main menus alone add 2049 database calls: | |||
Page generation time: 0.27 | |||
Total SQL Queries: 2065 | |||
Total SQL Cache Hits: 9188 | |||
Total Local Cache Hits: 65 | |||
Total Memcache Hits: | |||
Memory Usage: 57344K | |||
Realpath Usage: 87K | |||
This is Input Absences screen on effective date 2024 May 6 by user Darren | |||
Duncan including all the above overhead (user Carol Adamson doesn't have | |||
privileges to see this screen); | |||
the initial filter settings of current/Senior campus only plus all | |||
day plus boarding students (about 600 students) | |||
there are 10131 database calls total: | |||
Page generation time: 1.471 | |||
Total SQL Queries: 10131 | |||
Total SQL Cache Hits: 31468 | |||
Total Local Cache Hits: 5351 | |||
Total Memcache Hits: | |||
Memory Usage: 180224K | |||
Realpath Usage: 125K | |||
This is the same Input Absences screen initial load but with the main menu | |||
generation disabled, so it is more clear what this screen alone is doing; | |||
there are 9594 database calls that are specific to this screen; | |||
many of those calls are the same as ones in the main menu generator, and | |||
those same calls are cached / only made once when both the main menu | |||
generator and this screen's logic are enabled: | |||
Page generation time: 1.406 | |||
Total SQL Queries: 9610 | |||
Total SQL Cache Hits: 23359 | |||
Total Local Cache Hits: 5343 | |||
Total Memcache Hits: | |||
Memory Usage: 167936K | |||
Realpath Usage: 126K | |||
Note that all further numbers below likewise have the main menu generation | |||
disabled, to just highlight what Input Absences alone is doing. | |||
This is the same thing but with the campus filter set to all campuses | |||
and showing both day and boarding students (about 1000 students); | |||
there are 3514 database calls specific to this screen; | |||
there are actually 2/3 fewer database calls despite showing more students: | |||
Page generation time: 0.499 | |||
Total SQL Queries: 3530 | |||
Total SQL Cache Hits: 17071 | |||
Total Local Cache Hits: 289 | |||
Total Memcache Hits: | |||
Memory Usage: 65536K | |||
Realpath Usage: 88K | |||
This is the same thing but with the filter set to day students only, | |||
and campus filter still main only; | |||
there are 9541 database calls specific to this screen; | |||
unlike the campus filter, the day student filter only has a small effect: | |||
Page generation time: 1.462 | |||
Total SQL Queries: 9557 | |||
Total SQL Cache Hits: 20482 | |||
Total Local Cache Hits: 5280 | |||
Total Memcache Hits: | |||
Memory Usage: 165888K | |||
Realpath Usage: 126K | |||
This is the same thing but with both filters toggled such that it shows | |||
day students only plus all campuses; | |||
there are 2942 database calls specific to this screen: | |||
Page generation time: 0.469 | |||
Total SQL Queries: 2958 | |||
Total SQL Cache Hits: 17827 | |||
Total Local Cache Hits: 217 | |||
Total Memcache Hits: | |||
Memory Usage: 55296K | |||
Realpath Usage: 89K | |||
About 80-90% of the database calls specific to this screen are about | |||
fetching a list of currently enrolled students for the effective date, with | |||
or without campus/day filters, and using that list both to populate a | |||
picklist of students and also to filter a display of existing absense | |||
records, because it is making a handful of separate database calls for each | |||
of about 1000 students (both when all 1000 are used and when some aren't). | |||
It also makes a handful of separate database calls for each existing | |||
absense record displayed, but these tend to be on the order of dozens of | |||
records instead of 1000. | |||
When the user performs an action like adding/editing/deleting an absence | |||
record on this screen, the actual database modification task is typically | |||
very fast, assuming just a single student updated, usually around 2-6 | |||
database calls for that, but then the page reloads carrying the large | |||
aforementioned overheads. As an exception to this, when the user performs | |||
a bulk addition of absence records, by selecting multiple students, each | |||
individual student involves 4-5 database calls, of which 2 are reads and | |||
2-3 are updates, and so in the unusual case of adding all thousand at once, | |||
it can make around 4000-5000 database calls, and take around 15 seconds on | |||
the newer hardware to do so; and then the page reloads. | |||
Following the changes of this task... | |||
This is the same thing, on just displaying the Input Absences screen, | |||
for effective date 2024 May 6, for user Darren Duncan; | |||
there are 8-9 database calls specific to this screen; | |||
it is 9 when filtering to day students only, and 8 otherwise; | |||
it is unchanged by filtering on one or all campuses: | |||
Page generation time: 0.093 | |||
Total SQL Queries: 24 | |||
Total SQL Cache Hits: 25 | |||
Total Local Cache Hits: 5 | |||
Total Memcache Hits: | |||
Memory Usage: 4096K | |||
Realpath Usage: 54K | |||
Page generation time: 0.108 | |||
Total SQL Queries: 25 | |||
Total SQL Cache Hits: 26 | |||
Total Local Cache Hits: 5 | |||
Total Memcache Hits:ƒ | |||
Memory Usage: 2048K | |||
Realpath Usage: 92K | |||
In addition, the logic for batch-adding adds almost no time to that in | |||
typical cases of 1 or few students, and otherwise a small fraction of 1 | |||
second for the extreme case of bulk-adding all thousand students at once. | |||
The main general change made by this task is to clone several call chains | |||
worth of source code for several dozen routines across a handful of classes | |||
into <code>input_absence.php</code> and iteratively refactoring that whole | |||
code mass to its most concise specific-to-this-screen's-use-case version. | |||
No code shared with other screens was altered, allowing a guarantee that | |||
no other screens are affected by these changes, making it much lower risk, | |||
and a much simpler task to accomplish. | |||
The most important effect of the main change is the consolidation of | |||
database calls. Typically how the logic worked before this task's changes | |||
was that in order to produce a list of something, first you had a query | |||
that just produced a set of record ids for the list we wanted, and then a | |||
separate query was made for each of those ids to get other fields we want | |||
for each record, sometimes in one call and sometimes in a handful, some for | |||
display purposes, and others for filtering purposes. Hence you may have | |||
around 3000-8000 calls to deal with a list of around 1000 students. This | |||
task changed things so that exactly 1 SQL query was done instead in place | |||
of that first id-fetching query that also gathered all the related details. | |||
The 4-5 display-only main queries then were for these purposes: | |||
* One to derive the input time range for the effective date, eg 8:30am-3:20pm, from a period list. | |||
* One to display the picklist of students to add/edit absences for. | |||
* One to display the picklist of attendence codes. | |||
* One to display the list of existing absence records. | |||
* Iff filtering students by day, one to fetch a list of fee statuses to filter with. | |||
(While in theory the last one could also have been inlined, I chose not to | |||
because it was relating fee statuses with "like" rather than equality.) | |||
There was also these 3 add-only SQL statements per each batch of 100 students: | |||
* One to test that the user-supplied student ids match student records. | |||
* One to insert up to 100 absence records. | |||
* An indirectly called one to insert a log entry for the prior insert. | |||
In the interest of security and scalability I compromised my adding logic | |||
between the pre-changes version of a separate database call per single | |||
student and using a single database call for the entire list of 1000+ | |||
students, into batches of 100, which is essentially the same performance as | |||
all-in-one but shouldn't be as likely to fail in pathological cases. | |||
A related general change is that the main screen logic no longer turns | |||
database records into cached PHP objects that it then calls methods on, and | |||
instead just works with the records as regular PHP arrays which typically | |||
don't correspond to single database tables but joins of several. The logic | |||
goes from interacting heavily with the app's record object cache to | |||
generally not interacting with it at all, either to add or remove items. | |||
In particular, the logic no longer has up to a handful of repeatedly | |||
converting back and forth between record ids and record objects per record. | |||
Between this, and the greatly reduced use of the app's SQL query cache due | |||
to fewer database calls, likely is the main reason why so much less memory | |||
is used by PHP in the new version. | |||
Note that while some database queries or other logic could possibly be | |||
streamlined or simplified further, I tried to maintain the exact effective | |||
logic as possible from before, in case there might be subtle behavior | |||
changes from such further streamlining. But an actual future rewrite of | |||
this screen such as under ''SDS Laravel'' could likely revisit this. | |||
Generally speaking, the screen logic I overhauled was that where many | |||
similar database queries could be consolidated, while logic that only had | |||
individual distinct queries, such as related to getting the current school | |||
day, I left alone. | |||
[[#top|RETURN]] | |||
=== 2024 Aug 20: SDS Gavintech: Attendance Code / Reason Picklists Slow === | |||
This task changes the ''SDS Gavintech'' app shared component that is used | |||
by all or most screens with an Attendance Code / Reason picklist so that it | |||
internally uses the database more efficiently and thus performs better. | |||
For a typical scenario of displaying 20 attendance codes, it now performs | |||
this using exactly 1 database call rather than 21 database calls. | |||
This benefit is seen on every screen using this picklist, not just 1. | |||
While the impact of this change alone on a single user action is likely too | |||
small for a user to notice, it still contributes to efficiency, and in | |||
aggregate, performance improvements users would notice. | |||
That change was entirely in the single PHP source file | |||
<code>overrides/classes/class_dropdowns.php</code>, in its single routine | |||
<code>attendanceCode()</code>. | |||
This task also changes the internals of the 1 screen Staff Menu -> | |||
Attendance -> Input Absences such that it now uses the same, now improved, | |||
app component for its Reason picklist, rather than duplicating it. | |||
That change was entirely in the single PHP source file | |||
<code>pages/attendance/input_absence.php</code>. | |||
[[#top|RETURN]] | |||
== Internal Design Changes == | |||
=== 2024 Apr 19: SDS Laravel: Consolidate Eloquent model classes to /app/Models === | |||
An idiomatic PHP Laravel project has its main files grouped into the | An idiomatic PHP Laravel project has its main files grouped into the | ||
Line 364: | Line 826: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
=== 2024 May 8: Sever Coupling of PHP Class Names to Database Data === | === 2024 May 8: SDS Laravel: Sever Coupling of PHP Class Names to Database Data === | ||
Laravel Eloquent has a particular more-advanced feature for defining | Laravel Eloquent has a particular more-advanced feature for defining | ||
Line 464: | Line 926: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
== SDS | === 2024 Aug 21: SDS Gavintech: Sever Coupling of Main Menu Builder to Write/Edit Reports Screen === | ||
=== 2024 May 3: Stop | This task refactored ''SDS Gavintech'' to sever the coupling between the | ||
main menu builder, defined in the single PHP source file | |||
<code>overrides/modules/sidebar_definition/smus_sidebar_definition.php</code>, | |||
and the screen known alternately as | |||
My Courses -> $course_name -> Reports & Marks -> Write reports -> $student_name | |||
and | |||
Staff Menu -> Advisor stuff -> Edit Reports -> $student_name -> $course_name, | |||
defined in the single PHP source file | |||
<code>pages/reports/reports_entry_spell.php</code>. | |||
That coupling took the form of the main menu builder fetching a list of | |||
students from the database on behalf of the screen and storing that list of | |||
students in the PHP session variable | |||
<code>$_SESSION["reports_entry_spell"]</code>, which was subsequently read | |||
by the screen. The use of this session variable was one-way; the menu | |||
builder only set it and the screen only read it. The menu builder set it | |||
unconditionally from scratch, and so could have just used a PHP "global" | |||
instead, which would have saved the app from the overhead of serializing | |||
and deserializing its value to disk, which may have consisted of a thousand | |||
"Student" PHP objects. The menu builder set it anew every time the user | |||
loaded/reloaded any app screen, and not just when viewing the specific | |||
screen it was used on, meaning almost all the time it consumed resources to | |||
do that work for nothing. | |||
The changes of this task consisted mainly of placing a stripped down clone | |||
of the main menu builder source file into the top of the screen source | |||
file, just the parts that built the student list. The changes otherwise | |||
consisted of entirely eliminating the use of the PHP session variable. | |||
The menu builder source file had no changes except for removing the 2 lines | |||
that set the session variable; the screen source file used a local variable | |||
to hold the student list instead of reading it from the session variable; | |||
the stripped down clone it gained totalled 70 lines of code, which was | |||
8% of the menu builder source file's lines. | |||
Following the changes of this task, the actual performance of the app was | |||
identical, using exactly the same count of database calls per each screen | |||
as prior to the changes of this task, and also the same page generation | |||
time. This was tested both on the app home screen, which used the menu | |||
builder logic but not the screen logic, and on the screen, which used both. | |||
Despite the duplicate database call main logic, this would be explained by | |||
these database calls being mediated by both an app SQL query cache and an | |||
app PHP object cache, which recognized that the results of the first | |||
instance of the calls by the menu builder could be reused by the screen. | |||
This decoupling resulted in the app being easier to maintain, as one can | |||
now change either the menu builder or the screen arbitrarily without being | |||
concerned that changes to one would affect the other, as conceptually the | |||
two things should be completely separate. | |||
[[#top|RETURN]] | |||
== Changes to Third-Party Dependencies == | |||
=== 2024 May 3: SDS Laravel: Stop Requiring dbal, revisionable-upgrade, laravel-cors === | |||
Prior to the performance of this task, the ''SDS Laravel'' Composer config | Prior to the performance of this task, the ''SDS Laravel'' Composer config | ||
Line 482: | Line 998: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
=== 2024 May 6: Reflect rename of fzaninotto to fakerphp === | === 2024 May 6: SDS Laravel: Reflect rename of fzaninotto to fakerphp === | ||
This task updated <code>composer.json</code> to reflect that the | This task updated ''SDS Laravel'' <code>composer.json</code> to reflect that | ||
third-party PHP library dependency <code>fzaninotto/faker</code> was | the third-party PHP library dependency <code>fzaninotto/faker</code> was | ||
renamed to <code>fakerphp/faker</code> (in fact, that rename occurred prior | renamed to <code>fakerphp/faker</code> (in fact, that rename occurred prior | ||
to the release of Laravel 8), and also require its latest version. | to the release of Laravel 8), and also require its latest version. | ||
Line 491: | Line 1,007: | ||
[[#top|RETURN]] | [[#top|RETURN]] | ||
=== 2024 May 6: Upgrade PHPUnit from 9.x to 10.x === | === 2024 May 6: SDS Laravel: Upgrade PHPUnit from 9.x to 10.x === | ||
This task updated ''SDS Laravel'' <code>composer.json</code> to require the | |||
latest PHP-8.1-compatible version of the PHP dev dependency PHPUnit from | |||
<code>9.6.19</code> to <code>10.5.20</code>. | |||
[[#top|RETURN]] | |||
=== 2024 Jul 15: SDS Laravel: Upgrade Laravel Mix from 5.x to 6.x and Upgrade Webpack === | |||
This task updated ''SDS Laravel'' so that its JavaScript build pipeline is | |||
implemented by Laravel Mix 6 rather than 5, and likewise a newer major | |||
version of Webpack. | |||
This task changed these dependencies in <code>package.json</code>: | |||
* cross-env (^7.0.3 removed as laravel-mix has its upgrade built-in) | |||
* postcss (^8.4.39 added) | |||
* laravel-mix (^5.0.9 to ^6.0.49) | |||
* vue-loader (^15.4.2 to ^15.11.1) | |||
This newer pipeline should be usable in the same way as the prior one, with | |||
<code>npm run prod</code> and <code>npm run dev</code> etc, but Laravel Mix | |||
6 makes it considerably more streamlined and ostensibly considerably less | |||
fragile than the Laravel Mix 5 version was. Much less underlying Webpack | |||
API details are exposed in the "scripts" commands, and also cross-env is no | |||
longer directly used and is dropped as a dependency. | |||
The following substitution was performed in <code>package.json</code>: | |||
"scripts": { | |||
"dev": "npm run development", | |||
"development": "cross-env NODE_ENV=development node_modules/webpack/bin/webpack.js --progress --hide-modules --config=node_modules/laravel-mix/setup/webpack.config.js", | |||
"watch": "npm run development -- --watch", | |||
"watch-poll": "npm run watch -- --watch-poll", | |||
"hot": "cross-env NODE_ENV=development node_modules/webpack-dev-server/bin/webpack-dev-server.js --inline --hot --config=node_modules/laravel-mix/setup/webpack.config.js", | |||
"prod": "npm run production", | |||
"production": "cross-env NODE_ENV=production node_modules/webpack/bin/webpack.js --no-progress --hide-modules --config=node_modules/laravel-mix/setup/webpack.config.js" | |||
}, | |||
"scripts": { | |||
"dev": "npm run development", | |||
"development": "mix", | |||
"watch": "mix watch", | |||
"watch-poll": "mix watch -- --watch-options-poll=1000", | |||
"hot": "mix watch --hot", | |||
"prod": "npm run production", | |||
"production": "mix --production" | |||
}, | |||
Two other files each had a single line change or addition for | |||
compatibility with the Laravel Mix version change. | |||
The following substitution was performed in <code>webpack.mix.js</code>: | |||
mix.js('resources/js/app.js', 'public/js') | |||
.sass('resources/sass/app.scss', 'public/css'); | |||
mix.js('resources/js/app.js', 'public/js') | |||
.vue() | |||
.sass('resources/sass/app.scss', 'public/css'); | |||
That is Laravel Mix 6 no longer assumes one is always using Vue, so now one | |||
opts into processing its templates explicitly with the <code>.vue()</code>. | |||
The following substitution was performed in <code>resources/js/app.js</code>: | |||
window.Vue = require('vue'); | |||
window.Vue = require('vue').default; | |||
Note that Laravel Mix 6 is required for Vue 3 compatibility but it also | |||
works with Vue 2; the latter is still in use as of this task's changes. | |||
[[#top|RETURN]] | |||
=== 2024 Jul 15: SDS Laravel: Replace laravel/helpers With laravel/framework === | |||
This task updated ''SDS Laravel'' <code>composer.json</code> to purge the | |||
PHP library dependency <code>laravel/helpers</code>, and also updated any | |||
''SDS Laravel'' PHP source files that used it to instead use | |||
<code>laravel/framework</code> built-ins. | |||
These 4 PHP source files were updated: | |||
* app/Http/Controllers/Auth/RegisterController.php | |||
* resources/views/admin/subjects/edit_rotating_courses/show.blade.php | |||
* resources/views/student/student_mark_collections/edit.blade.php | |||
* resources/views/student/student_mark_collections/show.blade.php | |||
For each of the above 4 files, there were either of these 3 subsitutions: | |||
str_contains(...) | |||
str_random(...) | |||
str_replace_array(...) | |||
Str::contains(...) | |||
Str::random(...) | |||
Str::replaceArray(...) | |||
Note that <code>laravel/helpers</code> was just a single PHP source file | |||
providing trivially thin wrapper functions over | |||
<code>laravel/framework</code> 6+ built-ins that emulated ones from Laravel | |||
5.8, and we were already mostly using the newer versions, so this task just | |||
replaced the last 7 instances where we weren't. | |||
[[#top|RETURN]] | |||
=== 2024 Jul 17: SDS Laravel: Upgrade Font Awesome from 4.x plus 5.x plus CDN to 6.x === | |||
This task updated ''SDS Laravel'' to require the latest version of the | |||
JavaScript library dependency Font Awesome, to 6.x from a mixture of 3 | |||
different versions 4.x plus 5.x plus CDN. | |||
This task changed these dependencies in <code>package.json</code>: | |||
* @fortawesome/fontawesome-free (^5.15.4 to ^6.6) | |||
* font-awesome (^4.7 removed as @fortawesome/fontawesome-free is its upgrade) | |||
This task deleted references to this CDN dependency in 3 app Blade template | |||
PHP source files: | |||
* <nowiki>https://use.fontawesome.com/releases/v5.7.1</nowiki> | |||
This task also made a small tweaks to a few other files for compatibility, | |||
replacing font/symbol/style name <code>fa</code> with <code>far</code>. | |||
[[#top|RETURN]] | |||
=== 2024 Jul 17: SDS Laravel: Stop Requiring tempusdominus, bootstrap-select, jquery-ui, lodash === | |||
Prior to the performance of this task, the ''SDS Laravel'' config | |||
file <code>package.json</code> explicitly required these 5 third-party | |||
JavaScript library dependencies, which were not actually used by the app: | |||
* bootstrap-select (^1.13.18 removed as not in use) | |||
* jquery-ui (^1.13.3 removed as not in use) | |||
* lodash (^4.17.21 removed as only used indirectly by laravel-mix) | |||
* tempusdominus-bootstrap-4 (^5.39.2 removed as not in use) | |||
* tempusdominus-core (^5.19.3 removed as not in use) | |||
This task updated <code>package.json</code> to remove those unused | |||
explicit dependencies. | |||
This task also updated 5 other app files for compatibility, typically by | |||
deleting explicit "require" or similar statements to these libraries since | |||
they weren't actually used otherwise. | |||
However, these 2 files were special: | |||
* resources/views/admin/subjects/edit_subject/show.blade.php | |||
* resources/views/application/studentinfo/index.blade.php | |||
Those 2 files actually DID use removed dependencies, and so technically | |||
they were broken by this task. But the 2 screens in question were not | |||
currently in use and so temporarily breaking them was acceptable. One | |||
screen was used historically but not any more. The other screen was still | |||
under construction. This task added comments to those 2 files explaining | |||
that they will need to be modified to use alternate and modern dependencies | |||
if they are going to be completed or used again. | |||
[[#top|RETURN]] | |||
=== 2024 Jul 18: SDS Laravel: Upgrade axios === | |||
This task updated <code>package.json</code> to upgrade the JavaScript library | |||
dependency <code>axios</code> from <code>0.27.2</code> to <code>1.7.2</code>. | |||
[[#top|RETURN]] | |||
=== 2024 Jul 25: SDS Laravel: Upgrade Vue.js from 2.x to 3.x and Upgrade Co-Dependent JS deps === | |||
This task updated <code>package.json</code> to require the latest versions | |||
of <code>vue</code> as well as of all of its co-dependent JavaScript | |||
library dependencies. | |||
To be specific, it made these dependency changes: | |||
* @formkit/pro (^0.119.14 added) | |||
* @formkit/themes (^1.6.5 added) | |||
* @formkit/vue (^1.6.5 added) | |||
* css-loader (^7.1.2 added) | |||
* vue (^2.7.16 to ^3.4.34) | |||
* vue-loader (^15.11.1 to ^17.4.2) | |||
* vue-bootstrap-datetimepicker (^5.0.1 removed as @formkit/pro "datepicker" is used in its' place) | |||
* vue-template-compiler (^2.7.16 removed as vue 3 has its upgrade built-in) | |||
Several changes for compatibility were made to the app source files. | |||
Partially rewrote source file <code>resources/js/app.js</code> to account | |||
for Vue 3 not having a single global state like Vue 2 but multiple possible | |||
object-scoped states. Many of the same changes were previously made in | |||
''SDS Gavintech'' which served as a guide. | |||
Rewrote source file <code>resources/js/components/inputs/DateTime.vue</code> | |||
to use <code>@formkit/pro</code> rather than | |||
<code>vue-bootstrap-datetimepicker</code>. The new version is a clone from | |||
''SDS Gavintech'' which had the corresponding changes made earlier. | |||
Updated source file <code>resources/js/components/admin/finance/Fees.vue</code> | |||
in several ways (some previously done in ''SDS Gavintech'', some brand new). | |||
As Vue 3 removed the filters feature, their use was replaced with ordinary | |||
function calls; <code>x|format_year</code> was replaced with <code>format_year(x)</code>. | |||
As Vue 3 reversed the precedence of <code>v-if</code> plus <code>v-for</code> | |||
in the same HTML/template tag, added a child <code>template</code> tag of | |||
the <code>tr</code> that used both, to force the expected precedence. | |||
Updated source file <code>resources/js/components/application/AddressInfo.vue</code> | |||
to fix a syntax/logic error that Vue 2 overlooked but Vue 3 doesn't. | |||
The logic on the System Settings -> Pages screen to fold/unfold the page | |||
hierarchy was disabled as it was incompatible with Vue 3 and not essential | |||
so the tree is just permanently unfolded. This was done by small changes in | |||
the source file <code>resources/views/system/page/index.blade.php</code>. | |||
The System Settings -> Groups screen was altered to be read-only because | |||
the logic for adding/editing/deleting database records was incompatible | |||
with Vue 3, a fix was elusive, and this was a good enough 90% solution for | |||
the short term given the infrequent use of the screen; a follow-up task can | |||
be performed later to enable editing on this screen again when needed. | |||
This was done by small changes in the source file | |||
<code>resources/views/system/group/index.blade.php</code>. | |||
Formatting configuration for date pickers was updated on a dozen or two | |||
screens replacing any <code>LL</code> and <code>LLL</code> shorthands with | |||
their longhands, namely <code>D MMM YYYY</code> and | |||
<code>D MMM YYYY HH:mm</code> respectively, since the new date pickers | |||
don't support the shorthands, only the longhands. | |||
This task also resulted in unintended changes in the app's visual | |||
appearance, such as fonts being a bit smaller or some elements being in | |||
slightly different locations. The cause of this proved elusive, and since | |||
it isn't actually a functional break, this didn't hold back the Vue 3 | |||
upgrade. A follow-up task can be performed later if changes to the visuals | |||
are desired. | |||
[[#top|RETURN]] | |||
=== 2024 Aug 2: SDS Laravel: Upgrade directorytree/ldaprecord-laravel from 2.x to 3.x === | |||
This task updated ''SDS Laravel'' <code>composer.json</code> to require the | |||
latest version the PHP library dependency | |||
<code>directorytree/ldaprecord-laravel</code>, going from <code>2.7.3</code> | |||
to <code>3.3.3</code>. | |||
This task also made 3 distinct sets of app source code changes to be | |||
compatible with that upgrade. | |||
See also for context: | |||
* https://ldaprecord.com/docs/laravel/v3/upgrading/ | |||
* https://ldaprecord.com/docs/laravel/v3/release-notes/ | |||
* https://github.com/DirectoryTree/LdapRecord-Laravel/releases | |||
The first set of changes... | |||
This task updated the 1 PHP config source file <code>config/ldap.php</code> | |||
to account for <code>logging</code> now being an array. There was this 1 | |||
substitution: | |||
'logging' => env('LDAP_LOGGING', true), | |||
'logging' => [ | |||
'enabled' => env('LDAP_LOGGING', true), | |||
], | |||
The second set of changes... | |||
For context, <code>directorytree/ldaprecord-laravel</code> had a PHP trait | |||
named <code>LdapRecord\Laravel\Auth\MultiDomainAuthentication</code> which | |||
was deprecated before version <code>2.7.3</code> and removed in version | |||
<code>3.0.0</code>. The 1 SDS PHP source file | |||
<code>app/Http/Controllers/Auth/LoginController.php</code> used that trait. | |||
This task updated <code>LoginController</code> to clone into itself the | |||
used portions of <code>MultiDomainAuthentication</code>, thus removing the | |||
external dependency of the former on the latter. | |||
As the removed trait was itself a sub-trait of | |||
<code>LdapRecord\Laravel\Auth\CreatesUserProvider</code>, | |||
<code>LoginController</code> now composed that directly and not indirectly. | |||
Also <code>LoginController</code> gained the new protected function | |||
<code>getLdapGuard</code> cloned from the removed trait. | |||
Note that the function <code>getLdapGuardFromRequest</code> was ''not'' | |||
cloned from the removed trait since <code>LoginController</code> already | |||
had its own version that overrode it. | |||
The third set of changes... | |||
For context, each of these 5 SDS classes composed one of the 2 classes | |||
<code>LdapRecord\Models\Model</code> or <code>LdapRecord\Models\Scope</code>: | |||
* app/Ldap/ExternalUser.php | |||
* app/Ldap/Scopes/OnlyStaffUsers.php | |||
* app/Ldap/Scopes/OnlyStudents.php | |||
* app/Ldap/SmusStudents.php | |||
* app/Ldap/SmusUser.php | |||
For each of those 5, it was updated such that for any property or method it | |||
contained which overrode or implemented a same-named one from | |||
<code>Model</code> or <code>Scope</code>, that property or method had | |||
explicit type annotations added to match the originals. This was required | |||
for the SDS code to satisfy a PHP or Laravel stricture so the code runs. | |||
[[#top|RETURN]] | |||
=== 2024 Aug 2: SDS Laravel: Upgrade intervention/image from 2.x to 3.x and Fix Import Photos Screen === | |||
This task made 3 distinct groups of changes to ''SDS Laravel'' all related | |||
to image handling and the Admin Menu -> Migration -> Import Photos screen. | |||
==== Changes For intervention/image Upgrade ==== | |||
This task also made 2 distinct sets of changes to be compatible with the | |||
<code>intervention/image</code> upgrade. | |||
For context, as part of the major update of <code>intervention/image</code> | |||
from version 2.x to 3.x, it was also split up into multiple libraries, with | |||
the core remaining under the old name and the optional Laravel-specific | |||
add-ons being in the new <code>intervention/image-laravel</code> library. | |||
We only need to directly require the latter, which brings in the former. | |||
See also for context: | |||
* https://image.intervention.io/v3/introduction/upgrade | |||
* https://github.com/Intervention/image-laravel/blob/main/README.md | |||
* https://image.intervention.io/v3/modifying/resizing | |||
The first set of changes... | |||
This task added the 1 PHP config source file <code>config/image.php</code> | |||
that is standard per the above-linked README. | |||
The second set of changes... | |||
This task replaced the use of the no longer existing PHP class | |||
<code>Intervention\Image\ImageServiceProvider</code> with the existing | |||
<code>Intervention\Image\Laravel\Facades\Image</code>. | |||
In addition, the class is now used directly by its full name in the 2 PHP | |||
source files where the image handling functionality is actually used, which | |||
are <code>app/Models/File.php</code> and | |||
<code>app/Http/Controllers/Admin/Migration/ImportPhotosController.php</code>. | |||
This task has purged any explicit registration in <code>config/app.php</code> | |||
of any class of this library in <code>providers</code> or | |||
<code>aliases</code>; the <code>Image</code> alias is now purged. | |||
The PHP source code that directly used the Intervention Image libraries was | |||
updated for compatibility with several method renames or substitutions. | |||
Changes were made to reflect the rename of the library static method | |||
<code>make</code> to <code>read</code>. | |||
Changes were made to reflect the changes with the set of image manipulation | |||
methods for image resizing. The <code>resize</code> method of version 2 had | |||
taken a third argument by which one specified the resize should keep the | |||
original aspect ratio. The <code>resize</code> method of version 3 doesn't | |||
take that argument, and instead the version 3 method <code>scale</code> is | |||
explicitly for resizing that maintains the aspect ratio. | |||
[[#top|RETURN]] | |||
==== Bug/Security Fixes In Photos Import ==== | |||
This task also updated the 2 files | |||
<code>app/Http/Controllers/Admin/Migration/ImportPhotosController.php</code> | |||
plus | |||
<code>resources/views/admin/migration/import_photos/index.blade.php</code> | |||
to fix bugs tangential to the image file path handling. | |||
One fix was in the <code>index</code> function so that the directory path | |||
reported to the user was the same one that <code>store</code> actually | |||
used, from <code>Storage::disk('photos')</code>, rather than being either | |||
of a couple of hard-coded alternatives toggled on an environment variable | |||
that isn't actually used. | |||
A second fix was to stop passing a <code>path</code> hidden form field | |||
value from <code>index</code> to <code>store</code>, whose value was set | |||
from the incorrect path in <code>index</code>, and which <code>store</code> | |||
validated as present and subsequently didn't use. Besides being dead code | |||
from the disuse, if this had been used it would have been a major security | |||
vulnerability, as it would have let web clients directly control actions | |||
against server local file system paths. | |||
A third fix was in the <code>store</code> such that logic to skip over | |||
files that were not of type <code>image/jpeg</code> was faulty and never | |||
skipped any kind of file, and now it skips those it is supposed to. | |||
The specific logic error was one of operator precedence, where it was of | |||
the form <code>!x == y</code> where it meant to be <code>!(x == y)</code>; | |||
the fixed version made it be <code>x !== y</code>. | |||
[[#top|RETURN]] | |||
==== Changes For league/flysystem Indirect Upgrade ==== | |||
This task also updated | |||
<code>app/Http/Controllers/Admin/Migration/ImportPhotosController.php</code> | |||
to be compatible with the <code>league/flysystem</code> upgrade from | |||
version 1 to version 3 that was indirect by way of the Laravel 9 upgrade. | |||
See also for context: | |||
* https://flysystem.thephpleague.com/docs/upgrade-from-1.x/ | |||
* https://laravel.com/docs/9.x/upgrade#flysystem-3 | |||
There were 3 substitutions for compatibility in the <code>store</code> function. | |||
There was 1 like this: | |||
Storage::disk('photos')->getMimeType($file) | |||
Storage::disk('photos')->mimeType($file) | |||
And there were 2 like this: | |||
Storage::disk('photos')->getDriver()->getAdapter()->applyPathPrefix($file); | |||
Storage::disk('photos')->path($file); | |||
This task also updated <code>config/filesystems.php</code> to add the | |||
element <code>'throw' => true</code> to each <code>disks</code> element. | |||
This is to help preserve Laravel 8 behavior that we might have been relying | |||
on when a write operation fails. For example, write operations such as | |||
<code>write</code> by default no longer throw an exception when a write | |||
operation fails and instead returns <code>false</code>; until we can audit | |||
our code for how we currently check for such failures, this config change | |||
should make it less likely our code was broken by the dependency update. | |||
Note that there are still other kinds of default behavior changes that we | |||
should audit our code for handling of. For example, write operations will | |||
overwrite existing files by default, so if we don't want that then we | |||
should explicitly be checking for their existence first. Also, reading a | |||
nonexisting file now returns null rather than an exception. | |||
[[#top|RETURN]] | |||
=== 2024 Aug 2: SDS Laravel: Upgrade laravel/framework from 8.x to 10.x and Upgrade Co-Dependent PHP deps === | |||
This task made 5 distinct groups of changes to ''SDS Laravel'' all related | |||
to upgrading it from Laravel 8 to Laravel 10. | |||
==== Summary of PHP Dependency Upgrades or Removals ==== | |||
This task updated <code>composer.json</code> to require the latest | |||
PHP-8.1-compatible versions of <code>laravel/framework</code> as well as of | |||
all of its co-dependent PHP library dependencies. In some cases, upgrading | |||
co-dependents meant removing them entirely in favor of Laravel built-ins. | |||
To be specific, it made these dependency changes: | |||
* barryvdh/laravel-debugbar (^3.7 to ^3.13.5) | |||
* directorytree/ldaprecord-laravel (^3.3.3 unchanged) | |||
* etern8ty/beanstream (dev-master unchanged and is custom fork) | |||
* fakerphp/faker (^1.23.1 unchanged) | |||
* fideloper/proxy (^4.4.2 removed as laravel/framework has its upgrade built-in) | |||
* goldspecdigital/laravel-eloquent-uuid (^8.0.1 removed as laravel/framework has its upgrade built-in) | |||
* guzzlehttp/guzzle (^7.9.2 unchanged) | |||
* intervention/image-laravel (^1.3 unchanged) | |||
* juliomotol/laravel-auth-timeout (^3.1.1 to ^4.1) | |||
* lab404/laravel-impersonate (^1.7.5 unchanged) | |||
* laravel/framework (^8.83.27 to ^10.48.18) | |||
* laravel/tinker (^2.9 unchanged) | |||
* laravel/ui (^3.4.6 to 4.5.2; but it recommends using Laravel Breeze or Laravel Jetstream instead) | |||
* mockery/mockery (^1.6.12 unchanged) | |||
* nunomaduro/collision (^5.11 to ^7.10) | |||
* phpunit/phpunit (^10.5.29 unchanged) | |||
* spatie/laravel-ignition (^1.7 to ^2.8) | |||
* staudenmeir/eloquent-has-many-deep (^1.14.4 to ^1.19.4) | |||
[[#top|RETURN]] | |||
==== Specify Minimum PHP Dependency Versions Used Are Stable ==== | |||
This task updated <code>composer.json</code> to specify an app default | |||
<code>minimum-stability</code> of <code>stable</code> rather than | |||
<code>dev</code>. | |||
This is per the Laravel 10 upgrading recommendation: | |||
* https://laravel.com/docs/10.x/upgrade#updating-minimum-stability | |||
While the setting could instead have been removed as <code>stable</code> is | |||
its default, being explicit seemed better here. | |||
Note that this didn't cause any changes to what PHP library versions are | |||
actually installed by Composer versus the old setting, but is a good | |||
constraint going forward. | |||
[[#top|RETURN]] | |||
==== Changes For laravel/framework Upgrade's Removal of "dates" Model Attribute ==== | |||
This task also updated 51 PHP source files to be compatible with a breaking | |||
change made by Laravel itself with version 10. | |||
Laravel supported a "dates" model attribute through version 9, and then | |||
Laravel 10 removed it. The function of this was to enumerate database/model | |||
fields that were supposed to be automatically converted to Carbon DateTime | |||
objects; so under Laravel 8, any "dates" declarations would be respected, | |||
while under Laravel 10 they would be ignored. | |||
See also: | |||
* https://laravel.com/docs/10.x/upgrade#model-dates-property | |||
Compare: | |||
* https://laravel.com/api/9.x/Illuminate/Database/Eloquent/Concerns/HasAttributes.html | |||
* https://laravel.com/api/10.x/Illuminate/Database/Eloquent/Concerns/HasAttributes.html | |||
As a result, simply upgrading ''SDS Laravel'' from Laravel 8 to 10 resulted | |||
in many parts of the app breaking in various ways including when simply | |||
visiting the post-login home screen, as PHP died with errors like | |||
<code>Call to a member function format() on int</code>. | |||
To fix this, any instances of <code>protected $dates = ['x',...]</code> | |||
in model classes were replaced with | |||
<code>protected $casts = ['x'=>'datetime',...]</code> | |||
which was the more modern way to get the same functionality, which exists | |||
in both Laravel 8 and 10. For the few model classes that already had other | |||
<code>$casts</code> declarations, the replacements were merged with those. | |||
While the "dates" change could have been its own task that was merged to | |||
trunk prior to and separately from the current Laravel 10 upgrade task, it | |||
was combined with the latter to streamline testing, as both had potential | |||
impacts over a large fraction of the app. | |||
[[#top|RETURN]] | |||
==== Changes For laravel/framework Upgrade's Miscellaneous Changes ==== | |||
This task also updated <code>app/Providers/AuthServiceProvider.php</code> | |||
to remove the call <code>$this->registerPolicies();</code> from | |||
<code>boot()</code>because Laravel 10 now calls it automatically, as | |||
recommended here: | |||
* https://laravel.com/docs/10.x/upgrade#register-policies | |||
[[#top|RETURN]] | |||
==== Changes For goldspecdigital/laravel-eloquent-uuid Removal ==== | |||
This task also updated these 5 PHP source files to be compatible with the | |||
replacement of <code>goldspecdigital/laravel-eloquent-uuid</code> with a | |||
Laravel built-in: | |||
* app/Models/Application/Application.php | |||
* app/Models/User.php | |||
* app/Models/User/Student.php | |||
* app/Models/User/Teacher.php | |||
* app/Models/User/UserContract.php | |||
These further 3 files also referenced the trait but commented out, so not | |||
current users but possible past or future users: | |||
* app/Models/Application/AppUser.php | |||
* app/Models/User/Address.php | |||
* app/Models/User/Guardian.php | |||
For each of the above 8 files, there were these 2 line subsitutions: | |||
use GoldSpecDigital\LaravelEloquentUUID\Database\Eloquent\Uuid; | |||
use Uuid; | |||
use Illuminate\Database\Eloquent\Concerns\HasUuids; | |||
use HasUuids; | |||
Here is a description of the above built-in feature in Laravel 9.3+: | |||
https://laravel.com/docs/11.x/eloquent#uuid-and-ulid-keys | |||
The purpose of that reimplemented functionality was to empower use of | |||
generated UUIDs for primary key fields of some database tables instead of | |||
the serially generated integers that ''SDS Laravel'' more typically uses; | |||
Laravel Eloquent only gained built-in support for UUIDs with version 9.3. | |||
This task also deleted the single PHP file | |||
<code>app/Models/Traits/Uuids.php</code> as it appeared to be unused. | |||
[[#top|RETURN]] | |||
==== Changes For fideloper/proxy Removal ==== | |||
This task also updated <code>app/Http/Middleware/TrustProxies.php</code> to | |||
be compatible with the replacement of <code>fideloper/proxy</code> with a | |||
Laravel built-in. The changes were in 2 spots. | |||
First was this substitution: | |||
use Fideloper\Proxy\TrustProxies as Middleware; | |||
use Illuminate\Http\Middleware\TrustProxies as Middleware; | |||
Second was this substitution: | |||
protected $headers = Request::HEADER_X_FORWARDED_ALL; | |||
protected $headers = | |||
Request::HEADER_X_FORWARDED_FOR | | |||
Request::HEADER_X_FORWARDED_HOST | | |||
Request::HEADER_X_FORWARDED_PORT | | |||
Request::HEADER_X_FORWARDED_PROTO | | |||
Request::HEADER_X_FORWARDED_AWS_ELB; | |||
See also https://laravel.com/docs/9.x/upgrade under "Trusted Proxies". | |||
[[#top|RETURN]] | |||
==== Changes For juliomotol/laravel-auth-timeout Upgrade ==== | |||
This task also updated | |||
<code>app/Http/Middleware/AuthTimeoutMiddleware.php</code> to be | |||
compatible with the <code>juliomotol/laravel-auth-timeout</code> upgrade. | |||
There was this 1 substitution: | |||
use JulioMotol\AuthTimeout\Middleware\AuthTimeoutMiddleware as BaseMiddleware; | |||
use JulioMotol\AuthTimeout\Middlewares\CheckAuthTimeout as BaseMiddleware; | |||
Note that <code>juliomotol/laravel-auth-timeout</code> must be upgraded | |||
simultaneously with Laravel since the former's versions 3.1.1 and 4.1 | |||
respectively require Laravel 8 and 10 respectively. | |||
See https://github.com/juliomotol/laravel-auth-timeout/blob/master/CHANGELOG.md | |||
for more change details and upgrade notes on that. | |||
[[#top|RETURN]] | |||
== Documentation Changes == | |||
=== 2024 Jul 16: SDS Laravel: Make README etc Say This App is SDS Rather Than Laravel === | |||
This task updated ''SDS Laravel'' <code>composer.json</code> to update | |||
primary documentation attributes so they describe the actual SDS | |||
application this is rather than the Laravel template used to build it. | |||
There were these substitutions: | |||
"name": "laravel/laravel", | |||
"description": "The Laravel Framework.", | |||
"keywords": ["framework","laravel"], | |||
"name": "smus/sds", | |||
"description": "School Data System (SDS)", | |||
"keywords": [], | |||
This task | This task also completely rewrote <code>README.md</code> so it describes | ||
this SDS application rather than the Laravel framework. | |||
[[#top|RETURN]] | [[#top|RETURN]] |
Latest revision as of 18:46, 20 August 2024
This document consists of multiple parts; for a directory to all of the
parts, see SDSM:Index.
Description
This part of the SDS Modernization (SDSM) document enumerates a not necessarily exhaustive list of historical changes or improvements that were made to SDS, made by Darren Duncan if by whom is not otherwise specified.
It exists to provide visibility into historical progress of the SDS Modernization (SDSM) project about which this document otherwise mainly just describes the current state of SDS.
This list often but not always corresponds to the list of Git pull requests at https://git.smus.ca which most readers of this document are not privileged to see directly.
Fixes to Broken Behavior
2024 Apr 18: SDS Laravel: User Preferences Screen Fails to Display
Context: User darren.duncan
attempted to visit their
SDS Laravel User Preferences screen, example urls
https://api.smus.ca/User/Preferences
(production) and
https://api-latest.smus.ca/User/Preferences
(preview).
Observed Behavior: In production, a non-graceful generic 500 internal
server error message was displayed rather than the expected User
Preferences screen. In preview, the revealed underlying symptom was that
the code line in PreferencesController
of
$addresses = $user->guardian->addresses;
was dying with
Attempt to read property addresses on null
. The problem
occurred for user darren.duncan
but not for other users.
Expected Behavior: The User Preferences screen should always display, regardless of whether the user in question is a guardian or has guardians. If some functionality of the screen is not applicable for users that aren't or don't have guardians, the screen should either hide that functionality or explain why it doesn't apply or both.
Applied Solution: The dying code line was replaced with code that only tried to fetch and return an address list when it was valid to do so, and it returned an empty list otherwise. As a result, the User Preferences screen never failed to display under this scenario.
Caveat: While the User Preferences screen no longer fails to display,
as of the completion of this fix, it still remained true that the
functionality to actually display the fetched address list in question was
never implemented, and so for every user, the screen carried the
placeholder message Shows address info
instead of each
individual address.
2024 May 21: SDS Laravel: Timeout Feature Broken In Sub-Directory Hosted Apps
SDS Laravel has a security feature such that if a user is inactive for a period of time, 15 minutes typically, they will be automatically logged out of the app. The app displays a countdown timer on the top right hand corner of the screen that ticks for every second of activity. Javascript code running in the client web browser will normally reset the countdown to the full amount whenever a user interface (UI) event occurs that it considers continued user activity, such as mouse movement over the screen.
This feature is implemented partly by the web client invoking the
/session
endpoint on the server.
That invocation fails on any SDS Laravel app instance that is hosted at
a web address which is a sub-directory of the base url (meaning it is based
at an address like https://foo.com/bar
rather than at an
address like just https://foo.com
). This is because the web
client is trying to unconditionally treat every app instance as if it is
hosted directly at the base url with respect to its attempts to invoke the
/session
endpoint, and so it is invoking the wrong web address
for instances that are at sub-directory urls.
As a result, for broken app instances, while the first UI activity indicating continued user activity will reset the countdown timer displayed on screen to the user, all subsequent UI activity will have no effect on the timer, and it will not reset, and the server will not be aware of this activity. Only a full page load like clicking a link to a new page will register as activity and reset the timer.
This breakage affects all SDS Laravel instances at https://sdsdev.smus.ca which are in sub-directories.
The primary change of this task fixes the problem by making the web client
respect the actual location of the app when invoking /session
.
Mainly it is a 1-line change in the Laravel Blade template file
resources/views/layouts/main.blade.php
to use
url: '{{route('session')}}'
rather than
url: '/session'
.
An additional change of this task is to fix a problem where the
app's background image doesn't display for the same underlying reason. The
problem is that the web client is trying to load the image
public/images/body.png
from the wrong location. The fix
updates 1 line in the CSS file public/css/smus_custom.css
to
background: url('../images/body.png')
from
background: url('/images/body.png')
; the newly-relative url is
relative to the location of the CSS file itself.
There is still additional broken behavior related to static asset loading
like the background image example, affecting custom fonts for example, but
these references are in the generated file public/css/app.css
as parts of third-party dependencies, and so these were left alone.
2024 Jun 12: SDS Laravel: Form Meister Screen Fails to Display
This task made 2 distinct changes.
The first change...
The SDS Laravel screen Admin Menu -> Online Forms -> Form Meister fails
to display when visited. In production it displays a generic 500 internal
server error message. In preview, the revealed underlying symptom was an
attempt to invoke the non-existent PHP class
App\Models\System\Form
.
This task fixed the offending PHP class reference to instead refer to the
existing PHP class App\Models\Abstract\Form
. The result was
that the Form Meister screen then displayed when visited.
(Note that the SDS Gavintech version of this screen was disabled in favor of the new version, so it was not functional for anyone.)
The second change...
For an idiomatic PHP Laravel project, each of its web controller classes
has an unqualified PHP class name in the form FooController
.
While nearly all of the 161 SDS Laravel controller classes followed that naming convention, these 3 were lone exceptions:
- Admin/OnlineForms/EditFormCampaigns.php
- Admin/OnlineForms/EditFormMeister.php
- Admin/OnlineForms/MergeFormCampaign.php
This task renamed each of those 3 to add Controller
to their
unqualified names.
2024 Jul 4: SDS Laravel: Functional Screens Not Distinguished in Main Menu Bar
The main navigation menus of SDS Laravel declare a hierarchical list of named screens, but a design decision was made such that, while the app is still incomplete and under development, the menus list many screens that do not yet exist in SDS Laravel as placeholders along with many which do.
There was a serious usability problem with how this was first implemented, such that each menu item did not distinguish in any way whether it was for a screen that existed or whether it was a placeholder; both kinds looked identical visually and both kinds were active hyperlinks that went somewhere, and a user could only tell which kind it was by choosing it and seeing where they went as a result. In the case of placeholders, every one un-helpfully went to the home screen.
This was a problem because users exploring SDS Laravel, whether to use it normally or to test it, would waste a lot of time searching for needles of functional screens in haystacks, with much trial and error, as about half of the total menu item count was placeholders.
This task made 2 related changes for usability without losing anything.
The first change is that now for each menu item which is a placeholder, its name is prefixed by "(N/A)" so it clearly stands out visually.
The second change is that now for each menu item which is a placeholder, its hyperlink is no longer functional, and a user isn't taken anywhere when clicking on it; they stay on their current screen.
Note that common established practice in desktop GUI apps is to use alternate fonts or colors for visual distinction, such as non-active menu items having grey text versus black text for active ones, but this task eschewed that solution given its greater complexity. A future enhancement can do this, however.
Note that an alternate solution could have been to just not display placeholders anymore, so all menu items went to screens, but it was deemed more useful to retain the placeholders.
Note that for the purposes of this menu logic, the determination of whether
a screen was implemented or a placeholder was just whether or not a Laravel
route was defined for it, in the source file /routes/web.php
.
Implementing this change was a single line change in the source file
/app/Models/Sub/Page.php
.
2024 Jul 23: SDS Laravel: Form Campaign Add Target Students Uncheck All Button Fails
The SDS Laravel screen Admin Menu -> Online Forms -> Form Campaign -> Consolidated Health Form -> Manage has broken behavior.
The "Add New Targets" screen section presents a list of students with a checkbox for each student. When visiting the screen, the checkboxes all default to a checked state. Often this is what the user wants but sometimes they want to just pick a few students instead, and to assist with this is an "Uncheck All" button.
Clicking the "Uncheck All" button as the user's first action on this screen succeeds. But if any boxes are manually checked afterwards, clicking "Uncheck All" again does nothing, and those boxes are still checked.
Furthermore, if SDS Laravel is updated to use Vue 3 from its current Vue 2, the same functionality manifests additional broken behavior such that the "Uncheck All" button has no effect even on initially visiting the page, so it doesn't even work a single time to uncheck all the boxes.
This task fixes the broken behaviour by just making a few small updates in
the 1 PHP source file
resources/views/admin/online_forms/form_campaigns/show.blade.php
.
Primarily there was this 1 substitution in the "Uncheck All" button definition:
onclick="$('#student_list input:checkbox').attr('checked', false);"
onclick="Array.from(document.getElementsByClassName('checkbox_student_list_add_target')).forEach(function(checkbox) { checkbox.checked = false; });"
In support of that, there was also this 1 addition to each student checkbox:
class="checkbox_student_list_add_target"
Those 2 line changes are all that was needed to fix the broken behavior.
The underlying problem appears to be that jQuery's mechanism to enumerate
the set of checkbox DOM nodes fails to find them when any environmental
change occurs such as when users click the checkboxes, so the JavaScript
code to uncheck those boxes acts like the checkboxes don't exist.
Note that built-ins like querySelectorAll()
have the same problem.
Whereas, getElementsByClassName()
is resilient to such
environmental changes and can still find the checkboxes after user actions.
This task also makes a user interface change to enhance users' experience, which is to add a complementing "Check All" button.
2024 Aug 15: SDS Gavintech: Input Absences Screen Campus Filter Not Sticky and Other Bugs
The SDS Gavintech screen Staff Menu -> Attendance -> Input Absences has several bugs or otherwise sub-optimal behaviors, and this task makes 7+ distinct changes to respectively fix or improve those.
All of the changes are isolated to the single PHP source file
pages/attendance/input_absence.php
and so should not impact
any part of SDS besides this one screen.
The first/primary change...
The filter toggle "Show all campuses"/"Restrict to campus only" had a bug such that the setting of this toggle was not sticky; as soon as the user took any action which reloaded the screen, such as adding/editing/deleting an input absence record, or such as flipping the filter toggle "Day students only"/"All students", the campus toggle would reset to its default of showing the current campus only. Besides this going against user expectations and making more work for them, it is also inconsistent with the behavior of the day students toggle which generally did persist through any corresponding user actions.
This task normalized the behavior of the screen so that both of these filter toggles behave the same and persist through any actions the user takes on the screen; they would only reset to defaults if the user leaves the screen entirely and returns.
In particular, this task's change makes things better because if one uses the screen to add an absence for a student in another campus, then they can actually see the record they just added on the screen after they submitted the form, while otherwise it would look more like their action had no effect, besides the transient status message of "Success: Added entries".
The second change...
This task also made a behavior change such that the current setting of the campus filter toggle is now reflected in the title of the screen with a "(campus only)" or "(all campuses)" suffix, so its more clear to users what the setting is. This also makes the screen more consistent with the other filters, such that the day students filter had a corresponding title suffix already, "(day students)" and "(all students)" respectively, and the date filter which shows the date its filtered to.
The third change...
This task also made a behavior change such that the value of the "Set date" form/button now defaults to the same date the screen is currently displaying, rather than the real-world current date, which is effectively like the screen not persisting their choice to an extent.
I feel that behavior is more useful and in line with user expectations or less surprising to them. It also means that this form is more thoroughly a localized version of the application global "Date" setting. That is, changing the date globally also affects this screen, but changing it on this screen only affects this screen, until the user leaves this screen and it resets to match the global setting.
The fourth change...
This task also made a behavior change such that the link to "Print list of absent day students" has been renamed to "Print list of absent students", and it now displays unconditionally instead of only when the screen is filtered to just day students.
This new behavior was deemed more useful such as if we have an emergency on campus and we have to evacuate, a full list would indicate day students with absences entered by Ally/Bonita and boarding student absences entered by the Health Centre.
However, there was no behavior change with respect to what actually happened when that link was clicked on; because of a prior bug, the link always led to a list of all students anyway, rather than filtering to show just day students.
The fifth change...
This task also fixed a bug in the "Existing absences" -> "Entered by" column such that the HTML for the link to email the person was malformed and the url did not go anywhere / got a 404, but with the fix it correctly goes to the email screen.
The sixth change...
This task also removed the link "Email Advisors & Students", which was never implemented and did nothing but reload the page when clicked on.
The seventh+ changes...
This task also further normalizes how urls are generated that point back to the current screen, reducing duplication of repeated details, and also normalizing that arguments for filters only appear in urls if they did before or are not defaults.
This task also normalizes some other implementation code, such as specifying that all the "Dropdown" calls return their results rather than printing them.
Fixes to Slow Performance
2024 May 14: SDS Laravel: Main Menu Bar Heavy Database Use Very Slow Page Loads
SDS Laravel currently relies on the "pages" database table to canonically define its app main navigation menus and it currently has 201 records where each defines a menu item.
The main menus are hierarchical, and the "order" field of each "pages" record serves double duty to define the parent/child aka super/sub relationships of menu items as well as their display order. The "order" of a record is used as a unique key for the record for this purpose, as well as a foreign key from child to parent.
Each "order" is a positive integer N such that:
- The result of log-100(N) is the item's tree level.
- The result of div-by-100(N) is the item's parent item's "order" in the tree.
- N itself is the item's display order relative to its siblings.
For example, a root level menu item has an "order" in 1..99, and for the root item with an "order" of 3, all of its children would have "order" in 301..399, and the child of 303 would have "order" in 30301..30399 etc.
(Very notably, the actual primary key field of "pages" is NOT used for describing menu parent/child relationships in conjunction with a second field, as is the more typical practice in other databases.)
SDS Laravel reads the entire "pages" database table and constructs a hierarchical menu in 2 different circumstances. The primary and most important circumstance is for nearly every single screen, those where the user is logged in, and it is a normal screen, and not an API, a menu for navigation appears at the top of each screen. The secondary circumstance is on the "System Settings"->"Pages" screen that is used to edit this database table, where the hierarchy is rendered as a left column.
Prior to the changes of this task, the hierarchical menu assembly was performed in a very inefficient manner such that the application made a separate database query for every single tree node to fetch its children, meaning that it was making 138 queries for nearly every screen displayed to the user just for making the navigation menu alone. And on the "Pages" screen it was making 427 additional queries (total 565) just to display the menu hierarchy on that one screen. For context, the sum total of all other database queries for all other purposes on a single screen was 20-30 or so.
Compounding the above query-per-node inefficiency was another kind of inefficiency where, for each menu node, the same query for its children was often done up to 3 times (or up to 5 times), where only the last query actually enumerated the list of child nodes, and prior times simply tested if there were any children, and if there were children, the query was run again to actually use them.
Following the changes of this task, the hierarchical menu assembly only involved 2 SQL queries as overhead for nearly every screen, and 2 more on the "Pages" screen, because it now selects all the "pages" records at once rather than a handful or zero at a time, and uses the same fetched records when building every menu node. The records are grouped when fetched in a PHP array of arrays, the root array keyed by parent "order" values, and the array value for each being an array of its child "pages" records.
This change had a massive impact on the user-visible screen loading speed of SDS Laravel, particularly when a developer runs the application and its database on opposite ends of a VPN tunnel where the added latency of each database call magnified the overhead time of each database query.
For example, before the changes, running the app on a local machine at home, where it talked to a database on a school server, took about 23..25 seconds (155 queries) on the fastest screen with a menu to display on that screen, and after the changes it took about 3.4-3.7 seconds (19 queries). And on the "Pages" screen it took about 1.5 minutes (581 queries) to display the screen before the changes, and about 4.1-4.2 seconds (20 queries) after the changes.
For a typical non-developer user scenario which didn't have that kind of added VPN latency, the time to load a faster screen before the changes was around 0.46-0.49 seconds, and after the changes it was about 0.12-0.13 seconds. For the "Pages" screen it was about 1.8-1.9 seconds before and about 0.1-0.3 seconds after.
This task kept its changes about as minimal as possible while making this change. There was no significant change to how screens or menus were defined, and anything else related to the application design, aside from the minimal needed to consolidate the database queries. While there is a lot of room to make other improvements to how menus are implemented, other such changes are out of scope for this task.
Here are specific numbers for before/after, school/home-VPN, on 4 screens.
SDS Laravel/API Screen: /
- Blade Views Rendered: 1
- Database Queries Run Before: 0
- Database Queries Run After: 0
- Eloquent Models Used Before: 0
- Eloquent Models Used After: 0
- HTTP Page Load Time at School Before: about same
- HTTP Page Load Time at School After: 58.8ms,59.9ms,55.3ms,59.3ms,63.1ms
- HTTP Page Load Time at Home/VPN Before: about same
- HTTP Page Load Time at Home/VPN After: about same
SDS Laravel/API Screen: /home
- Blade Views Rendered: 3
- Database Queries Run Before: 192
- Database Queries Run After: 56 (136 fewer)
- Eloquent Models Used Before: 455
- Eloquent Models Used After: 475
- HTTP Page Load Time at School Before: 672ms,671ms,734ms,725ms,617ms
- HTTP Page Load Time at School After: 362ms,416ms,364ms,271ms,256ms
- HTTP Page Load Time at Home/VPN Before: 30.22s,30.78s,29.54s,29.90,29.83
- HTTP Page Load Time at Home/VPN After: 9.30s,9.07s,9.21s,9.24s,9.35s
SDS Laravel/API Screen: /Event/Calendar/33297
- Blade Views Rendered: 4
- Database Queries Run Before: 155
- Database Queries Run After: 19 (136 fewer)
- Eloquent Models Used Before: 200
- Eloquent Models Used After: 220
- HTTP Page Load Time at School Before: 462ms,487ms,589ms,493ms,491ms
- HTTP Page Load Time at School After: 126ms,131ms,125ms,121ms,125ms
- HTTP Page Load Time at Home/VPN Before: 24.06s,25.20s,24.06s,24.67s,23.72s
- HTTP Page Load Time at Home/VPN After: 3.38s,3.41s,3.39s,3.72s,4.73s
SDS Laravel/API Screen: /System/Page
- Blade Views Rendered: 34
- Database Queries Run Before: 581
- Database Queries Run After: 20 (561=136+425 fewer)
- Eloquent Models Used Before: 779
- Eloquent Models Used After: 436
- HTTP Page Load Time at School Before: 1.86s,1.85s,1.79s,1.88s,1.80s
- HTTP Page Load Time at School After: 170ms,207ms,173ms,316ms,208ms
- HTTP Page Load Time at Home/VPN Before: 1.5min,1.5min,1.5min
- HTTP Page Load Time at Home/VPN After: 4.16s,4.17s,4.09s,4.12s,4.12s
2024 Aug 20: SDS Gavintech: Input Absences Screen Heavy Database Use Very Slow
The SDS Gavintech app has widespread structural inefficiencies in the way that it uses the database, and this manifests as the primary cause of very slow app performance for users. The app may make 1000s or 10000s of distinct database calls every time a user visits any screen or performs any action on the screen that involves reloading the page, when it should only be making on the order of 1-20 database calls total each such time. As a result, each user action that loads/reloads a screen may carry a delay of 1-2 seconds at the bare minimum and often 10-20 seconds or several minutes, which cumulatively is a very poor user experience.
To be specific, that was the experience of the production SDS instance in early June of 2024. A pending migration as of this writing to newer production hardware would yield performance improvements of 2-5X alone, which helps significantly, but is still very noticeably slow. In contrast, changes to the app source code to correct the structural inefficiencies would instead yield further performance improvements in the 15X+ range, such that user actions then actually may seem instant. Meanwhile, any specific metrics given below for this task are all using the same newer hardware, to be apples to apples comparisons, and also each version is refreshed several times and the shortest generation time is used.
This task makes several distinct changes specific/isolated to the 1 screen Staff Menu -> Attendance -> Input Absences such that the logic specific to that screen is optimized in how it uses the database.
As a result of this task's changes, in order to display the screen, a total of exactly 8-9 database calls are made the screen-specific elements (4 trivial ones plus 4-5 main ones), rather than about 9594 calls.
Further, as a result of this task's changes, in order to bulk-add new absence records, a total in typical cases of exactly 4 database calls are made for the database updating logic (2 trivial ones plus 2 main ones), or a typical extreme maximum of about 24 database calls if adding around 1100 students, rather than a minimum of 6 calls and a maximum of about 4000-5000 respectively.
As a result of this task's changes, all user actions on the Input Absences screen that involve a screen load/reload now take about 0.1 seconds, which is essentially immediately, including displaying and bulk adding, as far as the work specific to the screen itself is concerned. Unlike the old logic, the new logic should scale and have essentially this same speed no matter how many students there are, rather than slowing down with more students.
This should be very noticeable to users, as every single individual action should feel a lot faster, which is even more significant given a typical user workflow of performing dozens to a hundred actions here each day.
As a result of this task's changes, the amount of server memory used also goes down by orders of magnitude, from about 168MB to about 4MB usually (2MB in some cases), a 42X savings, as far as the work specific to the screen itself is concerned. If widely applied, such changes can also mean SDS can scale to handle orders of magnitude more concurrent users on the same hardware without users seeing further slowdowns due to competition for resources with other concurrent users.
All of the changes are isolated to the single PHP source file
pages/attendance/input_absence.php
and so should not impact
any part of SDS besides this one screen.
Prior to the changes of this task...
This is the overhead for every screen minus that specific to the main menus; it comprises 16 generally distinct simple database calls:
Page generation time: 0.011 Total SQL Queries: 16 Total SQL Cache Hits: 15 Total Local Cache Hits: 2 Total Memcache Hits: Memory Usage: 2048K Realpath Usage: 87K
This is the overhead for every screen inclusive of the main menus, when using SDS with the effective date of 2024 May 6 and as a test/staff user "Darren Duncan" that is not a teacher/parent/student; the main menus alone add 1567 database calls:
Page generation time: 0.231 Total SQL Queries: 1583 Total SQL Cache Hits: 7078 Total Local Cache Hits: 10 Total Memcache Hits: Memory Usage: 32768K Realpath Usage: 125K
This is the overhead for every screen inclusive of the main menus, when using SDS with the effective date of 2024 May 6 and as a teacher user "Carol Adamson", who also has a "My Courses" main menu but lacks some other staff menu items; the main menus alone add 2049 database calls:
Page generation time: 0.27 Total SQL Queries: 2065 Total SQL Cache Hits: 9188 Total Local Cache Hits: 65 Total Memcache Hits: Memory Usage: 57344K Realpath Usage: 87K
This is Input Absences screen on effective date 2024 May 6 by user Darren Duncan including all the above overhead (user Carol Adamson doesn't have privileges to see this screen); the initial filter settings of current/Senior campus only plus all day plus boarding students (about 600 students) there are 10131 database calls total:
Page generation time: 1.471 Total SQL Queries: 10131 Total SQL Cache Hits: 31468 Total Local Cache Hits: 5351 Total Memcache Hits: Memory Usage: 180224K Realpath Usage: 125K
This is the same Input Absences screen initial load but with the main menu generation disabled, so it is more clear what this screen alone is doing; there are 9594 database calls that are specific to this screen; many of those calls are the same as ones in the main menu generator, and those same calls are cached / only made once when both the main menu generator and this screen's logic are enabled:
Page generation time: 1.406 Total SQL Queries: 9610 Total SQL Cache Hits: 23359 Total Local Cache Hits: 5343 Total Memcache Hits: Memory Usage: 167936K Realpath Usage: 126K
Note that all further numbers below likewise have the main menu generation disabled, to just highlight what Input Absences alone is doing.
This is the same thing but with the campus filter set to all campuses and showing both day and boarding students (about 1000 students); there are 3514 database calls specific to this screen; there are actually 2/3 fewer database calls despite showing more students:
Page generation time: 0.499 Total SQL Queries: 3530 Total SQL Cache Hits: 17071 Total Local Cache Hits: 289 Total Memcache Hits: Memory Usage: 65536K Realpath Usage: 88K
This is the same thing but with the filter set to day students only, and campus filter still main only; there are 9541 database calls specific to this screen; unlike the campus filter, the day student filter only has a small effect:
Page generation time: 1.462 Total SQL Queries: 9557 Total SQL Cache Hits: 20482 Total Local Cache Hits: 5280 Total Memcache Hits: Memory Usage: 165888K Realpath Usage: 126K
This is the same thing but with both filters toggled such that it shows day students only plus all campuses; there are 2942 database calls specific to this screen:
Page generation time: 0.469 Total SQL Queries: 2958 Total SQL Cache Hits: 17827 Total Local Cache Hits: 217 Total Memcache Hits: Memory Usage: 55296K Realpath Usage: 89K
About 80-90% of the database calls specific to this screen are about fetching a list of currently enrolled students for the effective date, with or without campus/day filters, and using that list both to populate a picklist of students and also to filter a display of existing absense records, because it is making a handful of separate database calls for each of about 1000 students (both when all 1000 are used and when some aren't). It also makes a handful of separate database calls for each existing absense record displayed, but these tend to be on the order of dozens of records instead of 1000.
When the user performs an action like adding/editing/deleting an absence record on this screen, the actual database modification task is typically very fast, assuming just a single student updated, usually around 2-6 database calls for that, but then the page reloads carrying the large aforementioned overheads. As an exception to this, when the user performs a bulk addition of absence records, by selecting multiple students, each individual student involves 4-5 database calls, of which 2 are reads and 2-3 are updates, and so in the unusual case of adding all thousand at once, it can make around 4000-5000 database calls, and take around 15 seconds on the newer hardware to do so; and then the page reloads.
Following the changes of this task...
This is the same thing, on just displaying the Input Absences screen, for effective date 2024 May 6, for user Darren Duncan; there are 8-9 database calls specific to this screen; it is 9 when filtering to day students only, and 8 otherwise; it is unchanged by filtering on one or all campuses:
Page generation time: 0.093 Total SQL Queries: 24 Total SQL Cache Hits: 25 Total Local Cache Hits: 5 Total Memcache Hits: Memory Usage: 4096K Realpath Usage: 54K
Page generation time: 0.108 Total SQL Queries: 25 Total SQL Cache Hits: 26 Total Local Cache Hits: 5 Total Memcache Hits:ƒ Memory Usage: 2048K Realpath Usage: 92K
In addition, the logic for batch-adding adds almost no time to that in typical cases of 1 or few students, and otherwise a small fraction of 1 second for the extreme case of bulk-adding all thousand students at once.
The main general change made by this task is to clone several call chains
worth of source code for several dozen routines across a handful of classes
into input_absence.php
and iteratively refactoring that whole
code mass to its most concise specific-to-this-screen's-use-case version.
No code shared with other screens was altered, allowing a guarantee that
no other screens are affected by these changes, making it much lower risk,
and a much simpler task to accomplish.
The most important effect of the main change is the consolidation of database calls. Typically how the logic worked before this task's changes was that in order to produce a list of something, first you had a query that just produced a set of record ids for the list we wanted, and then a separate query was made for each of those ids to get other fields we want for each record, sometimes in one call and sometimes in a handful, some for display purposes, and others for filtering purposes. Hence you may have around 3000-8000 calls to deal with a list of around 1000 students. This task changed things so that exactly 1 SQL query was done instead in place of that first id-fetching query that also gathered all the related details.
The 4-5 display-only main queries then were for these purposes:
- One to derive the input time range for the effective date, eg 8:30am-3:20pm, from a period list.
- One to display the picklist of students to add/edit absences for.
- One to display the picklist of attendence codes.
- One to display the list of existing absence records.
- Iff filtering students by day, one to fetch a list of fee statuses to filter with.
(While in theory the last one could also have been inlined, I chose not to because it was relating fee statuses with "like" rather than equality.)
There was also these 3 add-only SQL statements per each batch of 100 students:
- One to test that the user-supplied student ids match student records.
- One to insert up to 100 absence records.
- An indirectly called one to insert a log entry for the prior insert.
In the interest of security and scalability I compromised my adding logic between the pre-changes version of a separate database call per single student and using a single database call for the entire list of 1000+ students, into batches of 100, which is essentially the same performance as all-in-one but shouldn't be as likely to fail in pathological cases.
A related general change is that the main screen logic no longer turns database records into cached PHP objects that it then calls methods on, and instead just works with the records as regular PHP arrays which typically don't correspond to single database tables but joins of several. The logic goes from interacting heavily with the app's record object cache to generally not interacting with it at all, either to add or remove items. In particular, the logic no longer has up to a handful of repeatedly converting back and forth between record ids and record objects per record. Between this, and the greatly reduced use of the app's SQL query cache due to fewer database calls, likely is the main reason why so much less memory is used by PHP in the new version.
Note that while some database queries or other logic could possibly be streamlined or simplified further, I tried to maintain the exact effective logic as possible from before, in case there might be subtle behavior changes from such further streamlining. But an actual future rewrite of this screen such as under SDS Laravel could likely revisit this.
Generally speaking, the screen logic I overhauled was that where many similar database queries could be consolidated, while logic that only had individual distinct queries, such as related to getting the current school day, I left alone.
2024 Aug 20: SDS Gavintech: Attendance Code / Reason Picklists Slow
This task changes the SDS Gavintech app shared component that is used by all or most screens with an Attendance Code / Reason picklist so that it internally uses the database more efficiently and thus performs better. For a typical scenario of displaying 20 attendance codes, it now performs this using exactly 1 database call rather than 21 database calls. This benefit is seen on every screen using this picklist, not just 1.
While the impact of this change alone on a single user action is likely too small for a user to notice, it still contributes to efficiency, and in aggregate, performance improvements users would notice.
That change was entirely in the single PHP source file
overrides/classes/class_dropdowns.php
, in its single routine
attendanceCode()
.
This task also changes the internals of the 1 screen Staff Menu -> Attendance -> Input Absences such that it now uses the same, now improved, app component for its Reason picklist, rather than duplicating it.
That change was entirely in the single PHP source file
pages/attendance/input_absence.php
.
Internal Design Changes
2024 Apr 19: SDS Laravel: Consolidate Eloquent model classes to /app/Models
An idiomatic PHP Laravel project has its main files grouped into the specific hierarchy of folders documented here:
https://laravel.com/docs/11.x/structure
Of particular relevance, the folder /app/Models
is where all
of the Laravel Eloquent model classes go.
Prior to the performance of this task, SDS Laravel conformed to the
idiomatic folder layout in all ways except with respect to the Eloquent
model classes, which it had instead spread across about a dozen
/app/FooModel
folders.
This task moved/renamed all of the Eloquent model classes, such that for
each class /app/FooModel/Bar.php
, it was moved/renamed to
/app/Models/Foo/Bar.php
. While that described nearly all of
those classes, a handful started with other names, but they were
appropriately gathered under /app/Models
too.
This task also included a normalization of how first-party classes were referred to in other code, such that any which weren't referred to by their fully-qualified names before were referred to in that way after.
2024 May 8: SDS Laravel: Sever Coupling of PHP Class Names to Database Data
Laravel Eloquent has a particular more-advanced feature for defining relationships between model classes that it refers to as a polymorphic relationship, such that a relationship is not just between 2 specific model classes, but rather between 1 model class and 1 set of all model classes that fulfill a particular role.
https://laravel.com/docs/8.x/eloquent-relationships#polymorphic-relationships
Prior to the performance of this task, SDS Laravel utilized such an
Eloquent relationship, between Group
on one side, and the set
comprising Page
and SubPage
on the other side.
And it did so in a design-flawed way that led to tight coupling between the
exact fully-qualified PHP class names of the latter 2 model classes, and
the database data, such that those fully-qualified class names
(App\SubModel\Page
and App\SubModel\SubPage
) were
stored in the database (specifically in the group_pages
table's class_type
column) and the fact that these matched was
relied on by the application for correct behavior.
This tight coupling manifested as broken app behavior when attempting to
move/rename the Page
and SubPage
classes (to
App\Models\Sub\Page
and App\Models\Sub\SubPage
),
because then their names no longer matched what was in the database.
The most common kind of breakage was that, for almost every app screen that was gated with a user group privilege check for the screen, the app thought no user had any privilege to use it; for example, this error message displayed when trying to app general search screen:
No access to Search.index You do not appear to have permission to access https://api-latest.smus.ca/Search as [User's Name]. If you believe you should have access to open a help ticket request permission, or you can go back / home and try again.
And had users been able to get past that, certain other screens or functionality that rely on displaying or editing user group privileges for pages and subpages would have had problems, such as failing to see existing privileges made before the rename (they might have worked to re-add the privileges, then using the new hard-coded class names in the database).
Without correcting the design flaw, renaming these model classes would have required a corresponding database migration to replace the old hard-coded-in-data PHP class names with their new ones, which carried more complexity and risk.
The more specific design flaw was the use of Eloquent's
morphToMany()
relationship builder without specifying an
explicit constant string to represent each morphed class in the database,
resulting in Eloquent defaulting to using its fully-qualified PHP class
name instead.
Both Page
and SubPage
had a copy of this method
(this version is post-rename) which was affected:
public function groups(){ return $this->morphToMany(\App\Models\Sub\Group::class, 'class', 'group_pages'); }
This task made the simplest possible fix to this design flaw by explicitly
configuring Eloquent to use an explicit constant string per each morphed
class, by adding the following in the file
app/Providers/AppServiceProvider.php
:
Relation::enforceMorphMap([ 'App\SubModel\Page' => 'App\Models\Sub\Page', 'App\SubModel\SubPage' => 'App\Models\Sub\SubPage', // TODO: Migrate database to use non-PHP-class-looking slugs // like following instead so they don't look tightly coupled. // 'page' => 'App\Models\Sub\Page', // 'sub_page' => 'App\Models\Sub\SubPage', ]);
This was the simplest possible fix, and ostensibly the most elegant, by
working with the system, using an Eloquent feature expressly designed for
this very scenario, rather than say trying to avoid using the
morph
features (although doing so may be better longer term),
and also by not requiring any database changes, so it works with the
database as-is.
https://laravel.com/docs/8.x/eloquent-relationships#custom-polymorphic-types
Following the changes of this task, there was still room for an even more elegant solution to follow that alters the database to use a non-php-class-looking constant in the database data to represent each class, so the app isn't forever using the old class name there, but that was considered higher complexity and risk and suited to be done as a separate task another day.
Note that a consequence of using Relation::enforceMorphMap()
is that it makes Eloquent treat the entire app more strictly, such that if
we try to use morphToMany()
or similar Eloquent methods with
any other classes besides Page
and SubPage
, it
will require us to add an entry to the above list for it, rather than
silently defaulting to the old behavior for other unspecified classes. I
consider this strictness to be a good thing.
2024 Aug 21: SDS Gavintech: Sever Coupling of Main Menu Builder to Write/Edit Reports Screen
This task refactored SDS Gavintech to sever the coupling between the
main menu builder, defined in the single PHP source file
overrides/modules/sidebar_definition/smus_sidebar_definition.php
,
and the screen known alternately as
My Courses -> $course_name -> Reports & Marks -> Write reports -> $student_name
and
Staff Menu -> Advisor stuff -> Edit Reports -> $student_name -> $course_name,
defined in the single PHP source file
pages/reports/reports_entry_spell.php
.
That coupling took the form of the main menu builder fetching a list of
students from the database on behalf of the screen and storing that list of
students in the PHP session variable
$_SESSION["reports_entry_spell"]
, which was subsequently read
by the screen. The use of this session variable was one-way; the menu
builder only set it and the screen only read it. The menu builder set it
unconditionally from scratch, and so could have just used a PHP "global"
instead, which would have saved the app from the overhead of serializing
and deserializing its value to disk, which may have consisted of a thousand
"Student" PHP objects. The menu builder set it anew every time the user
loaded/reloaded any app screen, and not just when viewing the specific
screen it was used on, meaning almost all the time it consumed resources to
do that work for nothing.
The changes of this task consisted mainly of placing a stripped down clone of the main menu builder source file into the top of the screen source file, just the parts that built the student list. The changes otherwise consisted of entirely eliminating the use of the PHP session variable.
The menu builder source file had no changes except for removing the 2 lines that set the session variable; the screen source file used a local variable to hold the student list instead of reading it from the session variable; the stripped down clone it gained totalled 70 lines of code, which was 8% of the menu builder source file's lines.
Following the changes of this task, the actual performance of the app was identical, using exactly the same count of database calls per each screen as prior to the changes of this task, and also the same page generation time. This was tested both on the app home screen, which used the menu builder logic but not the screen logic, and on the screen, which used both. Despite the duplicate database call main logic, this would be explained by these database calls being mediated by both an app SQL query cache and an app PHP object cache, which recognized that the results of the first instance of the calls by the menu builder could be reused by the screen.
This decoupling resulted in the app being easier to maintain, as one can now change either the menu builder or the screen arbitrarily without being concerned that changes to one would affect the other, as conceptually the two things should be completely separate.
Changes to Third-Party Dependencies
2024 May 3: SDS Laravel: Stop Requiring dbal, revisionable-upgrade, laravel-cors
Prior to the performance of this task, the SDS Laravel Composer config
file composer.json
explicitly required these 3 third-party
PHP library dependencies, which were not actually used by the app:
- doctrine/dbal (^3.5)
- fico7489/laravel-revisionable-upgrade (*)
- fruitcake/laravel-cors (^2.0)
This task updated composer.json
to remove those unused
explicit dependencies, which also had the effect of removing 6 other
Composer-resolved dependencies.
2024 May 6: SDS Laravel: Reflect rename of fzaninotto to fakerphp
This task updated SDS Laravel composer.json
to reflect that
the third-party PHP library dependency fzaninotto/faker
was
renamed to fakerphp/faker
(in fact, that rename occurred prior
to the release of Laravel 8), and also require its latest version.
2024 May 6: SDS Laravel: Upgrade PHPUnit from 9.x to 10.x
This task updated SDS Laravel composer.json
to require the
latest PHP-8.1-compatible version of the PHP dev dependency PHPUnit from
9.6.19
to 10.5.20
.
2024 Jul 15: SDS Laravel: Upgrade Laravel Mix from 5.x to 6.x and Upgrade Webpack
This task updated SDS Laravel so that its JavaScript build pipeline is implemented by Laravel Mix 6 rather than 5, and likewise a newer major version of Webpack.
This task changed these dependencies in package.json
:
- cross-env (^7.0.3 removed as laravel-mix has its upgrade built-in)
- postcss (^8.4.39 added)
- laravel-mix (^5.0.9 to ^6.0.49)
- vue-loader (^15.4.2 to ^15.11.1)
This newer pipeline should be usable in the same way as the prior one, with
npm run prod
and npm run dev
etc, but Laravel Mix
6 makes it considerably more streamlined and ostensibly considerably less
fragile than the Laravel Mix 5 version was. Much less underlying Webpack
API details are exposed in the "scripts" commands, and also cross-env is no
longer directly used and is dropped as a dependency.
The following substitution was performed in package.json
:
"scripts": { "dev": "npm run development", "development": "cross-env NODE_ENV=development node_modules/webpack/bin/webpack.js --progress --hide-modules --config=node_modules/laravel-mix/setup/webpack.config.js", "watch": "npm run development -- --watch", "watch-poll": "npm run watch -- --watch-poll", "hot": "cross-env NODE_ENV=development node_modules/webpack-dev-server/bin/webpack-dev-server.js --inline --hot --config=node_modules/laravel-mix/setup/webpack.config.js", "prod": "npm run production", "production": "cross-env NODE_ENV=production node_modules/webpack/bin/webpack.js --no-progress --hide-modules --config=node_modules/laravel-mix/setup/webpack.config.js" },
"scripts": { "dev": "npm run development", "development": "mix", "watch": "mix watch", "watch-poll": "mix watch -- --watch-options-poll=1000", "hot": "mix watch --hot", "prod": "npm run production", "production": "mix --production" },
Two other files each had a single line change or addition for compatibility with the Laravel Mix version change.
The following substitution was performed in webpack.mix.js
:
mix.js('resources/js/app.js', 'public/js') .sass('resources/sass/app.scss', 'public/css');
mix.js('resources/js/app.js', 'public/js') .vue() .sass('resources/sass/app.scss', 'public/css');
That is Laravel Mix 6 no longer assumes one is always using Vue, so now one
opts into processing its templates explicitly with the .vue()
.
The following substitution was performed in resources/js/app.js
:
window.Vue = require('vue');
window.Vue = require('vue').default;
Note that Laravel Mix 6 is required for Vue 3 compatibility but it also works with Vue 2; the latter is still in use as of this task's changes.
2024 Jul 15: SDS Laravel: Replace laravel/helpers With laravel/framework
This task updated SDS Laravel composer.json
to purge the
PHP library dependency laravel/helpers
, and also updated any
SDS Laravel PHP source files that used it to instead use
laravel/framework
built-ins.
These 4 PHP source files were updated:
- app/Http/Controllers/Auth/RegisterController.php
- resources/views/admin/subjects/edit_rotating_courses/show.blade.php
- resources/views/student/student_mark_collections/edit.blade.php
- resources/views/student/student_mark_collections/show.blade.php
For each of the above 4 files, there were either of these 3 subsitutions:
str_contains(...) str_random(...) str_replace_array(...)
Str::contains(...) Str::random(...) Str::replaceArray(...)
Note that laravel/helpers
was just a single PHP source file
providing trivially thin wrapper functions over
laravel/framework
6+ built-ins that emulated ones from Laravel
5.8, and we were already mostly using the newer versions, so this task just
replaced the last 7 instances where we weren't.
2024 Jul 17: SDS Laravel: Upgrade Font Awesome from 4.x plus 5.x plus CDN to 6.x
This task updated SDS Laravel to require the latest version of the JavaScript library dependency Font Awesome, to 6.x from a mixture of 3 different versions 4.x plus 5.x plus CDN.
This task changed these dependencies in package.json
:
- @fortawesome/fontawesome-free (^5.15.4 to ^6.6)
- font-awesome (^4.7 removed as @fortawesome/fontawesome-free is its upgrade)
This task deleted references to this CDN dependency in 3 app Blade template PHP source files:
- https://use.fontawesome.com/releases/v5.7.1
This task also made a small tweaks to a few other files for compatibility,
replacing font/symbol/style name fa
with far
.
2024 Jul 17: SDS Laravel: Stop Requiring tempusdominus, bootstrap-select, jquery-ui, lodash
Prior to the performance of this task, the SDS Laravel config
file package.json
explicitly required these 5 third-party
JavaScript library dependencies, which were not actually used by the app:
- bootstrap-select (^1.13.18 removed as not in use)
- jquery-ui (^1.13.3 removed as not in use)
- lodash (^4.17.21 removed as only used indirectly by laravel-mix)
- tempusdominus-bootstrap-4 (^5.39.2 removed as not in use)
- tempusdominus-core (^5.19.3 removed as not in use)
This task updated package.json
to remove those unused
explicit dependencies.
This task also updated 5 other app files for compatibility, typically by deleting explicit "require" or similar statements to these libraries since they weren't actually used otherwise.
However, these 2 files were special:
- resources/views/admin/subjects/edit_subject/show.blade.php
- resources/views/application/studentinfo/index.blade.php
Those 2 files actually DID use removed dependencies, and so technically they were broken by this task. But the 2 screens in question were not currently in use and so temporarily breaking them was acceptable. One screen was used historically but not any more. The other screen was still under construction. This task added comments to those 2 files explaining that they will need to be modified to use alternate and modern dependencies if they are going to be completed or used again.
2024 Jul 18: SDS Laravel: Upgrade axios
This task updated package.json
to upgrade the JavaScript library
dependency axios
from 0.27.2
to 1.7.2
.
2024 Jul 25: SDS Laravel: Upgrade Vue.js from 2.x to 3.x and Upgrade Co-Dependent JS deps
This task updated package.json
to require the latest versions
of vue
as well as of all of its co-dependent JavaScript
library dependencies.
To be specific, it made these dependency changes:
- @formkit/pro (^0.119.14 added)
- @formkit/themes (^1.6.5 added)
- @formkit/vue (^1.6.5 added)
- css-loader (^7.1.2 added)
- vue (^2.7.16 to ^3.4.34)
- vue-loader (^15.11.1 to ^17.4.2)
- vue-bootstrap-datetimepicker (^5.0.1 removed as @formkit/pro "datepicker" is used in its' place)
- vue-template-compiler (^2.7.16 removed as vue 3 has its upgrade built-in)
Several changes for compatibility were made to the app source files.
Partially rewrote source file resources/js/app.js
to account
for Vue 3 not having a single global state like Vue 2 but multiple possible
object-scoped states. Many of the same changes were previously made in
SDS Gavintech which served as a guide.
Rewrote source file resources/js/components/inputs/DateTime.vue
to use @formkit/pro
rather than
vue-bootstrap-datetimepicker
. The new version is a clone from
SDS Gavintech which had the corresponding changes made earlier.
Updated source file resources/js/components/admin/finance/Fees.vue
in several ways (some previously done in SDS Gavintech, some brand new).
As Vue 3 removed the filters feature, their use was replaced with ordinary
function calls; x|format_year
was replaced with format_year(x)
.
As Vue 3 reversed the precedence of v-if
plus v-for
in the same HTML/template tag, added a child template
tag of
the tr
that used both, to force the expected precedence.
Updated source file resources/js/components/application/AddressInfo.vue
to fix a syntax/logic error that Vue 2 overlooked but Vue 3 doesn't.
The logic on the System Settings -> Pages screen to fold/unfold the page
hierarchy was disabled as it was incompatible with Vue 3 and not essential
so the tree is just permanently unfolded. This was done by small changes in
the source file resources/views/system/page/index.blade.php
.
The System Settings -> Groups screen was altered to be read-only because
the logic for adding/editing/deleting database records was incompatible
with Vue 3, a fix was elusive, and this was a good enough 90% solution for
the short term given the infrequent use of the screen; a follow-up task can
be performed later to enable editing on this screen again when needed.
This was done by small changes in the source file
resources/views/system/group/index.blade.php
.
Formatting configuration for date pickers was updated on a dozen or two
screens replacing any LL
and LLL
shorthands with
their longhands, namely D MMM YYYY
and
D MMM YYYY HH:mm
respectively, since the new date pickers
don't support the shorthands, only the longhands.
This task also resulted in unintended changes in the app's visual appearance, such as fonts being a bit smaller or some elements being in slightly different locations. The cause of this proved elusive, and since it isn't actually a functional break, this didn't hold back the Vue 3 upgrade. A follow-up task can be performed later if changes to the visuals are desired.
2024 Aug 2: SDS Laravel: Upgrade directorytree/ldaprecord-laravel from 2.x to 3.x
This task updated SDS Laravel composer.json
to require the
latest version the PHP library dependency
directorytree/ldaprecord-laravel
, going from 2.7.3
to 3.3.3
.
This task also made 3 distinct sets of app source code changes to be compatible with that upgrade.
See also for context:
- https://ldaprecord.com/docs/laravel/v3/upgrading/
- https://ldaprecord.com/docs/laravel/v3/release-notes/
- https://github.com/DirectoryTree/LdapRecord-Laravel/releases
The first set of changes...
This task updated the 1 PHP config source file config/ldap.php
to account for logging
now being an array. There was this 1
substitution:
'logging' => env('LDAP_LOGGING', true),
'logging' => [ 'enabled' => env('LDAP_LOGGING', true), ],
The second set of changes...
For context, directorytree/ldaprecord-laravel
had a PHP trait
named LdapRecord\Laravel\Auth\MultiDomainAuthentication
which
was deprecated before version 2.7.3
and removed in version
3.0.0
. The 1 SDS PHP source file
app/Http/Controllers/Auth/LoginController.php
used that trait.
This task updated LoginController
to clone into itself the
used portions of MultiDomainAuthentication
, thus removing the
external dependency of the former on the latter.
As the removed trait was itself a sub-trait of
LdapRecord\Laravel\Auth\CreatesUserProvider
,
LoginController
now composed that directly and not indirectly.
Also LoginController
gained the new protected function
getLdapGuard
cloned from the removed trait.
Note that the function getLdapGuardFromRequest
was not
cloned from the removed trait since LoginController
already
had its own version that overrode it.
The third set of changes...
For context, each of these 5 SDS classes composed one of the 2 classes
LdapRecord\Models\Model
or LdapRecord\Models\Scope
:
- app/Ldap/ExternalUser.php
- app/Ldap/Scopes/OnlyStaffUsers.php
- app/Ldap/Scopes/OnlyStudents.php
- app/Ldap/SmusStudents.php
- app/Ldap/SmusUser.php
For each of those 5, it was updated such that for any property or method it
contained which overrode or implemented a same-named one from
Model
or Scope
, that property or method had
explicit type annotations added to match the originals. This was required
for the SDS code to satisfy a PHP or Laravel stricture so the code runs.
2024 Aug 2: SDS Laravel: Upgrade intervention/image from 2.x to 3.x and Fix Import Photos Screen
This task made 3 distinct groups of changes to SDS Laravel all related to image handling and the Admin Menu -> Migration -> Import Photos screen.
Changes For intervention/image Upgrade
This task also made 2 distinct sets of changes to be compatible with the
intervention/image
upgrade.
For context, as part of the major update of intervention/image
from version 2.x to 3.x, it was also split up into multiple libraries, with
the core remaining under the old name and the optional Laravel-specific
add-ons being in the new intervention/image-laravel
library.
We only need to directly require the latter, which brings in the former.
See also for context:
- https://image.intervention.io/v3/introduction/upgrade
- https://github.com/Intervention/image-laravel/blob/main/README.md
- https://image.intervention.io/v3/modifying/resizing
The first set of changes...
This task added the 1 PHP config source file config/image.php
that is standard per the above-linked README.
The second set of changes...
This task replaced the use of the no longer existing PHP class
Intervention\Image\ImageServiceProvider
with the existing
Intervention\Image\Laravel\Facades\Image
.
In addition, the class is now used directly by its full name in the 2 PHP
source files where the image handling functionality is actually used, which
are app/Models/File.php
and
app/Http/Controllers/Admin/Migration/ImportPhotosController.php
.
This task has purged any explicit registration in config/app.php
of any class of this library in providers
or
aliases
; the Image
alias is now purged.
The PHP source code that directly used the Intervention Image libraries was updated for compatibility with several method renames or substitutions.
Changes were made to reflect the rename of the library static method
make
to read
.
Changes were made to reflect the changes with the set of image manipulation
methods for image resizing. The resize
method of version 2 had
taken a third argument by which one specified the resize should keep the
original aspect ratio. The resize
method of version 3 doesn't
take that argument, and instead the version 3 method scale
is
explicitly for resizing that maintains the aspect ratio.
Bug/Security Fixes In Photos Import
This task also updated the 2 files
app/Http/Controllers/Admin/Migration/ImportPhotosController.php
plus
resources/views/admin/migration/import_photos/index.blade.php
to fix bugs tangential to the image file path handling.
One fix was in the index
function so that the directory path
reported to the user was the same one that store
actually
used, from Storage::disk('photos')
, rather than being either
of a couple of hard-coded alternatives toggled on an environment variable
that isn't actually used.
A second fix was to stop passing a path
hidden form field
value from index
to store
, whose value was set
from the incorrect path in index
, and which store
validated as present and subsequently didn't use. Besides being dead code
from the disuse, if this had been used it would have been a major security
vulnerability, as it would have let web clients directly control actions
against server local file system paths.
A third fix was in the store
such that logic to skip over
files that were not of type image/jpeg
was faulty and never
skipped any kind of file, and now it skips those it is supposed to.
The specific logic error was one of operator precedence, where it was of
the form !x == y
where it meant to be !(x == y)
;
the fixed version made it be x !== y
.
Changes For league/flysystem Indirect Upgrade
This task also updated
app/Http/Controllers/Admin/Migration/ImportPhotosController.php
to be compatible with the league/flysystem
upgrade from
version 1 to version 3 that was indirect by way of the Laravel 9 upgrade.
See also for context:
- https://flysystem.thephpleague.com/docs/upgrade-from-1.x/
- https://laravel.com/docs/9.x/upgrade#flysystem-3
There were 3 substitutions for compatibility in the store
function.
There was 1 like this:
Storage::disk('photos')->getMimeType($file)
Storage::disk('photos')->mimeType($file)
And there were 2 like this:
Storage::disk('photos')->getDriver()->getAdapter()->applyPathPrefix($file);
Storage::disk('photos')->path($file);
This task also updated config/filesystems.php
to add the
element 'throw' => true
to each disks
element.
This is to help preserve Laravel 8 behavior that we might have been relying
on when a write operation fails. For example, write operations such as
write
by default no longer throw an exception when a write
operation fails and instead returns false
; until we can audit
our code for how we currently check for such failures, this config change
should make it less likely our code was broken by the dependency update.
Note that there are still other kinds of default behavior changes that we should audit our code for handling of. For example, write operations will overwrite existing files by default, so if we don't want that then we should explicitly be checking for their existence first. Also, reading a nonexisting file now returns null rather than an exception.
2024 Aug 2: SDS Laravel: Upgrade laravel/framework from 8.x to 10.x and Upgrade Co-Dependent PHP deps
This task made 5 distinct groups of changes to SDS Laravel all related to upgrading it from Laravel 8 to Laravel 10.
Summary of PHP Dependency Upgrades or Removals
This task updated composer.json
to require the latest
PHP-8.1-compatible versions of laravel/framework
as well as of
all of its co-dependent PHP library dependencies. In some cases, upgrading
co-dependents meant removing them entirely in favor of Laravel built-ins.
To be specific, it made these dependency changes:
- barryvdh/laravel-debugbar (^3.7 to ^3.13.5)
- directorytree/ldaprecord-laravel (^3.3.3 unchanged)
- etern8ty/beanstream (dev-master unchanged and is custom fork)
- fakerphp/faker (^1.23.1 unchanged)
- fideloper/proxy (^4.4.2 removed as laravel/framework has its upgrade built-in)
- goldspecdigital/laravel-eloquent-uuid (^8.0.1 removed as laravel/framework has its upgrade built-in)
- guzzlehttp/guzzle (^7.9.2 unchanged)
- intervention/image-laravel (^1.3 unchanged)
- juliomotol/laravel-auth-timeout (^3.1.1 to ^4.1)
- lab404/laravel-impersonate (^1.7.5 unchanged)
- laravel/framework (^8.83.27 to ^10.48.18)
- laravel/tinker (^2.9 unchanged)
- laravel/ui (^3.4.6 to 4.5.2; but it recommends using Laravel Breeze or Laravel Jetstream instead)
- mockery/mockery (^1.6.12 unchanged)
- nunomaduro/collision (^5.11 to ^7.10)
- phpunit/phpunit (^10.5.29 unchanged)
- spatie/laravel-ignition (^1.7 to ^2.8)
- staudenmeir/eloquent-has-many-deep (^1.14.4 to ^1.19.4)
Specify Minimum PHP Dependency Versions Used Are Stable
This task updated composer.json
to specify an app default
minimum-stability
of stable
rather than
dev
.
This is per the Laravel 10 upgrading recommendation:
While the setting could instead have been removed as stable
is
its default, being explicit seemed better here.
Note that this didn't cause any changes to what PHP library versions are actually installed by Composer versus the old setting, but is a good constraint going forward.
Changes For laravel/framework Upgrade's Removal of "dates" Model Attribute
This task also updated 51 PHP source files to be compatible with a breaking change made by Laravel itself with version 10.
Laravel supported a "dates" model attribute through version 9, and then Laravel 10 removed it. The function of this was to enumerate database/model fields that were supposed to be automatically converted to Carbon DateTime objects; so under Laravel 8, any "dates" declarations would be respected, while under Laravel 10 they would be ignored.
See also:
Compare:
- https://laravel.com/api/9.x/Illuminate/Database/Eloquent/Concerns/HasAttributes.html
- https://laravel.com/api/10.x/Illuminate/Database/Eloquent/Concerns/HasAttributes.html
As a result, simply upgrading SDS Laravel from Laravel 8 to 10 resulted
in many parts of the app breaking in various ways including when simply
visiting the post-login home screen, as PHP died with errors like
Call to a member function format() on int
.
To fix this, any instances of protected $dates = ['x',...]
in model classes were replaced with
protected $casts = ['x'=>'datetime',...]
which was the more modern way to get the same functionality, which exists
in both Laravel 8 and 10. For the few model classes that already had other
$casts
declarations, the replacements were merged with those.
While the "dates" change could have been its own task that was merged to trunk prior to and separately from the current Laravel 10 upgrade task, it was combined with the latter to streamline testing, as both had potential impacts over a large fraction of the app.
Changes For laravel/framework Upgrade's Miscellaneous Changes
This task also updated app/Providers/AuthServiceProvider.php
to remove the call $this->registerPolicies();
from
boot()
because Laravel 10 now calls it automatically, as
recommended here:
Changes For goldspecdigital/laravel-eloquent-uuid Removal
This task also updated these 5 PHP source files to be compatible with the
replacement of goldspecdigital/laravel-eloquent-uuid
with a
Laravel built-in:
- app/Models/Application/Application.php
- app/Models/User.php
- app/Models/User/Student.php
- app/Models/User/Teacher.php
- app/Models/User/UserContract.php
These further 3 files also referenced the trait but commented out, so not current users but possible past or future users:
- app/Models/Application/AppUser.php
- app/Models/User/Address.php
- app/Models/User/Guardian.php
For each of the above 8 files, there were these 2 line subsitutions:
use GoldSpecDigital\LaravelEloquentUUID\Database\Eloquent\Uuid; use Uuid;
use Illuminate\Database\Eloquent\Concerns\HasUuids; use HasUuids;
Here is a description of the above built-in feature in Laravel 9.3+:
https://laravel.com/docs/11.x/eloquent#uuid-and-ulid-keys
The purpose of that reimplemented functionality was to empower use of generated UUIDs for primary key fields of some database tables instead of the serially generated integers that SDS Laravel more typically uses; Laravel Eloquent only gained built-in support for UUIDs with version 9.3.
This task also deleted the single PHP file
app/Models/Traits/Uuids.php
as it appeared to be unused.
Changes For fideloper/proxy Removal
This task also updated app/Http/Middleware/TrustProxies.php
to
be compatible with the replacement of fideloper/proxy
with a
Laravel built-in. The changes were in 2 spots.
First was this substitution:
use Fideloper\Proxy\TrustProxies as Middleware;
use Illuminate\Http\Middleware\TrustProxies as Middleware;
Second was this substitution:
protected $headers = Request::HEADER_X_FORWARDED_ALL;
protected $headers = Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_X_FORWARDED_AWS_ELB;
See also https://laravel.com/docs/9.x/upgrade under "Trusted Proxies".
Changes For juliomotol/laravel-auth-timeout Upgrade
This task also updated
app/Http/Middleware/AuthTimeoutMiddleware.php
to be
compatible with the juliomotol/laravel-auth-timeout
upgrade.
There was this 1 substitution:
use JulioMotol\AuthTimeout\Middleware\AuthTimeoutMiddleware as BaseMiddleware;
use JulioMotol\AuthTimeout\Middlewares\CheckAuthTimeout as BaseMiddleware;
Note that juliomotol/laravel-auth-timeout
must be upgraded
simultaneously with Laravel since the former's versions 3.1.1 and 4.1
respectively require Laravel 8 and 10 respectively.
See https://github.com/juliomotol/laravel-auth-timeout/blob/master/CHANGELOG.md for more change details and upgrade notes on that.
Documentation Changes
2024 Jul 16: SDS Laravel: Make README etc Say This App is SDS Rather Than Laravel
This task updated SDS Laravel composer.json
to update
primary documentation attributes so they describe the actual SDS
application this is rather than the Laravel template used to build it.
There were these substitutions:
"name": "laravel/laravel", "description": "The Laravel Framework.", "keywords": ["framework","laravel"],
"name": "smus/sds", "description": "School Data System (SDS)", "keywords": [],
This task also completely rewrote README.md
so it describes
this SDS application rather than the Laravel framework.