Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

持ち物追加処理を実装 #112

Merged
merged 6 commits into from
May 20, 2023
Merged

持ち物追加処理を実装 #112

merged 6 commits into from
May 20, 2023

Conversation

seigi0714
Copy link
Owner

@seigi0714 seigi0714 commented May 13, 2023

改修内容

  • 持ち物追加処理を追加
  • 持ち物追加フォーム用のボトムシートを実装
  • controller周りのテスト実装

※ 一覧画面の見た目は次回以降のタスク

改修後キャプチャ

2023-05-13.16.34.24.mov

Copy link
Owner Author

@seigi0714 seigi0714 left a comment

Choose a reason for hiding this comment

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

@shimizu-saffle
持ち物一覧追加処理実装した!
暇なとき確認お願い!

);
return [result, ...state.value ?? []];
});
ref.read(overlayLoadingProvider.notifier).endLoading();
Copy link
Owner Author

Choose a reason for hiding this comment

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

[memo]

  • 追加した持ち物は一番上に来るように修正
    • TODO: サーバーサイドからidが降順で帰ってくるように修正する
  • オーバーレイのローディングを出したいのでAsyncValueのisLoadingではなくoverlayLoadingProviderを使用

expect(result, [testAddBelonging, ...testFetchBelongings]);
});
});
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

[memo]
正常系のみ実装
理由としては
エラー帰ってきた際,AsyncValueのstateがAsyncValue.errorになるかなどはriverpod側の仕様なのでわざわざ
自分たちのテストで書かなくてもよいかなとおもったから

@seigi0714 seigi0714 marked this pull request as ready for review May 14, 2023 05:56
Copy link
Collaborator

@shimizu-saffle shimizu-saffle left a comment

Choose a reason for hiding this comment

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

@seigi0714
AddTripBelongingSheet のデザインと UXライティング良いね!
一箇所コメントした〜!

VoidCallback? onSuccess,
}) async {
ref.read(overlayLoadingProvider.notifier).startLoading();
state = await AsyncValue.guard(() async {
Copy link
Collaborator

@shimizu-saffle shimizu-saffle May 14, 2023

Choose a reason for hiding this comment

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

badge
ここで、AsyncValue.guard() すると、ref.read(tripInteractorProvider).addTripBelonging() を呼んだ時に例外が生じると、TripBelongingListErrorCat が表示されちゃう。
(fetchTripBelongings に失敗してデータ取得できなかった時は ErrorCat で良い)

例外発生時の挙動は上記のようになるよりは、すでにフェッチ成功している持ち物リストを表示したままスナックバーを出したほうが UX が良いと思う💡

Future<void> addBelonging({
    required int tripId,
    required String name,
    required int numOf,
    required bool isShareAmongMember,
    VoidCallback? onSuccess,
  }) async {
    ref.read(overlayLoadingProvider.notifier).startLoading();
    // 例外発生時に AsyncValue.when でビルドしているウィジェットを error にしないため、
    // AsyncValue.guard() を使っていない。
    try {
      final result = await ref.read(tripInteractorProvider).addTripBelonging(
            tripId: tripId,
            name: name,
            numOf: numOf,
            isShareAmongMember: isShareAmongMember,
          );
      state = AsyncValue.data([result, ...state.value ?? []]);
    } on Exception catch (e) {
      ref.read(exceptionHandlerProvider).handleException(e);
    } finally {
      ref.read(overlayLoadingProvider.notifier).endLoading();
      onSuccess?.call();
    }
  }

Copy link
Owner Author

Choose a reason for hiding this comment

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

ありがとう確かにそやね〜!修正する!

Copy link
Owner Author

Choose a reason for hiding this comment

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

@shimizu-saffle
修正した!

Copy link
Collaborator

@shimizu-saffle shimizu-saffle left a comment

Choose a reason for hiding this comment

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

LGTM!!(遅くなってごめん🙏) @seigi0714

@seigi0714 seigi0714 merged commit ff4a775 into main May 20, 2023
3 checks passed
@seigi0714 seigi0714 deleted the feature/add_belonging branch May 20, 2023 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants