Fixed \XF\Repository\User::findValidUsers() doesn't allow setting the $recentlyActive flag

[DragonByte Tech]

Affected version

2.0.7

    /**
     * @return Finder
     */
    public function findValidUsers()
    {
        return $this->finder('XF:User')->isValidUser();
    }

Is missing the $recentlyActive flag, meaning you cannot use this function if you need to set the flag. It should be changed like so:

    /**
     * @param bool $recentlyActive
     *
     * @return Finder
     */
    public function findValidUsers($recentlyActive = false)
    {
        return $this->finder('XF:User')->isValidUser($recentlyActive);
    }

Fillip

 

[Snog]

Is there a reason you can't use the finder? Something like this...

$finder = \XF::finder('XF:User');

$finder->isValidUser()
   ->where('last_activity', '>=', time() - (86400))
   ->order('last_activity', 'DESC')
   ->limit($limit);

Or

$finder = \XF::finder('XF:User');

$finder
   ->isRecentlyActive(10)
   ->isValidUser();

 

[S Thomas]

Isn't that a job for a finder collection filter?

$users = $repo->findValidUsers();
$usersFiltered = $users->filter(function(\XF\Entity\User $user) {
    return $user->recentlyActive;
});

 

[DragonByte Tech]

Is there a reason you can't use the finder?

Of course I can just manually call the recentlyActive method on the finder, but why do with two lines what should be doable with one?

It's a simple oversight in the API, surely.

Isn't that a job for a finder collection filter?

$users = $repo->findValidUsers();
$usersFiltered = $users->filter(function(\XF\Entity\User $user) {
    return $user->recentlyActive;
});

That would fetch way more data than what's needed. There's no reason why you shouldn't be able to set the $recentlyActive variable in the repository method as well, to pass it through to the finder method being called.


Fillip

 

[Chris D]

We'd actually find it difficult to change the API here because that would technically be a breaking change for any code currently extending that method. So it would require either a brand new method that rolls both into one or just accept that the method there doesn't support that and do this, instead:

$validUsers = $this->repository('XF:User')->findValidUsers()->isRecentlyActive()->fetch()

So, still do-able with a single line (or two, depending on how you like to separate chained method calls).

 

[Snog]

It could be just me doing something wrong, but from my tests using the finder seems to be more memory efficient than using the repository method. Especially on large scale responses.

As usual, I claim insanity if I'm wrong.

 

[Chris D]

There's a new findRecentlyActiveValidUsers method in the User repo in XF 2.0.10.

public function findRecentlyActiveValidUsers()
{
   return $this->finder('XF:User')->isValidUser(true);
}