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

yii\data\Sort::getAttributeOrders 优化 #20071

Open
forwzb opened this issue Nov 21, 2023 · 4 comments
Open

yii\data\Sort::getAttributeOrders 优化 #20071

forwzb opened this issue Nov 21, 2023 · 4 comments
Labels
status:ready for adoption Feel free to implement this issue. type:enhancement

Comments

@forwzb
Copy link

forwzb commented Nov 21, 2023

What steps will reproduce the problem?

yii\data\Sort:

/**
     * Returns the currently requested sort information.
     * @param bool $recalculate whether to recalculate the sort directions
     * @return array sort directions indexed by attribute names.
     * Sort direction can be either `SORT_ASC` for ascending order or
     * `SORT_DESC` for descending order.
     */
    public function getAttributeOrders($recalculate = false)
    {
        if ($this->_attributeOrders === null || $recalculate) {
            $this->_attributeOrders = [];
            if (($params = $this->params) === null) {
                $request = Yii::$app->getRequest();
                $params = $request instanceof Request ? $request->getQueryParams() : [];
            }

            //反馈:这里当存在排序参数并且排序参数为空时,也会执行下面的代码,从而导致无法执行最后的默认排序,
           //当我想继承这个Sort类,对这部分代码进行部分修改时,private _attributeOrders 属性又限制对这个方法的部分代码修改,
           //建议:将 isset($params[$this->sortParam]) 改为 isset($params[$this->sortParam])&&empty(isset($params[$this->sortParam])) 或者其他
            if (isset($params[$this->sortParam])) {
                foreach ($this->parseSortParam($params[$this->sortParam]) as $attribute) {
                    $descending = false;
                    if (strncmp($attribute, '-', 1) === 0) {
                        $descending = true;
                        $attribute = substr($attribute, 1);
                    }

                    if (isset($this->attributes[$attribute])) {
                        $this->_attributeOrders[$attribute] = $descending ? SORT_DESC : SORT_ASC;
                        if (!$this->enableMultiSort) {
                            return $this->_attributeOrders;
                        }
                    }
                }

                return $this->_attributeOrders;
            }
            if (empty($this->_attributeOrders) && is_array($this->defaultOrder)) {
                $this->_attributeOrders = $this->defaultOrder;
            }
        }

        return $this->_attributeOrders;
    }
@bizley
Copy link
Member

bizley commented Nov 22, 2023

I'm not sure why it was done like that, I think we should indeed check isset and empty.

@samdark samdark added status:ready for adoption Feel free to implement this issue. type:enhancement labels Nov 23, 2023
@samdark
Copy link
Member

samdark commented Nov 23, 2023

Do you have time for a pull request?

@forwzb
Copy link
Author

forwzb commented Nov 27, 2023

Do you have time for a pull request?

Here is a little busy, it can only be submitted slightly later.

@xicond
Copy link
Contributor

xicond commented Dec 30, 2023

I think supposedly fallback defaultOrder if empty string, am I correct ?

[ 'params' => [ 'sort' => '' ]]

@yiisoft yiisoft deleted a comment from forwzb Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:enhancement
Projects
None yet
Development

No branches or pull requests

4 participants