From c6bcfea3e36a4211d2f69ff5c73db2fcab474ba8 Mon Sep 17 00:00:00 2001
From: Jonathon Menz <jonomenz@gmail.com>
Date: Sat, 16 May 2015 15:49:11 -0700
Subject: [PATCH] BUG: FieldList::changeFieldOrder() leftovers discarded

Logical error. Use of + operator means items from second array are only merged if the key does not already appear in the first array. The first array has numeric keys 0,1,2 etc. The second array is keyed by field name, but array_values() resets the keys to be numberic starting at 0. This means that some or all leftovers are discarded instead of appended.
---
 forms/FieldList.php           |  2 +-
 tests/forms/FieldListTest.php | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/forms/FieldList.php b/forms/FieldList.php
index b7e535d6d..6781c452e 100644
--- a/forms/FieldList.php
+++ b/forms/FieldList.php
@@ -561,7 +561,7 @@ class FieldList extends ArrayList {
 		}
 		
 		// Add the leftover fields to the end of the list.
-		$fields = $fields + array_values($fieldMap);
+		$fields = array_values($fields + $fieldMap);
 		
 		// Update our internal $this->items parameter.
 		$this->items = $fields;
diff --git a/tests/forms/FieldListTest.php b/tests/forms/FieldListTest.php
index 0496ca905..4bfa3ab32 100644
--- a/tests/forms/FieldListTest.php
+++ b/tests/forms/FieldListTest.php
@@ -753,6 +753,37 @@ class FieldListTest extends SapphireTest {
 		$this->assertEquals(3, $tabB2->fieldPosition('B_post'));
 	}
 	
+	/**
+	 * FieldList::changeFieldOrder() should place specified fields in given
+	 * order then add any unspecified remainders at the end. Can be given an
+	 * array or list of arguments.
+	 */
+	public function testChangeFieldOrder() {
+		$fieldNames = array('A','B','C','D','E');
+		$setArray = new FieldList();
+		$setArgs = new FieldList();
+		foreach ($fieldNames as $fN) {
+			$setArray->push(new TextField($fN));
+			$setArgs->push(new TextField($fN));
+		}
+
+		$setArray->changeFieldOrder(array('D','B','E'));
+		$this->assertEquals(0, $setArray->fieldPosition('D'));
+		$this->assertEquals(1, $setArray->fieldPosition('B'));
+		$this->assertEquals(2, $setArray->fieldPosition('E'));
+		$this->assertEquals(3, $setArray->fieldPosition('A'));
+		$this->assertEquals(4, $setArray->fieldPosition('C'));
+
+		$setArgs->changeFieldOrder('D','B','E');
+		$this->assertEquals(0, $setArgs->fieldPosition('D'));
+		$this->assertEquals(1, $setArgs->fieldPosition('B'));
+		$this->assertEquals(2, $setArgs->fieldPosition('E'));
+		$this->assertEquals(3, $setArgs->fieldPosition('A'));
+		$this->assertEquals(4, $setArgs->fieldPosition('C'));
+
+		unset($setArray, $setArgs);
+	}
+	
 	public function testFieldPosition() {
 		$set = new FieldList(
 			new TextField('A'),