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

Unread messages divider did not appear in the message list when marking unread #3444

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

laevandus
Copy link
Contributor

@laevandus laevandus commented Oct 2, 2024

🔗 Issue Links

Resolves: PBE-5549

🎯 Goal

Fix an issue where unread messages divider did not appear on mark as unread

📝 Summary

  • Invalid unread message id was returned because of incorrect get message before implementation

🛠 Implementation

We need to find a previous message from the message we are marking as unread. API requires the last read message, not the first unread message id. The sorting logic and limiting the fetch to 1 ended up returning the oldest loaded message instead (filter matches, let's say 25 messages before limiting to 1, but we picked the id from the wrong end of results). Due to that reason, MessageListVC tried to reload cell for an older message instead (reloading is needed for making the cell to show the unread messages divider).

🎨 Showcase

Before After
img img

🧪 Manual Testing Notes

  1. Open a channel
  2. Mark the last message as unread
    Result: Unread message divider appears.

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • This change should be manually QAed
  • Changelog is updated with client-facing changes
  • Changelog is updated with new localization keys
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

@laevandus laevandus added 🐞 Bug An issue or PR related to a bug 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK 🤞 Ready For QA A PR that is Ready for QA labels Oct 2, 2024
@laevandus laevandus requested a review from a team as a code owner October 2, 2024 12:16
@Stream-SDK-Bot
Copy link
Collaborator

SDK Size

title develop branch diff status
StreamChat 6.9 MB 6.9 MB 0 KB 🟢
StreamChatUI 4.95 MB 4.95 MB 0 KB 🟢

@Stream-SDK-Bot
Copy link
Collaborator

SDK Performance

target metric benchmark branch performance status
MessageList Hitches total duration 10 ms 0.0 ms 100.0% 🔼 🟢
Duration 2.6 s 2.55 s 1.92% 🔼 🟢
Hitch time ratio 4 ms per s 0.0 ms per s 100.0% 🔼 🟢
Frame rate 75 fps 78.65 fps 4.87% 🔼 🟢
Number of hitches 1 0.0 100.0% 🔼 🟢
ChannelList Hitches total duration 12.5 ms 10.85 ms 13.2% 🔼 🟢
Duration 2.6 s 2.55 s 1.92% 🔼 🟢
Hitch time ratio 5 ms per s 4.26 ms per s 14.8% 🔼 🟢
Frame rate 72 fps 74.72 fps 3.78% 🔼 🟢
Number of hitches 1.2 0.8 33.33% 🔼 🟢

Copy link

github-actions bot commented Oct 2, 2024

1 Warning
⚠️ The changes should be manually QAed before the Pull Request will be merged

Generated by 🚫 Danger

@laevandus laevandus force-pushed the fix/unread-message-divider-hidden branch from 6832a7d to 7574866 Compare October 4, 2024 08:32
@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Oct 4, 2024

SDK Size

title develop branch diff status
StreamChat 6.9 MB 6.9 MB 0 KB 🟢
StreamChatUI 4.95 MB 4.95 MB 0 KB 🟢

@laevandus laevandus force-pushed the fix/unread-message-divider-hidden branch from 7574866 to dec8476 Compare October 8, 2024 08:43
Copy link

sonarqubecloud bot commented Oct 9, 2024

@testableapple testableapple added 🟢 QAed A PR that was QAed and removed 🤞 Ready For QA A PR that is Ready for QA labels Oct 9, 2024
@laevandus laevandus merged commit 996ec3b into develop Oct 9, 2024
15 checks passed
@laevandus laevandus deleted the fix/unread-message-divider-hidden branch October 9, 2024 09:06
@Stream-SDK-Bot Stream-SDK-Bot mentioned this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug 🟢 QAed A PR that was QAed 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants