chore_: update status-go #20537
chore_: update status-go #20537
Conversation
Jenkins BuildsClick to see older builds (12)
|
84% of end-end tests have passedFailed tests (4)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Expected to fail tests (4)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Passed tests (43)Click to expandClass TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
|
|
@kaichaosun thank you for the PR. I have couple of questions regarding the feature, would appreciate your answers. Could you please clarify, what is meant by acknowledged and unacknowledged messages? As far as I understand (from UI perspective), messages become acknowledged once receiver of these messages comes back online, is this correct? What I now see from user perspective: unacknowledged messages are displayed "greyed out" until receiver comes back online (see the video below). I am not sure such UI behaviour is expected. From sender's perspective it looks a little bit confusing to see messages greyed out, while sender is online and message is in fact sent to the receiver (it is just not Delivered because receiver is offline). Maybe here we need some input from design team or mobile devs cc @ilmotta . telegram-cloud-document-2-5463105977110777521.1.mp4 |
|
Another question is - does this logic affects only 1-1 chats or group chats/community channels as well? I think we have an issue related to community channels ISSUE 1 Messages are displayed greyed out (unacknowledged?) for sender in community channelSteps:
Actual result: messages stay greyed out. In fac messages are delivered to community channel and can be seen by other channel members. Status-debug-logs - 2024-06-24T123255.744.zip telegram-cloud-document-2-5463105977110777531.mp4 |
|
@pavloburykh thanks for sharing your result!
If click on a message, it should show something like
The outgoing messages should be marked as
The messages should also be marked as |
|
@kaichaosun thanx for the answers.
On the video below you can see messages get stuck greyed out for more than 15 seconds sometimes. When I tested PR today morning a have faced grayed out state for more than a minute. I would say that even 10 or 15 seconds is too much from user perspective. Also, you can see that messages which have been in fact delivered to the Receiver are still displayed greyed out for some time on Sender's side which is also not expected I guess. greyed_out.mp4
There is no Delivered state. But as you can see in ISSUE 1 all messages stuck in greyed out (Sending) state for Sender. And it seems to be a regression of this PR. Would appreciate if you can take a look at this. |
|
@pavloburykh I pushed a change to tuning the "gray" out period to 3~6 seconds.
When receiver receiver a message, it will send another "ack" message to sender which takes a regular time for delivery.
I just tested community chat in my end, the message should works fine after tunning. There are scenarios that store node is in capacity or user has unhealthy connection with store node, it will affect "gray out" period, or may even stay in "gray" for a pretty long time. There are ongoing tasks to make a e2e acknowledgement for community chat, but it's not in the scope of this task. Please review and let me know if there are issues, thanks for pointing out above improvement. |
kaichaosun
force-pushed
the
tunining-mvds
branch
from
f1872cc to
9f7074f
Compare
82% of end-end tests have passedFailed tests (5)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestWalletMultipleDevice:
Expected to fail tests (4)Click to expandClass TestWalletOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (42)Click to expandClass TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
@kaichaosun thank you! Looks better and ISSUE 1 is not reproducible. I think we can merge status go. |
Summary
Changes: This PR reset the epoch (next sending time for unacknowledged messages) in MVDS if receiver becomes online. When getting a user status (from receiver) is automatic, alwaysOnline or inactive, it will trigger this logic, and resend the message for ack in a few seconds.
Status-go PR: status-im/status-go#5349
Risk: low
How to test:
Review notes
Testing notes
Platforms
Areas that maybe impacted
Functional
Non-functional
Steps to test
Before and after screenshots comparison
status: ready