-
Notifications
You must be signed in to change notification settings - Fork 987
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
View token-gated requirements is shown for non toked gated communitie… #20240
View token-gated requirements is shown for non toked gated communitie… #20240
Conversation
Jenkins BuildsClick to see older builds (10)
|
Hey @flexsurfer! Actually, this feature has been implemented. In case of token gated community it will show the requirements. So as was mentioned by @churik, expected result is:
Below is the video how it looks like in case of token gated community: telegram-cloud-document-2-5384422923572366550.mp4 |
thank @pavloburykh could you please explain how the community can be token gated but has no permissions? does that mean only some channels are gated in that token-gated community? |
by token gated community we mean only those communities that have set permissions for membership. Other kinds of permissions (Admin/channel permissions) do not make community token gated. |
thanks @pavloburykh should be fixed now |
I am temporary blocked from testing this PR, as my node is down and communities data is not fetched. Waiting for fix from Waku team. |
67% of end-end tests have passed
Not executed tests (1)Failed tests (13)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletMultipleDevice:
Expected to fail tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Passed tests (34)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
|
Hey @flexsurfer! Thanks for the fix. Please take a look at the issue: ISSUE 1 View token gated requirements option is not shown in communities with permissionsPlease, also see the comment below Example of community https://status.app/c/ixaACiwKEnRva2VuIGdhdGVkIG1heSAzMBILZnNhZnNkYXNhZmQYASIHIzQzNjBERgM=#zQ3shahCqDog69f3hu6YdeZmdpyssWVUUMYuuxkTjvdnkjXLw Steps:
Actual result: View token gated requirements option is not shown Below is comparison PR with nightly telegram-cloud-document-2-5386803860167740805.mp4 |
@flexsurfer I propose to get back to your initial solution and just hide this option behind feature flag. We should not spend too much time on this issue now, as it is not a priority. After hiding this option I will log a separate issue so we can fix the logic of showing this option in future. |
6661ca1
to
2e6b50a
Compare
thanks @pavloburykh done |
skipping manual qa, as we got back to safe changes. |
84% of end-end tests have passed
Not executed tests (1)Failed tests (4)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (43)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
fixes #20237
feature is not implemented so hide it under non implemented flag, no QA needed