SDSM:Historical Changes

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.

RETURN

SDS Laravel: Fixes to Broken Behavior

2024 Apr 18: 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.

RETURN

2024 May 21: 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.

RETURN

2024 Jun 12: 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.

RETURN

2024 Jul 4: 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.

RETURN

2024 Jul 23: 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.

RETURN

SDS Laravel: Fixes to Slow Performance

2024 May 14: 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

RETURN

SDS Laravel: Internal Design Changes

2024 Apr 19: 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.

RETURN

2024 May 8: 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.

RETURN

SDS Laravel: Changes to Third-Party Dependencies

2024 May 3: 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.

RETURN

2024 May 6: 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.

RETURN

2024 May 6: 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.

RETURN

2024 Jul 15: 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.

RETURN

2024 Aug 2: 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.

RETURN

2024 Aug 2: 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.

RETURN

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.

RETURN

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.

RETURN

2024 Aug 2: 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)

RETURN

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:

• https://laravel.com/docs/10.x/upgrade#updating-minimum-stability

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.

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 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.

RETURN

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:

• https://laravel.com/docs/10.x/upgrade#register-policies

RETURN

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.

RETURN

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".

RETURN

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.

RETURN

SDS Laravel: Documentation Changes

2024 Jul 16: 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.

RETURN