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

The "last page" link in bukuserver doesn't use the current index #606

Closed
LeXofLeviafan opened this issue Nov 12, 2022 · 8 comments · Fixed by #630
Closed

The "last page" link in bukuserver doesn't use the current index #606

LeXofLeviafan opened this issue Nov 12, 2022 · 8 comments · Fixed by #630

Comments

@LeXofLeviafan
Copy link
Collaborator

When clicking on the "last page" link in bukuserver pagination (»), the page that's being opened isn't the current last page but the one that was last at the time of the current page rendering; if the bookmarks DB had been updated since then (i.e. there was enough bookmarks added/removed to change the amount of pages), the link would be pointing on a page which isn't the last one, or even a non-existing one.

A correct way to open the last page in the list would be to send a parameter requesting the actual last page (such as page=last), to which the server could redirect to the current last page (e.g. using HTTP 302), or the URL could be updated by the client in-browser.

@rachmadaniHaryono
Copy link
Collaborator

rachmadaniHaryono commented Nov 12, 2022

i can't decide about this one

first this is flask-admin feature

quick search for last page show no issue (only first page, but i will continue to search it later)

if we want to implement this, it mean several things

  • edit route from flask-admin to accept page=last query
  • edit the link for that character. i suspect we have to create new html template that inherit flask-admin template

looking at that i don't know if the change is worth it

@jarun comment?

e: TIL it is called double angle quotation mark

https://en.wikipedia.org/wiki/Guillemet

@jarun
Copy link
Owner

jarun commented Nov 12, 2022

Please leave it as it is. Not a big deal.

@rachmadaniHaryono
Copy link
Collaborator

i think this issue can be closed for now

reopen this if flask-admin have easier method to implement page=last query or someone can hack flask-admin just like above description

@LeXofLeviafan
Copy link
Collaborator Author

LeXofLeviafan commented Dec 1, 2022

…After I've done some fiddling with flask/flask-admin, I can say for sure that this URL can be modified at runtime (i.e. after the page is rendered) to point at a custom redirect endpoint (or just set page number as -1 and add a conditional redirect in get_list).

LeXofLeviafan added a commit to LeXofLeviafan/buku that referenced this issue Dec 2, 2022
@LeXofLeviafan
Copy link
Collaborator Author

…Correction: apparently redirects only work as return values, so get_list doesn't support those (and using it to precalculate the list requires preprocessing parameters).

@LeXofLeviafan
Copy link
Collaborator Author

…I've made a working implementation in a feature branch; it can be adopted as a pull request should you decide to go through with it.

@jarun
Copy link
Owner

jarun commented Dec 2, 2022

Please raise the PR.

@rachmadaniHaryono
Copy link
Collaborator

reopen this because there is ongoing pr

@jarun jarun closed this as completed in #630 Dec 3, 2022
jarun pushed a commit that referenced this issue Dec 3, 2022
* [#606] implemented a generic last-page endpoint

* PR edit
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants