Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User statuses are confusing (Craft 2 and Craft 3) #3556

Closed
missmatsuko opened this issue Dec 14, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@missmatsuko
Copy link
Contributor

commented Dec 14, 2018

Description

Screenshot of 'Pending' status filter applied on Users

  1. When we filter users by status and select 'Pending' (orange), users with the suspended status (red) will appear. I'm guessing this is because these suspended users never verified their account, but it is confusing that the filter doesn't show only users whose status is set to 'Pending'.

Screenshot of an 'Unverified' user status
2. When I inspected some of these users, I found that even the users marked with orange that I thought were 'Pending' were actually 'Unverified'. I don't know where this term is coming from, since the docs below only list 'active', 'locked', 'suspended', 'pending', 'archived', and null as possible values for User Status. I did notice that there's a 'pending' class on this element, though.

Steps to reproduce

  1. Set up a Craft site and add some users
  2. Suspend some users
  3. Go to /admin/users and select the 'Pending' status

Additional info

  • Craft version: All? Observed on Craft 2 and Craft 3 sites
  • PHP version:
  • Database driver & version:
  • Plugins & versions:
@brandonkelly

This comment has been minimized.

Copy link
Member

commented Dec 14, 2018

It’s a little awkward because a user technically could be both pending and suspended (though suspending a user that hasn’t even activated their account yet, let alone done something suspension-worthy, is a little Dickish).

Considering that I would say this is expected behavior. If their account is pending, they should show up in this list, regardless of whether they’ve been suspended. If they’ve been suspended, I do think that’s an important thing to show in the Status column, even when searching for Pending users.

  1. When I inspected some of these users, I found that even the users marked with orange that I thought were 'Pending' were actually 'Unverified'.

Where are you seeing the reference to “unverified” ? Pending and unverified mean the same thing, but we should be consistent.

@missmatsuko

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2018

It’s a little awkward because a user technically could be both pending and suspended (though suspending a user that hasn’t even activated their account yet, let alone done something suspense-worthy, is a little Dickish).

Yeah, I'm not sure what the reason for suspending users was. It looks like maybe the accounts were created with spammy-looking emails, so maybe that's why they got suspended.

Considering that I would say this is expected behavior. If their account is pending, they should show up in this list, regardless of whether they’ve been suspended. If they’ve been suspended, I do think that’s an important thing to show in the Status column, even when searching for Pending users.

In this particular case, the client wants to see only Pending users who are not Suspended.

If this is the expected behaviour, then the user detail view should display both 'Pending' and 'Suspended'. Currently it looks like the user's status is only 'Suspended'.

The dot in the list-view items should also match the filter, maybe by showing both colors. Currently it seems like the filter is broken because the filter's dot color doesn't match the result dot colors.

The documentation for user status should also be updated to make it clear that a user can have multiple statuses. Is this really the case though? It seems like user status is a string, so a user shouldn't be able to have multiple statuses: Craft 3 User Model - Status

Where are you seeing the reference to “unverified” ? Pending and unverified mean the same thing, but we should be consistent.

I saw this in the user's detail page (e.g. /admin/users/26), in the right sidebar.
I'm guessing this term is being set here: UsersController.php line 606

@brandonkelly

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

In this particular case, the client wants to see only Pending users who are not Suspended.

I guess I could see why… if a user is suspended, even if they haven’t activated their account yet they’re not really “pending” anything… in fact they should get an error when they try to activate their account at that point.

The documentation for user status should also be updated to make it clear that a user can have multiple statuses. Is this really the case though? It seems like user status is a string, so a user shouldn't be able to have multiple statuses: Craft 3 User Model - Status

“Status” is a core feature for all elements. Users just happen to define their status based on multiple independent properties:

cms/src/elements/User.php

Lines 933 to 952 in 71b8848

public function getStatus()
{
if ($this->locked) {
return self::STATUS_LOCKED;
}
if ($this->suspended) {
return self::STATUS_SUSPENDED;
}
if ($this->archived) {
return self::STATUS_ARCHIVED;
}
if ($this->pending) {
return self::STATUS_PENDING;
}
return self::STATUS_ACTIVE;
}

It wouldn’t be possible to show more than once status for users without introducing a breaking change for all element types (including plugin-supplied ones), so not something we could even consider until Craft 4.

I saw this in the user's detail page (e.g. /admin/users/26), in the right sidebar.

Ah yes, thanks, will fix that.

brandonkelly added a commit that referenced this issue Dec 17, 2018

@missmatsuko

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

Thank you! Would it be possible to apply to Craft 2 as well? 🤞

brandonkelly added a commit that referenced this issue Dec 17, 2018

@brandonkelly

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

Yep done.

@missmatsuko

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

Champ!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.