Skip to content

Inconsistent Redirect in Admin Notification Controller #13828

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

Merged
merged 2 commits into from
Feb 28, 2018
Merged

Inconsistent Redirect in Admin Notification Controller #13828

merged 2 commits into from
Feb 28, 2018

Conversation

chickenland
Copy link
Contributor

This controller has an inconsistent redirect mechanism (than other controllers in admin-notification). This means that if the admin has a different URL, the admin user is returned to the frontend URL of the default store instead.

Manual testing scenarios

  1. Setup admin to run on a separate domain to the main site
  2. Attempt to mass remove all notifications within admin

This controller has an inconsistent redirect mechanism (than other controllers in admin-notification).  This means that if the admin has a different URL, the admin user is returned to the frontend URL of the default store instead.
@chickenland chickenland changed the title Inconsistent Redirect Inconsistent Redirect in Admin Notification Controller Feb 23, 2018
@@ -39,6 +39,7 @@ public function execute()
$this->messageManager->addException($e, __("We couldn't remove the messages because of an error."));
}
}
$this->getResponse()->setRedirect($this->_redirect->getRedirectUrl($this->getUrl('*')));
$this->_redirect('adminhtml/*/');
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the end of the method. No need for return here

end of class meant return was un-needed
@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
ENGCOM-589 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@chickenland thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

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