Updating grouping for existing or new subscriber

[jbroberg]

Hi there,

Thanks so much for this library - it is very useful. It would be great if you could include an example (e.g. in the unit tests) of setting or updating the grouping information on a subscriber, for instance when adding a new subscriber via listSubscribe using GROUPINGS in merge_vars.

cheers

James

[basiliscus]

Hi James,

Thank you for the feedback.

I've added an example as you requested. See testSubscribe() method in https://github.com/Ecwid/ecwid-mailchimp/blob/master/src/test/java/com/ecwid/mailchimp/method/list/InterestGroupingMethodsTest.java

Thanks

Vasily

[basiliscus]

By the way, I was using your maven-gae-plugin a year ago :) Nice job, thanks.

[jbroberg]

No problem and thanks for the addition. I'm just one of many contributors to the gae plugin :)

Speaking of GAE however - that is what I was hoping to use your sdk on. However due to the HttpClient dependencies it is not working (as HttpClient uses sockets which are not white listed on GAE). I suppose one solution I could persue would be to swap out the HttpClient code dependency for URLFetch - we have done this in the past for other libs.

[jbroberg]

Had a further think about it. I was going to rewrite MailChimpClient to use GAE URLFetch, but that would add another dependency to the gae sdk. What I ended up doing was replacing HttpClient with standard java.net.* routines, which removes the HttpClient dependency and on GAE platforms is implemented transparently using the URL Fetch service.

It only involved changing MailChimpClient.java - I will check it into my fork soon and you can integrate it if you like.

[basiliscus]

The main reason I chose HttpClient is that it effectively caches connections. That is important for our main project (Ecwid). I'm afraid your implementation won't be as effective as HttpClient, will it?

Maybe you want me to make method "execute(String url, String request)" protected, so that it can simply be overriden in the GAE-specific implementation?

[jbroberg]

Yep - it won't be as efficient, but I don't really have a choice if I want to run it on GAE. Also, I suspect my scaling requirements are a fair bit lower than yours (I'm just dealing with my own customers/subscribers).

I have checked in my rewrite of the execute method in my fork. It's called MailChimpGAEClient.java - it's a bit kludgy - I think your protected method idea is better.

[jbroberg]

Is this what you had in mind? https://github.com/jbroberg/ecwid-mailchimp/commit/88c1d9367320ab663911f99ce7f5d076a75f731f

cheers

james

[basiliscus]

Yes, that is exactly what I had in mind.

However, I've come to something which I think is a better solution: https://github.com/Ecwid/ecwid-mailchimp/commit/d2befa3ddf784dc87d249aab77ad433caf463e29

Just supply your implementation of MailChimpAPIConnection to MailChimpClient's constructor.

[basiliscus]

James,

Again, I've reconsidered the desing: https://github.com/Ecwid/ecwid-mailchimp/commit/addbdaf48b355c4a76c272edd88c6e8966b2a7ef

What we should do now to support GAE is as follows:

1. Create an implementation of https://github.com/Ecwid/ecwid-mailchimp/blob/master/src/main/java/com/ecwid/mailchimp/connection/MailChimpConnectionManager.java (which should be called something like PlainURLConnectionManager).
2. Add a note to MailChimpClient's constructor to use that implementation in GAE environment.

Thanks

[jbroberg]

Sounds good - I will try the above and you can cherry pick it if you like it. If all works well it would be great to push a new version to maven central also - I'd rather not maintain a parallel version of the artefacts. I'll merge again and keep you posted.

[jbroberg]

Hi Vasily,

Here is the commit: https://github.com/jbroberg/ecwid-mailchimp/commit/17baaee5711fb35998c7e2a4c956b9fed9c65e14

Could you check one final thing before you push again to mvn central? I notice that when I use the ListSubscribeMethod twice in a row for the same subscriber, but change the groupings, new groups are added but omitted groups are not removed, despite setting replace_interests = true

Can you replicate this issue?

[basiliscus]

Thank you for the contribution, I've cherry-picked your commit. Made some corrections though to fix some minor issues (such as extra '\n' at the end of the response). Also, I've changed the class name I originally proposed :)

Strangely, my integration tests run slightly faster with your implementation comparing to the default one. Maybe I should consider getting rid of HttpClient dependency.

As for the issue with groupings, I'll have a look at it soon, thanks for reporting.

[basiliscus]

James,

I have doubts regarding the following line in your code:

• conn.addRequestProperty("Content-Type", "application/" + "POST");

I've never heard about application/POST mime type, so I supposed that it was a typo and changed it. Please let me know if actually you set it intentionally.

[basiliscus]

Can you replicate this issue?

Yes, I've replicated this. It seems that the API docs are simply misleading. Have a look at the following thread: https://groups.google.com/forum/#!msg/mailchimp-api-discuss/5y-Td4k-39k/kq18CiNBHkQJ

Experimenting, I've found out that replace_interests=true affects merge_vars.GROUPINGS.groups values, not merge_vars.GROUPINGS as one may expect.

[jbroberg]

Hi Vasily,

Regarding the "application/POST" type - I think I was initially confused as to how the parameters were passed/serialised. If it works without it, leave it out.

Ok - so workaround for "removing" a grouping according to that thread is to assign it an empty or null value. If that works I'm happy!

[jbroberg]

Hi Vasily,

I can confirm setting an empty merge_vars.GROUPINGS.groups value achieves the necessary function I was after (removing them from the group).

Do you think you could cut a new release and send it to maven central?

cheers!

James

[basiliscus]

Hi James,

The new release had already been sent: 1.3.0.2. Again, thank you for the feedback. You are the first person who've shown the interest :)