diff --git a/src/Database/Traits/HasSortableRelations.php b/src/Database/Traits/HasSortableRelations.php index f613e476f..630a975c7 100644 --- a/src/Database/Traits/HasSortableRelations.php +++ b/src/Database/Traits/HasSortableRelations.php @@ -2,7 +2,8 @@ use Exception; -use Winter\Sorm\Database\Model; +use Winter\Storm\Database\Model; +use Winter\Storm\Database\Models\DeferredBinding; /** * HasSortableRelations trait @@ -17,7 +18,7 @@ * * To set orders: * - * $model->setSortableRelationOrder($relationName, $recordIds, $recordOrders); + * $model->setRelationOrder($relationName, $recordIds, $recordOrders); * */ trait HasSortableRelations @@ -41,8 +42,20 @@ public function initializeHasSortableRelations() : void if (array_key_exists($relationName, $sortableRelations)) { $column = $this->getRelationSortOrderColumn($relationName); + // If the records were attached with an explicit sort order (e.g. when committing + // deferred bindings that already carry a pivot sort order), keep it - otherwise + // auto-append the records to the end of the relation. $data is the list of pivot + // insert rows, one per attached record. + if (is_array($data)) { + foreach ($data as $row) { + if (is_array($row) && isset($row[$column])) { + return; + } + } + } + foreach ($attached as $id) { - $this->updateRelationOrder($relationName, $id, $column); + $this->updateRelationOrder($relationName, $id, $column, null); } } }); @@ -52,39 +65,62 @@ public function initializeHasSortableRelations() : void if (array_key_exists($relationName, $sortableRelations)) { $column = $this->getRelationSortOrderColumn($relationName); - $this->updateRelationOrder($relationName, $relatedModel->getKey(), $column); + // No order - auto-append to the end of the relation. + $this->updateRelationOrder($relationName, $relatedModel->getKey(), $column, null); } }); foreach ($sortableRelations as $relationName => $column) { $relationType = $this->getRelationType($relationName); - if (!in_array($relationType, ['belongsToMany', 'morphToMany'])) { + if (!in_array($relationType, ['belongsToMany', 'morphToMany', 'morphedByMany'])) { continue; } $definition = $this->getRelationDefinition($relationName); - $pivot = array_wrap(array_get($definition, 'pivot', [])); + // Make sure the sort order column is available as pivot data. + $pivot = array_wrap(array_get($definition, 'pivot', [])); if (!in_array($column, $pivot)) { - // Make sure the sort order column is available as pivot data. $pivot[] = $column; $definition['pivot'] = $pivot; - $this->$relationType[$relationName] = $definition; } + + // Auto-add an ordering clause so the relation is returned in sort order by default. + // Qualify with the pivot table name to avoid colliding with a column of the same + // name on the related model (which would silently order by the wrong column). + if (!array_key_exists('order', $definition)) { + $pivotTable = array_get($definition, 'table'); + $definition['order'] = ($pivotTable ? $pivotTable . '.' : '') . $column . ' asc'; + } + + $this->$relationType[$relationName] = $definition; } } /** - * Set the sort order of records to the specified orders. If the orders is - * undefined, the record identifier is used. + * Set the sort order of records to the specified orders. If the orders are + * undefined, a sequential 1..N order is assigned in the given id order. + * + * When a $sessionKey is provided the relation is operating in deferred mode: + * the sort order is written to the pivot_data of the matching deferred_bindings + * records instead of the pivot table. The caller (e.g. RelationController) is + * responsible for passing the sessionKey only when in deferred mode. */ - public function setRelationOrder(string $relationName, string|int|array $itemIds, array $itemOrders = []) : void - { + public function setRelationOrder( + string $relationName, + string|int|array $itemIds, + array $itemOrders = [], + ?string $sessionKey = null + ) : void { if (!is_array($itemIds)) { $itemIds = [$itemIds]; } + if (empty($itemIds)) { + return; + } + if (empty($itemOrders)) { - $itemOrders = $itemIds; + $itemOrders = range(1, count($itemIds)); } if (count($itemIds) != count($itemOrders)) { @@ -93,21 +129,56 @@ public function setRelationOrder(string $relationName, string|int|array $itemIds $column = $this->getRelationSortOrderColumn($relationName); + /* + * Deferred mode - update pivot_data on the deferred_bindings records. + * Batch the lookup into a single query, then save each binding (each row + * carries its own JSON pivot_data so individual saves are required). + */ + if ($sessionKey) { + $bindings = DeferredBinding::where('master_type', get_class($this)) + ->where('master_field', $relationName) + ->whereIn('slave_id', $itemIds) + ->where('session_key', $sessionKey) + ->where('is_bind', 1) + ->get() + ->keyBy('slave_id'); + + foreach ($itemIds as $index => $id) { + if ($binding = $bindings->get($id)) { + $pivotData = $binding->pivot_data ?: []; + $pivotData[$column] = (int) $itemOrders[$index]; + $binding->pivot_data = $pivotData; + $binding->save(); + } + } + + return; + } + foreach ($itemIds as $index => $id) { - $order = (int)$itemOrders[$index]; - $this->updateRelationOrder($relationName, $id, $column, $order); + // Pass the explicit order (which may legitimately be 0) so it is not mistaken + // for the "auto-append" case. + $this->updateRelationOrder($relationName, $id, $column, (int) $itemOrders[$index]); } } /** - * Update relation record sort_order. + * Update relation record sort_order. A null $order auto-appends the record to the end + * of the relation; an explicit integer (including 0) is written as-is. */ - protected function updateRelationOrder(string $relationName, string|int|Model $id, string $column, int $order = 0) : void + protected function updateRelationOrder(string $relationName, string|int|Model $id, string $column, ?int $order = null) : void { $relation = $this->{$relationName}(); - if (!$order) { - $order = $relation->count(); + if ($order === null) { + // Append to the end of the relation. For pivot-based relations use the pivot + // query directly so a defined "order" clause on the relation cannot interfere + // with the aggregate, and so non-contiguous sort orders are handled correctly. + if (method_exists($relation, 'newPivotQuery')) { + $order = ((int) $relation->newPivotQuery()->max($column)) + 1; + } else { + $order = $relation->count() + 1; + } } if (method_exists($relation, 'updateExistingPivot')) { $relation->updateExistingPivot($id, [ $column => (int)$order ]); @@ -123,7 +194,15 @@ protected function updateRelationOrder(string $relationName, string|int|Model $i } /** - * Get the name of the "sort_order" column. + * Returns true if the given relation is configured as a sortable relation. + */ + public function isSortableRelation(string $relationName) : bool + { + return array_key_exists($relationName, $this->getSortableRelations()); + } + + /** + * Get the name of the "sort_order" column for the given relation. */ public function getRelationSortOrderColumn(string $relationName) : string { @@ -133,7 +212,7 @@ public function getRelationSortOrderColumn(string $relationName) : string /** * Return all configured sortable relations. */ - protected function getSortableRelations() : array + public function getSortableRelations() : array { if (property_exists($this, 'sortableRelations')) { return $this->sortableRelations; diff --git a/tests/Database/Fixtures/SortableArticle.php b/tests/Database/Fixtures/SortableArticle.php new file mode 100644 index 000000000..fad7cecfa --- /dev/null +++ b/tests/Database/Fixtures/SortableArticle.php @@ -0,0 +1,74 @@ + 'sort_order', + ]; + + /** + * @var array Relations + */ + public $belongsToMany = [ + 'authors' => [ + Author::class, + 'table' => 'database_tester_sortable_article_author', + 'key' => 'article_id', + 'otherKey' => 'author_id', + ], + ]; + + public static function migrateUp(Builder $builder): void + { + if ($builder->hasTable('database_tester_sortable_articles')) { + return; + } + + $builder->create('database_tester_sortable_articles', function ($table) { + $table->engine = 'InnoDB'; + $table->increments('id'); + $table->string('title')->nullable(); + $table->timestamps(); + }); + + $builder->create('database_tester_sortable_article_author', function ($table) { + $table->engine = 'InnoDB'; + $table->integer('article_id')->unsigned(); + $table->integer('author_id')->unsigned(); + $table->integer('sort_order')->default(0); + $table->primary(['article_id', 'author_id']); + }); + } + + public static function migrateDown(Builder $builder): void + { + if (!$builder->hasTable('database_tester_sortable_articles')) { + return; + } + + $builder->dropIfExists('database_tester_sortable_article_author'); + $builder->dropIfExists('database_tester_sortable_articles'); + } +} diff --git a/tests/Database/Traits/HasSortableRelationsTest.php b/tests/Database/Traits/HasSortableRelationsTest.php index b9cf350b0..d67cee734 100644 --- a/tests/Database/Traits/HasSortableRelationsTest.php +++ b/tests/Database/Traits/HasSortableRelationsTest.php @@ -1,26 +1,211 @@ assertTrue($model instanceof TestModel); + Model::unguard(); + $article = SortableArticle::create(['title' => 'Article']); + $a = Author::create(['name' => 'A']); + $b = Author::create(['name' => 'B']); + $c = Author::create(['name' => 'C']); + Model::reguard(); + + return [$article, $a, $b, $c]; } -} -/* -* Class with HasSortableRelations trait -*/ -class TestModel extends \Winter\Storm\Database\Model -{ - use \Winter\Storm\Database\Traits\HasSortableRelations; + public function testGetSortableRelationsFromProperty() + { + $article = new SortableArticle(); + $this->assertSame(['authors' => 'sort_order'], $article->getSortableRelations()); + } - protected $sortableRelations = [ - 'relationToSelf' => 'sort_order', - ]; + public function testGetSortableRelationsIsPublic() + { + // Behaviors call this externally; it must be public. + $method = new \ReflectionMethod(SortableArticle::class, 'getSortableRelations'); + $this->assertTrue($method->isPublic()); + } + + public function testIsSortableRelation() + { + $article = new SortableArticle(); + $this->assertTrue($article->isSortableRelation('authors')); + $this->assertFalse($article->isSortableRelation('nonexistent')); + } + + public function testGetRelationSortOrderColumn() + { + $article = new SortableArticle(); + $this->assertSame('sort_order', $article->getRelationSortOrderColumn('authors')); + // Falls back to the default when the relation is not configured. + $this->assertSame('sort_order', $article->getRelationSortOrderColumn('whatever')); + } + + public function testInitAddsPivotColumnAndQualifiedOrderClause() + { + $article = new SortableArticle(); + $definition = $article->belongsToMany['authors']; - public $belongsToMany = [ - 'relationToSelf' => TestModel::class, - ]; + $this->assertContains('sort_order', $definition['pivot']); + // Order clause must be qualified with the pivot table name to avoid + // colliding with a same-named column on the related model. + $this->assertSame('database_tester_sortable_article_author.sort_order asc', $definition['order']); + } + + public function testInitInjectsForMorphToMany() + { + $model = new class extends Model { + use HasSortableRelations; + public $morphToMany = [ + 'tags' => [Tag::class, 'name' => 'taggable', 'table' => 'database_tester_taggables'], + ]; + public $sortableRelations = ['tags' => 'sort_order']; + }; + + $this->assertContains('sort_order', $model->morphToMany['tags']['pivot']); + $this->assertSame('database_tester_taggables.sort_order asc', $model->morphToMany['tags']['order']); + } + + public function testInitInjectsForMorphedByMany() + { + $model = new class extends Model { + use HasSortableRelations; + public $morphedByMany = [ + 'posts' => [Tag::class, 'name' => 'taggable', 'table' => 'database_tester_taggables'], + ]; + public $sortableRelations = ['posts' => 'sort_order']; + }; + + $this->assertContains('sort_order', $model->morphedByMany['posts']['pivot']); + $this->assertSame('database_tester_taggables.sort_order asc', $model->morphedByMany['posts']['order']); + } + + public function testAutoAttachAssignsSequentialSortOrder() + { + [$article, $a, $b, $c] = $this->makeArticleWithAuthors(); + + $article->authors()->attach($a->id); + $article->authors()->attach($b->id); + $article->authors()->attach($c->id); + + $fresh = SortableArticle::find($article->id); + $orders = $fresh->authors->pluck('pivot.sort_order', 'name')->all(); + $this->assertSame(['A' => 1, 'B' => 2, 'C' => 3], $orders); + } + + public function testSetRelationOrderReassignsAndReturnsInOrder() + { + [$article, $a, $b, $c] = $this->makeArticleWithAuthors(); + $article->authors()->attach([$a->id, $b->id, $c->id]); + + // Drag C to the front: new id order [C, A, B], explicit orders [1, 2, 3] + $article->setRelationOrder('authors', [$c->id, $a->id, $b->id], [1, 2, 3]); + + $fresh = SortableArticle::find($article->id); + $this->assertSame(['C', 'A', 'B'], $fresh->authors->pluck('name')->all()); + $this->assertSame( + [$c->id => 1, $a->id => 2, $b->id => 3], + $fresh->authors->pluck('pivot.sort_order', 'pivot.author_id')->all() + ); + } + + public function testSetRelationOrderEmptyOrdersUsesSequentialRangeNotIds() + { + [$article, $a, $b, $c] = $this->makeArticleWithAuthors(); + $article->authors()->attach([$a->id, $b->id, $c->id]); + + // No explicit orders -> 1..N in the given id order (NOT the ids themselves) + $article->setRelationOrder('authors', [$c->id, $a->id, $b->id]); + + $fresh = SortableArticle::find($article->id); + $this->assertSame( + [$c->id => 1, $a->id => 2, $b->id => 3], + $fresh->authors->pluck('pivot.sort_order', 'pivot.author_id')->all() + ); + } + + public function testAttachWithExplicitSortOrderIsNotAutoAppended() + { + [$article, $a, $b, $c] = $this->makeArticleWithAuthors(); + + // Attaching with an explicit pivot sort order (as happens when committing deferred + // bindings that carry pivot_data) must be preserved, not overwritten by auto-append. + $article->authors()->attach($a->id, ['sort_order' => 5]); + $article->authors()->attach($b->id, ['sort_order' => 9]); + // No explicit order -> auto-append to the end. + $article->authors()->attach($c->id); + + $fresh = SortableArticle::find($article->id); + $orders = $fresh->authors->pluck('pivot.sort_order', 'pivot.author_id')->all(); + + $this->assertSame(5, (int) $orders[$a->id]); + $this->assertSame(9, (int) $orders[$b->id]); + $this->assertSame(10, (int) $orders[$c->id]); // max(9) + 1 + } + + public function testSetRelationOrderPreservesExplicitZeroOrder() + { + [$article, $a, $b, $c] = $this->makeArticleWithAuthors(); + $article->authors()->attach([$a->id, $b->id, $c->id]); + + // An explicit sort order of 0 must be written as 0, not treated as "auto-append". + $article->setRelationOrder('authors', [$c->id, $a->id, $b->id], [0, 1, 2]); + + $fresh = SortableArticle::find($article->id); + $this->assertSame( + [$c->id => 0, $a->id => 1, $b->id => 2], + $fresh->authors->pluck('pivot.sort_order', 'pivot.author_id')->all() + ); + $this->assertSame(['C', 'A', 'B'], $fresh->authors->pluck('name')->all()); + } + + public function testSetRelationOrderDeferredWritesToPivotData() + { + DeferredBinding::truncate(); + + Model::unguard(); + $a = Author::create(['name' => 'A']); + $b = Author::create(['name' => 'B']); + $c = Author::create(['name' => 'C']); + Model::reguard(); + + // Unsaved parent -> deferred binding + $article = new SortableArticle(); + $article->title = 'Deferred'; + $sessionKey = uniqid('session_key', true); + + $article->authors()->add($a, $sessionKey); + $article->authors()->add($b, $sessionKey); + $article->authors()->add($c, $sessionKey); + + $article->setRelationOrder('authors', [$c->id, $a->id, $b->id], [1, 2, 3], $sessionKey); + + $bindings = DeferredBinding::where('session_key', $sessionKey) + ->where('is_bind', 1) + ->get() + ->keyBy('slave_id'); + + $this->assertSame(1, (int) $bindings[$c->id]->pivot_data['sort_order']); + $this->assertSame(2, (int) $bindings[$a->id]->pivot_data['sort_order']); + $this->assertSame(3, (int) $bindings[$b->id]->pivot_data['sort_order']); + } + + public function testSetRelationOrderThrowsOnCountMismatch() + { + [$article] = $this->makeArticleWithAuthors(); + + $this->expectException(\Exception::class); + $article->setRelationOrder('authors', [1, 2, 3], [1, 2]); + } }