Skip to content

Conversation

@ilnytskyi
Copy link
Contributor

Description (*)

The classes
\Magento\ProductAlert\Controller\Add\Stock
\Magento\ProductAlert\Controller\Add\Price

Has been marked with \Magento\Framework\App\Action\HttpPostActionInterface
that caused 404 page issue after a redirect in \Magento\Customer\Controller\Account\LoginPost

Fixed Issues (if relevant)

  1. Product Alert after login shows 404 page #22266

Manual testing scenarios (*)

  1. Click subscribe for stock alert on product page
  2. Log in
  3. See product page and message about successful subscription for notification

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)

@m2-assistant
Copy link

m2-assistant bot commented Apr 11, 2019

Hi @ilnytskyi. 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 2.3-develop instance - deploy vanilla Magento instance

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

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Apr 11, 2019

CLA assistant check
All committers have signed the CLA.

@ilnytskyi ilnytskyi force-pushed the product-alert-22266 branch from 999d736 to 0d2b291 Compare April 11, 2019 06:15
@orlangur orlangur self-assigned this Apr 11, 2019
Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

@ilnytskyi GET must be implemented as separate action I believe.

Cc: @nmalevanec

@ilnytskyi
Copy link
Contributor Author

@orlangur
All actions in this module only receive requests to subscribe customer for notification, I am not sure what exactly has to be introduced in separate GET action.
What was the reason to restrict this method to allow only POST ?
I belive the problem is here Magento\Customer\Controller\Account\LoginPost::execute that caused by setting up just a url to redirect here
\Magento\ProductAlert\Controller\Add::dispatch btw then we could just mark this Add class with HttpPostActionInterface

@ilnytskyi ilnytskyi requested a review from orlangur May 17, 2019 08:10
@ilnytskyi
Copy link
Contributor Author

Closed since similar changes were implemented in #23218 by @ArjenMiedema
and no answer was given in this topic

@ilnytskyi ilnytskyi closed this Jul 8, 2019
@m2-assistant
Copy link

m2-assistant bot commented Jul 8, 2019

Hi @ilnytskyi, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@korostii
Copy link
Contributor

korostii commented Nov 5, 2020

FYI as of 2.3.5 it still gives 404 - unless you either add both of those interfaces, ass suggested by this PR (or, alternatively, remove them completely), tested this myself + here's a testimonial from stackoverlow:
https://magento.stackexchange.com/questions/269825/magento-2-customer-click-stock-alert-on-product-page-after-logged-in-it-shows
IMO this should be reopened and merged in. Can this be picked up please @magento-engcom-team ?

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.

5 participants