diff --git a/app/Livewire/Books/BookIndex.php b/app/Livewire/Books/BookIndex.php index e23c918..7ebdd09 100644 --- a/app/Livewire/Books/BookIndex.php +++ b/app/Livewire/Books/BookIndex.php @@ -89,14 +89,19 @@ public function updateStatus(Book $book, string $status): void 'date_finished' => $status === 'read' && ! $book->date_finished ? now() : $book->date_finished, ]); - // Auto-remove from queue when marked as read + // Auto-remove from queue when marked as read. Queue reordering is + // organizational, so it must not bump updated_at — otherwise every + // shifted book jumps to the top of the "Recently Updated" sort. if ($status === 'read' && $book->queue_position !== null) { $oldPosition = $book->queue_position; - $book->update(['queue_position' => null]); - Book::where('user_id', Auth::id()) - ->where('queue_position', '>', $oldPosition) - ->decrement('queue_position'); + Book::withoutTimestamps(function () use ($book, $oldPosition) { + $book->update(['queue_position' => null]); + + Book::where('user_id', Auth::id()) + ->where('queue_position', '>', $oldPosition) + ->decrement('queue_position'); + }); } } diff --git a/app/Livewire/Books/BookShow.php b/app/Livewire/Books/BookShow.php index bf1e9d7..e152be7 100644 --- a/app/Livewire/Books/BookShow.php +++ b/app/Livewire/Books/BookShow.php @@ -61,7 +61,8 @@ public function addToQueue(): void ->max('queue_position'); $maxPosition = is_numeric($maxPosition) ? (int) $maxPosition : 0; - $this->book->update(['queue_position' => $maxPosition + 1]); + // Queue position is organizational, not content — don't bump updated_at. + Book::withoutTimestamps(fn () => $this->book->update(['queue_position' => $maxPosition + 1])); $this->book->refresh(); } @@ -70,13 +71,18 @@ public function removeFromQueue(): void $this->authorize('update', $this->book); $oldPosition = $this->book->queue_position; - $this->book->update(['queue_position' => null]); - if ($oldPosition !== null) { - Book::where('user_id', Auth::id()) - ->where('queue_position', '>', $oldPosition) - ->decrement('queue_position'); - } + // Reordering the queue must not touch updated_at, or every shifted book + // would jump to the top of the "Recently Updated" sort on /books. + Book::withoutTimestamps(function () use ($oldPosition) { + $this->book->update(['queue_position' => null]); + + if ($oldPosition !== null) { + Book::where('user_id', Auth::id()) + ->where('queue_position', '>', $oldPosition) + ->decrement('queue_position'); + } + }); $this->book->refresh(); } diff --git a/app/Livewire/Books/ReadQueue.php b/app/Livewire/Books/ReadQueue.php index 40ecdc5..1e90533 100644 --- a/app/Livewire/Books/ReadQueue.php +++ b/app/Livewire/Books/ReadQueue.php @@ -27,7 +27,8 @@ public function addToQueue(Book $book): void ->max('queue_position'); $maxPosition = is_numeric($maxPosition) ? (int) $maxPosition : 0; - $book->update(['queue_position' => $maxPosition + 1]); + // Queue position is organizational, not content — don't bump updated_at. + Book::withoutTimestamps(fn () => $book->update(['queue_position' => $maxPosition + 1])); } public function removeFromQueue(Book $book): void @@ -35,14 +36,18 @@ public function removeFromQueue(Book $book): void $this->authorize('update', $book); $oldPosition = $book->queue_position; - $book->update(['queue_position' => null]); - // Reorder remaining items - if ($oldPosition !== null) { - Book::where('user_id', Auth::id()) - ->where('queue_position', '>', $oldPosition) - ->decrement('queue_position'); - } + // Reordering must not touch updated_at, or shifted books jump to the + // top of the "Recently Updated" sort on /books. + Book::withoutTimestamps(function () use ($book, $oldPosition) { + $book->update(['queue_position' => null]); + + if ($oldPosition !== null) { + Book::where('user_id', Auth::id()) + ->where('queue_position', '>', $oldPosition) + ->decrement('queue_position'); + } + }); } public function moveUp(Book $book): void @@ -58,8 +63,10 @@ public function moveUp(Book $book): void ->first(); if ($swapWith) { - $swapWith->update(['queue_position' => $book->queue_position]); - $book->update(['queue_position' => $book->queue_position - 1]); + Book::withoutTimestamps(function () use ($book, $swapWith) { + $swapWith->update(['queue_position' => $book->queue_position]); + $book->update(['queue_position' => $book->queue_position - 1]); + }); } } @@ -76,8 +83,10 @@ public function moveDown(Book $book): void ->first(); if ($swapWith) { - $swapWith->update(['queue_position' => $book->queue_position]); - $book->update(['queue_position' => $book->queue_position + 1]); + Book::withoutTimestamps(function () use ($book, $swapWith) { + $swapWith->update(['queue_position' => $book->queue_position]); + $book->update(['queue_position' => $book->queue_position + 1]); + }); } } @@ -89,12 +98,14 @@ public function moveToTop(Book $book): void return; } - // Shift all items above down by 1 - Book::where('user_id', Auth::id()) - ->where('queue_position', '<', $book->queue_position) - ->increment('queue_position'); + Book::withoutTimestamps(function () use ($book) { + // Shift all items above down by 1 + Book::where('user_id', Auth::id()) + ->where('queue_position', '<', $book->queue_position) + ->increment('queue_position'); - $book->update(['queue_position' => 1]); + $book->update(['queue_position' => 1]); + }); } public function moveToBottom(Book $book): void @@ -113,12 +124,14 @@ public function moveToBottom(Book $book): void return; } - // Shift all items below up by 1 - Book::where('user_id', Auth::id()) - ->where('queue_position', '>', $book->queue_position) - ->decrement('queue_position'); + Book::withoutTimestamps(function () use ($book, $maxPosition) { + // Shift all items below up by 1 + Book::where('user_id', Auth::id()) + ->where('queue_position', '>', $book->queue_position) + ->decrement('queue_position'); - $book->update(['queue_position' => $maxPosition]); + $book->update(['queue_position' => $maxPosition]); + }); } public function updateStatus(Book $book, string $status): void diff --git a/tests/Feature/BookQueueTimestampTest.php b/tests/Feature/BookQueueTimestampTest.php new file mode 100644 index 0000000..6996099 --- /dev/null +++ b/tests/Feature/BookQueueTimestampTest.php @@ -0,0 +1,99 @@ +user = User::factory()->create(); +}); + +/** + * Create `count` queued books (positions 1..count) whose updated_at is + * backdated to a known point, so we can detect any spurious "touch". + */ +function queuedBooks(User $user, int $count, Carbon\Carbon $backdatedTo): Illuminate\Support\Collection +{ + $books = collect(range(1, $count))->map(fn (int $position) => Book::factory()->wantToRead()->create([ + 'user_id' => $user->id, + 'queue_position' => $position, + ])); + + Book::withoutTimestamps(fn () => Book::whereIn('id', $books->pluck('id')) + ->update(['updated_at' => $backdatedTo])); + + return $books->each->refresh(); +} + +it('does not bump other queued books updated_at when one is marked read', function () { + $past = now()->subDays(10); + [$first, $second, $third] = queuedBooks($this->user, 3, $past)->all(); + + // Marking the top-of-queue book read auto-removes it and re-numbers the rest. + Livewire::actingAs($this->user) + ->test(BookIndex::class) + ->call('updateStatus', $first->id, 'read'); + + $second->refresh(); + $third->refresh(); + + // The shifted books were re-numbered... + expect($second->queue_position)->toBe(1) + ->and($third->queue_position)->toBe(2); + + // ...but their updated_at must be untouched (this is the regression we fixed). + expect($second->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')) + ->and($third->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')); + + // The book we actually changed *should* be updated for real. + $first->refresh(); + expect($first->status)->toBe(ReadingStatus::Read) + ->and($first->queue_position)->toBeNull() + ->and($first->updated_at->timestamp)->toBeGreaterThan($past->timestamp); +}); + +it('does not bump other queued books updated_at when one is removed from the queue', function () { + $past = now()->subDays(10); + [$first, $second, $third] = queuedBooks($this->user, 3, $past)->all(); + + Livewire::actingAs($this->user) + ->test(ReadQueue::class) + ->call('removeFromQueue', $first->id); + + $second->refresh(); + $third->refresh(); + + expect($second->queue_position)->toBe(1) + ->and($third->queue_position)->toBe(2) + ->and($second->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')) + ->and($third->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')); +}); + +it('does not bump updated_at when reordering the queue', function () { + $past = now()->subDays(10); + [$first, $second, $third] = queuedBooks($this->user, 3, $past)->all(); + + // moveToTop exercises the mass increment() branch. + Livewire::actingAs($this->user) + ->test(ReadQueue::class) + ->call('moveToTop', $third->id); + + $first->refresh(); + $second->refresh(); + $third->refresh(); + + // Positions reshuffled: the third book is now first. + expect($third->queue_position)->toBe(1) + ->and($first->queue_position)->toBe(2) + ->and($second->queue_position)->toBe(3); + + // No book's updated_at was touched by the reorder. + expect($first->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')) + ->and($second->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')) + ->and($third->updated_at->format('Y-m-d H:i:s'))->toBe($past->format('Y-m-d H:i:s')); +});