From fe230a980c8aa4bf110d80e0bb631cb38317fac5 Mon Sep 17 00:00:00 2001 From: 128Na Date: Fri, 14 Aug 2026 23:05:43 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20Pages=20=E3=82=B3=E3=83=B3=E3=83=9D?= =?UTF-8?q?=E3=83=BC=E3=83=8D=E3=83=B3=E3=83=88=E3=81=AE=E3=83=9A=E3=83=BC?= =?UTF-8?q?=E3=82=B8=E3=83=8D=E3=83=BC=E3=82=B7=E3=83=A7=E3=83=B3=E7=8A=B6?= =?UTF-8?q?=E6=85=8B=E3=82=92#[Url]=20$page=E3=81=AB=E4=B8=80=E6=9C=AC?= =?UTF-8?q?=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WithPaginationトレイトの内部状態($paginators)とSearchActionへ渡す$page プロパティが二重管理になっており、render()冒頭のresetPage()は $paginatorsのみを操作するため実際には無効化されていた。結果として 検索条件を変更しても表示ページ番号がリセットされず、フィルタ後の 結果件数によっては空ページが表示されうるバグがあった。 - WithPaginationトレイトを削除し、$pageのみを唯一の状態源にする - onConditionUpdate()はrender()を直接呼ばず、ページ状態のリセットに専念 (再描画はLivewireのライフサイクルに委譲) - 検索結果を#[Computed] pages()に切り出し、render()を単純化 - $keywordに#[Validate('string|max:191')]を追加しバリデーションエラー表示を追加 - Pagesコンポーネントのテストを新規追加(既存はゼロだった) --- app/Livewire/Pages.php | 55 ++++++++------- resources/views/livewire/pages.blade.php | 9 ++- tests/Feature/Livewire/PagesTest.php | 90 ++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 29 deletions(-) create mode 100644 tests/Feature/Livewire/PagesTest.php diff --git a/app/Livewire/Pages.php b/app/Livewire/Pages.php index deddb9a..0bdf214 100644 --- a/app/Livewire/Pages.php +++ b/app/Livewire/Pages.php @@ -7,22 +7,21 @@ use App\Actions\SearchPage\SearchAction; use App\Enums\PakSlug; use App\Enums\SiteName; +use App\Models\Page; +use Illuminate\Contracts\Pagination\LengthAwarePaginator; use Illuminate\Contracts\View\View; +use Livewire\Attributes\Computed; use Livewire\Attributes\Url; +use Livewire\Attributes\Validate; use Livewire\Component; -use Livewire\WithPagination; final class Pages extends Component { - use WithPagination; - + #[Validate('string|max:191')] public string $keyword = ''; - /** - * @var int|string|null - */ #[Url] - public $page = 1; + public int $page = 1; /** * @var array @@ -42,33 +41,37 @@ final class Pages extends Component SiteName::Portal->value => true, ]; - public function render(SearchAction $searchAction): View + public function render(): View { - $this->resetPage(); - if (! is_numeric($this->page)) { - $this->page = 1; - } - - return view('livewire.pages', [ - 'pages' => $searchAction([ - 'keyword' => $this->keyword, - 'paks' => $this->selectedPaks(), - 'sites' => $this->selectedSites(), - 'page' => $this->page, - ]), - ]); + return view('livewire.pages'); } - public function onConditionUpdate(SearchAction $searchAction): View + /** + * ページネーションリンクは通常のURL遷移（`?page=N`）で行われ、`#[Url] $page` が唯一の状態源。 + * 検索条件（キーワード・pak・サイト）を変えたときだけ、ここで明示的に1ページ目へ戻す。 + */ + public function onConditionUpdate(): void { - - return $this->render($searchAction); + $this->page = 1; } public function clear(): void { - $this->resetPage(); - $this->reset('keyword', 'paks', 'sites'); + $this->reset('keyword', 'paks', 'sites', 'page'); + } + + /** + * @return LengthAwarePaginator + */ + #[Computed] + public function pages(): LengthAwarePaginator + { + return (new SearchAction)([ + 'keyword' => $this->keyword, + 'paks' => $this->selectedPaks(), + 'sites' => $this->selectedSites(), + 'page' => $this->page, + ]); } /** diff --git a/resources/views/livewire/pages.blade.php b/resources/views/livewire/pages.blade.php index 30bbd9a..2bc0ac5 100644 --- a/resources/views/livewire/pages.blade.php +++ b/resources/views/livewire/pages.blade.php @@ -16,13 +16,16 @@ + @error('keyword') +

{{ $message }}

+ @enderror
- {{ $pages->onEachSide(1)->links('tailwind_custom') }} + {{ $this->pages->onEachSide(1)->links('tailwind_custom') }}
    - @forelse ($pages as $page) + @forelse ($this->pages as $page)
  • @foreach ($page->paks as $pak) @@ -42,6 +45,6 @@ @endforelse
- {{ $pages->onEachSide(1)->links('tailwind_custom') }} + {{ $this->pages->onEachSide(1)->links('tailwind_custom') }}
diff --git a/tests/Feature/Livewire/PagesTest.php b/tests/Feature/Livewire/PagesTest.php new file mode 100644 index 0000000..b8d8b0d --- /dev/null +++ b/tests/Feature/Livewire/PagesTest.php @@ -0,0 +1,90 @@ +assertOk(); + } + + public function test_default_state_selects_all_paks_and_sites(): void + { + Livewire::test(Pages::class) + ->assertSet('paks', [ + PakSlug::Pak64->value => true, + PakSlug::Pak128->value => true, + PakSlug::Pak128Jp->value => true, + ]) + ->assertSet('sites', [ + SiteName::Japan->value => true, + SiteName::Twitrans->value => true, + SiteName::Portal->value => true, + ]); + } + + public function test_condition_update_resets_page_to_one(): void + { + Livewire::test(Pages::class) + ->set('page', 3) + ->call('onConditionUpdate') + ->assertSet('page', 1); + } + + public function test_clear_resets_keyword_paks_sites_and_page(): void + { + Livewire::test(Pages::class) + ->set('keyword', 'foo') + ->set('paks.'.PakSlug::Pak64->value, false) + ->set('page', 3) + ->call('clear') + ->assertSet('keyword', '') + ->assertSet('page', 1) + ->assertSet('paks.'.PakSlug::Pak64->value, true); + } + + public function test_keyword_over_max_length_fails_validation(): void + { + Livewire::test(Pages::class) + ->set('keyword', str_repeat('a', 192)) + ->assertHasErrors(['keyword' => 'max']); + } + + public function test_search_results_reflect_keyword_filter(): void + { + $pak = Pak::factory()->create(['slug' => PakSlug::Pak128]); + + $matching = Page::factory()->create([ + 'site_name' => SiteName::Japan, + 'title' => 'Steam Locomotive Addon', + ]); + $matching->paks()->attach($pak); + + $other = Page::factory()->create([ + 'site_name' => SiteName::Japan, + 'title' => 'Bus Addon', + ]); + $other->paks()->attach($pak); + + $testable = Livewire::test(Pages::class) + ->set('keyword', 'Locomotive') + ->call('onConditionUpdate'); + + $results = $testable->instance()->pages; + + $this->assertCount(1, $results); + $this->assertSame($matching->id, $results->first()->id); + } +} From dd9371f2448d2190890becedd4592a23fa81f80b Mon Sep 17 00:00:00 2001 From: 128Na Date: Fri, 14 Aug 2026 23:26:04 +0900 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20=E3=82=BB=E3=83=AB=E3=83=95=E3=83=AC?= =?UTF-8?q?=E3=83=93=E3=83=A5=E3=83=BC=E6=8C=87=E6=91=98=E3=81=AB=E5=AF=BE?= =?UTF-8?q?=E5=BF=9C=EF=BC=88=E3=83=9A=E3=83=BC=E3=82=B8=E3=83=8D=E3=83=BC?= =?UTF-8?q?=E3=82=B7=E3=83=A7=E3=83=B3=E3=83=AA=E3=83=B3=E3=82=AF=E7=A0=B4?= =?UTF-8?q?=E6=90=8D=E3=81=AE=E5=9B=9E=E5=B8=B0=E4=BF=AE=E6=AD=A3=E3=81=BB?= =?UTF-8?q?=E3=81=8B=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review によるセルフレビューで、直前のコミットが以下の回帰・不整合を 含んでいたことが判明したため修正する。 - WithPaginationトレイトを外した際、その内部でLaravelのPaginatorへ 登録していたパス解決処理(Paginator::currentPathResolver)まで一緒に 失っていた。これによりLivewireのアクション経由で再描画した後は、 ページネーションリンクの遷移先がLivewireの内部updateエンドポイント (POST専用)になり、通常のリンククリック(GET)が405になっていた。 boot()で明示的に同等の処理を肩代わりして修正。 - $pageに下限チェックが無く、?page=-1のような不正な値がそのまま SearchAction/paginate()に渡っていたため、max(1, $this->page)で クランプ。 - 追加した#[Validate('string|max:191')]が、同じ検索機能の既存API側 バリデーション(PageSearchRequest::rules()のkeyword=>'present|max:20') と食い違っていたため、max:20に統一。 - 新規テストの軽微な指摘（不要なアクション呼び出しの削除、テスト名が 謳うsitesリセットの検証漏れ）を修正。 - ページネーションリンクが内部updateエンドポイントを指さないことを 検証する回帰テストを追加。 なお、検索条件(keyword/paks/sites)自体が`#[Url]`化されておらず、 ページネーションのプレーンリンク遷移時にURLへ乗らず消えてしまう 既存の制限は、本PRのスコープ（ページ番号の二重管理解消）外として コード上にコメントで明記するに留めた。 --- app/Livewire/Pages.php | 30 +++++++++++++++--- tests/Feature/Livewire/PagesTest.php | 46 +++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/app/Livewire/Pages.php b/app/Livewire/Pages.php index 0bdf214..c35d954 100644 --- a/app/Livewire/Pages.php +++ b/app/Livewire/Pages.php @@ -10,14 +10,16 @@ use App\Models\Page; use Illuminate\Contracts\Pagination\LengthAwarePaginator; use Illuminate\Contracts\View\View; +use Illuminate\Pagination\Paginator; use Livewire\Attributes\Computed; use Livewire\Attributes\Url; use Livewire\Attributes\Validate; use Livewire\Component; +use Livewire\Livewire; final class Pages extends Component { - #[Validate('string|max:191')] + #[Validate('string|max:20')] public string $keyword = ''; #[Url] @@ -41,14 +43,32 @@ final class Pages extends Component SiteName::Portal->value => true, ]; + /** + * `WithPagination`トレイトを外した分、ページネーションリンクの生成元URLを + * 実際のページURLに合わせる処理（本来はトレイトの`boot()`が担っていた）を + * ここで肩代わりする。これが無いと、Livewireのアクション経由でページ送り + * リンクを再生成した際にリンク先がLivewireの内部エンドポイント（POST専用） + * になってしまい、クリック時に405が返る。 + */ + public function boot(): void + { + Paginator::currentPathResolver(fn (): string => Livewire::originalPath()); + } + public function render(): View { return view('livewire.pages'); } /** - * ページネーションリンクは通常のURL遷移（`?page=N`）で行われ、`#[Url] $page` が唯一の状態源。 - * 検索条件（キーワード・pak・サイト）を変えたときだけ、ここで明示的に1ページ目へ戻す。 + * ページネーションリンクは通常のURL遷移（`?page=N`）で行われ、`#[Url] $page` が + * 「今何ページ目か」を表す唯一の状態源。検索条件（キーワード・pak・サイト）を + * 変えたときは、ここで明示的に1ページ目へ戻す。 + * + * 注意: `$keyword`/`$paks`/`$sites`は`#[Url]`化していないため、ページネーション + * リンク（プレーンな``によるページ遷移）をクリックすると検索条件はURLに + * 乗らず、デフォルト状態でコンポーネントが再マウントされる（既知の制限。今回の + * 修正はページ番号の二重管理を解消する範囲に限定しており、この制限自体はスコープ外）。 */ public function onConditionUpdate(): void { @@ -70,7 +90,9 @@ public function pages(): LengthAwarePaginator 'keyword' => $this->keyword, 'paks' => $this->selectedPaks(), 'sites' => $this->selectedSites(), - 'page' => $this->page, + // #[Url]は型不一致(TypeError)は弾くが、0以下の値はintとして許してしまうため、 + // ?page=-1 のような不正なURLでもここで1に丸める。 + 'page' => max(1, $this->page), ]); } diff --git a/tests/Feature/Livewire/PagesTest.php b/tests/Feature/Livewire/PagesTest.php index b8d8b0d..bff6b44 100644 --- a/tests/Feature/Livewire/PagesTest.php +++ b/tests/Feature/Livewire/PagesTest.php @@ -9,6 +9,7 @@ use App\Livewire\Pages; use App\Models\Page; use App\Models\Pak; +use App\Models\RawPage; use Livewire\Livewire; use Tests\Feature\TestCase; @@ -43,22 +44,60 @@ public function test_condition_update_resets_page_to_one(): void ->assertSet('page', 1); } + public function test_pagination_links_do_not_point_to_the_livewire_update_endpoint(): void + { + // WithPagination除去に伴い、ページネーションリンクの生成元パスをboot()で + // 明示的に補っている(Livewire::originalPath())。これが無いと、Livewireの + // アクション経由で再描画した際にリンク先がPOST専用の内部updateエンドポイント + // になり、通常のリンククリック(GET)が405になる回帰を防ぐテスト。 + $pak = Pak::factory()->create(['slug' => PakSlug::Pak128]); + // Fakerの乱数urlは60件生成すると衝突しうる(raw_pages/pagesのurlユニーク制約)ため、 + // テストの再現性を優先して明示的にユニークなurlを採番する。 + for ($i = 0; $i < 60; $i++) { + $page = Page::factory()->create([ + 'site_name' => SiteName::Japan, + 'url' => "https://example.test/page-{$i}", + 'raw_page_id' => RawPage::factory()->create(['url' => "https://example.test/raw-{$i}"])->id, + ]); + $page->paks()->attach($pak); + } + + $html = Livewire::test(Pages::class) + ->call('onConditionUpdate') + ->html(); + + $this->assertMatchesRegularExpression('#href="[^"]*\?page=2"#', $html); + $this->assertStringNotContainsString('/update?page=2', $html); + } + public function test_clear_resets_keyword_paks_sites_and_page(): void { Livewire::test(Pages::class) ->set('keyword', 'foo') ->set('paks.'.PakSlug::Pak64->value, false) + ->set('sites.'.SiteName::Japan->value, false) ->set('page', 3) ->call('clear') ->assertSet('keyword', '') ->assertSet('page', 1) - ->assertSet('paks.'.PakSlug::Pak64->value, true); + ->assertSet('paks.'.PakSlug::Pak64->value, true) + ->assertSet('sites.'.SiteName::Japan->value, true); + } + + public function test_negative_page_is_clamped_to_one(): void + { + $results = Livewire::test(Pages::class) + ->set('page', -1) + ->instance() + ->pages; + + $this->assertSame(1, $results->currentPage()); } public function test_keyword_over_max_length_fails_validation(): void { Livewire::test(Pages::class) - ->set('keyword', str_repeat('a', 192)) + ->set('keyword', str_repeat('a', 21)) ->assertHasErrors(['keyword' => 'max']); } @@ -79,8 +118,7 @@ public function test_search_results_reflect_keyword_filter(): void $other->paks()->attach($pak); $testable = Livewire::test(Pages::class) - ->set('keyword', 'Locomotive') - ->call('onConditionUpdate'); + ->set('keyword', 'Locomotive'); $results = $testable->instance()->pages;