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.
This commit is contained in:
Jonathon Menz 2015-05-16 15:49:11 -07:00
parent c1d500dedc
commit c6bcfea3e3
2 changed files with 32 additions and 1 deletions

View File

@ -561,7 +561,7 @@ class FieldList extends ArrayList {
} }
// Add the leftover fields to the end of the list. // 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. // Update our internal $this->items parameter.
$this->items = $fields; $this->items = $fields;

View File

@ -753,6 +753,37 @@ class FieldListTest extends SapphireTest {
$this->assertEquals(3, $tabB2->fieldPosition('B_post')); $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() { public function testFieldPosition() {
$set = new FieldList( $set = new FieldList(
new TextField('A'), new TextField('A'),