[PC-1642] Profile 수정, 로그아웃, 탈퇴, 토큰 만료 시 로컬 MyProfile 데이터 clear 로직 추가 #205
Conversation
📝 WalkthroughWalkthroughAdds a repository API to clear local profile state, injects a local profile data source into auth flows, and invokes profile-clearing in logout/withdraw, profile update/upload success paths, and auth-error handlers to invalidate local profile cache. Changes
Sequence Diagram(s)sequenceDiagram
actor ViewModel
participant ProfileRepo as ProfileRepository
participant RemoteDS as RemoteProfileDataSource
participant LocalDS as LocalProfileDataSource
ViewModel->>ProfileRepo: updateProfile()/uploadProfile()
ProfileRepo->>RemoteDS: send update/upload request
RemoteDS-->>ProfileRepo: success
ProfileRepo->>LocalDS: clearMyProfile()
LocalDS-->>ProfileRepo: cleared
ProfileRepo->>ProfileRepo: loadMyProfile() (refresh)
ProfileRepo->>LocalDS: read profile
LocalDS-->>ProfileRepo: profile data
ProfileRepo-->>ViewModel: updated profile / COMPLETE
sequenceDiagram
actor User
participant AuthRepo as AuthRepositoryImpl
participant LocalDS as LocalProfileDataSource
participant OtherCleanup as OtherCleanupJobs
User->>AuthRepo: logout()/withdraw()
AuthRepo->>LocalDS: launch clearMyProfile()
AuthRepo->>OtherCleanup: launch other cleanup jobs (tokens, dialogs)
Note right of AuthRepo: coroutineScope waits for all launches
LocalDS-->>AuthRepo: cleared
OtherCleanup-->>AuthRepo: cleaned
AuthRepo-->>User: completed logout/withdraw
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
feature/profile/src/main/java/com/puzzle/profile/graph/register/RegisterProfileViewModel.kt (1)
267-355: Consider extracting common success/failure handling.
updateProfile()anduploadProfile()share nearly identicalonSuccessandonFailureblocks. Consider extracting the common handling logic into a private helper to reduce duplication.♻️ Example refactor
private suspend fun handleProfileSaveResult(result: Result<Unit>) { result.onSuccess { profileRepository.clearMyProfile() loadMyProfile() setState { copy(currentPage = RegisterProfileState.Page.COMPLETE) } }.onFailure { exception -> if (exception is HttpResponseException) { exception.msg?.let { message -> eventHelper.sendEvent(PieceEvent.ShowSnackBar(SnackBarState.TextOnly(message))) } setState { copy(currentPage = RegisterProfileState.Page.VALUE_TALK) } return@onFailure } errorHelper.sendError(exception) setState { copy(currentPage = RegisterProfileState.Page.VALUE_TALK) } } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/data/src/main/java/com/puzzle/data/repository/AuthRepositoryImpl.ktcore/data/src/main/java/com/puzzle/data/repository/ProfileRepositoryImpl.ktcore/domain/src/main/java/com/puzzle/domain/repository/ProfileRepository.ktfeature/profile/src/main/java/com/puzzle/profile/graph/register/RegisterProfileViewModel.ktpresentation/src/main/java/com/puzzle/presentation/MainViewModel.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
core/data/src/main/java/com/puzzle/data/repository/AuthRepositoryImpl.kt (3)
3-3: LGTM!The
LocalProfileDataSourcedependency is correctly injected following the existing pattern, enabling profile data cleanup in authentication flows.Also applies to: 21-21
49-61: LGTM!The
clearMyProfileJobfollows the established parallel cleanup pattern consistently with other cleanup operations inlogout(). This ensures profile data is cleared alongside tokens and user role data.
63-77: LGTM!The
withdraw()flow correctly mirrors thelogout()implementation, ensuring profile data is cleared during account withdrawal.core/domain/src/main/java/com/puzzle/domain/repository/ProfileRepository.kt (1)
75-76: LGTM!The
clearMyProfile()interface method is a clean addition that aligns with the repository's responsibility for profile data management.feature/profile/src/main/java/com/puzzle/profile/graph/register/RegisterProfileViewModel.kt (2)
294-297: LGTM!The cache invalidation sequence (clear → load → update state) correctly ensures fresh profile data is displayed after an update.
339-342: LGTM!Cache invalidation is correctly applied in the upload flow, consistent with the update flow.
core/data/src/main/java/com/puzzle/data/repository/ProfileRepositoryImpl.kt (1)
255-258: LGTM!Clean delegation to the local data source, consistent with the repository's pattern for other profile operations.
presentation/src/main/java/com/puzzle/presentation/MainViewModel.kt (2)
84-91: LGTM!The parallel cleanup pattern correctly clears both the dialog state and profile data before navigation when the refresh token expires.
coroutineScopeensures both operations complete before proceeding.
153-160: LGTM!Consistent application of the parallel cleanup pattern for the unauthorized/invalid token error path, ensuring profile data is cleared before redirecting to onboarding.
There was a problem hiding this comment.
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)
core/data/src/test/java/com/puzzle/data/repository/AuthRepositoryImplTest.kt (1)
25-37:localProfileDataSourceis never initialized—tests will crash.The
localProfileDataSourcefield is declared aslateinit varbut is never assigned before being passed to theAuthRepositoryImplconstructor. This will cause all tests to fail withUninitializedPropertyAccessException.🐛 Proposed fix
@BeforeEach fun setUp() { authDataSource = FakeAuthDataSource() notificationDataSource = SpyNotificationDataSource() localTokenDataSource = FakeLocalTokenDataSource() localUserDataSource = FakeLocalUserDataSource() + localProfileDataSource = FakeLocalProfileDataSource() authRepository = AuthRepositoryImpl( authDataSource = authDataSource, notificationDataSource = notificationDataSource, localTokenDataSource = localTokenDataSource, localUserDataSource = localUserDataSource, localProfileDataSource = localProfileDataSource ) }
🧹 Nitpick comments (1)
core/data/src/test/java/com/puzzle/data/repository/AuthRepositoryImplTest.kt (1)
62-90: Tests should verify that profile data is cleared on logout and withdraw.Per the PR objectives, local profile data should be cleared on logout and withdrawal. These tests verify token and user data are cleared but don't assert that
localProfileDataSourcewas also cleared.♻️ Suggested additions
Add an assertion to verify profile clearing in both tests. The exact assertion depends on
FakeLocalProfileDataSource's API, but something like:// In `로그아웃을 할 경우 로컬에 저장된 유저 데이터를 모두 제거한다` assertTrue(localProfileDataSource.myProfile.first() == null) // or equivalent // In `회원탈퇴를 할 경우 로컬에 저장된 유저 데이터를 모두 제거한다` assertTrue(localProfileDataSource.myProfile.first() == null) // or equivalent
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/data/src/test/java/com/puzzle/data/repository/AuthRepositoryImplTest.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/data/src/test/java/com/puzzle/data/repository/AuthRepositoryImplTest.kt (2)
79-93: Same test setup issue: profile data not populated before withdrawal.Same concern as the logout test—the assertion passes trivially because profile data was never set up. Populate profile data in the "given" section to make this test meaningful.
63-77: Test does not effectively verify the clearing behavior.The test asserts
myValueTalksis null or empty after logout, but the "given" section never populates profile data. This test would pass even ifclearMyProfile()wasn't called, sincemyValueTalksstarts as null.Consider populating profile data before logout to verify the clearing logic:
💡 Suggested improvement
@Test fun `로그아웃을 할 경우 로컬에 저장된 유저 데이터를 모두 제거한다`() = runTest { // given authRepository.loginOauth(OAuthProvider.KAKAO, "OAuthClientToken") + localProfileDataSource.setMyValueTalks(listOf( + MyValueTalk(1, 1, "category", "title", "answer", "summary", emptyList(), "placeholder") + )) // when authRepository.logout() // then assertTrue(localTokenDataSource.accessToken.first().isEmpty()) assertTrue(localTokenDataSource.refreshToken.first().isEmpty()) assertTrue(localUserDataSource.userRole.first().isEmpty()) assertTrue(localUserDataSource.lastMatchingPoolEmptyDialogShownDate.first().isEmpty()) assertTrue(localProfileDataSource.myValueTalks.first().isNullOrEmpty()) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/data/src/test/java/com/puzzle/data/repository/AuthRepositoryImplTest.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| clearTokenJob.join() | ||
| clearLastMatchingPoolEmptyDialogShownDateJob.join() | ||
| clearUserRoleJob.join() | ||
| clearMyProfileJob.join() |
| clearTokenJob.join() | ||
| clearLastMatchingPoolEmptyDialogShownDateJob.join() | ||
| clearUserRoleJob.join() | ||
| clearMyProfileJob.join() |
There was a problem hiding this comment.
위에꺼 포함해서 리팩토링 때 한 번에 하려고 하긴 했는데 이번 pr 관련은 미리 하겠습니다
=> 3ec13b7
| }, | ||
| ) | ||
| }.onSuccess { | ||
| profileRepository.clearMyProfile() |
There was a problem hiding this comment.
플로우를 잘 모르겠어서 질문드리는데 uploadProfile 성공 시 clearMyProfile 하고있는데 이건 api와 로컬인가요? 어떤관계인지??
There was a problem hiding this comment.
기존 유저의 가치관 픽, 톡은 데이터스토어로 캐싱을 합니다.
그리고 캐싱이 되어있지 않은 상태에서 load를 해야한다면 api 호출 -> 데이터 스토어 저장 -> 데이터 스토어 값 load 입니다.
업로드 프로필(PUT)을 하면 서버에 변경 요청을 하지만 기존에 데이터 스토어 값이 그대로 남아있어 다른 곳에서 load할 때 이전 값을 사용합니다.
그래서 추가적으로 토큰 만료, 로그아웃, 탈퇴 시 clear를 해줘야 서버의 데이터 동기화가 가능합니다.
로컬 캐싱 관련 로직 미리 봐두시면 좋을 거 같네요
| launch { | ||
| profileRepository.clearMyProfile() | ||
| } | ||
| } |
There was a problem hiding this comment.
coroutineScope 내부 코루틴이 실패하면 뷰모델스콮까지 취소되어 navigate 가 수행되지 않을텐데, 물론 그럴 확률은 적겠지만 clear가 그정도로 크리티컬한가 싶어요.
There was a problem hiding this comment.
실패 또는 취소되는 경우를 대비해 로그를 남기고, 해당 로직은 함수로 분리했습니다.
clear가 그정도로 크리티컬한가 싶어요.
위의 질문이 clear 로직이 꼭 필요한지에 대한 질문이라면, 필요합니다.
clear 처리를 하지 않을 경우, A계정에서 세션 또는 토큰 만료로 인해 로그인 화면으로 이동한 뒤 B 계정으로 다시 로그인했을 때,
로컬에 남아 있는 A 계정의 가치관 톡/픽 데이터가 B 계정 화면에 표출됩니다.
=> d55eb92
| launch { | ||
| profileRepository.clearMyProfile() | ||
| } | ||
| } |
There was a problem hiding this comment.
여기도 비슷한생각입니다. 글고 위에랑 중복로직인것 같은데 따로 분리하는것도 좋아보이네요
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt (1)
96-101: Consider removing or deleting the orphanedBACKGROUND_CHANNEL.
BACKGROUND_CHANNELis now unused dead code. Additionally, users who had the app previously will still have the old "백그라운드 알림" channel registered on their devices, which could cause confusion in notification settings.Consider:
- Removing the unused constant if it's no longer needed.
- Adding logic to delete the old channel during migration:
♻️ Suggested cleanup for old channel
fun createNotificationChannel(context: Context) { val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager + // Delete legacy channel if it exists + notificationManager.deleteNotificationChannel(BACKGROUND_CHANNEL) + val channel = NotificationChannel( NOTICE_CHANNEL_ID, NOTICE_CHANNEL_NAME, NotificationManager.IMPORTANCE_HIGH ).apply { description = "서비스의 중요한 공지와 알림을 전달합니다." setShowBadge(true) } notificationManager.createNotificationChannel(channel) }And remove the unused constant:
companion object { - // 기존 ID - private const val BACKGROUND_CHANNEL = "백그라운드 알림" - // 중요 알림 private const val NOTICE_CHANNEL_ID = "piece_official_notice" private const val NOTICE_CHANNEL_NAME = "공지 및 주요 알림"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-10T10:27:20.895Z
Learnt from: comst19
Repo: Piece-Puzzly/Piece-Android PR: 203
File: feature/setting/src/main/java/com/puzzle/setting/graph/main/SettingScreen.kt:242-246
Timestamp: 2026-01-10T10:27:20.895Z
Learning: In the Piece-Android project's SettingScreen.kt and AccessRightsPage.kt: The notification toggle represents the actual notification receivable state (combining system permission and app setting), and when system permission becomes granted, the app automatically enables push notifications through onPushNotificationCheckedChange() callback - this is intentional behavior per the app's notification policy.
Applied to files:
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
app/src/main/java/com/puzzle/piece/notification/NotificationService.kt (2)
103-117: Channel configuration is appropriate for high-priority notifications.The
IMPORTANCE_HIGHsetting combined withsetShowBadge(true)and descriptive Korean text is well-configured for official notices.
75-83: Notification channel initialization is correct.The
createNotificationChannel()call inApplication.onCreate()ensures the newNOTICE_CHANNEL_IDis created early in the app lifecycle, before FCM messages arrive. The switch toPRIORITY_HIGHandDEFAULT_ALLis appropriate for important notifications.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
presentation/src/main/java/com/puzzle/presentation/MainViewModel.kt (1)
166-173: Consider separating clearing operations to ensure both execute.Currently, if
clearLastMatchingPoolEmptyDialogShownDate()throws,clearMyProfile()will never be called. Both clearing operations should be attempted independently to ensure complete cleanup on logout/token expiry.♻️ Suggested refactor
private fun clearAndNavigateToAuth() = viewModelScope.launch { suspendRunCatching { userRepository.clearLastMatchingPoolEmptyDialogShownDate() + }.onFailure { errorHelper.recordError(it) } + + suspendRunCatching { profileRepository.clearMyProfile() }.onFailure { errorHelper.recordError(it) } navigationHelper.navigate(TopLevelTo(AuthGraphBaseRoute)) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
presentation/src/main/java/com/puzzle/presentation/MainViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
presentation/src/main/java/com/puzzle/presentation/MainViewModel.kt (1)
core/common/src/main/java/com/puzzle/common/ResultUtil.kt (1)
suspendRunCatching(5-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
presentation/src/main/java/com/puzzle/presentation/MainViewModel.kt (2)
81-81: LGTM!Clean delegation to the centralized function. Fire-and-forget pattern is appropriate here since
handleHttpErroris a non-suspend callback.
138-139: LGTM!Correctly delegates to the centralized cleanup function and returns early.
1. ⭐️ 변경된 내용
2. 🖼️ 스크린샷(선택)
3. 💡 알게된 부분
4. 📌 이 부분은 꼭 봐주세요!
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Notifications
✏️ Tip: You can customize this high-level summary in your review settings.