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

[NFC] CRM-21739 Test improvements for mailing getRecipients #11642

Merged
merged 3 commits into from
Feb 27, 2018

Conversation

JKingsnorth
Copy link
Contributor

@JKingsnorth JKingsnorth commented Feb 6, 2018

Add test for getRecipients method to ensure that mailing groups included/excluded from email mailings are included in the recipient list for a mailing.

@seamuslee001
Copy link
Contributor

Jenkins Re test this please

@eileenmcnaughton
Copy link
Contributor

@JKingsnorth it's passing is it still wip?

@JKingsnorth
Copy link
Contributor Author

Yes indeed, I have a whole suite of additional tests to add.

@JKingsnorth JKingsnorth changed the title [WIP] CRM-21739 Tests for CRM_Mailing_BAO_Mailing::getRecipients CRM-21739 Test improvements for mailing getRecipients Feb 27, 2018
@JKingsnorth
Copy link
Contributor Author

@eileenmcnaughton @seamuslee001 @monishdeb I've done all I can on tests for this for now. Better than nothing I hope! This should help prevent future regressions in the functionality to include/exclude mailing groups in email mailings.

There are still some significant gaps in the tests, but this is a starting point at least.

Tests are passing locally, so if someone could review the test logic then it would be great to get this merged.

@JKingsnorth JKingsnorth changed the title CRM-21739 Test improvements for mailing getRecipients [NFC] CRM-21739 Test improvements for mailing getRecipients Feb 27, 2018
@monishdeb
Copy link
Member

Great work @JKingsnorth :) Will test and merge this PR shortly 👍

@monishdeb
Copy link
Member

Reviewed the code and it covers the scenario well. Also the test build passes now, hence merging.

@monishdeb monishdeb merged commit ea0a911 into civicrm:master Feb 27, 2018
@mlutfy mlutfy added this to the 4.7.32 milestone Mar 6, 2018
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.

6 participants