Improve user sync - Githubissues

DynamoMTL / spree_chimpy

Spree/MailChimp Integration

BSD 3-Clause "New" or "Revised" License

35 stars 123 forks source link

Improve user sync #87

Closed jormon closed 5 years ago

jormon commented 7 years ago

This PR attempts to improve the user performace from Mailchimp. The diff covers the following improvements:

Speed: instead of iterating through Spree users one by one and looking them up in mailchimp, prefer to pull all subscription data in a big batch from Mailchimp, and then update local db as needed.
User-friendliness: while I preserved exact functionality, I renamed/scoped the task to more clearly reflect the directionality of this sync. In particular, based on comments and lack of documentation, it was unclear what this task was actually doing. Now we specify explicitly in the README and code that we're pulling subscription status from Mailchimp.

jormon commented 7 years ago

Viewing changes in split mode is a little easier on the eyes because of where git chose to split things...

https://github.com/DynamoMTL/spree_chimpy/pull/87/files?diff=split

jormon commented 5 years ago

@braidn any thoughts on getting this pulled in?

braidn commented 5 years ago

Absolutely! I will take a look at it tomorrow

braidn commented 5 years ago

Tomorrow turned into forever. Sorry @jormon. Will take a peek at this tomorrow (Tuesday the 4th)

jormon commented 5 years ago

no worries

braidn commented 5 years ago

@jormon are you using the master branch here or does this need to be added to a specific tagged branch?

jormon commented 5 years ago

I was using 2-4 stable, but I don't think this touches any files that differ between that and master.

braidn commented 5 years ago

So all good to merge?

jormon commented 5 years ago

yes, i think so. how do you manage backporting to other branches? (apologies if this is what you were asking before, if I want this in 2-4-stable as well, should I submit a separate PR to that branch too?)

braidn commented 5 years ago

Aw, yes! That's what I was wondering @jormon . I or you can create a pull request with the cherry-pick of this squashed pull request. Merging now, hit me up if you want me to backport it into 2-4.

jormon commented 5 years ago

It would be awesome if you could backport this to 2-4 as well. I hope to be off it soon, but am having a hard time deciding between Spree and Solidus as the upward migration path.

braidn commented 5 years ago

Sweet! Putting this on my list. Should get it in soon.