fix: changes invalid toString() calls on Stringable objects, to string casts. (!126) · Merge requests · composer / laravel-api-controller · GitLab

GitLab now enforces expiry dates on tokens that originally had no set expiration date. Those tokens were given an expiration date of one year later. Please review your personal access tokens, project access tokens, and group access tokens to ensure you are aware of upcoming expirations. Administrators of GitLab can find more information on how to identify and mitigate interruption in our documentation.

Robert Sinton requested to merge fix/remove-tostring-calls-on-stringable into 5.x Mar 12, 2023

Background

In two separate files, ->toString() was being called on Stringable objects.

Stringable doesn't support this, and it was causing exceptions when the code in the AllowableFields class was triggered by Art Money unit tests.

Action Plan / Actions Taken

Replaced the two calls with a string cast.

Added tests?

Yes, of course
No, and here's why: too lazy 😄
I need help writing tests

Does this MR target the `master` branch?

No, it's targeted at a release branch.
Yes, and here's why it would be safe to deploy to production at any time: production issue that needs resolving.

Implications

Ideally the package would have a unit test which covered this code, and which would have picked up the error 👀
In the case of AllowableFields, I found that the string cast wasn't actually necessary. However I wasn't sure whether it would be necessary for the other case, so I put it in both cases to be safe.

Setup

None

How to test

Code review.
Test with the full test suite in the Art Money API project, in branch feature/upgrade-php-api-package
- Check out that branch and fin composer install.
- fin test and confirm lots of fail.
- Switch to this branch of the API package (or just copy over the changes) and confirm that the full test suite now passes again.
Try and break it!