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

REST API Pagination Does not work as expected #26986

Closed
lbajsarowicz opened this issue Feb 23, 2020 · 4 comments · Fixed by #26988
Closed

REST API Pagination Does not work as expected #26986

lbajsarowicz opened this issue Feb 23, 2020 · 4 comments · Fixed by #26988
Assignees
Labels
Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reported on 2.3.4 Indicates original Magento version for the Issue report.

Comments

@lbajsarowicz
Copy link
Contributor

Preconditions (*)

  1. Magento 2.3.4
  2. REST API Call

Steps to reproduce (*)

  1. Fixture with 3 orders
  2. Request with page_size = 1 and current_page = 99

Expected result (*)

  1. total_count returns 3
  2. items returns empty array OR OutOfRangeException

Actual result (*)

  1. total_count is 3
  2. items returns repeated last page of results

Kudos for

@ihor-sviziev , @Vinai , @nuzil for discussion at #appdesign channel about this issue.
image
image
image

@m2-assistant
Copy link

m2-assistant bot commented Feb 23, 2020

Hi @lbajsarowicz. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

@lbajsarowicz do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Feb 23, 2020
@lbajsarowicz
Copy link
Contributor Author

lbajsarowicz commented Feb 23, 2020

I'm going to address the issue with data described with dataProvider:

/**
* Keep in mind: Fixture contains 5 products
*
* @return array
*/
public function productPaginationDataProvider()
{
return [
'expect-all-items' => [
'pageSize' => 10,
'currentPage' => 1,
'expectedCount' => 5
],
'expect-page=size-items' => [
'pageSize' => 2,
'currentPage' => 1,
'expectedCount' => 2
],
'expect-less-than-pagesize-elements' => [
'pageSize' => 3,
'currentPage' => 2,
'expectedCount' => 2
],
'expect-no-items' => [
'pageSize' => 100,
'currentPage' => 99,
'expectedCount' => 0
]
];
}

lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 23, 2020
@ghost ghost assigned lbajsarowicz Feb 23, 2020
@lbajsarowicz
Copy link
Contributor Author

The source of the issue is located in:

public function getCurPage($displacement = 0)
{
if ($this->_curPage + $displacement < 1) {
return 1;
} elseif ($this->_curPage + $displacement > $this->getLastPageNumber()) {
return $this->getLastPageNumber();
} else {
return $this->_curPage + $displacement;
}
}

Although $this->_curPage is overwritten by the

    if ($this->_curPage + $displacement > $this->getLastPageNumber()) {
        return $this->getLastPageNumber();
    }

That is not passed back to all APIs. There are two options, actually:

  1. Remove above-mentioned mechanism to return no values when we reached the last page.
  2. Pass the information about overwritten currentPage to API

I choose (1) as for me it's more obvious.

lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 23, 2020
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 24, 2020
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 24, 2020
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 24, 2020
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 24, 2020
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 24, 2020
@sdzhepa sdzhepa linked a pull request Feb 24, 2020 that will close this issue
4 tasks
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 24, 2020
lbajsarowicz added a commit to lbajsarowicz/magento2 that referenced this issue Feb 25, 2020
@ghost ghost assigned lbajsarowicz and unassigned lbajsarowicz Feb 28, 2020
@magento-engcom-team
Copy link
Contributor

Hi @lbajsarowicz. Thank you for your report.
The issue has been fixed in #26988 by @lbajsarowicz in 2.4-develop branch
Related commit(s):

The fix will be available with the upcoming 2.4.0 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label Apr 10, 2020
magento-engcom-team added a commit that referenced this issue Apr 10, 2020
 - Merge Pull Request #26988 from lbajsarowicz/magento2:bugfix/rest-api-pagination
 - Merged commits:
   1. a6f76f3
   2. b254764
   3. 11567f6
   4. 6771122
   5. e001493
   6. ecb3a8d
   7. 015c5fc
   8. 18e43a8
   9. 9d0ae13
   10. 2439e17
   11. 14ea335
   12. 0fe9cf2
   13. dab0cda
   14. c403c52
   15. d7cdd7b
   16. 05f1a2b
   17. f25de7b
   18. 51a18f6
   19. 0bae5d5
   20. 1e7082d
   21. f5aa61d
   22. e8d3b78
   23. 6c171ce
   24. 8c9189f
   25. 68cf64d
   26. 0db9220
   27. c44bbf5
   28. 84fdc7a
   29. 162eb14
   30. 11c3eea
   31. 986a4ef
   32. 80cb06e
   33. db71c46
   34. e105152
   35. bd5c98e
@magento-engcom-team magento-engcom-team added the Reported on 2.3.4 Indicates original Magento version for the Issue report. label Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reported on 2.3.4 Indicates original Magento version for the Issue report.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants