Skip to content

fix: changes invalid toString() calls on Stringable objects, to string casts.

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

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!

Merge request reports