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

dev/core#448 Fix issue where when building mailings with smart groups, removed members of the smart group were being included #12962

Merged
merged 1 commit into from
Oct 19, 2018

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Oct 18, 2018

Overview

When using a smart group as a mailing list, users who unsubscribe from the smart group are still included in the mailing.

Before

When creating a mailing using a smart group as a mailing list, users who have previously unsubscribed from the smart group mailing list will still receive the mailing.

No Unit Test on this particular part of the recipient building logic

After

Users who have unsubscribed form smart group mailing lists and are marked as removed in the civicrm_group_contact table, no longer receive the mailings.

Unit test exists

Technical Details

In CRM/Mailing/BAO/Mailing.php the query below doesn't join to the civicrm_group_contact to check if the recipient has been removed.

if (count($includeSmartGroupIDs)) {
      $query = CRM_Utils_SQL_Select::from($contact)
        ->select("$contact.id as contact_id, $entityTable.id as $entityColumn")
        ->join($entityTable, " INNER JOIN $entityTable ON $entityTable.contact_id = $contact.id ")
        ->join('gc', " INNER JOIN civicrm_group_contact_cache gc ON $contact.id = gc.contact_id ")
        ->join('mg', " INNER JOIN civicrm_mailing_group mg  ON  gc.group_id = mg.entity_id AND mg.search_id IS NULL ")
        ->join('temp', " LEFT JOIN $excludeTempTablename temp ON $contact.id = temp.contact_id ")
        ->where('gc.group_id IN (#groups)')
        ->merge($criteria)
        ->replaceInto($includedTempTablename, array('contact_id', $entityColumn))
        ->param('#groups', $includeSmartGroupIDs)
        ->param('#mailingID', $mailingID)
        ->execute();
    }

Comments

This PR replaces #12943 which was against the wrong branch & had no test but had the correct fix, which has been cherry-picked in.

This appears to be a semi-recent regression - having seemingly worked in 5.2

Already merged to master #12945 so some git foo may be needed

… account

dev/core#448 Add in a Unit test to demonstrate the problem with smart groups and contats removed from smart groups not being properly checked
@civibot
Copy link

civibot bot commented Oct 18, 2018

(Standard links)

@civibot civibot bot added the 5.7 label Oct 18, 2018
@eileenmcnaughton eileenmcnaughton changed the title Port of PR #12945 for 5.7 dev/core#448 Fix issue where when building mailings with smart groups, removed members of the smart group were being included Oct 18, 2018
@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@totten totten merged commit 0d2229b into civicrm:5.7 Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants