feat: add dedicated methods for cursor based pagination [CAPI-2357] #2824
Conversation
…ods for entry, asset, and content-type [CAPI-2357] Adds getManyWithCursor as a dedicated cursor based method for entry, asset, and content-type and getPublishedWithCursor for only the asset and entry entities
…response and params and building utilities to do so [CAPI-2357] * fix: getPublished does not currently support cursor based pagination, removing relevant methods for entities * fix: normalize pagination params to filter prevPage and prevNext if falsey * fix: normalize pagination response to parse next, prev tokens if present
… for getManyWithCursor for content-type, asset, and entry entities [CAPI-2357]
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
…console statements, adding cbp to toc in readme [CAPI-2357]
ebefarooqui
force-pushed
the
feature/add-dedicated-methods-for-cursor-based-pagination
branch
from
2ac557b to
09e8e07
Compare
…2819) Co-authored-by: Ely Lucas <[email protected]>
…etManyWithCursor methods [CAPI-2357]
| } | ||
|
|
||
| export interface BasicCursorPaginationOptions extends Omit<BasicQueryOptions, 'skip'> { | ||
| skip?: never |
There was a problem hiding this comment.
Isn't this skip unnecessary given the Omit above?
There was a problem hiding this comment.
Why is this type used in all the non cursor based functions as signature? 🤔
There was a problem hiding this comment.
@ethan-ozelius-contentful It's there because unless strictly forbidden, the value can still be passed in at runtime. I wanted to make sure to have a type error there just in case.
@marcolink Could you point to where that is the case? I'm not sure I follow right now, apologies
There was a problem hiding this comment.
Here for example:
(not changed in this PR)
There was a problem hiding this comment.
I see. I'm not sure why, but can investigate. I was using the same type to idiomatically match how we were using it elsewhere.
There was a problem hiding this comment.
Unfortunately, changing that now would likely create a breaking change ...
There was a problem hiding this comment.
Agreed, we can make a point to investigate what's happening here and if need be spin up another ticket?
| }, | ||
|
|
||
| /** | ||
| * Gets a collection of Content Types with cursor based pagination |
There was a problem hiding this comment.
These two URLs listed in this @param should probably point to the follow URL right?
ethan-ozelius-contentful
left a comment
ethan-ozelius-contentful
left a comment
There was a problem hiding this comment.
Left 1-2 non-blocking questions, but otherwise looks good.
| console.log(response.items); // Array of items | ||
| console.log(response.pages?.next); // Cursor for next page | ||
| ``` | ||
| Use the value from `response.pages.next` to fetch the next page. |
There was a problem hiding this comment.
Should we also add an example on how to use the pages.next token?
| } | ||
|
|
||
| export interface BasicCursorPaginationOptions extends Omit<BasicQueryOptions, 'skip'> { | ||
| skip?: never |
There was a problem hiding this comment.
Why is this type used in all the non cursor based functions as signature? 🤔
…nation example to readme [CAPI-2357]
Summary
Introduces three dedicated cursor pagination methods, one per each relevant entity:
getAssetsWithCursorgetContentTypesWithCursorgetEntriesWithCursorDescription
These changes expose three new dedicated methods in the plain client for cursor based pagination. These methods map to the
getManyendpoint per each entity and are wrapped accordingly with the appropriate cursor pagination collection. Relevant unit and integration tests are also included.Motivation and Context
Previously we tried to provide correct typing for
cma.jsby checking the cursor parameter to be true. As the type system ofcma.jsis already quite complex, this introduced a bug we were not able to foresee. As a result, this was rolled back and the decision was taken to introduce dedicated methods for cursor-based pagination.PR Checklist
CONTRIBUTING.mdfile