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

FIX Prevent array to string conversion for default_sort on a fixed field #8090

Conversation

robbieaverill
Copy link
Contributor

@robbieaverill robbieaverill commented May 23, 2018

This shows a regression in 3.7.x-dev where you can no longer define a default_sort that is a "fixed field" (e.g. Created from DataObject) without throwing an "array to string conversion" error.

One option is to cast empty arrays in Object::parse_class_spec to an empty string so this doesn't happen. I'm not sure where the regression happened, but this test shows it.

@robbieaverill robbieaverill changed the title Add test to show breakage in default sorting on a fixed field FIX Prevent array to string conversion for default_sort on fixed_fields May 23, 2018
@robbieaverill robbieaverill changed the title FIX Prevent array to string conversion for default_sort on fixed_fields FIX Prevent array to string conversion for default_sort on a fixed field May 23, 2018
@kinglozzer
Copy link
Member

I suspect this is related to #7945. It looks like DataObject::db() will always return an array for fixed fields 😕. Perhaps this could be refactored to something like

// Outside the parent foreach()
$fields = DataObject::database_fields(get_class($this));

// ...

$columnSpec = $fields[$column];
list($fieldType) = SS_Object::parse_class_spec($columnSpec);

@@ -213,6 +213,9 @@ public static function create_from_string($classSpec, $firstArg = null) {
* Returns a 2-elemnent array, with classname and arguments
*/
public static function parse_class_spec($classSpec) {
if ($classSpec === array()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little obscure, where the underlying reason is "sometimes code passes in an empty array in some edge case". It'd be better to never call this method with an invalid arg, rather than silently ignoring the error.

@tractorcow
Copy link
Contributor

I've fixed this with #8146

The issue is nothing to do with parse_class_spec, it's a distraction.

database_fields() doesn't return anything for ID so you're out of luck there anyway.

The fix was to use hasOwnTableDatabaseField() instead. See #8146

@robbieaverill
Copy link
Contributor Author

Thank you!

@tractorcow tractorcow deleted the pulls/3.7/default-sort-on-fixed-fields branch June 7, 2018 02:59
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.

None yet

3 participants