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: remove TestObserver class #13174

Merged

Conversation

coderimus
Copy link
Contributor

@coderimus coderimus commented Jan 14, 2018

Description

Hello,

The Magento\ProductAlert\Controller\Add\TestObserver, as I can see, was used for testing propose. It can run methods for sending product alert (price and stock) emails and, also, error emails for admin: Magento\ProductAlert\Model\Observer::process(). The main reason why I created this PR and remove reported class is that it can be run by any logged in customer while process() method should be run only by an event or by cron.

Looking forward to your reply,
Alex

Fixed Issues (if relevant)

N/A

Manual testing scenarios

  1. Logged in to the frontend area using your customer account
  2. Set the breakpoint for debugging in your IDE here: Magento\ProductAlert\Controller\Add\TestObserver::execute() line 18
  3. Use next URL for test/debug: <base_url>/productalert/add/testObserver/
  4. In browser you will see the blank page. In your debug console you will be stopped at the breakpoint and can continue with the process() method

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)

@dmanners dmanners self-assigned this Jan 15, 2018
@dmanners dmanners added this to the January 2018 milestone Jan 15, 2018
@magento-team magento-team merged commit 7af2391 into magento:2.2-develop Jan 16, 2018
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.

3 participants