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] Export cleanup - filter at point of query on postal exports #13216

Merged
merged 1 commit into from
Dec 15, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Follow on from https://github.com/civicrm/civicrm-core/pull/13213/files
This culls out the non-postal rows at the query rather than deleting them at the end

Before

Query fetches rows that can't be emailed and THEN they are culled

After

Where filters added to query

Technical Details

Comments

@civibot
Copy link

civibot bot commented Dec 3, 2018

(Standard links)

@mattwire
Copy link
Contributor

See mattwire@4b8a3a2 for some tweaks to fix issues in #13213 and get merge household working

'address_options', TRUE, NULL, TRUE
);
if (!empty($addressOptions['supplemental_address_1'])) {
$addressWhere .= " AND civicrm_address.supplemental_address_1 <> ''";
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton I think this should be OR not AND!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you are right - if street address is not empty or supplemental address is not empty then we have an address we can email - updated

@mattwire
Copy link
Contributor

@eileenmcnaughton This is tested and working now on a live site. Ok to merge.

I did have to make one change to set the table type to MYISAM instead of INNODB here https://github.com/eileenmcnaughton/civicrm-core/blob/30eab4c17d15d688acc039a5974a695386271a9e/CRM/Export/BAO/Export.php#L713 as I get row size to large otherwise. But that's a long-standing issue before/after this PR :-) It does, however, allow me to test and verify that all three contact export modes with/without postal selected now work correctly! Well done!

@eileenmcnaughton eileenmcnaughton merged commit 7bdfc94 into civicrm:master Dec 15, 2018
@eileenmcnaughton eileenmcnaughton deleted the export_postal branch December 15, 2018 02:07
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