Conversation
# Conflicts: # feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt # feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt
|
Caution Review failedThe pull request is closed. Walkthrough카카오·네이버 소셜 로그인 SDK 초기화 및 로그인 흐름 추가, 토큰 DataStore 제공자 리네임·파일 경로 변경, 인증 인터셉터 추가, API 응답 모델 필드명 변경(data→result, success→isSuccess), 로그인·사용자 도메인·데이터·온보딩 네비게이션 및 UI/뷰모델 추가·재구성. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LandingScreen
participant LandingViewModel
participant KakaoSDK
participant NidOAuth
participant TokenRepository
participant LoginService
participant DataStore
User->>LandingScreen: 카카오/네이버 로그인 버튼 클릭
LandingScreen->>LandingViewModel: startSocialLogin(type, activity)
alt Kakao 흐름
LandingViewModel->>KakaoSDK: 로그인 시작
KakaoSDK-->>LandingViewModel: accessToken
LandingViewModel->>TokenRepository: finishSocialLogin(KAKAO, accessToken)
else Naver 흐름
LandingViewModel->>NidOAuth: OAuth 시작
NidOAuth-->>LandingViewModel: accessToken
LandingViewModel->>TokenRepository: finishSocialLogin(NAVER, accessToken)
end
TokenRepository->>LoginService: getCallBackToken(type, accessToken)
LoginService-->>TokenRepository: BaseResponse<TokenResponseDto>
TokenRepository->>DataStore: accessToken/refreshToken 저장
LandingViewModel-->>LandingScreen: LoginSucceeded 이벤트 방출
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분 검토 시 우선 확인할 항목:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/GageBar.kt (1)
37-47: Preview 함수명 일관성 업데이트 필요검색 결과, 코드베이스의 다른 곳에서 이전 함수명
GaugeBar에 대한 호출이 없으므로 breaking change는 제대로 관리되었습니다.다만 다음 사항이 확인되었습니다:
Preview 함수명 불일치:
- Line 78:
fun Preview_GaugeBar_15()- Line 91:
fun Preview_GaugeBar_75()이 preview 함수들의 이름이 여전히 구 이름인 "GaugeBar"를 포함하고 있으나, 내부에서는 새 이름
GageBar를 올바르게 호출합니다. 일관성을 위해 함수명을Preview_GageBar_15(),Preview_GageBar_75()로 업데이트해주세요.철자 선택: 표준 영문 철자 "Gauge"를 "Gage"로 변경한 특별한 이유가 있는지 여전히 불명확합니다. 한국어 주석의 "게이지 바"와도 표준 철자 "Gauge"가 더 일치합니다.
🧹 Nitpick comments (34)
feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyFilterScreen.kt (1)
249-261: StudyTheme enum 이름 변경에 맞춘 아이콘 매핑은 적절하지만 리소스명 정리는 여지가 있습니다
LICENSE → CERTIFICATION,EMPLOYMENT → CAREER등 enum 이름 변경에 맞춰 각 테마별 아이콘 매핑을 잘 맞춰 주셨습니다. 기능적인 문제는 없어 보입니다. 다만 drawable 이름은 여전히license,employment,discussion,news등 옛 명칭을 사용하고 있어, 나중에 enum/리소스 이름을 일관되게 맞춰 두면 유지보수 시 혼동을 줄이는 데 도움이 될 것 같습니다.domain/user/src/main/java/com/umcspot/spot/user/model/UserTheme.kt (1)
1-7: 요청 바디 모델 설계는 적절하나, 중복/순서 정책은 상위에서 한 번 확인 필요
StudyThemeRequestBody(categories: List<StudyTheme>)구조 자체는 API 요청 바디로 깔끔합니다.
다만List그대로 사용하면 같은 테마가 중복 들어갈 수 있으니,
- ViewModel/UseCase 단에서
Set으로 관리 후List로 변환해서 넣는지,- 혹은 서버가 중복을 허용/무시하는지
한 번만 정리해 두면 이후 유지보수에 도움이 될 것 같습니다.data/user/proguard-rules.pro (1)
1-21: 현재는 스캐폴드 상태로 충분하지만, 추후 네트워크/직렬화 룰 추가를 고려해 보세요지금은 전부 주석 처리된 템플릿이라 릴리즈 빌드에 영향은 없습니다.
향후에
- Retrofit/OkHttp DTO,
@Serializable/Gson/Moshi 모델,- Hilt/DI 관련 클래스
등이 난독화로 깨질 수 있으니, 실제 크래시나 릴리즈 이슈가 보이면 이 파일에 모듈 전용-keep룰을 추가하는 방향이 좋겠습니다.core/model/src/main/java/com/umcspot/spot/model/Global.kt (2)
51-60: StudyTheme enum 이름 변경에 따른 직렬화/호환성 영향 점검 필요
LICENSE → CERTIFICATION,EMPLOYMENT → CAREER등 enum 상수명이 변경되었는데,
- 앱 ↔ 서버 통신에서 enum
name을 그대로 직렬화하거나- 로컬 DB(예: Room)에
name기반으로 저장하고 있다면
기존 데이터와의 호환성 문제가 생길 수 있습니다.현재 어디에서 어떤 방식으로
StudyTheme를 직렬화/저장하는지 한 번 점검하시고,
- 필요하면
@SerialName등 명시적 매핑,- 혹은 서버/DB 마이그레이션
전략을 맞춰 두는 것을 추천합니다.
62-66: SocialLoginType의 문자열 값이 백엔드 계약과 확실히 일치하는지 확인 권장
SocialLoginType에서
KAKAO("kakao")NAVER("naver")
처럼 소문자 문자열을 사용하고 있는데, 이 값이 실제 소셜 로그인 API/백엔드가 기대하는 provider 코드와 정확히 맞아야 합니다.특히 PR 설명에 “카카오/네이버 각각에서 AccessToken 발급 시 계정이 2개 생성된 것 같다”는 내용이 있어,
- 같은 이메일/식별자라도 provider 코드 차이로 별도 계정이 생성되는 것인지,
- 기존 계정 통합 규칙과 일치하는지
백엔드 쪽 계약을 한 번 더 확인해 보시면 좋겠습니다.build-logic/convention/src/main/java/com/umcspot/spot/convention/BuildConfig.kt (1)
10-50: 필수 환경 변수 누락 시 빈 문자열로 빌드되는 부분은 한 번 고민해 볼 만합니다
BASE_URL,KAKAO_NATIVE_KEY,NAVER_CLIENT_ID,NAVER_CLIENT_SECRET를
프로퍼티 미존재 시""로 대체해BuildConfig에 넣고 있는데, 이 경우:
- 빌드는 성공하지만
- 런타임에서만 “왜 요청이 안 가지?” 같은 형태로 문제를 발견하게 되어
원인 추적이 어려울 수 있습니다.예를 들어 아래처럼 필수 키에 대해서는 빌드 타임에 바로 실패시키거나, 최소한 워닝을 남기는 것도 고려해 볼 수 있습니다.
fun Project.requireProperty(name: String): String = gradleLocalProperties(rootDir, providers).getProperty(name) ?: error("Missing required property: $name") val baseUrl = requireProperty("BASE_URL") val kakaoNativeKey = requireProperty("KAKAO_NATIVE_KEY") // NAVER 키도 동일하게 처리현재 구현도 동작에는 문제 없으나, CI/배포 안정성을 위해 이런 쪽 강화도 좋겠습니다.
feature/signup/src/main/java/com/umcspot/spot/landing/navigation/SavingNavigation.kt (1)
12-29: 네비게이션 패턴이 기존 구조와 잘 맞습니다
navigate(Saving, navOptions)+composable<Saving>조합으로 Route 기반 네비게이션 패턴을 잘 맞춰주신 것 같습니다.
SavingScreen의autoProgress,autoDurationMs,blockBackPress는 기본값에 의존하므로,
- 추후 테스트/디버깅용으로 수동 제어가 필요해지면
savingGraph파라미터에 옵션을 더 받는 방향도 쉽게 확장 가능해 보입니다.feature/signup/src/main/java/com/umcspot/spot/landing/SavingScreen.kt (1)
40-113: 로딩 애니메이션 + 완료 상태 전환 플로우가 자연스럽게 구성되어 있습니다
Animatable로 게이지를 채우고, 완료 후 3초 대기 뒤onFinished()를 호출하는 흐름이 직관적입니다.BackHandler(blockBackPress)로 뒤로가기를 제어하는 것도 온보딩 플로우와 잘 맞습니다.GageBar에internal.value를 그대로 연결해 시각적 진행 상태도 잘 반영되고 있습니다.추가로 고려해볼 만한 정도의 부분은:
- 3000ms 지연, 1800ms duration 같은 매직 넘버를 상수로 분리하면 나중에 튜닝/AB 테스트 시 조금 더 편리할 수 있습니다.
현재 상태로도 기능상 문제는 없어 보입니다.feature/signup/src/main/java/com/umcspot/spot/checkList/CheckListViewModel.kt (1)
18-44: 관심 테마 토글/저장 구조는 좋고, 키 상수/에러 처리 쪽만 약간 다듬으면 더 안정적일 것 같습니다좋은 점:
MutableStateFlow+LinkedHashSet으로 선택 순서를 유지하면서 중복 없이 토글하는 구조가 깔끔합니다.SavedStateHandle에List<StudyTheme>로 저장해 프로세스 재생성에도 선택 상태를 복원하는 흐름도 잘 구성되어 있습니다.submitThemes()에서 비어 있으면 early-return 하는 것도 불필요한 API 호출을 막아줘 좋습니다.개선 제안:
KEY_THEMES는 인스턴스 프로퍼티 대신companion object의const val로 올리면 재사용성과 의도를 더 분명히 할 수 있습니다.@HiltViewModel class CheckListViewModel @Inject constructor( private val savedStateHandle: SavedStateHandle, private val userRepository: UserRepository ) : ViewModel() { companion object { private const val KEY_THEMES = "recruit_filter_themes" } ... }
submitThemes()에서setUserTheme실패 시 UI에 알려줄 수 있는UiState등을 두면, 네트워크 오류 상황에서 사용자 피드백을 주기 쉬워집니다 (현재는 조용히 실패할 가능성).전체적으로 구조는 잘 잡혀 있고, 위 두 부분은 추후 개선 포인트 정도로 보입니다.
app/src/main/java/com/umcspot/spot/SpotApplication.kt (1)
4-8: Kakao/Naver SDK 초기화 위치는 적절하나, 클라이언트 시크릿 노출 리스크는 고려 필요Application.onCreate에서 KakaoSdk와 NidOAuth를 한 번만 초기화한 점은 구조상 적절해 보입니다. 다만
NAVER_CLIENT_SECRET을 포함한 키들이 BuildConfig를 통해 앱 바이너리에 들어가기 때문에, 리버스 엔지니어링 시 노출될 수 있는 리스크는 여전히 존재합니다. 장기적으로는 서버에서 토큰 교환을 대리 수행하고 앱에는 주로 clientId만 두는 구조로 전환하는 것도 검토해 보시면 좋겠습니다.Also applies to: 16-17
data/user/src/main/java/com/umcspot/spot/user/datasource/UserDataSource.kt (1)
9-9: 사용되지 않는 import 제거 권장
UserThemeResponseDtoimport가 이 인터페이스에서 사용되지 않습니다. 코드 정리를 위해 제거를 고려하세요.data/login/src/main/java/com/umcspot/spot/login/datasource/LoginDataSource.kt (1)
4-4: 타입 안정성 개선 권장
SocialLoginType을 import하고 있지만finishSocialLogin메서드의type파라미터는String을 사용합니다. 타입 안정성을 위해String대신SocialLoginType열거형을 사용하는 것을 고려하세요.다음과 같이 변경을 고려하세요:
- suspend fun finishSocialLogin(type : String, accessToken : String): BaseResponse<TokenResponseDto> + suspend fun finishSocialLogin(type : SocialLoginType, accessToken : String): BaseResponse<TokenResponseDto>단, 백엔드 API가 문자열을 요구하는 경우, 서비스 계층이나 매퍼에서 변환을 수행할 수 있습니다.
Also applies to: 8-8
data/login/src/main/java/com/umcspot/spot/login/di/LoginServiceModule.kt (1)
16-17: 메서드 이름 일관성 개선 제안메서드 이름이
provideOAuthApi이지만 반환 타입은LoginService입니다. 일관성을 위해provideLoginService로 이름을 변경하는 것이 더 명확할 수 있습니다.- fun provideOAuthApi(retrofit: Retrofit): LoginService = + fun provideLoginService(retrofit: Retrofit): LoginService = retrofit.create(LoginService::class.java)domain/user/src/main/java/com/umcspot/spot/user/repository/UserRepository.kt (2)
5-5: 사용되지 않는 import 문
UserTheme이 import되었지만 인터페이스에서 사용되지 않습니다. 인터페이스는StudyTheme만 사용합니다.다음 diff를 적용하여 사용되지 않는 import를 제거하세요:
import com.umcspot.spot.model.StudyTheme import com.umcspot.spot.user.model.UserResult -import com.umcspot.spot.user.model.UserTheme
9-9: 제네릭한 반환 타입 개선 권장
setUserName의 반환 타입이Result<Any?>로 되어 있어 타입 안정성이 떨어집니다. 사용자 이름 설정은 일반적으로 성공/실패만 반환하므로Result<Unit>이 더 적절합니다.다음 diff를 적용하여 반환 타입을 명확하게 하세요:
- suspend fun setUserName(name : String) : Result<Any?> + suspend fun setUserName(name : String) : Result<Unit>data/user/src/main/java/com/umcspot/spot/user/di/UserDataModule.kt (1)
11-11: 불필요한 공백 라인Line 11에 불필요한 공백 라인이 있습니다.
공백 라인을 제거하세요:
import javax.inject.Singleton - @Moduledata/user/src/main/java/com/umcspot/spot/user/dto/response/UserResponseDto.kt (1)
4-4: 사용되지 않는 import 문
WeatherType이 import되었지만 이 DTO에서 사용되지 않습니다.다음 diff를 적용하여 사용되지 않는 import를 제거하세요:
import android.annotation.SuppressLint -import com.umcspot.spot.model.WeatherType import kotlinx.serialization.SerialNameapp/src/main/AndroidManifest.xml (1)
35-47: Kakao 앱 키를 BuildConfig로 관리하는 것을 고려하세요.Line 45에서 Kakao 스킴에 앱 키가 하드코딩되어 있습니다. 다양한 환경(개발/스테이징/프로덕션)을 관리하고 보안을 강화하기 위해
BuildConfig를 사용하거나manifestPlaceholders를 활용하는 것이 좋습니다.build.gradle.kts에서:
android { defaultConfig { manifestPlaceholders["kakaoScheme"] = "kakao${project.findProperty("KAKAO_NATIVE_KEY")}" } }AndroidManifest.xml에서:
- <data android:host="oauth" - android:scheme="kakao7878cb24cec56458df067991de5e7786" /> + <data + android:host="oauth" + android:scheme="${kakaoScheme}" />data/login/src/main/java/com/umcspot/spot/login/datasourceimpl/LoginDataSourceImpl.kt (1)
3-3: 사용되지 않는 import를 제거하세요.Line 3의
androidx.datastore.core.DataStoreimport가 사용되지 않습니다.-import androidx.datastore.core.DataStoredata/login/src/main/java/com/umcspot/spot/login/di/LoginDataModule.kt (1)
14-16: 함수 이름을 더 명확하게 변경하세요.Line 16의 함수 이름
bindDummyRemoteDataSource가 "Dummy"를 포함하고 있어 테스트용이나 임시 코드처럼 보입니다. 실제 용도를 반영하는 이름으로 변경하는 것이 좋습니다.- abstract fun bindDummyRemoteDataSource(impl: LoginDataSourceImpl): LoginDataSource + abstract fun bindLoginDataSource(impl: LoginDataSourceImpl): LoginDataSourcefeature/signup/build.gradle.kts (1)
22-22: 주석 처리된 코드를 정리하세요.Line 22의 주석 처리된
libs.naver.jdk의존성이 필요 없다면 제거하거나, 필요한 경우 왜 주석 처리되었는지 설명을 추가하는 것이 좋습니다.data/user/src/main/java/com/umcspot/spot/user/mapper/UserMapper.kt (1)
10-13: 사용하지 않는 import를 제거하세요.
LocalTime,DateTimeFormatter,DateTimeParseException,Localeimport가 파일 내에서 사용되지 않습니다.-import java.time.LocalTime -import java.time.format.DateTimeFormatter -import java.time.format.DateTimeParseException -import java.util.Localedata/login/src/main/java/com/umcspot/spot/login/repositoryimpl/LoginRepositoryImpl.kt (1)
12-14: 변수명을 타입과 일치시키세요.변수가
LoginService타입인데studyService로 명명되어 혼란을 초래합니다.class LoginRepositoryImpl @Inject constructor( - private val studyService: LoginService, + private val loginService: LoginService, private val spotTokenDataStore: DataStore<SpotTokenData> ) : TokenRepository {그리고 Line 23에서 사용처도 업데이트하세요:
- val response = studyService.getCallBackToken(type.title, accessToken) + val response = loginService.getCallBackToken(type.title, accessToken)data/login/src/main/java/com/umcspot/spot/login/dto/response/TokenResponseDto.kt (1)
3-4: 사용하지 않는 import를 제거하세요.
SuppressLint와BoardTypeimport가 파일 내에서 사용되지 않습니다.-import android.annotation.SuppressLint -import com.umcspot.spot.model.BoardTypecore/network/src/main/java/com/umcspot/spot/network/model/BaseResponse.kt (1)
8-15: BaseResponse/NullResultResponse 구조 중복 및 응답 스펙 확인 제안
BaseResponse<T>와NullResultResponse가isSuccess / code / message필드를 그대로 중복하고 있어서, 유지보수 측면에서는BaseResponse<Unit?>나BaseResponse<Nothing?>+ 별도 Empty DTO 등으로 통합하는 것도 한 번 고려해볼 만 합니다. 서버 응답 스펙이 고정되어 있다면 그대로 두어도 문제는 없지만, 필드가 추가/변경될 때 두 클래스를 항상 같이 수정해야 한다는 점만 염두에 두시면 좋겠습니다.또한
code타입을String으로 잡으신 부분이 실제 백엔드 응답(JSON)과 정확히 일치하는지 한 번만 더 확인해보면 좋겠습니다. (서버에서 숫자 코드로 내려보내고 있다면 직렬화 에러가 발생할 수 있습니다.)Also applies to: 18-25
feature/signup/src/main/java/com/umcspot/spot/checkList/CheckListScreen.kt (2)
25-72: 이름 저장 → 테마 전송 → 화면 전환 순서가 자연스럽고, 빈 선택 방지도 잘 되어 있습니다
themes를collectAsStateWithLifecycle()으로 관찰하고,theme.isNotEmpty()일 때만 "다음" 버튼을 활성화한 부분이 UX·안정성 측면에서 좋습니다.- 클릭 시
saveNameIfChanged()→submitThemes()→onNextClick()순서도, 로컬 상태/서버 전송을 처리한 뒤 내비게이션하는 흐름으로 자연스럽습니다.현재 구조만 보면 큰 문제는 없어 보이지만, 추후 테스트 용이성을 위해:
CheckListScreen을 미리보기/테스트에서 재사용하고 싶다면SignUpViewModel과CheckListViewModel을 외부에서 항상 주입하는 패턴(기본값 없이)으로 바꾸는 것도 고려해볼 수 있겠습니다. (필수는 아니고, 확장성을 위한 옵션입니다.)
74-107: StudyTheme 그리드 UI는 공통 컴포저블로 추출하는 것을 고려해 볼 만합니다
ActivityThemeSection에서StudyTheme.entries를 돌면서 아이콘 매핑 +MultiButton을 그리는 로직이,RecruitingStudyFilterScreen쪽의ActivityThemeSection과 거의 동일하게 보입니다(리터럴 텍스트 유무 정도 차이).향후 테마가 추가·변경될 때 두 곳을 모두 수정해야 하므로,
- 아이콘 매핑 +
MultiButton리스트 부분을 공용 컴포저블로 추출하거나- 최소한 아이콘 매핑 로직만이라도 별도 util/extension으로 분리
하는 방식을 고려해 보시면 유지보수에 도움이 될 것 같습니다.
data/user/src/main/java/com/umcspot/spot/user/dto/response/UserThemeResponseDto.kt (1)
3-12: UnsafeOptInUsageError 억제 방식은 한 번 더 점검해 보시는 것을 추천드립니다
UserThemeResponseDto에@SuppressLint("UnsafeOptInUsageError")를 전체적으로 걸어두신 것은, 경고를 통째로 가리는 느낌이라 원인이 무엇인지 파악하기가 어려울 수 있습니다.
- 만약
StudyTheme혹은 관련 직렬화 API가@RequiresOptIn(실험 API)인 경우라면,
- 이 DTO에
@OptIn(해당ExperimentalApi::class)를 명시적으로 추가하는 편이 더 의도가 분명하고- 실제로 허용하고 싶은 사용 범위만 한정할 수 있습니다.
- 단순히 Lint 경고만 막기 위해 Suppress를 사용하셨다면, 어떤 API 때문에 경고가 뜨는지 다시 확인해 보시고, 가능하면 근본적인 원인(실험 API 사용 위치)에서 처리하는 쪽을 추천드립니다.
지금도 빌드에는 문제 없겠지만, 나중에 관련 API가 변경될 경우 추적이 어려워질 수 있어 한 번만 더 검토해 보시면 좋겠습니다.
feature/signup/src/main/java/com/umcspot/spot/signup/AgreementModal.kt (1)
37-253: 두 동의 Dialog의 레이아웃이 거의 동일하므로 공용 컴포저블 추출을 고려해볼 수 있습니다
PrivacyConsentDialog와UniqueConsentDialog가
Dialog→Surface→ 상단 닫기 버튼 Row → 제목 Row → 스크롤 영역 → "동의" 버튼
구조를 그대로 공유하고 있고, 제목/본문 텍스트만 다른 형태입니다.현재 PR 범위에서는 그대로 사용해도 동작에는 문제가 없지만,
- 공용
ConsentDialog(title: String, content: @Composable () -> Unit, …)같은 컴포저블로 레이아웃을 추출하고- 각 동의 종류별로 본문 내용만 슬롯으로 넘기는 형태
로 정리하면, 향후 디자인 변경이나 스타일 수정 시 한 곳만 고치면 되어 유지보수가 훨씬 수월해질 것 같습니다.
feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt (1)
72-83: Landing/SignUp/CheckList/Saving에 대한 표시 조건이 의도와 일치하는지 한 번만 점검해 주세요현재 설정을 보면:
isInLanding()은Landing,Saving그래프에서만true- 상단 뒤로가기 TopBar(
showBackTopBar)는SignUp,CheckList에서만 추가로true(Saving은 제외)- FAB 조건은 온보딩 플로우와는 무관
즉, 온보딩 플로우 네 화면이 서로 다른 조건 조합을 가지게 되는데,
- 디자인/UX 측에서 Landing + Saving을 같은 그룹으로 취급하고
- SignUp + CheckList만 별도로 뒤로가기 바를 노출하는 것이 정확한 요구사항이라면 지금 구현이 맞고,
- 만약 온보딩 전체를 하나의 “Landing 계열”로 보고 싶다면
isInLanding이나 TopBar 표시 조건에 SignUp/CheckList/Saving 중 일부를 추가해야 할 수도 있습니다.외부에서
isInLanding()과showBackTopBar()를 어떻게 사용하는지(예: 상태바 색, 뒤로가기 정책 등)에 따라 영향 범위가 달라지므로, 의도와 맞는지 한 번만 더 확인 부탁드립니다.feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt (1)
38-69: LoginEvent 오류 처리 UI 연동 제안
LandingViewModel.LoginEvent.ShowError분기(특히 Line 51-52)가 비어 있어서, 카카오/네이버 로그인 실패 시 사용자가 아무 피드백을 받지 못합니다. 토스트·스낵바·다이얼로그 등으로message를 노출하거나, 최소한 화면 단에서 로그라도 남기는 쪽으로 처리해 두면 UX/디버깅에 도움이 될 것 같습니다.feature/signup/src/main/java/com/umcspot/spot/signup/SignUpScreen.kt (3)
67-180: 이름 변경을 서버에 저장하는 시점 확인 필요
SignUpScreen에서:
- 이름 로딩:
LaunchedEffect(uiState.user)→viewmodel.load()- 이름 수정:
EditableNameRow의onNameChange에서viewmodel.setName(it)호출까지는 잘 연결되어 있습니다. 다만 이 화면 내부에서는
viewmodel.saveNameIfChanged()가 한 번도 호출되지 않아, 현재 코드만 보면:
- 실명 수정이 ViewModel 상태에는 반영되지만,
- 실제 서버에 저장(
UserRepository.setUserName)되는 시점이 보이지 않습니다.
onNextClick바깥에서(예: Navigation 레벨)saveNameIfChanged()를 호출하고 있다면 괜찮지만, 그렇지 않다면:TextButton( text = "다음", enabled = privacyChecked && uniqueChecked, onClick = { viewmodel.saveNameIfChanged() onNextClick() } )처럼 이 화면에서 저장을 트리거하는 것이 안전합니다. 현재 어디서 저장을 호출하는지 한 번만 확인 부탁드립니다.
211-339: 이름 15자 제한 로직이 붙여넣기/16자 이상 입력 시 비직관적일 수 있음
EditableNameRow의onValueChange에서:when { new.length <= 15 -> draft = new new.length > draft.length -> { val last = new.last() draft = draft.take(14) + last } else -> { draft = new.take(15) } }와 같이 처리하고 있어서,
- 16자째 입력 시 기존 마지막 글자가 교체되거나,
- 긴 문자열을 붙여넣을 때 거의 마지막 글자만 남는 등
사용자 입장에서 다소 예상 밖 동작이 나올 수 있습니다.
요구사항이 단순 “최대 15자”라면, 다음처럼 직관적으로 잘리는 구현이 더 깔끔합니다.
onValueChange = { new -> draft = new.take(15) }또는 붙여넣기/중간 수정까지 고려하더라도, 우선
new.take(15)로 자르고 추가 로직이 꼭 필요한 경우에만 분기하는 쪽을 추천드립니다. 현재 비즈니스 요구에 따라 선택하시면 될 것 같습니다.
341-388: ConsentItem의 접근성 semantics 보완 제안 (선택 상태 노출)
ConsentItem에서role = Role.Checkbox로 클릭 역할은 잘 노출되고 있지만, 선택 상태(checked)에 대한 정보는 semantics에 포함되지 않아 스크린 리더 사용 시 “체크 됨/안 됨”을 바로 알기 어렵습니다.예를 들어 다음처럼
selected또는stateDescription을 추가해 주면 접근성이 좋아집니다.modifier = Modifier .fillMaxWidth() .heightIn(min = 52.dp) .clip(RoundedCornerShape(12.dp)) .clickable( interactionSource = remember { MutableInteractionSource() }, indication = LocalIndication.current, role = Role.Checkbox, onClick = onClick ) .semantics { role = Role.Checkbox stateDescription = if (checked) "동의됨" else "미동의" }필수 사항은 아니지만, 약관 동의 화면 특성상 접근성을 신경 쓸 가치가 있어 보여 옵션 리팩터 수준으로 제안드립니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (78)
app/build.gradle.kts(2 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/com/umcspot/spot/SpotApplication.kt(1 hunks)build-logic/convention/src/main/java/com/umcspot/spot/convention/BuildConfig.kt(1 hunks)core/buildconfig/build.gradle.kts(1 hunks)core/datastore/src/main/java/com/umcspot/spot/datastore/SpotTokenData.kt(0 hunks)core/datastore/src/main/java/com/umcspot/spot/datastore/di/DataStoreModule.kt(2 hunks)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/GageBar.kt(3 hunks)core/designsystem/src/main/java/com/umcspot/spot/designsystem/shapes/Shapes.kt(0 hunks)core/model/src/main/java/com/umcspot/spot/model/Global.kt(1 hunks)core/network/build.gradle.kts(1 hunks)core/network/src/main/java/com/umcspot/spot/network/AuthInterceptor.kt(1 hunks)core/network/src/main/java/com/umcspot/spot/network/di/NetworkModule.kt(2 hunks)core/network/src/main/java/com/umcspot/spot/network/model/BaseResponse.kt(1 hunks)data/alert/src/main/java/com/umcspot/spot/alert/repositoryimpl/AlertRepositoryImpl.kt(2 hunks)data/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.kt(1 hunks)data/login/build.gradle.kts(1 hunks)data/login/proguard-rules.pro(1 hunks)data/login/src/main/AndroidManifest.xml(1 hunks)data/login/src/main/java/com/umcspot/spot/login/datasource/LoginDataSource.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/datasourceimpl/LoginDataSourceImpl.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/di/LoginDataModule.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/di/LoginRepositoryModule.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/di/LoginServiceModule.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/dto/request/TokenRequestDto.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/dto/response/TokenResponseDto.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/mapper/TokenMapper.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/repositoryimpl/LoginRepositoryImpl.kt(1 hunks)data/login/src/main/java/com/umcspot/spot/login/service/LoginService.kt(1 hunks)data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt(3 hunks)data/user/.gitignore(1 hunks)data/user/build.gradle.kts(1 hunks)data/user/proguard-rules.pro(1 hunks)data/user/src/main/AndroidManifest.xml(1 hunks)data/user/src/main/java/com/umcspot/spot/user/datasource/UserDataSource.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/datasourceimpl/UserDataSourceImpl.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/di/UserDataModule.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/di/UserRepositoryModule.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/di/UserServiceModule.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/dto/request/UserRequestDto.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/dto/response/UserResponseDto.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/dto/response/UserThemeResponseDto.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/mapper/UserMapper.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/repositoryimpl/UserRepositoryImpl.kt(1 hunks)data/user/src/main/java/com/umcspot/spot/user/service/UserService.kt(1 hunks)data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt(1 hunks)domain/token/.gitignore(1 hunks)domain/token/build.gradle.kts(1 hunks)domain/token/proguard-rules.pro(1 hunks)domain/token/src/main/java/com/umcspot/spot/token/model/TokenResult.kt(1 hunks)domain/token/src/main/java/com/umcspot/spot/token/model/Tokentype.kt(1 hunks)domain/token/src/main/java/com/umcspot/spot/token/repository/TokenRepository.kt(1 hunks)domain/user/.gitignore(1 hunks)domain/user/build.gradle.kts(1 hunks)domain/user/src/main/java/com/umcspot/spot/user/model/UserResult.kt(1 hunks)domain/user/src/main/java/com/umcspot/spot/user/model/UserTheme.kt(1 hunks)domain/user/src/main/java/com/umcspot/spot/user/repository/UserRepository.kt(1 hunks)feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt(2 hunks)feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt(4 hunks)feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt(2 hunks)feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt(2 hunks)feature/signup/build.gradle.kts(1 hunks)feature/signup/src/main/java/com/umcspot/spot/checkList/CheckListScreen.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/checkList/CheckListViewModel.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/checkList/navigation/CheckListNavigation.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt(3 hunks)feature/signup/src/main/java/com/umcspot/spot/landing/LoginViewmodel.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/landing/SavingScreen.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/landing/navigation/LandingNavigation.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/landing/navigation/SavingNavigation.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/signup/AgreementModal.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/signup/SignUpScreen.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/signup/SignUpViewModel.kt(1 hunks)feature/signup/src/main/java/com/umcspot/spot/signup/navigation/SignUpNavigation.kt(1 hunks)feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyFilterScreen.kt(2 hunks)feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyScreen.kt(2 hunks)gradle/libs.versions.toml(3 hunks)settings.gradle.kts(2 hunks)
💤 Files with no reviewable changes (2)
- core/datastore/src/main/java/com/umcspot/spot/datastore/SpotTokenData.kt
- core/designsystem/src/main/java/com/umcspot/spot/designsystem/shapes/Shapes.kt
🧰 Additional context used
🧬 Code graph analysis (10)
feature/signup/src/main/java/com/umcspot/spot/checkList/CheckListScreen.kt (3)
feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyFilterScreen.kt (1)
ActivityThemeSection(229-273)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/TextButton.kt (1)
TextButton(191-245)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageTextButton.kt (1)
MultiButton(132-222)
feature/signup/src/main/java/com/umcspot/spot/landing/navigation/SavingNavigation.kt (2)
feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt (1)
navigate(45-64)feature/signup/src/main/java/com/umcspot/spot/landing/SavingScreen.kt (1)
SavingScreen(40-114)
feature/signup/src/main/java/com/umcspot/spot/landing/SavingScreen.kt (1)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/GageBar.kt (1)
GageBar(36-72)
feature/signup/src/main/java/com/umcspot/spot/checkList/navigation/CheckListNavigation.kt (2)
feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt (1)
navigate(45-64)feature/signup/src/main/java/com/umcspot/spot/checkList/CheckListScreen.kt (1)
CheckListScreen(25-72)
feature/signup/src/main/java/com/umcspot/spot/landing/navigation/LandingNavigation.kt (1)
feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt (1)
LandingScreen(38-69)
feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt (2)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/SocialLoginButton.kt (2)
KakaoStartButton(81-95)NaverStartButton(98-112)core/designsystem/src/main/java/com/umcspot/spot/designsystem/theme/Theme.kt (1)
SpotTheme(54-79)
feature/signup/src/main/java/com/umcspot/spot/signup/navigation/SignUpNavigation.kt (2)
feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt (1)
navigate(45-64)feature/signup/src/main/java/com/umcspot/spot/signup/SignUpScreen.kt (1)
SignUpScreen(67-181)
feature/signup/src/main/java/com/umcspot/spot/signup/SignUpScreen.kt (3)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/TextButton.kt (1)
TextButton(191-245)feature/signup/src/main/java/com/umcspot/spot/signup/AgreementModal.kt (2)
PrivacyConsentDialog(37-144)UniqueConsentDialog(146-253)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageTextButton.kt (1)
MultiButton(132-222)
feature/signup/src/main/java/com/umcspot/spot/signup/AgreementModal.kt (1)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/TextButton.kt (1)
TextButtonM(289-307)
feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (3)
feature/signup/src/main/java/com/umcspot/spot/signup/navigation/SignUpNavigation.kt (1)
signupGraph(20-38)feature/signup/src/main/java/com/umcspot/spot/checkList/navigation/CheckListNavigation.kt (1)
checkListGraph(20-38)feature/signup/src/main/java/com/umcspot/spot/landing/navigation/SavingNavigation.kt (1)
savingGraph(16-26)
🔇 Additional comments (49)
domain/token/.gitignore (1)
1-1: 표준 빌드 아티팩트 제외 설정 적절함
.gitignore파일이 모듈의/build디렉토리를 올바르게 제외하도록 구성되어 있습니다. 이는 Gradle 기반 Android 프로젝트에서 로컬 빌드 산출물이 버전 관리에서 제외되도록 하는 표준 관례를 따르고 있습니다.feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyScreen.kt (1)
3-61: 현재 변경은 import 추가로만 구성되어 있어 기능 영향은 없어 보입니다.
BackHandler와Timberimport 추가는 기존RecruitingStudyScreenUI 로직이나 상태 흐름을 변경하지 않으며, 빌드/동작 측면에서 위험 요소는 없어 보입니다. 추후 뒤로가기 처리나 로깅을 이 화면에서 사용할 준비 작업 정도로 이해할 수 있습니다.feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyFilterScreen.kt (1)
3-5: 새 import 추가는 현재 동작에 영향을 주지 않습니다
Log,Toast,BackHandlerimport 추가로 인한 동작 변화는 현재 없습니다. 이후 뒤로가기 처리나 디버깅, 사용자 피드백 토스트를 붙일 예정이라면 그대로 두셔도 무방해 보입니다.core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/GageBar.kt (1)
78-100: Preview 함수 업데이트 확인Preview 함수들이 새로운
GageBar이름으로 올바르게 업데이트되었습니다.core/network/build.gradle.kts (1)
16-16: 의존성 추가가 적절합니다.AuthInterceptor에서 DataStore를 사용하기 위해 필요한 의존성이 올바르게 추가되었습니다.
core/network/src/main/java/com/umcspot/spot/network/di/NetworkModule.kt (1)
31-37: AuthInterceptor 통합이 올바르게 구현되었습니다.DI를 통한 AuthInterceptor 주입과 OkHttpClient 설정이 적절합니다. 현재 인터셉터 순서는 loggingInterceptor → authInterceptor인데, 이는 로깅이 Authorization 헤더 추가 전에 발생함을 의미합니다. 디버깅 시 Authorization 헤더도 로그에 남기고 싶다면 순서를 변경하는 것을 고려해보세요.
현재 설정으로 Authorization 헤더가 로그에 출력되는지 확인이 필요합니다. 필요하다면 다음과 같이 순서를 변경할 수 있습니다:
fun providesOkHttpClient( loggingInterceptor: HttpLoggingInterceptor, authInterceptor: AuthInterceptor ): OkHttpClient = OkHttpClient.Builder() - .addInterceptor(loggingInterceptor) .addInterceptor(authInterceptor) + .addInterceptor(loggingInterceptor) .build()core/datastore/src/main/java/com/umcspot/spot/datastore/di/DataStoreModule.kt (1)
1-1: 패키지 및 함수 이름 변경사항이 모두 정상적으로 반영되었습니다.검증 결과:
- 이전 패키지명(
com.umcspot.spot.datastore.di) 참조: 없음 ✓- 이전 함수명(
providesDataStore) 참조: 없음 ✓- 새 패키지명(
com.umcspot.spot.datastore): 확인됨 ✓모든 참조가 올바르게 업데이트되었으며, 분산된 참조가 남아있지 않습니다.
data/user/.gitignore (1)
1-1: LGTM!Android 모듈의 표준 .gitignore 구성입니다.
domain/user/.gitignore (1)
1-1: LGTM!Android 모듈의 표준 .gitignore 구성입니다.
data/user/src/main/AndroidManifest.xml (1)
1-4: LGTM!Android 라이브러리 모듈의 표준 manifest 구조입니다. 컴포넌트나 권한이 필요하지 않은 경우 이와 같이 최소한의 manifest를 사용하는 것이 적절합니다.
data/login/src/main/AndroidManifest.xml (1)
1-4: LGTM!Android 라이브러리 모듈의 표준 manifest 구조입니다. OAuth 흐름과 관련된 실제 컴포넌트는 앱 레벨 manifest에서 정의됩니다.
settings.gradle.kts (2)
54-57: LGTM!소셜 로그인 기능을 위한 모듈 구조 개편이 적절합니다.
:domain:signup을 제거하고 user/login/token 관심사를 분리한 4개의 새 모듈을 추가한 것은 좋은 아키텍처 결정입니다.
21-21: 카카오 Maven 저장소 URL 검증 완료 - 접근 가능 확인됨저장소 URL 검증 결과, 카카오 Maven 저장소(
https://devrepo.kakao.com/nexus/content/groups/public/)에 정상적으로 접근 가능함이 확인되었습니다. HTTP 200 OK 응답을 받았으므로 저장소 설정이 올바르게 구성되어 있습니다.data/login/proguard-rules.pro (1)
1-21: LGTM!표준 ProGuard 템플릿 파일입니다. 모든 규칙이 주석 처리되어 있으며, 필요시 나중에 활성화할 수 있는 예제로 제공됩니다.
domain/token/src/main/java/com/umcspot/spot/token/model/TokenResult.kt (1)
1-6: 단순·명확한 토큰 도메인 모델 구조 👍액세스/리프레시 토큰을 분리한
TokenResult도메인 모델 설계가 직관적이고, 레이어 간 전달 타입으로 쓰기 적절해 보입니다.
필요 시 이후에 만료 시간(exp) 등 메타 정보를 확장하기에도 무난한 형태입니다.gradle/libs.versions.toml (2)
93-104: datastoreCore 버전 값 한 번 확인 필요이미
androidx-datastore = "1.1.1"이 정의되어 있는데, 별도로datastoreCore = "1.1.7"을 사용하고 있어 오타나 의도치 않은 버전 상이 가능성이 있습니다. 실제 존재하는 버전인지, 그리고datastore-core아티팩트에 이 값을 쓰는 것이 맞는지 한 번 더 확인해 주시면 좋겠습니다.
259-262: 카카오/네이버 OAuth 의존성 정의는 적절하나, 모듈 간 alias 일관성 점검 권장
kakao-login,kakao-auth,kakao-common및naver-oauth,naver-jdk,androidx-browser,datastore-core가 버전 카탈로그에 잘 추가돼서 이후 유지보수도 수월해 보입니다. 다만app,data/login,data/user등 실제 사용하는 모듈에서 모두 이 alias 들을 동일하게 사용하고 있는지만 한 번 더 점검해 두면, 나중에 버전 업/교체 시 혼선을 줄일 수 있을 것 같습니다.Also applies to: 272-276
feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt (1)
26-27: SignUp/CheckList 화면의 상단바 타이틀 분기 추가 적절함새 온보딩 라우트(SignUp, CheckList)에 대해 기존 Alert/Recruiting과 동일 패턴으로 타이틀을 분기해 주셔서 UX 일관성이 좋아 보입니다.
navigator.showBackTopBar()및isInLanding()조건도 그대로 재사용돼서 흐름 상 문제는 없어 보입니다.Also applies to: 32-32, 56-57
data/user/build.gradle.kts (1)
1-10: data:user 모듈 설정이 역할에 맞게 최소화되어 있음spot.data 컨벤션 플러그인과
projects.domain.user의존성만 두어서 data 계층의 책임이 명확하게 분리되어 있습니다. 이후 네트워크/로컬 데이터 소스가 추가되더라도 이 구조를 유지하면서 확장하면 될 것 같습니다.data/alert/src/main/java/com/umcspot/spot/alert/repositoryimpl/AlertRepositoryImpl.kt (1)
16-16: BaseResponse 필드명 변경(result) 반영이 일관되게 잘 되어 있음
getAlerts와getAppliedAlerts모두response.result.toDomainList()로 변경해 BaseResponse 구조 변경과 정합성을 맞춰 주신 부분 좋습니다.runCatching/recoverCatching패턴과 더미 데이터 복구 로직도 유지되어 있어서 기존 호출부 동작에 문제는 없어 보입니다.Also applies to: 27-27
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (1)
17-17: Weather API 응답 필드 변경(result)에 맞춘 매핑 수정 적절
response.result.toDomain()으로 변경해 BaseResponse 구조와 일관되게 맞춰 주신 부분 좋습니다. 실패 시WeatherResult.dummyFrom()으로 복구하는 로직도 그대로 유지되어 있어서, 기존 호출부 동작에는 변화 없이 응답 모델만 정상적으로 따라갈 것 같습니다.domain/user/build.gradle.kts (1)
1-10: domain:user 모듈이 코어 모델/코루틴에만 의존하도록 구성된 점이 좋음
spot.android.java.library플러그인과core.model, 코루틴 번들만 의존하도록 잡혀 있어서 도메인 계층이 상위(Android) 레이어에 새는 부분이 없습니다. 다른 도메인 모듈과도 일관된 형태라 유지보수에 유리해 보입니다.domain/token/proguard-rules.pro (1)
1-21: ProGuard 스캐폴딩 파일 추가로 모듈별 난독화 설정 확장성 확보현재는 전부 주석 처리된 템플릿이라 빌드/런타임 동작에는 영향이 없고, 추후 token 도메인에 대해 난독화 예외나 디버깅용 설정이 필요할 때 이 파일에서 모듈 단위로 관리할 수 있어 보여 좋습니다.
data/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.kt (1)
18-18: 일관된 응답 모델 마이그레이션 적용됨세 메서드 모두에서
res.data를res.result로 일관되게 변경하여 BaseResponse 모델 업데이트를 올바르게 반영했습니다.Also applies to: 26-26, 34-34
data/login/src/main/java/com/umcspot/spot/login/dto/request/TokenRequestDto.kt (1)
7-11: 적절한 직렬화 구성TokenRequestDto가 kotlinx.serialization을 사용하여 올바르게 구성되었으며, @SerialName 어노테이션으로 JSON 키 매핑이 명확합니다.
data/user/src/main/java/com/umcspot/spot/user/datasource/UserDataSource.kt (1)
11-16: 인터페이스 설계가 적절함사용자 정보 조회 및 업데이트를 위한 메서드 시그니처가 명확하며, suspend 함수를 사용하여 코루틴 지원이 올바르게 구현되었습니다.
domain/user/src/main/java/com/umcspot/spot/user/model/UserResult.kt (1)
5-11: 명확한 도메인 모델 정의
UserResult와UserTheme데이터 클래스가 간결하고 명확하게 정의되어 있으며, 도메인 계층의 역할에 적합합니다.feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt (1)
15-16: 랜딩 플로우 통합이 적절함
LandingViewModel과Landingnavigation import 추가는 소셜 로그인 플로우 재구성에 맞춰 올바르게 업데이트되었습니다.data/login/src/main/java/com/umcspot/spot/login/di/LoginServiceModule.kt (1)
11-18: DI 모듈 구성이 적절함Hilt 모듈이 올바르게 구성되었으며, Retrofit을 통한 LoginService 싱글톤 제공 패턴이 적절합니다.
data/user/src/main/java/com/umcspot/spot/user/di/UserServiceModule.kt (1)
11-19: LGTM!Retrofit 서비스 제공을 위한 표준 Dagger/Hilt 패턴이 올바르게 구현되었습니다.
data/login/build.gradle.kts (1)
1-15: LGTM!로그인 데이터 모듈에 대한 Gradle 설정이 적절하게 구성되었습니다. 의존성 구조가 명확하고 필요한 모듈들이 올바르게 포함되어 있습니다.
data/login/src/main/java/com/umcspot/spot/login/service/LoginService.kt (1)
9-17: 구현이 올바릅니다.OAuth 콜백 토큰을 가져오는 Retrofit 서비스 인터페이스가 올바르게 정의되어 있습니다. REST API 엔드포인트 구조와 파라미터 매핑이 적절합니다.
data/login/src/main/java/com/umcspot/spot/login/datasourceimpl/LoginDataSourceImpl.kt (1)
14-18: 구현이 올바릅니다.LoginService에 대한 위임이 깔끔하고 적절하게 구현되어 있습니다.
data/login/src/main/java/com/umcspot/spot/login/mapper/TokenMapper.kt (1)
8-17: 구현이 올바릅니다.토큰 매핑 함수들이 명확하고 간결하게 구현되어 있습니다. DTO와 도메인 모델 간의 변환 로직이 적절합니다.
domain/token/src/main/java/com/umcspot/spot/token/repository/TokenRepository.kt (1)
6-8: 구현이 올바릅니다.TokenRepository 인터페이스가 명확하게 정의되어 있으며,
Result타입을 사용한 에러 처리 방식이 적절합니다.data/user/src/main/java/com/umcspot/spot/user/dto/request/UserRequestDto.kt (1)
7-17: 구현이 올바릅니다.사용자 요청 DTO들이 올바르게 정의되어 있습니다. Kotlinx Serialization 어노테이션이 적절하게 사용되었으며, 구조가 명확합니다.
data/user/src/main/java/com/umcspot/spot/user/repositoryimpl/UserRepositoryImpl.kt (2)
23-26: LGTM!
setUserName()구현이 올바르며 표준 패턴을 따릅니다.
29-32: LGTM!
setUserTheme()구현이 올바르며 일관된 패턴을 따릅니다.feature/signup/src/main/java/com/umcspot/spot/landing/navigation/LandingNavigation.kt (1)
15-21: LGTM!두 개의 콜백(
onKakaoClick,onNaverClick)을 단일onLoginSuccess콜백으로 통합한 것은 API를 단순화하고 관심사 분리를 개선합니다.LandingScreen이 이제 내부적으로 소셜 로그인을 처리하므로 적절한 리팩토링입니다.data/user/src/main/java/com/umcspot/spot/user/mapper/UserMapper.kt (1)
15-30: LGTM!매퍼 함수들이 올바르게 구현되었으며 도메인과 DTO 레이어 간의 변환을 적절히 처리합니다.
feature/signup/src/main/java/com/umcspot/spot/signup/navigation/SignUpNavigation.kt (1)
20-37: LGTM!
NavHostController를 사용한 ViewModel 스코핑 패턴이 올바르게 구현되었습니다.NavHost루트에 ViewModel을 스코핑하여 여러 화면에서 공유할 수 있도록 한 것은 적절한 접근입니다.data/login/src/main/java/com/umcspot/spot/login/dto/response/TokenResponseDto.kt (1)
8-14: LGTM!
TokenResponseDto가kotlinx.serialization을 사용하여 올바르게 정의되었으며 JSON 매핑이 적절합니다.data/user/src/main/java/com/umcspot/spot/user/service/UserService.kt (1)
12-26: LGTM!Retrofit 서비스 인터페이스가 올바르게 정의되었습니다. 세 가지 엔드포인트 모두 적절한 HTTP 메서드와 응답 타입을 사용하고 있습니다.
feature/signup/src/main/java/com/umcspot/spot/checkList/navigation/CheckListNavigation.kt (1)
20-38: LGTM!
CheckListNavigation이SignUpNavigation과 일관된 패턴을 사용하여 구현되었습니다.SignUpViewModel을 부모 엔트리에 스코핑하여 여러 화면에서 공유하는 것은 올바른 접근입니다.data/login/src/main/java/com/umcspot/spot/login/repositoryimpl/LoginRepositoryImpl.kt (1)
17-36:SocialLoginType.title속성이 올바르게 정의되어 있습니다.검증 결과,
SocialLoginTypeenum 클래스(core/model/src/main/java/com/umcspot/spot/model/Global.kt, 62-67줄)에title: String속성이 명확히 존재하며, 각 enum 값(KAKAO, NAVER)에 대해 적절한 문자열 값("kakao", "naver")이 할당되어 있습니다. 따라서 Line 23에서type.title을 사용하는 것이 완전히 유효합니다.data/user/src/main/java/com/umcspot/spot/user/datasourceimpl/UserDataSourceImpl.kt (1)
15-32: UserDataSourceImpl 위임 구조는 현재 형태로 적절해 보입니다
UserDataSourceImpl이UserService로 깔끔하게 위임만 하고 있어서, 데이터 소스 레이어 역할에 잘 맞습니다. 추가적인 에러 매핑·공통 처리(예:Result래핑 등)가 필요해지면 이후 레이어에서 도입하셔도 될 것 같고, 지금 PR 범위에서는 이 정도 얇은 구현이 가장 유지보수하기 좋아 보입니다.app/build.gradle.kts (1)
55-63: 소셜 로그인 관련 모듈 및 라이브러리 의존성 연결은 방향성이 잘 맞습니다
data.user/data.login모듈과 Kakao·Naver OAuth 라이브러리를app모듈에 추가하신 구조가 이번 PR 목표(소셜 로그인 + 사용자 정보 설정)와 잘 일치해 보입니다. 버전 카탈로그(libs.*)와 실제 Gradle Sync 시 의존성이 정상 해석되는지만 한 번 확인해 주시면 될 것 같습니다.feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (2)
55-83: Landing → SignUp → CheckList → Saving → Home 플로우와 백스택 정리는 자연스럽습니다
onLoginSuccess에서navigateToSignUp호출 시,graph.findStartDestination().id까지popUpTo(inclusive = true)로 날려주는 구조라서 Landing 관련 스택을 깨끗하게 비우고 온보딩 플로우로 진입하는 구조가 잘 잡혀 있습니다.savingGraph완료 시에도 동일하게findStartDestination()기준으로popUpTo(inclusive = true)후 홈으로 보내기 때문에, 회원가입/체크리스트/세이빙 단계들이 홈 진입 후에는 back stack에 남지 않는 점도 UX 측면에서 합리적으로 보입니다.현재 설계 의도대로라면 이 네비게이션 흐름은 그대로 사용하셔도 무방해 보입니다.
94-102: 모집 공고 리스트 아이템 클릭 no-op 상태 확인 필요코드 검증 결과, 지적하신 내용이 정확합니다:
MainNavHost.kt115줄의onItemClick = {}는 실제로 비어있는 상태recruitingStudyGraph의onItemClick파라미터가RecruitingStudyScreen으로 전달되어 185줄에서onClick = { onItemClick(item) }으로 사용됨- 유사한 패턴이 BoardScreen의 여러 곳에서도 발견됨 (라인 118, 132, 146)
이 no-op이 의도적인 임시 조치인지 아니면 상세 화면 미구현 상태인지 개발자의 명시적인 확인이 필요합니다. 의도적 임시 조치라면 TODO 코멘트 추가를 권장합니다.
app/build.gradle.kts
Outdated
| signingConfigs { | ||
| getByName("debug") { | ||
| storeFile = file("key/SpotKey") | ||
| storePassword = "spotspot" | ||
| keyAlias = "spotkey0" | ||
| keyPassword = "spotspot" | ||
| } | ||
| } |
There was a problem hiding this comment.
서명 설정에 노출된 keystore 비밀번호는 분리하는 것이 안전합니다
signingConfigs 안에 storePassword / keyPassword를 하드코딩해서 커밋하면,
- 저장소가 공개되거나 유출됐을 때 키 폐기·교체가 필요한 보안 이슈가 될 수 있고
- 현재
release도debug서명 구성을 재사용하고 있어서(기존 코드일 수 있지만), 실 기기 배포용 빌드에 같은 키를 쓸 경우 리스크가 커집니다.
가능하면 아래처럼 정리하는 것을 추천드립니다.
- 비밀번호/경로는
local.properties또는 Gradlegradle.properties(VCS 미추적)에서 읽기 debug와release는 별도signingConfigs를 두고, 실제 스토어/비밀번호는 로컬 환경에서만 관리- 저장소에 이미 커밋된 keystore/비밀번호는 한 번 공개된 것으로 보고, 필요하다면 새 키로 회전하는 것도 검토
빌드가 이미 내부 테스트 용도로만 쓰이는 경우라도, 지금 단계에서 분리해두면 이후 배포 파이프라인 구성 시에도 훨씬 안전합니다.
🤖 Prompt for AI Agents
In app/build.gradle.kts around lines 10-17, the debug signing config currently
hardcodes storePassword and keyPassword; remove these secrets and instead load
storeFile path and passwords from an untracked properties source (e.g.,
local.properties or a VCS-ignored gradle.properties) using Gradle property
access, create separate signingConfigs for debug and release so release does not
reuse debug credentials, and ensure any keystore files and secrets are excluded
from VCS (or rotated if already committed); implement a fallback for CI by
reading environment variables when local properties are absent.
| val accessToken = runBlocking { | ||
| spotTokenDataStore.data.first().accessToken | ||
| } |
There was a problem hiding this comment.
OkHttp Interceptor에서 runBlocking 사용은 성능 문제를 일으킬 수 있습니다.
Interceptor는 네트워크 스레드에서 실행되며, runBlocking으로 DataStore를 읽으면 해당 스레드가 차단되어 ANR이나 심각한 성능 저하가 발생할 수 있습니다.
다음 해결 방법을 권장합니다:
- 토큰을 메모리에 캐시하고 Interceptor에서는 캐시된 값만 참조
- Authenticator를 사용하여 401 응답 시에만 토큰 갱신
- 토큰 관리 레포지토리 레이어를 추가하여 동기적으로 접근 가능한 토큰 제공
참고 구현 예시:
// 토큰 캐시를 사용하는 방식
class AuthInterceptor @Inject constructor(
private val tokenRepository: TokenRepository
) : Interceptor {
override fun intercept(chain: Interceptor.Chain): Response {
val original = chain.request()
// 동기적으로 접근 가능한 캐시된 토큰 사용
val accessToken = tokenRepository.getCachedAccessToken()
if (accessToken.isBlank()) {
return chain.proceed(original)
}
val newRequest = original.newBuilder()
.addHeader("Authorization", "Bearer $accessToken")
.build()
return chain.proceed(newRequest)
}
}🤖 Prompt for AI Agents
In core/network/src/main/java/com/umcspot/spot/network/AuthInterceptor.kt around
lines 18-20, remove the runBlocking call that reads DataStore on the network
thread; instead, inject a TokenRepository (or similar) that exposes a
synchronous getCachedAccessToken() method and use that cached token in the
interceptor, falling back to proceeding without Authorization when blank;
additionally, implement token refresh via an OkHttp Authenticator (or
repository-managed refresh) on 401 responses and ensure the TokenRepository
updates the in-memory cache when DataStore emits new tokens so the interceptor
never blocks.
| @Binds | ||
| @Singleton | ||
| abstract fun bindsDummyRepository(dummyRepositoryImpl: LoginRepositoryImpl): TokenRepository |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
프로덕션 코드에 부적절한 명명
메서드 이름 bindsDummyRepository와 파라미터 이름 dummyRepositoryImpl 모두 "Dummy"를 포함하고 있어 테스트/임시 코드처럼 보입니다.
다음 diff를 적용하여 명명을 개선하세요:
- abstract fun bindsDummyRepository(dummyRepositoryImpl: LoginRepositoryImpl): TokenRepository
+ abstract fun bindsTokenRepository(impl: LoginRepositoryImpl): TokenRepository📝 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.
| @Binds | |
| @Singleton | |
| abstract fun bindsDummyRepository(dummyRepositoryImpl: LoginRepositoryImpl): TokenRepository | |
| @Binds | |
| @Singleton | |
| abstract fun bindsTokenRepository(impl: LoginRepositoryImpl): TokenRepository |
🤖 Prompt for AI Agents
In data/login/src/main/java/com/umcspot/spot/login/di/LoginRepositoryModule.kt
around lines 14 to 16, the method name `bindsDummyRepository` and parameter
`dummyRepositoryImpl` wrongly imply test/temporary code; rename the method to
something production-appropriate (e.g., `bindTokenRepository` or
`bindLoginRepository`) and rename the parameter to match the implementation type
(e.g., `loginRepositoryImpl` or `tokenRepositoryImpl`) so the signature reads
like a normal binding of LoginRepositoryImpl to TokenRepository; update any
references and keep the @Binds/@Singleton annotations unchanged.
| runCatching { | ||
| val response = studyService.getPopularStudies() | ||
| response.data.toDomainList() | ||
| response.result!!.toDomainList() |
There was a problem hiding this comment.
null 안전성 문제: !! 연산자 사용
네 메서드 모두에서 response.result!!를 사용하고 있어 result가 null일 경우 NullPointerException이 발생합니다. recoverCatching이 이를 처리하긴 하지만, 실제 문제를 숨기고 더미 데이터로 복구하는 것은 디버깅을 어렵게 만듭니다.
다음 중 하나를 권장합니다:
- 안전 호출 연산자와 엘비스 연산자 사용:
response.result?.toDomainList() ?: throw IllegalStateException("Empty result") - 명시적 null 체크 후 처리
다음 diff를 적용하여 안전 호출로 변경하세요:
- response.result!!.toDomainList()
+ response.result?.toDomainList() ?: throw IllegalStateException("Response result is null")모든 네 메서드에 동일하게 적용해야 합니다.
Also applies to: 31-31, 43-43, 51-51
🤖 Prompt for AI Agents
In
data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt
around lines 19, 31, 43 and 51, replace unsafe uses of response.result!! with a
safe-null handling pattern: check response.result with a safe-call and Elvis
operator (or explicit null check) and throw a clear exception when null (e.g.,
IllegalStateException("Empty result")) so the methods return
response.result?.toDomainList() ?: throw IllegalStateException("Empty result")
(apply same change to all four locations) to avoid NullPointerException and
surface missing data instead of masking it.
| @Binds | ||
| @Singleton | ||
| abstract fun bindDummyRemoteDataSource(impl: UserDataSourceImpl): UserDataSource |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
프로덕션 코드에 부적절한 메서드 명명
메서드 이름 bindDummyRemoteDataSource에 "Dummy"가 포함되어 있어 테스트/임시 코드처럼 보입니다.
다음 diff를 적용하여 메서드 이름을 개선하세요:
- abstract fun bindDummyRemoteDataSource(impl: UserDataSourceImpl): UserDataSource
+ abstract fun bindsUserDataSource(impl: UserDataSourceImpl): UserDataSource📝 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.
| @Binds | |
| @Singleton | |
| abstract fun bindDummyRemoteDataSource(impl: UserDataSourceImpl): UserDataSource | |
| @Binds | |
| @Singleton | |
| abstract fun bindsUserDataSource(impl: UserDataSourceImpl): UserDataSource |
🤖 Prompt for AI Agents
In data/user/src/main/java/com/umcspot/spot/user/di/UserDataModule.kt around
lines 15 to 17, the method name bindDummyRemoteDataSource wrongly implies a
test/temporary implementation; rename it to a clear production name such as
bindUserDataSource (keeping the @Binds, @Singleton annotations and the signature
unchanged: impl: UserDataSourceImpl): UserDataSource), and update any
references/usages across the codebase to the new method name to maintain
consistency.
| @@ -1,15 +1,21 @@ | |||
| package com.umcspot.spot.main | |||
|
|
|||
| import android.content.Intent | |||
There was a problem hiding this comment.
🧩 Analysis chain
추가된 import의 사용 여부 확인 필요
Intent, Uri, Log import가 추가되었으나 제공된 코드 범위 내에서는 사용되지 않습니다. 파일의 다른 부분에서 사용되는지 확인하세요.
다음 스크립트를 실행하여 import 사용 여부를 확인하세요:
Also applies to: 5-5, 7-7
🏁 Script executed:
#!/bin/bash
# MainActivity.kt에서 추가된 imports의 실제 사용 확인
echo "=== Intent 사용 확인 ==="
rg -n "Intent\(" feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt
echo "=== Uri 사용 확인 ==="
rg -n "Uri\." feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt
echo "=== Log 사용 확인 ==="
rg -n "Log\.[diwev]" feature/main/src/main/java/com/umcspot/spot/main/MainActivity.ktLength of output: 56
🏁 Script executed:
#!/bin/bash
# 파일 존재 여부 확인 및 import 확인
echo "=== 파일 존재 여부 ==="
if [ -f "feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt" ]; then
echo "파일 존재함"
else
echo "파일 없음 - 다른 경로 검색 중..."
find . -name "MainActivity.kt" -type f 2>/dev/null | head -5
fi
echo ""
echo "=== MainActivity.kt의 import 섹션 ==="
head -20 feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt 2>/dev/null
echo ""
echo "=== Intent, Uri, Log 선언 확인 (전체) ==="
rg -n "import.*(Intent|Uri|Log)" feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt
echo ""
echo "=== Intent, Uri, Log 사용 확인 (패턴 없음) ==="
rg -n "Intent|Uri|Log" feature/main/src/main/java/com/umcspot/spot/main/MainActivity.ktLength of output: 950
🏁 Script executed:
#!/bin/bash
# 파일 존재 여부 확인 및 import 섹션 검토
echo "=== 파일 존재 여부 ==="
if [ -f "feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt" ]; then
echo "파일 존재함"
echo ""
echo "=== 파일의 처음 30줄 (import 섹션) ==="
head -30 "feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt"
else
echo "경로 확인 실패. 다른 경로 검색 중..."
find . -name "MainActivity.kt" -type f 2>/dev/null
fi
echo ""
echo "=== Intent, Uri, Log 사용 여부 (전체 파일에서 검색) ==="
rg -n "Intent|Uri|Log" feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt 2>/dev/null | head -20Length of output: 1215
🏁 Script executed:
#!/bin/bash
# 파일 전체 내용 확인 및 사용 여부 최종 확인
echo "=== 파일 전체 라인 수 ==="
wc -l feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt
echo ""
echo "=== 파일 전체 내용 ==="
cat feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt
echo ""
echo "=== 8번 라인 이후 Intent, Uri, Log 사용 여부 ==="
tail -n +8 feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt | rg -n "Intent|Uri|Log"Length of output: 1282
추가된 import 제거 필요
Intent, Uri, Log import가 추가되었으나 파일 전체(36줄)에서 사용되지 않습니다. 라인 3, 5, 7의 미사용 import를 제거하세요.
🤖 Prompt for AI Agents
In feature/main/src/main/java/com/umcspot/spot/main/MainActivity.kt around lines
3 to 7, the imports Intent, Uri and Log are unused in the file; remove the
import statements at lines 3, 5 and 7 to eliminate unused imports (or run the
IDE's optimize imports/refactor -> remove unused imports) so the file no longer
includes those unused imports.
| implementation(projects.core.designsystem) | ||
| implementation(projects.domain.user) | ||
|
|
||
| implementation(libs.naver.oauth) |
There was a problem hiding this comment.
중복된 의존성을 제거하세요.
libs.naver.oauth가 Line 14와 Line 21에서 중복으로 선언되어 있습니다. 중복 항목을 제거해야 합니다.
implementation(libs.kakao.login)
implementation(libs.kakao.auth)
implementation(libs.kakao.common)
-
- implementation(libs.naver.oauth)
// implementation(libs.naver.jdk)Also applies to: 21-21
🤖 Prompt for AI Agents
In feature/signup/build.gradle.kts around lines 14 and 21, the dependency
libs.naver.oauth is declared twice; remove the duplicate declaration so the
dependency is only declared once (delete either the Line 14 or Line 21 entry),
ensuring the remaining dependency is kept in the appropriate configuration and
the file compiles.
| fun startSocialLogin( | ||
| type: SocialLoginType, | ||
| activity: Activity, | ||
| ) = viewModelScope.launch { | ||
| lastSocialLoginType = type | ||
|
|
||
| if(type == SocialLoginType.KAKAO) { | ||
| try { | ||
| UserApiClient.instance.loginWithKakaoAccount(context) { token, error -> | ||
| if (error != null) { | ||
| Log.e(TAG, "로그인 실패", error) | ||
| } else if (token != null) { | ||
| Log.i(TAG, "로그인 성공 ${token.accessToken}") | ||
| viewModelScope.launch { | ||
| runCatching { | ||
| // 보통은 accessToken만 넘김 | ||
| loginRepository.finishSocialLogin( | ||
| type = lastSocialLoginType!!, | ||
| accessToken = token.accessToken | ||
| ) | ||
| }.onSuccess { | ||
| _events.emit(LoginEvent.LoginSucceeded) | ||
| }.onFailure { e -> | ||
| Log.e(TAG, "서버 로그인 실패", e) | ||
| _events.emit(LoginEvent.ShowError("서버 로그인 실패: ${e.message}")) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
|
|
||
| } | ||
| } else if (type == SocialLoginType.NAVER) { | ||
| val nidOAuthCallback = object : NidOAuthCallback { | ||
| override fun onSuccess() { | ||
| val accessToken = NidOAuth.getAccessToken() | ||
|
|
||
| if (accessToken.isNullOrBlank()) { | ||
| Log.e(TAG, "네이버 로그인 성공했지만 accessToken이 null/blank") | ||
| viewModelScope.launch { | ||
| _events.emit(LoginEvent.ShowError("네이버 로그인 토큰을 가져오지 못했습니다.")) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| Log.i(TAG, "네이버 로그인 성공 accessToken = $accessToken") | ||
|
|
||
| // 카카오와 동일하게 서버 로그인 처리 | ||
| viewModelScope.launch { | ||
| runCatching { | ||
| loginRepository.finishSocialLogin( | ||
| type = lastSocialLoginType!!, | ||
| accessToken = accessToken | ||
| ) | ||
| }.onSuccess { | ||
| _events.emit(LoginEvent.LoginSucceeded) | ||
| }.onFailure { e -> | ||
| Log.e(TAG, "네이버 서버 로그인 실패", e) | ||
| _events.emit(LoginEvent.ShowError("서버 로그인 실패: ${e.message}")) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
lastSocialLoginType!! 제거 및 Kakao 로그인 실패 시 이벤트 발행 정리 권장
현재 finishSocialLogin 호출 시:
loginRepository.finishSocialLogin(
type = lastSocialLoginType!!,
accessToken = token.accessToken
)처럼 lastSocialLoginType!!에 의존하고 있습니다. 이 방식은:
- 불필요한 전역 상태(
lastSocialLoginType)를 유지해야 하고, - 향후 코드 변경/동시 호출 시
null또는 엉뚱한 타입이 들어갈 여지가 있으며, - 이미 매개변수로
type: SocialLoginType을 받고 있으므로 중복입니다.
더 안전하고 단순하게는, lastSocialLoginType을 없애고 type을 그대로 캡처해서 넘기는 쪽이 좋습니다.
fun startSocialLogin(
type: SocialLoginType,
activity: Activity,
) = viewModelScope.launch {
if (type == SocialLoginType.KAKAO) {
UserApiClient.instance.loginWithKakaoAccount(context) { token, error ->
if (error != null) {
// 아래 ShowError emit 추가
} else if (token != null) {
viewModelScope.launch {
runCatching {
loginRepository.finishSocialLogin(
type = type, // ✅ 직접 전달
accessToken = token.accessToken
)
} ...
}
}
}
} else if (type == SocialLoginType.NAVER) {
...
loginRepository.finishSocialLogin(
type = type, // ✅ 직접 전달
accessToken = accessToken
)
}
}또한 Kakao 쪽은:
- 콜백
error != null인 경우, 그리고try { ... } catch (e: Exception)블록에서 - 단순히
Log.e만 찍고,LoginEvent.ShowError를 emit 하지 않습니다.
Naver 분기에서는 실패 시 ShowError를 emit 하고 있으니, Kakao 분기도 동일하게 ShowError 이벤트를 발행해 주면 UX가 일관되고, 화면에서 에러 처리하기도 수월합니다.
🤖 Prompt for AI Agents
feature/signup/src/main/java/com/umcspot/spot/landing/LoginViewmodel.kt lines
40-101: replace uses of the nullable global lastSocialLoginType!! by directly
passing the local parameter type into finishSocialLogin (and remove the
unnecessary lastSocialLoginType state if unused elsewhere), and ensure Kakao
failure paths mirror Naver by emitting LoginEvent.ShowError when the Kakao
callback returns error and inside the surrounding catch block (log as before,
then _events.emit(LoginEvent.ShowError(...)) with a meaningful message).
| if(type == SocialLoginType.KAKAO) { | ||
| try { | ||
| UserApiClient.instance.loginWithKakaoAccount(context) { token, error -> | ||
| if (error != null) { | ||
| Log.e(TAG, "로그인 실패", error) | ||
| } else if (token != null) { | ||
| Log.i(TAG, "로그인 성공 ${token.accessToken}") | ||
| viewModelScope.launch { | ||
| runCatching { | ||
| // 보통은 accessToken만 넘김 | ||
| loginRepository.finishSocialLogin( | ||
| type = lastSocialLoginType!!, | ||
| accessToken = token.accessToken | ||
| ) | ||
| }.onSuccess { | ||
| _events.emit(LoginEvent.LoginSucceeded) | ||
| }.onFailure { e -> | ||
| Log.e(TAG, "서버 로그인 실패", e) | ||
| _events.emit(LoginEvent.ShowError("서버 로그인 실패: ${e.message}")) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
|
|
||
| } | ||
| } else if (type == SocialLoginType.NAVER) { | ||
| val nidOAuthCallback = object : NidOAuthCallback { | ||
| override fun onSuccess() { | ||
| val accessToken = NidOAuth.getAccessToken() | ||
|
|
||
| if (accessToken.isNullOrBlank()) { | ||
| Log.e(TAG, "네이버 로그인 성공했지만 accessToken이 null/blank") | ||
| viewModelScope.launch { | ||
| _events.emit(LoginEvent.ShowError("네이버 로그인 토큰을 가져오지 못했습니다.")) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| Log.i(TAG, "네이버 로그인 성공 accessToken = $accessToken") | ||
|
|
||
| // 카카오와 동일하게 서버 로그인 처리 | ||
| viewModelScope.launch { | ||
| runCatching { | ||
| loginRepository.finishSocialLogin( | ||
| type = lastSocialLoginType!!, | ||
| accessToken = accessToken | ||
| ) | ||
| }.onSuccess { | ||
| _events.emit(LoginEvent.LoginSucceeded) | ||
| }.onFailure { e -> | ||
| Log.e(TAG, "네이버 서버 로그인 실패", e) | ||
| _events.emit(LoginEvent.ShowError("서버 로그인 실패: ${e.message}")) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
액세스 토큰을 로그에 출력하면 보안상 매우 위험합니다
카카오/네이버 로그인 성공 시:
- Line 52:
Log.i(TAG, "로그인 성공 ${token.accessToken}") - Line 85:
Log.i(TAG, "네이버 로그인 성공 accessToken = $accessToken")
처럼 액세스 토큰 전체를 로그에 남기고 있습니다. 액세스 토큰은 사실상 비밀번호와 동일한 민감 자격 증명이라, 로그/Crashlytics 등에 남으면 유출 시 계정 탈취 위험이 큽니다.
운영 전까지는 반드시:
- 토큰 전체를 로그에서 제거하거나,
- 디버깅이 꼭 필요하다면 앞 몇 글자만 남기고 나머지는 마스킹하는 형태로 변경
하는 것을 강하게 권장드립니다.
예시:
// before
Log.i(TAG, "로그인 성공 ${token.accessToken}")
// after (마스킹 예시)
Log.i(TAG, "로그인 성공 (provider=KAKAO, tokenPrefix=${token.accessToken.take(4)})")네이버 쪽도 동일하게 처리해 주시면 좋겠습니다.
🤖 Prompt for AI Agents
In feature/signup/src/main/java/com/umcspot/spot/landing/LoginViewmodel.kt
around lines 46-101, the code logs full OAuth access tokens on successful login
(lines ~52 and ~85); remove printing the entire token and instead either omit
the token from logs or log only a masked prefix (e.g., provider name and first N
chars) to avoid leaking sensitive credentials — update both the Kakao and Naver
success branches to replace full-token log statements with masked/omitted
versions and ensure no other logs/crash reports capture the full token.
| fun saveNameIfChanged() { | ||
| val (original, current) = getNamePair() | ||
|
|
||
| if (current.isNullOrBlank() || current == original) return | ||
|
|
||
| viewModelScope.launch { | ||
| userRepository.setUserName(current) | ||
| .onSuccess { | ||
| Log.d("ChangeName", "이름 변경 성공") | ||
| // 서버에도 저장됐으니, 로컬 state도 맞춰주기 | ||
| _name.update { | ||
| it.copy( | ||
| user = UiState.Success(current), | ||
| originalName = current, | ||
| currentName = current | ||
| ) | ||
| } | ||
| } | ||
| .onFailure { e -> | ||
| Log.e("ChangeName", "이름 변경 실패", e) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
| fun getNamePair(): Pair<String?, String?> { | ||
| Log.d("NamePair", "${_name.value.originalName} ::: ${_name.value.currentName}") | ||
| return _name.value.originalName to _name.value.currentName | ||
| } |
There was a problem hiding this comment.
사용자 이름을 그대로 로그에 남기는 것은 PII 측면에서 위험
saveNameIfChanged와 getNamePair에서 "ChangeName", "NamePair" 태그로 사용자 실명을 그대로 로그에 남기고 있습니다. 이름도 개인정보(PII)에 해당하므로, 운영 환경에서 로그 수집·보관 시 이슈가 될 수 있습니다.
운영 빌드 기준으로는:
- 해당 로그를 제거하거나,
- 최소한 이름은 마스킹/익명화(예: 앞 한 글자만 남기고
***처리)하는 방향
으로 조정하는 것을 권장드립니다. 실패 로그는 예외 메시지/에러 코드만 남겨도 디버깅에는 큰 지장이 없을 것 같습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt (1)
83-83: 불필요한 명시적 파라미터 전달
spacing파라미터는 이미FabStack함수에서 기본값12.dp로 정의되어 있으므로, 명시적으로 전달할 필요가 없습니다.다음 diff를 적용하여 중복 코드를 제거하세요:
FabStack( showToTop = navigator.showToTopFab(), onClickToTop = { scrollToTop?.invoke() }, showMultiple = navigator.showMultipleFab(), onClickMultiple = { /* TODO */ }, - spacing = 12.dp, )feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt (2)
36-67: 이벤트 수집 구조는 적절하지만 LaunchedEffect 키와 에러 처리 보완을 권장합니다
LandingScreen에서 ViewModel 주입 + UI 분리는 방향성 좋습니다.- 다만
LaunchedEffect(Unit)은 최초 구성 시점의onLoginSuccess람다를 캡처하고, 이후 상위에서 다른 람다를 넘겨도 다시 구독되지 않습니다. 안정성을 위해viewModel,onLoginSuccess를 키로 사용하는 쪽을 권장합니다.- LaunchedEffect(Unit) { + LaunchedEffect(viewModel, onLoginSuccess) { viewModel.events.collect { ev -> when (ev) { is LandingViewModel.LoginEvent.LoginSucceeded -> { onLoginSuccess() } is LandingViewModel.LoginEvent.ShowError -> { - } + // TODO: 스낵바 / 다이얼로그 등으로 에러 노출 고려 + // ex) scaffoldState.snackbarHostState.showSnackbar(ev.message) + } } } }
- 또한
activity가 null인 경우 클릭이 조용히 무시되는데, 이 상황이 실제 런타임에서도 가능하다면(특수한 호스팅 컨텍스트 등) 로그 또는 예외로 빨리 드러내는 것도 고려해볼 수 있습니다.[uggest_recommended_refactor]
36-67: 이벤트 수집 구조는 적절하지만 LaunchedEffect 키와 에러 처리 보완을 권장합니다 (수정된 코멘트)
LandingScreen에서 ViewModel 주입 + UI 분리는 방향성 좋습니다.- 다만
LaunchedEffect(Unit)은 최초 구성 시점의onLoginSuccess람다를 캡처하고, 이후 상위에서 다른 람다를 넘겨도 다시 구독되지 않습니다. 안정성을 위해viewModel,onLoginSuccess를 키로 사용하는 쪽을 권장합니다.- LaunchedEffect(Unit) { + LaunchedEffect(viewModel, onLoginSuccess) { viewModel.events.collect { ev -> when (ev) { is LandingViewModel.LoginEvent.LoginSucceeded -> { onLoginSuccess() } is LandingViewModel.LoginEvent.ShowError -> { - } + // TODO: 스낵바 / 다이얼로그 등으로 에러 노출 고려 + // ex) scaffoldState.snackbarHostState.showSnackbar(ev.message) + } } } }
activity가 null인 경우 클릭이 조용히 무시되는데, 이 상황이 실제 런타임에서도 가능하다면(특수한 호스트 컨텍스트 등) 로그를 남기거나 즉시 실패하도록 처리해 두면 디버깅에 도움이 됩니다.feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt (1)
82-88: 랜딩/온보딩 구간 및 TopBar 노출 조건 재점검 제안
isInLanding()이Landing·Saving만 포함하고,showBackTopBar()는SignUp,CheckList,RegisterStudy만 포함하며Saving은 제외되어 있습니다. 온보딩 플로우가Landing → SignUp → CheckList → Saving → Home구조라면,
- SignUp / CheckList도 “랜딩/온보딩 단계”로 취급해야 하는지 (예: Activity/Scaffold에서
isInLanding()을 기준으로 시스템 UI, 하단바 노출 등을 제어하는 경우),- Saving 단계에서만 TopBar 뒤로가기를 숨기려는 의도인지 (실제 시스템 뒤로가기는 여전히 가능하므로 UX 일관성 관점에서 괜찮은지)
한 번 더 기획/디자인과 맞춰보시면 좋겠습니다. 의도와 다르다면, 공통 온보딩 그래프 집합(예:
private val onboardingGraphs = arrayOf(Landing::class, SignUp::class, CheckList::class, Saving::class))을 상수로 두고isInLanding/showBackTopBar가 이를 공유하게 하면 조건 관리가 더 명확해질 것 같습니다.Also applies to: 92-92
feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (1)
50-61: 로그인/온보딩 완료 시 back stack 정리 로직 통합 권장
landingGraph의onLoginSuccess와savingGraph의onFinished에서 모두
navigator.navController.graph.findStartDestination().id기준popUpTo(..., inclusive = true)를 호출하고,launchSingleTop = true,restoreState = false를 설정해각각 SignUp과 Home을 새로운 루트로 만드는 동일한 패턴을 사용하고 있습니다. 동작 자체는 “로그인/온보딩 이후 뒤로가기로 랜딩/온보딩 스택으로 돌아가지 않기”라는 목적에 잘 맞지만, 사실상 동일한
navOptions블록이 두 군데에 복붙되어 있어 startDestination이나 옵션을 바꿀 때 동기화 누락 위험이 있습니다.MainNavigator 쪽에
navigateToSignUpAfterLogin()/navigateToHomeAfterOnboarding()같은 헬퍼를 두거나, 이미 존재하는navigateToHomeAfterLogin()을 활용해 한 곳에서만popUpTo기준과 옵션을 관리하는 구조로 정리하면 유지보수성이 더 좋아질 것 같습니다.Also applies to: 76-86
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt(3 hunks)feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt(2 hunks)feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt(3 hunks)feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (3)
feature/signup/src/main/java/com/umcspot/spot/signup/navigation/SignUpNavigation.kt (1)
signupGraph(20-38)feature/signup/src/main/java/com/umcspot/spot/checkList/navigation/CheckListNavigation.kt (1)
checkListGraph(20-38)feature/signup/src/main/java/com/umcspot/spot/landing/navigation/SavingNavigation.kt (1)
savingGraph(16-26)
🔇 Additional comments (5)
feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt (1)
26-26: LGTM! 온보딩 플로우를 위한 라우팅 지원이 올바르게 추가되었습니다.SignUp 및 CheckList 네비게이션 컴포넌트의 import와 해당 라우트에 대한 top bar 제목 처리가 기존 패턴을 따르고 있으며, 새로운 회원가입 플로우를 적절하게 지원합니다.
Also applies to: 32-32, 56-57
feature/signup/src/main/java/com/umcspot/spot/landing/LandingScreen.kt (3)
3-35: 새로 추가된 의존성(import) 구성은 적절해 보입니다
Activity,LaunchedEffect,LocalContext,hiltViewModel,SocialLoginType모두 아래 코드에서 사용되고 있고, 불필요한 의존성이나 순환 의존은 없어 보입니다. 이 수준에서는 그대로 유지해도 무방해 보입니다.
69-73: UI/로직 분리 및 클릭 핸들러 위임 방식이 깔끔합니다
LandingScreenContent로 순수 UI를 분리하고,onKakaoClick/onNaverClick만 상위에서 주입하는 패턴은 테스트·프리뷰·재사용성 관점에서 좋습니다. 하위 컴포저블이 ViewModel이나Activity를 직접 알지 않는 구조도 유지하면 좋겠습니다.Also applies to: 119-120
128-130: 프리뷰에서 ViewModel 의존성을 제거한 점이 좋습니다
LandingScreenPreview가LandingScreenContent만 호출하도록 바뀌면서 Hilt/ViewModel에 의존하지 않고, 프리뷰 안정성이 높아졌습니다.private으로 스코프를 줄인 것도 적절합니다.feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (1)
64-75: 온보딩(SignUp → CheckList → Saving) 그래프 연결 구조 적절해 보입니다
signupGraph/checkListGraph에navigator.navController와contentPadding을 그대로 넘기고,
- SignUp의
onNextClick에서navigateToCheckList(),- CheckList의
onNextClick에서navigateToSaving()으로 이어지는 플로우가 명확하게 구성되어 있어 읽기 쉽고 유지보수에도 유리해 보입니다. 실제 기기에서 뒤로가기 동작(예: Saving → CheckList → SignUp 역방향)이 기획 의도와 정확히 일치하는지만 한 번만 확인해 보시면 좋겠습니다.
2 participants
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
PR Point 📌
트러블 슈팅 💥
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.