feat: use 1 content-topic for all chats within a community (not to merge) #21407
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
Jenkins BuildsClick to see older builds (12)
|
flexsurfer
added
status-go
waku
|
Blocking it until 2.31 is cut |
chaitanyaprem
force-pushed
the
feat/community-content-topic
branch
2 times, most recently
from
519e4fb to
e57f206
Compare
|
@chaitanyaprem |
chaitanyaprem
force-pushed
the
feat/community-content-topic
branch
from
e57f206 to
66733bc
Compare
|
@churik this code is ready from status-go side and has been validated and dogfooded by devs. |
0% of end-end tests have passedFailed tests (8)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
|
9 tasks
chaitanyaprem
force-pushed
the
feat/community-content-topic
branch
from
66733bc to
e1f0fc7
Compare
86% of end-end tests have passedFailed tests (8)Click to expandClass TestWalletMultipleDevice:
Class TestFallbackMultipleDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (48)Click to expandClass TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
95% of end-end tests have passedFailed tests (3)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Passed tests (53)Click to expandClass TestFallbackMultipleDevice:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
|
93% of end-end tests have passedFailed tests (4)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Passed tests (52)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestFallbackMultipleDevice:
Class TestActivityMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
|
What has been tested so far:
Areas not tested so far:
Testing is still in progress. |
@churik as far as I understood, in @chaitanyaprem's status-go PR the plan is outlined that we need to essentially QA everything regarding communities. @chaitanyaprem created mobile and desktop PRs for that purpose. Once both QA teams from both clients give the green light and demonstrate that no notable regressions will be introduced, we can think about when to introduce the breaking change (thus merge). Could be in 2.33 or later (still open for debate). cc @qfrank |
@ilmotta note that this PR doesn't introduce breaking changes, rather has backward compatibility as this is only phase-1 part of changes. the phase-2 which is a different PR introduces breaking changes. |
Ah great, then if I understood correctly the status-go PR is mergeable as soon as we get green light from QAs. Thanks for clarifying this point. |
chaitanyaprem
force-pushed
the
feat/community-content-topic
branch
from
e1f0fc7 to
4da4ef2
Compare
|
What I have tested:
I didn't try token gated community or permission test as it require some tokens, I'll leave it to QA. |
|
@chaitanyaprem @ilmotta @qfrank I have additionally tested admin interactions in communities including token gated in following cases:
List of admin interactions that have been tested:
Everything was okay until at some point I was not able to login my Desktop Admin account anymore. I have no idea what is the reason. I just closed the app, then reopened, tried to login and failed. It happened in Desktop PR build, attaching logs and video below. Unless this critical issue is not PR related I would say you have a green light from mobile QA team. I hope attached logs will help in identifying the root cause. I am unable to reproduce this bug with other account, it just happened randomly and so no steps available. We definitely need to wait until feedback from Desktop QA team on corresponding PR, because admin actions and managing token gated communities is entirely Desktop feature, I only performed some basic checks and interactions with mobile. I see @noeliaSD is assigned to that PR, maybe she can provide some update on the progress (cc @anastasiyaig) screencast.2025-01-08.16-40-57.mp4 |
|
@pavloburykh Noelia is out for parental leave. I am on leave till next monday. @glitchminer @virginiabalducci pls check on what is needed there |
|
@pavloburykh Thanks for all the tests!
|
@chaitanyaprem then no issues from mobile side. Let's wait for Desktop QA feedback. |
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.
Summary
This PR is mainly to dogfood underlying changes made in status-go.
Reference PR status-im/status-go#5864 which has details.
This PR uses single content-topic for all community chats to send messages. It still receives messages on all existing content-topics used by previous versions in order to maintain compatibility.
Details of the discussion are available https://forum.vac.dev/t/status-communities-review-and-proposed-usage-of-waku-content-topics/335/29
Testing notes
Platforms
Areas that maybe impacted
Community chats and control flows
Functional
Steps to test
Risk
Described potential risks and worst case scenarios.
Tick one:
-->