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

apiv3 #214 V2.0 biggie #216

Merged
merged 6 commits into from
Jun 21, 2016

Conversation

artfulrobot
Copy link
Contributor

Here's a rather big patch. It's called v2 because it's in many ways a rewrite and introduces forward-only changes.

It comes with lots of documentation (updated README.md, new README-tech.md, and lots in code) and lots of tests (71 tests, 622 assertions).

It began life as a way to move to Mailchimp's API v3.0 which is going to be the only way to talk to Mailchimp from the end of 2016 on.

I'm getting good results in my real-world use case (a simple case with ~5,000 subscribers) but it could do with wider testing.

I'm really keen to get test cases written for any problems and to check all existing tests still pass before changes are accepted

There are a couple of issues outstanding to my mind: Mailchimp's re-subscribe policy (see README.md) and currently we cannot check for errors on the bulk updates. The latter is because Mailchimp's new batch API only offers feedback in a gzipped tar file and then only in a particular format that PHP's PharData class is unable to open...

Also I think it fixes

  • #213.
  • #194 use bulk mail - does this now.
  • #201 my bug about timeouts.
  • #203 failed when same email on one contact lots of times. This is fixed by better mailchimp member to civi contact matching.
  • #182 export API no longer used.
  • #181 duplicate contacts - now see tech readme and code notes.
  • #160 readme updates. - clarify MC policy, etc.
  • #171 scheduled job only MC precedence. Fixed. (also note on that bug is not a fix)
  • #139 sort of fixed - no notifications at all now!
  • #129 export API bug fix (doesn't use it!)

Possibly fixes

  • #210 - hanging: please try this version (in a sandbox and after backing up your mailchimp lists). I had a hang with the old version and it was to do with a list that no longer existed at Mailchimp.
  • #134 matching is different now, might make this possible. (worth a try in a sandbox and after backing up your mailchimp lists) e.g. if contact A and B have same primary email, but CiviCRM knows one of them is in the sync-ed group, then syncs will always update that contact and leave the other alone.
  • #165 I think this is fixed - because the scheduled sync always did pull, push before. Now you can choose. Pull will always 'remove from group' in Civi if member not subscribed at MC.
  • #145 parent groups - creating interest group giving error. Might be worth re-trying in sandbox after backup.

@deepak-srivastava
Copy link
Collaborator

deepak-srivastava commented Jun 13, 2016

@artfulrobot thanks. We 'll run some manual tests and come back with questions or feedback.

@Kajakaran
Copy link
Contributor

@artfulrobot

I am trying the basic example with one group with contacts in CiviCRM and empty mailchimp list and CiviCRM to Mailchimp Sync.

I could not get sync button / Dry-Run. Please help on this

Please see screenshot
mailchimp_sync

@artfulrobot
Copy link
Contributor Author

artfulrobot commented Jun 16, 2016

Initial checks:

  1. I've been using 4.6 LTS - what are you on? (If you're not on LTS, suggest you start with running the tests from the command line first.)
  2. When you visit the push page it checks all lists (referenced from CiviCRM groups) at Mailchimp exist. If they don't it shows what you see. So you could validly get that error if:
    1. you deleted the list at Mailchimp since mapping it to the group
    2. you changed your API key to point to a different account (I kept making that mistake while swapping between my client and my own test MC accounts!) and therefore the list does not exist.

I will try on a fresh installation of 4.6 to see if I can replicate the problem you're having.

@artfulrobot
Copy link
Contributor Author

artfulrobot commented Jun 16, 2016

I've squished a few bugs based on running it with a fresh 4.6.18 install. But I still can't reproduce the behaviour you're seeing.

I've also installed a fresh 4.7.8 install and that seems to be running fine.

@Kajakaran
Copy link
Contributor

@artfulrobot
i found the problem with api call -> https://us8.api.mailchimp.com/3.0/lists?fields=lists.id%2Clists.name

The above returns only 10 results. I have 12 in my case, might be checking pager information

Thanks
Kajan

@artfulrobot
Copy link
Contributor Author

Thanks. Try that.

@Kajakaran
Copy link
Contributor

@artfulrobot
It works now.
If I have invalid email in civicrm group, it fails in the middle of queue.

[Error: List 0 Newsletter: Completed additions/updates/unsubscribes.]
Mailchimp API said: Invalid Resource

Thanks
Kajan

@artfulrobot
Copy link
Contributor Author

OK, how did you enter an invalid email into CiviCRM? I'll build a test.

@Kajakaran
Copy link
Contributor

@artfulrobot
In my install of civi has already a contact with email 'sample2000@example.com'. This contact is also in newsletter group. In the log i got following error message.

Response Body: stdClass::__set_state(array(
'type' => 'http://developer.mailchimp.com/documentation/mailchimp/guides/error-glossary/',
'title' => 'Invalid Resource',
'status' => 400,
'detail' => 'sample2000@example.com looks fake or invalid, please enter a real email address.',
'instance' => '',
))

@artfulrobot
Copy link
Contributor Author

Oh ok, so we need to protect against Mailchimp failures on small updates. Will write test.

@Kajakaran
Copy link
Contributor

Kajakaran commented Jun 16, 2016

@artfulrobot

(1) I have another issue. Please see below.

I have a list with one group under list in mailchimp
I have added a subscriber to list and not to the group in mailchimp under above list
This subscriber is added to my civicrm membership group - This is ok

However I updated newly added subscriber to group under list in mailchimp, I expected this subscriber added to my civicrm child group under membership group. It did not happen

Then I did civi to mailchimp sync, this sync removed newly added subscriber from group under list in mailchimp - This is not expected.

But other way it works fine.

I have a contact in membership group and child group in CiviCRM and removed this contact from child group. This action removed from group under list in mailchimp.

When I rejoin to this child group back, also add to group under list in mailchimp.

(2)
I have a contact in both membership and child groups in civicrm and list and group in mailchimp.
I unsubscribed this contact in mailchimp and expected to remove from membership and child group in civi. However it removed only from membership group.

Thanks
Kajan

@artfulrobot
Copy link
Contributor Author

artfulrobot commented Jun 16, 2016

@Kajakaran Can you clarify, I'm not quite sure I follow.

Nb. it might be helpful if we can agree to use these terms:

  • Contact always refers to a CiviCRM contact
  • List - always refers to a Mailchimp List.
  • Interest - refers to a specific Mailchimp "Interest" in a specific Mailchimp Interest Category (aka Mailchimp Group).
  • Group - always refers to a CiviCRM group
  • Membership Group - refers to a CiviCRM group that is configured to map to the List in a membership way.
  • Interest Group - refers to a CiviCRM group that is configured to map to an Interest in Mailchimp.

As I understood your question:

At Mailchimp: List 1 has Interest A in a Mailchimp Group/Interest Category

At CiviCRM:

  • Group 1 is a Membership Group.
  • Group 2 is an Interest Group. And is a child group of the Membership Group?

However I updated newly added subscriber to group under list in mailchimp, I expected this subscriber added to my civicrm child group under membership group. It did not happen

I would expect the webhook to make that happen. Note that webhooks are not necessarily fired immediately, and that on a profile update there is an additional 10s delay on profile updates.

I have not tested an interest group as a child group of the membership group - trying that now - but certainly if the Interest Group is just a regular group (no parents) then the webhook deals with this case and adds the Contact to the Interest Group.

Then I did civi to mailchimp sync, this sync removed newly added subscriber from group under list in mailchimp - This is not expected.

That is expected because of your result in the first part of the test: A CiviCRM to Mailchimp sync assumes CiviCRM is correct. As CiviCRM does not think that the Contact is in the Interest Group (you said as much above), then it makes sure that Interest is unset at Mailchimp.

So the question is your first one: what happens with Interest Group as a child of Membership Group? I'm trying to re-create that now.

It's v frustrating that the Mailchimp web interface lags updates made by the API significantly. I'm having to wait about 5 minutes before changes made (and verifiable) by the API are shown online. Refresh Page...Wait...Refresh Page...Wait....

@artfulrobot
Copy link
Contributor Author

Actually this might be a Mailchimp bug. (I've found a few).

I've just updated the interest at mailchimp.com and the profile webhook has not been called. (not that my code is failing; but that the webhook request is not being made at all - nothing in web server logs!)

I had this problem with the Subscribe button on the mailchimp website. Eventually after a long time with support they confirmed it was a bug their end. I'll ask them.

@Kajakaran
Copy link
Contributor

@artfulrobot
Yes your assumption regarding wording is correct.

"Group 2 is an Interest Group. And is a child group of the Membership Group?"
It is not child group, regular group.

Thanks
Kajan

@artfulrobot
Copy link
Contributor Author

Yeah, then that works for me. (It also seems to work for child groups.)

Coming back after lunch I can see that Mailchimp's webhooks have finally fired! All of them at once! About 20 mins after I made changes on Mailchimp.com

So if you could repeat your test: Change the Interest at Mailchimp, then wait on Mailchimp to bother sending the webhook to your server, then check your CiviCRM handled it right - I think it should work.

Do use the lastest updated version though (i.e. from the current HEAD of my master branch which includes the commits above.).

@vishram74
Copy link

Does this patch fix the error Invalid MailChimp List ID: e33355b9f6 that I get when I try to access the Mailchimp settings from Civicrm?

@artfulrobot
Copy link
Contributor Author

@vishram74 by "this patch" do you mean this whole Pull Request or a specific commit?

This PR is for testers only at the mo. You can try it but it introduces forward-only changes (so you can't downgrade to the current version without some technical work and reconfiguring all your lists).

The settings page checks have been rewritten, so you should get more meaningful errors, like "the list that this group linked to no longer exists at Mailchimp", if that's the case, but without more details (and here's probably not the place for them) I couldnt's say more. If you are in a position (i.e. you are a developer, know about backups...) to test the master branch of my fork (i.e. this PR) then please do report any specific problems here, as Kajakaran has been and I'll try to help.

@vishram74
Copy link

This issue Im getting is when I try and access the page https://fremantlechamber.com.au/civicrm/mailchimp/settings?reset=1

Im getting an error shown below
mailchimp_error

Im not a developer.. Just configure civicrm for our clients so Im probably not he best person to test this.

Thanks anyway for your help

@artfulrobot
Copy link
Contributor Author

You could open an issue here. Whether it gets a fix on the current branch will depend on whether this PR gets accepted. Hope so after the more than 100 hours work that's been put in, but more testing is needed by the devs before that can happen.

@Kajakaran
Copy link
Contributor

@artfulrobot
My contact did not get to Interest Group when new subscriber was updated to Interest

Please see civi logs

Jun 16 14:08:59 [info] End-CRM_Mailchimp_Utils getGroupsToSync $groups
array (
145 =>
array (
'list_id' => 'a51d9b170b',
'list_name' => 'One More Test with no limit',
'category_id' => '',
'category_name' => NULL,
'interest_id' => '',
'interest_name' => NULL,
'is_mc_update_grouping' => NULL,
'civigroup_title' => 'no limit list membership',
'civigroup_uses_cache' => false,
'grouping_id' => '',
'grouping_name' => NULL,
'group_id' => '',
'group_name' => NULL,
),
146 =>
array (
'list_id' => 'a51d9b170b',
'list_name' => 'One More Test with no limit',
'category_id' => '336331efa0',
'category_name' => 'Grouping',
'interest_id' => 'de941e7f6f',
'interest_name' => 'Group 1',
'is_mc_update_grouping' => NULL,
'civigroup_title' => 'No Limit Group 1',
'civigroup_uses_cache' => false,
'grouping_id' => '336331efa0',
'grouping_name' => 'Grouping',
'group_id' => 'de941e7f6f',
'group_name' => 'Group 1',
),
)

Jun 16 14:08:59 [info] Webhook: profile with request data: {"type":"profile","fired_at":"2016-06-16 14:05:38","data":{"id":"ea3bc030e4","email":"kajanonemoregopi@vedaconsulting.co.uk","email_type":"html","ip_opt":"185.53.226.156","web_id":"285503445","merges":{"EMAIL":"kajanonemoregopi@vedaconsulting.co.uk","FNAME":"kajanonemoregopi","LNAME":"kajanonemoregopiln","INTERESTS":"Group 1","GROUPINGS":[{"id":"12001","name":"Grouping","groups":"Group 1"}]},"list_id":"a51d9b170b"}}

Jun 16 14:09:10 [info] Webhook response code 200 (200 = ok)

@artfulrobot
Copy link
Contributor Author

@Kajakaran Looks like you have configured your CiviCRM group to disallow updates from Mailchimp: see in your logs: is_mc_update_grouping' => NULL, (that should be 1).

Check the settings in Civi for that group. You want the option "Subscribers are able to update this grouping using Mailchimp".

@Kajakaran
Copy link
Contributor

@artfulrobot
It works now.

Thanks

@artfulrobot
Copy link
Contributor Author

:-D

@deepak-srivastava deepak-srivastava merged commit 922edef into veda-consulting-company:master Jun 21, 2016
@deepak-srivastava
Copy link
Collaborator

Have merged in. Any API v2 or new work would continue to go in master, while any critical fixes for v1 should go to API-v1 branch.

Many Thanks @artfulrobot.

@Kajakaran Kajakaran mentioned this pull request Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants