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

Fixed wrong url redirect when edit product review from Customer view page #22426

Conversation

ravi-chandra3197
Copy link
Contributor

@ravi-chandra3197 ravi-chandra3197 commented Apr 19, 2019

Fixed wrong URL redirect when edit product review from the Customer view page
Fixed wrong URL redirect when edit product review from the view product page

Description (*)

Fixed Issues (if relevant)

  1. wrong url redirect when edit product review from Customer view page #22425: wrong url redirect when edit product review from Customer view page

Manual testing scenarios (*)

  1. Navigate to Customer for edit

  2. CUSTOMER INFORMATION tab -> Product Reviews -> Edit Review

  3. After edit review -> click on save

  4. Catalog-> product-> Product Reviews

  5. edit any review -> click on save

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 19, 2019

Hi @ravi-chandra3197. 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

@@ -85,7 +85,8 @@ protected function _prepareForm()
[
'id' => $this->getRequest()->getParam('id'),
'ret' => $this->_coreRegistry->registry('ret'),
'productId' => $this->getRequest()->getParam('productId')
'productId' => $this->getRequest()->getParam('productId'),
'customerId' => $this->getRequest()->getParam('customerId')
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of edit from customer page productId will be empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @orlangur
I have checked this in case of edit from customer page productId is not require because review edit using review id, productId, and customerId is additional param
here I have attached snapshot of edit form action which clearly show productId is not set so it will not be passed in action URL param.

https://prnt.sc/ne3if0

Copy link
Contributor

Choose a reason for hiding this comment

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

@ravi-chandra3197 I mean, there is no need to add productId / customerId when it is empty. Please refactor and squash changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ravi-chandra3197 why did you mark conversation as resolved?

'productId' => $this->getRequest()->getParam('productId'),

here we should not add productId when it is empty. Same for customerId.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @orlangur
I have update PR code as per your suggestion now we also check param before adding array can you review it.

@@ -85,7 +85,8 @@ protected function _prepareForm()
[
'id' => $this->getRequest()->getParam('id'),
'ret' => $this->_coreRegistry->registry('ret'),
'productId' => $this->getRequest()->getParam('productId')
'productId' => $this->getRequest()->getParam('productId'),
'customerId' => $this->getRequest()->getParam('customerId')
Copy link
Contributor

Choose a reason for hiding this comment

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

@ravi-chandra3197 I mean, there is no need to add productId / customerId when it is empty. Please refactor and squash changes.

@ravi-chandra3197
Copy link
Contributor Author

Hello @orlangur
I have check form post URL with 3 possible way edit review

  1. edit review from review grid
    https://prnt.sc/ng6d04

  2. edit review from the view product page
    https://prnt.sc/ng6c7q

  3. edit review from the veiw customer page
    https://prnt.sc/ne3if0

if productId / customerId empty then not added.

@@ -85,7 +85,8 @@ protected function _prepareForm()
[
'id' => $this->getRequest()->getParam('id'),
'ret' => $this->_coreRegistry->registry('ret'),
'productId' => $this->getRequest()->getParam('productId')
'productId' => $this->getRequest()->getParam('productId'),
'customerId' => $this->getRequest()->getParam('customerId')
Copy link
Contributor

Choose a reason for hiding this comment

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

@ravi-chandra3197 why did you mark conversation as resolved?

'productId' => $this->getRequest()->getParam('productId'),

here we should not add productId when it is empty. Same for customerId.

@ravi-chandra3197
Copy link
Contributor Author

ravi-chandra3197 commented Apr 24, 2019

Hello @orlangur
I have check form post URL with 3 possible way edit review

  1. edit review from review grid
    https://prnt.sc/ng6d04
  2. edit review from the view product page
    https://prnt.sc/ng6c7q
  3. edit review from the veiw customer page
    https://prnt.sc/ne3if0

if productId / customerId empty then not added.

Hello @orlangur
I have checked this if empty productId / customerId so it will not be added in URL which shows in above snapshot

@orlangur
Copy link
Contributor

@ravi-chandra3197 they must not be added to array in the first place.

@ravi-chandra3197
Copy link
Contributor Author

ravi-chandra3197 commented Apr 24, 2019

Hello @orlangur
I have check form post URL with 3 possible way edit review
I have use print_r($form->getData()); and get following output

  1. edit review from review grid
    Array ( [id] => edit_form [action] => http://10.16.16.55/magento227/admin/review/product/save/id/374/key/6ea4cd512d29e609f66278ee7655f1da7562f8866296f20f0e0ec25fb5e43b90/ [method] => post )

https://prnt.sc/ng6d04

  1. edit review from the view product page
    Array ( [id] => edit_form [action] => http://10.16.16.55/magento227/admin/review/product/save/id/1/productId/1/key/6ea4cd512d29e609f66278ee7655f1da7562f8866296f20f0e0ec25fb5e43b90/ [method] => post )

https://prnt.sc/ng6c7q

  1. edit review from the veiw customer page
    Array ( [id] => edit_form [action] => http://10.16.16.55/magento227/admin/review/product/save/id/374/customerId/2/key/6ea4cd512d29e609f66278ee7655f1da7562f8866296f20f0e0ec25fb5e43b90/ [method] => post )

https://prnt.sc/ne3if0

if productId / customerId empty then not added.

@@ -85,7 +85,8 @@ protected function _prepareForm()
[
'id' => $this->getRequest()->getParam('id'),
'ret' => $this->_coreRegistry->registry('ret'),
'productId' => $this->getRequest()->getParam('productId')
'productId' => $this->getRequest()->getParam('productId'),
'customerId' => $this->getRequest()->getParam('customerId')
Copy link
Contributor

Choose a reason for hiding this comment

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

@ravi-chandra3197 here data must not be added to array when it is empty (current code produces lines like 'productId' => null).

@ravi-chandra3197
Copy link
Contributor Author

Hello @orlangur
I have update PR code as per your suggestion now we also check param before adding array can you review it.

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.

After changes are applied and all builds are green, please squash them into a single commit so that we have perfectly clean history 😉

@@ -75,18 +75,23 @@ protected function _prepareForm()
$review = $this->_coreRegistry->registry('review_data');
$product = $this->_productFactory->create()->load($review->getEntityPkValue());

$formActionParam['id'] = $this->getRequest()->getParam('id');
Copy link
Contributor

Choose a reason for hiding this comment

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

formActionParams and please do initial assignment like $var = ['id' => ..., 'ret' => ...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @orlangur
I have change formActionParams and add initial assignment

@@ -75,18 +75,23 @@ protected function _prepareForm()
$review = $this->_coreRegistry->registry('review_data');
$product = $this->_productFactory->create()->load($review->getEntityPkValue());

$formActionParam['id'] = $this->getRequest()->getParam('id');
$formActionParam['ret'] = $this->_coreRegistry->registry('ret');
if ($this->getRequest()->getParam('productId') != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ($this->getRequest()->getParam('productId') != null) {
if ($this->getRequest()->getParam('productId')) {

Same for customerId

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @orlangur
I have updated PR as your suggestion can you review it.

@ravi-chandra3197 ravi-chandra3197 force-pushed the patch-product-review-customer-redirect branch from 5b89e92 to e7034d3 Compare April 29, 2019 04:30
@@ -75,18 +75,24 @@ protected function _prepareForm()
$review = $this->_coreRegistry->registry('review_data');
$product = $this->_productFactory->create()->load($review->getEntityPkValue());

$formActionParams =['id' => $this->getRequest()->getParam('id'),
Copy link
Contributor

Choose a reason for hiding this comment

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

@ravi-chandra3197 does this formatting looks good to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @orlangur
I have updated PR.

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.

Please squash changes into a single commit so that we have perfectly clean history 😉

@ravi-chandra3197 ravi-chandra3197 force-pushed the patch-product-review-customer-redirect branch from 871e351 to 19e7380 Compare May 1, 2019 04:03
@ravi-chandra3197
Copy link
Contributor Author

Please squash changes into a single commit so that we have perfectly clean history

Hello @orlangur
I have squash changes into a single commit, can you review it.

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-5195 has been created to process this Pull Request

@VasylShvorak
Copy link
Contributor

Hi @ravi-chandra3197 I cannot reproduce issue "wrong URL redirect when edit product review from the view product page". Part of code in Save.php is not needed because of after edit and save review from product edit page we are redirected to product edit page as expected

@ravi-chandra3197
Copy link
Contributor Author

ravi-chandra3197 commented Jun 4, 2019

Hi @ravi-chandra3197 I cannot reproduce issue "wrong URL redirect when edit product review from the view product page". Part of code in Save.php is not needed because of after edit and save review from product edit page we are redirected to product edit page as expected

Hello @VasylShvorak
In my Previous PR #22202 , fix the issue for the wrong redirect after edit review from the view product page in this PR I have change for customer and update formActionParams which is used for both customer and product as well.

@VasylShvorak
Copy link
Contributor

@ravi-chandra3197 I double check it again. Code from Save.php is used when edit review from customer edit page. Thanks

@VasylShvorak
Copy link
Contributor

✔️ QA passed

@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-5195 has been created to process this Pull Request

@m2-assistant
Copy link

m2-assistant bot commented Jun 7, 2019

Hi @ravi-chandra3197, 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.

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.3 milestone Jun 7, 2019
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.

None yet

7 participants