Check MaxPageSize before using PageSize throughout the code

[MikeAlhayek]

Describe the bug

The only place we check against MaxPageSize in on OC.Contents.AdminController

The logic should be

if (_pagerOptions.MaxPageSize > 0 && pager.PageSize > _pagerOptions.MaxPageSize)
{
    pager.PageSize = _pagerOptions.MaxPagedCount;
}

I suggest we create GetPageSize() method that looks like this and use irt instead of using PageSize property

    public int GetPageSize()
    {
        if (MaxPageSize > 0 && PageSize > MaxPageSize)
        {
            return MaxPageSize;
        }

        return PageSize > 0 ? PageSize : DefaultPageSize;
    }

Also, something went wrong with recent merge where these 2 code blocks were added instead of replacing the second block

[jtkech]

So, MaxPageSize limit the number of items per page, and MaxPagedCount limit the total number of items we query. But as I understand the previous code, MaxPagedCount was also used to limit MaxPageSize, to not specify a number per page > total items count, unless we assume that MaxPageSize is always less than MaxPagedCount, but they are settings that can be updated and where 0 means don't use it.

So maybe MaxPagedCount should still be also used to limit MaxPageSize, it should if we want to keep the previous behavior.

[MikeAlhayek]

@jtkech Why would we want to check MaxPagedCount to determine the PageSize? MaxPagedCount is only to limit the total records coming from the database. I don’t think it should have to do with determining the page size

[jtkech]

Because we init the pager TotalItemCount (here the pager shape) with it and we don't want an use case with the pager PageSize being higher than the pager shape TotalItemCount.

[MikeAlhayek]

@jtkech I get that, but in that case we just render all the items on 1 page. Are you saying having TotalItemCount higher values that PageSize will cause issue?

[jtkech]

Yes I think you're right, in that case it will render a List of items without the pager.

Are you saying having TotalItemCount higher values that PageSize will cause issue?

No because this is in that case that the pager is used.

So looks good but maybe check what the pagerShape code does with pagerSize and TotalItemCount.

[MikeAlhayek]

Yes I already checked.

OrchardCore/src/OrchardCore.Modules/OrchardCore.Navigation/PagerShapes.cs

Line 173 in 40d3f65

var totalPageCount = pageSize > 0 ? (int)Math.Ceiling(TotalItemCount / pageSize) : 1;

It returns 1 so checking MaxPagedCount is not needed

[jtkech]

Okay cool then