-
Notifications
You must be signed in to change notification settings - Fork 984
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 improper use of current-chat-id and rename subs #18389
fix improper use of current-chat-id and rename subs #18389
Conversation
Jenkins BuildsClick to see older builds (16)
|
75% of end-end tests have passed
Failed tests (8)Click to expandClass TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (36)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
e59ccf6
to
2453b77
Compare
@Parveshdhull thanks for the PR. Failed e2e are known and not related to the PR. |
hi @pavloburykh, Thank you very much for checking the e2e tests. But it looks currently adding-removing members from home screen is not working because not only group details screen but most of the events related to management of group are also relying on current-chat-id. PR will also going to need manual-qa-testing as bug is more complicated then it initially looked. Will ping qa once PR is ready to test. |
2453b77
to
fffe947
Compare
88% of end-end tests have passed
Failed tests (3)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (3)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (42)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
|
fffe947
to
c755025
Compare
No new changes, just rebased PR. |
@Parveshdhull thanks for PR!
By this, did you mean that in this PR we need to check that everything works the same as in nightly, or that you also fix other issues related to the group management (in this case, we'll log the last two separately)? ISSUE 1 (PR only): Group members list is not displayed in group details anymoreISSUE 2 (nightly and PR): Non-admin can "remove" a group memberSteps:
In this case, the UI behaves weirdly: the system message video_2024-01-10_17-58-04.mp4As I can see from Figma a non-admin should not have this ability to remove user, but I might be wrong. Do you know for sure? If not, we can check this with the designers. ISSUE 3 (nightly and PR): Name of a new member is not displayed in the system message for other users if they are not contacts / before receiving first message from the new memberSteps:
|
Hi @qoqobolo, Thank you very much for testing the PR and finding these issues.
Sorry for the confusion, I meant please check everything works the same as in nightly + fixes the linked bug #18391 So, yes please log last two issues separately. Issue 1 should be fixed now. |
Thanks for the fixes @Parveshdhull! |
d205f9b
to
15d0b8c
Compare
fixes #18391
Summary
:current-chat-id
just for checking group detailsTesting
Please make sure management of group is working fine (removing, adding of members etc.)
status: ready