processwire / processwire-requests

ProcessWire feature requests.
39 stars 0 forks source link

users()->has($selector, $verbose = false) #528

Open JanRomero opened 1 week ago

JanRomero commented 1 week ago

Short description of the enhancement

pages() exposes a method has() to quickly check if a matching page exists. This is currently lacking for the users() object. Having such an analogous method would strenghen the coherence of PW’s API, improve code clarity in users’ projects, and potentially prevent subtle bugs in setups with custom user templates or parents.

Optional: Steps that explain the enhancement

Implementation could be as simple as adding this method to wire/core/Users.php:

public function has($selector, $verbose = false) {
    return $this->wire()->pages->has($this->selectorString($selector), $verbose);
}

It would be interesting to add the method to PagesType, but this leads to a conflict with Permissions, which unfortunately also exposes a method by this name, but with a different signature:

Fatal error: Declaration of ProcessWire\Permissions::has($name) must be compatible with ProcessWire\PagesType::has($selector, $verbose = false)

It would actually be kind of sweet to fully unify this API. In fact, since has() is already able to take a raw page name in place of a selector, this may be possible without any breakage. (Unfortunately permissions()->has() always returns a bool and changing it would break people’s strict identity comparisons).

Current vs. suggested behavior

Currently to check if a user exists, for instance during sign-up, one has these options as far as I’m aware:

//loads unnecessary data, intent not quite as clearly as has(),
//requires verbose sanitization because array syntax is unavailable,
//also NullPage is not falsy
if (users->get("name={$sanitizer->selectorValue($username)}")->id)
//quickly becomes overly complicated when using custom user templates/parents
if (pages()->has(['template' => 'user', 'name' => $username]))

Why would the enhancement be useful to users?

Knowing one is supposed to use has() to check page existance, one is tempted to assume the same applies to users. This suggestion would make the API more intuitive by unifying it and would allow one to write the following without having to worry about custom parents and templates:

if (users->has(['name' => $username]))
    die('Sorry, that name is already taken');

Thanks!

JanRomero commented 1 week ago

Indeed I just noticed not even pages()->get() and users()->get() are unified as the latter only takes a selector string. That’s a separate issue, but further illustrates the problem with using get() as a workaround, because it clutters the code with more sanitization. (In my pagename examples above, this is less urgent, but one may want to allow nice-looking user names potentially containing commas, for example, necessitating sanitization just for the sake of the selector).

adrianbj commented 1 week ago

On a similar note, it would be great to have a findMany method for Users as well.