fix: don't delete chat for blocked contacts

[saledjenic]

No description provided.

[status-github-bot]

Pull Request Checklist

• Have you updated the documentation, if impacted (e.g. docs.status.im)?
• Have you tested changes with mobile?
• Have you tested changes with desktop?

[status-im-auto]

Jenkins Builds

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	3e7b3b2	#1	2022-03-28 08:56:27	~2 min	linux	📦zip
✔️	3e7b3b2	#1	2022-03-28 08:58:08	~4 min	android	📦aar
✔️	3e7b3b2	#1	2022-03-28 08:58:40	~4 min	ios	📦zip

[cammellos]

@saledjenic Thanks for the raising the PR!
Not sure it's quite how we want to implement it, deleting messages is something that most likely we'd like to keep (though we can have a conversation if there's a need to be changed).
Just checking on mobile, we don't quite have the same issue, if you block/unblock you are able to receive new messages (though you'll see a notification in the activity center).
Not sure how it plays out with mutual contact requests, but maybe there's some state in the client to be changed?

[saledjenic]

@cammellos let's discuss it then... at this moment, properties (coming from status-go) which actually mark in which state the contact is are added, blocked, hasAddedUs, Removed and all of them are bool type.

New design, at least for the desktop app, has more granular contact section view, there are the following views and condition what contacts belong to what view:

1. Contacts
   1.1. identity verified contacts -> not implemented yet
   1.2. mutual contacts -> if (added AND hasAddedUs)
2. Pending Requests
   2.1. Received -> if (not added AND hasAddedUs)
   2.2. Sent -> if (added AND not hasAddedUs)
3. Rejected Requests
   3.1. Received -> if (added AND Removed)
   3.2. Sent -> not implemented yet
4. Blocked -> if (blocked)

Based on above a change made in protocol/contact.go was necessary, otherwise after unblocking a blocked contact it would be treated as a new strange contact instead of being added to the group it belonged to before we make it blocked.

The reason why I did changes in protocol/message_persistence.go and protocol/messenger_contacts.go cause if at the moment of blocking contact you have 1:1 chat with it added to the Chat section, after app restart it won't be displayed there because we're building list of chats for the Chat section based on wakuext_chats rpc call and that chats is deleted in mentioned places. That's the reason why I did that update, so now after we block some contact and restart the app it will look the same as it was before closing (1:1 chat will be properly loaded and it will be blocked in that case).

@cammellos that's how we do it in the desktop app. Any suggestion is welcomed, also you know who else we should include in this discussion if needed.

but maybe there's some state in the client to be changed?

Even if we play with the above mentioned flags on the desktop app side, and make it work somehow that way, we cannot have consistent state among two app starts if we don't update status-go and store the real app state there.

[cammellos]

@saledjenic Thanks for the explanation.

It seems like there's two different concerns at play, one is how contacts will be looking like once we implement all the different states that you mentioned, the other is how to handle blocking/unblocking contacts.

In terms of blocking contacts, looks like there's some discrepancies on how mobile & desktop are handling the UI/UX, specifically:

The reason why I did changes in protocol/message_persistence.go and protocol/messenger_contacts.go cause if at the moment of blocking contact you have 1:1 chat with it added to the Chat section, after app restart it won't be displayed there because we're building list of chats for the Chat section based on wakuext_chats rpc call and that chats is deleted in mentioned places.

In the case of mobile, the chat is removed as soon as the RPC is successful, so the behavior is consistent and it looks like it does not result in the issue you see on desktop, so I think in this case it looks like you can adapt the same behavior client side on desktop? (We want to remove blocked users' messages and chat because one of the reason you might block a contact is spam, or dos of the app, and messages/chat might have an impact on it).

Based on above a change made in protocol/contact.go was necessary, otherwise after unblocking a blocked contact it would be treated as a new strange contact instead of being added to the group it belonged to before we make it blocked.

This seems to be a bit of a choice we want to make, in terms of UX, we decided previously that if you block someone that is in your contacts, you also want to remove them from your contacts, the argument is that those two options go almost always together, therefore when/if unblocking the contact, it should not be restored to your previous state. (We can discuss this choice, though mind this is a ui/ux edge case most likely)

Just to understand, is this behavior currently resulting in some issues on desktop?
status-im/status-desktop#5198 does not happen on mobile, so just wondering whether it's actually related to the fact that we remove a contact when we block it, but I might be missing some context.
Thanks!

[saledjenic]

In the case of mobile, the chat is removed as soon as the RPC is successful, so the behavior is consistent and it looks like it does not result in the issue you see on desktop, so I think in this case it looks like you can adapt the same behavior client side on desktop? (We want to remove blocked users' messages and chat because one of the reason you might block a contact is spam, or dos of the app, and messages/chat might have an impact on it).

I've thought the request is to have the app in the same state among multiple runs. And that is not possible (in case the 1:1 chat is added to Chat section and we block that user) if we don't received blocked chats as part of wakuext_chats response.

But if request is to not display blocked chats, then we're good, we just need to revert changes done in protocol/message_persistence.go and protocol/messenger_contacts.go.

we decided previously that if you block someone that is in your contacts, you also want to remove them from your contacts, the argument is that those two options go almost always together, therefore when/if unblocking the contact, it should not be restored to your previous state

I agree, that was a request, but is that still a request considering that we have a new design now? If we unblock a blocked user and we don't need to get him into the state it was before blocking we can revert changes done in protocol/contact.go as well.

Just to understand, is this behavior currently resulting in some issues on desktop?

Referring to the status-im/status-desktop#5198 the thing why we are not able to receive messages for the user after we unblock it (in case we don't restart the app) is that messages.new signal sent from status-go contains a default chat (at chats[0] position) with the active property set to false, but on the desktop app side we're checking that field and just ignore messages for inactive chats, see here.

@cammellos all in all based on what is discussed here, now the question is what do we want to achieve? Also to have all this set up properly according to the new design requirements we will need more new properties for a contact, but what do you propose so far?

[cammellos]

Referring to the status-im/status-desktop#5198 the thing why we are not able to receive messages for the user after we unblock it (in case we don't restart the app) is that messages.new signal sent from status-go contains a default chat (at chats[0] position) with the active property set to false, but on the desktop app side we're checking that field and just ignore messages for inactive chats, see here.

Do you receive a notification in the activity center? That should be the one we use on mobile in this case, since the chat is inactive

@cammellos all in all based on what is discussed here, now the question is what do we want to achieve? Also to have all this set up properly according to the new design requirements we will need more new properties for a contact, but what do you propose so far?

We will definitely need to add some of the fields you were mentioning, some of them I am working on (for example the time a contact request was sent), but probably the best place for conversation is in the PRs we are going to make those changes ( I am working on https://github.com/status-im/status-go/tree/feature/mutual-contact-requests ), but any suggestion is welcome

[saledjenic]

Do you receive a notification in the activity center?

@cammellos a notification that user is blocked?

All in all, if I leave in this PR only changes done here protocol/contact.go, and revert other two files, is that ok from your side?

[saledjenic]

@cammellos closing this cause we temporary proceeded with "desktop only" solution.

• fix: set only blocked flag to blocked contact for desktop app #2635