Merge branch 'b-7.3.x-fix-duplicate-email-error-OXDEV-8351' into b-8.…

…0.x-fix-duplicate-email-error-OXDEV-8351 # Conflicts: # .github/oxid-esales/shop_ce.yaml # CHANGELOG-7.3.md # composer.json
OXID-eSales · Nov 27, 2024 · 86fd5ad · 86fd5ad
2 parents 59a1c53 + b988107
commit 86fd5ad
Show file tree

Hide file tree

Showing 13 changed files with 297 additions and 114 deletions.
diff --git a/.github/oxid-esales/shop_ce.yaml b/.github/oxid-esales/shop_ce.yaml
@@ -4,6 +4,8 @@
 install: &install
   git:
     sdk_ref: b-8.0.x
+    ref: &ref 'b-8.0.x-fix-duplicate-email-error-OXDEV-8351'
+    shop_ref: *ref
   add_services: 'selenium-chrome-126'
   method: script
   script: 'source/.github/oxid-esales/install.sh'

diff --git a/composer.json b/composer.json
@@ -51,7 +51,7 @@
         "incenteev/composer-parameter-handler": "^2.1.4",
         "mikey179/vfsstream": "~1.6.8",
         "oxid-esales/codeception-modules": "dev-b-8.0.x",
-        "oxid-esales/codeception-page-objects": "dev-b-8.0.x",
+        "oxid-esales/codeception-page-objects": "dev-b-8.0.x-fix-duplicate-email-error-OXDEV-8351",
         "oxid-esales/developer-tools": "dev-b-8.0.x",
         "oxid-esales/oxideshop-ide-helper": "dev-b-8.0.x",
         "phpspec/prophecy-phpunit": "^2.0.1",

diff --git a/source/Application/Component/UserComponent.php b/source/Application/Component/UserComponent.php
@@ -643,59 +643,60 @@ protected function changeUserWithoutRedirect()
         if (!$user) {
             return;
         }
+
+        $currentEmail = $user->getFieldData('oxusername');
+        $password = $user->getFieldData('oxpassword');
         $shippingAddress = $this->getShippingAddress();
         $billingAddress = $this->getBillingAddress();
+        $newEmail = $billingAddress['oxuser__oxusername'] ?? '';
 
-        $username = $user->getFieldData('oxusername');
-        $password = $user->getFieldData('oxpassword');
         try {
-            $newName = $billingAddress['oxuser__oxusername'] ?? '';
+            $isUsernameUpdated = $this->isUserNameUpdated($currentEmail ?? '', $newEmail);
+            if ($isUsernameUpdated && $this->isGuestUser($user)) {
+                $this->deleteExistingGuestUser($newEmail);
+            }
             if (
-                $this->isGuestUser($user)
-                && $this->isUserNameUpdated($user->oxuser__oxusername->value ?? '', $newName)
+                !$this->isGuestUser($user)
+                && $isUsernameUpdated
+                && $user->isEmailInUse($newEmail)
             ) {
-                $this->deleteExistingGuestUser($newName);
+                Registry::getUtilsView()->addErrorToDisplay('ERROR_MESSAGE_USER_USEREXISTS');
+                return false;
             }
-            $user->changeUserData($username, $password, $password, $billingAddress, $shippingAddress);
+            $user->changeUserData($currentEmail, $password, $password, $billingAddress, $shippingAddress);
 
             $isSubscriptionRequested = Registry::getRequest()->getRequestEscapedParameter('blnewssubscribed');
-            $userSubscriptionStatus = $isSubscriptionRequested
-                ?? $user->getNewsSubscription()->getOptInStatus();
-            // check if email address changed, if so, force check newsletter subscription settings.
-            $billingUsername = $billingAddress['oxuser__oxusername'] ?? null;
-            $forceSubscriptionCheck = ($billingUsername !== null && $billingUsername !== $username);
+            $userSubscriptionStatus = $isSubscriptionRequested ?? $user->getNewsSubscription()->getOptInStatus();
             $isSubscriptionEmailRequested = Registry::getConfig()->getConfigParam('blOrderOptInEmail');
+
             $this->_blNewsSubscriptionStatus = $user->setNewsSubscription(
                 $userSubscriptionStatus,
                 $isSubscriptionEmailRequested,
-                $forceSubscriptionCheck
+                $isUsernameUpdated
             );
+
+            $this->resetPermissions();
+
+            $orderRemark = Registry::getRequest()->getRequestParameter('order_remark', true);
+            if ($orderRemark) {
+                $session->setVariable('ordrem', $orderRemark);
+            } else {
+                $session->deleteVariable('ordrem');
+            }
+
+            if ($basket = $session->getBasket()) {
+                $basket->setBasketUser(null);
+                $basket->onUpdate();
+            }
+
+            return true;
         } catch (UserException | ConnectionException | InputException $exception) {
             Registry::getUtilsView()->addErrorToDisplay($exception, false, true);
-
             return;
         } catch (\Throwable) {
             Registry::getUtilsView()->addErrorToDisplay('ERROR_MESSAGE_USER_UPDATE_FAILED', false, true);
             return false;
         }
-
-        $this->resetPermissions();
-
-        // order remark
-        $orderRemark = Registry::getRequest()->getRequestParameter('order_remark', true);
-
-        if ($orderRemark) {
-            $session->setVariable('ordrem', $orderRemark);
-        } else {
-            $session->deleteVariable('ordrem');
-        }
-
-        if ($basket = $session->getBasket()) {
-            $basket->setBasketUser(null);
-            $basket->onUpdate();
-        }
-
-        return true;
     }
 
     /**

diff --git a/source/Application/Controller/Admin/UserMain.php b/source/Application/Controller/Admin/UserMain.php
@@ -7,6 +7,7 @@
 
 namespace OxidEsales\EshopCommunity\Application\Controller\Admin;
 
+use OxidEsales\Eshop\Application\Model\User;
 use OxidEsales\Eshop\Core\Registry;
 use stdClass;
 use Exception;
@@ -26,7 +27,7 @@ public function render()
         parent::render();
 
         // malladmin stuff
-        $oAuthUser = oxNew(\OxidEsales\Eshop\Application\Model\User::class);
+        $oAuthUser = oxNew(User::class);
         $oAuthUser->loadAdminUser();
         $blisMallAdmin = $oAuthUser->oxuser__oxrights->value == "malladmin";
 
@@ -52,7 +53,7 @@ public function render()
         $soxId = $this->_aViewData["oxid"] = $this->getEditObjectId();
         if (isset($soxId) && $soxId != "-1") {
             // load object
-            $oUser = oxNew(\OxidEsales\Eshop\Application\Model\User::class);
+            $oUser = oxNew(User::class);
             $oUser->load($soxId);
             $this->_aViewData["edit"] = $oUser;
 
@@ -102,50 +103,46 @@ public function save()
     {
         parent::save();
 
-        //allow admin information edit only for MALL admins
         $soxId = $this->getEditObjectId();
         if ($this->allowAdminEdit($soxId)) {
             $aParams = Registry::getRequest()->getRequestEscapedParameter("editval");
 
-            // checkbox handling
             if (!isset($aParams['oxuser__oxactive'])) {
                 $aParams['oxuser__oxactive'] = 0;
             }
 
-            $oUser = oxNew(\OxidEsales\Eshop\Application\Model\User::class);
+            $oUser = oxNew(User::class);
             if ($soxId != "-1") {
                 $oUser->load($soxId);
             } else {
                 $aParams['oxuser__oxid'] = null;
             }
 
-            //setting new password
             if (($sNewPass = Registry::getRequest()->getRequestEscapedParameter("newPassword"))) {
                 $oUser->setPassword($sNewPass);
             }
 
-            //FS#2167 V checks for already used email
-
-            if (isset($aParams['oxuser__oxusername']) && $oUser->checkIfEmailExists($aParams['oxuser__oxusername'])) {
+            if (
+                isset($aParams['oxuser__oxusername'])
+                && ($aParams['oxuser__oxusername'] !== $oUser->getRawFieldData('oxusername'))
+                && $oUser->isEmailInUse($aParams['oxuser__oxusername'])
+            ) {
                 $this->_sSaveError = 'EXCEPTION_USER_USEREXISTS';
 
                 return;
             }
 
             $oUser->assign($aParams);
 
-            //seting shop id for ONLY for new created user
             if ($soxId == "-1") {
                 $this->onUserCreation($oUser);
             }
 
-            // A. changing field type to save birth date correctly
             $oUser->oxuser__oxbirthdate->fldtype = 'char';
 
             try {
                 $oUser->save();
 
-                // set oxid if inserted
                 $this->setEditObjectId($oUser->getId());
             } catch (Exception $oExcp) {
                 $this->_sSaveError = $oExcp->getMessage();
@@ -168,9 +165,9 @@ protected function calculateAdditionalRights($userRights)
     /**
      * Additional actions on user creation.
      *
-     * @param \OxidEsales\Eshop\Application\Model\User $user
+     * @param User $user
      *
-     * @return \OxidEsales\Eshop\Application\Model\User
+     * @return User
      */
     protected function onUserCreation($user)
     {

diff --git a/source/Application/Model/User.php b/source/Application/Model/User.php
@@ -20,6 +20,7 @@
 use OxidEsales\EshopCommunity\Core\Di\ContainerFacade;
 use OxidEsales\EshopCommunity\Internal\Domain\Authentication\Bridge\PasswordServiceBridgeInterface;
 use OxidEsales\EshopCommunity\Internal\Domain\Authentication\Bridge\RandomTokenGeneratorBridgeInterface;
+use OxidEsales\EshopCommunity\Internal\Framework\Database\QueryBuilderFactoryInterface;
 
 /**
  * User manager.
@@ -1711,55 +1712,79 @@ protected function update()
     /**
      * Checks for already used email
      *
-     * @param string $sEmail user email/login
+     * @param string $email user email/login
      *
      * @return bool
      */
-    public function checkIfEmailExists($sEmail)
+    public function checkIfEmailExists($email)
     {
-        $myConfig = \OxidEsales\Eshop\Core\Registry::getConfig();
-        // We force reading from master to prevent issues with slow replications or open transactions (see ESDEV-3804).
-        $masterDb = \OxidEsales\Eshop\Core\DatabaseProvider::getMaster();
-        $iShopId = $myConfig->getShopId();
-        $blExists = false;
+        if (!$email) {
+            return false;
+        }
 
-        $sQ = 'select oxshopid, oxrights, oxpassword from oxuser where oxusername = :oxusername';
-        $params = [
-            ':oxusername' => (string) $sEmail
-        ];
-        if (($sOxid = $this->getId())) {
-            $sQ .= " and oxid <> :notoxid";
-            $params[':notoxid'] = $sOxid;
+        $config = Registry::getConfig();
+        $masterDb = DatabaseProvider::getMaster();
+        $shopId = $config->getShopId();
+
+        $query = 'SELECT oxshopid, oxrights, oxpassword FROM oxuser WHERE oxusername = :oxusername';
+        $params = [':oxusername' => (string) $email];
+
+        if ($id = $this->getId()) {
+            $query .= " AND oxid <> :notoxid";
+            $params[':notoxid'] = $id;
         }
-        $oRs = $masterDb->select($sQ, $params);
-        if ($oRs != false && $oRs->count() > 0) {
-            if ($this->_blMallUsers) {
-                $blExists = true;
-                if ($oRs->fields[1] == 'user' && !$oRs->fields[2]) {
-                    // password is not set - allow to override
-                    $blExists = false;
-                }
-            } else {
-                $blExists = false;
-                while (!$oRs->EOF) {
-                    if ($oRs->fields[1] != 'user') {
-                        // exists admin with same login - must not allow
-                        $blExists = true;
-                        break;
-                    } elseif ($oRs->fields[0] == $iShopId && $oRs->fields[2]) {
-                        // exists same login (with password) in same shop
-                        $blExists = true;
-                        break;
-                    }
 
-                    $oRs->fetchRow();
-                }
+        $result = $masterDb->select($query, $params);
+
+        if (!$result || $result->count() === 0) {
+            return false;
+        }
+
+        if ($this->_blMallUsers) {
+            if ($result->fields[1] === 'user' && !$result->fields[2]) {
+                return false; // No password set - allow override
+            }
+            return true;
+        }
+
+        while (!$result->EOF) {
+            if ($result->fields[1] !== 'user') {
+                return true; // Admin user exists
             }
+
+            if ($result->fields[0] == $shopId && ($result->fields[2])) {
+                return true; // User exists in same shop with password
+            }
+
+            $result->fetchRow();
         }
 
-        return $blExists;
+        return false;
     }
 
+    public function isEmailInUse(string $email): bool
+    {
+        $queryBuilder = ContainerFacade::get(QueryBuilderFactoryInterface::class)->create();
+        $queryBuilder
+            ->select('1')
+            ->from('oxuser')
+            ->where('oxusername = :email')
+            ->andWhere('oxshopid = :shopId')
+            ->setParameters([
+                'email' => $email,
+                'shopId' => Registry::getConfig()->getShopId()
+            ]);
+
+        if ($this->getId()) {
+            $queryBuilder
+                ->andWhere('oxid != :currentUserId')
+                ->setParameter('currentUserId', $this->getId());
+        }
+
+        return (bool) $queryBuilder->execute()->fetchOne();
+    }
+
+
     /**
      * Returns user recommendation list object
      *

diff --git a/source/Application/translations/de/lang.php b/source/Application/translations/de/lang.php
@@ -163,7 +163,7 @@
 'ERROR_MESSAGE_USER_NOVALUES'                                 => 'E-Mail und Passwort müssen ausgefüllt sein!',
 'ERROR_MESSAGE_USER_USERCREATIONFAILED'                       => 'Fehler beim Anlegen des Benutzers!',
 'ERROR_MESSAGE_USER_UPDATE_FAILED'                            => 'Fehler beim Aktualisieren der Benutzerdaten!',
-'ERROR_MESSAGE_USER_USEREXISTS'                               => '%s konnte nicht registriert werden. Haben Sie bereits ein Kundenkonto bei uns?',
+'ERROR_MESSAGE_USER_USEREXISTS'                               => 'Bei dieser E-Mail-Adresse ist ein Fehler aufgetreten. Bitte wenden Sie sich an unseren Support.',
 'ERROR_MESSAGE_VERSION_EXPIRED1'                              => 'Ihre Version ist leider abgelaufen. Bitte kontaktieren Sie',
 'ERROR_MESSAGE_VOUCHER_INCORRECTPRICE'                        => 'Einkaufswert ist zu niedrig für diesen Gutschein!',
 'ERROR_MESSAGE_VOUCHER_ISRESERVED'                            => 'Gutschein ist reserviert!',

diff --git a/source/Application/translations/en/lang.php b/source/Application/translations/en/lang.php
@@ -163,7 +163,7 @@
 'ERROR_MESSAGE_USER_NOVALUES'                                 => 'E-mail address and password have to be entered!',
 'ERROR_MESSAGE_USER_USERCREATIONFAILED'                       => 'Error creating the user!',
 'ERROR_MESSAGE_USER_UPDATE_FAILED'                            => 'Error while updating the user data!',
-'ERROR_MESSAGE_USER_USEREXISTS'                               => 'Not possible to register %s. Maybe you have already registered?',
+'ERROR_MESSAGE_USER_USEREXISTS'                               => 'An error has occurred with this e-mail address. Please contact our support.',
 'ERROR_MESSAGE_VERSION_EXPIRED1'                              => 'Your version is expired. Please contact',
 'ERROR_MESSAGE_VOUCHER_INCORRECTPRICE'                        => 'The total price is too low for this coupon!',
 'ERROR_MESSAGE_VOUCHER_ISRESERVED'                            => 'This coupon is reserved!',