Skip to content

Fix For Issue:Incorrect redirect navigate on the review edit page #29318 #30436

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: 2.4-develop
Choose a base branch
from
Open

Fix For Issue:Incorrect redirect navigate on the review edit page #29318 #30436

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
440a4cf
Update BackButton.php
Yash-Bhayani Oct 10, 2020
206335f
Update Form.php
Yash-Bhayani Oct 10, 2020
0a81ed8
Update Form.php
Yash-Bhayani Oct 10, 2020
c3f6365
Merge branch '2.4-develop-mainline' into 2.4-develop-Yash-Bhayani
engcom-Foxtrot Jan 18, 2021
2ded1c1
magento/magento2#30436: Fix For Issue:Incorrect redirect navigate on …
engcom-Foxtrot Jan 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions app/code/Magento/Customer/Block/Adminhtml/Edit/BackButton.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,26 @@
*/
class BackButton extends GenericButton implements ButtonProviderInterface
{
/**
* @var Form
*/
private $form;
/**
* @var \Magento\Framework\Url\DecoderInterface
*/
private $decoder;

public function __construct(
\Magento\Framework\Url\DecoderInterface $decoder,
\Magento\Customer\Block\Adminhtml\Edit\Form $form,
\Magento\Backend\Block\Widget\Context $context,
\Magento\Framework\Registry $registry
) {
parent::__construct($context, $registry);
$this->form = $form;
$this->decoder = $decoder;
}

/**
* @return array
*/
Expand All @@ -32,6 +52,12 @@ public function getButtonData()
*/
public function getBackUrl()
{
if ($fromPath=$this->decoder->decode($this->form->getRequest()->getParam('fromPath'))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding this parameter from the request, can't we use the RedirectInterface and the method getRefererUrl? This should return the URL that the request came from.

Copy link
Contributor

@engcom-Foxtrot engcom-Foxtrot Feb 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@gabrieldagama I don't think it's a good idea. This will cause redirect to any previously opened page and works as the 'back' button in the browser.

return $this->getUrl(
$fromPath,
['id' => $this->form->getRequest()->getParam('review_id')]
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we need a universal solution for similar cases instead of fixing particular places. We see the same situation when we go to the customer information page from the order view page. So, we may have a couple more places in the admin panel where we have the same issue with the back button.

It would be great if we could find out how to solve this issue for all occurrences.

Hope it makes sense

Copy link
Contributor

@engcom-Foxtrot engcom-Foxtrot Oct 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hello, @rogyar! I've done some investigation and I don't see the possibility to make a universal and reasonable solution for all "Back" buttons.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @engcom-Foxtrot. I've been reflected on that for some time and you are right. It's not an easy task unless we have the same name for an entity identifier.

Anyway, thank you for checking!

);
}
return $this->getUrl('*/*/');
}
}
18 changes: 17 additions & 1 deletion app/code/Magento/Review/Block/Adminhtml/Edit/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ class Form extends \Magento\Backend\Block\Widget\Form\Generic
* @var \Magento\Store\Model\System\Store
*/
protected $_systemStore;
/**
* @var \Magento\Framework\Url\EncoderInterface
*/
private $urlEncoder;

/**
* @param \Magento\Backend\Block\Template\Context $context
Expand All @@ -45,6 +49,7 @@ class Form extends \Magento\Backend\Block\Widget\Form\Generic
* @param \Magento\Customer\APi\CustomerRepositoryInterface $customerRepository
* @param \Magento\Catalog\Model\ProductFactory $productFactory
* @param \Magento\Review\Helper\Data $reviewData
* @param \Magento\Framework\Url\EncoderInterface $urlEncoder
* @param array $data
*/
public function __construct(
Expand All @@ -55,12 +60,14 @@ public function __construct(
\Magento\Customer\Api\CustomerRepositoryInterface $customerRepository,
\Magento\Catalog\Model\ProductFactory $productFactory,
\Magento\Review\Helper\Data $reviewData,
\Magento\Framework\Url\EncoderInterface $urlEncoder,
array $data = []
) {
$this->_reviewData = $reviewData;
$this->customerRepository = $customerRepository;
$this->_productFactory = $productFactory;
$this->_systemStore = $systemStore;
$this->urlEncoder = $urlEncoder;
parent::__construct($context, $registry, $formFactory, $data);
}

Expand Down Expand Up @@ -120,10 +127,19 @@ protected function _prepareForm()
);

try {
$fromPath=$this->urlEncoder->encode('review/product/edit');
$customer = $this->customerRepository->getById($review->getCustomerId());
$customerText = __(
'<a href="%1" onclick="this.target=\'blank\'">%2 %3</a> <a href="mailto:%4">(%4)</a>',
$this->getUrl('customer/index/edit', ['id' => $customer->getId(), 'active_tab' => 'review']),
$this->getUrl(
'customer/index/edit',
[
'id' => $customer->getId(),
'active_tab' => 'review',
'fromPath' => $fromPath,
'review_id' => $this->getRequest()->getParam('id')
]
),
$this->escapeHtml($customer->getFirstname()),
$this->escapeHtml($customer->getLastname()),
$this->escapeHtml($customer->getEmail())
Expand Down