1. If we add a Genesis hack here, we should add a task to ENG-55 to revert it.
2. We should make sure that it is still possible to send a message to a non-friend, either by searching for their username in the search field, or by using the chat composer. I noticed something mentioned about this in the test plan, but it would be good to address this in more detail (ie. mention testing both methods in the test plan).
3. I noticed that the changes here are only on the keyserver side. What does the "error" look like when the user attempts to create a chat with multiple people, one of which is a non-friend? (Perhaps the client-side logic was already fixed up... just the keyserver side needed updates?)
4. It would be helpful to get more details on how this change is addressing the issue, as the code in validateCandidateMembers is rather complicated. Can you explain what's going on here, and why it's helpful to add the memberID to ignoreMembers in the case where isParentThreadGenesis?

In D6744#200902, @ashoat wrote:

1. If we add a Genesis hack here, we should add a task to ENG-55 to revert it.

Sure, I've added it: ENG-3024

2. We should make sure that it is still possible to send a message to a non-friend, either by searching for their username in the search field, or by using the chat composer. I noticed something mentioned about this in the test plan, but it would be good to address this in more detail (ie. mention testing both methods in the test plan).

I didn't know that we can initiate a chat in that way (searching for a username). I check it and it works correctly - we can create a new chat in that way.

3. I noticed that the changes here are only on the keyserver side. What does the "error" look like when the user attempts to create a chat with multiple people, one of which is a non-friend? (Perhaps the client-side logic was already fixed up... just the keyserver side needed updates?)

The client-side logic was already covered. When you try to add someone you are not friends with as another person to the chat an alert is displayed like this:

EDIT: Have to fix it on web.

4. It would be helpful to get more details on how this change is addressing the issue, as the code in validateCandidateMembers is rather complicated. Can you explain what's going on here, and why it's helpful to add the memberID to ignoreMembers in the case where isParentThreadGenesis?

I figured out that ignoreMembers stores filtered members, which whom we can't create a chat. It can happen for several reasons i.g. users have blocked each other.

There are two cases:

• If we don't requireRelationship (meaning that the chat is personal): we don't have to check anything, because we want to allow people to create private chats.
• Otherwise, when we requireRelationship (= it is not a personal chat) and isParentThreadGenesis (= it is not a subthread): we want to allow creating a new chat only when all invited members are friends with the creator.

I found other possible problems:

• When we use chat composer and try to find a user, with whom we don't have any common chats, it doesn't show up in the list.
• If we want to add a user to sub chat (who is in the parent thread != genesis) who is not our friend, we can't do that. The GUI doesn't let us do that.

Screenshot:

kuba4 is in parent chat All kubas, but I can't add him to the chat, because the GUI blocks it.

In D6744#200943, @inka wrote:

I feel like we should handle this differently. It is entirely unclear to the user what went wrong and how to fix it

I 100% agree with you. I verified it on the native side (where there is an alert). And on the web app (your screenshot), we should definitely do something about it. I will look at it tomorrow.

In D6744#200943, @inka wrote:

I feel like we should handle this differently. It is entirely unclear to the user what went wrong and how to fix it

I checked it right now, and it appears that someone has already covered it: https://phab.comm.dev/D5518. But apparently, it doesn't work. I am investigating that.

It was already fixed by @michal in https://phab.comm.dev/D5518. He said he will land the changes when he has time.