Skip to content

Login with wishlist raise report after logout. #16386

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

Conversation

swnsma
Copy link
Contributor

@swnsma swnsma commented Jun 25, 2018

Description

Fix login issue through wishlist url which raise error right after login and report after logout.

  • fix error message after login with wishlist;
  • fix possibility to logout after login with wishlist;

Fixed Issues (if relevant)

  1. magento/magento2#?: can not found any issue on github, related to this error.

Manual testing scenarios

Pre-conditions

  1. Empty Magento.
  2. Registered customer.

Steps to reproduce

  1. Go to frontend.
  2. Go to route /wishlist
  3. You will be redirected to login form.
  4. Login to your customer account.
  5. Logout from the account.

Actual results

  1. You can see error after login.
  2. You will see report screen each time you will try to logout.

Expected results

  1. You can see no error after login.
  2. You successfully logout.

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 on Travis CI are green)

- fix error message after login with wishlist;
- fix possibility to logout after login with wishlist;
@magento-engcom-team
Copy link
Contributor

Hi @swnsma. 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@swnsma swnsma added the Partner: ISM eCompany Pull Request is created by partner ISM eCompany label Jun 25, 2018
@phoenix128 phoenix128 self-assigned this Jun 25, 2018
Copy link
Contributor

@phoenix128 phoenix128 left a comment

Choose a reason for hiding this comment

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

Hello @swnsma ,
can you please check my review?

@@ -45,7 +45,7 @@ public function aroundExecute(FlushFormKey $subject, callable $proceed, ...$args
$currentFormKey = $this->dataFormKey->getFormKey();
$proceed(...$args);
$beforeParams = $this->session->getBeforeRequestParams();
if ($beforeParams['form_key'] == $currentFormKey) {
if (isset($beforeParams['form_key']) && $beforeParams['form_key'] == $currentFormKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please switch it to strict comparison?
$beforeParams['form_key'] === $currentFormKey

@sidolov
Copy link
Contributor

sidolov commented Jun 26, 2018

@phoenix128 , strict comparison was added

@magento-engcom-team
Copy link
Contributor

Hi @phoenix128, thank you for the review.
ENGCOM-2133 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

Hi @swnsma. Thank you for your contribution.
We will aim to release these changes as part of 2.2.6.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants