Skip to content

アニメリストのフェッチ方法 (#77)#164

Merged
KOU050223 merged 3 commits intomainfrom
vk/fe75-77
Jan 26, 2026
Merged

アニメリストのフェッチ方法 (#77)#164
KOU050223 merged 3 commits intomainfrom
vk/fe75-77

Conversation

@KOU050223
Copy link
Owner

@KOU050223 KOU050223 commented Jan 25, 2026

GitHub issue #77: 初回のみフェッチするようにする
現在はアニメ一覧を取得しようとして10000件くらいのfirestoreの情報を毎回とってしまうのでDBにすごいダメージが出てしまう

Summary by CodeRabbit

  • Refactor

    • Centralized anime list state into an application-wide provider, removing local view-model instantiation and simplifying lifecycle.
    • View now binds to the shared state and triggers a one-time fetch on first display to avoid redundant loads.
  • New Features

    • Added a fetch-control flag and a public indicator to allow forced refreshes and to track whether an initial server fetch has occurred.

✏️ Tip: You can customize this high-level summary in your review settings.

## 実装内容

### 問題の原因
- `AnimeListPage`が表示されるたびに新しい`AnimeListViewModel`インスタンスが作成され、毎回`fetchFromServer()`が呼ばれていた
- これにより約10,000件のFirestoreドキュメントが毎回読み込まれ、大量のDBアクセスが発生していた

### 解決策

#### 1. **アプリケーション全体でViewModelを共有** (`app.dart:10-16`)
```dart
MultiProvider(
  providers: [
    ChangeNotifierProvider(
      create: (_) => AnimeListViewModel(),
    ),
  ],
  ...
)
```

#### 2. **初回フェッチフラグの追加** (`anime_list_view_model.dart:19, 30`)
- `_hasFetchedFromServer`フラグを追加
- サーバーから取得済みかどうかを記録

#### 3. **フェッチロジックの改善** (`anime_list_view_model.dart:408-482`)
- `fetchFromServer()`に`force`パラメータを追加
- `force: false`の場合、既に取得済みならスキップ
- `force: true`の場合、強制的に再フェッチ

#### 4. **ページでの使用方法を変更** (`anime_list_page.dart:17-24`)
```dart
// 既存のViewModelを使用
final viewModel = Provider.of<AnimeListViewModel>(context, listen: false);

// 初回のみフェッチ
if (!viewModel.isLoading && viewModel.animeList.isEmpty) {
  WidgetsBinding.instance.addPostFrameCallback((_) {
    viewModel.fetchFromServer();
  });
}
```

#### 5. **手動取得ボタンの対応** (`anime_list_page.dart:401`)
- 「オンラインから取得」ボタンは`force: true`で強制再フェッチ

### 効果
- **初回アクセス時のみ**Firestoreから約10,000件のアニメリストを取得
- **2回目以降のアクセス**ではメモリ上のキャッシュを使用し、Firestoreへのアクセスなし
- ユーザーが明示的に「オンラ��ンから取得」ボタンを押した場合のみ再フェッチ
- DBの読み込み負荷が大幅に削減される

これで issue #77 の修正が完了しました!
@coderabbitai
Copy link

coderabbitai bot commented Jan 25, 2026

📝 Walkthrough

Walkthrough

Promotes AnimeListViewModel to an application-wide ChangeNotifierProvider at app root; AnimeListPage now accesses the shared view model and triggers a one-time post-frame fetch (calling fetchFromServer(force:true) when needed). ViewModel adds _hasFetchedFromServer flag and a force parameter to fetchFromServer.

Changes

Cohort / File(s) Summary
Provider setup & app root
lib/app.dart
Wraps MaterialApp in MultiProvider, registering a global ChangeNotifierProvider<AnimeListViewModel>; adds imports for anime_list_view_model and provider.
Page-level access & fetch trigger
lib/ui/animes/view/anime_list_page.dart
Removes local provider creation; uses Provider.of(context, listen: false) to obtain shared view model. Adds post-frame callback to call fetchFromServer(force: true) on first build when list is empty. Removes unused imports.
ViewModel API & fetch state
lib/ui/animes/view_model/anime_list_view_model.dart
Adds private _hasFetchedFromServer and public bool get hasFetchedFromServer. Changes Future<void> fetchFromServer({bool force = false}) to skip when already fetched unless force is true; sets _hasFetchedFromServer = true on successful or fallback fetch paths.

Sequence Diagram

sequenceDiagram
    participant App as App
    participant Provider as MultiProvider
    participant Page as AnimeListPage
    participant VM as AnimeListViewModel
    participant Server as Server

    App->>Provider: Initialize MultiProvider with AnimeListViewModel
    Provider->>VM: Create shared instance (_hasFetchedFromServer = false)

    Page->>Provider: Provider.of(context, listen:false)
    Provider-->>Page: Shared ViewModel instance

    Page->>Page: Build -> detect empty list & not loading
    Page->>Page: Schedule post-frame callback
    Page->>VM: fetchFromServer(force: true)
    alt !_hasFetchedFromServer or force==true
        VM->>Server: Request anime data
        Server-->>VM: Return data / error
        VM->>VM: _hasFetchedFromServer = true
        VM-->>Page: notifyListeners (data available)
    else _hasFetchedFromServer == true and force==false
        VM-->>Page: Return early (skip fetch)
    end

    Page->>Page: Rebuild with updated state
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A provider hopped into the app today,
Shared my list so all could play 🎬
Fetched once, or forced when called,
Carrots of state, never stalled 🥕
Hop happy, view synced all the way ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete compared to the template. It lacks sections for '変更内容' (detailed changes), テスト (test checklist), and other template sections, containing only a brief explanation of the issue. Expand the description to follow the repository template structure: add '変更内容' section with specific changes, complete the '関連Issue' section, and fill in the テスト checklist with test verification status.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'アニメリストのフェッチ方法' accurately summarizes the main change of refactoring how anime list data is fetched to optimize Firestore queries, with issue reference #77.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/ui/animes/view/anime_list_page.dart (1)

273-274: Fix syntax error causing pipeline failure.

The pipeline reports a Dart format parse error at lines 273-274. There appears to be an extra closing parenthesis. The Scaffold widget should close after the floatingActionButton parameter, but the current structure has unbalanced delimiters.

🐛 Proposed fix
             ),
           ),
-        ),
-      ),
+        )
     );
   }

Trace the widget tree: Scaffold opens at line 25 and takes body: and floatingActionButton:. After the FAB's Consumer closes at line 272, only one ); is needed to close the Scaffold and return statement.

🧹 Nitpick comments (1)
lib/ui/animes/view/anime_list_page.dart (1)

15-23: Consider using hasFetchedFromServer in the fetch condition.

The current condition !viewModel.isLoading && viewModel.animeList.isEmpty could trigger redundant addPostFrameCallback calls on rebuilds before data loads. While the ViewModel's _hasFetchedFromServer flag prevents actual Firestore reads, it would be cleaner to use this flag in the UI condition as well.

♻️ Proposed fix
     // 初回のみフェッチ
-    if (!viewModel.isLoading && viewModel.animeList.isEmpty) {
+    if (!viewModel.hasFetchedFromServer && !viewModel.isLoading) {
       WidgetsBinding.instance.addPostFrameCallback((_) {
         viewModel.fetchFromServer();
       });
     }

This directly leverages the caching flag and avoids repeated callback scheduling when the list happens to be empty for other reasons.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/ui/animes/view/anime_list_page.dart (1)

230-274: Fix syntax error around floatingActionButton (CI parse failure).

There’s an extra closing parenthesis after the Consumer, which matches the formatter error (Line 273–274). Remove the stray ) so the Scaffold closes correctly.

🐛 Proposed fix
       floatingActionButton: Consumer<AnimeListViewModel>(
         builder: (context, viewModel, child) {
           ...
         },
       ),
-      ),
     );
🤖 Fix all issues with AI agents
In `@lib/ui/animes/view/anime_list_page.dart`:
- Around line 15-23: The initial fetch scheduling should be guarded by a new
hasFetchedFromServer flag on AnimeListViewModel so empty results or failures
don't cause repeated DB reads; modify the condition around
WidgetsBinding.instance.addPostFrameCallback to check
!viewModel.hasFetchedFromServer in addition to !viewModel.isLoading and
viewModel.animeList.isEmpty, and ensure fetchFromServer sets
hasFetchedFromServer = true (or set it immediately inside the post-frame
callback before/after calling fetchFromServer) so subsequent rebuilds won't
re-schedule the fetch; update AnimeListViewModel to expose and persist
hasFetchedFromServer accordingly.

@github-actions
Copy link

Visit the preview URL for this PR (updated for commit 53079d9):

https://animeishi-73560--pr164-vk-fe75-77-yddxa2nc.web.app

(expires Sun, 01 Feb 2026 19:22:25 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f1f494a67df8ccbb2232f73b6ccab4934a51e505

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/ui/animes/view/anime_list_page.dart (1)

530-548: Using context after Navigator.pop() may cause a "deactivated widget" error.

After Navigator.of(context).pop() is called, the dialog's context is no longer mounted. Calling AnimeNotification.showSuccess/showError with this context will fail at runtime.

Capture the outer context or show the notification before popping, or use mounted check (though that requires a StatefulWidget).

Proposed fix: Show notification after returning from dialog
 confirmDismiss: (direction) async {
-  return await _showUnregisterDialog(
+  final result = await _showUnregisterDialog(
       context,
       viewModel,
       tid,
       anime['title'] ?? 'タイトル不明');
+  // Handle notification in the parent context after dialog closes
+  return result;
 },

Then move the notification logic to confirmDismiss or onDismissed callback where the parent context is still valid, or restructure _showUnregisterDialog to return a result and let the caller show the notification.

Alternatively, capture the ScaffoldMessenger before the async gap:

 onPressed: () async {
+  final messenger = ScaffoldMessenger.of(context);
+  final navigator = Navigator.of(context);
   try {
     viewModel.selectAnime(tid);
     await viewModel.deleteSelectedAnime();
-    Navigator.of(context).pop(true);
-    AnimeNotification.showSuccess(
-      context,
+    navigator.pop(true);
+    // Note: AnimeNotification needs refactoring to accept messenger
     ...

@KOU050223 KOU050223 self-assigned this Jan 26, 2026
@KOU050223 KOU050223 merged commit 61a08de into main Jan 26, 2026
5 checks passed
@KOU050223 KOU050223 deleted the vk/fe75-77 branch January 26, 2026 11:31
KOU050223 added a commit that referenced this pull request Jan 28, 2026
既存の実装(#164)を改善し、アプリ終了後も初回ダウンロードフラグを保持するようにしました。

## 変更内容

### 問題点
- 既存実装ではメモリ上のフラグ(_hasFetchedFromServer)でサーバー取得状態を管理
- アプリを再起動するとフラグがリセットされ、毎回約10,000件をダウンロード
- アプリのライフサイクル全体での最適化になっていない

### 解決策

#### 1. SharedPreferencesで永続化
- `_hasDownloadedAnimeList()`: 初回ダウンロード済みかチェック
- `_markAsDownloaded()`: ダウンロード完了をデバイスに保存
- アプリ終了後も初回取得状態を保持

#### 2. 新しいinitAnimeList()メソッド
- 初回ダウンロード済みかチェック
- 未ダウンロード: サーバーから取得してフラグを保存
- ダウンロード済み: Firestoreキャッシュから読み込み

#### 3. app.dartの簡素化
- MultiProviderによるグローバル共有を削除
- 各ページで独立したViewModelインスタンスを作成
- SharedPreferencesによる永続化で状態を共有

#### 4. forceRefreshパラメータ
- 手動更新時は`forceRefresh: true`で強制的にサーバーから取得
- ユーザーが明示的に最新データを取得可能

### 効果
- **真の初回のみ取得**: アプリ再起動しても2回目以降はキャッシュを使用
- **長期的なDB負荷削減**: Firestoreの読み取りコストを大幅に削減
- **ユーザー体験向上**: アプリ起動速度の改善
- **シンプルな実装**: MultiProviderが不要になり、コードが簡潔に

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
KOU050223 added a commit that referenced this pull request Jan 28, 2026
既存の実装(#164)を改善し、アプリ終了後も初回ダウンロードフラグを保持するようにしました。

## 変更内容

### 問題点
- 既存実装ではメモリ上のフラグ(_hasFetchedFromServer)でサーバー取得状態を管理
- アプリを再起動するとフラグがリセットされ、毎回約10,000件をダウンロード
- アプリのライフサイクル全体での最適化になっていない

### 解決策

#### 1. SharedPreferencesで永続化
- `_hasDownloadedAnimeList()`: 初回ダウンロード済みかチェック
- `_markAsDownloaded()`: ダウンロード完了をデバイスに保存
- アプリ終了後も初回取得状態を保持

#### 2. 新しいinitAnimeList()メソッド
- 初回ダウンロード済みかチェック
- 未ダウンロード: サーバーから取得してフラグを保存
- ダウンロード済み: Firestoreキャッシュから読み込み

#### 3. app.dartの簡素化
- MultiProviderによるグローバル共有を削除
- 各ページで独立したViewModelインスタンスを作成
- SharedPreferencesによる永続化で状態を共有

#### 4. forceRefreshパラメータ
- 手動更新時は`forceRefresh: true`で強制的にサーバーから取得
- ユーザーが明示的に最新データを取得可能

### 効果
- **真の初回のみ取得**: アプリ再起動しても2回目以降はキャッシュを使用
- **長期的なDB負荷削減**: Firestoreの読み取りコストを大幅に削減
- **ユーザー体験向上**: アプリ起動速度の改善
- **シンプルな実装**: MultiProviderが不要になり、コードが簡潔に

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant