Skip to content

Paginate the notes incremental fetch endpoint

Nick Thomas requested to merge 217673-keyset-paginate-notes-backend-only into master

What does this MR do?

This is an extraction from !33619 (closed) , which contains (rudimentary) frontend + backend changes.

Returning an unlimited number of elements from any endpoint is a bad idea. For the notes endpoint specifically, we return all notes since a given X-Last-Fetched-At value. The backend specifies the new value to use for that item in the response, so if we return a limited set of notes and use an appropriate value, we get a rudimentary form of keyset pagination for free.

The way the notes endpoint is used today, this is mostly a theoretical worry, and in practice, basically every response will fit within a single page anyway. However, we can use the notes endpoint instead of the discussions endpoint to fill a merge request discussion from the beginning of time. When we do this, large discussions are loaded in batches automatically, using pre-existing frontend code. So this change is best seen as preparatory work for removing discussions altogether.

This commit also fixes a pre-existing problem with the endpoint, which is the fact that updated_at is stored with microsecond precision in the database. Converting the X-Last-Fetched-At value from integer to float allows us to avoid a range of boundary conditions that result from the difference in resolution between the two levels.

Screenshots

No FE-visible changes, but here's a screenshot of some incrementally-added notes in an MR to demonstrate that the functionality continues to work:

Screenshot_from_2020-06-16_14-19-32

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

If a very large number of notes is created within a short period, they'll take slightly longer to be shown in the frontend. However, you won't have finished reading the first 50 new notes before the next batch comes in.

Part of #217673 (closed)

Edited by Mayra Cabrera

Merge request reports