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

[REF] Don't add Contacts to Groups they are already Added to on Edit #26519

Conversation

larssandergreen
Copy link
Contributor

@larssandergreen larssandergreen commented Jun 13, 2023

Overview

If you edit a Contact (contact/add?action=update), on saving, the form calls CRM_Contact_BAO_Contact::create with params['groups'] including all the groups the contact was in before editing and wasn't removed from, which calls CRM_Contact_BAO_GroupContact::addContactsToGroup for each group, which provides the pre and post hooks and calls CRM_Contact_BAO_GroupContact::bulkAddContactsToGroup which finally checks the table and says, oh wait, that Contact is already in that Group, there's nothing to do here. Rinse and repeat for each Group.

Before

Attempt to add Contact to all Groups submitted in the edit form.

After

Only attempt to add Contact to Groups that it was not already added to. On my local, this saves a few tenths of a second with a handful of groups.

This will also stop erroneous GroupContact hook triggers, but that's a more general problem that also needs to be solved in GroupContact.

Technical Details

We don't technically need to check $params['group'][$key['group_id']] == 1 here, but it seems safer and more readable to do so.

@civibot
Copy link

civibot bot commented Jun 13, 2023

(Standard links)

@civibot civibot bot added the master label Jun 13, 2023
@larssandergreen larssandergreen changed the title Don't add Contacts to Groups they are already Added to on Edit [REF] Don't add Contacts to Groups they are already Added to on Edit Jun 13, 2023
@colemanw
Copy link
Member

That makes sense to me. Testing this...

@colemanw
Copy link
Member

r-run turned up no problems. Thanks for this @larssandergreen

@colemanw colemanw merged commit 99908f7 into civicrm:master Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants