christianwach / civicrm-wp-profile-sync

Keeps a WordPress User profile in sync with a CiviCRM Contact and integrates WordPress and CiviCRM Entities with data synced via Advanced Custom Fields.
https://wordpress.org/plugins/civicrm-wp-profile-sync/
GNU General Public License v2.0
13 stars 17 forks source link

PHP 8 Deprecation Notices #38

Closed rbaugh closed 2 years ago

rbaugh commented 2 years ago

After switching over to PHP 8.0, I noticed the following notices:

Required parameter $dedupe_rule_id follows optional parameter $contact_type in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-contact.php on line 679
Required parameter $type follows optional parameter $value in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-acf-field.php on line 571
Required parameter $settings follows optional parameter $value in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-acf-field.php on line 571
Required parameter $settings follows optional parameter $value in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-acf-field.php on line 699
Required parameter $settings follows optional parameter $value in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-acf-field.php on line 720
Required parameter $settings follows optional parameter $value in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-acf-field.php on line 741
Required parameter $data follows optional parameter $action in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/acfe/form-actions/cwps-acf-acfe-form-action-base.php on line 117
Required parameter $data follows optional parameter $action in /wp-content/plugins/civicrm-wp-profile-sync/includes/acf/acfe/form-actions/cwps-acf-acfe-form-action-base.php on line 194

I was looking to just add on a default value to make these optional, but then thought the code may need some checks to verify that the expected variable has data you expect. So checking an array property exists before checking against it for a value.

christianwach commented 2 years ago

Thanks @rbaugh fixed in 44f9e4e3d0102f5813f9513d4bd1a92f54f64613

christianwach commented 2 years ago

I was looking to just add on a default value to make these optional, but then thought the code may need some checks to verify that the expected variable has data you expect. So checking an array property exists before checking against it for a value.

@rbaugh I think those methods should be okay as-is because of the ACF Field Types that trigger them being called. I think the "optional first" params were probably an artefact of early development. Still, let me know if you find anything in your logs that shouldn't be there are we can work backwards to the cause.

rbaugh commented 2 years ago

@christianwach I wasn't sure if there was a reason for the defaults and was just looking for least amount of work needed to remedy the issue. I will let you know if I see anything in the logs. Thanks for the quick fix.