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

no contact names for contact-IDs used in mailinglists' reaction details #5184

Open
r10s opened this issue Jan 15, 2024 · 19 comments
Open

no contact names for contact-IDs used in mailinglists' reaction details #5184

r10s opened this issue Jan 15, 2024 · 19 comments
Labels
bug Something is not working discussion

Comments

@r10s
Copy link
Member

r10s commented Jan 15, 2024

the reaction details in mailinglists seems not to use getSenderName() as in similar places (group bubbles etc.); i've seen this on the screen of @hpk42

@adbenitez i do not have and android with such mailinglists, can you confirm and fix?

@r10s r10s added the bug Something is not working label Jan 15, 2024
@adbenitez
Copy link
Member

adbenitez commented Jan 15, 2024

this is a core issue, you can't use "getSenderName" since the reactions API what gives you is contact IDs and that contacts don't have the proper display name since in mailing lists these display names are treated as overriden display names (~) so if you open the profile of any user in the maling list you will see that the name is not displayed in the profile either, because core has no saved the display name for that contacts, so it is not affecting only reactions but profiles, search results etc.

@r10s
Copy link
Member Author

r10s commented Jan 16, 2024

i see. getSenderName() requires sender-message-information which we do not have for mailinglists

@adbenitez adbenitez transferred this issue from deltachat/deltachat-android Jan 16, 2024
@r10s r10s changed the title reaction details in mailinglists show weird names weird contact names for contact-IDs used in mailinglists Jan 20, 2024
@r10s
Copy link
Member Author

r10s commented Jan 20, 2024

do we know internally if a given contact-ID was created for a mailing lists and the "real" name is available only when using getSenderName() for an explicit message?

if so, we could alter getDisplayname() so that it returns just "Mailinglist User" in that case, this seems to be good enough and might avoid larger refactoring when many ppl potentially share the same address and we cannot attach a name there

@adbenitez adbenitez removed their assignment Jan 21, 2024
@iequidoo
Copy link
Collaborator

I see that in receive_imf.rs mailing list contacts are created just with Origin::Hidden:

let (contact_id, _) = Contact::add_or_lookup(context, "", &list_post, Origin::Hidden).await?;

But when you call Contact::get_all_blocked() the first time they turn into Origin::MailinglistAddress. It's a question for me what happens if another message is received then because it triggers Contact::add_or_lookup(... Origin::Hidden) again. I think a Rust test should be written for this (at least to record the current behaviour). Anyway we can determine if a contact was created for a mailing list, but it's a bit complicated now (see Contact::update_blocked_mailinglist_contacts() for the reference), maybe we should add a db migration instead marking such contacts somehow.

Also it's a question how to make this "Mailinglist User" displayname translatable.

@iequidoo iequidoo self-assigned this Feb 26, 2024
iequidoo added a commit that referenced this issue Feb 28, 2024
…name (#5184)

If we cannot use the display name from the "From" header for a new contact, e.g. for mailing list
messages, use the "From" address, it's better than an empty string.
@iequidoo
Copy link
Collaborator

I was looking at the wrong place, that is the code creating a contact for a mailing list itself, not for mailing list users.

do we know internally if a given contact-ID was created for a mailing lists and the "real" name is available only when using getSenderName() for an explicit message?

So, for already existing contacts we don't know that. But we can create new contacts using "Mailinglist User" as a display name, but i'd suggest to use the From address instead, this way display names would differ and also we don't need translation for this string. I made a draft PR, if you want, you can try it out.

@r10s r10s changed the title weird contact names for contact-IDs used in mailinglists no contact names for contact-IDs used in mailinglists' reaction details Feb 29, 2024
@iequidoo
Copy link
Collaborator

There's also some discussion about creating a new contact if a display name changes in the "From" header: #5294 (comment)

@iequidoo
Copy link
Collaborator

iequidoo commented Mar 8, 2024

So, for now we don't have any idea how to differ single-address mailing lists from separate-addresses ones which is the source of the problem. We can name all ML users "Mailinglist User", but i'd suggest some emoji like 👤 to avoid translation.

@r10s
Copy link
Member Author

r10s commented Mar 9, 2024

translation is not an issue, we do that for far less visible aspects

@adbenitez
Copy link
Member

I still don't understand why we keep considering "mailing list user" as a possible solution, it is better to leave it without any name then and at least have the address as a way to differentiate users, otherwise a bunch of "mailing list user" in the reactions overview is WAY worse

@iequidoo
Copy link
Collaborator

iequidoo commented Mar 10, 2024

@adbenitez But the address is displayed right under the username, checked on Android and Desktop, and i guess it's so everywhere, not only in the reactions overview.

But we can also extend the reactions API to store and return usernames to truly solve the problem. Looks like we even don't need to deprecate the current API, only add smth like dc_msg_get_override_sender_name().

@iequidoo
Copy link
Collaborator

Checked the code, now the overriden name isn't saved to the db at all for trashed messages (what reactions are), see receive_imf::add_parts():

                    if trash {
                        "".to_string()
                    } else {
                        param.to_string()                                                                                                                                                                            
                    },

So, the username can't even be restored. Not sure how useful this information is in the single-address MLs, but maybe it's not so bad to save it for reactions.

@adbenitez
Copy link
Member

@adbenitez But the address is displayed right under the username, checked on Android and Desktop, and i guess it's so everywhere, not only in the reactions overview.

Good point, but even then the "mailing list user" thing is pointless and misleading ex. for GitHub it would be weird to see "mailing list user" as display name because it is not exactly a discussion mailing list, I guess this could also be the case for other mailing lists, putting a dummy display name is not useful IMHO

@iequidoo
Copy link
Collaborator

iequidoo commented Mar 13, 2024

Btw, currently reactions are broken at all for single-address MLs because all users are mapped to a signle contact and reactions from the most recent user override reactions from the previous one. Apparently we indeed need separate contacts for users having different display names. There is no limitation currently on the number of contacts having the same address, but of course such a change should be made carefully.

@r10s
Copy link
Member Author

r10s commented Mar 13, 2024

if all users use the same address, assigning reactions by name only, seems weak and error prone as well (can one really reply to these ml in general?)

compared to other issues, it seems questionable if single-user ml as github are worth the effort of large contact-/peer-handling refactoring/logic-changing and risking by far more worse bugs

@adbenitez
Copy link
Member

I don't think the single address mailing list is a common scenario, but saving reaction per overridden sender name in fact would be a great improvement since it would also allow bridge bots to properly map reactions from other platforms, ex. from matrix or telegram, to delta chat, currently it is not possible to properly bridge reactions because of this, oth only using overridden sender as id for each reaction sounds a bit of a hack

@iequidoo
Copy link
Collaborator

I don't think the single address mailing list is a common scenario, but saving reaction per overridden sender name in fact would be a great improvement since it would also allow bridge bots to properly map reactions from other platforms, ex. from matrix or telegram, to delta chat, currently it is not possible to properly bridge reactions because of this, oth only using overridden sender as id for each reaction sounds a bit of a hack

By default we shouldn't use the overriden sender name as an id (or to create separate contacts for different names), but only in some special cases. So, we need some flag that a message is bridged. Or maybe better that a contact (mailing list or bot) is a "collector" for users on some other service. I don't think it's a very bad hack, it's responsibility of that bot to assign unique names to users, otherwise it would be impossible to differ them anyway. But of course better to add some message header to identify users.

@gerryfrancis
Copy link
Contributor

@link2xt Do you think a similar fix like #5375 could be applied here as well?

@iequidoo
Copy link
Collaborator

@link2xt Do you think a similar fix like #5375 could be applied here as well?

The problem is that for reactions overriden sender names aren't even saved to the db. But the bigger problem is that we don't know how to differ contacts from each other in a single-address ML. In this case we handle reactions as if they are from the same contact, so reactions are broken completely. Probably we need to create separate contacts for different overriden sender names, this doesn't change the db structure, but anyway this may be a dangerous change as r10s noted.

@link2xt
Copy link
Collaborator

link2xt commented May 16, 2024

To summarize, there are multiple use cases:

  • Anonymizing mailing lists that map each user to an unique address. In this case we can save display name in the contact and display the last seen display name.
  • Anonymizing mailing lists that map all users to the same address.
  • Bridges that map all users to the same address.
  • Shared (community) accounts DeltaLab experiment where self-sent messages have different display names.

Bridge bot is the most complicated scenario to think about, because there may be multiple users behind the bridge that want to put the same thumbs-up reaction. In this case we need to display multiple reactions with different display name coming from the same email address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working discussion
Projects
None yet
Development

No branches or pull requests

5 participants