craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

[4.x]: Users query not correctly filtering by status #11322

Closed k7y6t5 closed 2 years ago

k7y6t5 commented 2 years ago

What happened?

Description

When updating from Craft 3 to 4, user queries are not correctly filtering by status.

According to the docs, active is the default. https://docs.craftcms.com/api/v4/craft-elements-db-userquery.html#method-status

Steps to reproduce

Scenario A: {% set people = craft.users.all() %} Scenario B: {% set people = craft.users.status('active').all() %}

<table>
    {% for o in people %}
        <tr>
            <td>{{ o.name }}</td>
            <td>{{ o.status }}</td>
        </tr>
    {% endfor %}
</table>

Expected behavior

Scenario A: The table should contain only users with status active (based on active being the default). Scenario B: The table should contain only users with status active.

Actual behavior

Scenario A: The table contains users with any of the statuses active, inactive, suspended, pending. Scenario B: The table contains users with status active and users with status suspended.

Craft CMS version

4.0.3

PHP version

8.0.19

Operating system and version

Ubuntu 20.04.4 LTS

Database type and version

MySQL 8.0.29

Image driver and version

Imagick 3.7.0 (ImageMagick 6.9.10-23)

Installed plugins and versions

Asset Usage 3.0.0-beta.1 Cookies 4.0.0 Feed Me 5.0.4 Redactor 3.0.0
Super Table 3.0.0-beta.4

brandonkelly commented 2 years ago

Thanks for pointing that out. It is intended that user queries don’t apply a status by default in Craft 4; the docs were just not updated accordingly. The docs will fix themselves after the next release gets tagged.

k7y6t5 commented 2 years ago

Ok thanks @brandonkelly. One follow-up though. {% set people = craft.users.status('active').all() %} returns both active and suspended users. Is that intended?

brandonkelly commented 2 years ago

Yeah that’s expected. active just means that they have the means to log in (they either have a password already, or they could create/recover a password from a lost-password workflow.

brandonkelly commented 2 years ago

Realized there was an undocumented change in behavior here, and the consensus internally is that we should retain the old behavior where active doesn’t include suspended users. So that has been reverted for the next release.

k7y6t5 commented 2 years ago

Huzzah! Thank you 🎉

brandonkelly commented 2 years ago

Craft 4.0.4 is out now with that change ✨