Opened 4 years ago

## #10743 new enhancement

Reported by: Owned by: Ryan J Ollos normal AccountManagerPlugin minor

### comment:2 follow-up:  12 Changed 4 years ago by Steffen Hoffmann

Severity: normal → minor

This might be most useful for user lists, if shown by the UserQuery macro that re-uses this template.

But there is a problem, that we don't guarantee to return valid email addresses. Note that all addresses are piped through Chrome.format_author, and we may need to at least check for tailing '...' before formatting them blindly afterwards, right?

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

Yes, both of those seem valid and neither were on my radar. I will investigate and rework the patch.

### comment:4 Changed 4 years ago by Steffen Hoffmann

Hold on, please. I'm about to push some related changes, and might finish them tonight.

I've refocused on the combined account verification concept, including new approval/ban feature, as I mentioned before. Therefore the code is significantly different from yours, and you'd better have a look at it, before investing more time.

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

Okay, I'll refocus elsewhere and wait for your changes. No rush on my part, just want to efficiently invest my time, so thanks for helping me make sure I am doing that by syncing my efforts with yours. I'll send you an email with a proposed plan for my work, so that we can better coordinate our efforts.

### comment:6 follow-up:  7 Changed 4 years ago by Ryan J Ollos

I think this feature still has value and remains unimplemented in the latest revision of the codebase. I'd care to rework the patch provided, with attention to the details noted in comment:2. Does that sound good?

Later on, it might even be nice to have the email address link to a small form, so that messages can be sent directly from Trac, thereby originating from the standard Trac notification sender email address and not requiring a client-side email client for sending messages. That seems like too much effort for now, but is a feature I would make use of if available.

### comment:7 in reply to:  6 ; follow-up:  8 Changed 4 years ago by Steffen Hoffmann

I think this feature still has value and remains unimplemented in the latest revision of the codebase. I'd care to rework the patch provided, with attention to the details noted in comment:2. Does that sound good?

Fine for me.

Later on, it might even be nice to have the email address link to a small form, so that messages can be sent directly from Trac, thereby originating from the standard Trac notification sender email address and not requiring a client-side email client for sending messages. That seems like too much effort for now, but is a feature I would make use of if available.

May I suggest to add such a feature to AnnouncerPlugin instead? That plugin might be able to provide a generic mail form on-top of every email address, given it has not been obfuscated (or even then while hiding the real address and just allowing to send), if a special permission EMAIL_* has been granted to the user. Especially that last bit, combined with SpamFilterPlugin/captcha/whatever support could make a really valuable addition. At least I see use for it the longer I think about it.

But for what its worth I would not dare to implement anything nearly as feature complete as on other platforms for now - think sent-box, internal user-to-user communication. Just keep it for starting communication - i.e. the sender must provide a valid email for reply.

### comment:8 in reply to:  7 ; follow-up:  9 Changed 4 years ago by Ryan J Ollos

May I suggest to add such a feature to AnnouncerPlugin instead? That plugin might be able to provide a generic mail form on-top of every email address, given it has not been obfuscated (or even then while hiding the real address and just allowing to send), if a special permission EMAIL_* has been granted to the user. Especially that last bit, combined with SpamFilterPlugin/captcha/whatever support could make a really valuable addition. At least I see use for it the longer I think about it.

Yes, that sounds like an excellent idea, and would allow for some other scenarios I had in the back of my mind.

But for what its worth I would not dare to implement anything nearly as feature complete as on other platforms for now - think sent-box, internal user-to-user communication. Just keep it for starting communication - i.e. the sender must provide a valid email for reply.

Yes, send-only seems like a suitable scope for the feature. By default [announcer] email_replyto could be used for setting the reply address, but we could also offer the sender the opportunity to add their own reply-to, to support cases such as multiple administrators for a Trac instance (oh, but also that the sender may not be an administrator! ... I'll need to think this through more completely).

### comment:9 in reply to:  8 ; follow-up:  10 Changed 4 years ago by Ryan J Ollos

Yes, send-only seems like a suitable scope for the feature. By default [announcer] email_replyto could be used for setting the reply address, but we could also offer the sender the opportunity to add their own reply-to, to support cases such as multiple administrators for a Trac instance (oh, but also that the sender may not be an administrator! ... I'll need to think this through more completely).

But of course we could just use the email address that the user has set in their preferences as the reply-to, which would make even more sense. This was probably obvious to you from the start!

### comment:10 in reply to:  9 Changed 4 years ago by Steffen Hoffmann

But of course we could just use the email address that the user has set in their preferences as the reply-to, which would make even more sense. This was probably obvious to you from the start!

Well, this has been a strong option. Now, that you talk about it, it might be good to require it (or even require it verified verified) before allowing to use this feature at all. Not only saves an extra email address input field in the email sent form, but provides an essential protection against foreseeable abuse.

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

I've been using Git a lot lately, posting my Trac patches to GitHub and very recently (#11071) even started posting patches for trac-hacks plugins to GitHub. I seem to have a good workflow now, seem to be moving past what I've found to be a fairly steep learning curve and I'm finding it easier to work with Git now than working with clunky patch files. It also open the possibility of using GitHub's code review tools on patches (for example, you can directly comment on a diff).

I just wanted to check and make sure it would work for you if I post patches, such as the one for this ticket, to branches on GitHub. I know you are an Hg user, and though I've used the git-svn connector a bit now, I've never used Hg or an Hg-Git connector. I have hardly any experience with Hg, and feel the need to gain a bit more experience with Git before trying to add another DVCS to my toolset.

### comment:12 in reply to:  2 Changed 4 years ago by Ryan J Ollos

But there is a problem, that we don't guarantee to return valid email addresses. Note that all addresses are piped through Chrome.format_author, and we may need to at least check for tailing '...' before formatting them blindly afterwards, right?

Yes, I see now. There are so many parameters governing whether the email address will be displayed that I don't see an easy way to sort it out through the session and configuration. The parameters include at least EMAIL_VIEW permission, [trac] show_email_addresses and [trac] never_obfuscate_mailto. It would be nice if Trac always wrapped an obfuscated email with <span class="obfuscated"> </span>, but it will be easy enough to check for the unicode character "…".

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

Patch in t10743-r13241-1.diff. I'd like to add some macro test cases, which will help with #11119 as well. I have in mind to use a test harness like [13129]. I would go ahead and write a patch that adds the test harness, but I want to check with you first to be sure that you are thinking similarly before making the effort.

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

I would go ahead and write a patch that adds the test harness, but I want to check with you first to be sure that you are thinking similarly before making the effort.

Sure, why not? I like unit testing. Just give me more, really. :-)

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

Owner: Steffen Hoffmann deleted

### Modify Ticket

Change Properties
Action
as new The ticket will remain with no owner.