search

veda-consulting-company / uk.co.vedaconsulting.mailchimp

Other
22 stars 43 forks source link

Closes #188 - Reduced no of API calls in sync Mailchimp to CiviCRM. #189

Closed johanv closed 8 years ago

johanv commented 8 years ago

This patch

In our particular case, synching using the web interface still does not work (timeouts, I guess), but you can sync using the api and drush as follows:

drush cvapi Mailchimp.sync

Now it is probably wise that someone reviews this patch before merging :-)

veda-consulting commented 8 years ago

Thanks Johan - We'll review and come back to you. Having a quick look I can see a couple of bits that may do with a tweak i.e. checking for duplicate emails should really be checking if the email belongs to a different contact as many times we find the same contact has the same email address more than once with differing types such as billing and main. Checking on the email id would mean you would think there is more than one when but the reality is there isn't more than one contact its attached to. So maybe a few more tweaks but we'll review and feedback as necessary. Thanks for taking the time to PR.

On 25 February 2016 at 16:22, Johan Vervloet notifications@github.com wrote:

This patch

  • detects the easy to match mailchimp contacts (whose e-mail address is unique in CiviCRM) using one query.
  • does not call the Contact.Create API for those detected contacts if the existing name in CiviCRM matches the name in Mailchimp.

In our particular case, synching using the web interface still does not work (timeouts, I guess), but you can sync using the api and drush as follows:

drush cvapi Mailchimp.sync

Now it is probably wise that someone reviews this patch before merging :-)

You can view, comment on, or merge this pull request online at:

https://github.com/veda-consulting/uk.co.vedaconsulting.mailchimp/pull/189 Commit Summary

  • Closes #188 - Reduced no of API calls in sync Mailchimp to CiviCRM.

File Changes

Patch Links:

- https://github.com/veda-consulting/uk.co.vedaconsulting.mailchimp/pull/189.patch

https://github.com/veda-consulting/uk.co.vedaconsulting.mailchimp/pull/189.diff

— Reply to this email directly or view it on GitHub https://github.com/veda-consulting/uk.co.vedaconsulting.mailchimp/pull/189 .

johanv commented 8 years ago

I still had problems on our live server, but the above commit (96a88e4) improves things a lot. I found out that hook_civicrm_pre retrieves an e-mail address at every Email API call, and this is not always needed. In our particular case, we had to create 1000+ new e-mail addresses at the initial sync, and I guess those 1000+ lookups in our e-mail table containing 160000+ addresses, caused the memory problems.

deepak-srivastava commented 8 years ago

@johanv many thanks. Have merged your PR.

Only problem we encountered while testing was a sql error " error 'Illegal mix of collations (utf8_general_ci,IMPLICIT) and (utf8_unicod...")'" - which we 'll be doing another commit to fix column char-set. Rest looked good.

Thanks.

johanv commented 8 years ago

No worries. Thanks for merging this :-)