search

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

Other
22 stars 43 forks source link

apiv3 #214 V2.0 biggie #216

Closed artfulrobot closed 8 years ago

artfulrobot commented 8 years ago

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

deepak-srivastava commented 8 years ago

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

Kajakaran commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

Thanks. Try that.

Kajakaran commented 8 years ago

@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 commented 8 years ago

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

Kajakaran commented 8 years ago

@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 commented 8 years ago

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

Kajakaran commented 8 years ago

@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 commented 8 years ago

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

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

As I understood your question:

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

At CiviCRM:

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

@artfulrobot It works now.

Thanks

artfulrobot commented 8 years ago

:-D

deepak-srivastava commented 8 years ago

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.