Merge pull request #154 from dhensby/pulls/simple-reorder

Bug with reordering sortable grid when some sort IDs are duplicated
This commit is contained in:
Marcus 2016-08-04 17:19:43 +10:00 committed by GitHub
commit b93b32b3f9
5 changed files with 103 additions and 24 deletions

View File

@ -1,25 +1,36 @@
language: php
# See https://github.com/silverstripe-labs/silverstripe-travis-support for setup details
sudo: false
php:
language: php
php:
- 5.3
- 5.4
- 5.5
- 5.6
env:
- DB=MYSQL CORE_RELEASE=3.1
- DB=MYSQL CORE_RELEASE=3.2
matrix:
include:
- php: 5.5
- php: 5.6
env: DB=MYSQL CORE_RELEASE=3
- php: 5.6
env: DB=MYSQL CORE_RELEASE=3.2
env: DB=PGSQL CORE_RELEASE=3.1
- php: 5.6
env: DB=PGSQL CORE_RELEASE=3.3
- php: 5.6
env: DB=PGSQL CORE_RELEASE=3.4
fast_finish: true
before_script:
- phpenv rehash
- composer self-update || true
- git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support
- php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss
- cd ~/builds/ss
- composer install
script:
- phpunit gridfieldextensions/tests
- vendor/bin/phpunit gridfieldextensions/tests

View File

@ -413,9 +413,13 @@ class GridFieldOrderableRows extends RequestHandler implements
}
protected function reorderItems($list, array $values, array $order) {
// Get a list of sort values that can be used.
$pool = array_values($values);
sort($pool);
$sortField = $this->getSortField();
/** @var SS_List $map */
$map = $list->map('ID', $sortField);
//fix for versions of SS that return inconsistent types for `map` function
if ($map instanceof SS_Map) {
$map = $map->toArray();
}
// If not a ManyManyList and using versioning, detect it.
$isVersioned = false;
@ -427,14 +431,15 @@ class GridFieldOrderableRows extends RequestHandler implements
// Loop through each item, and update the sort values which do not
// match to order the objects.
if (!$isVersioned) {
$sortTable = $this->getSortTable($list);
$additionalSQL = (!$list instanceof ManyManyList) ? ', "LastEdited" = NOW()' : '';
foreach(array_values($order) as $pos => $id) {
if($values[$id] != $pool[$pos]) {
if($map[$id] != $pos) {
DB::query(sprintf(
'UPDATE "%s" SET "%s" = %d%s WHERE %s',
$this->getSortTable($list),
$this->getSortField(),
$pool[$pos],
$sortTable,
$sortField,
$pos,
$additionalSQL,
$this->getSortTableClauseForIds($list, $id)
));
@ -445,11 +450,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.
$sortField = $this->getSortField();
foreach(array_values($order) as $pos => $id) {
if($values[$id] != $pool[$pos]) {
if($map[$id] != $pos) {
$record = $class::get()->byID($id);
$record->$sortField = $pool[$pos];
$record->$sortField = $pos;
$record->write();
}
}

View File

@ -41,7 +41,7 @@ class GridFieldAddNewMultiClassTest extends SapphireTest {
* @ignore
*/
class GridFieldAddNewMultiClassTest_A {
class GridFieldAddNewMultiClassTest_A implements TestOnly {
public function i18n_singular_name() {
$class = get_class($this);
return substr($class, strpos($class, '_') + 1);
@ -52,7 +52,7 @@ class GridFieldAddNewMultiClassTest_A {
}
}
class GridFieldAddNewMultiClassTest_B extends GridFieldAddNewMultiClassTest_A {}
class GridFieldAddNewMultiClassTest_C extends GridFieldAddNewMultiClassTest_A {}
class GridFieldAddNewMultiClassTest_B extends GridFieldAddNewMultiClassTest_A implements TestOnly {}
class GridFieldAddNewMultiClassTest_C extends GridFieldAddNewMultiClassTest_A implements TestOnly {}
/**#@-*/

View File

@ -6,6 +6,44 @@ class GridFieldOrderableRowsTest extends SapphireTest {
protected $usesDatabase = true;
protected static $fixture_file = 'GridFieldOrderableRowsTest.yml';
protected $extraDataObjects = array(
'GridFieldOrderableRowsTest_Parent',
'GridFieldOrderableRowsTest_Ordered',
'GridFieldOrderableRowsTest_Subclass',
);
public function testReorderItems() {
$orderable = new GridFieldOrderableRows('ManyManySort');
$reflection = new ReflectionMethod($orderable, 'executeReorder');
$reflection->setAccessible(true);
$parent = $this->objFromFixture('GridFieldOrderableRowsTest_Parent', 'parent');
$config = new GridFieldConfig_RelationEditor();
$config->addComponent($orderable);
$grid = new GridField(
'MyManyMany',
'My Many Many',
$parent->MyManyMany()->sort('ManyManySort'),
$config
);
$originalOrder = $parent->MyManyMany()->sort('ManyManySort')->column('ID');
$desiredOrder = array_reverse($originalOrder);
$this->assertNotEquals($originalOrder, $desiredOrder);
$reflection->invoke($orderable, $grid, $desiredOrder);
$newOrder = $parent->MyManyMany()->sort('ManyManySort')->column('ID');
$this->assertEquals($desiredOrder, $newOrder);
}
/**
* @covers GridFieldOrderableRows::getSortTable
*/
@ -42,7 +80,7 @@ class GridFieldOrderableRowsTest extends SapphireTest {
* @ignore
*/
class GridFieldOrderableRowsTest_Parent extends DataObject {
class GridFieldOrderableRowsTest_Parent extends DataObject implements TestOnly {
private static $has_many = array(
'MyHasMany' => 'GridFieldOrderableRowsTest_Ordered',
@ -59,7 +97,7 @@ class GridFieldOrderableRowsTest_Parent extends DataObject {
}
class GridFieldOrderableRowsTest_Ordered extends DataObject {
class GridFieldOrderableRowsTest_Ordered extends DataObject implements TestOnly {
private static $db = array(
'Sort' => 'Int'
@ -69,9 +107,13 @@ class GridFieldOrderableRowsTest_Ordered extends DataObject {
'Parent' => 'GridFieldOrderableRowsTest_Parent'
);
private static $belongs_many_many =array(
'MyManyMany' => 'GridFieldOrderableRowsTest_Parent',
);
}
class GridFieldOrderableRowsTest_Subclass extends GridFieldOrderableRowsTest_Ordered {
class GridFieldOrderableRowsTest_Subclass extends GridFieldOrderableRowsTest_Ordered implements TestOnly {
}
/**#@-*/

View File

@ -0,0 +1,22 @@
GridFieldOrderableRowsTest_Ordered:
item1:
item2:
item3:
item4:
item5:
item6:
GridFieldOrderableRowsTest_Parent:
parent:
MyManyMany:
- 0: =>GridFieldOrderableRowsTest_Ordered.item1
ManyManySort: 1
- 1: =>GridFieldOrderableRowsTest_Ordered.item2
ManyManySort: 1
- 2: =>GridFieldOrderableRowsTest_Ordered.item3
ManyManySort: 2
- 3: =>GridFieldOrderableRowsTest_Ordered.item4
ManyManySort: 2
- 4: =>GridFieldOrderableRowsTest_Ordered.item5
ManyManySort: 108
- 5: =>GridFieldOrderableRowsTest_Ordered.item6
ManyManySort: 108