Opened 5 years ago

Closed 15 months ago

# [Patch] Provide indicator for verification status of email addresses on the Manage User Accounts page

Reported by: Owned by: Ryan J Ollos Steffen Hoffmann normal AccountManagerPlugin minor web-UI CSS

### Description

The verification status of an email address for an individual user can be seen by navigating to the Account Details page. However, on installations with many users, it would be nice to be able to see which accounts haven't been verified while viewing the Manage User Accounts page.

The initial idea I have for this is to the use the light grey color that is used for missing permissions (see t:#10752) on the Manage Permissions and Groups page. How does that sound? Any better ideas?

### comment:1 Changed 5 years ago by Steffen Hoffmann

Keywords: web-UI CSS added normal → minor

I agree.

In fact I'm already working on related improvements for the account table. But this is WiP and dedicated to adding some more tiny icons like for account locking. Suggestions welcome. A gray-out looks like a good one.

### comment:2 Changed 5 years ago by Ryan J Ollos

Thanks for the quick feedback. The patch in t:#10742 just applied the missing class to inactive permissions. This is the same class used for missing tickets, wiki pages, etc. So the first thought I had was to do the same for email addresses that haven't been validated and I'll work up a patch for this.

### comment:3 Changed 5 years ago by Ryan J Ollos

The icons for locked accounts sound very useful, btw. Looking forward to seeing that.

### comment:5 Changed 5 years ago by Ryan J Ollos

It looks like fetch_user_data could be called in _do_acct_details to eliminate some redundant code. I'll investigate that in another ticket later on.

### Changed 5 years ago by Ryan J Ollos

Patch against r12500 of the trunk.

### comment:6 Changed 5 years ago by Ryan J Ollos

Summary: Provide indicator for verification status of email addresses on the Manage User Accounts page → [Patch] Provide indicator for verification status of email addresses on the Manage User Accounts page

### comment:7 Changed 5 years ago by Steffen Hoffmann

This looks like a good start, but because users with unverified account are treated similar to users in an anonymous session, I'll alter this into graying text of the whole row.

And the upcoming account approval/ban feature will use the same representation, probably with different tool-tip (title HTML tag). And different icon, that is what I've been working on before diving more into substantial enhancements with AcctMgr lately.

### comment:8 follow-up:  9 Changed 5 years ago by Ryan J Ollos

I guess that graying out the whole row is okay. I rather liked the idea of just having an explicit indicator on the email, but I'm not too wed to either approach.

### comment:9 in reply to:  8 Changed 5 years ago by Steffen Hoffmann

I guess that graying out the whole row is okay. I rather liked the idea of just having an explicit indicator on the email, but I'm not too wed to either approach.

Seen that and I like the idea too, but OTOH this might suggest an almost usable account, while quite the contrary is true: Until email is verify, there's hardly a difference to an unauthenticated session, given all elevated permissions are wiped for each request.

### comment:10 follow-up:  12 Changed 5 years ago by Ryan J Ollos

I've implemented the suggestion of graying out the entire row in th10741-r12501-1.patch. The patch also implements #10743, and adds a title to the link for reviewing user attributes. Now that I've seen the change in action, I do prefer graying out the entire row rather than just the email address.

### comment:11 Changed 5 years ago by Ryan J Ollos

I'm thinking that eventually we should replace the link on the account name with a More details icon in the far right column.

Alternatively, we could drop the Review user account details page and show those details on the Manage user accounts page when expanding a More info section. Here is an example from a site that I use:

I'm partial to the latter idea at the moment, though it looks like it will only work when JavaScript is enabled, so we'd probably want to leave the Review user account details in place as a fallback when JavaScript is not enabled.

### comment:12 in reply to:  10 Changed 5 years ago by Steffen Hoffmann

Now that I've seen the change in action, I do prefer graying out the entire row rather than just the email address.

I'm glad that we agree on this, but I've been unable to reproduce the gray-out be adding 'missing' class to 'tr' HTML tag. While I succeeded after adding a custom CSS rule, it still doesn't apply to the link labels, so could you investigate, where this CSS is coming from in your setup, please?

### comment:13 follow-up:  14 Changed 5 years ago by Ryan J Ollos

Yeah, I'll investigate. I'm not very proficient with CSS, so I wouldn't be surprised if the rule could be coded better. What version(s) of Trac did you test with?

### comment:14 in reply to:  13 Changed 5 years ago by Steffen Hoffmann

Yeah, I'll investigate. I'm not very proficient with CSS, so I wouldn't be surprised if the rule could be coded better. What version(s) of Trac did you test with?

I've been testing with Trac-1.0 by now, but I'm fine with a few extra CSS rules in acct_mgr.css, and it's actually working fine now. So don't dig for it too long - just make sure to test, if my final version is working for you as well.

### comment:15 follow-up:  16 Changed 5 years ago by Ryan J Ollos

Am I reading your comment correct then that it works with the CSS added in admin.css, and just didn't have that included in your initial testing?

I was about to post the following:

I've tested with several browsers on Debian 6 with Trac 1.1.1dev-r11483: Chrome 23, FF 17, Epiphany 3.4.2.

Here is what I see in Chrome 23:

### comment:16 in reply to:  15 Changed 5 years ago by Steffen Hoffmann

Am I reading your comment correct then that it works with the CSS added in admin.css, and just didn't have that included in your initial testing?

Sorry, I had overseen that, because I just extracted parts of the proposed changes at a time, so nevermind.

### comment:17 Changed 5 years ago by Steffen Hoffmann

I like the idea about moving the link to a dedicated column and label.

But the further it goes into implementation detail the more I get discouraged by the growing complexity. As witnessed with the email link issue (#10743) AcctMgr has already quite complex code structures, and I'd like to keep it at a reasonable level for now.

I don't say 'Never', but these changes won't happen in the near future. For now it works well, and the only thing that could force that change would be the introduction of some dedicated user page, like we have it at t-h.o, but this is not yet a generally agreed pattern.

### comment:18 Changed 5 years ago by Ryan J Ollos

That's fine. I'll probably be doing custom templates for bloodhound anyway, and can just keep the work over there if you don't wish to include it in AccountManager core.

### comment:19 Changed 5 years ago by Steffen Hoffmann

(In [12505]) AccountManagerPlugin: Some enhancements regarding approval/ban feature, refs #843, #8595 and #10741.

Adding the registration approval configuration option to config admin panel. Taking care for marking all potential message parts for translation as well. A recent suggestion by Ryan J Ollos is merged in here too, so that all kinds of restricted accounts are clearly visible in user listings now.

### comment:20 follow-up:  23 Changed 5 years ago by Ryan J Ollos

I've tested with Trac 1.1.1dev and everything is working well. I like that the configuration panel is starting to take shape now, with multiple options per fieldset. Before, it seemed overly boxed, but along with the change in [12506] in which two fieldsets were combined, it is starting to look nice, and logically organized.

I'll have to remember to take care in the future for translations. So far I have trained myself to remember to do this in Python code, but frequently tend to forget when working in the templates.

### comment:21 Changed 5 years ago by Ryan J Ollos

Here is a potential issue with [12506]. The system-message wasn't moved when the Password Reset fieldset was moved:

### comment:22 follow-up:  24 Changed 5 years ago by Ryan J Ollos

Although the more I look at this, I think maybe the system-message should be displayed in the location shown in comment:21. On first look, it just appears slightly out of place, particularly since add_notice is usually used to position similar messages at the top of the page.

### comment:23 in reply to:  20 Changed 5 years ago by Steffen Hoffmann

I've tested with Trac 1.1.1dev and everything is working well. I like that the configuration panel is starting to take shape now, with multiple options per fieldset. Before, it seemed overly boxed, but along with the change in [12506] in which two fieldsets were combined, it is starting to look nice, and logically organized.

Ah well, you're welcome. It just occurred to me, that there was a more logical order, and binding related options would make it look even clearer, and I took the chance.

### comment:24 in reply to:  22 ; follow-up:  25 Changed 5 years ago by Steffen Hoffmann

Although the more I look at this, I think maybe the system-message should be displayed in the location shown in comment:21. On first look, it just appears slightly out of place, particularly since add_notice is usually used to position similar messages at the top of the page.

I agree, that this is an odd place according to all standards. No wonder, I didn't take care for it. I would rather have it above the new Refresh/Reset, or even better: inside the section, just to make clear what you did right now. Or the standard location on-top of the page, sure. But it looks a bit too far away.

### Changed 5 years ago by Steffen Hoffmann

suggestion: restart-confirmation replaced right above dedicated button

### comment:25 in reply to:  24 ; follow-up:  26 Changed 5 years ago by Ryan J Ollos

Or the standard location on-top of the page, sure. But it looks a bit too far away.

Yeah, I agree, it would be too far away at the top of the page. I like the placement you propose in 20130104_admin_acctcfg-SystemMessage.png.

In fact, I've been considering a patch to the Trac core for the plugin admin page that is patterned after your inline notices. After enabling or disabling a component, there is a notice at the top of the page, but it is usually out of site since the redirect contains a fragment that lands the user on the plugin for which the action was invoked.

### comment:26 in reply to:  25 Changed 5 years ago by Steffen Hoffmann

Or the standard location on-top of the page, sure. But it looks a bit too far away.

Yeah, I agree, it would be too far away at the top of the page. I like the placement you propose in 20130104_admin_acctcfg-SystemMessage.png.

Ok, will commit that one.

In fact, I've been considering a patch to the Trac core for the plugin admin page that is patterned after your inline notices. After enabling or disabling a component, there is a notice at the top of the page, but it is usually out of site since the redirect contains a fragment that lands the user on the plugin for which the action was invoked.

I know what you mean. This has occurred to me many times before as well with similar uneasy feelings.

### comment:27 Changed 5 years ago by Steffen Hoffmann

(In [12513]) AccountManagerPlugin: Move hash refresh procedure restart message, refs #10741.

I feel that it looks much better now, close to the command button.

### comment:28 Changed 15 months ago by Ryan J Ollos

Resolution: → fixed new → closed

### Modify Ticket

Change Properties