Skip to content

Commit da36a17

Browse files
ManukMinasyanManukMinasyan
committed
refactor: replace 'id' with model key name for consistent ordering in queries
1 parent 35ea240 commit da36a17

File tree

5 files changed

+28
-17
lines changed

5 files changed

+28
-17
lines changed

‎src/Commands/DiagnosePositionsCommand.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,15 @@ private function validateModelClass(string $value): ?string
138138
private function checkGaps(Model $model, string $columnField, string $positionField): array
139139
{
140140
$issues = [];
141+
$keyName = $model->getKeyName();
141142
$columns = $model->query()->distinct()->pluck($columnField)->map(fn ($value) => $value instanceof \BackedEnum ? $value->value : $value);
142143

143144
foreach ($columns as $column) {
144145
$positions = $model->query()
145146
->where($columnField, $column)
146147
->whereNotNull($positionField)
147148
->orderBy($positionField)
148-
->orderBy('id')
149+
->orderBy($keyName)
149150
->pluck($positionField);
150151

151152
if ($positions->count() < 2) {
@@ -190,13 +191,14 @@ private function checkGaps(Model $model, string $columnField, string $positionFi
190191
private function checkInversions(Model $model, string $columnField, string $positionField): array
191192
{
192193
$issues = [];
194+
$keyName = $model->getKeyName();
193195
$columns = $model->query()->distinct()->pluck($columnField)->map(fn ($value) => $value instanceof \BackedEnum ? $value->value : $value);
194196

195197
foreach ($columns as $column) {
196198
$records = $model->query()
197199
->where($columnField, $column)
198200
->whereNotNull($positionField)
199-
->orderBy('id')
201+
->orderBy($keyName)
200202
->get();
201203

202204
if ($records->count() < 2) {

‎src/Commands/RepairPositionsCommand.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,15 +278,16 @@ private function calculateChanges(string $model, string $columnField, string $po
278278
private function getRecordsForStrategy(string $model, string $columnField, string $positionField, mixed $group, string $strategy, $baseQuery = null): Collection
279279
{
280280
$query = $baseQuery ? (clone $baseQuery)->where($columnField, $group) : $model::where($columnField, $group);
281+
$keyName = $query->getModel()->getKeyName();
281282

282283
return match ($strategy) {
283-
'regenerate' => $query->orderBy('id')->get(),
284-
'fix_missing' => $query->whereNull($positionField)->orderBy('id')->get(),
284+
'regenerate' => $query->orderBy($keyName)->get(),
285+
'fix_missing' => $query->whereNull($positionField)->orderBy($keyName)->get(),
285286
'fix_duplicates' => $this->getDuplicateRecords($query, $positionField),
286287
'fix_all' => $query->where(function ($q) use ($positionField) {
287288
$q->whereNull($positionField)
288289
->orWhereIn($positionField, $this->getDuplicatePositions($q, $positionField));
289-
})->orderBy('id')->get(),
290+
})->orderBy($keyName)->get(),
290291
default => new Collection,
291292
};
292293
}
@@ -302,7 +303,9 @@ private function getDuplicateRecords(Builder $query, string $positionField): Col
302303
return new Collection;
303304
}
304305

305-
return $query->whereIn($positionField, $duplicatePositions)->orderBy('id')->get();
306+
$keyName = $query->getModel()->getKeyName();
307+
308+
return $query->whereIn($positionField, $duplicatePositions)->orderBy($keyName)->get();
306309
}
307310

308311
/**

‎src/Concerns/HasBoardRecords.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,11 @@ public function getBoardRecords(string $columnId): Collection
8282
}
8383

8484
$positionField = $this->getPositionIdentifierAttribute();
85+
$keyName = $queryClone->getModel()->getKeyName();
8586

8687
if ($positionField && $this->modelHasColumn($queryClone->getModel(), $positionField)) {
8788
$queryClone->orderBy($positionField, 'asc')
88-
->orderBy('id', 'asc'); // Tie-breaker for deterministic order
89+
->orderBy($keyName, 'asc'); // Tie-breaker for deterministic order
8990
}
9091

9192
return $queryClone->limit($limit)->get();
@@ -171,12 +172,13 @@ public function getRecordsBeforePosition(string $columnId, string $position, int
171172

172173
$statusField = $this->getColumnIdentifierAttribute();
173174
$positionField = $this->getPositionIdentifierAttribute();
175+
$keyName = $query->getModel()->getKeyName();
174176

175177
return (clone $query)
176178
->where($statusField, $columnId)
177179
->where($positionField, '<', $position)
178180
->orderBy($positionField, 'desc')
179-
->orderBy('id', 'desc') // Tie-breaker for deterministic order
181+
->orderBy($keyName, 'desc') // Tie-breaker for deterministic order
180182
->limit($limit)
181183
->get();
182184
}
@@ -194,12 +196,13 @@ public function getRecordsAfterPosition(string $columnId, string $position, int
194196

195197
$statusField = $this->getColumnIdentifierAttribute();
196198
$positionField = $this->getPositionIdentifierAttribute();
199+
$keyName = $query->getModel()->getKeyName();
197200

198201
return (clone $query)
199202
->where($statusField, $columnId)
200203
->where($positionField, '>', $position)
201204
->orderBy($positionField, 'asc')
202-
->orderBy('id', 'asc') // Tie-breaker for deterministic order
205+
->orderBy($keyName, 'asc') // Tie-breaker for deterministic order
203206
->limit($limit)
204207
->get();
205208
}
@@ -217,11 +220,12 @@ public function getLastPositionInColumn(string $columnId): ?string
217220

218221
$statusField = $this->getColumnIdentifierAttribute();
219222
$positionField = $this->getPositionIdentifierAttribute();
223+
$keyName = $query->getModel()->getKeyName();
220224

221225
$record = (clone $query)
222226
->where($statusField, $columnId)
223227
->orderBy($positionField, 'desc')
224-
->orderBy('id', 'desc') // Tie-breaker for deterministic order
228+
->orderBy($keyName, 'desc') // Tie-breaker for deterministic order
225229
->first();
226230

227231
return $record?->getAttribute($positionField);

‎src/Concerns/InteractsWithBoard.php‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,11 @@ protected function calculateAndUpdatePosition(
130130

131131
// LOCK reference cards for reading to prevent stale data
132132
$afterCard = $afterCardId
133-
? (clone $query)->where('id', $afterCardId)->lockForUpdate()->first()
133+
? (clone $query)->whereKey($afterCardId)->lockForUpdate()->first()
134134
: null;
135135

136136
$beforeCard = $beforeCardId
137-
? (clone $query)->where('id', $beforeCardId)->lockForUpdate()->first()
137+
? (clone $query)->whereKey($beforeCardId)->lockForUpdate()->first()
138138
: null;
139139

140140
// Get positions from locked cards
@@ -155,10 +155,10 @@ protected function calculateAndUpdatePosition(
155155

156156
// Recalculate position after rebalancing
157157
$afterCard = $afterCardId
158-
? (clone $query)->where('id', $afterCardId)->lockForUpdate()->first()
158+
? (clone $query)->whereKey($afterCardId)->lockForUpdate()->first()
159159
: null;
160160
$beforeCard = $beforeCardId
161-
? (clone $query)->where('id', $beforeCardId)->lockForUpdate()->first()
161+
? (clone $query)->whereKey($beforeCardId)->lockForUpdate()->first()
162162
: null;
163163

164164
$afterPos = $afterCard?->getAttribute($positionField);
@@ -512,14 +512,15 @@ public function getBoardPositionInColumn(string $columnId, string $position = 't
512512
$board = $this->getBoard();
513513
$statusField = $board->getColumnIdentifierAttribute();
514514
$positionField = $board->getPositionIdentifierAttribute();
515+
$keyName = $query->getModel()->getKeyName();
515516
$queryClone = (clone $query)->where($statusField, $columnId);
516517

517518
if ($position === 'top') {
518519
// Get first valid position (ignore null positions)
519520
$firstRecord = $queryClone
520521
->whereNotNull($positionField)
521522
->orderBy($positionField, 'asc')
522-
->orderBy('id', 'asc') // Tie-breaker for deterministic order
523+
->orderBy($keyName, 'asc') // Tie-breaker for deterministic order
523524
->first();
524525

525526
if ($firstRecord) {
@@ -536,7 +537,7 @@ public function getBoardPositionInColumn(string $columnId, string $position = 't
536537
$lastRecord = $queryClone
537538
->whereNotNull($positionField)
538539
->orderBy($positionField, 'desc')
539-
->orderBy('id', 'desc') // Tie-breaker for deterministic order
540+
->orderBy($keyName, 'desc') // Tie-breaker for deterministic order
540541
->first();
541542

542543
if ($lastRecord) {

‎src/Services/PositionRebalancer.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,12 @@ public function rebalanceColumn(
3333
string $columnId,
3434
string $positionField
3535
): int {
36+
$keyName = $query->getModel()->getKeyName();
3637
$records = (clone $query)
3738
->where($columnField, $columnId)
3839
->whereNotNull($positionField)
3940
->orderBy($positionField)
40-
->orderBy('id') // Tie-breaker for deterministic order
41+
->orderBy($keyName) // Tie-breaker for deterministic order
4142
->get();
4243

4344
if ($records->isEmpty()) {

0 commit comments

Comments
 (0)