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.

Sign up for GitHub

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 #12945

Merged
merged 2 commits into from
Oct 17, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc44d94
dev/core#448 Add in a Unit test to demonstrate the problem with smart…
seamuslee001 Oct 16, 2018
b14b420
smart groups as mailing lists were not taking unsubscribed users into…
Oct 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CRM/Mailing/BAO/Mailing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,11 @@ public static function getRecipients($mailingID) {
->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('gcr', " LEFT JOIN civicrm_group_contact gcr ON gc.group_id = gcr.group_id AND gc.contact_id = gcr.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)')
->where('gcr.status IS NULL OR gcr.status != "Removed"')
->merge($criteria)
->replaceInto($includedTempTablename, array('contact_id', $entityColumn))
->param('#groups', $includeSmartGroupIDs)
Expand Down
15 changes: 15 additions & 0 deletions tests/phpunit/CRM/Mailing/BAO/MailingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,21 @@ public function testgetRecipientsEmailGroupIncludeExclude() {
unset($expected[0], $expected[4], $expected[8]);
$this->assertRecipientsCorrect($mailing['id'], $expected);

// Tear down: delete mailing, groups, contacts
$this->deleteMailing($mailing['id']);

// Create a New mailing, Testing contacts removed from smart group.
// In this case groupIDs6 will only pick up contacts[0] amd contacts[8] with it's
// criteria. However we are deliberly going to remove contactIds[8] from the group
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seamuslee001 I think you mean "deliberately" - not "deliberly"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or he was slurring....

// Which should mean the mainling only finds 1 contact that is contactIds[0]
$mailing = $this->callAPISuccess('Mailing', 'create', array());
$this->callAPISuccess('GroupContact', 'Create', array(
'group_id' => $groupIDs[6],
'contact_id' => $contactIDs[8],
'status' => 'Removed',
));
$this->createMailingGroup($mailing['id'], $groupIDs[6]);
$this->assertRecipientsCorrect($mailing['id'], [$contactIDs[0]]);
// Tear down: delete mailing, groups, contacts
$this->deleteMailing($mailing['id']);
foreach ($groupIDs as $groupID) {
Expand Down