fix: clamp page if it exceeds the maximum page

[alex-lambdaloopers]

Hello,

This change was asked by a client complaining about the error when filtering and being on a high page.

Imagine we have 200 rows to display. The page_size is 10, hence the maximum page we should access is 20.

Now imagine that we want to filter out some of the rows, using the search parameter. After the search, there are 100 rows to display only, so the new maximum page is 10.

This parameter is applied after typing, and the current page parameter does not change. If we are in a page greater than the new maximum, we will be prompted with the error

raise HTTPException(
                status_code=400, detail="Invalid page or pageSize parameter"
            )

This seems like unintended behaviour to me. In my view, trying to access a page higher than the max in this situation should not trigger an error, but instead display the last page of them all.

This PR clamps the page to its maximum value if possible.

There is a caveat: the url displays the page that you specified, even though the UI will show the max page. so for example, the URL could be:

http://my-app/admin/some-item/list?search=someval&page=15

But the UI would show that we are on page 10.

[aminalaee]

Thanks for the PR, I think it makes sense.
Just an idea that maybe we could redirect to URL so that the caveat is avoided and if User enters page=10000 manually they will be redirected to page 10.

[alex-lambdaloopers]

agreed. I thought so too. One idea could be adding this:

    @login_required
    async def list(self, request: Request) -> Response:
        """List route to display paginated Model instances."""

        await self._list(request)

        model_view = self._find_model_view(request.path_params["identity"])
        pagination = await model_view.list(request)
        pagination.add_pagination_urls(request.url)

+        if request_page := request.query_params.get("page"):
+            request_page = model_view.validate_page_number(request_page)
+            if request_page > pagination.page:
+                return RedirectResponse(
+                    request.url.replace(query=f"page={pagination.page}")
+                )

in application.py. it feels clunky to me since we will need to execute model_view.list again.

Another option would be add some js on the list.html that would run the same comparison, replacing the url if needed. This also feels clunky to me.

Do you like any of those alternatives? Or if you have another idea I´m open to implementing it.

[aminalaee]

I thought we already had that logic in here: https://github.com/aminalaee/sqladmin/blame/e5e7d5073f5bd8a2a48adf1a30b8df4ffafdc647/sqladmin/application.py#L450

Instead of just raising an error we want to redirect. And the reason I think redirect is fine is that it shouldn't happen in a normal case, it's only when someone is entering the URL manually and it's wrong.

[alex-lambdaloopers]

you are correct. ill edit the pr

[alex-lambdaloopers]

it shouldn't happen in a normal case

the redirect would happen in the case described in this pr: we are on page 20 and add a search, then the max page becomes 10. We will call the list method, then redirect, then call the list method again. If this is not a problem for you it isn't for me

[aminalaee]

Yeah I think it's fine for now. I don't have a better solution and we can re-visit this later.

[alex-lambdaloopers]

ok updated. let me know what you think of this.

[aminalaee]

Minor comment, but I think this is a nice fix

[aminalaee]

Thanks