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

Fix missing messages when forward paging with chunks > 50 messages #5450

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

SpiritCroc
Copy link
Contributor

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • offsets() was not limiting in the right direction when loading
    messages forwards
  • after fixing offsets(), more recent messages would not be loaded due
    to the isLastForward() check, so better prioritize the SUCCESS
    LoadMoreResult over the REACHED_END here

Motivation and context

Fixes #5448

Tests

  1. Permalink the latest message in a room
  2. Go back to room list
  3. Receive lots of new messages in that room (e.g. 100)
  4. Open the room from the permalink
  5. Check if all messages are shown

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 11

Checklist

Signed-off-by: Tobias Büttner [email protected]

- offsets() was not limiting in the right direction when loading
  messages forwards
- after fixing offsets(), more recent messages would not be loaded due
  to the isLastForward() check, so better prioritize the SUCCESS
  LoadMoreResult over the REACHED_END here
@bmarty bmarty added the Z-Community-PR Issue is solved by a community member's PR label Mar 8, 2022
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks for your PR.
LGTM, but I do not see your sign off. Can you add a comment to the PR please? (this is why I request change here)

Also @ganfra can you have a second check on the changes please?

@SpiritCroc
Copy link
Contributor Author

Thanks for your PR. LGTM, but I do not see your sign off. Can you add a comment to the PR please? (this is why I request change here)

It's in the last line of the PR message, should I move it?

Copy link
Member

@ganfra ganfra left a comment

Choose a reason for hiding this comment

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

Good catch, thanks!

@bmarty
Copy link
Member

bmarty commented Mar 10, 2022

Thanks for your PR. LGTM, but I do not see your sign off. Can you add a comment to the PR please? (this is why I request change here)

It's in the last line of the PR message, should I move it?

Ah no, that's fine, sorry I was blind!

@bmarty bmarty merged commit 66f76fb into element-hq:develop Mar 10, 2022
@SpiritCroc SpiritCroc deleted the fix-5448 branch March 10, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

Messages missing/skipped when opening a room at a permalink
3 participants