add members to open community #16917
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jo-mut
requested review from
flexsurfer,
ilmotta,
siddarthkay,
J-Son89 and
smohamedjavid
Jenkins BuildsClick to see older builds (79)
|
jo-mut
force-pushed
the
bug/missing-community-members
branch
from
0e1f93b
to
79fa7ea
Compare
flexsurfer
approved these changes
Aug 8, 2023
|
hey @jo-mut tests and lint are broken |
siddarthkay
approved these changes
Aug 8, 2023
jo-mut
force-pushed
the
bug/missing-community-members
branch
from
79fa7ea
to
fe17032
Compare
smohamedjavid
approved these changes
Aug 8, 2023
ilmotta
approved these changes
Aug 8, 2023
41% of end-end tests have passedFailed tests (24)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (17)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
|
@jo-mut hi. Please take a look at the following issue. ISSUE 1 Unable to join open communitySteps:
Actual result: nothing happens when tapping Join community button telegram-cloud-document-2-5373339562896534883.mp4Expected result: user is able to join community |
54% of end-end tests have passedFailed tests (19)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Passed tests (22)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
|
jo-mut
force-pushed
the
bug/missing-community-members
branch
from
fe17032
to
249c051
Compare
|
@pavloburykh I have made an update to fix that. Works fine now WhatsApp.Video.2023-08-08.at.20.39.28.mp4 |
jo-mut
force-pushed
the
bug/missing-community-members
branch
2 times, most recently
from
628535b
to
72033e3
Compare
85% of end-end tests have passedFailed tests (6)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (35)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
|
jo-mut
force-pushed
the
bug/missing-community-members
branch
from
72033e3
to
ccb80d9
Compare
|
Thank you @VolodLytvynenko |
Got it. Thank you for the clarification! |
@VolodLytvynenko the above statement is incorrect, both open and closed communities can have ‘auto accept requests to join (e.g. requests to join are automatically accepted by the control node) switched on or off. An ‘open’ community is a community with zero ‘tokens needed to become a member’ permissions set. A ‘closed’ community has one or more ‘tokens needed to become a member’ permissions set. I’ll be back at work tomorrow, can hop on a call to further clarify if useful. |
|
Thanks @John-44 for clarifying I stand corrected. |
jo-mut
force-pushed
the
bug/missing-community-members
branch
from
c941ea2
to
5f83337
Compare
|
@VolodLytvynenko regarding this pr as I discussed with @cammellos, I think it would be okay to create a separate follow up issue and make an updates once the figma designs have been updated. In the meantime since this pr. seeks to resolve displaying the members of an open community which really is the major thing and is unhindered by the figma designs. We can merge the pr as is and address ISSUE: 7, 6, 5 & 3 in a follow up pr. What do you think? |
jo-mut
force-pushed
the
bug/missing-community-members
branch
2 times, most recently
from
acb7ee8
to
7e06a1a
Compare
jo-mut
force-pushed
the
bug/missing-community-members
branch
from
7e06a1a
to
c3ae91f
Compare
Hi @jo-mut, yes, I'm okay with creating follow-ups for issues 7, 6, 5, and 3. I noticed that some new commits have been added. Let me check this PR once again today and I'll let you know. |
|
@VolodLytvynenko 7/6/5/3 are all due to owner offline/online, which is not something we should be taking into consideration, I believe there was a mixup with the designs and were not updated after our conversation, @John-44 had been looking into it, so for now, those are not issues to be tracked, we can revisit once the designs are updated (please @John-44 let us know if you find the updated designs). |
88% of end-end tests have passedFailed tests (5)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (38)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
|
@cammellos @jo-mut @VolodLytvynenko I hope the updated figma designs will be ready by the end of today, Mario is working on them now. I'll post a link here as soon as the updated designs are ready. Apologies for the confusion and the designs being incorrect. I thought we had updated them about 6 months ago, but it turned out that design had only updated parts of these designs but not all. Sorry for the oversight. |
|
Hi @jo-mut, thank you for PR. No additional issues from my side. PR can be merged |
|
@VolodLytvynenko thank you. Please assign the follow-up issue to me when you log it |
Mario is a star, the updates to these flows are all done, reviewed by myself and ready to go :-) @cammellos @jo-mut @VolodLytvynenko see https://www.figma.com/file/h9wo4GipgZURbqqr1vShFN/Communities-for-Mobile?type=design&node-id=13995-172448&mode=design&t=3PvI6EPtc2EqcsLs-0 Mario is going to be off for the next two weeks, but I can answer any questions related to these flows next week. Happy to do a call to review these flows together next week if that would be useful. |
This was referenced Aug 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #14956
Summary
There has been some updates on joining communities. Before users would basically add themselves to the member list and didn't have to wait for the control node to confirm the request (in fact it wouldn't actually send a request in the first place, it'd just join right away).
With the updates we have moved on from the part where
JoinCommunity()would add the user to the member list because what actually needs to happen is that there has to be aRequestToJoinResponse(). This is regardless of whether the community is open to join or notMobile still uses
JoinCommunity()which mean the control node never receives a the request to join for approval and therefore no RequestToJoinResponse which leads to the user not being added to the member listCurrent what happens is:
JoinCommunity()In this case the user does not get listed in the member list
What needs to happen is:
truethe user becomes part of the communityThe mobile client most likely receives a new CommunityDescription with your user as a member, shortly before receiving the request to join response
This PR removes the updates the client to call
RequestToJoinCommunity()instead ofJoinCommunity()which resolves the issue