Skip to content
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

Voice Broadcast - Update last message in the room list #7719

Merged
merged 11 commits into from
Dec 14, 2022

Conversation

Florian14
Copy link
Contributor

@Florian14 Florian14 commented Dec 6, 2022

Type of change

  • WIP Feature
  • Bugfix
  • Technical
  • Other :

Content

Display "Live broadcast" in the room list if a VB is live
Show a notice in the room list and in the timeline when a voice broadcast is ended

Improve RoomSummary.latestPreviewableEvent computation in the sdk:

  • Event types with a reference relationship are not filtered anymore (was done to filter poll answers in encrypted events)
  • Filter the encrypted events by checking the clear type instead

Note: For the moment, voice messages related to a VB are not filtered out on the sdk side, if the computed latestPreviewableEvent is a VB message, the application will just display a blank instead.

Motivation and context

Continue #7127

Design request:
image

Screenshots / GIFs

voice_broadcast_last_message.mp4

Tests

  • Start a VB from device A
  • On device B, verify that the last message in the related room is "Live Broadcast" (even if the VB is paused/resumed)
  • Verify that new messages or typing events are not shown in the room list while the VB is live
  • Verify that a notice is display in the room list (last message) and in the timeline when the VB is stopped
  • Verify that voice messages related to the VB are not shown in the room list (if a voice message is catch after the stopped event, it will be blank instead)
  • Verify that typing events and new messages are shown again

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here

Generated by 🚫 dangerJS against ceeaaa9

@Florian14 Florian14 force-pushed the feature/fre/voice_broadcast_last_message branch from ceeaaa9 to 5a257be Compare December 7, 2022 09:28
@Florian14 Florian14 force-pushed the feature/fre/voice_broadcast_last_message branch from 5a257be to 59859ec Compare December 7, 2022 17:12
@Florian14 Florian14 requested review from a team and jonnyandrew and removed request for a team December 7, 2022 17:12
@Florian14 Florian14 marked this pull request as ready for review December 7, 2022 17:12
@sonarcloud
Copy link

sonarcloud bot commented Dec 7, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

8.9% 8.9% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@jonnyandrew jonnyandrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me.

There is however quite a bit of new logic but no tests added/changed. Is it worth adding some (at least for the Matrix SDK)?

@giomfo
Copy link
Member

giomfo commented Dec 14, 2022

Thanks for your review @jonnyandrew , we agree about your remark about the missing tests
In order to unblock merge of this PR, I created a dedicated issue to add tests on last message logic : #7780

@yostyle yostyle merged commit c74ea2d into develop Dec 14, 2022
@yostyle yostyle deleted the feature/fre/voice_broadcast_last_message branch December 14, 2022 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants