Move the front page to cursor based pagination#1985
Move the front page to cursor based pagination#1985BentiGorlich wants to merge 9 commits intomainfrom
Conversation
BentiGorlich
added
backend
BentiGorlich
force-pushed
the
new/front-cursor-pagination
branch
from
1069a55 to
fe09af6
Compare
BentiGorlich
force-pushed
the
new/front-cursor-pagination
branch
from
02e5a72 to
d147ba7
Compare
BentiGorlich
force-pushed
the
new/front-cursor-pagination
branch
from
d147ba7 to
5d48db3
Compare
| { | ||
| $initialCursor = $contentRepository->guessInitialCursor($criteria); | ||
| if ($initialCursor instanceof \DateTime || $initialCursor instanceof \DateTimeImmutable) { | ||
| $currentCursor = null !== $cursor ? new \DateTimeImmutable($cursor) : $initialCursor; |
There was a problem hiding this comment.
errors from converting the supplied cursor should be caught and wrapped in a bad-request exception with a meaningful message
(same for line 537)
There was a problem hiding this comment.
So do you mean the LogicException, or the DateMalformedStringException? Or just a general try catch around the method call?
There was a problem hiding this comment.
The DateMalformedStringException (and whatever else can occur from the date / int -conversation). It should result for the user in a 400 http response which also explicitly indicates that the cursor param was invalid.
The LogicException sounds more like a 500 error.
|
Regarding the design of the forward and back button: IMO it would look better if at the bottom (and maybe also on the top) there would be two buttons with an arrow to the left / right (spread so that they are on the opposite ends of the container). |
- instead of the usual `LIMIT`/`OFFSET` pagination use cursor based one based on the current ordering - Write classes that are inspired by the pagination of doctrine for cursor based pagination - make the `ContentRepository` work with the new pagination, while still working with the regular one
- add API endpoint to retrieve content using the cursor pagination - add tests for the `CombinedRetrieveApi` - make `CombinedRetrieveApi` actually returning combined content
…est cases - remove some unnecessary stuff from the `EntryFrontController` - Add test case for the `cursorUserCollection` endpoint in the `CombinedRetrieveApi` - assert the item counts in the pagination test
BentiGorlich
force-pushed
the
new/front-cursor-pagination
branch
from
5d48db3 to
d87b945
Compare
|
Regarding design: could you make a quick sketch, so I can see what you mean exactly? |
here you go |
|
Hmm ok... My though process was: "if I am at the bottom of a page, I will most likely want to go to the next page. If I am at the top then the previous page". |
2 participants
LIMIT/OFFSETpagination use cursor based one based on the current orderingContentRepositorywork with the new pagination, while still working with the regular oneLeft ToDos:
Fixes #1869