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

Trying to join two different communities with misleading prompt #1847

Closed
kingalg opened this issue Sep 21, 2023 · 15 comments
Closed

Trying to join two different communities with misleading prompt #1847

kingalg opened this issue Sep 21, 2023 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@kingalg
Copy link
Collaborator

kingalg commented Sep 21, 2023

Version: [email protected] (but I doubt that it was introduced in this version. Probably something that was already there)
System: all desktop

Steps to recreate:

  1. Have links to two working communities.
  2. With third app try to join first community via link but DON'T choose a username
  3. Without closing the app click on the link to join second community
  4. You will get popup with "You already belong to a community We're sorry but for now you can only be a member of a single community at a time."

Text on the popup suggest that the user will join first community that they tried to join.
What happens: the user join second community (or any community that was clicked as last if there is several of them).

Screenshot 2023-09-21 at 15 37 47

@holmesworcester I don't have strong feeling about whether user should join first or last community they try to join but what we have now is misleading, so we need to either make sure that they will join first community as popup suggests or change text of this popup. Let us know what you think is the best idea.

@kingalg kingalg added the bug Something isn't working label Sep 21, 2023
@kingalg kingalg added this to Quiet Sep 21, 2023
@kingalg kingalg moved this to Backlog - Desktop & Backend in Quiet Sep 21, 2023
@holmesworcester
Copy link
Contributor

This is a bug yes. If you are already in a community or joining, you should have to leave it before you can join another.

It took me a few reads to understand what was happening here. In case this restatement is helpful for anyone else, what is happening here is that clicking new join links will keep joining new communities even if the user never left the previous community. People will lose their accounts so this is not desired.

@holmesworcester holmesworcester moved this from Backlog - Desktop & Backend to Sprint in Quiet Sep 21, 2023
@holmesworcester
Copy link
Contributor

Moving to sprint.

@kingalg
Copy link
Collaborator Author

kingalg commented Sep 22, 2023

@holmesworcester could you let me know at which point I lost you and I will edit it to clarify? I'm fairly sure that we are talking about two different situations. In point 2 it's said "DON'T register username" so the registration process is actually never triggered. We are safe, no one will loose their accounts. It's only for fresh new users that tries to click on joining link for two different communities before entering their username.

@holmesworcester
Copy link
Contributor

The confusion comes from the word 'register' since registering happens after somebody joins the community. So 'don't register' would mean that they joined but didn't register.

'Don't choose a username' would be more clear.

Okay yes then this isn't as high priority.

@holmesworcester holmesworcester moved this from Sprint to Backlog - Desktop & Backend in Quiet Sep 22, 2023
@kingalg
Copy link
Collaborator Author

kingalg commented Sep 22, 2023

True, in our current process with unregistered and registered users this may be confusing, I'll change this.

@leblowl leblowl moved this from Backlog - Desktop & Backend to Sprint in Quiet Oct 4, 2023
@holmesworcester
Copy link
Contributor

holmesworcester commented Oct 4, 2023

The solution is that it should follow through on what it says. If the user is in a community (even if they have no chosen a username) they should not join a new community. If current community is set in any way (even if username is not chosen) don't process the invite link in any way.

@siepra
Copy link
Contributor

siepra commented Nov 8, 2023

@holmesworcester what's the factor determining membership in a community?

Is it:
a) creation of local network data structures
b) broadcasting a CSR
c) connecting to other peer for the first time
d) something else?

In case of an a we probably don't have to change anything and we can close this issue.

In case of a b or c we should implement a new feature on UI for letting users "leave" (erase network data structures and start from scratch before they choose a username).

Another option is to add a path that ignores deep linking mechanism in case of not chosen username but it may be misleading as the user won't be able to say which community he's choosing a username for.

@holmesworcester
Copy link
Contributor

I think the criteria must be: "user has opened an invite link". Once they have opened an invite link, they have started the process of joining a community and should not accidentally change what community they are joining or have any ambiguity about it.

Once they open an invite link, clicking a new invite link should not join a new community, it should tell them that they are already in a community.

@siepra
Copy link
Contributor

siepra commented Nov 8, 2023

It fits into the criteria of a, which is how it works atm. Does it mean we can close this issue?

@siepra
Copy link
Contributor

siepra commented Nov 8, 2023

Ok, nevermind. I see the problem is wrong data is being passed under the hood. So we don't change anything from the user perspective, we just correct the network data being processed

@siepra siepra self-assigned this Nov 13, 2023
@siepra siepra moved this from Sprint to In progress in Quiet Nov 13, 2023
@siepra
Copy link
Contributor

siepra commented Nov 13, 2023

@kingalg could you please check it once again? I'm attaching a video that presents it working fine on my end. I also wrote tests for deep linking behavior in both desktop and mobile and they passes as though everything works fine.

Here are the steps I took:
https://github.com/TryQuiet/quiet/assets/15381135/897ce719-81f2-4a26-a86d-dbaa59219832

Here is the result (couldn't upload the full video as it was too heavy)
Zrzut ekranu 2023-11-13 o 17 21 42

@siepra siepra moved this from In progress to Waiting for review in Quiet Nov 14, 2023
@kingalg
Copy link
Collaborator Author

kingalg commented Nov 14, 2023

This is still an issue in both old and newest (Version: 2.0.3-alpha.4) version. @siepra and @EmiM 've seen that I was able to reproduce it several times. With the same links Wiktor is joining correctly all times but Emi managed to reproduce it same as I did.

@siepra
Copy link
Contributor

siepra commented Nov 14, 2023

We still don't know what the factor is though

@siepra
Copy link
Contributor

siepra commented Nov 14, 2023

Me and Emilia have found some tricky bug being hidden in state-manager. It was unnecessarily updating store data on desktop causing hard to spot race condition

siepra added a commit that referenced this issue Nov 15, 2023
…s being stored #1847 (#2068)

* fix: mock store readyness for mobile tests

* test: deep linking on mobile

* test: deep linking on desktop

* fix: correct store data injecting #1847

* test: remove component from rendering

* chore: update CHANGELOG

* test: adjust tests after merging psk work

* chore: refactor invitation data mocking

* fix: lint

* fix: test snapshot for mobile deep linking

* chore: skip mobile splash screen tests due to changes made to data mocking
@siepra siepra moved this from Waiting for review to Merged (develop) in Quiet Nov 15, 2023
@Kacper-RF Kacper-RF moved this from Merged (develop) to Ready for QA in Quiet Nov 21, 2023
@kingalg
Copy link
Collaborator Author

kingalg commented Nov 24, 2023

Desktop: [email protected]
Mobile: [email protected] (iOS 337)

It's fixed on desktop but there is problem on mobile. I've created separate task for it and put it to the "next sprint" column: #2114

@kingalg kingalg closed this as completed Nov 24, 2023
@kingalg kingalg moved this from Ready for QA to Done in Quiet Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

3 participants