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

#26986 REST API Pagination issue #26988

Merged
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
a6f76f3
#26986 API Functional Test to cover Pagination issue
lbajsarowicz Feb 23, 2020
b254764
#26986 Remove "feature" that overrides the currentPage value
lbajsarowicz Feb 23, 2020
11567f6
Fix static tests (`@return` alignment)
lbajsarowicz Feb 23, 2020
6771122
Merge remote-tracking branch 'origin/2.4-develop' into bugfix/rest-ap…
lbajsarowicz Feb 23, 2020
e001493
#26986 Fix invalid value returned for "records found" section in Admi…
lbajsarowicz Feb 24, 2020
ecb3a8d
#26986 Fix Static checks for Collection file
lbajsarowicz Feb 24, 2020
015c5fc
#26986 Fix failing Functional Test (assert empty page)
lbajsarowicz Feb 24, 2020
18e43a8
#26986 Fix failing Unit Tests
lbajsarowicz Feb 24, 2020
9d0ae13
#26986 Fix failing Functional Test: Change of behaviour when loading …
lbajsarowicz Feb 24, 2020
2439e17
#26986 Add missing PageSize to the ElasticSearch results.
lbajsarowicz Feb 24, 2020
14ea335
Add missing Copyright
lbajsarowicz Feb 24, 2020
0fe9cf2
#26986 Fix failing Functional Test: Do not display repeated products
lbajsarowicz Feb 25, 2020
dab0cda
Fix failing test on ElasticSearch Suite
lbajsarowicz Feb 25, 2020
c403c52
Fix failing Functional Tests
lbajsarowicz Feb 25, 2020
d7cdd7b
I have no idea why it should work
lbajsarowicz Feb 25, 2020
05f1a2b
Redirect to first page after changing `product_list_limit` to value h…
lbajsarowicz Feb 28, 2020
f25de7b
Introduce redirect pattern during change of limit
lbajsarowicz Feb 28, 2020
51a18f6
Fix invalid redirect pattern
lbajsarowicz Feb 28, 2020
0bae5d5
Static Tests fix
lbajsarowicz Feb 28, 2020
1e7082d
Merge remote-tracking branch 'origin/2.4-develop' into bugfix/rest-ap…
lbajsarowicz Feb 28, 2020
f5aa61d
One more static tests fix
lbajsarowicz Feb 28, 2020
e8d3b78
Fix Functional tests for pagination
lbajsarowicz Feb 29, 2020
6c171ce
Merge remote-tracking branch 'origin/2.4-develop' into bugfix/rest-ap…
lbajsarowicz Feb 29, 2020
8c9189f
Merge branch '2.4-develop' into bugfix/rest-api-pagination
ihor-sviziev Mar 3, 2020
68cf64d
Merge remote-tracking branch 'origin/2.4-develop' into bugfix/rest-ap…
lbajsarowicz Mar 7, 2020
0db9220
Merge remote-tracking branch 'lbajsarowicz/bugfix/rest-api-pagination…
lbajsarowicz Mar 7, 2020
c44bbf5
Merge branch '2.4-develop' into bugfix/rest-api-pagination
lbajsarowicz Mar 13, 2020
84fdc7a
Merge branch '2.4-develop' into bugfix/rest-api-pagination
lenaorobei Mar 24, 2020
162eb14
Merge remote-tracking branch 'origin/2.4-develop' into bugfix/rest-ap…
lbajsarowicz Mar 25, 2020
11c3eea
Reduce the complexity proudly introduced `(╯°□°)╯︵ ┻━┻`
lbajsarowicz Mar 25, 2020
986a4ef
Merge remote-tracking branch 'lbajsarowicz/bugfix/rest-api-pagination…
lbajsarowicz Mar 25, 2020
80cb06e
Extract the problematic test to separate file `(╯°□°)╯︵ ┻━┻`
lbajsarowicz Mar 26, 2020
db71c46
Add missing Copyright
lbajsarowicz Mar 26, 2020
e105152
Merge branch '2.4-develop' into bugfix/rest-api-pagination
lenaorobei Mar 26, 2020
bd5c98e
Merge branch '2.4-develop' into bugfix/rest-api-pagination
lenaorobei Mar 27, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 40 additions & 34 deletions app/code/Magento/Catalog/Model/ResourceModel/Product/Collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Magento\Catalog\Model\Indexer\Product\Price\PriceTableResolver;
use Magento\Catalog\Model\Product\Attribute\Source\Status as ProductStatus;
use Magento\Catalog\Model\Product\Gallery\ReadHandler as GalleryReadHandler;
use Magento\Catalog\Model\ResourceModel\Category;
use Magento\Catalog\Model\ResourceModel\Product\Collection\ProductLimitationFactory;
use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
use Magento\CatalogUrlRewrite\Model\Storage\DbStorage;
Expand All @@ -23,7 +24,6 @@
use Magento\Framework\Indexer\DimensionFactory;
use Magento\Store\Model\Indexer\WebsiteDimensionProvider;
use Magento\Store\Model\Store;
use Magento\Catalog\Model\ResourceModel\Category;

/**
* Product collection
Expand Down Expand Up @@ -1180,9 +1180,33 @@ protected function _getSelectCountSql(?Select $select = null, $resetLeftJoins =
if ($resetLeftJoins) {
$countSelect->resetJoinLeft();
}

$this->removeEntityIdentifierFromGroupBy($countSelect);

return $countSelect;
}

/**
* Using `entity_id` for `GROUP BY` causes COUNT() return {n} rows of value = 1 instead of 1 row of value {n}
*
* @param Select $select
* @throws \Zend_Db_Select_Exception
*/
private function removeEntityIdentifierFromGroupBy(Select $select): void
{
$originalGroupBy = $select->getPart(Select::GROUP);

if (!is_array($originalGroupBy)) {
return;
}

$groupBy = array_filter($originalGroupBy, function ($field) {
return false === strpos($field, $this->getIdFieldName());
});

$select->setPart(Select::GROUP, $groupBy);
}

/**
* Prepare statistics data
*
Expand Down Expand Up @@ -1765,30 +1789,19 @@ public function addAttributeToSort($attribute, $dir = self::SORT_ORDER_ASC)
*/
protected function _prepareProductLimitationFilters()
{
if (isset(
$this->_productLimitationFilters['visibility']
) && !isset(
$this->_productLimitationFilters['store_id']
)
) {
if (isset($this->_productLimitationFilters['visibility'])
&& !isset($this->_productLimitationFilters['store_id'])) {
$this->_productLimitationFilters['store_id'] = $this->getStoreId();
}
if (isset(
$this->_productLimitationFilters['category_id']
) && !isset(
$this->_productLimitationFilters['store_id']
)
) {

if (isset($this->_productLimitationFilters['category_id'])
&& !isset($this->_productLimitationFilters['store_id'])) {
$this->_productLimitationFilters['store_id'] = $this->getStoreId();
}
if (isset(
$this->_productLimitationFilters['store_id']
) && isset(
$this->_productLimitationFilters['visibility']
) && !isset(
$this->_productLimitationFilters['category_id']
)
) {

if (isset($this->_productLimitationFilters['store_id'])
&& isset($this->_productLimitationFilters['visibility'])
&& !isset($this->_productLimitationFilters['category_id'])) {
$this->_productLimitationFilters['category_id'] = $this->_storeManager->getStore(
$this->_productLimitationFilters['store_id']
)->getRootCategoryId();
Expand Down Expand Up @@ -1819,14 +1832,8 @@ protected function _productLimitationJoinWebsite()
$filters['website_ids'],
'int'
);
} elseif (isset(
$filters['store_id']
) && (!isset(
$filters['visibility']
) && !isset(
$filters['category_id']
)) && !$this->isEnabledFlat()
) {
} elseif (isset($filters['store_id']) && !$this->isEnabledFlat()
&& (!isset($filters['visibility']) && !isset($filters['category_id']))) {
$joinWebsite = true;
$websiteId = $this->_storeManager->getStore($filters['store_id'])->getWebsiteId();
$conditions[] = $this->getConnection()->quoteInto('product_website.website_id = ?', $websiteId, 'int');
Expand Down Expand Up @@ -1901,9 +1908,9 @@ protected function _productLimitationJoinPrice()
/**
* Join Product Price Table with left-join possibility
*
* @see \Magento\Catalog\Model\ResourceModel\Product\Collection::_productLimitationJoinPrice()
* @param bool $joinLeft
* @return $this
* @see \Magento\Catalog\Model\ResourceModel\Product\Collection::_productLimitationJoinPrice()
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
protected function _productLimitationPrice($joinLeft = false)
Expand Down Expand Up @@ -2335,8 +2342,8 @@ public function addPriceDataFieldFilter($comparisonFormat, $fields)
* @return $this
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
* @since 101.0.1
* @throws \Magento\Framework\Exception\LocalizedException
* @since 101.0.1
*/
public function addMediaGalleryData()
{
Expand All @@ -2361,7 +2368,7 @@ public function addMediaGalleryData()
'entity.' . $linkField . ' IN (?)',
array_map(
function ($item) use ($linkField) {
return (int) $item->getOrigData($linkField);
return (int)$item->getOrigData($linkField);
},
$items
)
Expand Down Expand Up @@ -2412,9 +2419,8 @@ private function getGalleryReadHandler()
/**
* Retrieve Media gallery resource.
*
* @deprecated 101.0.1
*
* @return \Magento\Catalog\Model\ResourceModel\Product\Gallery
* @deprecated 101.0.1
*/
private function getMediaGalleryResource()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
<actionGroup name="AssertStorefrontNoProductsFoundActionGroup">
<see userInput="We can't find products matching the selection." stepKey="seeEmptyNotice"/>
</actionGroup>
</actionGroups>
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,12 @@
<data key="path">catalog/frontend/grid_per_page_values</data>
<data key="value">1,2</data>
</entity>
<entity name="DefaultGridPerPageDefaultConfigData">
<data key="path">catalog/frontend/grid_per_page</data>
<data key="value">12</data>
</entity>
<entity name="CustomGridPerPageDefaultConfigData">
<data key="path">catalog/frontend/grid_per_page</data>
<data key="value">1</data>
</entity>
</entities>
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@

<!-- # Category should open successfully # <product1> should be absent on the page -->
<see userInput="$$createAnchoredCategory1.name$$" selector="{{StorefrontCategoryMainSection.CategoryTitle}}" stepKey="seeCategory1Name"/>
<see userInput="We can't find products matching the selection." stepKey="seeEmptyNotice"/>
<actionGroup ref="AssertStorefrontNoProductsFoundActionGroup" stepKey="seeEmptyNotice"/>
<dontSee userInput="$$simpleProduct.name$$" selector="{{StorefrontCategoryMainSection.productName}}" stepKey="dontseeProduct"/>

<!-- Log in to the backend: Admin user is logged in-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@

<!-- The category is still empty -->
<see userInput="$$createCategoryA.name$$" selector="{{StorefrontCategoryMainSection.CategoryTitle}}" stepKey="seeCategoryA1Name"/>
<see userInput="We can't find products matching the selection." stepKey="seeEmptyNotice"/>
<actionGroup ref="AssertStorefrontNoProductsFoundActionGroup" stepKey="seeEmptyNotice"/>
<dontSee userInput="$$createProductA1.name$$" selector="{{StorefrontCategoryMainSection.productName}}" stepKey="dontseeProductA1"/>

<!-- 4. Run cron to reindex -->
Expand Down Expand Up @@ -128,7 +128,7 @@

<!-- Category A is empty now -->
<see userInput="$$createCategoryA.name$$" selector="{{StorefrontCategoryMainSection.CategoryTitle}}" stepKey="seeOnPageCategoryAName"/>
<see userInput="We can't find products matching the selection." stepKey="seeOnPageEmptyNotice"/>
<actionGroup ref="AssertStorefrontNoProductsFoundActionGroup" stepKey="seeOnPageEmptyNotice"/>
<dontSee userInput="$$createProductA1.name$$" selector="{{StorefrontCategoryMainSection.productName}}" stepKey="dontseeProductA1OnPage"/>

<!-- Case: change product status -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ define([
direction: 'product_list_dir',
order: 'product_list_order',
limit: 'product_list_limit',
page: 'p',
modeDefault: 'grid',
directionDefault: 'asc',
orderDefault: 'position',
Expand Down Expand Up @@ -81,24 +82,63 @@ define([
},

/**
* @param {String} paramName
* @param {*} paramValue
* @param {*} defaultValue
* @private
*/
changeUrl: function (paramName, paramValue, defaultValue) {
getUrlParams: function () {
var decode = window.decodeURIComponent,
urlPaths = this.options.url.split('?'),
baseUrl = urlPaths[0],
urlParams = urlPaths[1] ? urlPaths[1].split('&') : [],
paramData = {},
parameters, i, form, params, key, input, formKey;
params = {},
parameters, i;

for (i = 0; i < urlParams.length; i++) {
parameters = urlParams[i].split('=');
paramData[decode(parameters[0])] = parameters[1] !== undefined ?
params[decode(parameters[0])] = parameters[1] !== undefined ?
decode(parameters[1].replace(/\+/g, '%20')) :
'';
}

return params;
},

/**
* @returns {String}
* @private
*/
getCurrentLimit: function () {
return this.getUrlParams()[this.options.limit] || this.options.limitDefault;
},

/**
* @returns {String}
* @private
*/
getCurrentPage: function () {
return this.getUrlParams()[this.options.page] || 1;
},

/**
* @param {String} paramName
* @param {*} paramValue
* @param {*} defaultValue
*/
changeUrl: function (paramName, paramValue, defaultValue) {
var urlPaths = this.options.url.split('?'),
baseUrl = urlPaths[0],
paramData = this.getUrlParams(),
currentPage = this.getCurrentPage(),
form, params, key, input, formKey, newPage;

if (currentPage > 1 && paramName === this.options.limit) {
newPage = Math.floor(this.getCurrentLimit() * (currentPage - 1) / paramValue) + 1;

if (newPage > 1) {
paramData[this.options.page] = newPage;
} else {
delete paramData[this.options.page];
}
}

paramData[paramName] = paramValue;

if (this.options.post) {
Expand Down Expand Up @@ -130,6 +170,7 @@ define([
if (paramValue == defaultValue) { //eslint-disable-line eqeqeq
delete paramData[paramName];
}

paramData = $.param(paramData);
location.href = baseUrl + (paramData.length ? '?' + paramData : '');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ public function resolve(): SearchCriteria
$searchCriteria->setRequestName($this->searchRequestName);
$searchCriteria->setSortOrders($this->orders);
$searchCriteria->setCurrentPage($this->currentPage - 1);
if ($this->size) {
$searchCriteria->setPageSize($this->size);
}

return $searchCriteria;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@
<field key="name">Product Simple AAA</field>
</createData>
<magentoCLI command="config:set {{CustomGridPerPageValuesConfigData.path}} {{CustomGridPerPageValuesConfigData.value}}" stepKey="setCustomGridPerPageValues"/>
<magentoCLI command="config:set {{CustomGridPerPageDefaultConfigData.path}} {{CustomGridPerPageDefaultConfigData.value}}" stepKey="setCustomGridPerPageDefaults"/>
<magentoCLI stepKey="flushConfigCache" command="cache:flush config"/>
<magentoCron groups="index" stepKey="runCronIndex"/>
</before>

<after>
<deleteData createDataKey="createFirstProduct" stepKey="deleteFirstProduct"/>
<deleteData createDataKey="createSecondProduct" stepKey="deleteSecondProduct"/>
<magentoCLI command="config:set {{DefaultGridPerPageValuesConfigData.path}} {{DefaultGridPerPageValuesConfigData.value}}" stepKey="setDefaultGridPerPageValues"/>
<magentoCLI command="config:set {{DefaultGridPerPageDefaultConfigData.path}} {{DefaultGridPerPageDefaultConfigData.value}}" stepKey="setDefaultGridPerPageDefaults"/>
</after>

<actionGroup ref="StorefrontOpenHomePageActionGroup" stepKey="openStorefrontHomePage"/>
Expand All @@ -57,6 +61,9 @@
</actionGroup>
<selectOption selector="{{StorefrontCategoryBottomToolbarSection.perPage}}" userInput="2" stepKey="selectDisplayedProductInGridPerPage"/>
<waitForPageLoad stepKey="waitForPageLoad"/>

<dontSeeInCurrentUrl stepKey="assertRedirectedToFirstPage" url="?p=2"/>

<actionGroup ref="AssertProductOnCategoryPageActionGroup" stepKey="assertFirstProductDisplayedOnCatalogSearchPage">
<argument name="product" value="$createFirstProduct$"/>
</actionGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,11 @@
<remove keyForRemoval="selectDisplayedProductInGridPerPage"/>
<remove keyForRemoval="assertFirstProductDisplayedOnCatalogSearchPage"/>
<remove keyForRemoval="assertSecondProductDisplayedOnCatalogSearchPage"/>
<grabTextFrom selector="{{StorefrontCategoryBottomToolbarSection.currentPage}}" stepKey="grabNumberOfLastPage"/>
<actionGroup ref="StorefrontQuickSearchWithPaginationActionGroup" stepKey="navigateToUnavailableCatalogSearchResultPage">
<remove keyForRemoval="selectDisplayedProductInGridPerPage"/>
<remove keyForRemoval="assertStillOnSecondPage"/>
<actionGroup ref="StorefrontQuickSearchWithPaginationActionGroup" stepKey="navigateToUnavailableCatalogSearchResultPage" before="waitForPageLoad">
<argument name="phrase" value="AAA"/>
<argument name="pageNumber" value="999"/>
</actionGroup>
<scrollTo selector="{{StorefrontCategoryBottomToolbarSection.currentPage}}" stepKey="scrollToBottomToolbarPager"/>
<grabTextFrom selector="{{StorefrontCategoryBottomToolbarSection.currentPage}}" stepKey="grabNumberOfCurrentPage"/>
<assertEquals stepKey="assertCurrentPageIsLastPageOfCatalogSearchResult">
<expectedResult type="variable">grabNumberOfLastPage</expectedResult>
<actualResult type="variable">grabNumberOfCurrentPage</actualResult>
</assertEquals>
<actionGroup ref="AssertProductOnCategoryPageActionGroup" stepKey="assertProductOnLastCatalogSearchPage">
<argument name="product" value="$createSecondProduct$"/>
</actionGroup>
<actionGroup ref="StorefrontCheckProductIsMissingInCategoryProductsPageActionGroup" stepKey="assertFirstProductIsMissingOnLastCatalogSearchPage">
<argument name="productName" value="$createFirstProduct.name$"/>
</actionGroup>
</test>
</tests>
Loading