MC-30224: [2.4] Deleting an empty user model caused deleting admin role

Author: zakdma

app/code/Magento/User/Model/ResourceModel/User.php

@@ -258,15 +258,17 @@ public function delete(\Magento\Framework\Model\AbstractModel $user)
 
         $uid = $user->getId();
         $connection->beginTransaction();
-        try {
-            $connection->delete($this->getMainTable(), ['user_id = ?' => $uid]);
-            $connection->delete(
-                $this->getTable('authorization_role'),
-                ['user_id = ?' => $uid, 'user_type = ?' => UserContextInterface::USER_TYPE_ADMIN]
-            );
-        } catch (\Magento\Framework\Exception\LocalizedException $e) {
-            $connection->rollBack();
-            return false;
+        if ($uid) {
+            try {
+                $connection->delete($this->getMainTable(), ['user_id = ?' => $uid]);
+                $connection->delete(
+                    $this->getTable('authorization_role'),
+                    ['user_id = ?' => $uid, 'user_type = ?' => UserContextInterface::USER_TYPE_ADMIN]
+                );
+            } catch (\Magento\Framework\Exception\LocalizedException $e) {
+                $connection->rollBack();
+                return false;
+            }
         }
         $connection->commit();
         $this->_afterDelete($user);

dev/tests/integration/testsuite/Magento/User/Model/ResourceModel/UserTest.php

@@ -5,9 +5,11 @@
  */
 namespace Magento\User\Model\ResourceModel;
 
+use Magento\Authorization\Model\ResourceModel\Role\Collection as UserRoleCollection;
+use Magento\Authorization\Model\UserContextInterface;
 use Magento\TestFramework\Helper\Bootstrap;
-use Magento\User\Model\User;
 use Magento\User\Model\ResourceModel\User as UserResourceModel;
+use Magento\User\Model\User;
 
 /**
  * @magentoAppArea adminhtml
@@ -46,6 +48,33 @@ public function testGetLatestPasswordWhenZeroPasswordLifetime()
         );
     }
 
+    /**
+     * Test that user role is not deleted after deleting empty user
+     */
+    public function testDelete()
+    {
+        $this->checkRoleCollectionSize();
+        /** @var User $user */
+        $user = Bootstrap::getObjectManager()->create(
+            User::class
+        );
+        $this->model->delete($user);
+        $this->checkRoleCollectionSize();
+    }
+
+    /**
+     * Ensure that role collection size is correct
+     */
+    private function checkRoleCollectionSize()
+    {
+        /** @var UserRoleCollection $roleCollection */
+        $roleCollection = Bootstrap::getObjectManager()->create(
+            UserRoleCollection::class
+        );
+        $roleCollection->setUserFilter(0, UserContextInterface::USER_TYPE_ADMIN);
+        $this->assertEquals(1, $roleCollection->getSize());
+    }
+
     public function testCountAll()
     {
         $this->assertSame(1, $this->model->countAll());