From 2b90ef17b7bbe66c889461d9160a84b57c420120 Mon Sep 17 00:00:00 2001 From: Hugh Messenger Date: Wed, 15 Apr 2026 19:04:12 -0500 Subject: [PATCH 1/8] Move convertEnumToString method to helper, fix rebalancing to use it --- src/Commands/RebalancePositionsCommand.php | 6 +++-- src/Commands/RepairPositionsCommand.php | 28 +++------------------- src/Services/PositionRebalancer.php | 9 ++++--- src/Support/EnumHelper.php | 28 ++++++++++++++++++++++ 4 files changed, 41 insertions(+), 30 deletions(-) create mode 100644 src/Support/EnumHelper.php diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php index c5ec68257..e70ea3b9a 100644 --- a/src/Commands/RebalancePositionsCommand.php +++ b/src/Commands/RebalancePositionsCommand.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Model; use Relaticle\Flowforge\Services\PositionRebalancer; +use Relaticle\Flowforge\Support\EnumHelper; use function Laravel\Prompts\confirm; use function Laravel\Prompts\error; use function Laravel\Prompts\info; @@ -162,8 +163,9 @@ private function rebalanceAllNeeded( $this->newLine(); foreach ($columnsNeedingRebalancing as $columnId) { - $stats = $rebalancer->getGapStatistics($query, $columnField, (string) $columnId, $positionField); - $this->line(" - {$columnId}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); + $columnValue = EnumHelper::convertEnumToString($columnId); + $stats = $rebalancer->getGapStatistics($query, $columnField, $columnValue, $positionField); + $this->line(" - {$columnValue}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); } $this->newLine(); diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index 0114d4dd3..6d116bc17 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 Relaticle\Flowforge\Support\EnumHelper; 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} @@ -212,7 +190,7 @@ private function analyzePositions(string $model, string $columnField, string $po ->pluck('record_count', $columnField) ->mapWithKeys(function ($count, $key) { // Convert enum to string value if needed - $stringKey = $this->convertEnumToString($key); + $stringKey = EnumHelper::convertEnumToString($key); return [$stringKey => $count]; }) @@ -260,7 +238,7 @@ private function calculateChanges(string $model, string $columnField, string $po foreach ($groups as $group) { // Convert enum to string for array key - $groupKey = $this->convertEnumToString($group); + $groupKey = EnumHelper::convertEnumToString($group); $records = $this->getRecordsForStrategy($model, $columnField, $positionField, $group, $strategy, $query); diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php index 83aa18b0e..77f4daf2c 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -9,6 +9,7 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use Relaticle\Flowforge\Support\EnumHelper; /** * Handles rebalancing of positions within a column when gaps become too small. @@ -104,7 +105,7 @@ public function findColumnsNeedingRebalancing( ->pluck($columnField); return $columns->filter(function ($columnId) use ($query, $columnField, $positionField) { - return $this->needsRebalancing($query, $columnField, (string) $columnId, $positionField); + return $this->needsRebalancing($query, $columnField, EnumHelper::convertEnumToString($columnId), $positionField); })->values(); } @@ -130,10 +131,12 @@ public function rebalanceAll( ); foreach ($columnsNeedingRebalancing as $columnId) { - $results[(string) $columnId] = $this->rebalanceColumn( + $columnValue = EnumHelper::convertEnumToString($columnId); + + $results[$columnValue] = $this->rebalanceColumn( $query, $columnField, - (string) $columnId, + $columnValue, $positionField ); } diff --git a/src/Support/EnumHelper.php b/src/Support/EnumHelper.php new file mode 100644 index 000000000..7ee076717 --- /dev/null +++ b/src/Support/EnumHelper.php @@ -0,0 +1,28 @@ +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; + } +} \ No newline at end of file From 5d907a60559661ed9a67c603dee2ff26a5d04ea4 Mon Sep 17 00:00:00 2001 From: cheesegrits <934456+cheesegrits@users.noreply.github.com> Date: Thu, 16 Apr 2026 00:04:37 +0000 Subject: [PATCH 2/8] Fix styling --- src/Commands/RebalancePositionsCommand.php | 2 +- src/Commands/RepairPositionsCommand.php | 2 +- src/Services/PositionRebalancer.php | 2 +- src/Support/EnumHelper.php | 5 +++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php index e70ea3b9a..4fb92955b 100644 --- a/src/Commands/RebalancePositionsCommand.php +++ b/src/Commands/RebalancePositionsCommand.php @@ -5,8 +5,8 @@ use Illuminate\Console\Command; use Illuminate\Database\Eloquent\Model; use Relaticle\Flowforge\Services\PositionRebalancer; - use Relaticle\Flowforge\Support\EnumHelper; + use function Laravel\Prompts\confirm; use function Laravel\Prompts\error; use function Laravel\Prompts\info; diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index 6d116bc17..99abb0d08 100644 --- a/src/Commands/RepairPositionsCommand.php +++ b/src/Commands/RepairPositionsCommand.php @@ -8,8 +8,8 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; use Relaticle\Flowforge\Services\DecimalPosition; - use Relaticle\Flowforge\Support\EnumHelper; + use function Laravel\Prompts\confirm; use function Laravel\Prompts\info; use function Laravel\Prompts\select; diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php index 77f4daf2c..271572f5f 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -132,7 +132,7 @@ public function rebalanceAll( foreach ($columnsNeedingRebalancing as $columnId) { $columnValue = EnumHelper::convertEnumToString($columnId); - + $results[$columnValue] = $this->rebalanceColumn( $query, $columnField, diff --git a/src/Support/EnumHelper.php b/src/Support/EnumHelper.php index 7ee076717..ec6ba6016 100644 --- a/src/Support/EnumHelper.php +++ b/src/Support/EnumHelper.php @@ -2,7 +2,8 @@ namespace Relaticle\Flowforge\Support; -class EnumHelper { +class EnumHelper +{ public static function convertEnumToString($value): string { if (is_object($value)) { @@ -25,4 +26,4 @@ public static function convertEnumToString($value): string return (string) $value; } -} \ No newline at end of file +} From dcde8fd2fb32a5deeca358a2ed39b3f92c815c2f Mon Sep 17 00:00:00 2001 From: Hugh Messenger Date: Mon, 20 Apr 2026 14:08:16 -0500 Subject: [PATCH 3/8] Move convertEnumToString method to helper, fix rebalancing to use it, add enums to test --- tests/Feature/PositionRebalancingTest.php | 455 +++++++++++----------- tests/Fixtures/StatusEnum.php | 15 + tests/Unit/EnumHelperTest.php | 49 +++ 3 files changed, 294 insertions(+), 225 deletions(-) create mode 100644 tests/Fixtures/StatusEnum.php create mode 100644 tests/Unit/EnumHelperTest.php diff --git a/tests/Feature/PositionRebalancingTest.php b/tests/Feature/PositionRebalancingTest.php index b0b4bb0e4..4369587e4 100644 --- a/tests/Feature/PositionRebalancingTest.php +++ b/tests/Feature/PositionRebalancingTest.php @@ -4,232 +4,237 @@ use Relaticle\Flowforge\Services\DecimalPosition; use Relaticle\Flowforge\Services\PositionRebalancer; +use Relaticle\Flowforge\Support\EnumHelper; +use Relaticle\Flowforge\Tests\Fixtures\StatusEnum; 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(); +function runPositionRebalancingTests(string $label, callable $s): void +{ + describe($label, function () use ($s) { + // $s is a callback that either converts a string to a StatusEnum, 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', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($s('todo')), + 'order_position' + ))->toBeFalse(); + }); + + test('returns false for empty column', function () use ($s) { + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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', EnumHelper::convertEnumToString($s('blocked')), 'order_position'))->toBeFalse() + ->and($rebalancer->needsRebalancing(Task::query(), 'status', EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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) => StatusEnum::tryFrom($v) ?? $v); diff --git a/tests/Fixtures/StatusEnum.php b/tests/Fixtures/StatusEnum.php new file mode 100644 index 000000000..96d677f56 --- /dev/null +++ b/tests/Fixtures/StatusEnum.php @@ -0,0 +1,15 @@ +toBe('todo') + ->and(EnumHelper::convertEnumToString(StatusEnum::InProgress))->toBe('in_progress'); + }); + + test('returns string unchanged', function () { + expect(EnumHelper::convertEnumToString('todo'))->toBe('todo') + ->and(EnumHelper::convertEnumToString('anything'))->toBe('anything'); + }); + + test('casts integer to string', function () { + expect(EnumHelper::convertEnumToString(42))->toBe('42'); + }); + + test('falls back to class basename for unknown objects', function () { + $obj = new class {}; + + expect(EnumHelper::convertEnumToString($obj))->toBeString()->not->toBeEmpty(); + }); + + test('returns value from object with value property', function () { + $obj = new class + { + public string $value = 'from_value_property'; + }; + + expect(EnumHelper::convertEnumToString($obj))->toBe('from_value_property'); + }); + + test('returns result of __toString for stringable objects', function () { + $obj = new class + { + public function __toString(): string + { + return 'stringable_result'; + } + }; + + expect(EnumHelper::convertEnumToString($obj))->toBe('stringable_result'); + }); +}); From f739eae1169a722e9f3c9ebccc251d3554744af3 Mon Sep 17 00:00:00 2001 From: Hugh Messenger Date: Wed, 15 Apr 2026 19:04:12 -0500 Subject: [PATCH 4/8] Move convertEnumToString method to helper, fix rebalancing to use it --- src/Commands/RebalancePositionsCommand.php | 6 +++-- src/Commands/RepairPositionsCommand.php | 28 +++------------------- src/Services/PositionRebalancer.php | 9 ++++--- src/Support/EnumHelper.php | 28 ++++++++++++++++++++++ 4 files changed, 41 insertions(+), 30 deletions(-) create mode 100644 src/Support/EnumHelper.php diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php index c5ec68257..e70ea3b9a 100644 --- a/src/Commands/RebalancePositionsCommand.php +++ b/src/Commands/RebalancePositionsCommand.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Model; use Relaticle\Flowforge\Services\PositionRebalancer; +use Relaticle\Flowforge\Support\EnumHelper; use function Laravel\Prompts\confirm; use function Laravel\Prompts\error; use function Laravel\Prompts\info; @@ -162,8 +163,9 @@ private function rebalanceAllNeeded( $this->newLine(); foreach ($columnsNeedingRebalancing as $columnId) { - $stats = $rebalancer->getGapStatistics($query, $columnField, (string) $columnId, $positionField); - $this->line(" - {$columnId}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); + $columnValue = EnumHelper::convertEnumToString($columnId); + $stats = $rebalancer->getGapStatistics($query, $columnField, $columnValue, $positionField); + $this->line(" - {$columnValue}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); } $this->newLine(); diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index 0114d4dd3..6d116bc17 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 Relaticle\Flowforge\Support\EnumHelper; 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} @@ -212,7 +190,7 @@ private function analyzePositions(string $model, string $columnField, string $po ->pluck('record_count', $columnField) ->mapWithKeys(function ($count, $key) { // Convert enum to string value if needed - $stringKey = $this->convertEnumToString($key); + $stringKey = EnumHelper::convertEnumToString($key); return [$stringKey => $count]; }) @@ -260,7 +238,7 @@ private function calculateChanges(string $model, string $columnField, string $po foreach ($groups as $group) { // Convert enum to string for array key - $groupKey = $this->convertEnumToString($group); + $groupKey = EnumHelper::convertEnumToString($group); $records = $this->getRecordsForStrategy($model, $columnField, $positionField, $group, $strategy, $query); diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php index 83aa18b0e..77f4daf2c 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -9,6 +9,7 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; +use Relaticle\Flowforge\Support\EnumHelper; /** * Handles rebalancing of positions within a column when gaps become too small. @@ -104,7 +105,7 @@ public function findColumnsNeedingRebalancing( ->pluck($columnField); return $columns->filter(function ($columnId) use ($query, $columnField, $positionField) { - return $this->needsRebalancing($query, $columnField, (string) $columnId, $positionField); + return $this->needsRebalancing($query, $columnField, EnumHelper::convertEnumToString($columnId), $positionField); })->values(); } @@ -130,10 +131,12 @@ public function rebalanceAll( ); foreach ($columnsNeedingRebalancing as $columnId) { - $results[(string) $columnId] = $this->rebalanceColumn( + $columnValue = EnumHelper::convertEnumToString($columnId); + + $results[$columnValue] = $this->rebalanceColumn( $query, $columnField, - (string) $columnId, + $columnValue, $positionField ); } diff --git a/src/Support/EnumHelper.php b/src/Support/EnumHelper.php new file mode 100644 index 000000000..7ee076717 --- /dev/null +++ b/src/Support/EnumHelper.php @@ -0,0 +1,28 @@ +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; + } +} \ No newline at end of file From 7f2b211e95ad1dd321bbb231227fbec3c0e6a9c3 Mon Sep 17 00:00:00 2001 From: cheesegrits <934456+cheesegrits@users.noreply.github.com> Date: Thu, 16 Apr 2026 00:04:37 +0000 Subject: [PATCH 5/8] Fix styling --- src/Commands/RebalancePositionsCommand.php | 2 +- src/Commands/RepairPositionsCommand.php | 2 +- src/Services/PositionRebalancer.php | 2 +- src/Support/EnumHelper.php | 5 +++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php index e70ea3b9a..4fb92955b 100644 --- a/src/Commands/RebalancePositionsCommand.php +++ b/src/Commands/RebalancePositionsCommand.php @@ -5,8 +5,8 @@ use Illuminate\Console\Command; use Illuminate\Database\Eloquent\Model; use Relaticle\Flowforge\Services\PositionRebalancer; - use Relaticle\Flowforge\Support\EnumHelper; + use function Laravel\Prompts\confirm; use function Laravel\Prompts\error; use function Laravel\Prompts\info; diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index 6d116bc17..99abb0d08 100644 --- a/src/Commands/RepairPositionsCommand.php +++ b/src/Commands/RepairPositionsCommand.php @@ -8,8 +8,8 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; use Relaticle\Flowforge\Services\DecimalPosition; - use Relaticle\Flowforge\Support\EnumHelper; + use function Laravel\Prompts\confirm; use function Laravel\Prompts\info; use function Laravel\Prompts\select; diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php index 77f4daf2c..271572f5f 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -132,7 +132,7 @@ public function rebalanceAll( foreach ($columnsNeedingRebalancing as $columnId) { $columnValue = EnumHelper::convertEnumToString($columnId); - + $results[$columnValue] = $this->rebalanceColumn( $query, $columnField, diff --git a/src/Support/EnumHelper.php b/src/Support/EnumHelper.php index 7ee076717..ec6ba6016 100644 --- a/src/Support/EnumHelper.php +++ b/src/Support/EnumHelper.php @@ -2,7 +2,8 @@ namespace Relaticle\Flowforge\Support; -class EnumHelper { +class EnumHelper +{ public static function convertEnumToString($value): string { if (is_object($value)) { @@ -25,4 +26,4 @@ public static function convertEnumToString($value): string return (string) $value; } -} \ No newline at end of file +} From 79da6d28ef7ab8beca9c0824167f5f3277ba7381 Mon Sep 17 00:00:00 2001 From: Hugh Messenger Date: Mon, 20 Apr 2026 14:08:16 -0500 Subject: [PATCH 6/8] Move convertEnumToString method to helper, fix rebalancing to use it, add enums to test --- tests/Feature/PositionRebalancingTest.php | 455 +++++++++++----------- tests/Fixtures/StatusEnum.php | 15 + tests/Unit/EnumHelperTest.php | 49 +++ 3 files changed, 294 insertions(+), 225 deletions(-) create mode 100644 tests/Fixtures/StatusEnum.php create mode 100644 tests/Unit/EnumHelperTest.php diff --git a/tests/Feature/PositionRebalancingTest.php b/tests/Feature/PositionRebalancingTest.php index b0b4bb0e4..4369587e4 100644 --- a/tests/Feature/PositionRebalancingTest.php +++ b/tests/Feature/PositionRebalancingTest.php @@ -4,232 +4,237 @@ use Relaticle\Flowforge\Services\DecimalPosition; use Relaticle\Flowforge\Services\PositionRebalancer; +use Relaticle\Flowforge\Support\EnumHelper; +use Relaticle\Flowforge\Tests\Fixtures\StatusEnum; 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(); +function runPositionRebalancingTests(string $label, callable $s): void +{ + describe($label, function () use ($s) { + // $s is a callback that either converts a string to a StatusEnum, 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', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($s('todo')), + 'order_position' + ))->toBeFalse(); + }); + + test('returns false for empty column', function () use ($s) { + $rebalancer = new PositionRebalancer; + + expect($rebalancer->needsRebalancing( + Task::query(), + 'status', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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', EnumHelper::convertEnumToString($s('blocked')), 'order_position'))->toBeFalse() + ->and($rebalancer->needsRebalancing(Task::query(), 'status', EnumHelper::convertEnumToString($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', + EnumHelper::convertEnumToString($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) => StatusEnum::tryFrom($v) ?? $v); diff --git a/tests/Fixtures/StatusEnum.php b/tests/Fixtures/StatusEnum.php new file mode 100644 index 000000000..96d677f56 --- /dev/null +++ b/tests/Fixtures/StatusEnum.php @@ -0,0 +1,15 @@ +toBe('todo') + ->and(EnumHelper::convertEnumToString(StatusEnum::InProgress))->toBe('in_progress'); + }); + + test('returns string unchanged', function () { + expect(EnumHelper::convertEnumToString('todo'))->toBe('todo') + ->and(EnumHelper::convertEnumToString('anything'))->toBe('anything'); + }); + + test('casts integer to string', function () { + expect(EnumHelper::convertEnumToString(42))->toBe('42'); + }); + + test('falls back to class basename for unknown objects', function () { + $obj = new class {}; + + expect(EnumHelper::convertEnumToString($obj))->toBeString()->not->toBeEmpty(); + }); + + test('returns value from object with value property', function () { + $obj = new class + { + public string $value = 'from_value_property'; + }; + + expect(EnumHelper::convertEnumToString($obj))->toBe('from_value_property'); + }); + + test('returns result of __toString for stringable objects', function () { + $obj = new class + { + public function __toString(): string + { + return 'stringable_result'; + } + }; + + expect(EnumHelper::convertEnumToString($obj))->toBe('stringable_result'); + }); +}); From bc2aaab23246b2118a2c5976e344c10045edd8f8 Mon Sep 17 00:00:00 2001 From: Manuk Date: Sat, 25 Apr 2026 13:43:11 +0400 Subject: [PATCH 7/8] refactor: tighten EnumHelper and dedupe rebalancer enum conversion - type EnumHelper::convertEnumToString param as mixed, mark class final, add strict_types - split BackedEnum / UnitEnum branches, throw InvalidArgumentException on unknown objects - convert column ids once inside findColumnsNeedingRebalancing so callers don't re-convert - update unit test to assert the throw --- src/Commands/RebalancePositionsCommand.php | 6 ++--- src/Commands/RepairPositionsCommand.php | 2 -- src/Services/PositionRebalancer.php | 13 +++++---- src/Support/EnumHelper.php | 31 +++++++++++++++------- tests/Unit/EnumHelperTest.php | 6 ++--- 5 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/Commands/RebalancePositionsCommand.php b/src/Commands/RebalancePositionsCommand.php index 4fb92955b..49a6808d6 100644 --- a/src/Commands/RebalancePositionsCommand.php +++ b/src/Commands/RebalancePositionsCommand.php @@ -5,7 +5,6 @@ use Illuminate\Console\Command; use Illuminate\Database\Eloquent\Model; use Relaticle\Flowforge\Services\PositionRebalancer; -use Relaticle\Flowforge\Support\EnumHelper; use function Laravel\Prompts\confirm; use function Laravel\Prompts\error; @@ -163,9 +162,8 @@ private function rebalanceAllNeeded( $this->newLine(); foreach ($columnsNeedingRebalancing as $columnId) { - $columnValue = EnumHelper::convertEnumToString($columnId); - $stats = $rebalancer->getGapStatistics($query, $columnField, $columnValue, $positionField); - $this->line(" - {$columnValue}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); + $stats = $rebalancer->getGapStatistics($query, $columnField, $columnId, $positionField); + $this->line(" - {$columnId}: {$stats['count']} records, {$stats['small_gaps']} small gaps"); } $this->newLine(); diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index 99abb0d08..b2a90f3f5 100644 --- a/src/Commands/RepairPositionsCommand.php +++ b/src/Commands/RepairPositionsCommand.php @@ -189,7 +189,6 @@ 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 = EnumHelper::convertEnumToString($key); return [$stringKey => $count]; @@ -237,7 +236,6 @@ private function calculateChanges(string $model, string $columnField, string $po ->pluck($columnField); foreach ($groups as $group) { - // Convert enum to string for array key $groupKey = EnumHelper::convertEnumToString($group); $records = $this->getRecordsForStrategy($model, $columnField, $positionField, $group, $strategy, $query); diff --git a/src/Services/PositionRebalancer.php b/src/Services/PositionRebalancer.php index 271572f5f..3a7a75731 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -102,10 +102,11 @@ public function findColumnsNeedingRebalancing( $columns = (clone $query) ->select($columnField) ->distinct() - ->pluck($columnField); + ->pluck($columnField) + ->map(fn ($columnId) => EnumHelper::convertEnumToString($columnId)); - return $columns->filter(function ($columnId) use ($query, $columnField, $positionField) { - return $this->needsRebalancing($query, $columnField, EnumHelper::convertEnumToString($columnId), $positionField); + return $columns->filter(function (string $columnId) use ($query, $columnField, $positionField) { + return $this->needsRebalancing($query, $columnField, $columnId, $positionField); })->values(); } @@ -131,12 +132,10 @@ public function rebalanceAll( ); foreach ($columnsNeedingRebalancing as $columnId) { - $columnValue = EnumHelper::convertEnumToString($columnId); - - $results[$columnValue] = $this->rebalanceColumn( + $results[$columnId] = $this->rebalanceColumn( $query, $columnField, - $columnValue, + $columnId, $positionField ); } diff --git a/src/Support/EnumHelper.php b/src/Support/EnumHelper.php index ec6ba6016..34996c9d2 100644 --- a/src/Support/EnumHelper.php +++ b/src/Support/EnumHelper.php @@ -1,27 +1,38 @@ value; + } + + if ($value instanceof UnitEnum) { + return $value->name; + } + 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); + throw new InvalidArgumentException(sprintf( + 'Cannot convert object of type %s to a column identifier string.', + $value::class, + )); } return (string) $value; diff --git a/tests/Unit/EnumHelperTest.php b/tests/Unit/EnumHelperTest.php index 443bca9fd..2c4bb4f6c 100644 --- a/tests/Unit/EnumHelperTest.php +++ b/tests/Unit/EnumHelperTest.php @@ -20,11 +20,11 @@ expect(EnumHelper::convertEnumToString(42))->toBe('42'); }); - test('falls back to class basename for unknown objects', function () { + test('throws for unknown objects', function () { $obj = new class {}; - expect(EnumHelper::convertEnumToString($obj))->toBeString()->not->toBeEmpty(); - }); + EnumHelper::convertEnumToString($obj); + })->throws(InvalidArgumentException::class); test('returns value from object with value property', function () { $obj = new class From 9c388f47f95c8c1898d3858ff2565885135abdaa Mon Sep 17 00:00:00 2001 From: Manuk Date: Sat, 25 Apr 2026 13:50:59 +0400 Subject: [PATCH 8/8] refactor: drop EnumHelper, use Laravel's enum_value() directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Laravel ships Illuminate\Support\enum_value() which handles BackedEnum, UnitEnum, and scalar passthrough — exactly what the helper wrapped. Cast its mixed return to string at the callsite. --- src/Commands/RepairPositionsCommand.php | 6 +-- src/Services/PositionRebalancer.php | 5 ++- src/Support/EnumHelper.php | 40 ------------------ tests/Feature/PositionRebalancingTest.php | 21 +++++----- tests/Unit/EnumHelperTest.php | 49 ----------------------- 5 files changed, 17 insertions(+), 104 deletions(-) delete mode 100644 src/Support/EnumHelper.php delete mode 100644 tests/Unit/EnumHelperTest.php diff --git a/src/Commands/RepairPositionsCommand.php b/src/Commands/RepairPositionsCommand.php index b2a90f3f5..45e7feed6 100644 --- a/src/Commands/RepairPositionsCommand.php +++ b/src/Commands/RepairPositionsCommand.php @@ -8,8 +8,8 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\DB; use Relaticle\Flowforge\Services\DecimalPosition; -use Relaticle\Flowforge\Support\EnumHelper; +use function Illuminate\Support\enum_value; use function Laravel\Prompts\confirm; use function Laravel\Prompts\info; use function Laravel\Prompts\select; @@ -189,7 +189,7 @@ private function analyzePositions(string $model, string $columnField, string $po ->groupBy($columnField) ->pluck('record_count', $columnField) ->mapWithKeys(function ($count, $key) { - $stringKey = EnumHelper::convertEnumToString($key); + $stringKey = (string) enum_value($key); return [$stringKey => $count]; }) @@ -236,7 +236,7 @@ private function calculateChanges(string $model, string $columnField, string $po ->pluck($columnField); foreach ($groups as $group) { - $groupKey = EnumHelper::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 3a7a75731..949f6b782 100644 --- a/src/Services/PositionRebalancer.php +++ b/src/Services/PositionRebalancer.php @@ -9,7 +9,8 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; -use Relaticle\Flowforge\Support\EnumHelper; + +use function Illuminate\Support\enum_value; /** * Handles rebalancing of positions within a column when gaps become too small. @@ -103,7 +104,7 @@ public function findColumnsNeedingRebalancing( ->select($columnField) ->distinct() ->pluck($columnField) - ->map(fn ($columnId) => EnumHelper::convertEnumToString($columnId)); + ->map(fn ($columnId) => (string) enum_value($columnId)); return $columns->filter(function (string $columnId) use ($query, $columnField, $positionField) { return $this->needsRebalancing($query, $columnField, $columnId, $positionField); diff --git a/src/Support/EnumHelper.php b/src/Support/EnumHelper.php deleted file mode 100644 index 34996c9d2..000000000 --- a/src/Support/EnumHelper.php +++ /dev/null @@ -1,40 +0,0 @@ -value; - } - - if ($value instanceof UnitEnum) { - return $value->name; - } - - if (is_object($value)) { - if (property_exists($value, 'value')) { - return (string) $value->value; - } - - if (method_exists($value, '__toString')) { - return (string) $value; - } - - throw new InvalidArgumentException(sprintf( - 'Cannot convert object of type %s to a column identifier string.', - $value::class, - )); - } - - return (string) $value; - } -} diff --git a/tests/Feature/PositionRebalancingTest.php b/tests/Feature/PositionRebalancingTest.php index 6b5ce8177..fef44468e 100644 --- a/tests/Feature/PositionRebalancingTest.php +++ b/tests/Feature/PositionRebalancingTest.php @@ -4,10 +4,11 @@ use Relaticle\Flowforge\Services\DecimalPosition; use Relaticle\Flowforge\Services\PositionRebalancer; -use Relaticle\Flowforge\Support\EnumHelper; use Relaticle\Flowforge\Tests\Fixtures\RebalancingStatusEnum; use Relaticle\Flowforge\Tests\Fixtures\Task; +use function Illuminate\Support\enum_value; + function runPositionRebalancingTests(string $label, callable $s): void { describe($label, function () use ($s) { @@ -28,7 +29,7 @@ function runPositionRebalancingTests(string $label, callable $s): void expect($rebalancer->needsRebalancing( Task::query(), 'status', - EnumHelper::convertEnumToString($s('in_progress')), + (string) enum_value($s('in_progress')), 'order_position' ))->toBeTrue(); }); @@ -39,7 +40,7 @@ function runPositionRebalancingTests(string $label, callable $s): void expect($rebalancer->needsRebalancing( Task::query(), 'status', - EnumHelper::convertEnumToString($s('todo')), + (string) enum_value($s('todo')), 'order_position' ))->toBeFalse(); }); @@ -50,7 +51,7 @@ function runPositionRebalancingTests(string $label, callable $s): void expect($rebalancer->needsRebalancing( Task::query(), 'status', - EnumHelper::convertEnumToString($s('done')), // No tasks in this column + (string) enum_value($s('done')), // No tasks in this column 'order_position' ))->toBeFalse(); }); @@ -63,7 +64,7 @@ function runPositionRebalancingTests(string $label, callable $s): void expect($rebalancer->needsRebalancing( Task::query(), 'status', - EnumHelper::convertEnumToString($s('review')), + (string) enum_value($s('review')), 'order_position' ))->toBeFalse(); }); @@ -76,7 +77,7 @@ function runPositionRebalancingTests(string $label, callable $s): void $count = $rebalancer->rebalanceColumn( Task::query(), 'status', - EnumHelper::convertEnumToString($s('todo')), + (string) enum_value($s('todo')), 'order_position' ); @@ -104,7 +105,7 @@ function runPositionRebalancingTests(string $label, callable $s): void $rebalancer->rebalanceColumn( Task::query(), 'status', - EnumHelper::convertEnumToString($s('testing')), + (string) enum_value($s('testing')), 'order_position' ); @@ -173,8 +174,8 @@ function runPositionRebalancingTests(string $label, callable $s): void ->and($results['blocked'])->toBe(2) ->and($results['review'])->toBe(2); - expect($rebalancer->needsRebalancing(Task::query(), 'status', EnumHelper::convertEnumToString($s('blocked')), 'order_position'))->toBeFalse() - ->and($rebalancer->needsRebalancing(Task::query(), 'status', EnumHelper::convertEnumToString($s('review')), 'order_position'))->toBeFalse(); + 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(); }); }); @@ -185,7 +186,7 @@ function runPositionRebalancingTests(string $label, callable $s): void $stats = $rebalancer->getGapStatistics( Task::query(), 'status', - EnumHelper::convertEnumToString($s('todo')), + (string) enum_value($s('todo')), 'order_position' ); diff --git a/tests/Unit/EnumHelperTest.php b/tests/Unit/EnumHelperTest.php deleted file mode 100644 index 32c7d88fd..000000000 --- a/tests/Unit/EnumHelperTest.php +++ /dev/null @@ -1,49 +0,0 @@ -toBe('todo') - ->and(EnumHelper::convertEnumToString(RebalancingStatusEnum::InProgress))->toBe('in_progress'); - }); - - test('returns string unchanged', function () { - expect(EnumHelper::convertEnumToString('todo'))->toBe('todo') - ->and(EnumHelper::convertEnumToString('anything'))->toBe('anything'); - }); - - test('casts integer to string', function () { - expect(EnumHelper::convertEnumToString(42))->toBe('42'); - }); - - test('throws for unknown objects', function () { - $obj = new class {}; - - EnumHelper::convertEnumToString($obj); - })->throws(InvalidArgumentException::class); - - test('returns value from object with value property', function () { - $obj = new class - { - public string $value = 'from_value_property'; - }; - - expect(EnumHelper::convertEnumToString($obj))->toBe('from_value_property'); - }); - - test('returns result of __toString for stringable objects', function () { - $obj = new class - { - public function __toString(): string - { - return 'stringable_result'; - } - }; - - expect(EnumHelper::convertEnumToString($obj))->toBe('stringable_result'); - }); -});