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

Hide private emails in the adminops user list #104

Merged
merged 2 commits into from
Apr 5, 2021
Merged

Hide private emails in the adminops user list #104

merged 2 commits into from
Apr 5, 2021

Conversation

alice-blue
Copy link
Contributor

@alice-blue alice-blue commented Mar 25, 2021

Related to iftechfoundation/ifdb-suggestion-tracker#46 and iftechfoundation/ifdb-suggestion-tracker#265

In the user list on the adminops page, hide user's private email addresses behind a spoiler button. (The background color of the spoiler button doesn't exactly go with this table, with its alternating color rows, but I don't know what to do about that.)

Related to iftechfoundation/ifdb-suggestion-tracker#46

In the user list on the adminops page, hide user's private email addresses behind a spoiler button. (The background color of the spoiler button doesn't exactly go with this table, with its alternating color rows, but I don't know what to do about that.)
Fix code spacing
@dfabulich
Copy link
Collaborator

I think this is a good start, but the email address is still showing in so many places, it might only be a first step. e.g. if you search for users, the email is showing there http://localhost:8080/adminops?finduser=rick&go=Find and of course if you click on any user, the email is showing there, too.

By contrast, for example, Discourse hides the email addresses in all of those places; it requires a click to see them.

Should I just merge this as-is? Or should we do more work first?

@dfabulich
Copy link
Collaborator

There was a tiny bit of inconclusive discussion about this with the moderators. I currently think that this is basically the right thing, and so I'll merge it tomorrow-ish unless I hear outcry.

I do mildly prefer a version that allows you to click one button to “Show all emails” in the report, especially if you are searching the user list by a partial email address or something. But that could be done in a separate PR, if ever.

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

Successfully merging this pull request may close these issues.

None yet

2 participants