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

👍 セッションのリフレッシュ処理を追加 #103

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

tatsutakein
Copy link
Member

@tatsutakein tatsutakein commented Dec 2, 2023

Issue

  • close #ISSUE_NUMBER 🦕

概要

セッションのリフレッシュ処理を追加します。

レビュー観点

特になし

レビューレベル

  • Lv0: まったく見ないで Approve する
  • Lv1: ぱっとみて違和感がないかチェックして Approve する
  • Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証して Approve する
  • Lv3: 実際に環境で動作確認したうえで Approve する

レビュー優先度

  • すぐに見てもらいたい ( hotfix など ) 🚀
  • 今日中に見てもらいたい 🚗
  • 今日〜明日中で見てもらいたい 🚶
  • 数日以内で見てもらいたい 🐢

参考リンク

スクリーンショット

Before After

Summary by CodeRabbit

  • 新機能

    • 認証状態のストリーミング機能を追加しました。
    • ログイン画面と設定画面に認証状態のストリーミングを統合しました。
  • バグ修正

    • 認証状態の観察ロジックを改善しました。
  • ドキュメント

    • 認証関連のドキュメントを更新しました。
  • リファクタ

    • 認証状態の取得方法をAuthStatusStreamUseCaseに変更しました。
  • スタイル

    • UIコンポーネントのスタイルを調整しました。
  • テスト

    • 認証機能のテストケースを追加しました。
  • チョア

    • 内部コードの整理を行いました。
  • リバート

    • 特定の変更を元に戻しました。

@tatsutakein tatsutakein requested a review from a team as a code owner December 2, 2023 14:13
Copy link

coderabbitai bot commented Dec 2, 2023

Walkthrough

認証状態の監視に関連するクラスとプロパティが更新されました。ObserveAuthStatusUseCaseAuthStatusStreamUseCaseに名称変更され、GoTrueAuthに変更されました。これらの変更は、認証状態のフローをより直接的に扱うようにリファクタリングされたことを示しています。また、新しいNetworkServiceクラスが導入され、ネットワーク操作の抽象化が行われました。

Changes

ファイルパス 変更概要
app/.../EntryPointTest.kt
app/.../LoginStateMachine.swift
app/.../AuthStatusStreamUseCaseProvider.swift
app/.../RootStateMachine.swift
app/.../ComposeSettingsScreen.swift
app/shared/.../NitoAppStateMachine.kt
app/shared/.../AppModule.kt
core/data/.../AuthRepository.kt
core/data/.../DefaultAuthRepository.kt
core/domain/.../AuthStatusStreamUseCase.kt
core/domain/.../UseCaseModule.kt
core/model/.../AuthState.kt
core/network/.../NetworkService.kt
core/network/.../AuthRemoteDataSource.kt
core/network/.../FakeAuthRemoteDataSource.kt
core/network/.../SupabaseAuthRemoteDataSource.kt
core/network/.../RemoteDataSourceModule.kt
core/network/.../SupabaseParticipantRemoteDataSource.kt
core/network/.../SupabaseScheduleRemoteDataSource.kt
core/network/.../SupabaseUserRemoteDataSource.kt
feature/auth/.../LoginScreenStateMachine.kt
feature/auth/.../AuthFeatureModule.kt
feature/settings/.../SettingsScreenStateMachine.kt
feature/settings/.../SettingsFeatureModule.kt
ObserveAuthStatusUseCaseからAuthStatusStreamUseCaseへの名称変更。
GoTrueからAuthへの名称変更。
AuthStatusStreamUseCaseProviderの新規追加。
NetworkServiceクラスの新規追加とネットワーク操作の抽象化。
AuthStatusの新しい状態LoadingNetworkErrorの追加。

🐰✨
コードの森を駆けるコードラビット、
リファクタの風が吹き抜けていく。
新しい流れを追いかけて、
未来へと続く道を築く。


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

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.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 3a638e1 and be1513e.
Files selected for processing (26)
  • app/ios-combined/src/iosTest/kotlin/club/nito/ios/combined/EntryPointTest.kt (3 hunks)
  • app/ios/Modules/Sources/Auth/ComposeLoginScreen.swift (1 hunks)
  • app/ios/Modules/Sources/Auth/LoginStateMachine.swift (2 hunks)
  • app/ios/Modules/Sources/KmpContainer/AuthStatusStreamUseCaseProvider.swift (1 hunks)
  • app/ios/Modules/Sources/Navigation/RootStateMachine.swift (2 hunks)
  • app/ios/Modules/Sources/Settings/ComposeSettingsScreen.swift (1 hunks)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoAppStateMachine.kt (2 hunks)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoNavHost.kt (1 hunks)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/di/AppModule.kt (1 hunks)
  • core/data/src/commonMain/kotlin/club/nito/core/data/AuthRepository.kt (2 hunks)
  • core/data/src/commonMain/kotlin/club/nito/core/data/DefaultAuthRepository.kt (1 hunks)
  • core/domain/src/commonMain/kotlin/club/nito/core/domain/AuthStatusStreamUseCase.kt (1 hunks)
  • core/domain/src/commonMain/kotlin/club/nito/core/domain/di/UseCaseModule.kt (3 hunks)
  • core/model/src/commonMain/kotlin/club/nito/core/model/AuthState.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/NetworkService.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/auth/AuthRemoteDataSource.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/auth/FakeAuthRemoteDataSource.kt (4 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/auth/SupabaseAuthRemoteDataSource.kt (2 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/di/RemoteDataSourceModule.kt (2 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/participation/SupabaseParticipantRemoteDataSource.kt (1 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/schedule/SupabaseScheduleRemoteDataSource.kt (3 hunks)
  • core/network/src/commonMain/kotlin/club/nito/core/network/user/SupabaseUserRemoteDataSource.kt (2 hunks)
  • feature/auth/src/commonMain/kotlin/club/nito/feature/auth/LoginScreenStateMachine.kt (4 hunks)
  • feature/auth/src/commonMain/kotlin/club/nito/feature/auth/di/AuthFeatureModule.kt (1 hunks)
  • feature/settings/src/commonMain/kotlin/club/nito/feature/settings/SettingsScreenStateMachine.kt (4 hunks)
  • feature/settings/src/commonMain/kotlin/club/nito/feature/settings/di/SettingsFeatureModule.kt (1 hunks)
Files skipped from review due to trivial changes (1)
  • app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoNavHost.kt
Additional comments: 48
app/ios-combined/src/iosTest/kotlin/club/nito/ios/combined/EntryPointTest.kt (3)
  • 8-8: AuthStatusStreamUseCaseの導入は、要約に記載されている通りです。これにより、認証ステータスのストリーミングが新しい方法で行われるようになります。

  • 16-16: GoTrueAuthに名前変更されたことが確認できます。これは要約に記載されている変更と一致しています。

  • 5-19: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [5-41]

セッションのリフレッシュ機能に関する記述は見当たりませんが、認証ステータスの取り扱いに関する変更が含まれている可能性があります。プルリクエストの目的と変更内容が一致しているか確認してください。

app/ios/Modules/Sources/Auth/ComposeLoginScreen.swift (1)
  • 15-21: 変更は適切に行われており、LoginScreenStateMachineの初期化にAuthStatusStreamUseCaseが使用されていることを確認してください。また、この変更により、LoginScreenStateMachine内の認証ステータスの観察と処理に関連するロジックが影響を受ける可能性があります。
app/ios/Modules/Sources/Auth/LoginStateMachine.swift (5)
  • 18-24: The replacement of observeAuthStatusUseCase with authStatusStream is confirmed. Ensure that all dependent code has been updated to use the new authStatusStream property and its methods.

  • 18-24: The LoginViewUIState structure now includes an authStatus property of type LoadingState<AuthStatusAuthenticated>. This change should be reflected in the summary to provide a complete overview of the changes made to the LoginStateMachine class.

  • 40-46: The load function now uses authStatusStream.execute() to set cachedAuthStatus. This change in the handling of authentication status should be included in the summary to accurately describe the behavioral changes in the LoginStateMachine class.

  • 40-46: The error handling within the load function has been updated to set self.state.authStatus to .failed(error). This change should be documented in the summary to provide a complete picture of the error handling improvements.

  • 40-46: The applyAuthStatusToState method now sets the event to .onLoginSuccess and updates state.authStatus based on cachedAuthStatus. This change should be included in the summary to accurately describe the behavioral changes in the LoginStateMachine class.

app/ios/Modules/Sources/KmpContainer/AuthStatusStreamUseCaseProvider.swift (1)
  • 4-27: このAuthStatusStreamUseCaseProvider構造体の実装は、セッションリフレッシュ機能を追加する目的に沿っているようです。
app/ios/Modules/Sources/Navigation/RootStateMachine.swift (2)
  • 24-30: 変更された依存関係プロパティ名と、authStatusStreamのメソッド使用方法がPRの目的と一致しています。また、loadTaskブロック内のロジックが簡素化され、authStatuscachedAuthStatusに直接割り当て、authStatusを出力するように変更されています。これらの変更は、セッション管理の改善に関連しているため、承認されます。

  • 49-57: loadTaskのロジックが変更され、authStatusStream.execute()からのauthStatuscachedAuthStatusに直接割り当てています。これは、セッションのリフレッシュ機能を追加するというPRの目的に沿っています。また、エラーが発生した場合には、state.authStatus.failedに設定しています。これにより、エラーハンドリングも適切に行われていることが確認できます。

app/ios/Modules/Sources/Settings/ComposeSettingsScreen.swift (1)
  • 11-17: 変更はPRの目的と一致しており、SettingsScreenStateMachineの初期化におけるobserveAuthStatusパラメータがauthStatusStreamに置き換えられています。これにより、設定画面内での認証ステータスの監視方法が変更された可能性があります。この変更により、SettingsScreenStateMachineを初期化する他のコードも更新する必要があるか確認してください。
app/shared/src/commonMain/kotlin/club/nito/app/shared/NitoAppStateMachine.kt (1)
  • 12-33: コードはPRの目的と一致しており、ObserveAuthStatusUseCaseからAuthStatusStreamUseCaseへの変更、およびFetchSingleResultからAuthStatusへの変更が適切に反映されています。ただし、AuthStatus.NetworkErrorのハンドリングがNitoAppUiState.Loadingを返している点については、TODOコメントがあるため、後で適切な処理を追加する必要があります。
app/shared/src/commonMain/kotlin/club/nito/app/shared/di/AppModule.kt (1)
  • 6-11: 変更はPRの目的と提供された要約と一致しており、NitoAppStateMachineのインスタンス化においてobserveAuthStatusからauthStatusStreamへのパラメータの変更が適切に行われています。依存するコードが新しいシグネチャに合わせて更新される必要があります。
core/data/src/commonMain/kotlin/club/nito/core/data/AuthRepository.kt (1)
  • 11-14: 変更されたauthStatusプロパティの型が、PRの目的と要約に記載されている内容と一致しています。この変更により、依存するコードが新しいシグネチャに適合するように更新される必要があります。
core/data/src/commonMain/kotlin/club/nito/core/data/DefaultAuthRepository.kt (1)
  • 11-11: 変更されたauthStatusプロパティの型は、セッションリフレッシュ機能の追加というプルリクエストの目的に合致しています。この変更により、認証ステータスの扱いが直接的になり、結果型でラップする必要がなくなりました。依存するコードが新しいシグネチャに適合するように更新される必要があります。
core/domain/src/commonMain/kotlin/club/nito/core/domain/AuthStatusStreamUseCase.kt (1)
  • 1-18: この変更は、認証状態のストリーミングを行う新しいメカニズムを導入しており、PRの目的と一致しています。AuthStatusStreamUseCaseAuthStatusStreamExecutorの実装は、コードベースの効率と保守性を向上させる可能性があります。依存する他のコンポーネントが新しいシグネチャと動作に適合するように更新される必要があります。
core/domain/src/commonMain/kotlin/club/nito/core/domain/di/UseCaseModule.kt (4)
  • 22-26: 変更はPRの目的に沿っており、AuthStatusStreamUseCaseの導入によりセッション管理が強化されています。

  • 1-7: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-26]

コードの構文やロジックに明らかな問題はありません。

  • 3-4: インポート文の追加は、要約に記載されている内容と一致しています。

  • 23-23: useCaseModuleのバインディングの変更は、要約に記載されている内容と一致しています。

core/model/src/commonMain/kotlin/club/nito/core/model/AuthState.kt (1)
  • 3-11: 変更はPRの目的と提供された要約と一致しており、AuthStatusインターフェースにLoadingNetworkErrorの状態が追加されたことは、依存するコンポーネントに影響を与える重要な変更です。これらの新しい状態を処理するために、関連する関数やコンポーネントの更新が必要になる可能性があります。
core/network/src/commonMain/kotlin/club/nito/core/network/NetworkService.kt (1)
  • 12-38: The NetworkService class and its functionality align with the PR objective of enhancing session management and abstracting network operations. The implementation of the invoke function and the toNitoError extension function are correctly handling authentication and exception mapping, respectively.
core/network/src/commonMain/kotlin/club/nito/core/network/auth/AuthRemoteDataSource.kt (1)
  • 7-14: 変更点はPRの目的と要約に沿っており、AuthRemoteDataSourceインターフェースのauthStatusプロパティの戻り値の型がFlow<FetchSingleResult<AuthStatus>>からFlow<AuthStatus>に変更され、新しいメソッドauthIfNeededが追加されています。これらの変更は、認証状態の取り扱いを直接的に行うようにするためのもので、問題は見受けられません。
core/network/src/commonMain/kotlin/club/nito/core/network/auth/FakeAuthRemoteDataSource.kt (3)
  • 15-29: 変更された_authStatusauthStatusの型は、PRの目的と提供された要約と一致しています。これにより、認証ステータスのストリーミングが直接行われるようになりました。

  • 61-67: loginおよびlogout関数がAuthStatusを直接発行するように変更されたことは、PRの目的と要約に記載されている内容と一致しています。

  • 75-77: 新しく追加されたauthIfNeeded関数は現在何も操作を行わないプレースホルダーとして存在しています。将来的な実装が予定されている場合は、その旨をコメントに記載すると良いでしょう。

core/network/src/commonMain/kotlin/club/nito/core/network/auth/SupabaseAuthRemoteDataSource.kt (2)
  • 3-37: 変更されたauthStatusプロパティの戻り値の型は、PRの目的と要約に一致しています。これにより、認証ステータスの直接的なストリームが可能になり、コードの効率と保守性が向上する可能性があります。

  • 51-57: 新しく追加されたauthIfNeededメソッドは、セッションの有効期限が切れている場合にセッションを更新する機能を提供します。これはセッションリフレッシュ機能を追加するというPRの目的に沿っています。

core/network/src/commonMain/kotlin/club/nito/core/network/di/RemoteDataSourceModule.kt (3)
  • 3-3: コードにNetworkServiceのインポートが追加されました。これはセッション管理の強化に関連しているため、変更は適切です。

  • 22-24: NetworkServiceのインスタンスがremoteDataSourceModuleに追加されています。これは新しいネットワーク操作に必要ですので、変更は適切です。

  • 27-42: SupabaseScheduleRemoteDataSourceSupabaseParticipantRemoteDataSource、およびSupabaseUserRemoteDataSourceの初期化にnetworkServiceパラメータが追加されています。これはPRの目的と要約に一致しており、変更は適切です。

core/network/src/commonMain/kotlin/club/nito/core/network/participation/SupabaseParticipantRemoteDataSource.kt (1)
  • 12-54: コードレビューを行いましたが、提供されたコード断片には問題は見つかりませんでした。ただし、プルリクエストの目的として「セッションリフレッシュ機能の追加」とされているにも関わらず、このコード断片にはそのような機能に関する記述が含まれていません。この点については、他のコード断片やコミットで対応されている可能性がありますので、全体のコンテキストを確認することをお勧めします。
core/network/src/commonMain/kotlin/club/nito/core/network/schedule/SupabaseScheduleRemoteDataSource.kt (3)
  • 44-48: getSchedule関数の戻り値の型が変更されたとの記述がありますが、hunkにはそのような変更は見受けられません。この点について確認が必要です。

  • 18-21: SupabaseScheduleRemoteDataSourceクラスが新たにNetworkServiceインスタンスを要求するようになりました。これは、セッションのリフレッシュ機能を追加するというPRの目的と一致しています。

  • 26-42: getScheduleList関数がNetworkServiceを使用するようにリファクタリングされました。これは、セッションのリフレッシュ機能を追加するというPRの目的と一致しています。

core/network/src/commonMain/kotlin/club/nito/core/network/user/SupabaseUserRemoteDataSource.kt (2)
  • 1-18: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [15-25]

コードレビューを行いましたが、getProfileメソッドの変更点に問題は見つかりませんでした。NetworkServiceを使用してネットワーク操作を行い、ドメインモデルに適切にマッピングしています。

  • 24-31: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [27-34]

getProfilesメソッドもレビューしましたが、こちらもNetworkServiceを使用しており、リストを適切にデコードしてドメインモデルにマッピングしているため、問題はありません。

feature/auth/src/commonMain/kotlin/club/nito/feature/auth/LoginScreenStateMachine.kt (4)
  • 3-3: インポートの変更は、PRの目的と要約に一致しています。AuthStatusStreamUseCaseの使用に更新されていることを確認してください。

  • 29-32: authStatus変数の初期化がAuthStatus.Loadingに変更されていることを確認してください。これは、セッションのリフレッシュ機能を追加するというPRの目的に沿っています。

  • 52-55: authStatusの条件ロジックがAuthStatus.Authenticatedに変更されていることを確認してください。これは、認証ステータスのストリーミングを新しい方法で扱うための変更です。

  • 18-35: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-57]

ファイル全体を確認しましたが、PRの目的に関連する他の変更は見つかりませんでした。

feature/auth/src/commonMain/kotlin/club/nito/feature/auth/di/AuthFeatureModule.kt (1)
  • 9-13: 変更は、LoginScreenStateMachineのファクトリ初期化において、パラメータ名observeAuthStatusUseCaseauthStatusStreamに変更することで、認証ステータスの観察または取得に関連するロジックの変更を反映しています。この変更は、PRの目的と提供された要約と一致しています。
feature/settings/src/commonMain/kotlin/club/nito/feature/settings/SettingsScreenStateMachine.kt (3)
  • 20-36: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [3-34]

コードレビューにおいて、SettingsScreenStateMachineクラスのコンストラクタがAuthStatusStreamUseCaseを受け取るように変更され、authStatusの初期値がAuthStatus.Loadingに設定されていることを確認しました。これはプルリクエストの目的であるセッションリフレッシュ機能の追加に関連しており、問題は見受けられません。

  • 50-55: uiStateのビルダーがAuthStatus.Loadingを使用してisSignOuting状態を設定するように変更されていることを確認しました。これは、FetchSingleResult.LoadingからAuthStatus.Loadingへの変更を反映しており、プルリクエストの目的に沿っています。

  • 61-67: initブロック内で、authStatusAuthStatus.NotAuthenticatedの場合にSignedOutイベントを発行するように変更されていることを確認しました。これは、イベント発行条件がAuthStatus.NotAuthenticatedのみをチェックするように単純化されたことを反映しており、プルリクエストの目的に適合しています。

feature/settings/src/commonMain/kotlin/club/nito/feature/settings/di/SettingsFeatureModule.kt (1)
  • 7-13: この変更は、SettingsScreenStateMachineのコンストラクター呼び出しでobserveAuthStatusからauthStatusStreamへのパラメータ名の変更を反映しています。これは、セッション管理を改善し、古いユースケースを新しいストリーミングアプローチに置き換えるためのより大きなリファクタリングの一部であることをPRの目的と変更の概要が示しています。この変更がコードベース全体で一貫していることを確認してください。

Comment on lines +5 to +6
private static var observeAuthStatusUseCase: AuthStatusStreamUseCase {
Container.shared.get(type: AuthStatusStreamUseCase.self)
Copy link

Choose a reason for hiding this comment

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

プロパティの名前が誤解を招く可能性があります。observeAuthStatusUseCaseAuthStatusStreamUseCaseのインスタンスを参照しているため、名前をauthStatusStreamUseCaseに変更することを検討してください。

- private static var observeAuthStatusUseCase: AuthStatusStreamUseCase {
+ private static var authStatusStreamUseCase: AuthStatusStreamUseCase {

Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
private static var observeAuthStatusUseCase: AuthStatusStreamUseCase {
Container.shared.get(type: AuthStatusStreamUseCase.self)
private static var authStatusStreamUseCase: AuthStatusStreamUseCase {
Container.shared.get(type: AuthStatusStreamUseCase.self)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant