feat: add cursor pagination on chat.syncMessages #33273
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #33273 +/- ##
===========================================
- Coverage 59.79% 59.68% -0.12%
===========================================
Files 2548 2551 +3
Lines 63412 63029 -383
Branches 14267 14123 -144
===========================================
- Hits 37920 37617 -303
+ Misses 23076 22996 -80
Partials 2416 2416
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -79,28 +79,42 @@ API.v1.addRoute( | |||
| { authRequired: true }, | |||
| { | |||
| async get() { | |||
| const { roomId, lastUpdate } = this.queryParams; | |||
| const { roomId, lastUpdate, count, next, previous, type } = this.queryParams; | |||
There was a problem hiding this comment.
Shouldn't we validate the type of these new parameters as well? AJV maybe? 🤔
| }); | ||
| const response = lastUpdate | ||
| ? await handleWithoutPagination(rid, lastUpdate) | ||
| : await handleCursorPagination(type ?? 'UPDATED', rid, count, next, previous); |
There was a problem hiding this comment.
Why are we using Updated as default?
| return null; | ||
| } | ||
|
|
||
| function popMessagesFromArray(messages: IMessage[], count?: number) { |
There was a problem hiding this comment.
I think we could pop the message directly there, since it's not being reused, we should not create this function.
| }>; | ||
| } | ||
| } | ||
|
|
||
| function extractTimestampFromCursor(cursor: string): Date { |
There was a problem hiding this comment.
Let's please add unit tests for this function.
| return new Date(parseInt(cursor, 10)); | ||
| } | ||
|
|
||
| function mountCursorQuery({ next, previous, count }: { next?: string; previous?: string; count: number }): { |
There was a problem hiding this comment.
Let's please add unit tests for this function.
| return { query: { $gt: new Date(0) }, options }; | ||
| } | ||
|
|
||
| function mountCursorFromMessage(message: IMessage & { _deletedAt?: Date }, type: 'UPDATED' | 'DELETED'): string { |
There was a problem hiding this comment.
Let's please add unit tests for this function.
| throw new Meteor.Error('error-cursor-not-found', 'Cursor not found', { method: 'messages/get' }); | ||
| } | ||
|
|
||
| function mountNextCursor(messages: IMessage[], type: CursorPaginationType, next?: string, previous?: string): string | null { |
There was a problem hiding this comment.
Let's please add unit tests for this function.
| return null; | ||
| } | ||
|
|
||
| function mountPreviousCursor(messages: IMessage[], type: CursorPaginationType, count?: number, next?: string): string | null { |
There was a problem hiding this comment.
Let's please add unit tests for this function.
| .end(done); | ||
| }); | ||
|
|
||
| it('should return an error when the neither "lastUpdate", "next" or "previous" parameter is sent', (done) => { |
There was a problem hiding this comment.
| it('should return an error when the neither "lastUpdate", "next" or "previous" parameter is sent', (done) => { | |
| it('should return an error when neither "lastUpdate", "next" or "previous" parameter is sent', (done) => { |
packages/rest-typings/src/v1/chat.ts
Outdated
There was a problem hiding this comment.
Probably this is an old problem, but we are not using this validation on the endpoint 😶
ricardogarim
force-pushed
the
feat/add-cursor-pagination-support-on-syncMessages
branch
from
1ee2ef8 to
465c8b8
Compare
As per CORE-686, 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 parameters, users will need to pass next or previous, along with count and type.
nextorprevious: 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 retrieveDELETED(for deleted messages) orUPDATED(for updated messages, which is the default use case).Notes:
lastUpdatewill continue to have the same experience, with no changes required on their end.