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

UI edge cases are not accomodated #63

Open
2 tasks
maxime-rainville opened this issue Apr 15, 2021 · 4 comments
Open
2 tasks

UI edge cases are not accomodated #63

maxime-rainville opened this issue Apr 15, 2021 · 4 comments

Comments

@maxime-rainville
Copy link

maxime-rainville commented Apr 15, 2021

The SessionLoginField doesn't currently cater to the following edge cases:

  • Member without any LoginSession
  • ReadOnly view
  • LoginSession for which you do not have Delete rights

Acceptance criteria

  • If a site customises the can view permission on LoginSession, the Login Session field is displayed when you are allowed to view session for a given user.
  • If you don't have permission to view a LoginSession, but not to delete it, no-login delete button is displayed.

@silverstripeux

@clarkepaul
Copy link

I think that for all those situations it makes sense. @maxime-rainville for these scenarios does the section appear as empty or is it completely removed?

I would imagine there would be a message saying there is no login sessions or that area would be completely missing. My preference is that the area is present with a message but that would mean potentially different messages for different cases unless we can create a generic one.

@maxime-rainville
Copy link
Author

TLDR None of those edge cases can be reached on a vanilla install of session manager, but it could be possible if third party dev give some members permissien to manage other users' sessions.

Once #62 is merged, none of those scenarios will be possible in a vanilla install:

  • "LoginSession for which you do not have Delete rights" won't be possible because you'll only be able to see your own sessions which you can delete
  • "ReadOnly view" can't be reached because you'll always have the right to edit your own profile
  • "Member without any LoginSession" won't be possible because you need to be logged in to view your sessions, so you at least have one session.

Things will get a little more complicated if devs decide to customise the behaviour. Assuming a scenario where you've created a role that allows you to view/manage other users session:

  • If there's no session, the field will be removed from the field list, so it won't show at all. You will get this state on a new user that has never logged in.
  • If you don't have permission to delete the current session, you'll still get a "delete" link, but you'll get an error when you click on it.

@brynwhyman
Copy link

Noting that I don't see this issue being a priority to resolve before the initial beta/ stable release.

When docs are created in #57 that outline the possibility of altering the behaviour to give privileged users access to other user sessions, we could make the recommendation that both CanView and CanDelete permissions should be provided, as a read-only view is not supported.

@maxime-rainville
Copy link
Author

A quick win could just be to not display logout session when the field is read-only. That's probably a 1-2 point job and would lead to less confusion than having some convoluted explanation in the doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants