Skip to content

Commit 23a5154

Browse files
chrispennychrispenny
authored
ENH: Update reorderItems() to use ORM where possible (#336)
* ENH: Update reorderItems() to use ORM where possible * Increase test coverage of orderable rows
1 parent 7a8f244 commit 23a5154

8 files changed

Lines changed: 181 additions & 40 deletions

‎src/GridFieldOrderableRows.php‎

Lines changed: 8 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,10 @@ protected function executeReorder(GridField $grid, $sortedIDs)
587587
*/
588588
protected function reorderItems($list, array $values, array $sortedIDs)
589589
{
590+
$this->extend('onBeforeReorderItems', $list, $values, $sortedIDs);
591+
590592
// setup
591593
$sortField = $this->getSortField();
592-
$class = $list->dataClass();
593594
// The problem is that $sortedIDs is a list of the _related_ item IDs, which causes trouble
594595
// with ManyManyThrough, where we need the ID of the _join_ item in order to set the value.
595596
$itemToSortReference = ($list instanceof ManyManyThroughList) ? 'getJoin' : 'Me';
@@ -598,53 +599,21 @@ protected function reorderItems($list, array $values, array $sortedIDs)
598599
// sanity check.
599600
$this->validateSortField($list);
600601

601-
$isVersioned = false;
602-
// check if sort column is present on the model provided by dataClass() and if it's versioned
603-
// cases:
604-
// Model has sort column and is versioned - handle as versioned
605-
// Model has sort column and is NOT versioned - handle as NOT versioned
606-
// Model doesn't have sort column because sort column is on ManyManyList - handle as NOT versioned
607-
// Model doesn't have sort column because sort column is on ManyManyThroughList - inspect through object
608-
if ($list instanceof ManyManyThroughList) {
609-
// We'll be updating the join class, not the relation class.
610-
$class = $this->getManyManyInspector($list)->getJoinClass();
611-
$isVersioned = $class::create()->hasExtension(Versioned::class);
612-
} elseif (!$this->isManyMany($list)) {
613-
$isVersioned = $class::create()->hasExtension(Versioned::class);
614-
}
615-
616-
// Loop through each item, and update the sort values which do not
617-
// match to order the objects.
618-
if (!$isVersioned) {
602+
// ManyManyList extra fields aren't easily updated via the ORM, and so they need to be updated through an SQL
603+
// Query
604+
if ($list instanceof ManyManyList) {
619605
$sortTable = $this->getSortTable($list);
620-
$now = DBDatetime::now()->Rfc2822();
621-
$additionalSQL = '';
622-
$baseTable = DataObject::getSchema()->baseDataTable($class);
623-
624-
$isBaseTable = ($baseTable == $sortTable);
625-
if (!$list instanceof ManyManyList && $isBaseTable) {
626-
$additionalSQL = ", \"LastEdited\" = '$now'";
627-
}
628606

607+
// Loop through each item, and update the sort values which do not match to order the objects.
629608
foreach ($sortedIDs as $newSortValue => $targetRecordID) {
630609
if ($currentSortList[$targetRecordID]->$sortField != $newSortValue) {
631610
DB::query(sprintf(
632-
'UPDATE "%s" SET "%s" = %d%s WHERE %s',
611+
'UPDATE "%s" SET "%s" = %d WHERE %s',
633612
$sortTable,
634613
$sortField,
635614
$newSortValue,
636-
$additionalSQL,
637615
$this->getSortTableClauseForIds($list, $targetRecordID)
638616
));
639-
640-
if (!$isBaseTable && !$list instanceof ManyManyList) {
641-
DB::query(sprintf(
642-
'UPDATE "%s" SET "LastEdited" = \'%s\' WHERE %s',
643-
$baseTable,
644-
$now,
645-
$this->getSortTableClauseForIds($list, $targetRecordID)
646-
));
647-
}
648617
}
649618
}
650619
} else {
@@ -656,6 +625,7 @@ protected function reorderItems($list, array $values, array $sortedIDs)
656625
// either the list data class (has_many, (belongs_)many_many)
657626
// or the intermediary join class (many_many through)
658627
$record = $currentSortList[$targetRecordID];
628+
659629
if ($record->$sortField != $newSortValue) {
660630
$record->$sortField = $newSortValue;
661631
$record->write();

‎tests/GridFieldOrderableRowsTest.php‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor;
99
use SilverStripe\ORM\DataList;
1010
use Symbiote\GridFieldExtensions\GridFieldOrderableRows;
11-
use Symbiote\GridFieldExtensions\Tests\Stub\PolymorphM2MChild;
1211
use Symbiote\GridFieldExtensions\Tests\Stub\PolymorphM2MMapper;
1312
use Symbiote\GridFieldExtensions\Tests\Stub\PolymorphM2MParent;
1413
use Symbiote\GridFieldExtensions\Tests\Stub\StubOrderableChild;
@@ -18,11 +17,14 @@
1817
use Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass;
1918
use Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned;
2019
use Symbiote\GridFieldExtensions\Tests\Stub\StubUnorderable;
20+
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs;
21+
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned;
2122
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner;
23+
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned;
2224
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediary;
23-
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs;
2425
use Symbiote\GridFieldExtensions\Tests\Stub\TitleObject;
2526
use Symbiote\GridFieldExtensions\Tests\Stub\TitleSortedObject;
27+
use Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediaryVersioned;
2628

2729
/**
2830
* Tests for the {@link GridFieldOrderableRows} component.
@@ -51,13 +53,21 @@ class GridFieldOrderableRowsTest extends SapphireTest
5153
ThroughBelongs::class,
5254
TitleObject::class,
5355
TitleSortedObject::class,
56+
ThroughDefinerVersioned::class,
57+
ThroughIntermediaryVersioned::class,
58+
ThroughBelongsVersioned::class,
5459
];
5560

5661
public function reorderItemsProvider()
5762
{
5863
return [
64+
[StubParent::class . '.parent', 'MyHasMany', 'Sort'],
65+
[StubParent::class . '.parent', 'MyHasManySubclass', 'Sort'],
66+
[StubParent::class . '.parent-subclass-ordered-versioned', 'MyHasManySubclassOrderedVersioned', 'Sort'],
5967
[StubParent::class . '.parent', 'MyManyMany', 'ManyManySort'],
68+
[StubParent::class . '.parent', 'MyManyManyVersioned', 'ManyManySort'],
6069
[ThroughDefiner::class . '.DefinerOne', 'Belongings', 'Sort'],
70+
[ThroughDefinerVersioned::class . '.DefinerOne', 'Belongings', 'Sort'],
6171
// [PolymorphM2MParent::class . '.ParentOne', 'Children', 'Sort']
6272
];
6373
}

‎tests/GridFieldOrderableRowsTest.yml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Symbiote\GridFieldExtensions\Tests\Stub\StubOrderableChild:
77
Sort: 3
88
item4:
99
Sort: 4
10+
1011
Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered:
1112
item1:
1213
Sort: 1
@@ -27,6 +28,20 @@ Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered:
2728
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrderableChild.item3
2829
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrderableChild.item4
2930

31+
Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass:
32+
item1:
33+
Sort: 1
34+
item2:
35+
Sort: 2
36+
item3:
37+
Sort: 3
38+
item4:
39+
Sort: 4
40+
item5:
41+
Sort: 5
42+
item6:
43+
Sort: 6
44+
3045
Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned:
3146
item1:
3247
ExtraField: 1
@@ -56,6 +71,29 @@ Symbiote\GridFieldExtensions\Tests\Stub\StubParent:
5671
ManyManySort: 108
5772
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item6:
5873
ManyManySort: 108
74+
MyManyManyVersioned:
75+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned.item1:
76+
ManyManySort: 1
77+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned.item2:
78+
ManyManySort: 1
79+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned.item3:
80+
ManyManySort: 108
81+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned.item4:
82+
ManyManySort: 108
83+
MyHasMany:
84+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item1
85+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item2
86+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item3
87+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item4
88+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item5
89+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered.item6
90+
MyHasManySubclass:
91+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass.item1
92+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass.item2
93+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass.item3
94+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass.item4
95+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass.item5
96+
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass.item6
5997
parent-subclass-ordered-versioned:
6098
MyHasManySubclassOrderedVersioned:
6199
- =>Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned.item1

‎tests/OrderableRowsThroughTest.yml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,34 @@ Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediary:
2828
Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner.DefinerTwo
2929
Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs.BelongsThree
3030
Sort: 2
31+
32+
Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned:
33+
DefinerOne:
34+
DefinerTwo:
35+
36+
Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned:
37+
BelongsOne:
38+
BelongsTwo:
39+
BelongsThree:
40+
41+
Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediaryVersioned:
42+
One:
43+
Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned.DefinerOne
44+
Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned.BelongsOne
45+
Sort: 3
46+
Two:
47+
Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned.DefinerOne
48+
Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned.BelongsTwo
49+
Sort: 2
50+
Three:
51+
Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned.DefinerOne
52+
Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned.BelongsThree
53+
Sort: 1
54+
Four:
55+
Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned.DefinerTwo
56+
Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned.BelongsTwo
57+
Sort: 1
58+
Five:
59+
Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefinerVersioned.DefinerTwo
60+
Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongsVersioned.BelongsThree
61+
Sort: 2

‎tests/Stub/StubParent.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ class StubParent extends DataObject implements TestOnly
1515

1616
private static $many_many = [
1717
'MyManyMany' => StubOrdered::class,
18+
'MyManyManyVersioned' => StubSubclassOrderedVersioned::class,
1819
];
1920

2021
private static $many_many_extraFields = [
2122
'MyManyMany' => ['ManyManySort' => 'Int'],
23+
'MyManyManyVersioned' => ['ManyManySort' => 'Int'],
2224
];
2325

2426
private static $table_name = 'StubParent';

‎tests/Stub/ThroughBelongsVersioned.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
namespace Symbiote\GridFieldExtensions\Tests\Stub;
4+
5+
use SilverStripe\Dev\TestOnly;
6+
use SilverStripe\ORM\DataObject;
7+
use SilverStripe\ORM\ManyManyList;
8+
use SilverStripe\Versioned\Versioned;
9+
10+
/**
11+
* @method ManyManyList|ThroughDefinerVersioned[] Definers()
12+
* @mixin Versioned
13+
*/
14+
class ThroughBelongsVersioned extends DataObject implements TestOnly
15+
{
16+
private static string $table_name = 'ThroughBelongsVersioned';
17+
18+
private static array $belongs_many_many = [
19+
'Definers' => ThroughDefinerVersioned::class,
20+
];
21+
22+
private static array $extensions = [
23+
Versioned::class,
24+
];
25+
}

‎tests/Stub/ThroughDefinerVersioned.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
namespace Symbiote\GridFieldExtensions\Tests\Stub;
4+
5+
use SilverStripe\Dev\TestOnly;
6+
use SilverStripe\ORM\DataObject;
7+
use SilverStripe\ORM\ManyManyThroughList;
8+
use SilverStripe\Versioned\Versioned;
9+
10+
/**
11+
* @method ManyManyThroughList|ThroughIntermediaryVersioned Belongings()
12+
* @mixin Versioned
13+
*/
14+
class ThroughDefinerVersioned extends DataObject implements TestOnly
15+
{
16+
private static string $table_name = 'ThroughDefinerVersioned';
17+
18+
private static array $many_many = [
19+
'Belongings' => [
20+
'through' => ThroughIntermediaryVersioned::class,
21+
'from' => 'Defining',
22+
'to' => 'Belonging',
23+
]
24+
];
25+
26+
private static array $owns = [
27+
'Belongings'
28+
];
29+
30+
private static array $extensions = [
31+
Versioned::class,
32+
];
33+
}

‎tests/Stub/ThroughIntermediaryVersioned.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Symbiote\GridFieldExtensions\Tests\Stub;
4+
5+
use SilverStripe\Dev\TestOnly;
6+
use SilverStripe\ORM\DataObject;
7+
use SilverStripe\Versioned\Versioned;
8+
9+
/**
10+
* @property int $DefiningID
11+
* @property int $BelongingID
12+
* @method ThroughDefinerVersioned Defining()
13+
* @method ThroughBelongsVersioned Belonging()
14+
* @mixin Versioned
15+
*/
16+
class ThroughIntermediaryVersioned extends DataObject implements TestOnly
17+
{
18+
private static string $table_name = 'ThroughIntermediaryVersioned';
19+
20+
private static array $db = [
21+
'Sort' => 'Int',
22+
];
23+
24+
private static array $has_one = [
25+
'Defining' => ThroughDefinerVersioned::class,
26+
'Belonging' => ThroughBelongsVersioned::class,
27+
];
28+
29+
private static array $extensions = [
30+
Versioned::class,
31+
];
32+
}

0 commit comments

Comments
 (0)