currentPageOutOfRange bug fix

[crowd-studio]

If page is greater than 1 throws currentPageOutOfRange error becouse the comparison is wrong, it need to be equal o more than 1 or less than max pages.

[sampart]

Hi @crowd-studio, thanks for the PR. I'm not sure that the original is wrong, though. The point of the original is that 1 is a valid page (regardless of the value from $this->getNbPages()) but that the value must not exceed $this->getNbPages().

Could you provide a code example that shows the failure you describe please? That'll help us work out the way forward here.

[crowd-studio]

Hi @sampart

Sorry for my bad english:

The currentPageOutOfRange function returns true if the page is out of range and false if not.

Now is:

return $currentPage > 1 && $currentPage > $this->getNbPages();

So, returns true (out of range) if page is numbered greater than 1 and is greater than the maximum (NbPages) and return false (in the range) if is numbered 1 or less. Only will work with page number 1, if you try any other page it will throw 'out of range' error.

This is incorrect. It should be:

return $currentPage < 1 && $currentPage > $this->getNbPages();

Returns true (out of range) if page is numbered less than one ore greater than the maximum number of pages (NbPages) and returns false (in the range) if page is numbered between 1 and maximum number of pages (NbPages).

[sampart]

Hi again,

The case of the page number being less than 1 is already covered by the checkCurrentPage function (see here), which is called before the function you've changed.

You said:

Only will work with page number 1, if you try any other page it will throw 'out of range' error.

This is only true if $this->getNbPages(); returns 1. Imagine that $this->getNbPages(); returned 5. In this case, the error would be thrown for numbers greater than 5, but lower numbers would be fine.

In short, I'm not sure a change is needed here - the < 1 case is covered already, and the existing logic in currentPageOutOfRange seems sound to me, as it supports any page number <= the number of pages with the special case that 1 is always legitimate (e.g. even if there are no results).

As I said, if you could show me some code of yours that is erroring because of the current behaviour, that would be helpful here.

Thanks

[crowd-studio]

My code:

$pagerFanta = $finder->findPaginated($finalQuery); $pagerFanta->setMaxPerPage($size); $pagerFanta->setCurrentPage($page); $results = $pagerFanta->getCurrentPageResults();

If $page is 1 all work great but fail (OutOfRange) if is 2.
The NbPages is 5

[sampart]

Sorry, I'm still confused by your example. You claim that you're passing in a value of 2 and that NbPages is 5. Consider the code at present:

$currentPage > 1 && $currentPage > $this->getNbPages()

This will return false (i.e. "not out of range") for the conditions that you describe, which is what you want.

If this is returning true, you're either passing in a value different to what you've told me, or NbPages is different to what you've told me. I suggest that you do some step-through debugging to check values here are what you think they are.