-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
recent_view: Fix error on initial fetch being all muted messages. #30122
Conversation
dbfe114
to
6804bb5
Compare
// This was likely our first fetch which didn't result in any messages being fetched. | ||
// We stay hopeful here and wait for our next fetch to update the banner. | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the bug is that we should be looking at first_including_muted
instead; we're guaranteed to have at least 1 message there after the first fetch unless the user has no message history at all. And it's accurate to say we're showing everything since the date of the first_including_muted message, even if there's things not displayed.
first(): Message {
return this._items[0];
}
first_including_muted(): Message {
return this._all_items[0];
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, makese sense. Updated!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote the comment and tweaked the check slightly to I think better reflect my understanding of the world. Please let me know if you see any issues with the changes.
See comments above. Also I've noticed that holding down Not sure if it's worth going deep into debugging if it seems hard, but maybe worth checking if there's an easy fix. |
7e21279
to
99044d8
Compare
If the initial fetch pre zulip#29740 fetches all muted messages we don't have any messages in all message list and hence query for oldest message is undefined. This results in us trying to render oldest_message_timestamp with its value as infinity. To fix it, we include muted messages in our search for oldest message in the list and if we still cannot find one, we wait for the next fetch.
99044d8
to
9fce91b
Compare
If the initial fetch pre #29740 fetches all muted messages we don't have any messages in all message list and hence query for oldest message is undefined.
This results in us trying to render oldest_message_timestamp with its value as infinity.
So, to fix it, we just wait for our other initial fetch of recent view to return us the messages we need.