[REF][PHP8.2] Declare property _amount on event confirm form #28766
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Declare property
_amount
on event confirm formBefore
Test failures on PHP 8.2, for example:
After
Property declared.
Technical Details
I'd assumed that
_amount
would be deeply interwined amongst multiple classes, but once I dug into it, that turned out not to be the case.That said, this property is fairly confusing. Elsewhere in Civi,
_amount
is afloat
(e.g. inCRM_Contribute_Form_Contribution_Confirm
). However, here it's anarray
, and there is is no overlap between these different types of_amount
.Just to further confuse things, the deprecated
testSubmit()
sets_amount
to a number, which works because_amount
is largely used as a truthy check in if statements. However,testSubmit()
is deprecated, so I don't think it matters to much.I have added a comment to the code explaining this confusion, and in time moving to getter functions probably makes sense. In the meantime this gets us closer to PHP 8.2 support.
@eileenmcnaughton This relates to this PR from a couple of days ago, so worth you taking a look at this.