XF 2.1 Multiple actions that use same functions

Ozzy47

Ozzy47

Well-known member
I am working on an addon that will allow users to register using existing emails or change their emails to an existing email.
I extended XF\Entity\User and the two functions that are changed is verifyEmail(&$email) and _preSave(). Here is my code that is working and allowing the use of an existing email in both instances.

PHP: 
<?php

namespace OzzModz\RequireUnique\XF\Entity;

class User extends XFCP_User
{
     protected function verifyEmail(&$email)
    {
          $options = \XF::options();
     
        $existingUser = $this->finder('XF:User')->where('email', $email)->fetchOne();
     
        if ($existingUser && $existingUser['user_id'] != $this->user_id)
        {
            return true;
        }
     
          return true;
     
          return parent::verifyEmail();
    }

      protected function _preSave()
    {
          $options = \XF::options();
     
          if (!$this->secret_key)
        {
            $this->secret_key = \XF::generateRandomString(32);
        }
     
        if ($this->isChanged('email') && $this->email && empty($this->_errors['email']))
        {
            // Redo the duplicate email check. This tries to reduce a race condition that can be extended
            // due to third-party spam checks.
            $matchUserId = $this->db()->fetchOne("
                SELECT user_id
                FROM xf_user
                WHERE email = ?
            ", $this->email);
            if ($matchUserId && (!$this->user_id || $matchUserId != $this->user_id))
            {
                  return true;
            }
        }
          return parent::_preSave();
    }
}

Both of those have to run in order to allow someone to register using an existing email or change their email to an existing one.

Now I have two options, Allow Registration and Allow change. The first one, ozzmodz_RequireUnique_registration controls if a user can register using an existing email and the second one, ozzmodz_RequireUnique_change controls if a user can change their email to an existing email.

Problem I am facing is if I set the options in any of the if statements, the code won't fire properly because the functions are both needed in each option. Basically what I need is a way to turn off the one or the other without affecting the other one.

I am probably missing something really simple, but the logic I need to follow is escaping me. :(
 
Lemme explain a different way.

I need that code to fire if the option ozzmodz_RequireUnique_registration is set to yes and someone is registering. I need that code to fire if the option ozzmodz_RequireUnique_change is set to yes and someone is changing their email.

Now of course one option could be off, and the other on, then I would need the code to fire on the option that is on, and not on the one that is off.
 
I think if you use an if statement and check if it's a new user with $this->isInsert() that should do what you want.

IE: if (($options->ozzmodz_RequireUnique_registration && $this->isInsert()) || ($options->ozzmodz_RequireUnique_change && !$this->isInsert()))

There is also a $this->isUpdate() check you could use.

DISCLAIMER: I could be wrong. ;)
 
Hmmm, either that is not working as intended, or my if statements are in the wrong place.

PHP: 
<?php

namespace OzzModz\RequireUnique\XF\Entity;

class User extends XFCP_User
{
     protected function verifyEmail(&$email)
    {
          $options = \XF::options();
      
        $existingUser = $this->finder('XF:User')->where('email', $email)->fetchOne();
      
        if ($existingUser && $existingUser['user_id'] != $this->user_id)
        {
              if (($options->ozzmodz_RequireUnique_registration && $this->isUpdate()) || ($options->ozzmodz_RequireUnique_change && !$this->isUpdate()))
            {
                return true;
            }else{
                  $this->error(\XF::phrase('email_addresses_must_be_unique'), 'username');

            }
        }
      
          return true;
      
          return parent::verifyEmail();
    }
 
      protected function _preSave()
    {
          $options = \XF::options();
      
          if (!$this->secret_key)
        {
            $this->secret_key = \XF::generateRandomString(32);
        }
      
        if ($this->isChanged('email') && $this->email && empty($this->_errors['email']))
        {
            // Redo the duplicate email check. This tries to reduce a race condition that can be extended
            // due to third-party spam checks.
            $matchUserId = $this->db()->fetchOne("
                SELECT user_id
                FROM xf_user
                WHERE email = ?
            ", $this->email);
            if ($matchUserId && (!$this->user_id || $matchUserId != $this->user_id))
            {
                if (($options->ozzmodz_RequireUnique_registration && $this->isUpdate()) || ($options->ozzmodz_RequireUnique_change && !$this->isUpdate()))
                {
                    return true;
                }else{
                    $this->error(\XF::phrase('email_addresses_must_be_unique'), 'username');
                }
            }
        }
          return parent::_preSave();
    }
}

If ozzmodz_RequireUnique_registration is set to no, users still can use an existing email. If ozzmodz_RequireUnique_change is set to no, users upun changin their email to an existing one are not shown the error, but the email does not change.

Note, it is the same result using $this->isInsert() or $this->isUpdate() .
 
TLDR and maybe just this should do it:

PHP: 
    /**
     * @param bool|null $isVerified
     */
    protected function handleUniqueEmail(bool &$isVerified = null) : void
    {
        if (!$this->app()->options()->ozzmodz_RequireUnique_registration)
        {
            $emailError = $this->_errors['email'] ?? null;
            if ($emailError && $emailError instanceof \XF\Phrase && $emailError->getName() === 'email_addresses_must_be_unique')
            {
                $isVerified = true;
                unset($this->_errors['email']);
            }
        }
    }
   
    /**
     * @param string $email
     *
     * @return bool
     */
    protected function verifyEmail(&$email)
    {
        $isVerified = parent::verifyEmail($email);

        if (!$isVerified)
        {
            $this->handleUniqueEmail($isVerified);
        }

        return $isVerified;
    }
   
    protected function _preSave()
    {
        parent::_preSave();
       
        $this->handleUniqueEmail();
    }

Edit: Made a change
 
Thanks for replying. Unfortunately that does not work, and it only covers registration, not change.
 
This works:
PHP: 
    /**
     * @param string|\XF\Phrase $message
     * @param null|string       $key
     * @param bool              $specificError
     */
    public function error($message, $key = null, $specificError = true)
    {
        if (!$this->app()->options()->ozzmodz_RequireUnique_registration && $message instanceof \XF\Phrase && $message->getName() === 'email_addresses_must_be_unique')
        {
            return;
        }

        parent::error($message, $key, $specificError);
    }

Edit: Yeah but no it doesn't...
 
Last edited:
Handling the verifyEmail function is fairly easy. The problems are going to come with the _presave function.

The errors there are keyed to the username...
$this->error(\XF::phrase('email_addresses_must_be_unique'), 'username');
$this->error(\XF::phrase('usernames_must_be_unique'), 'username');

So in order to reset the email error, you would have to completely clear the "username" error. Which wouldn't be a good idea in the event a user name is duplicated.

The only way I can think of getting around that is to completely replace the _presave function (with all of it's code and your changed code) and not call the parent. Which MIGHT violate the resource standards.

I say "MIGHT" because the _presave function doesn't return anything except errors. I think @Chris D might shed some better light on this.

Anyway, this should take care of your verifyEmail function...
Code: 
protected function verifyEmail(&$email)
{
    $options = \XF::options();

    $emailVerified = parent::verifyEmail($email);

    $existingUser = $this->finder('XF:User')->where('email', $email)->fetchOne();

    if ($existingUser && $existingUser['user_id'] != $this->user_id)
    {
        // ASSUMING ozzmodz_RequireUnique_change IS NOT SELECTED
        // THEREFORE A UNIQUE EMAIL IS NOT NEEDED
        if (!$options->ozzmodz_RequireUnique_change && $this->isUpdate())
        {
            if(isset($this->_errors['email']))
            {
                unset($this->_errors['email']);
            }

            $emailVerified = true;
        }

        // ASSUMING $options->ozzmodz_RequireUnique_registration IS NOT SELECTED
        // THEREFORE A UNIQUE EMAIL IS NOT NEEDED
        if (!$options->$options->ozzmodz_RequireUnique_registration && !$this->isUpdate())
        {
            if(isset($this->_errors['email']))
            {
                unset($this->_errors['email']);
            }

            $emailVerified = true;
        }
    }

    return $emailVerified;
}

That is untested and you may need to change the RequireUnique_registration to this;
if (!$options->$options->ozzmodz_RequireUnique_registration && $this->isInsert())
 
Last edited:
Thanks, I’ll check this out when I get off of work, assuming it will not violate the resource standards.
 
Just remember in my comments about the NOT SELECTED, you also may need to change that to meet your logic...

$options->ozzmodz_RequireUnique_change
$options->ozzmodz_RequireUnique_registration
 
With the help of @Snog it is finally working after sorting the logic.
PHP: 
<?php

namespace OzzModz\RequireUnique\XF\Entity;

class User extends XFCP_User
{
    protected function verifyEmail(&$email)
    {
        $options = \XF::options();

        $emailVerified = parent::verifyEmail($email);

        $existingUser = $this->finder('XF:User')->where('email', $email)->fetchOne();

        if ($existingUser && $existingUser['user_id'] != $this->user_id)
        {
            $x = $options->ozzmodz_RequireUnique_change;
            $y = $options->ozzmodz_RequireUnique_registration;

            if ($x && $this->isUpdate())
            {
                if(isset($this->_errors['email']))
                {
                    unset($this->_errors['email']);
                }

                $emailVerified = true;
            }

            if ($y && !$this->isUpdate())
            {
                if(isset($this->_errors['email']))
                {
                    unset($this->_errors['email']);
                }

                $emailVerified = true;
            }
        }

        return $emailVerified;
    }

    protected function _preSave()
    {
        $options = \XF::options();

        if ($this->isChanged('user_group_id') || $this->isChanged('secondary_group_ids'))
        {
            $groupRepo = $this->getUserGroupRepo();
            $this->display_style_group_id = $groupRepo->getDisplayGroupIdForUser($this);
        }

        if ($this->isChanged(['user_group_id', 'secondary_group_ids']))
        {
            // Do not allow a primary user group also be a secondary user group.
            $this->secondary_group_ids = array_diff(
                $this->secondary_group_ids,
                [$this->user_group_id]
            );
        }

        if (!$this->secret_key)
        {
            $this->secret_key = \XF::generateRandomString(32);
        }

        if ($this->isInsert() && !$this->isChanged('email') && empty($this->_errors['email']))
        {
            $this->email = '';
        }

        if ($this->isChanged('email') && $this->email && empty($this->_errors['email']))
        {
            // Redo the duplicate email check. This tries to reduce a race condition that can be extended
            // due to third-party spam checks.
            $matchUserId = $this->db()->fetchOne("
                        SELECT user_id
                        FROM xf_user
                        WHERE email = ?
                    ", $this->email);

            if ($matchUserId && (!$this->user_id || $matchUserId != $this->user_id))
            {
                $x = $options->ozzmodz_RequireUnique_change;
                $y = $options->ozzmodz_RequireUnique_registration;

                if (!$x && $this->isUpdate())
                {
                    $this->error(\XF::phrase('email_addresses_must_be_unique'), 'username');
                }

                if (!$y && !$this->isUpdate())
                {
                    $this->error(\XF::phrase('email_addresses_must_be_unique'), 'username');
                }
            }
        }

        if ($this->isChanged('username') && empty($this->_errors['username']))
        {
            // Redo the duplicate name check. This tries to reduce a race condition that can be extended
            // due to third-party spam checks.
            $matchUserId = $this->db()->fetchOne("
                        SELECT user_id
                        FROM xf_user
                        WHERE username = ?
                    ", $this->username);

            if ($matchUserId && (!$this->user_id || $matchUserId != $this->user_id))
            {
                $this->error(\XF::phrase('usernames_must_be_unique'), 'username');
            }
        }
    }
}

Now that it is working, it would be nice to get clarification from @Chris D if it is okay to replace the entire _preSave function in an instance like this.
 
ozzy47 said:
Now that it is working, it would be nice to get clarification from @Chris D if it is okay to replace the entire _preSave function in an instance like this.
Click to expand...
Not in the slightest, unfortunately.

While I don't have any specific advice to give, you can't replace methods in this way because it prevents changes we make to the code in the future from working and depending on where your code is executed, it would prevent other add-on code from running too.
 
Chris D said:
Not in the slightest, unfortunately.

While I don't have any specific advice to give, you can't replace methods in this way because it prevents changes we make to the code in the future from working and depending on where your code is executed, it would prevent other add-on code from running too.
Click to expand...

Thanks for replying. I was hoping for a "Yes" but I fully expected it was not allowed. :(
 
You must log in or register to reply here.
Top Bottom