[feat/#36] logout, 회원탈퇴, CICD Error #37
Conversation
WalkthroughThis PR introduces logout and account withdrawal functionality by adding new authentication DTOs, service endpoints, repository methods, view model functions, and UI dialogs with confirmation flows across the authentication and profile layers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as MypageScreen
participant VM as MypageViewModel
participant Repo as AuthRepository
participant Service as AuthService
participant API as Backend API
participant Store as TokenDataStore
User->>UI: Click logout/withdraw button
UI->>UI: Show ConfirmDialog
User->>UI: Confirm action
UI->>VM: Call logout() or withdraw()
VM->>VM: Set isAuthLoading = true
VM->>Repo: Call logout() or withdraw()
Repo->>Store: Read refresh token
alt Token exists
Repo->>Service: Call logout/withdraw with token
Service->>API: POST/DELETE request
API-->>Service: Response
alt Success
Repo->>Store: Clear tokens
Repo-->>VM: Return Result.success()
VM->>VM: Set logoutSuccess/withdrawSuccess = true
VM->>VM: Set isAuthLoading = false
VM-->>UI: Success state emitted
UI->>UI: Navigate to login
else Failure
VM->>VM: Set error message
VM->>VM: Set isAuthLoading = false
VM-->>UI: Error state emitted
end
else No token
Repo-->>VM: Return Result.failure()
VM->>VM: Set isAuthLoading = false
VM-->>UI: Error state emitted
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.idea/caches/deviceStreaming.xml (1)
1-1002: Remove IDE cache file from version control.IDE configuration and cache files (
.idea/caches/deviceStreaming.xml) should not be committed to the repository. These are auto-generated by IntelliJ IDEA and are user-specific. Additionally, the device preset changes in this file are unrelated to the PR objectives (logout, withdrawal, CICD fixes) and have no impact on the authentication features being implemented.Action items:
- Remove this file from the commit.
- Ensure
.idea/is in your.gitignoreif not already present:.idea/ *.iml- If already ignored, ensure your git cache is clean:
git rm --cached .idea/(and commit separately if needed).
🧹 Nitpick comments (3)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/ConfirmDialog.kt (3)
21-22: Remove unused imports.
HsLinkButtonSizeandHsLinkSelectButtonare imported but never used in this file.Apply this diff:
-import com.hsLink.hslink.core.designsystem.component.HsLinkButtonSize -import com.hsLink.hslink.core.designsystem.component.HsLinkSelectButton
53-53: Consider using theme color instead of hardcoded value.The dialog background uses
Color.Whitedirectly. Consider using a theme color for consistency and to support potential theme variations.- containerColor = Color.White, // ← 완전 하얀색 배경 + containerColor = HsLinkTheme.colors.Common,
75-75: Fixed lineHeight in sp may not scale optimally.Using
spforlineHeightcreates a fixed value that may not scale proportionally with different font sizes. Consider using a relative multiplier oremunit for better text scaling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/ci-cd.yml(2 hunks).idea/caches/deviceStreaming.xml(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/auth/LogoutRequestDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/auth/WithdrawRequestDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/auth/WithdrawResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/AuthRepositoryImpl.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/service/login/AuthService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/domain/repository/AuthRepository.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/ConfirmDialog.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt(5 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/viewmodel/MypageViewModel.kt(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/ConfirmDialog.kt (1)
ConfirmDialog(41-106)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/ConfirmDialog.kt (2)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkActionButton.kt (1)
HsLinkActionButton(37-93)
⏰ 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-and-test
🔇 Additional comments (7)
.github/workflows/ci-cd.yml (1)
45-46: LGTM! Environment variable formatting corrected.The indentation adjustments ensure proper YAML syntax for environment variables. No functional changes to the workflow.
Also applies to: 99-99
app/src/main/java/com/hsLink/hslink/data/dto/response/auth/WithdrawResponseDto.kt (1)
1-10: LGTM! Clean DTO structure.The
WithdrawResponseDtois properly structured with appropriate types and serialization support.app/src/main/java/com/hsLink/hslink/data/dto/request/auth/LogoutRequestDto.kt (1)
1-9: LGTM! Clean request DTO.The
LogoutRequestDtofollows standard patterns and is properly annotated for serialization.app/src/main/java/com/hsLink/hslink/data/dto/request/auth/WithdrawRequestDto.kt (1)
1-9: LGTM! Clean request DTO.The
WithdrawRequestDtois properly structured with serialization support. While identical toLogoutRequestDto, maintaining separate DTOs provides better type safety and intent clarity.app/src/main/java/com/hsLink/hslink/domain/repository/AuthRepository.kt (1)
4-4: LGTM! Clean interface additions.The new
logout()andwithdraw()methods follow the existing pattern with proper suspend modifiers and Result type wrappers.Also applies to: 12-13
app/src/main/java/com/hsLink/hslink/data/service/login/AuthService.kt (1)
18-22: Verify response wrapping inconsistency.The
logoutendpoint returnsResponse<Unit>whilewithdrawreturnsResponse<BaseResponse<WithdrawResponseDto>>. This wrapping inconsistency suggests different backend response structures. Ensure this matches the actual API contract.app/src/main/java/com/hsLink/hslink/data/repositoryimpl/AuthRepositoryImpl.kt (1)
41-60: LGTM! Well-structured logout implementation.The logout flow properly validates token presence, handles API responses, clears local tokens on success, and includes appropriate error handling.
| override suspend fun withdraw(): Result<WithdrawResponseDto> { | ||
| return try { | ||
| val refreshToken = tokenDataStore.refreshToken.first() // ← 수정 | ||
| if (refreshToken.isNullOrEmpty()) { | ||
| return Result.failure(Exception("토큰이 존재하지 않습니다")) | ||
| } | ||
|
|
||
| val request = WithdrawRequestDto(refreshToken) | ||
| val response = authService.withdraw(request) | ||
|
|
||
| if (response.isSuccessful && response.body()?.isSuccess == true) { | ||
| tokenDataStore.clearTokens() | ||
| Result.success(response.body()!!.result) | ||
| } else { | ||
| Result.failure(Exception(response.body()?.message ?: "계정 탈퇴에 실패했습니다")) | ||
| } | ||
| } catch (e: Exception) { | ||
| Result.failure(e) | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential NPE risk with !! operator.
Line 74 uses the !! operator on response.body(), which could throw a NullPointerException even after checking isSuccessful and isSuccess. Although unlikely, HTTP responses can have null bodies in edge cases.
Apply this diff to handle the null case safely:
- if (response.isSuccessful && response.body()?.isSuccess == true) {
+ val body = response.body()
+ if (response.isSuccessful && body?.isSuccess == true && body.result != null) {
tokenDataStore.clearTokens()
- Result.success(response.body()!!.result)
+ Result.success(body.result)
} else {
- Result.failure(Exception(response.body()?.message ?: "계정 탈퇴에 실패했습니다"))
+ Result.failure(Exception(body?.message ?: "계정 탈퇴에 실패했습니다"))
}🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/repositoryimpl/AuthRepositoryImpl.kt
around lines 62 to 81, the code uses response.body()!! at line 74 which can
throw an NPE even after basic success checks; change the logic to safely handle
a null body by first capturing val body = response.body() and then if
(response.isSuccessful && body?.isSuccess == true) return
Result.success(body.result) else return Result.failure(Exception(body?.message
?: "계정 탈퇴에 실패했습니다")); remove the use of !! and ensure every access to body is
null-checked, returning a failure with a clear message when body is null.
| if (logoutSuccess || withdrawSuccess) { | ||
| navController.navigateToLogin( | ||
| navOptions = NavOptions.Builder() | ||
| .setPopUpTo(0, inclusive = true) // ← 모든 백스택 클리어 | ||
| .build() | ||
| ) |
There was a problem hiding this comment.
Fix back stack clearing when navigating to Login
setPopUpTo(0, inclusive = true) never finds a destination with id 0, so nothing is removed from the back stack. After logging out or withdrawing, the user can press Back and land on the previous protected screen, effectively bypassing logout. Point popUpTo at a real destination (e.g., the graph’s start destination) so the entire stack is cleared.
navController.navigateToLogin(
navOptions = NavOptions.Builder()
- .setPopUpTo(0, inclusive = true) // ← 모든 백스택 클리어
- .build()
+ .setPopUpTo(navController.graph.startDestinationId, inclusive = true)
+ .setLaunchSingleTop(true)
+ .build()
)📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (logoutSuccess || withdrawSuccess) { | |
| navController.navigateToLogin( | |
| navOptions = NavOptions.Builder() | |
| .setPopUpTo(0, inclusive = true) // ← 모든 백스택 클리어 | |
| .build() | |
| ) | |
| if (logoutSuccess || withdrawSuccess) { | |
| navController.navigateToLogin( | |
| navOptions = NavOptions.Builder() | |
| .setPopUpTo(navController.graph.startDestinationId, inclusive = true) | |
| .setLaunchSingleTop(true) | |
| .build() | |
| ) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt
around lines 102 to 107, the call uses setPopUpTo(0, inclusive = true) which
never matches any destination id so the back stack isn’t cleared; change the
popUpTo target to a real destination (for example the nav graph’s start
destination id or the login route id) and keep inclusive = true so all entries
are removed (e.g. use navController.graph.startDestinationId or the login
destination id as the popUpTo value).
vvan2
changed the title
ISSUE
❗ WORK DESCRIPTIONAdd commentMore actions
📸 SCREENSHOT
2025-11-13.18.23.08.mov
📢 TO REVIEWERS
Summary by CodeRabbit