diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php index c5ec68257..49a6808d6 100644 --- a/src/Commands/RebalancePositionsCommand.php +++ b/src/Commands/RebalancePositionsCommand.php @@ -162,7 +162,7 @@ private function rebalanceAllNeeded( $this->newLine(); foreach ($columnsNeedingRebalancing as $columnId) { - $stats = $rebalancer->getGapStatistics($query, $columnField, (string) $columnId, $positionField); + $stats = $rebalancer->getGapStatistics($query, $columnField, $columnId, $positionField); $this->line(" - {$columnId}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); } diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index 0114d4dd3..45e7feed6 100644 --- a/src/Commands/RepairPositionsCommand.php +++ b/src/Commands/RepairPositionsCommand.php @@ -9,6 +9,7 @@ use Illuminate\Support\Facades\DB; use Relaticle\Flowforge\Services\DecimalPosition; +use function Illuminate\Support\enum_value; use function Laravel\Prompts\confirm; use function Laravel\Prompts\info; use function Laravel\Prompts\select; @@ -154,29 +155,6 @@ private function applyFilters($query) return $query; } - private function convertEnumToString($value): string - { - if (is_object($value)) { - // Handle Laravel Enums (implements UnitEnum) - if ($value instanceof \UnitEnum) { - return $value->value ?? $value->name; - } - // Handle objects with value property - if (property_exists($value, 'value')) { - return (string) $value->value; - } - // Handle objects with __toString method - if (method_exists($value, '__toString')) { - return (string) $value; - } - - // Fallback: try to get class name or serialize - return class_basename($value); - } - - return (string) $value; - } - /** * @param class-string $model * @return array{total: int, null_positions: int, duplicates: int, groups: array} @@ -211,8 +189,7 @@ private function analyzePositions(string $model, string $columnField, string $po ->groupBy($columnField) ->pluck('record_count', $columnField) ->mapWithKeys(function ($count, $key) { - // Convert enum to string value if needed - $stringKey = $this->convertEnumToString($key); + $stringKey = (string) enum_value($key); return [$stringKey => $count]; }) @@ -259,8 +236,7 @@ private function calculateChanges(string $model, string $columnField, string $po ->pluck($columnField); foreach ($groups as $group) { - // Convert enum to string for array key - $groupKey = $this->convertEnumToString($group); + $groupKey = (string) enum_value($group); $records = $this->getRecordsForStrategy($model, $columnField, $positionField, $group, $strategy, $query); diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php index 83aa18b0e..949f6b782 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -10,6 +10,8 @@ use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use function Illuminate\Support\enum_value; + /** * Handles rebalancing of positions within a column when gaps become too small. * @@ -101,10 +103,11 @@ public function findColumnsNeedingRebalancing( $columns = (clone $query) ->select($columnField) ->distinct() - ->pluck($columnField); + ->pluck($columnField) + ->map(fn ($columnId) => (string) enum_value($columnId)); - return $columns->filter(function ($columnId) use ($query, $columnField, $positionField) { - return $this->needsRebalancing($query, $columnField, (string) $columnId, $positionField); + return $columns->filter(function (string $columnId) use ($query, $columnField, $positionField) { + return $this->needsRebalancing($query, $columnField, $columnId, $positionField); })->values(); } @@ -130,10 +133,10 @@ public function rebalanceAll( ); foreach ($columnsNeedingRebalancing as $columnId) { - $results[(string) $columnId] = $this->rebalanceColumn( + $results[$columnId] = $this->rebalanceColumn( $query, $columnField, - (string) $columnId, + $columnId, $positionField ); } diff --git a/tests/Feature/PositionRebalancingTest.php b/tests/Feature/PositionRebalancingTest.php index b0b4bb0e4..fef44468e 100644 --- a/tests/Feature/PositionRebalancingTest.php +++ b/tests/Feature/PositionRebalancingTest.php @@ -4,232 +4,238 @@ use Relaticle\Flowforge\Services\DecimalPosition; use Relaticle\Flowforge\Services\PositionRebalancer; +use Relaticle\Flowforge\Tests\Fixtures\RebalancingStatusEnum; use Relaticle\Flowforge\Tests\Fixtures\Task; -beforeEach(function () { - // Create tasks with various positions - Task::create(['title' => 'Task 1', 'status' => 'todo', 'order_position' => '1000.0000000000']); - Task::create(['title' => 'Task 2', 'status' => 'todo', 'order_position' => '2000.0000000000']); - Task::create(['title' => 'Task 3', 'status' => 'todo', 'order_position' => '3000.0000000000']); -}); - -describe('PositionRebalancer::needsRebalancing()', function () { - test('detects gap below MIN_GAP', function () { - // Create tasks with very small gap - Task::create(['title' => 'Close 1', 'status' => 'in_progress', 'order_position' => '1000.0000000000']); - Task::create(['title' => 'Close 2', 'status' => 'in_progress', 'order_position' => '1000.00005']); // Gap < MIN_GAP - - $rebalancer = new PositionRebalancer; - - expect($rebalancer->needsRebalancing( - Task::query(), - 'status', - 'in_progress', - 'order_position' - ))->toBeTrue(); +use function Illuminate\Support\enum_value; + +function runPositionRebalancingTests(string $label, callable $s): void +{ + describe($label, function () use ($s) { + // $s is a callback that either converts a string to a RebalancingStatusEnum, or returns the string unchanged + beforeEach(function () use ($s) { + Task::create(['title' => 'Task 1', 'status' => $s('todo'), 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Task 2', 'status' => $s('todo'), 'order_position' => '2000.0000000000']); + Task::create(['title' => 'Task 3', 'status' => $s('todo'), 'order_position' => '3000.0000000000']); + }); + + describe('PositionRebalancer::needsRebalancing()', function () use ($s) { + test('detects gap below MIN_GAP', function () use ($s) { + Task::create(['title' => 'Close 1', 'status' => $s('in_progress'), 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Close 2', 'status' => $s('in_progress'), 'order_position' => '1000.00005']); // Gap < MIN_GAP + + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + (string) enum_value($s('in_progress')), + 'order_position' + ))->toBeTrue(); + }); + + test('returns false when gaps are healthy', function () use ($s) { + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + (string) enum_value($s('todo')), + 'order_position' + ))->toBeFalse(); + }); + + test('returns false for empty column', function () use ($s) { + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + (string) enum_value($s('done')), // No tasks in this column + 'order_position' + ))->toBeFalse(); + }); + + test('returns false for single item column', function () use ($s) { + Task::create(['title' => 'Alone', 'status' => $s('review'), 'order_position' => '1000.0000000000']); + + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + (string) enum_value($s('review')), + 'order_position' + ))->toBeFalse(); + }); + }); + + describe('PositionRebalancer::rebalanceColumn()', function () use ($s) { + test('redistributes positions evenly', function () use ($s) { + $rebalancer = new PositionRebalancer; + + $count = $rebalancer->rebalanceColumn( + Task::query(), + 'status', + (string) enum_value($s('todo')), + 'order_position' + ); + + expect($count)->toBe(3); + + $tasks = Task::where('status', $s('todo'))->orderBy('order_position')->get(); + + expect(DecimalPosition::normalize($tasks[0]->order_position))->toBe('65535.0000000000') + ->and(DecimalPosition::normalize($tasks[1]->order_position))->toBe('131070.0000000000') + ->and(DecimalPosition::normalize($tasks[2]->order_position))->toBe('196605.0000000000'); + }); + + test('maintains original order after rebalancing', function () use ($s) { + Task::create(['title' => 'A', 'status' => $s('testing'), 'order_position' => '100.0000000000']); + Task::create(['title' => 'B', 'status' => $s('testing'), 'order_position' => '100.0001000000']); + Task::create(['title' => 'C', 'status' => $s('testing'), 'order_position' => '100.0001500000']); + Task::create(['title' => 'D', 'status' => $s('testing'), 'order_position' => '100.0001600000']); + + $originalOrder = Task::where('status', $s('testing')) + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + $rebalancer = new PositionRebalancer; + $rebalancer->rebalanceColumn( + Task::query(), + 'status', + (string) enum_value($s('testing')), + 'order_position' + ); + + $newOrder = Task::where('status', $s('testing')) + ->orderBy('order_position') + ->pluck('title') + ->toArray(); + + expect($newOrder)->toBe($originalOrder); + }); + + test('returns zero for empty column', function () { + $rebalancer = new PositionRebalancer; + + $count = $rebalancer->rebalanceColumn( + Task::query(), + 'status', + 'nonexistent', + 'order_position' + ); + + expect($count)->toBe(0); + }); + }); + + describe('PositionRebalancer::findColumnsNeedingRebalancing()', function () use ($s) { + test('identifies columns with small gaps', function () use ($s) { + Task::create(['title' => 'Healthy 1', 'status' => $s('done'), 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Healthy 2', 'status' => $s('done'), 'order_position' => '2000.0000000000']); + + Task::create(['title' => 'Cramped 1', 'status' => $s('blocked'), 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Cramped 2', 'status' => $s('blocked'), 'order_position' => '1000.00005']); // Gap < MIN_GAP + + $rebalancer = new PositionRebalancer; + + $needsRebalancing = $rebalancer->findColumnsNeedingRebalancing( + Task::query(), + 'status', + 'order_position' + ); + + expect($needsRebalancing)->toContain('blocked') + ->and($needsRebalancing)->not->toContain('done') + ->and($needsRebalancing)->not->toContain('todo'); + }); + }); + + describe('PositionRebalancer::rebalanceAll()', function () use ($s) { + test('processes all columns needing rebalancing', function () use ($s) { + Task::create(['title' => 'Col1 A', 'status' => $s('blocked'), 'order_position' => '1000.0000000000']); + Task::create(['title' => 'Col1 B', 'status' => $s('blocked'), 'order_position' => '1000.00005']); + + Task::create(['title' => 'Col2 A', 'status' => $s('review'), 'order_position' => '2000.0000000000']); + Task::create(['title' => 'Col2 B', 'status' => $s('review'), 'order_position' => '2000.00003']); + + $rebalancer = new PositionRebalancer; + + $results = $rebalancer->rebalanceAll( + Task::query(), + 'status', + 'order_position' + ); + + expect($results)->toHaveKey('blocked') + ->and($results)->toHaveKey('review') + ->and($results['blocked'])->toBe(2) + ->and($results['review'])->toBe(2); + + expect($rebalancer->needsRebalancing(Task::query(), 'status', (string) enum_value($s('blocked')), 'order_position'))->toBeFalse() + ->and($rebalancer->needsRebalancing(Task::query(), 'status', (string) enum_value($s('review')), 'order_position'))->toBeFalse(); + }); + }); + + describe('PositionRebalancer::getGapStatistics()', function () use ($s) { + test('returns correct statistics for column', function () use ($s) { + $rebalancer = new PositionRebalancer; + + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + (string) enum_value($s('todo')), + 'order_position' + ); + + expect($stats['count'])->toBe(3) + ->and($stats['min_gap'])->toBe('1000.0000000000') + ->and($stats['max_gap'])->toBe('1000.0000000000') + ->and($stats['avg_gap'])->toBe('1000.0000000000') + ->and($stats['small_gaps'])->toBe(0); + }); + + test('returns nulls for single item column', function () use ($s) { + Task::create(['title' => 'Solo', 'status' => $s('solo_column'), 'order_position' => '1000.0000000000']); + + $rebalancer = new PositionRebalancer; + + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + 'solo_column', + 'order_position' + ); + + expect($stats['count'])->toBe(1) + ->and($stats['min_gap'])->toBeNull() + ->and($stats['max_gap'])->toBeNull() + ->and($stats['avg_gap'])->toBeNull() + ->and($stats['small_gaps'])->toBe(0); + }); + + test('counts small gaps correctly', function () use ($s) { + Task::create(['title' => 'A', 'status' => $s('cramped'), 'order_position' => '1000.0000000000']); + Task::create(['title' => 'B', 'status' => $s('cramped'), 'order_position' => '1000.00005']); // Small gap + Task::create(['title' => 'C', 'status' => $s('cramped'), 'order_position' => '2000.0000000000']); // Large gap + + $rebalancer = new PositionRebalancer; + + $stats = $rebalancer->getGapStatistics( + Task::query(), + 'status', + 'cramped', + 'order_position' + ); + + expect($stats['count'])->toBe(3) + ->and($stats['small_gaps'])->toBe(1); + }); + }); }); +} - test('returns false when gaps are healthy', function () { - $rebalancer = new PositionRebalancer; - - expect($rebalancer->needsRebalancing( - Task::query(), - 'status', - 'todo', - 'order_position' - ))->toBeFalse(); - }); - - test('returns false for empty column', function () { - $rebalancer = new PositionRebalancer; - - expect($rebalancer->needsRebalancing( - Task::query(), - 'status', - 'done', // No tasks in this column - 'order_position' - ))->toBeFalse(); - }); - - test('returns false for single item column', function () { - Task::create(['title' => 'Alone', 'status' => 'review', 'order_position' => '1000.0000000000']); - - $rebalancer = new PositionRebalancer; - - expect($rebalancer->needsRebalancing( - Task::query(), - 'status', - 'review', - 'order_position' - ))->toBeFalse(); - }); -}); - -describe('PositionRebalancer::rebalanceColumn()', function () { - test('redistributes positions evenly', function () { - $rebalancer = new PositionRebalancer; - - $count = $rebalancer->rebalanceColumn( - Task::query(), - 'status', - 'todo', - 'order_position' - ); - - expect($count)->toBe(3); - - // Verify positions are evenly spaced - $tasks = Task::where('status', 'todo')->orderBy('order_position')->get(); - - expect(DecimalPosition::normalize($tasks[0]->order_position))->toBe('65535.0000000000') - ->and(DecimalPosition::normalize($tasks[1]->order_position))->toBe('131070.0000000000') - ->and(DecimalPosition::normalize($tasks[2]->order_position))->toBe('196605.0000000000'); - }); - - test('maintains original order after rebalancing', function () { - // Create tasks with irregular positions - Task::create(['title' => 'A', 'status' => 'testing', 'order_position' => '100.0000000000']); - Task::create(['title' => 'B', 'status' => 'testing', 'order_position' => '100.0001000000']); - Task::create(['title' => 'C', 'status' => 'testing', 'order_position' => '100.0001500000']); - Task::create(['title' => 'D', 'status' => 'testing', 'order_position' => '100.0001600000']); - - // Get original order - $originalOrder = Task::where('status', 'testing') - ->orderBy('order_position') - ->pluck('title') - ->toArray(); - - // Rebalance - $rebalancer = new PositionRebalancer; - $rebalancer->rebalanceColumn(Task::query(), 'status', 'testing', 'order_position'); - - // Get new order - $newOrder = Task::where('status', 'testing') - ->orderBy('order_position') - ->pluck('title') - ->toArray(); - - expect($newOrder)->toBe($originalOrder); - }); - - test('returns zero for empty column', function () { - $rebalancer = new PositionRebalancer; - - $count = $rebalancer->rebalanceColumn( - Task::query(), - 'status', - 'nonexistent', - 'order_position' - ); - - expect($count)->toBe(0); - }); -}); - -describe('PositionRebalancer::findColumnsNeedingRebalancing()', function () { - test('identifies columns with small gaps', function () { - // Create column with healthy gaps - Task::create(['title' => 'Healthy 1', 'status' => 'done', 'order_position' => '1000.0000000000']); - Task::create(['title' => 'Healthy 2', 'status' => 'done', 'order_position' => '2000.0000000000']); - - // Create column with small gaps - Task::create(['title' => 'Cramped 1', 'status' => 'blocked', 'order_position' => '1000.0000000000']); - Task::create(['title' => 'Cramped 2', 'status' => 'blocked', 'order_position' => '1000.00005']); // Gap < MIN_GAP - - $rebalancer = new PositionRebalancer; - - $needsRebalancing = $rebalancer->findColumnsNeedingRebalancing( - Task::query(), - 'status', - 'order_position' - ); - - expect($needsRebalancing)->toContain('blocked') - ->and($needsRebalancing)->not->toContain('done') - ->and($needsRebalancing)->not->toContain('todo'); - }); -}); - -describe('PositionRebalancer::rebalanceAll()', function () { - test('processes all columns needing rebalancing', function () { - // Create multiple columns needing rebalancing - Task::create(['title' => 'Col1 A', 'status' => 'blocked', 'order_position' => '1000.0000000000']); - Task::create(['title' => 'Col1 B', 'status' => 'blocked', 'order_position' => '1000.00005']); - - Task::create(['title' => 'Col2 A', 'status' => 'review', 'order_position' => '2000.0000000000']); - Task::create(['title' => 'Col2 B', 'status' => 'review', 'order_position' => '2000.00003']); - - $rebalancer = new PositionRebalancer; - - $results = $rebalancer->rebalanceAll( - Task::query(), - 'status', - 'order_position' - ); - - expect($results)->toHaveKey('blocked') - ->and($results)->toHaveKey('review') - ->and($results['blocked'])->toBe(2) - ->and($results['review'])->toBe(2); - - // Verify gaps are now healthy - expect($rebalancer->needsRebalancing(Task::query(), 'status', 'blocked', 'order_position'))->toBeFalse() - ->and($rebalancer->needsRebalancing(Task::query(), 'status', 'review', 'order_position'))->toBeFalse(); - }); -}); - -describe('PositionRebalancer::getGapStatistics()', function () { - test('returns correct statistics for column', function () { - $rebalancer = new PositionRebalancer; - - $stats = $rebalancer->getGapStatistics( - Task::query(), - 'status', - 'todo', - 'order_position' - ); - - expect($stats['count'])->toBe(3) - ->and($stats['min_gap'])->toBe('1000.0000000000') - ->and($stats['max_gap'])->toBe('1000.0000000000') - ->and($stats['avg_gap'])->toBe('1000.0000000000') - ->and($stats['small_gaps'])->toBe(0); - }); - - test('returns nulls for single item column', function () { - Task::create(['title' => 'Solo', 'status' => 'solo_column', 'order_position' => '1000.0000000000']); - - $rebalancer = new PositionRebalancer; - - $stats = $rebalancer->getGapStatistics( - Task::query(), - 'status', - 'solo_column', - 'order_position' - ); - - expect($stats['count'])->toBe(1) - ->and($stats['min_gap'])->toBeNull() - ->and($stats['max_gap'])->toBeNull() - ->and($stats['avg_gap'])->toBeNull() - ->and($stats['small_gaps'])->toBe(0); - }); - - test('counts small gaps correctly', function () { - Task::create(['title' => 'A', 'status' => 'cramped', 'order_position' => '1000.0000000000']); - Task::create(['title' => 'B', 'status' => 'cramped', 'order_position' => '1000.00005']); // Small gap - Task::create(['title' => 'C', 'status' => 'cramped', 'order_position' => '2000.0000000000']); // Large gap - - $rebalancer = new PositionRebalancer; - - $stats = $rebalancer->getGapStatistics( - Task::query(), - 'status', - 'cramped', - 'order_position' - ); - - expect($stats['count'])->toBe(3) - ->and($stats['small_gaps'])->toBe(1); - }); -}); +runPositionRebalancingTests('with string statuses', fn (string $v) => $v); +runPositionRebalancingTests('with enum statuses', fn (string $v) => RebalancingStatusEnum::tryFrom($v) ?? $v); diff --git a/tests/Fixtures/RebalancingStatusEnum.php b/tests/Fixtures/RebalancingStatusEnum.php new file mode 100644 index 000000000..17e5d38f6 --- /dev/null +++ b/tests/Fixtures/RebalancingStatusEnum.php @@ -0,0 +1,15 @@ +