Merge remote-tracking branch 'origin/MAGETWO-95536' into chaika_november

Author: olysenko

app/code/Magento/Integration/Plugin/Model/AdminUser.php

@@ -3,6 +3,7 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Integration\Plugin\Model;
 
 use Magento\Integration\Model\AdminTokenService;
@@ -31,14 +32,15 @@ public function __construct(
      *
      * @param \Magento\User\Model\User $subject
      * @param \Magento\Framework\DataObject $object
-     * @return $this
+     * @return \Magento\User\Model\User
+     * @throws \Magento\Framework\Exception\LocalizedException
      */
     public function afterSave(
         \Magento\User\Model\User $subject,
         \Magento\Framework\DataObject $object
-    ) {
+    ): \Magento\User\Model\User {
         $isActive = $object->getIsActive();
-        if (isset($isActive) && $isActive == 0) {
+        if ($isActive !== null && $isActive == 0) {
             $this->adminTokenService->revokeAdminAccessToken($object->getId());
         }
         return $subject;

app/code/Magento/User/Controller/Adminhtml/User/Role/SaveRole.php

@@ -11,6 +11,7 @@
 use Magento\Authorization\Model\Acl\Role\Group as RoleGroup;
 use Magento\Authorization\Model\UserContextInterface;
 use Magento\Framework\Controller\ResultFactory;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Exception\State\UserLockedException;
 use Magento\Security\Model\SecurityCookie;
 
@@ -77,10 +78,8 @@ public function execute()
 
         $rid = $this->getRequest()->getParam('role_id', false);
         $resource = $this->getRequest()->getParam('resource', false);
-        $roleUsers = $this->getRequest()->getParam('in_role_user', null);
-        parse_str($roleUsers, $roleUsers);
-        $roleUsers = array_keys($roleUsers);
-
+        $oldRoleUsers = $this->parseRequestVariable('in_role_user_old');
+        $roleUsers = $this->parseRequestVariable('in_role_user');
         $isAll = $this->getRequest()->getParam('all');
         if ($isAll) {
             $resource = [$this->_objectManager->get(\Magento\Framework\Acl\RootResource::class)->getId()];
@@ -106,28 +105,24 @@ public function execute()
             $role->save();
 
             $this->_rulesFactory->create()->setRoleId($role->getId())->setResources($resource)->saveRel();
-
-            $this->processPreviousUsers($role);
-
-            foreach ($roleUsers as $nRuid) {
-                $this->_addUserToRole($nRuid, $role->getId());
-            }
-            $this->messageManager->addSuccess(__('You saved the role.'));
+            $this->processPreviousUsers($role, $oldRoleUsers);
+            $this->processCurrentUsers($role, $roleUsers);
+            $this->messageManager->addSuccessMessage(__('You saved the role.'));
         } catch (UserLockedException $e) {
             $this->_auth->logout();
             $this->getSecurityCookie()->setLogoutReasonCookie(
                 \Magento\Security\Model\AdminSessionsManager::LOGOUT_REASON_USER_LOCKED
             );
             return $resultRedirect->setPath('*');
         } catch (\Magento\Framework\Exception\AuthenticationException $e) {
-            $this->messageManager->addError(
+            $this->messageManager->addErrorMessage(
                 __('The password entered for the current user is invalid. Verify the password and try again.')
             );
             return $this->saveDataToSessionAndRedirect($role, $this->getRequest()->getPostValue(), $resultRedirect);
         } catch (\Magento\Framework\Exception\LocalizedException $e) {
-            $this->messageManager->addError($e->getMessage());
+            $this->messageManager->addErrorMessage($e->getMessage());
         } catch (\Exception $e) {
-            $this->messageManager->addError(__('An error occurred while saving this role.'));
+            $this->messageManager->addErrorMessage(__('An error occurred while saving this role.'));
         }
 
         return $resultRedirect->setPath('*/*/');
@@ -151,32 +146,64 @@ protected function validateUser()
         return $this;
     }
 
+    /**
+     * Parse request value from string
+     *
+     * @param string $paramName
+     * @return array
+     */
+    private function parseRequestVariable($paramName): array
+    {
+        $value = $this->getRequest()->getParam($paramName, null);
+        parse_str($value, $value);
+        $value = array_keys($value);
+        return $value;
+    }
+
     /**
      * Process previous users
      *
      * @param \Magento\Authorization\Model\Role $role
+     * @param array $oldRoleUsers
      * @return $this
      * @throws \Exception
      */
-    protected function processPreviousUsers(\Magento\Authorization\Model\Role $role)
+    protected function processPreviousUsers(\Magento\Authorization\Model\Role $role, array $oldRoleUsers): self
     {
-        $oldRoleUsers = $this->getRequest()->getParam('in_role_user_old');
-        parse_str($oldRoleUsers, $oldRoleUsers);
-        $oldRoleUsers = array_keys($oldRoleUsers);
-
         foreach ($oldRoleUsers as $oUid) {
             $this->_deleteUserFromRole($oUid, $role->getId());
         }
 
         return $this;
     }
 
+    /**
+     * Processes users to be assigned to roles
+     *
+     * @param \Magento\Authorization\Model\Role $role
+     * @param array $roleUsers
+     * @return $this
+     */
+    private function processCurrentUsers(\Magento\Authorization\Model\Role $role, array $roleUsers): self
+    {
+        foreach ($roleUsers as $nRuid) {
+            try {
+                $this->_addUserToRole($nRuid, $role->getId());
+            } catch (LocalizedException $e) {
+                $this->messageManager->addErrorMessage($e->getMessage());
+            }
+        }
+
+        return $this;
+    }
+
     /**
      * Assign user to role
      *
      * @param int $userId
      * @param int $roleId
      * @return bool
+     * @throws LocalizedException
      */
     protected function _addUserToRole($userId, $roleId)
     {