Fixed Issue #14633 Sub-admin role related issue in order view page in magento 2 admin #20903
Conversation
|
Hi @GovindaSharma. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
There was a problem hiding this comment.
Thanks for the pull requests @GovindaSharma ! Please take a look at the code review notes. Please apply the fixes not only on the commented line but use the same approach for all the files in the pull request in general.
| @@ -35,7 +47,10 @@ public function getTabTitle() | |||
| */ | |||
| public function canShowTab() | |||
| { | |||
| return true; | |||
| if($this->authorization->isAllowed('Magento_Sales::invoice')) | |||
There was a problem hiding this comment.
it is fine to shorten this construction to return $this->authorization->isAllowed('Magento_Sales::creditmemo');
| * @param \Magento\Framework\AuthorizationInterface|null $authorization | ||
| */ | ||
|
|
||
| public function __construct( |
There was a problem hiding this comment.
Please consider that parent class has constructor
(\Magento\Framework\View\Element\AbstractBlock::__construct), please ensure you call it.
| public function __construct( | ||
| \Magento\Framework\AuthorizationInterface $authorization = null | ||
| ){ | ||
| $this->authorization = $authorization?: \Magento\Framework\App\ObjectManager::getInstance()->get(Magento\Framework\AuthorizationInterface::class); |
| * @param array $data | ||
| */ | ||
| public function __construct( | ||
| \Magento\Framework\View\Element\Context $context, | ||
| \Magento\Framework\Registry $coreRegistry, | ||
| \Magento\Framework\AuthorizationInterface $authorization = null, |
There was a problem hiding this comment.
New constructor parameters should be at the end of the list (after $data in this case)
|
hi @sivaschenko i have updated all the changes suggested by you.Please check and let me know on this |
There was a problem hiding this comment.
Thanks for updates @GovindaSharma ! Please see my code review notes.
| /** | ||
| * @var Magento\Framework\AuthorizationInterface | ||
| */ | ||
| protected $authorization; |
There was a problem hiding this comment.
please change the access level to private
| /** | ||
| * @var Magento\Framework\AuthorizationInterface | ||
| */ | ||
| protected $authorization; |
There was a problem hiding this comment.
please change the access level to private
| /** | ||
| * @var Magento\Framework\AuthorizationInterface | ||
| */ | ||
| protected $authorization; |
There was a problem hiding this comment.
please change the access level to private
| array $data = [], | ||
| \Magento\Framework\AuthorizationInterface $authorization = null) | ||
| { | ||
| $this->authorization = $authorization ? : \Magento\Framework\App\ObjectManager::getInstance ()->get ( Magento\Framework\AuthorizationInterface::class ); |
There was a problem hiding this comment.
Please fix code style/formatting for all files
|
Hi @GovindaSharma it would be great if you can squash all changes to one commit with a meaningful name after the update. |
|
Hi @sivaschenko sure i will do and update you. |
Update Creditmemos.php Update Shipments.php Update Creditmemos.php Update Invoices.php Update Invoices.php Update Creditmemos.php Update Shipments.php Update Creditmemos.php Update Invoices.php Update Creditmemos.php Update Creditmemos.php Made Requested Changes
There was a problem hiding this comment.
@GovindaSharma thanks for updates. Please fix code style and use meaningful commit message, i.e. '#20903: Fixed Issue #14633 Sub-admin role related issue in order view page in magento 2 admin'
|
@GovindaSharma , I am closing this PR now due to inactivity. |
|
Hi @GovindaSharma, thank you for your contribution! |
Fixed Issue #14633 Sub-admin role related issue in order view page in magento 2 admin
Description (*)
Fixed Issue #14633 Sub-admin role related issue in order view page in magento 2 admin
Fixed Issues (if relevant)
Manual testing scenarios (*)
I have created one sub admin user from Magento admin. This user can only view orders, add a comment and operate on shipping. I gave some permissions for sub admin user. Please check the below image:
So as you can see I have disabled Invoice and Credit memo role of this sub-admin.
But when I logged in as sub admin and go to order view page.Sometime Pop will come with "Something Went Wrong" and sometimes js error comes in console
Contribution checklist (*)