Fix php 7.2+ deprecation error

[eileenmcnaughton]

Overview

FIxes
The each() function is deprecated. This message will be suppressed on further calls in /var/www/sln/wordpress/wp-content/plugins/civicrm/civicrm/bin/cli.class.php on line 298"
from wp cli

Before

Error on php 7.2+

After

No error

Technical Details

@kcristiano @seamuslee001 I haven't tested this - it's just off chat but I think this is the syntax for getting rid of each

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

I think that looks right to me

[kcristiano]

The patch looks sensible, but in r-run I had an issue.

php 7.3, WP 5.2.2, CiviviCRM master without the patch:

calling cli.php works fine. I did not get any errors in my php logs and as I had call -e Job -a execute I was able to confirm that the the cron jobs ran.

Command used /usr/bin/php ./bin/cli.php -e Job -a execute -s wpcv.test -u admin -p admin

After the patch was applied, I ran the same command and received a cryptic message:

The --c argument is required

Reverting the patch and cli.php runs properly.

Perhaps there is a flaw in how i am testing. Any ideas?

[demeritcowboy]

By default the error_reporting() value doesn't often include E_DEPRECATED so you won't see the message. I can reproduce it if I add E_DEPRECATED to error_reporting(). For the patch, the letter 'c' is appearing because $var[1] is the second letter in 'action':

Suggest to change foreach ($required as $vars) to foreach ($required as $var) and remove the $var = $vars[1]; line.

[seamuslee001]

@demeritcowboy i believe wordpress by default turns on E_ALL for error_reporting

[demeritcowboy]

@seamuslee001 Ok I admit I was using drupal but when I echo'd error_reporting() at the line just before the each() it said 1 (E_ERROR) only, which now that I think more about it must be set somewhere in civi because that's not my setting or drupal's. But then how did the original person see it. Hmmm.

[eileenmcnaughton]

OK doing a code step through

this the vars....

[eileenmcnaughton]

& with @demeritcowboy code

So yes Dave nailed figuring out what the crufty old code was trying to say - I will update & merge (treating myself as the review of @demeritcowboy's suggested code change)

[demeritcowboy]

@eileenmcnaughton Some extra contribution-related code seems to have snuck into the patch?

[kcristiano]

@demeritcowboy Good pickup. I'll test again and not include that

[kcristiano]

did an r-run works with and without the change to CRM/Contribute/BAO/Contribution.php likely a false commit sneaked in.

[eileenmcnaughton]

OK - removed the extra - I had created a local 'origin/master' branch & rebased over that not the remove tracking branch - merging now