Fix unexpected live voice broadcast in the room list #7898

Florian14 · 2023-01-05T10:01:17Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Sometimes, Voice Broadcasts were shown as live in the room list by error due to redaction on state events.
In this PR, the most recent state event is taken like in the room timeline to display the correct voice broadcast state. Event redactions are correctly handled.

Motivation and context

Fix #7832

Screenshots / GIFs

Tests

Start a live voice broadcast
Pause and Resume the broadcast
Delete the voice broadcast

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

ElementBot · 2023-01-05T10:03:57Z

	Warnings
⚠️	Please add a changelog. See instructions here

Generated by 🚫 dangerJS against 9ca6a44

ElementBot · 2023-01-06T14:01:44Z

	Warnings
⚠️	Please add a reviewer to your PR.

Generated by 🚫 dangerJS against fb7388d

ElementBot · 2023-01-06T14:03:30Z

	Warnings
⚠️	Please add a reviewer to your PR.

Generated by 🚫 dangerJS against fb7388d

mnaturel

I have added comments about small improvements in code that could be done.

When testing with web, I achieved to reproduce the issue but I don't know if it is normal/expected. Here is how I reproduced:

Go into the room list in Android.
From web, start a live broadcast in a room.
In Android, see the indicator for the room.
In Web, pause and resume the live.
In Web move the floating window of the live to be able to reach the option menu for the live broacast message in the timeline.
From the context menu, remove the corresponding message.
In Android, it says a broadcast is live even if it has been deleted.

mnaturel · 2023-01-09T08:36:24Z

...java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastStateEventUseCaseTest.kt

+        // Given
+        val aVoiceBroadcast = VoiceBroadcast(A_VOICE_BROADCAST_ID, A_ROOM_ID)
+        val aListOfTimelineEvents = listOf<TimelineEvent>(
+                mockk(relaxed = true) {


I think we could introduce a private method like givenAVoiceBroadcastEvent(eventId: String, isRedacted: Boolean, timestamp: Long) to be reused accross tests. It will make the tests faster to read.

Done 493fa7a

mnaturel · 2023-01-09T08:45:00Z

vector/src/test/java/im/vector/app/test/fakes/FakeRoom.kt

 class FakeRoom(
        private val fakeLocationSharingService: FakeLocationSharingService = FakeLocationSharingService(),
        private val fakeSendService: FakeSendService = FakeSendService(),
-        private val fakeTimelineService: FakeTimelineService = FakeTimelineService(),


Normally there is no need to make the fakes public. You could mock the call in your tests without using the "fake" components like this:

fakeSession.roomService() .getRoom(A_ROOM_ID) .timelineService() .getTimelineEventsRelatedTo(any(), any())

Fixed 2df9480

ElementBot · 2023-01-11T16:34:59Z

	Warnings
⚠️	Please add a reviewer to your PR.

Generated by 🚫 dangerJS against 6b60b0a

ElementBot · 2023-01-11T16:39:20Z

	Fails
🚫	vector/src/test/java/im/vector/app/features/voicebroadcast/usecase/GetVoiceBroadcastStateEventUseCaseTest.kt#L127 - Needless blank line(s)

Generated by 🚫 dangerJS against 6b60b0a

Florian14 · 2023-01-11T16:53:59Z

I have added comments about small improvements in code that could be done.

When testing with web, I achieved to reproduce the issue but I don't know if it is normal/expected. Here is how I reproduced:
* Go into the room list in Android.

* From web, start a live broadcast in a room.

* In Android, see the indicator for the room.

* In Web, pause and resume the live.

* In Web move the floating window of the live to be able to reach the option menu for the live broacast message in the timeline.

* From the context menu, remove the corresponding message.

* In Android, it says a broadcast is live even if it has been deleted.

@mnaturel Thanks for your good test, I mainly fixed your issue but it will be totally fixed when implementing the MSC3912 because when you delete the started event, the related events are not redacted for the moment.

In your test, the latest event in the room summary is the resume event, when you delete the tile, only the started event is redacted, it does not update the room summary and does not refresh the room list. What I did is relying on the started event instead, it does not trigger a refresh of the room list but if you restart the app or if there is another change in the room summary, the room list will be refreshed and the event will be correctly shown as redacted. Also, if there are some messages after the voice broadcast state events, there is no issue because the latest event is not the voice broadcast.

ElementBot · 2023-01-11T16:58:31Z

	Warnings
⚠️	Please add a reviewer to your PR.

Generated by 🚫 dangerJS against f62f661

sonarqubecloud · 2023-01-11T17:41:27Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

71.7% Coverage
0.0% Duplication

mnaturel

Thanks for the updates and the explanation. LGTM!

Florian14 marked this pull request as ready for review January 6, 2023 13:59

Florian14 force-pushed the bugfix/fre/unexpected_live_vb_room_list branch from c1f2573 to fb7388d Compare January 6, 2023 14:01

Florian14 force-pushed the bugfix/fre/unexpected_live_vb_room_list branch from fb7388d to 21ca7c2 Compare January 9, 2023 08:14

Florian14 requested review from a team and mnaturel and removed request for a team January 9, 2023 08:14

mnaturel reviewed Jan 9, 2023

View reviewed changes

giomfo added the A-Voice Broadcast Broadcast-style voice messages label Jan 10, 2023

Florian Renaud added 5 commits January 11, 2023 17:31

Fix unexpected live voice broadcast in the room list

c63d6fa

Add changelog file

63dccb4

Add unit test

39c0cb2

Restore private visibility of fake component fields

2df9480

Use private method to mockk voice broadcast event

493fa7a

Florian14 force-pushed the bugfix/fre/unexpected_live_vb_room_list branch from 21ca7c2 to 6b60b0a Compare January 11, 2023 16:32

Room list - Do not show live broadcast if the started event is redacted

f62f661

Florian14 force-pushed the bugfix/fre/unexpected_live_vb_room_list branch from 6b60b0a to f62f661 Compare January 11, 2023 16:56

mnaturel approved these changes Jan 12, 2023

View reviewed changes

Florian14 merged commit b1d2581 into develop Jan 12, 2023

Florian14 deleted the bugfix/fre/unexpected_live_vb_room_list branch January 12, 2023 09:42

Fix unexpected live voice broadcast in the room list #7898

Fix unexpected live voice broadcast in the room list #7898

Uh oh!

Conversation

Florian14 commented Jan 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

Uh oh!

ElementBot commented Jan 5, 2023

Uh oh!

ElementBot commented Jan 6, 2023

Uh oh!

ElementBot commented Jan 6, 2023

Uh oh!

mnaturel left a comment

Choose a reason for hiding this comment

Uh oh!

mnaturel Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

Florian14 Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

mnaturel Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

Florian14 Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

ElementBot commented Jan 11, 2023

Uh oh!

ElementBot commented Jan 11, 2023

Uh oh!

Florian14 commented Jan 11, 2023

Uh oh!

ElementBot commented Jan 11, 2023

Uh oh!

sonarqubecloud bot commented Jan 11, 2023

Uh oh!

mnaturel left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Florian14 commented Jan 5, 2023 •

edited

Loading