SDSM:Pending Changes: Difference between revisions

From SMUSwiki
Jump to navigation Jump to search
(Add item on timeout broken on sub-directory based apps)
(Large updates to the item on Laravel 8 to 10 upgrade, merging in 2 other items, using UUID built-in)
Line 73: Line 73:
these references are in the generated file <code>public/css/app.css</code>
these references are in the generated file <code>public/css/app.css</code>
as parts of third-party dependencies, and so these were left alone.
as parts of third-party dependencies, and so these were left alone.
== SDS Laravel: Internal Design Changes ==
=== 2024 May 7: Replace model attrs "dates" with "casts" ===
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.
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.
The changes of this task were not merged to trunk yet because it was deemed
important to give each instance of the changed 51 model classes more
thorough testing before merging than they had received as of 2024 May 7.
One key reason for this is the idea that the fields in question might not
all be date+time, and rather may be date-only or time-only.
All ''SDS Laravel'' screens accessible from a main menu item were found to
load and at a glance seemed to display correctly, with these changes
included, but that is as far as any testing went as of 2024 May 7.
(There are 2 such screens that didn't load, but they didn't load prior to
these changes either, and that is a separate concern.)
As of 2024 May 7, the changes of this task have been bundled with the
changes of the task upgrading ''SDS Laravel'' from Laravel 8 to 10, so the
sum changes would receive the necessary thorough testing together before
being merged to trunk. However, since the changes of this task do not
depend on the upgrade, it is possible that a subset of them may be fully
tested and merged sooner when other changes using the same models occur.
[[#top|RETURN]]


== SDS Laravel: Changes to Third-Party Dependencies ==
== SDS Laravel: Changes to Third-Party Dependencies ==


=== 2024 May 16: Stop requiring laravel-eloquent-uuid ===
=== 2024 May 21: Upgrade Laravel from 8.x to 10.x ===


''SDS Laravel'' uses serially generated integers in most of its database
This task updated <code>composer.json</code> to require the latest
tables for their primary key fields, but for a small number of tables it
PHP-8.1-compatible major version of the PHP library dependency Laravel,
instead uses generated UUIDs for their primary key fields.
thus taking it from <code>8.x</code> to <code>10.x</code>.


Prior to the performance of this task, the third-party PHP library
To be more specific, it made these dependency changes:
dependency <code>goldspecdigital/laravel-eloquent-uuid</code> version
<code>v8.0.1</code> was used to empower such UUID use, since Laravel
Eloquent itself did not support these prior to version 9.3. While we can
make use of equivalent functionality built-in to Laravel once we upgrade
to Laravel 10, we can't while we are on Laravel 8.


Fundamentally, the feature in question that we use is summed up by a single
* barryvdh/laravel-debugbar (^3.7 to ^3.13.5)
very small PHP trait defined in a single file of a few dozen lines of code.
* directorytree/ldaprecord-laravel (^2.7.3 unchanged but upgrade exists)
This is the case both for the <code>goldspecdigital</code> version used
* etern8ty/beanstream (dev-master unchanged but upgrade exists)
prior to this task and the Laravel built-in that can be used later.
* fakerphp/faker (^1.23.1 unchanged)
 
* fideloper/proxy (^4.4.2 removed as Laravel has its upgrade built-in)
Here is a description of the built-in feature in Laravel 9.3+:
* goldspecdigital/laravel-eloquent-uuid (^8.0.1 removed as Laravel has its upgrade built-in)
 
* guzzlehttp/guzzle (^7.8.1 unchanged)
https://laravel.com/docs/11.x/eloquent#uuid-and-ulid-keys
* intervention/image (^2.7.2 unchanged but upgrade exists)
* juliomotol/laravel-auth-timeout (^3.1.1 to ^4.1)
* lab404/laravel-impersonate (^1.7.5)
* laravel/framework (^8.83.27 to ^10.48.11)
* laravel/helpers (^1.7 unchanged but possibly no longer needed)
* laravel/tinker (^2.9 unchanged)
* laravel/ui (^3.4.6 to 4.5.2)
* mockery/mockery (^1.6.12 unchanged)
* nunomaduro/collision (^5.11 to ^7.10)
* phpunit/phpunit (^10.5.20 unchanged)
* spatie/laravel-ignition (^1.6.4 to ^2.7)
* staudenmeir/eloquent-has-many-deep (^1.14.4 to ^1.19.3)


Regardless of which of these options is used, most of core UUID-specific
This task also updated these 5 PHP source files to be compatible with the
functionality is provided by the third-party PHP library
replacement of <code>goldspecdigital/laravel-eloquent-uuid</code> with a
<code>ramsey/uuid</code>, and Laravel 8+ itself already has this as its own
Laravel built-in:
dependency for other reasons.
 
The primary change of this current task is to clone the single PHP file
from <code>goldspecdigital</code> that we actually use into ''SDS Laravel''
itself, as the new file <code>app/Models/Traits/Uuid.php</code>, and to
update <code>composer.json</code> to remove the explicit external
dependency on <code>goldspecdigital/laravel-eloquent-uuid</code>. In the
process, the fully-qualified PHP trait name was renamed to
<code>App\Models\Traits\Uuid</code> from
<code>GoldSpecDigital\LaravelEloquentUUID\Database\Eloquent\Uuid</code>,
but it had no other changes of any kind.
 
The only 5 other app PHP source files that directly used it were updated to
account for the new name:


* app/Models/Application/Application.php
* app/Models/Application/Application.php
Line 176: Line 121:
* app/Models/User/Guardian.php
* app/Models/User/Guardian.php


Another change made by this current task was to delete the single PHP file
For each of the above 8 files, there were these 2 line subsitutions:
<code>app/Models/Traits/Uuids.php</code> as it appeared to be unused.


Note that <code>goldspecdigital/laravel-eloquent-uuid</code> had a newer
    use GoldSpecDigital\LaravelEloquentUUID\Database\Eloquent\Uuid;
version <code>10.x</code> that was explicitly compatible with Laravel 10,
    use Uuid;
however a source comparison of the 2 versions found that they were actually
identical in every way aside from adding a half dozen PHP type annotations
in the source code and changing the <code>composer.json</code> to require
Laravel 10 instead of 8.
As such, it was actually the <code>Uuid.php</code> file from version 10
cloned by this task rather than the one from version 8, so we benefitted
from the added PHP type annotations, and the behavior should be identical.


The net effect of this task was to maintain exactly the same app behavior
    use Illuminate\Database\Eloquent\Concerns\HasUuids;
while dropping a hard external dependency, this reducing our overall
    use HasUuids;
complexity and simplifying the later task of upgrading to Laravel 10.


It is expected that the future upgrade to Laravel 10 will take the next
Here is a description of the above built-in feature in Laravel 9.3+:
step and remove this cloned trait file, and its users will use the
Larvel 10 built-in instead.


[[#top|RETURN]]
https://laravel.com/docs/11.x/eloquent#uuid-and-ulid-keys


=== 2024 May 3: Upgrade Laravel from 8.x to 10.x ===
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 updated <code>composer.json</code> to require the latest
This task also deleted the single PHP file
PHP-8.1-compatible major version of the PHP library dependency Laravel from
<code>app/Models/Traits/Uuids.php</code> as it appeared to be unused.
8.x to 10.x.
 
To be more specific, it made these dependency changes:
 
* barryvdh/laravel-debugbar (^3.7 to ^3.13.5)
* directorytree/ldaprecord-laravel (^2.7.3 unchanged but upgrade exists)
* etern8ty/beanstream (dev-master unchanged but upgrade exists)
* fakerphp/faker (^1.23.1 unchanged)
* fideloper/proxy (^4.4.2 removed as Laravel has its upgrade built-in)
* goldspecdigital/laravel-eloquent-uuid (^8.0.1 to ^10.0)
* guzzlehttp/guzzle (^7.8.1 unchanged)
* intervention/image (^2.7.2 unchanged but upgrade exists)
* juliomotol/laravel-auth-timeout (^3.1.1 to ^4.1)
* lab404/laravel-impersonate (^1.7.5)
* laravel/framework (^8.83.27 to ^10.48.10)
* laravel/helpers (^1.7 unchanged but possibly no longer needed)
* laravel/pint (^1.15.3 added but not yet used)
* laravel/sail (^1.29.1 added but not yet used)
* laravel/sanctum (^3.3.3 added but not yet used)
* laravel/tinker (^2.9 unchanged)
* laravel/ui (^3.4.6 to 4.5.1)
* mockery/mockery (^1.6.11 unchanged)
* nunomaduro/collision (^5.11 to ^7.10)
* phpunit/phpunit (^10.5.20 unchanged)
* spatie/laravel-ignition (^1.6.4 to ^2.7)
* staudenmeir/eloquent-has-many-deep (^1.14.4 to ^1.19.3)
 
Note that <code>juliomotol/laravel-auth-timeout</code> version 4.1 requires
Laravel version 9 or greater so it can not be upgraded from 3.1.1 prior to
the Laravel 10 upgrade.


This task also updated <code>app/Http/Middleware/TrustProxies.php</code> to
This task also updated <code>app/Http/Middleware/TrustProxies.php</code> to
Line 264: Line 171:


     use JulioMotol\AuthTimeout\Middlewares\CheckAuthTimeout 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.
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.
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.


This task also explored upgrades to some other PHP library dependencies but
This task also explored upgrades to some other PHP library dependencies but

Revision as of 17:38, 21 May 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 pending changes or improvements that were made to SDS, made by Darren Duncan if by whom is not otherwise specified.

It is similar to the Historical Changes part but that it describes work which was prepared and published in a Git branch but it was deemed premature to merge it to the trunk, such as due to a desire for more testing first, or because it was possibly unfinished; in contrast, Historical is for work that was merged to trunk.

RETURN

SDS Laravel: Fixes to Broken Behavior

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

SDS Laravel: Changes to Third-Party Dependencies

2024 May 21: Upgrade Laravel from 8.x to 10.x

This task updated composer.json to require the latest PHP-8.1-compatible major version of the PHP library dependency Laravel, thus taking it from 8.x to 10.x.

To be more specific, it made these dependency changes:

  • barryvdh/laravel-debugbar (^3.7 to ^3.13.5)
  • directorytree/ldaprecord-laravel (^2.7.3 unchanged but upgrade exists)
  • etern8ty/beanstream (dev-master unchanged but upgrade exists)
  • fakerphp/faker (^1.23.1 unchanged)
  • fideloper/proxy (^4.4.2 removed as Laravel has its upgrade built-in)
  • goldspecdigital/laravel-eloquent-uuid (^8.0.1 removed as Laravel has its upgrade built-in)
  • guzzlehttp/guzzle (^7.8.1 unchanged)
  • intervention/image (^2.7.2 unchanged but upgrade exists)
  • juliomotol/laravel-auth-timeout (^3.1.1 to ^4.1)
  • lab404/laravel-impersonate (^1.7.5)
  • laravel/framework (^8.83.27 to ^10.48.11)
  • laravel/helpers (^1.7 unchanged but possibly no longer needed)
  • laravel/tinker (^2.9 unchanged)
  • laravel/ui (^3.4.6 to 4.5.2)
  • mockery/mockery (^1.6.12 unchanged)
  • nunomaduro/collision (^5.11 to ^7.10)
  • phpunit/phpunit (^10.5.20 unchanged)
  • spatie/laravel-ignition (^1.6.4 to ^2.7)
  • staudenmeir/eloquent-has-many-deep (^1.14.4 to ^1.19.3)

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.

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;

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.

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.

Compare:

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.

This task also explored upgrades to some other PHP library dependencies but they were excluded due to requiring more substantial code changes for compatibility, and will be returned to later.

RETURN