Skip to content

logging-in in order to checkout when guest checkout is disabled #6015 #24794

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

Open
wants to merge 11 commits into
base: 2.4-develop
Choose a base branch
from

Conversation

ivanko-dev
Copy link
Contributor

@ivanko-dev ivanko-dev commented Sep 30, 2019

#6015

Description (*)

Issue appear in case if "login_redirect" value in cookie missing. This case was reproduced with login using dialog opened by click "Proceed to Checkout" on cart page.

Fixed Issues (if relevant)

  1. Fixes logging-in in order to checkout when guest checkout is disabled #6015: logging-in in order to checkout when guest checkout is disabled

Manual testing scenarios (*)

  1. Go to admin panel Stores -> (Settings) Configuration -> (Sales) Checkout and Set Allow Guest Checkout = No in Checkout Options. Save config.
  2. Open application with clean cookie in browser (no login_redirect value should be defined)
  3. Add product to cart without login
  4. Go to cart page and click "Proceed to Checkout" on right side
  5. Add login data in dialog and login

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@ivanko-dev ivanko-dev requested a review from DrewML as a code owner September 30, 2019 17:17
@m2-assistant
Copy link

m2-assistant bot commented Sep 30, 2019

Hi @ivan-koliadynskyy. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@ivanko-dev
Copy link
Contributor Author

I have feeling that error on Functional Tests build appear because testing flow expect user will be not redirect to checkout page after login / sign up and will need to navigate to it

Copy link
Contributor

@VladimirZaets VladimirZaets left a comment

Choose a reason for hiding this comment

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

Hi @ivan-koliadynskyy. Thanks for collaboration. Due to Magento Definition of Done all code must be covered by tests. Please cover your fix by automated tests.

@ghost ghost assigned VladimirZaets Oct 1, 2019
@ivanko-dev
Copy link
Contributor Author

Hi @ivan-koliadynskyy. Thanks for collaboration. Due to Magento Definition of Done all code must be covered by tests. Please cover your fix by automated tests.

@VladimirZaets , thank you about feedback. According to issue description:

One would expect that you are automatically redirected to the checkout after logging in (you did choose to "go to checkout"...). Even when you go to the cart via "view cart" first and then click "proceed to checkout", the behavior is the same... It doesn't seem logical to have to click the button a second time after signing in because you stay on the same page after sign-in...

And I see that there is additional step to click checkout link after login according to Functional Test:
image
This testing step is blocking success result. Looks like steps for MFT should be updated or ticket #6015 is not an issue.
Could you please suggest what is right solution in this case?
Thanks

Copy link
Contributor Author

@ivanko-dev ivanko-dev left a comment

Choose a reason for hiding this comment

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

Could you please check information related to this change from comment #24794 (comment)

@ivanko-dev ivanko-dev requested a review from paliarush as a code owner October 4, 2019 17:18
…ter click checkout and login instead of cart page like it was before.
@ivanko-dev
Copy link
Contributor Author

Idea of this fix is redirect user to checkout page when user click checkout and made login with dialog.
I have tried to update MFTF according to this scenario but some tests still fail.
Can somebody help to check what is better to do in this case? Should I look on these MFTF tests and update them to get pull request pass or maybe this is not an issue if user get back to cart page after login and need to clock checkout button one more time?

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:19
@engcom-Echo engcom-Echo self-assigned this May 8, 2020
@engcom-Echo
Copy link
Contributor

I will take care of test coverage.

@engcom-Echo
Copy link
Contributor

Hi, @ivan-koliadynskyy. Failed functional tests not related to the changes in this PR

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo engcom-Echo added Auto-Tests: Covered All changes in Pull Request is covered by auto-tests and removed help wanted labels May 29, 2020
@engcom-Echo
Copy link
Contributor

Hi @VladimirZaets , @paliarush , @DrewML
Could you please approve it?

@engcom-Echo
Copy link
Contributor

engcom-Echo commented May 29, 2020

Hi @VladimirZaets.

Hi @ivan-koliadynskyy. Thanks for collaboration. Due to Magento Definition of Done all code must be covered by tests. Please cover your fix by automated tests.

In the file
app/code/Magento/Checkout/Test/Mftf/Test/StorefrontCustomerCheckoutOnLoginWhenGuestCheckoutIsDisabledTest.xml

<click selector="{{CheckoutCartSummarySection.proceedToCheckout}}" stepKey="goToCheckout1"/>

was be deleted the second click to 'Proceed To Checkout'. This is exactly the goal that PR has achieved.

@VladimirZaets
Copy link
Contributor

@magento run all tests

@VladimirZaets
Copy link
Contributor

@engcom-Echo can you please take a look at failed tests?

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo
Copy link
Contributor

Failed functional tests not related to the changes in this PR

@ghost ghost added Progress: review Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. Priority: P3 May be fixed according to the position in the backlog. and removed Progress: needs update labels Aug 7, 2020
@joshuaswarren
Copy link

Hello, @ivanko-dev - thanks for your efforts contributing to Magento and your work on this pull request! It's one of the oldest pull requests still open on the Magento 2 repository, and I can see it was originally authored against Magento 2.3. I'm wondering if you are aware of this is still an issue in Magento 2.4, or if you have the time and interest in helping work to get this pull request moved closer to being merged or closed?

Let me know how I can help. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Frontend Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Checkout Priority: P3 May be fixed according to the position in the backlog. Progress: review Release Line: 2.4 Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
Status: Review in Progress
Development

Successfully merging this pull request may close these issues.

logging-in in order to checkout when guest checkout is disabled
6 participants