Skip to content

Conversation

@osrecio
Copy link
Member

@osrecio osrecio commented Nov 5, 2017

Generate new FormKey afterLogin and set to the beforeRequest(Wishlist)

Description

Generate new FormKey afterLogin and set to the beforeRequest(Wishlist)

Fixed Issues (if relevant)

  1. 2.1.9 Item not added to the Wishlist if the user is not logged at the moment he click on the button to add it. #11825: 2.1.9 Item not added to the Wishlist if the user is not logged at the moment he click on the button to add it
  2. Adding to wishlist doesn't work when not logged in #11908: Adding to wishlist doesn't work when not logged in

Manual testing scenarios

  1. Create an user account.
  2. Logout from the user account
  3. Add a product to your Wishlist , you will get redirected to the login page
  4. Login

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)

@vkublytskyi vkublytskyi self-assigned this Nov 5, 2017
@vkublytskyi vkublytskyi added this to the November 2017 milestone Nov 5, 2017
@vkublytskyi vkublytskyi added Release Line: 2.1 2.2.x duplicate Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 5, 2017
@vkublytskyi
Copy link

@osrecio tests failed as this fix introduces dependency for PageCache on Customer module. We should avoid this for bugfixes. Also we should avoid this as Customer module already has a dependency on PageCashe module and we must avoid circular dependencies. As a solution, I may suggest make a plugin on \Magento\PageCache\Observer\FlushFormKey in a customer module

@osrecio
Copy link
Member Author

osrecio commented Nov 26, 2017

Hi @vkublytskyi I added a Plugin in Customer Module to make the same logic.

Choose a reason for hiding this comment

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

Please add @SuppressWarnings(PHPMD.UnusedFormalParameter) to avoid static tests failure

Choose a reason for hiding this comment

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

Please set copyright same as in other files for 2.1-develop:

/**
 * Copyright © 2013-2017 Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed by:

/**
 * Copyright © 2013-2017 Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */

Choose a reason for hiding this comment

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

Please do not modify copyright.

@osrecio osrecio force-pushed the PR#11825_2.1 branch 6 times, most recently from f6843ae to 3b9e91b Compare November 27, 2017 20:12
@osrecio
Copy link
Member Author

osrecio commented Nov 27, 2017

@vkublytskyi Added param @SuppressWarnings(PHPMD.UnusedFormalParameter) but still failing :(

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Nov 28, 2017

@osrecio please add following line to phpdoc block * @return void, it should fix static tests. Also in 2.2 and 2.3 you already have unit tests. Please add it there

@vkublytskyi
Copy link

@osrecio I've pushed commit with fixed static test and backport of unit test. Please take a look to fix in unit test to solve an issue with an incorrect value passed to aroundExecute in original test from 2.2. Please fill free to squash my commit and force push branch to clean up git history.

@magento-team magento-team merged commit f79d97e into magento:2.1-develop Dec 1, 2017
magento-team pushed a commit that referenced this pull request Dec 1, 2017
magento-team pushed a commit that referenced this pull request Dec 1, 2017
[EngCom] Public Pull Requests - 2.1-develop
 - MAGETWO-83288: [Backport 2.1] #11825: Generate new FormKey and replace for oldRequestParams Wishlist #12041
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Award: test coverage duplicate Progress: accept Release Line: 2.1 Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants