Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

prevent unread event for room with empty timeline #9144

Closed
wants to merge 1 commit into from

Conversation

smoebody
Copy link

@smoebody smoebody commented Aug 8, 2022

fixes element-hq/element-web#19560

Checklist

  • Tests written for new code (and old code if feasible)
    No test perfomed on doesRoomHaveUnreadMessages
  • Linter and other CI checks pass
    Linter failed before my code already
  • Sign-off given on the changes (see CONTRIBUTING.md)

This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

@smoebody smoebody requested a review from a team as a code owner August 8, 2022 07:45
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Aug 8, 2022
Comment on lines +58 to +61
// if there is no timeline at all, e.g. by message retention
if (room.timeline.length === 0) {
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

This is insufficient, just because the client's loaded timeline has length 0, doesn't mean it has yet tried to backpaginate to see if there are any events. The server sends an unread count for a reason

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

It's not immediately clear to me that this is a solution to the issue at hand given the issue needs to be investigated and reviewed itself. In fact, the issue looks to be something we likely don't support or intend on supporting at this time, so a fix would be relatively inappropriate.

This PR might be best positioned to solve related issues though, so leaving it open for your consideration.

Thank you for taking the time to write a PR in any case!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clearing the cache creates spurious unread markers for empty rooms
3 participants