-
Notifications
You must be signed in to change notification settings - Fork 985
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
fix(universal-link): more new link format, handle old link format #17721
Conversation
1ebc19a
to
d74f564
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much!
Jenkins BuildsClick to see older builds (89)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@yqrashawn thanx for the PR Issue is fixed but only for 1-1 chat. Still reproducing the bug in case of communities and group chats. ISSUE 1 Tapping on community/group chat PN leads to "Browse" popup on AndroidSteps:
Actual result: "Browse" popup is opened telegram-cloud-document-2-5309925075145733258.mp4Expected result: user is redirected to community/group chat |
64% of end-end tests have passed
Failed tests (13)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (3)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (29)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
@yqrashawn please ping me up for re-test when you'll have a chance to fix the issue. Thank you. |
@pavloburykh @yqrashawn is under the weather. Checking it out. I have enabled for group chats, still trying to figure out why |
Updates: although there's a conflict between old and new
they have different info in the URL, so I moved the old |
d74f564
to
03e922e
Compare
Thank you @yqrashawn! So once status-go PR is approved we are ready for re-test, correct? |
@pavloburykh also this one, code changed a lot so I dismissed the approved state of this PR. And I just find out the share profile URL is broken as well. I might need to fix it in this PR as well. I'll put this PR into draft state and reevaluate what needs to be done. |
75a64cf
to
8a32d46
Compare
@yqrashawn just want to understand what is the difference between https://status.app/cc/Ow==#zQ3shYSHp7GoiXaauJMnDcjwU2yNjdzpXLosAWapPS4CFxc11 AND https://status.app/c/Ow==#zQ3shYSHp7GoiXaauJMnDcjwU2yNjdzpXLosAWapPS4CFxc11 Why do we need those https://status.app/cc/ links if they do not open channel but community? In your file those links are listed within community channel links section |
So basically, what happens are
that's why there are both |
@yqrashawn are we leaving ISSUE 4 as a followup? |
@pavloburykh Yes. I can reproduce it but have some trouble debug it. I'll look into it tomorrow. Can you create an issue for it? |
Sure! I will create a followup once we merge this PR. I am waiting for e2e run and after that will move this PR to merge. |
84% of end-end tests have passed
Failed tests (3)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (38)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
|
@yqrashawn thank you for the PR and all the additional explanations. Ready for merge. |
32689af
to
503178b
Compare
blocking e2e test Signed-off-by: yqrashawn <namy.19@gmail.com>
status-im/status-go@3326362...678fc03 Signed-off-by: yqrashawn <namy.19@gmail.com>
503178b
to
669440e
Compare
rebase and update status-go version, waiting for ci |
…7721) status-im/status-go@d7e7792...678fc03 Co-authored-by: pavloburykh <pavlo@status.im>
Summary
/u#<user-chat-key>
/c#<community-id>
/c/<user-chat-key>
to new/c#<user-chat-key>
/cc/<community-channel-id>#<community-id>
fixes #17718
fixes #17725
fixes #17722
fixes #17723
Testing notes
Platforms
Areas that maybe impacted
anywhere related to old
/u/pubkey
linkanything related to universal/deep link
status: ready