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

Mailchimp contacts for not registered users #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SergeyMell
Copy link
Contributor

Implemented creating mailchimp contacts for not registered users (Abandoned carts list)

@mjuneja mjuneja requested a review from vinay-mittal February 15, 2019 15:03
@vinay-mittal
Copy link
Member

@SergeyMell The change you have done as far as i am understanding here is If a guest user comes and adds products without logging in and signing up. Then you want to include it in Abandoned List, but there is a case when user just adds the item and does not proceed to checkout further, in that case we don't even have its email and according to the change you have done it will be invalid as well.

@@ -6,16 +6,14 @@ class AbandonedCart < Spree::Marketing::List
NAME_TEXT = 'Abandoned Cart'
AVAILABLE_REPORTS = [:purchases_by].freeze

def user_ids
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SergeyMell You can't just replace user_ids method. It will simply raise exception as mentioned here .

If you want to change this list type. You will have need to change the whole flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only place where user_ids is used is method users. As long as I've changed this method it won't break anything. The only thing I should change is 3 test cases - I'll do this ASAP.

@SergeyMell
Copy link
Contributor Author

@SergeyMell The change you have done as far as i am understanding here is If a guest user comes and adds products without logging in and signing up. Then you want to include it in Abandoned List, but there is a case when user just adds the item and does not proceed to checkout further, in that case we don't even have its email and according to the change you have done it will be invalid as well.

No, it will not because I'm using pluck(:email).compact. So it will remove empty emails

@vinay-mittal
Copy link
Member

@SergeyMell I will verify this thing by running you code. And get back to you.

@vinay-mittal
Copy link
Member

@SergeyMell Have you been able to change the test cases as mentioned above?

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.

2 participants