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

Set order from sort_parameters as fallback #7529

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

franmomu
Copy link
Member

Subject

This was triggered investigating the warnings triggered by PHP 8.1: https://github.com/sonata-project/SonataAdminBundle/pull/7528/checks?check_run_id=3837943478#step:10:53

The problem is that when doing {{ sort_by|lower }}, sort_by is null in our tests.

This PR will partially fix it (I'll make another one in 4.x removing our implementation in tests of DatagridInterface and using the one from the bundle).

What it does it to set the order from sort_parameters if it is not defined in the field description.

I am targeting this branch, because this is a bugfix.

Changelog

### Fixed
- Fixed setting a default order if it is not set in the field description

@franmomu franmomu added the patch label Oct 10, 2021
Copy link
Member

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to understand the issue ; do we have a way to reproduce it in the tests ?

@franmomu franmomu force-pushed the fix_sort_order_param_3 branch from 3d54b87 to 3ea45cf Compare October 17, 2021 16:15
@franmomu
Copy link
Member Author

The issue is about to provide a default value for sort_by in case it is not set in the field description, I saw it when using

$list
->add('name', FieldDescriptionInterface::TYPE_STRING, [
'sortable' => true,
])

where sort_by is null in

{% set sort_by = current ? admin.datagrid.values[constant('Sonata\\AdminBundle\\Datagrid\\DatagridInterface::SORT_ORDER')] : field_description.option(constant('Sonata\\AdminBundle\\Datagrid\\DatagridInterface::SORT_ORDER')) %}

with this change sort_by will be ASC since it's the default value from sort_parameters.

It's in twig, so I don't think there is an easy way to test this.

@VincentLanglet VincentLanglet requested a review from a team October 19, 2021 15:37
@core23 core23 merged commit b4686cf into sonata-project:3.x Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants