lcdservices / CiviCRM-Group-Sync

Two-way synchronize CiviCRM groups with Joomla ACL groups.
4 stars 1 forks source link

Edits to CiviCRM contact are rolled back (not saved) in various circumstances #12

Closed twomice closed 4 years ago

twomice commented 4 years ago

Some changes to a civicrm contact are refusing to save in certain circumstances when this plugin is installed.

General prerequisites:

  1. This extension must be installed and configured with one or more synched groups.
  2. The contact must be lacking membership in any of the CiviCRM groups configured for sync according to the rules at /administrator/index.php?option=com_civigroupsync&view=synchronizationrules , and then any of the following scenarios can be performed to experience the described bad behavior.

Circumstances:

  1. As described in #10: adding / removing of Groups for a contact is predictably refusing to save in civicrm under certain predictable conditions.
  2. When a contact's uf_match.uf_name value (generally the corresponding Joomla user's email address) is not an exact case-sensitive match for the contact's primary email address; then any code execution path that includes CRM_Core_BAO_UFMatch::updateUFName() (e.g., saving the Contact Edit form, use of the contact.create api, etc.) will silently fail to save the given changes.

This appears to be caused by behavior at this line in plgSystemCiviGroupSyncLCD::onUserAfterSave(). This loop calls the groupContact.delete api for each synched Joomla/CiviCRM group, if the relevant Joomla user is not a member of that group -- even if the relevant CiviCRM contact is already not a member of that group, which is a case that will generate an error in the api ("Cannot Delete GroupContact"), and which also causes a database transaction rollback, thus reverting (refusing to save) all submitted parameters for the contact, along with any other SQL queries that were executed in that database transaction.

The solution appears to be first checking that the groupContact record exists, using groupContact.get, before attempting to delete it with groupContact.delete.

PR forthcoming.

lcdservices commented 4 years ago

@twomice I've pushed up a patch that conditions deletion on existence. Please pull from master branch and test, and let me know if it's working.

twomice commented 4 years ago

Thanks @lcdservices I've got this scheduled for this week.