DataList::canSortBy can't handle dot notation properly

[dhensby]

There are two instances using dot notation that fail where I think it should work. Take the following examples:

Example 1 - Bad sort resolution

class MyDataObject extends DataObject {

    private static $has_one = array(
        'Person' => 'Member',
    );

    private static $summary_fields = array(
        'Person.Name' => 'Person name',
    );

}

This results in a column that is identified as sortable but when a sort is attempted by clicking on the column header an error is thrown. "Unknown column Person.Name"

Example 2 - Fail to resolve formatted fields

class MyDataObject extends DataObject {

    private static $db = array(
        'Date' => 'SS_Datetime',
    );

    private static $summary_fields = array(
        'Date.Nice' => 'Date',
    );

}

In this example, the column is not sortable (though it really should be because we are just manipulating the appearance of the data from a column).

---

Currently GridFieldSortableHeader@getHTMLFragments is failing to correctly identify if dot notation is just a style manipulation or if it's a relationship. If it's a relationship, it's failing to correctly work out the relation field to sort on.

I think the logic in getHTMLFragments is too complex as it is written to work out an infinite level of nesting (though breaks out on SS_List instances which is surely the only time infinite nesting could work?!).

I propose we limit the logic to assume there can only be a maximum of 3 levels of nesting. (Relation.Field.Format / Field.Format / Relation.Field / Field). Perhaps even assuming that the format would default to "Nice"? [edit: removed as not a job for sortable headers]

This would allow simpler code and for clearer documentation.

[sminnee]

It's a missing feature more than a bug because the current behaviour is internally consistent.

Note that canFilterBy() has a similar weakness.

[dhensby]

Hm - I think a PR was opened a while ago attempting to fix this - @kinglozzer were you involved in that?

[kinglozzer]

This may have been fixed in #4812?

[dhensby]

Hmm - I was thinking more about the GridFieldSortableHeader component - I'll look into testing this soon