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

Rejecting a Contact Request blocks further attempts to add user as a contact #16817

Closed
virginiabalducci opened this issue Nov 26, 2024 · 9 comments · Fixed by status-im/status-go#6140
Assignees
Labels
bug Something isn't working core-team
Milestone

Comments

@virginiabalducci
Copy link

Bug Report

Description

user 1 sends a contact request to user 2. User 2 rejects. The contact request gets stuck and these users cannot add each other.
There seems to be an issue triggered when the user rejects the contact request, blocking any other ways to add the contact.

Steps to reproduce

  1. Create a new user (user 1)
  2. send a contact request to user 2
  3. As user 2, reject the contact request

Expected behavior

As user 1, when going to Settings > Messaging > contacts > Pending Requests, the request sent to user 2 should be deleted

Actual behavior

As user 1, when going to Settings > Messaging > contacts > Pending Requests, the request sent to user 2 is still showing up. I've tested with 3 new users, and the behavior can be reproduced.

Also, if the user 2 sends a contact request to user 1:

  • no notification is shown on user 1's end
  • On user 1's end, the previous request sent by user 1 (step 2) is still there as a Pending Request.

If user 1 blocks and unblocks user 2 to remove them from the 'Pending Requests', when trying to add them as a contact, user 2 cannot add user 1.

Additional Information

test 054 logs.zip
test054 geth.log.zip

test053 geth.log.zip
test 053 logs.zip

Image

  • Status desktop version: latest master
  • Operating System: Mac OS Sonoma. Also tested on Windows.
@caybro
Copy link
Member

caybro commented Nov 26, 2024

I think this was designed as a "feature" to protect from spam; CC @jrainville @iurimatias do we still want that? 🤔

@virginiabalducci
Copy link
Author

Thanks @caybro for taking a look!

What if the user rejects the contact by mistake? It would be nice to give the user one more chance.
I'm not certain how we could handle a spam scenario, perhaps it’s worth revisiting? Thanks!

@jrainville
Copy link
Member

I think this was designed as a "feature" to protect from spam; CC @jrainville @iurimatias do we still want that? 🤔

Yeah, it might be by design. We don't want spammers to know that they got declined so that they can try again.

What if the user rejects the contact by mistake? It would be nice to give the user one more chance.
I'm not certain how we could handle a spam scenario, perhaps it’s worth revisiting? Thanks!

I think User 2 can then be the one sending the contact request and User 1's request should then show as "completed" and they would be mutual. Maybe there is an issue there though. I can try to reproduce and fix if that's the case.

@jrainville jrainville self-assigned this Nov 26, 2024
@jrainville jrainville added this to the 2.32.0 Beta milestone Nov 26, 2024
@caybro
Copy link
Member

caybro commented Nov 26, 2024

Thinking about this usecase again; I think it's even easier to the person receiving the CR to just go ahead and straight "ban" the other user. That way, the request at least shows up in the Banned section, and can be revisited at any later time

@jrainville
Copy link
Member

Thinking about this usecase again; I think it's even easier to the person receiving the CR to just go ahead and straight "ban" the other user. That way, the request at least shows up in the Banned section, and can be revisited at any later time

Do you mean the "reject" should act as a block? Do note that block straight up makes it so that User 2 would not be able to see any message from User 1 in that case (in a community for example).

If you just mean that rejecting should put them in another tab, I think it makes more sense. We had commented out code for that I think (because it never worked and it was super messy). We have a similar thing in the community for rejected requests.
I think with the latest changes by Michal and me, it should be doable to add this new tab called "Rejected contact requests" and then it would work as you probably proposed.

@caybro
Copy link
Member

caybro commented Nov 26, 2024

Thinking about this usecase again; I think it's even easier to the person receiving the CR to just go ahead and straight "ban" the other user. That way, the request at least shows up in the Banned section, and can be revisited at any later time

Do you mean the "reject" should act as a block? Do note that block straight up makes it so that User 2 would not be able to see any message from User 1 in that case (in a community for example).

If you just mean that rejecting should put them in another tab, I think it makes more sense. We had commented out code for that I think (because it never worked and it was super messy). We have a similar thing in the community for rejected requests. I think with the latest changes by Michal and me, it should be doable to add this new tab called "Rejected contact requests" and then it would work as you probably proposed.

Yeah, I meant for the "Reject CR" to work something like that 👍

@jrainville jrainville moved this from Next to In Progress in Status Desktop/Mobile Board Nov 27, 2024
@jrainville
Copy link
Member

@virginiabalducci I just tested this and it works as expected from my POV of the requirements.

Here's what I did:

  1. Create a new user (user 1)
  2. send a contact request to user 2
  3. As user 2, reject the contact request

Now, User 2 doesn't have a CR anymore and the state for User 1 stays as it was, because we do not want to let others know we rejected them

Now I want to "undo" that. Here's how:

  1. User 2 sends a contact request to User 1
  2. User 1 receives it and there are contacts

Nothing else is needed here. They are now mutual contacts, because they are both in a state where they send a contact request to each other (mutual).


Two improvements are possible though:

  1. Do as Lukas proposed and move "Rejected contact requests" to a new tab in the Contacts panel so that you can "undo" the rejection with a more intuitive solution
  2. There is a small bug where the CR message sent by User 2 on step 4 is not shown in the chat for User 1. See screenshot:
    Image

I will try to fix the second point now.

I don't know if we should create a new issue for the bug or for the new feature? What do you guys think?

@jrainville
Copy link
Member

Just as confirmation, we have a test case in status-go that does just as I said here:

https://github.com/status-im/status-go/blob/ee7f4d249c29c34298541722ca7f7694dc3f23b8/protocol/messenger_contact_requests_test.go#L1163-L1168

// The scenario tested is as follow:
// 1) Alice sends a contact request to Bob
// 2) Bob declines the contact request from Alice
// 3) Bob sends a contact request to Alice
// 4) Alice and Bob are mutual contacts (because Alice's CR is "pending" on her side), Both CRs are accepted
func (s *MessengerContactRequestSuite) TestBobSendsContactRequestAfterDecliningOneFromAlice() {

jrainville added a commit to status-im/status-go that referenced this issue Nov 28, 2024
… or updating the AC notif

Fixes status-im/status-desktop#16817

There were two issues.
When dismissing a CR, then sending one back, it did mark the two contacts as mutual and showed the 1-1 chat. However, the message sent in the second/final CR was not shown in the first person's client.
Also, the AC notification for the first user didn't update, so it got stuck in a "pending" state.

Those two issues are fixed now with a test to confirm.
@jrainville jrainville moved this from In Progress to Code Review in Status Desktop/Mobile Board Nov 28, 2024
@jrainville
Copy link
Member

I opened a new issue for the new tab: #16844

jrainville added a commit to status-im/status-go that referenced this issue Nov 29, 2024
… or updating the AC notif

Fixes status-im/status-desktop#16817

There were two issues.
When dismissing a CR, then sending one back, it did mark the two contacts as mutual and showed the 1-1 chat. However, the message sent in the second/final CR was not shown in the first person's client.
Also, the AC notification for the first user didn't update, so it got stuck in a "pending" state.

Those two issues are fixed now with a test to confirm.
jrainville added a commit to status-im/status-go that referenced this issue Dec 3, 2024
…e or updating the AC notif

Fixes status-im/status-desktop#16817

There were two issues.
When dismissing a CR, then sending one back, it did mark the two contacts as mutual and showed the 1-1 chat. However, the message sent in the second/final CR was not shown in the first person's client.
Also, the AC notification for the first user didn't update, so it got stuck in a "pending" state.

Those two issues are fixed now with a test to confirm.
jrainville added a commit to status-im/status-go that referenced this issue Dec 3, 2024
Fixes status-im/status-desktop#16817

There were two issues.
When dismissing a CR, then sending one back, it did mark the two contacts as mutual and showed the 1-1 chat. However, the message sent in the second/final CR was not shown in the first person's client.
Also, the AC notification for the first user didn't update, so it got stuck in a "pending" state.

Those two issues are fixed now with a test to confirm.
@github-project-automation github-project-automation bot moved this from Code Review to Done in Status Desktop/Mobile Board Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core-team
Projects
Archived in project
3 participants