fix: messages list IndexOutOfBoundsException [WPB-5612] #2457
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Issues
We were getting
IndexOutOfBoundsException
when opening conversation screen.Causes (Optional)
shouldHaveSmallBottomPadding
didn't have any limit, onlyindex > 0
, but index could be higher than the items count and that's what was happening, becauselazyPagingMessages.itemSnapshotList.items
is a list of only presented data, excluding placeholders. It was working fine when the app was always fetching messages starting from the most recent one, but in case the user has multiple unread messages or opens the conversation from the message search, the app may fetch not the last page but can start from the middle and then indexes don't match.For instance, if we have 100 total messages, page size is 20 and we have 30 unread messages, then the initially loaded batch should have first unread message, so it should contain for instance messages 20 to 39.
ItemSnapshotList
will have size of 100 (all available messages) and these fetched already elements will be at their proper indexes 20 to 39, all other elements are placeholders and they will be null, butItemSnapshotList.items
only contains these 20 non-null elements with indexes starting from 0 to 19, because it's just a sublist, and we can easily haveIndexOutOfBounds
when we want to get item with index 30 from that sublist, and items don't match -itemSnapshotList.items[10]
is not the same item asitemSnapshotList[10]
so the list could look wrong.Solutions
Get elements using their indexes from the
itemSnapshotList
instead ofitemSnapshotList.items
, usederivedStateOf
to reduce number of recompositions andremember
with keys to update value ofshouldHaveSmallBottomPadding
orshouldShowHeader
when a neighbouring element is no longer a placeholder.The whole list was also recomposing when scrolling, so it could be reduced as well by creating dedicated states for
isScrollInProgress
andmostRecentMessage
usingderivedStateOf
.Testing
Test Coverage (Optional)
How to Test
Have a lot of unread messages, like more than 50, and open conversation - it shouldn't crash and each message should look properly (without avatar, header and with small paddings in between if they are grouped messages from the same user).
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.