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

Show inactive users warning #2994

Merged
merged 14 commits into from
Aug 2, 2018
Merged

Show inactive users warning #2994

merged 14 commits into from
Aug 2, 2018

Conversation

daniel-wer
Copy link
Member

@daniel-wer daniel-wer commented Jul 31, 2018

This PR adds an Antd Alert when there are new inactive users (<14 days). They can be activated directly from this Banner. The original issue #2928 also shows a delete button, but afaik we don't implement this functionality. Should this be added in your opinion? Otherwise there would be no way to get rid of unwanted users, as activating them would be the only option.

new-users

URL of deployed dev instance (used for testing):

Steps to test:

  • Create one or more new users. Then login again with an admin account.
  • Visit the user list and there should be an Alert notifying you of the new inactive users. Activating them should work. Deactivating them again should add them back to the Alert.

Issues:


Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Looks really good.

Maybe we should add a small information icon next to the alert title, which says something like this on hover: "The displayed users are inactive and were created in the past 14 days.". This might avoid confusion if somebody willingly disables a user and cannot dismiss the list. In that case, the admin will understand that the alert will go away after some time.

@daniel-wer
Copy link
Member Author

I like the idea with small info icon, I'll add that :)
What's your opinion on giving the admin another option, apart from activating the user? Could be a simple X Delete User. @fm3 Does the backend have a concept of deleted users and would a user deletion be reversable (otherwise we'll add a confirm)?

@philippotto
Copy link
Member

What's your opinion on giving the admin another option, apart from activating the user? Could be a simple X Delete User. @fm3 Does the backend have a concept of deleted users and would a user deletion be reversable (otherwise we'll add a confirm)?

I think we should defer this until we get feedback on that. I think it doesn't happen often, that a user is deactivated shortly after it was created.
If we want to add a possibility to discard the user, I'd do a simple "ignore" action which sets another flag. I'd consider "Deleting a user" as only a tangential feature, which we would merely exploit for this use case. In theory, it's not unrealistic that an admin wants to get the list out of his way, but doesn't care for the fact that the user is dangling around. We shouldn't force an admin to delete users just so he isn't annoyed by the UI.

@daniel-wer daniel-wer merged commit e553b90 into master Aug 2, 2018
@daniel-wer daniel-wer deleted the show-inactive-users-warning branch August 2, 2018 10:03
@daniel-wer daniel-wer mentioned this pull request Aug 7, 2018
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show a "there are new, inactive users" hint
3 participants