setAppendToTop() Does Not Work When the DataObjects Within the GridField are not Part of a RelationList

[HARVS1789UK]

I am guessing this is not an 'issue' as such as it seems to be intended behaviour but I cannot see any reason why.

I am using a GridField to display a number of 'ExportCheckpoint' DataObjects. These DataObject's have no relationships of any kind to anything else (i.e. are not related to a specific page or other type of DataObject). The have a 'SortOrder' field which is also used as its default sort (in ASC order).

Basically I am using GridFieldSortableRows to allow for these DataObjects to be re-ordered, because when you click the 'allow drag and drop re-ordering' checkbox it sorts by 'SortOrder ASC' I am having to make my 'active' ExportCheckpoint be the one with the LOWEST SortOrder value. The CMS users can then drag and drop an older ExportCheckpoint to the top of the GridField (giving it a lower SortOrder).

By default newly created DataObject's would go to the bottom of the list (with the highest SortOrder) so I have enabled the 'append to top' setting. I was expecting this to set the newly created DataObject's SortOrder to 1 and then +1 to all other existing DataObjects in the list.

That seems to be exactly what the code does when $append_to_top == true AND the GridField's list is a RelationList however, the below code prevents this if the GridField's list is not a RelationList (and is a regular DataList presumably):

GridFieldSortableRows.php ln 219 - 221

if($this->append_to_top && !($list instanceof RelationList)) {
    $idCondition='"ID" IN(\''.implode("','", $list->getIDList()).'\')';
}

This code add an extra WHERE condition which will only apply the '+1 to all SortOrder fields' logic to the newly created DataObject and not to any of the existing ones.

That leaves the scenario where you have multiple items in the list all with the same SortOrder of 1 (as per the attached screenshot - I have added 'Sort Order' and 'Chronological Order' to the GridField columns to make this clear).

I can prevent this by replacing the above code with this:

if($this->append_to_top && !($list instanceof RelationList)) {
    $idCondition='"ID" > 0';
}

Just for the sake of satisfying some kind of WHERE clause (as it is referenced in the code below) but that means editing the modules core code, which I want to avoid. Also I assume this has been done on purpose, but I can see no explanation as to what that purpose was.

If it is not required I am happy to make some alterations to remove it and submit a pull request for that.

Can you explain what that code is for?

Kind regards,

HARVS1789UK

[UndefinedOffset]

I'm not sure actually, this code was committed almost a year ago back in eb1e736. It looks to me like it's something to do with the raw query filtering likely for ModelAdmin.

What type of relationship are you managing (and where)? Also what SS version are you using?

[HARVS1789UK]

Hi @UndefinedOffset
Sorry I should of provided that information originally. I am using SS 3.1.2 and I am managing these DataObjects in a ModelAdmin extension. The DataObjects have no relationships to anything else, they are created, re-ordered and deleted (if needed) all via the ModelAdmin interface.

[UndefinedOffset]

Ah no worries, I think that in is probably critical in this situation (though I haven't tested). I wonder if the result of $list->getIDList() is not returning the actual ID's or at least not all of them. I'd need to spend some time with a few tests to see for sure. That would be my initial guess as to the issue, I wonder if changing that line to use $list->column('ID') would be better suited to this case. I think ID>0 could be risky since it's using raw queries you would want it to update just the items in the list. Which obviously it's not doing correctly.

[HARVS1789UK]

I think $list->getIDList() is already doing exactly the same as $list->column('ID') it's just that $list only contains records where SortOrder is 0 as per line 171 of GridFieldSortableRows.php and in my case that means only 1 records, the newly added one.

$list=$list->filter($this->sortColumn, 0)->sort("Created,ID");

[UndefinedOffset]

Ah so the fix then should be in the case of relation lists to pull the id list from the gridfield's list. So $list->getIDList() (on line 220) should really be $dataList->getIDList().

[HARVS1789UK]

Yes that also does the trick as it just adds a filter to the effect of "only do this for DataObject's which are listed in this GridField" which is a bit safer as you say.

Do you want me to raise a pull request for that change or are you happy to make it?

[UndefinedOffset]

i'll just throw a commit save you having to fork :)

[UndefinedOffset]

There I've implemented the fix and I'll tag a new release (0.4.2) as soon as the build finishes. Thanks for the report.