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

feat: add cursor pagination support on sync messages #33810

Merged
merged 26 commits into from
Nov 19, 2024

Conversation

ricardogarim
Copy link
Contributor

@ricardogarim ricardogarim commented Oct 28, 2024

As per CORE-686 and CORE-682, this PR introduces cursor pagination to the messages/get Meteor method route, which supports the chat.syncMessages route. This improvement allows users to retrieve updates from a given date more efficiently, without compromising response times.

While cursor pagination is now available, it does not affect any existing behaviors. To use cursor pagination, instead of passing lastUpdate as query parameter, users will need to pass next or previous, along with count and type.

  • next or previous: These represent the cursor pointers indicating whether the query should retrieve items from a later or earlier point in time. The cursor value must be the number of milliseconds, as it follows Date getTime().
  • count: Specifies the number of records to return.
  • type: With the introduction of cursors, both updated and deleted messages cannot be retrieved in the same response. Users must specify whether they want to retrieve DELETED (for deleted messages) or UPDATED (for updated messages, which is the default use case).

Notes:

  • In the future, it might be worth considering isolating this functionality into a separate route. However, for now, the current implementation ensures no conflicts or breaking changes, and the route’s signature remains unchanged.
  • This implementation has been tested to ensure that existing users relying on lastUpdate will continue to have the same experience, with no changes required on their end.
  • Keep in mind that while performance is acceptable for now, there may be opportunities to optimize database indexes further. Pagination has been a missing feature, so addressing it was a high priority.
  • This change is especially beneficial for users who frequently work with large volumes of messages, such as when a username with a high message count in a room is changed, as cursor pagination offers a more efficient way to paginate through results.

Edit: As per OPI-53, it adds cursor null conditions when there's no more records to fetch.

Copy link

changeset-bot bot commented Oct 28, 2024

🦋 Changeset detected

Latest commit: 3921018

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 35 packages
Name Type
@rocket.chat/rest-typings Minor
@rocket.chat/meteor Minor
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/ui-contexts Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/network-broker Patch
@rocket.chat/livechat Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/ui-avatar Major
@rocket.chat/ui-client Major
@rocket.chat/ui-video-conf Major
@rocket.chat/ui-voip Major
@rocket.chat/web-ui-registration Major
@rocket.chat/core-typings Minor
@rocket.chat/apps Patch
@rocket.chat/cron Patch
@rocket.chat/freeswitch Patch
@rocket.chat/model-typings Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/models Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

dionisio-bot bot commented Oct 28, 2024

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@ricardogarim ricardogarim force-pushed the feat/add-cursor-pagination-support-on-syncMessages branch from 1ee2ef8 to 3f3d255 Compare October 28, 2024 17:00
Copy link
Contributor

github-actions bot commented Oct 28, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-33810/
on branch gh-pages at 2024-11-19 17:55 UTC

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

Attention: Patch coverage is 69.86301% with 22 lines in your changes missing coverage. Please review.

Project coverage is 75.16%. Comparing base (6ba7372) to head (30d44c8).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #33810      +/-   ##
===========================================
- Coverage    75.18%   75.16%   -0.03%     
===========================================
  Files          495      497       +2     
  Lines        21600    21688      +88     
  Branches      5362     5392      +30     
===========================================
+ Hits         16241    16301      +60     
- Misses        4717     4743      +26     
- Partials       642      644       +2     
Flag Coverage Δ
unit 75.16% <69.86%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

---- 🚨 Try these New Features:

@ricardogarim ricardogarim added this to the 7.1.0 milestone Oct 29, 2024
Copy link
Member

@MarcosSpessatto MarcosSpessatto left a comment

Choose a reason for hiding this comment

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

Can we add a changeset?

@ricardogarim ricardogarim marked this pull request as ready for review November 4, 2024 13:44
@ricardogarim ricardogarim requested review from a team as code owners November 4, 2024 13:44
.changeset/mean-cobras-sneeze.md Outdated Show resolved Hide resolved
@scuciatto scuciatto added the stat: QA assured Means it has been tested and approved by a company insider label Nov 19, 2024
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Nov 19, 2024
@ggazzo ggazzo merged commit e7edeac into develop Nov 19, 2024
2 checks passed
@ggazzo ggazzo deleted the feat/add-cursor-pagination-support-on-syncMessages branch November 19, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants