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

CMS Pages Item Count Wrong When Filtering By Store #1335

Closed
sprankhub opened this issue Dec 10, 2020 · 4 comments
Closed

CMS Pages Item Count Wrong When Filtering By Store #1335

sprankhub opened this issue Dec 10, 2020 · 4 comments

Comments

@sprankhub
Copy link
Contributor

Preconditions (*)

  1. OpenMage LTS 20.0.2
  2. Sample data installed

I used https://www.splendid-internet.de/magento-hosting/magento-shop-demo-store-demo-admin-deutsche-version/ to verify the issue.

Steps to reproduce (*)

  1. Log into the backend.
  2. Go to CMS > Pages.
  3. Verify number of pages (in my case, 19).
    image
  4. Filter by English store view.

Expected result (*)

  1. Result count should be the same or lower than before applying the filter.

Actual result (*)

  1. Result count is higher than before (in my case, 22) and incorrect. Due to this, a second page is shown, which is completely empty.

image

image

I think this can be solved by a similar approach as in Mage_Catalog_Model_Resource_Product_Collection::_getSelectCountSql:

protected function _getSelectCountSql($select = null, $resetLeftJoins = true)
{
$this->_renderFilters();
$countSelect = (is_null($select)) ?
$this->_getClearSelect() :
$this->_buildClearSelect($select);
// Clear GROUP condition for count method
$countSelect->reset(Zend_Db_Select::GROUP);
$countSelect->columns('COUNT(DISTINCT e.entity_id)');
if ($resetLeftJoins) {
$countSelect->resetJoinLeft();
}
return $countSelect;
}

Instead of using COUNT(*), COUNT(DISTINCT main_table.page_id) should be used in Mage_Cms_Model_Resource_Page_Collection::getSelectCountSql. In my first tests, this solves the issue.

@sprankhub sprankhub added the bug label Dec 10, 2020
@colinmollenhour
Copy link
Member

I haven't tested this in OpenMage code but here is very general purpose version of getSelectCountSql() that uses the same concept as you proposed only generalized to work with any(?) grid and also removes unnecessary joins which can improve performance.

    /**
     * Get SQL for get record count
     *
     * @return Varien_Db_Select
     * @throws Zend_Db_Select_Exception
     */
    public function getSelectCountSql()
    {
        $this->_renderFilters();

        $countSelect = clone $this->getSelect();
        $countSelect->reset(Zend_Db_Select::ORDER);
        $countSelect->reset(Zend_Db_Select::LIMIT_COUNT);
        $countSelect->reset(Zend_Db_Select::LIMIT_OFFSET);
        $countSelect->reset(Zend_Db_Select::COLUMNS);

        // Count doesn't work with group by columns keep the group by
        // http://stackoverflow.com/questions/3485455/using-group-breaks-getselectcountsql-in-magento/4219386#4219386
        if(count($this->getSelect()->getPart(Zend_Db_Select::GROUP)) > 0) {
            $countSelect->reset(Zend_Db_Select::GROUP);
            $group = $this->getSelect()->getPart(Zend_Db_Select::GROUP);
            $countSelect->columns("COUNT(DISTINCT ".implode(", ", $group).")");
        } else {
            $countSelect->columns('COUNT(*)');

            // Simple optimization - remove all joins if there are no where clauses using joined tables and all joins are left joins
            $leftJoins = array_filter($countSelect->getPart(Zend_Db_Select::FROM), function($table) {
                return ($table['joinType'] == Zend_Db_Select::LEFT_JOIN || $table['joinType'] == Zend_Db_Select::FROM);
            });
            if (count($leftJoins) == count($countSelect->getPart(Zend_Db_Select::FROM))) {
                $mainTable = array_filter($leftJoins, function ($table) {
                    return $table['joinType'] == Zend_Db_Select::FROM;
                });
                $mainTable = key($mainTable);
                $mainTable = preg_quote($mainTable, '/');
                $pattern = "/^$mainTable\\.\\w+/";
                $whereUsingJoin = array_filter($countSelect->getPart(Zend_Db_Select::WHERE), function ($clause) use ($pattern) {
                    $clauses = preg_split('/(^|\s+)(AND|OR)\s+/', $clause, -1, PREG_SPLIT_NO_EMPTY);
                    return array_filter($clauses, function ($clause) use ($pattern) {
                        $clause = preg_replace('/[()`\s]+/', '', $clause);
                        return !preg_match($pattern, $clause);
                    });
                });
                if (empty($whereUsingJoin)) {
                    $from = array_slice($leftJoins, 0, 1);
                    $countSelect->setPart(Zend_Db_Select::FROM, $from);
                }
            }
        }

        return $countSelect;
    }

@ADDISON74
Copy link
Contributor

It seems a good improvement provided by @colinmollenhour. Maybe a PR will be a good addition to OM.

@colinmollenhour
Copy link
Member

@ADDISON74 This particular issue seems to be fixed as I couldn't reproduce it but I still opened a PR with the code from my comment (#2210)

@colinmollenhour
Copy link
Member

Closing since I believe this must have already been fixed elsewhere - I could not reproduce the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants