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

Fix : Cannot complete checkout on second visit when using Anonymous user #3640

Merged

Conversation

prinzdezibel
Copy link
Contributor

@prinzdezibel prinzdezibel commented Feb 2, 2018

Closes #3638

Changes:
Whenever the user choose to checkout as anonymous user, the address
book step should be skipped, if there's already a shipping AND billing
address specified for the cart.

Otherwise the address book grid would come up, but both options are
already selected (green check icon). Repeatedly clicking the active
item does not help, one would need to change the address to move beyond
the addressBook workflow step.

To test:

1A - Place an order for a single product as an anonymous user

  • Click on “Basic Reaction Product”
  • Click “Add to Cart”
  • Click on “Checkout Now”
  • Click on “Continue As Guest”
  • Fill out address
  • Click on “Save and Continue”
  • Select “Free Shipping”
  • (The Generic Payment method should be expanded by default) Enter any name
  • Enter 4242424242424242 for credit card number
  • Choose an expiration month
  • Choose an expiration year
  • Enter 345 as the CV2 code
  • Select “Complete your order”
  • Verify that you receive the confirmation screen (“Thank You!”)

1B - Place two consecutive orders as an anonymous user

  • Follow the instructions for “Place an order for a single product as an anonymous user”
  • Return to the home screen by clicking on “Reaction” up in the upper left corner
  • Click on “Basic Reaction Product”
  • Click “Add to Cart”
  • Click on “Checkout Now”
  • Select "Continue As Guest"

EXPECTED: The shipping workflow step should be active now.

EDIT 2: Please do also check the above another time with stripe payment selected.

Closes #3638

Changes:
Whenever the user choosed to checkout as anonymous user, the address
book step should be skipped, if there's already a shipping AND billing
address specified for the cart.

Otherwise the address book grid would come up, but both options are
already selected (green check icon). Repeatedly clicking the active
item does not help, one would need to change the address to move beyond
the addressBook workflow step.
@prinzdezibel prinzdezibel requested a review from jshimko February 2, 2018 10:36
@prinzdezibel
Copy link
Contributor Author

@jshimko Brent asked me to get my code changes reviewed from you.

@mattt416
Copy link
Contributor

mattt416 commented Feb 2, 2018

Hi @prinzdezibel I've made this modification locally but am still running into the reported issue. I reset the database after making the change and tried two consecutive checkouts as an anon user but can't proceed on 2nd checkout. As I'm unsure if anything needs to be reset/flushed in the JavaScript console, I've tried with a new browser/email combination but still hit the same problem on the 2nd checkout.

@prinzdezibel
Copy link
Contributor Author

@mattt416 Did you have a chance to look into this again? I can't reproduce the problem you're describing.

@mattt416
Copy link
Contributor

mattt416 commented Feb 5, 2018

Hi @prinzdezibel , I just started a new app (reaction init), applied your fix, ran reaction run, and then went through the steps to replicate this. I'm still seeing the behaviour from before. Are there any client-side caches that I would need to flush here, even though I've started with a fresh app?

@prinzdezibel
Copy link
Contributor Author

Hi @mattt416 , can we chat on Gitter? I'd like to go this through with you.

also result in a double-push of the workflow if shipping & billing is already set.
@prinzdezibel
Copy link
Contributor Author

@mattt416 I also fixed it for the case that Stripe as payment option is enabled. Which was obviously the case in your scenario, even if it was not checked active in the settings (I can re-construct your behavior when changing the following values directly in db - which essentially is a data corruption):

edit_document

I don't know if this is what resulted in the observations that you made, nor do I know if such a db state can happen in an erroneous situation. If so and you know why, I'd be glad to get more details on this.

But as far as the original problem goes: This should be fixed with my latest changes.

Thank you again, Michael.

@mattt416
Copy link
Contributor

mattt416 commented Feb 5, 2018

Hi @prinzdezibel, I just did a db reset on my dummy install and this is what I see:

meteor:PRIMARY> db.Packages.find({name: 'reaction-stripe'}).pretty()
{
        "_id" : "LD9s6WgxZqu8otQbx",
        "name" : "reaction-stripe",
        "shopId" : "J8Bhq3uTtdgwZx3rz",
        "icon" : "fa fa-cc-stripe",
        "enabled" : true,
        "settings" : {
                "mode" : false,
                "api_key" : "",
                "reaction-stripe" : {
                        "enabled" : false,
                        "support" : [
                                "Authorize",
                                "Capture",
                                "Refund"
                        ]
                },
                "public" : {
                        "client_id" : ""
                },
                "connectAuth" : {

                }
        },
        "registry" : [
                {
                        "label" : "Stripe",
                        "provides" : [
                                "paymentSettings"
                        ],
                        "container" : "dashboard",
                        "template" : "stripeSettings",
                        "hideForShopTypes" : [
                                "merchant",
                                "affiliate"
                        ]
                },
                {
                        "template" : "stripePaymentForm",
                        "provides" : [
                                "paymentMethod",
                                "marketplacePaymentMethod"
                        ],
                        "icon" : "fa fa-cc-stripe"
                },
                {
                        "route" : "/stripe/connect/authorize",
                        "template" : "stripeConnectAuthorize"
                },
                {
                        "label" : "Stripe Merchant Account",
                        "icon" : "fa fa-cc-stripe",
                        "container" : "dashboard",
                        "provides" : [
                                "marketplaceMerchantSettings"
                        ],
                        "template" : "stripeConnectMerchantSignup",
                        "hideForShopTypes" : [
                                "primary"
                        ]
                }
        ],
        "layout" : null
}
meteor:PRIMARY>

Nothing has been toggled in the admin panel yet.

@prinzdezibel
Copy link
Contributor Author

@mattt416 Thanks for hint! this looks like an error. I'm going to file a separate ticket for this.

@mattt416
Copy link
Contributor

mattt416 commented Feb 5, 2018

@prinzdezibel Regarding the actual bug tho -- I've tested this with stripe implicitly enabled, with it explicitly enabled, and with it explicitly disabled, and the 2nd checkout now pops up the shipping selection as expected. So, tl;dr your fix seems to be working! 👍

@prinzdezibel
Copy link
Contributor Author

prinzdezibel commented Feb 5, 2018

@mattt416 Please check your reaction.json file in /private/settings/. I guess there's a wrong default there. If enabled = true you'd also need this:

"settings" : {
        "mode" : false,
        "api_key" : "XXXX",
        "reaction-stripe" : {
            "enabled" : true,
            // more stuff
        }
    },

@mattt416
Copy link
Contributor

mattt416 commented Feb 5, 2018

@prinzdezibel
Copy link
Contributor Author

@mattt416 that's it !! Thanks for your help. I'm going to file a new ticket for that.

@mattt416
Copy link
Contributor

mattt416 commented Feb 5, 2018

@prinzdezibel Unless the cart code shouldn't be using isStripeEnabled, but rather something that checks if the enabled inside reaction-stripe's settings is true?

@prinzdezibel
Copy link
Contributor Author

@mattt416 yup, not sure which one is the source of truth actually. Not sure why this field is redundant at all. Need to dig into it.

@brent-hoover
Copy link
Collaborator

@prinzdezibel Just to clarify. There should be two enabled values there. The top-level one might be better labeled "installed" but it controls whether the payment function would display in the dashboard, the second whether the payment method is actually used during checkout.

@prinzdezibel
Copy link
Contributor Author

prinzdezibel commented Feb 6, 2018 via email

@brent-hoover
Copy link
Collaborator

@prinzdezibel So what's the status of this PR?

@prinzdezibel
Copy link
Contributor Author

prinzdezibel commented Feb 8, 2018 via email

@spencern spencern changed the base branch from master to release-1.8.0 February 13, 2018 16:26
@spencern spencern merged commit 0367619 into release-1.8.0 Feb 13, 2018
@spencern spencern deleted the fix-3638-prinzdezibel-fix-second-checkout-anonymous branch February 13, 2018 16:27
This was referenced Feb 15, 2018
@aaronjudd
Copy link
Contributor

@jshimko @prinzdezibel do you think this PR introduced this new behavior?

1__rc__node__and_basic_reaction_product_and_slack_-_reaction_commerce

Was checkout testing completed with anonymous, guest, registered users?
Was checkout testing completed with Avalara, Shippo, multiple payment packages?

@prinzdezibel
Copy link
Contributor Author

prinzdezibel commented Feb 16, 2018

@aaronjudd: yes, but should be addressed already by this: #3766

Will test again after applying @Akarshit 's PR

@prinzdezibel
Copy link
Contributor Author

prinzdezibel commented Feb 19, 2018

@aaronjudd Tested with avalara, stripe, paypal, shippo.
Tested anonymously and registered. Looks good to me after applying @Akarshit 's PR. Moving to closed again.

@spencern spencern mentioned this pull request Mar 9, 2018
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.

6 participants