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

Ability to turn the sorting off for a clicked column in GridView with multisort ignores defaultOrder #19731

Closed
spapad opened this issue Jan 3, 2023 · 5 comments
Labels
Milestone

Comments

@spapad
Copy link

spapad commented Jan 3, 2023

If a field is set at the defaultOrder of a Sort specification with a SORT_DESC ordering, and enableMultiSort is set to true, the sort link is always generated the same and sorting in SORT_ASC order is not possible.

What steps will reproduce the problem?

Enable multisort and set defaultOrder on a property with SORT_DESC, i.e.

        $dataProvider = new ActiveDataProvider([
            'query' => $query,
            'sort' => [
                'enableMultiSort' => true,
                'defaultOrder' => [
                    'date' => SORT_DESC
                ]
            ],
        ]);

What is the expected result?

After #18826
changing the sorting of the designated field would be expected in the order:
DESC -> off -> ASC -> DESC ...

What do you get instead?

The sort link generated by $sort->link() is repeatedly setting sorting to OFF, due to the fact that the default ordering sets the order to DESC.

Additional info

Q A
Yii version 2.0.47
PHP version 8.2.0
Operating system Ubuntu 22.04.1 LTS (Jammy Jellyfish) 64-bit
@samdark samdark added the type:bug Bug label Jan 7, 2023
@samdark
Copy link
Member

samdark commented Jan 7, 2023

@ditibal, @bizley would you please take a look?

@bizley
Copy link
Member

bizley commented Jan 8, 2023

I've added more tests to check the link() in #19733
I see no problems here. It's true that for multisort expected order is DESC -> off -> ASC -> DESC but in case there are no current sort params we use defaultOrder so for that param this will start with off and not DESC. Maybe I'm missing something from your description, please let me know.

@spapad
Copy link
Author

spapad commented Jan 8, 2023

Thank you for taking the time to look into it! I must say that this is not actually a bug, but does allow for this weird scenario. Please let me try to describe the observed result.

Following the example above, where sort is configured like this

'sort' => [
    'enableMultiSort' => true,
    'defaultOrder' => [
        'date' => SORT_DESC
    ]
],

a GridView using the dataprovider will sort by default on the date field in descending order.
As expected, the corresponding sort link for that date column will have an empty sort query param.
For example .../index.php?r=ga%2Findex&sort=

Clicking this link, which as expected does not provide a sort query param, will result in displaying the page again with the defaultOrder (that is the GridView will sort again by default on the date field in descending order).
Thus we will not be able to sort in ascending order.

This only applies when sorting with the denoted field alone.

@bizley
Copy link
Member

bizley commented Jan 8, 2023

Oh, I get it. Currently we cannot distinguish "no sort" from "desc sort switched off" resulting in default sorting applied in both cases automatically. Let me think about possible solution...

@bizley
Copy link
Member

bizley commented Jan 9, 2023

Ok, this should fix the problem without BC breaks.

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

No branches or pull requests

3 participants