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

BUG: default_sort throws an Array to string conversion error when defined as an array #357

Closed
pjayme opened this issue Mar 2, 2023 · 4 comments

Comments

@pjayme
Copy link

pjayme commented Mar 2, 2023

When a DataObject defines the default_sort property using the following convention:

private static array $default_sort = [
    'SortOrder' => 'ASC',
];

Then this throws an Array to string conversion error as updates from Feb 22’ now expects this property to be of type string rather than of type array.

The simple solution is to use the string convention instead:

private static string $default_sort = 'SortOrder ASC';

I’m raising this as I’m not sure if this should be treated as a regression since the module was capable of handling the array convention or if the expectation is that users should simply stick to using the new string convention instead? Or should the module be backwards compatible in that it can handle both conventions?

Acceptance criteria

Original PRs

PRs

@sunnysideup
Copy link
Contributor

sunnysideup commented Mar 6, 2023

This is going to break a lot of things, so I would love to see this fixed ASAP.

It is tricky, because you have to make sure that the "local" sort is the same as the default_sort. Probably the easiest way is to check the default_sort. If it is an array then add "local" sort (the ones compiled in the code) as a array as well and vice versa.
https://github.com/symbiote/silverstripe-gridfieldextensions/blob/3/src/GridFieldOrderableRows.php#L379-L406

@maxime-rainville
Copy link
Collaborator

I think this came from #325

@GuySartorelli
Copy link
Collaborator

Fixed in 3.5.1 and 3.6.1

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

4 participants