Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed fatal error in PHP 8.0 #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Edzelopez
Copy link

@Edzelopez Edzelopez commented Sep 15, 2023

When submitting an ACF form containing address fields, the code seems to assume that a new contact has a previous address. Submitting the form leads to this fatal error post the PHP upgrade:

[Wed Sep 13 07:04:15.499080 2023] [php:error] [pid 1497650] [client 110.226.178.216:12564] PHP Fatal error:  Uncaught Error: Attempt to assign property "is_primary" on null in /var/www/xxx/htdocs/wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-google-map.php:787
Stack trace:
#0 /var/www/xxx/htdocs/wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-google-map.php(689): CiviCRM_Profile_Sync_ACF_CiviCRM_Google_Map->address_properties_check()
#1 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(308): CiviCRM_Profile_Sync_ACF_CiviCRM_Google_Map->address_edited()
#2 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#3 /var/www/xxx/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action()
#4 /var/www/xxx/htdocs/wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-mapper.php(2417): do_action()
#5 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(308): CiviCRM_Profile_Sync_ACF_Mapper->address_edited()
#6 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#7 /var/www/xxx/htdocs/wp-includes/plugin.php(565): WP_Hook->do_action()
#8 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/CRM/Utils/Hook/WordPress.php(108): do_action_ref_array()
#9 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/Core/CiviEventDispatcher.php(318): CRM_Utils_Hook_WordPress->invokeViaUF()
#10 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/vendor/symfony/event-dispatcher/EventDispatcher.php(251): Civi\\Core\\CiviEventDispatcher::delegateToUF()
#11 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/vendor/symfony/event-dispatcher/EventDispatcher.php(73): Symfony\\Component\\EventDispatcher\\EventDispatcher->callListeners()
#12 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/Core/CiviEventDispatcher.php(260): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispatch()
#13 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/CRM/Utils/Hook.php(366): Civi\\Core\\CiviEventDispatcher->dispatch()
#14 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/CRM/Core/BAO/Address.php(111): CRM_Utils_Hook::post()
#15 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/CRM/Core/BAO/Address.php(38): CRM_Core_BAO_Address::add()
#16 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/api/v3/Address.php(71): CRM_Core_BAO_Address::create()
#17 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_address_create()
#18 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(158): Civi\\API\\Provider\\MagicFunctionProvider->invoke()
#19 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(81): Civi\\API\\Kernel->runRequest()
#20 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/api/api.php(133): Civi\\API\\Kernel->runSafe()
#21 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/action-provider/Civi/ActionProvider/Action/Contact/ContactActionUtils.php(182): civicrm_api3()
#22 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/action-provider/Civi/ActionProvider/Action/Contact/ContactActionUtils.php(32): Civi\\ActionProvider\\Action\\Contact\\ContactActionUtils::createAddress()
#23 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/action-provider/Civi/ActionProvider/Action/Contact/CreateUpdateIndividual.php(134): Civi\\ActionProvider\\Action\\Contact\\ContactActionUtils::createAddressForContact()
#24 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/action-provider/Civi/ActionProvider/Action/AbstractAction.php(141): Civi\\ActionProvider\\Action\\Contact\\CreateUpdateIndividual->doAction()
#25 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/form-processor/Civi/FormProcessor/Runner.php(120): Civi\\ActionProvider\\Action\\AbstractAction->execute()
#26 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/form-processor/Civi/FormProcessor/API/FormProcessor.php(188): Civi\\FormProcessor\\Runner::run()
#27 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/form-processor/Civi/FormProcessor/API/FormProcessor.php(173): Civi\\FormProcessor\\API\\FormProcessor->invokeFormProcessor()
#28 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(158): Civi\\FormProcessor\\API\\FormProcessor->invoke()
#29 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(81): Civi\\API\\Kernel->runRequest()
#30 /var/www/xxx/htdocs/wp-content/plugins/civicrm/civicrm/api/api.php(133): Civi\\API\\Kernel->runSafe()
#31 /var/www/xxx/htdocs/wp-content/plugins/acfdraft/acfdraft.php(421): civicrm_api3()
#32 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(308): handle_form_submission()
#33 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#34 /var/www/xxx/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action()
#35 /var/www/xxx/htdocs/wp-content/plugins/advanced-forms-pro/core/forms/forms-submissions.php(191): do_action()
#36 /var/www/xxx/htdocs/wp-content/plugins/advanced-forms-pro/core/forms/forms-submissions.php(131): AF_Core_Forms_Submissions->process_submission()
#37 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(306): AF_Core_Forms_Submissions->pre_form()
#38 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#39 /var/www/xxx/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action()
#40 /var/www/xxx/htdocs/wp-settings.php(623): do_action()
#41 /var/www/xxx/htdocs/wp-config.php(98): require_once('...')
#42 /var/www/xxx/htdocs/wp-load.php(50): require_once('...')
#43 /var/www/xxx/htdocs/wp-blog-header.php(13): require_once('...')
#44 /var/www/xxx/htdocs/index.php(17): require('...')
#45 {main}
  thrown in /var/www/xxx/htdocs/wp-content/plugins/civicrm-wp-profile-sync/includes/acf/classes/cwps-acf-google-map.php on line 787, referer: https://xxx/grant-proposals/

@christianwach
Copy link
Owner

christianwach commented Sep 15, 2023

When submitting an ACF form containing address fields, the code seems to assume that a new contact has a previous address.

@Edzelopez Thanks for the PR, however I'm not entirely sure that it solves the problem - it only masks it. From your backtrace:

#5 /var/www/xxx/htdocs/wp-includes/class-wp-hook.php(308): CiviCRM_Profile_Sync_ACF_Mapper->address_edited()

If you look at the method in question, it bails unless the API operation is edit, which means there must be an existing address or else the API callback would be reporting an operation of create.

So it seems that the real issue is why $op = 'edit' at this point in your code and why $previous is empty. The only thing I can think of is that you are saving multiple addresses in one transaction. Is that right?

@Edzelopez
Copy link
Author

Hey @christianwach

Yes, I am using a FormProcessor to save two addresses, but they are for two different actions altogether (One creates an address for an Individual and one for an Organization). The code that apparently is causing an issue is here and it certainly does look like 'create' is passed. We've had this working for a while before we switched to PHP 8.0, when this came up. The only thing I can think of is if somehow the action is returning something here even for new contacts, which would trigger an edit instead of a create. Let me investigate and get back to you as I might have to submit a PR to action-provider then. Thanks for the direction!

@christianwach
Copy link
Owner

@Edzelopez One more thought... what is plugins/acfdraft/acfdraft.php? The backtrace seems to go via its usage of civicrm_api3() at item 31:

#31 /var/www/xxx/htdocs/wp-content/plugins/acfdraft/acfdraft.php(421): civicrm_api3()

I can't find any mention of that plugin anywhere. It would be worth sending the content of the API call to your error logs to see what's happening there

@Edzelopez
Copy link
Author

Hey @christianwach

That is a custom plugin developed by JMA. L421 just calls the FormProcessor API

Screenshot 2023-09-29 at 5 45 55 PM

@christianwach
Copy link
Owner

christianwach commented Sep 29, 2023

@Edzelopez Okay, so you'll need to dig into what that's doing behind the scenes and what's in the $submission data. I see that somewhere down the line it calls the CiviCRM API which involves the Address data:

#21 /var/www/xxx/htdocs/wp-content/uploads/civicrm/ext/action-provider/Civi/ActionProvider/Action/Contact/ContactActionUtils.php(182): civicrm_api3()

Lots of error logging in the path of the backtrace should give you a clearer idea of the sequence of calls - without that, I'm not sure how to debug what's going on.

EDIT 1: Okay, so I see that that's what you linked me to above - sorry! FYI I've never seen a FormProcessor API call before... where does that come from?

EDIT 2: Oh, I see, it's an Extension. There sure are a lot of moving parts to your issue! As I said, I think lots of error logging is needed.

@christianwach
Copy link
Owner

@Edzelopez FWIW I can see a weakness in the code in this plugin... when we consider:

public function address_pre_edit( $args ) {
// Always clear properties if set previously.
if ( isset( $this->address_pre ) ) {
unset( $this->address_pre );
}
// Grab the Address object.
$address = $args['objectRef'];
// We need a Contact ID in the edited Address.
if ( empty( $address->contact_id ) ) {
return;
}
// Grab the previous Address data from the database via API.
$this->address_pre = $this->plugin->civicrm->address->address_get_by_id( $address->id );
}
/**
* A CiviCRM Contact's Address has just been edited.
*
* @since 0.4
*
* @param array $args The array of CiviCRM params.
*/
public function address_edited( $args ) {
// Grab the Address object.
$address = $args['objectRef'];
// We need a Contact ID in the edited Address.
if ( empty( $address->contact_id ) ) {
return;
}
// Check if the edited Address has had its properties toggled.
$address = $this->address_properties_check( $address, $this->address_pre );

So, let's say an Address with a Contact ID is edited:

  • civicrm_pre fires and $this->address_pre is populated.
  • Now let's say something hooks into civicrm_pre after this plugin does and edits an Address without a Contact ID.
  • civicrm_pre fires again and $this->address_pre is unset.
  • civicrm_post fires and bails because there's no Contact ID.
  • civicrm_post fires on the "outer" Address edit and $this->address_pre is not populated...
  • The error you encountered could happen.

Unfortunately, unlike some hooks, civicrm_pre and civicrm_post don't contain anything that identifies the transaction, so nested edits cannot be distinguished. Avoiding this might mean transitioning this plugin to make use of Symfony hooks and callbacks... which would be a huge task... so all I can suggest at this stage is that (if the above is what is happening in your system) you try and avoid nested Address edits.

@christianwach
Copy link
Owner

@Edzelopez I don't know if you're still hitting this problem, but perhaps e6c6092 goes some way to solving it? Switching to Symfony hooks is too great a task for me without someone helping out, but this might just do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants