Merge pull request #160 from open-sausages/pulls/fix-sorting-unsaved

BUG Ensure that unsaved items can be sorted

code/GridFieldAddNewInlineButton.php

@@ -101,7 +101,14 @@ class GridFieldAddNewInlineButton implements GridField_HTMLProvider, GridField_S
 
 				$content = $field->Field();
 			} else {
-				$content = null;
+				$content = $grid->getColumnContent($record, $column);
+
+				// Convert GridFieldEditableColumns to the template format
+				$content = str_replace(
+					'[GridFieldEditableColumns][0]',
+					'[GridFieldAddNewInlineButton][{%=o.num%}]',
+					$content
+				);
 			}
 
 			$attrs = '';
@@ -129,7 +136,10 @@ class GridFieldAddNewInlineButton implements GridField_HTMLProvider, GridField_S
 		}
 
 		$class    = $grid->getModelClass();
+		/** @var GridFieldEditableColumns $editable */
 		$editable = $grid->getConfig()->getComponentByType('GridFieldEditableColumns');
+		/** @var GridFieldOrderableRows $sortable */
+		$sortable = $grid->getConfig()->getComponentByType('GridFieldOrderableRows');
 		$form     = $editable->getForm($grid, $record);
 
 		if(!singleton($class)->canCreate()) {
@@ -143,6 +153,12 @@ class GridFieldAddNewInlineButton implements GridField_HTMLProvider, GridField_S
 			$form->loadDataFrom($fields, Form::MERGE_CLEAR_MISSING);
 			$form->saveInto($item);
 
+			// Check if we are also sorting these records
+			if ($sortable) {
+				$sortField = $sortable->getSortField();
+				$item->setField($sortField, $fields[$sortField]);
+			}
+
 			if($list instanceof ManyManyList) {
 				$extra = array_intersect_key($form->getData(), (array) $list->getExtraFields());
 			}

code/GridFieldEditableColumns.php

@@ -91,6 +91,9 @@ class GridFieldEditableColumns extends GridFieldDataColumns implements
 			return;
 		}
 
+		/** @var GridFieldOrderableRows $sortable */
+		$sortable = $grid->getConfig()->getComponentByType('GridFieldOrderableRows');
+
 		$form = $this->getForm($grid, $record);
 
 		foreach($value[__CLASS__] as $id => $fields) {
@@ -109,6 +112,12 @@ class GridFieldEditableColumns extends GridFieldDataColumns implements
 			$form->loadDataFrom($fields, Form::MERGE_CLEAR_MISSING);
 			$form->saveInto($item);
 
+			// Check if we are also sorting these records
+			if ($sortable) {
+				$sortField = $sortable->getSortField();
+				$item->setField($sortField, $fields[$sortField]);
+			}
+
 			if($list instanceof ManyManyList) {
 				$extra = array_intersect_key($form->getData(), (array) $list->getExtraFields());
 			}

code/GridFieldOrderableRows.php

@@ -196,7 +196,22 @@ class GridFieldOrderableRows extends RequestHandler implements
 	}
 
 	public function getColumnContent($grid, $record, $col) {
-		return ViewableData::create()->renderWith('GridFieldOrderableRowsDragHandle');
+		// In case you are using GridFieldEditableColumns, this ensures that
+		// the correct sort order is saved. If you are not using that component,
+		// this will be ignored by other components, but will still work for this.
+		$sortFieldName = sprintf(
+			'%s[GridFieldEditableColumns][%s][%s]',
+			$grid->getName(),
+			$record->ID,
+			$this->getSortField()
+		);
+		$sortField = new HiddenField($sortFieldName, false, $record->getField($this->getSortField()));
+		$sortField->addExtraClass('ss-orderable-hidden-sort');
+		$sortField->setForm($grid->getForm());
+
+		return ViewableData::create()->customise(array(
+			'SortField' => $sortField
+		))->renderWith('GridFieldOrderableRowsDragHandle');
 	}
 
 	public function getColumnAttributes($grid, $record, $col) {
@@ -262,14 +277,16 @@ class GridFieldOrderableRows extends RequestHandler implements
 			$this->httpError(403);
 		}
 
-		// Save any un-comitted changes to the gridfield
+		// Save any un-committed changes to the gridfield
 		if(($form = $grid->getForm()) && ($record = $form->getRecord()) ) {
 			$form->loadDataFrom($request->requestVars(), true);
 			$grid->saveInto($record);
 		}
 
-		$ids   = $request->postVar('order');
+		// Get records from the `GridFieldEditableColumns` column
-		if (!$this->executeReorder($grid, $ids))
+		$data = $request->postVar($grid->getName());
+		$sortedIDs = $this->getSortedIDs($data);
+		if (!$this->executeReorder($grid, $sortedIDs))
 		{
 			$this->httpError(400);
 		}
@@ -277,6 +294,26 @@ class GridFieldOrderableRows extends RequestHandler implements
 		return $grid->FieldHolder();
 	}
 
+	/**
+	 * Get mapping of sort value to ID from posted data
+	 *
+	 * @param array $data Raw posted data
+	 * @return array
+	 */
+	protected function getSortedIDs($data) {
+		if (empty($data['GridFieldEditableColumns'])) {
+			return array();
+		}
+
+		$sortedIDs = array();
+		foreach($data['GridFieldEditableColumns'] as $id => $recordData) {
+			$sortValue = $recordData[$this->sortField];
+			$sortedIDs[$sortValue] = $id;
+		}
+		ksort($sortedIDs);
+		return $sortedIDs;
+	}
+
 	/**
 	 * Handles requests to move an item to the previous or next page.
 	 */
@@ -343,23 +380,21 @@ class GridFieldOrderableRows extends RequestHandler implements
 	public function handleSave(GridField $grid, DataObjectInterface $record) {
 		if (!$this->immediateUpdate)
 		{
-			$list  = $grid->getList();
 			$value = $grid->Value();
-			if (isset($value['GridFieldEditableColumns']) && $value['GridFieldEditableColumns'])
+			$sortedIDs = $this->getSortedIDs($value);
-			{
+			if ($sortedIDs) {
-				$rows = $value['GridFieldEditableColumns'];
+				$this->executeReorder($grid, $sortedIDs);
-				$ids = array();
-				foreach ($rows as $id => $data)
-				{
-					$ids[] = $id;
-				}
-				$this->executeReorder($grid, $ids);
 			}
 		}
 	}
 
-	protected function executeReorder(GridField $grid, $ids) {
+	/**
-		if(!is_array($ids)) {
+	 * @param GridField $grid
+	 * @param array $sortedIDs List of IDS, where the key is the sort field value to save
+	 * @return bool
+	 */
+	protected function executeReorder(GridField $grid, $sortedIDs) {
+		if(!is_array($sortedIDs)) {
 			return false;
 		}
 		$field = $this->getSortField();
@@ -376,10 +411,10 @@ class GridFieldOrderableRows extends RequestHandler implements
 		}
 		$list = $grid->getList();
 		$sortterm .= '"'.$this->getSortTable($list).'"."'.$field.'"';
-		$items = $list->filter('ID', $ids)->sort($sortterm);
+		$items = $list->filter('ID', $sortedIDs)->sort($sortterm);
 
 		// Ensure that each provided ID corresponded to an actual object.
-		if(count($items) != count($ids)) {
+		if(count($items) != count($sortedIDs)) {
 			return false;
 		}
 
@@ -408,11 +443,11 @@ class GridFieldOrderableRows extends RequestHandler implements
 		}
 
 		// Perform the actual re-ordering.
-		$this->reorderItems($list, $current, $ids);
+		$this->reorderItems($list, $current, $sortedIDs);
 		return true;
 	}
 
-	protected function reorderItems($list, array $values, array $order) {
+	protected function reorderItems($list, array $values, array $sortedIDs) {
 		$sortField = $this->getSortField();
 		/** @var SS_List $map */
 		$map = $list->map('ID', $sortField);
@@ -433,13 +468,13 @@ class GridFieldOrderableRows extends RequestHandler implements
 		if (!$isVersioned) {
 			$sortTable = $this->getSortTable($list);
 			$additionalSQL = (!$list instanceof ManyManyList) ? ', "LastEdited" = NOW()' : '';
-			foreach(array_values($order) as $pos => $id) {
+			foreach($sortedIDs as $sortValue => $id) {
-				if($map[$id] != $pos) {
+				if($map[$id] != $sortValue) {
 					DB::query(sprintf(
 						'UPDATE "%s" SET "%s" = %d%s WHERE %s',
 						$sortTable,
 						$sortField,
-						$pos,
+						$sortValue,
 						$additionalSQL,
 						$this->getSortTableClauseForIds($list, $id)
 					));
@@ -450,10 +485,10 @@ class GridFieldOrderableRows extends RequestHandler implements
 			// *_versions table is updated. This ensures re-ordering works
 			// similar to the SiteTree where you change the position, and then
 			// you go into the record and publish it.
-			foreach(array_values($order) as $pos => $id) {
+			foreach($sortedIDs as $sortValue => $id) {
-				if($map[$id] != $pos) {
+				if($map[$id] != $sortValue) {
 					$record = $class::get()->byID($id);
-					$record->$sortField = $pos;
+					$record->$sortField = $sortValue;
 					$record->write();
 				}
 			}

javascript/GridFieldExtensions.js

@@ -52,7 +52,7 @@
 				var dialog = this.closest(".add-existing-search-dialog")
 					 .addClass("loading")
 					 .children(".ui-dialog-content")
-				                 .empty()
+					 .empty();
 
 				$.post(link, { id: id }, function() {
 					dialog.data("grid").reload();
@@ -79,13 +79,40 @@
 		$(".ss-gridfield.ss-gridfield-editable").entwine({
 			reload: function(opts, success) {
 				var grid  = this;
-				var added = grid.find("tbody:first").find(".ss-gridfield-inline-new").detach();
+				// Record position of all items
+				var added = [];
+				var index = 0; // 0-based index
+				grid.find("tbody:first .ss-gridfield-item").each(function() {
+					// Record inline items with their original positions
+					if ($(this).is(".ss-gridfield-inline-new")) {
+						added.push({
+							'index': index,
+							'row': $(this).detach()
+						});
+					}
+					index++;
+				});
 
 				this._super(opts, function() {
-					if(added.length) {
+					var body = grid.find("tbody:first");
-                        added.appendTo(grid.find("tbody:first"));
+					$.each(added, function(i, item) {
-                        grid.find("tbody:first").children(".ss-gridfield-no-items").hide();
+						var row = item['row'],
+							index = item['index'],
+							replaces;
+						// Insert at index position
+						if (index === 0) {
+							body.prepend(row);
+						} else {
+							// Find item that we could potentially insert this row after
+							replaces = body.find('.ss-gridfield-item:nth-child(' + index + ')');
+							if (replaces.length) {
+								replaces.after(row);
+							} else {
+								body.append(row);
 							}
+						}
+						grid.find("tbody:first").children(".ss-gridfield-no-items").hide();
+					});
 
 					if(success) success.apply(grid, arguments);
 				});
@@ -159,6 +186,9 @@
 				this.find("tbody:first").append(tmpl(this[0].id + "ss-gridfield-add-inline-template", { num: num }));
 				this.find("tbody:first").children(".ss-gridfield-no-items").hide();
 				this.data("add-inline-num", num + 1);
+
+				// Rebuild sort order fields
+				$(".ss-gridfield-orderable tbody").rebuildSort();
 			}
 		});
 
@@ -234,6 +264,36 @@
 		 */
 
 		$(".ss-gridfield-orderable tbody").entwine({
+			rebuildSort: function() {
+				var grid = this.getGridField();
+
+				// Get lowest sort value in this list (respects pagination)
+				var minSort = null;
+				grid.getItems().each(function() {
+					// get sort field
+					var sortField = $(this).find('.ss-orderable-hidden-sort');
+					if (sortField.length) {
+						var thisSort = sortField.val();
+						if (minSort === null && thisSort > 0) {
+							minSort = thisSort;
+						} else if (thisSort > 0) {
+							minSort = Math.min(minSort, thisSort);
+						}
+					}
+				});
+				minSort = Math.max(1, minSort);
+
+				// With the min sort found, loop through all records and re-arrange
+				var sort = minSort;
+				grid.getItems().each(function() {
+					// get sort field
+					var sortField = $(this).find('.ss-orderable-hidden-sort');
+					if (sortField.length) {
+						sortField.val(sort);
+						sort++;
+					}
+				});
+			},
 			onadd: function() {
 				var self = this;
 
@@ -246,18 +306,22 @@
 							  .end();
 				};
 
-				var update = function() {
+				var update = function(event, ui) {
+					// If the item being dragged is unsaved, don't do anything
+					var postback = true;
+					if (ui.item.hasClass('ss-gridfield-inline-new')) {
+						postback = false;
+					}
+
+					// Rebuild all sort hidden fields
+					self.rebuildSort();
+
+					// Check if we are allowed to postback
 					var grid = self.getGridField();
-
+					if (grid.data("immediate-update") && postback)
-					var data = grid.getItems().map(function() {
-						return { name: "order[]", value: $(this).data("id") };
-					});
-
-					if (grid.data("immediate-update"))
 					{
 						grid.reload({
-							url: grid.data("url-reorder"),
+							url: grid.data("url-reorder")
-							data: data.get()
 						});
 					}
 					else

templates/GridFieldAddNewInlineRow.ss

@@ -1,5 +1,5 @@
 <script type="text/x-tmpl" class="ss-gridfield-add-inline-template">
-	<tr class="ss-gridfield-inline-new">
+	<tr class="ss-gridfield-item ss-gridfield-inline-new">
 		<% loop $Me %>
 			<% if $IsActions %>
 				<td$Attributes>

templates/GridFieldOrderableRowsDragHandle.ss

@@ -1 +1,2 @@
 <span class="handle"><span class="icon"></span></span>
+$SortField

tests/GridFieldOrderableRowsTest.php

@@ -32,13 +32,18 @@ class GridFieldOrderableRowsTest extends SapphireTest {
 		);
 
 		$originalOrder = $parent->MyManyMany()->sort('ManyManySort')->column('ID');
-		$desiredOrder = array_reverse($originalOrder);
+		$desiredOrder = array();
+
+		// Make order non-contiguous, and 1-based
+		foreach(array_reverse($originalOrder) as $index => $id) {
+			$desiredOrder[$index * 2 + 1] = $id;
+		}
 
 		$this->assertNotEquals($originalOrder, $desiredOrder);
 
 		$reflection->invoke($orderable, $grid, $desiredOrder);
 
-		$newOrder = $parent->MyManyMany()->sort('ManyManySort')->column('ID');
+		$newOrder = $parent->MyManyMany()->sort('ManyManySort')->map('ManyManySort', 'ID')->toArray();
 
 		$this->assertEquals($desiredOrder, $newOrder);