HyphaApp / hypha

Submission management software for open calls
https://www.hypha.app
BSD 3-Clause "New" or "Revised" License
67 stars 39 forks source link

Utilize emails as display name on comments when no name is set #3896

Closed wes-otf closed 2 months ago

wes-otf commented 4 months ago

Fixes #3895. Quick one I noticed while working on the partner comment stuff. Comments will now display with the user email where no full name is set.

Screenshots

Screenshot 2024-04-22 at 05 20 24

Test Steps

wes-otf commented 4 months ago

branched from my other PR on accident, should be all set now.

theskumar commented 3 months ago

Is there any privacy concern with emails being display. Is so we can try showing only first few characters and the domain.

e.g. for saurabh@example.com -> sau...@example.com

wes-otf commented 3 months ago

Hey! In OTF's usage I don't see a scenario where there could be a concern of privacy. Most of the time the email address is accessable via the application overview anyway as far as I know. Let me know if there's a scenario you think this could be an issue though

theskumar commented 3 months ago

Hey! In OTF's usage I don't see a scenario where there could be a concern of privacy. Most of the time the email address is accessable via the application overview anyway as far as I know. Let me know if there's a scenario you think this could be an issue though

I think i was primarily thinking around the emails of reviewers (internal and external). But then i as also thinking stricter and more privacy friendly implementation without a use-case.

I used something like this for the long filenames, where the use-case was more of UX but here it might act as good UX (for long emails) & privacy by default feature.

just to be sure, I don't have a strong argument/opinion for this. We go with what you decide, I'm just get worried when PII get exposed to a wider audience.

https://github.com/HyphaApp/hypha/blob/d647118ee827eb1c30ff50620ac221a78726591c/hypha/apply/utils/templatetags/apply_tags.py#L30-L40

wes-otf commented 3 months ago

ahh I see, that totally makes sense! I'm absolutely all for preserving privacy wherever possible.

I chatted with staff a bit today and I guess per their bylaws OTF's external reviewers shouldn't even be able to interact with the applicants (more in #3907). If we move forward and make it so only staff can see comments from external reviewers, then only internal reviewers are to be considered, which would always be staff and thus appear as the Org's long name in comments rather than any other display name. I'll ask around and see what folks think though, especially if there's a need for partners' identities to be concealed or anything.

frjo commented 3 months ago

Works well in my testing.

wes-otf commented 3 months ago

Chatted with staff & Slammer around this - currently the only time someone who isn't staff would be seeing a user's email would be in the case of a partner and in OTF's use case that's fine, the partner & applicant are expected to have this info already. I think this might be worth a revisit down the road though as we continue to expand and rethink the comment section.

wes-otf commented 3 months ago

For now I think this is ready to go though unless there's any objections

theskumar commented 3 months ago

When a new comment is posted, the applicant get this email. hypha/apply/activity/templates/messages/email/comment.html. If the name of the commenter is not set, the email contains the email of the commenter. Should that be fixed? Maybe use more generic phrase. like comment by Staff instead or comment by xyz@email.com

frjo commented 3 months ago

@theskumar Good point!

How do you all feel about this suggestion?

We let the short name be just the first part of the e-mail address.

    def get_short_name(self):
        return self.email.split("@")[0]

We then replace most uses of "full_name" with "display_name" throughout Hypha.

If the user has not set a name we have something to display, but we do not leak the full e-mail address unnecessarily.

wes-otf commented 3 months ago

All great catches! The short name seems to work nice & will even tighten up the options in the top right now that it doesn't need to include an email domain:
before

Screenshot 2024-05-08 at 14 15 52

after

Screenshot 2024-05-08 at 14 16 16

other changes should also be more in-line with what was mentioned here