Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough온보딩 설문조사 기능의 전체 스택을 추가·연결했습니다. UI 컴포저블들, ViewModel, 도메인 엔티티·매퍼, 원격 서비스·데이터소스·리포지토리, 인증 인터셉터 및 Hilt 바인딩, 관련 리소스와 Modifier 확장 등이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as SurveyMainScreen (UI)
participant VM as SurveyViewModel (Presentation)
participant Repo as SurveyRepositoryImpl (Repository)
participant Remote as SurveyRemoteDataSourceImpl (RemoteDS)
participant Service as SurveyService (Retrofit)
participant API as Backend API
User->>UI: 설문 시작/질문 선택
UI->>VM: loadSurveyQuestions() / updateAnswer(...)
VM->>Repo: getSurveyQuestions()
Repo->>Remote: getSurveyQuestions()
Remote->>Service: getSurveyQuestions()
Service->>API: GET /api/v1/surveys/questions
API-->>Service: SurveyQuestionsResponse
Service-->>Remote: ApiResponse
Remote-->>Repo: ApiResponse
Repo->>Repo: DTO -> Domain (toDomain())
Repo-->>VM: Result<SurveyQuestions>
VM-->>UI: questions StateFlow 업데이트
User->>UI: 마지막 질문 제출
UI->>VM: submitSurvey()
VM->>Repo: submitSurvey(answers)
Repo->>Remote: submitSurvey(request)
Remote->>Service: submitSurvey(request)
Service->>API: POST /api/v1/surveys (AuthInterceptor adds header)
API-->>Service: SurveyResponse
Service-->>Remote: ApiResponse
Remote-->>Repo: ApiResponse
Repo->>Repo: DTO -> Domain (toDomain())
Repo-->>VM: Result<SurveyResult>
VM->>UI: uiState -> Success(result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/gildongmu/dduru/data/di/NetworkModule.kt (1)
67-71:⚠️ Potential issue | 🟠 Major
SurveyService에 대한@Provides함수가 누락되었습니다.
AuthService는 제공되지만SurveyService는 제공되지 않습니다.SurveyRemoteDataSourceImpl이SurveyService를 주입받으려면 provider가 필요합니다.🔧 SurveyService provider 추가 제안
`@Provides` `@Singleton` fun provideAuthService(retrofit: Retrofit): AuthService { return retrofit.create(AuthService::class.java) } + + `@Provides` + `@Singleton` + fun provideSurveyService(retrofit: Retrofit): SurveyService { + return retrofit.create(SurveyService::class.java) + } }
SurveyServiceimport도 추가해야 합니다:import com.gildongmu.dduru.data.remote.service.SurveyService
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/gildongmu/dduru/data/model/SurvayState.kt`:
- Around line 21-23: The validation currently only checks for null singleAnswers
and empty q7 but misses enforcing the Q7 "max 3 selections" rule; update the
validation in SurvayState (the block using singleAnswers and q7) to return null
when q7.size > 3 (i.e., add a check like q7.size > 3 to the existing if
condition or a separate guard) so requests that select more than three items are
rejected before creation.
- Around line 18-37: The toRequest() method on SurveyState is exposing a
data/remote DTO (SurveyRequest) to the presentation layer; refactor by removing
direct DTO creation from SurveyState and moving the conversion into a
repository/mapper layer: keep SurveyState as a presentation-only model (or
create a new UI model), delete or deprecated SurveyState.toRequest(), and
implement a new mapper function (e.g., SurveyMapper.toSurveyRequest(uiState) or
in SurveyRepository.buildSurveyRequest(uiState)) that validates q1..q11
(existing symbols q1..q11, q7.toList()) and returns SurveyRequest; update
callers (ViewModel) to send the UI model to Repository/Mapper instead of calling
toRequest() directly.
In `@app/src/main/java/com/gildongmu/dduru/data/remote/service/SurveyService.kt`:
- Around line 11-20: The NetworkModule is missing a provider for SurveyService
which causes DI failures when SurveyRemoteDataSourceImpl expects it; add a
provider function similar to provideAuthService in the same module: create a
`@Provides` `@Singleton` fun provideSurveyService(retrofit: Retrofit): SurveyService
that returns retrofit.create(SurveyService::class.java) so SurveyService can be
injected into SurveyRemoteDataSourceImpl.
In
`@app/src/main/java/com/gildongmu/dduru/data/repositoryimpl/SurveyRepositoryImpl.kt`:
- Line 16: In SurveyRepositoryImpl, update the status-code check in the
submitSurvey flow: replace the current response.status == 201 check with
response.status == 200 so it matches the project's backend behavior and stays
consistent with getMySurveyResult and getSurveyQuestions; ensure the null check
on response.data remains intact and adjust any related branching that assumes a
201 success code.
In `@app/src/main/java/com/gildongmu/dduru/domain/repository/SurveyRepository.kt`:
- Around line 3-10: The repository interface currently leaks data layer DTOs;
change SurveyRepository so its methods return/accept domain models (e.g.,
replace SurveyRequest/SurveyResponse/SurveyQuestionsResponse with domain types
like SurveyCommand/SurveyResult/SurveyQuestions) for submitSurvey,
getMySurveyResult and getSurveyQuestions, then implement DTO↔domain mapping
inside SurveyRepositoryImpl (map incoming domain request to SurveyRequest DTO
before calling remote data source and map returned
SurveyResponse/SurveyQuestionsResponse DTOs into domain
SurveyResult/SurveyQuestions in the implementation). Ensure only domain types
appear in this interface and keep mapping functions (or mappers) inside
SurveyRepositoryImpl.
In
`@app/src/main/java/com/gildongmu/dduru/domain/usecase/survey/SubmitSurveyUseCase.kt`:
- Around line 3-4: SubmitSurveyUseCase currently depends on data layer DTOs
(SurveyRequest, SurveyResponse); create domain models (e.g., SurveySubmission,
SurveyResult) and change the use case signature to accept/return those domain
types instead; update the domain SurveyRepository interface to suspend fun
submitSurvey(submission: SurveySubmission): Result<SurveyResult>; move DTO ↔
domain mapping into the data-layer implementation (SurveyRepositoryImpl) so
SubmitSurveyUseCase and other domain code no longer import
com.gildongmu.dduru.data.remote.dto.*; mirror how AuthRepositoryImpl maps DTOs
to domain in LoginGoogleUseCase.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/PenguinCharacter.kt`:
- Around line 24-30: The Modifier chain in PenguinCharacter (the modifier
variable passed into the composable) applies .background() before .size(), which
can cause the background shape to not match the intended layout; reorder the
chain so .size(width = 48.dp, height = 60.dp) comes before .background(color =
DduruTheme.colors.gray50, shape = RoundedCornerShape(99.dp)), and keep
.padding(8.dp) after the background so the background covers the sized area and
padding is applied inside it.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/QuestionCard.kt`:
- Around line 3-5: The import androidx.compose.foundation.layout.* is a wildcard
— remove it and replace it with explicit imports for only the layout symbols
used in this file (e.g., Column, Row, Box, Spacer, Arrangement, Alignment,
Modifier.padding, Modifier.fillMaxWidth, etc. as actually referenced by the
QuestionCard composable and any helper functions in QuestionCard.kt); update the
import block to list each needed androidx.compose.foundation.layout.X explicitly
so ktlint no-wildcard-imports rule passes.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/QuestionNumberBadge.kt`:
- Around line 3-5: There are wildcard imports (e.g.,
androidx.compose.foundation.layout.*) which violate ktlint no-wildcard-imports;
edit QuestionNumberBadge.kt to remove any '*' imports and replace them with
explicit imports for only the layout symbols used in this file (for example
import androidx.compose.foundation.layout.Row,
androidx.compose.foundation.layout.Column,
androidx.compose.foundation.layout.Spacer,
androidx.compose.foundation.layout.size,
androidx.compose.foundation.layout.padding,
androidx.compose.foundation.layout.Arrangement, androidx.compose.ui.Alignment if
those are used). Keep existing explicit imports like RoundedCornerShape and
background, and ensure only the exact classes/functions referenced by the
QuestionNumberBadge composable are imported.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/QuestionTitle.kt`:
- Around line 6-7: Remove the unused imports TextStyle and FontWeight from the
top of QuestionTitle.kt: locate the import lines that reference
androidx.compose.ui.text.TextStyle and androidx.compose.ui.text.font.FontWeight
and delete them (or run an auto-organize-imports/optimize-imports action) after
confirming no usages of TextStyle or FontWeight remain in the QuestionTitle
composable.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/SurveyQuestionLayout.kt`:
- Around line 3-6: The file SurveyQuestionLayout.kt currently uses a wildcard
import (androidx.compose.foundation.layout.*); replace that wildcard with
explicit imports for only the layout symbols actually used in this file (for
example Column, Row, Box, Spacer, Arrangement, Alignment, PaddingValues or
whatever layout composables/modifier extensions are referenced by the
SurveyQuestionLayout composable), so the ktlint no-wildcard-imports rule is
satisfied.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyQuestionScreen.kt`:
- Around line 3-16: Replace the wildcard import
"com.gildongmu.dduru.presentation.feature.register.survey.components.*" in
SurveyQuestionScreen.kt with explicit imports for each component actually used
by the SurveyQuestionScreen composable (remove the * and add one import line per
symbol from that package). Locate the SurveyQuestionScreen composable and the
usages of components from
com.gildongmu.dduru.presentation.feature.register.survey.components, then add
explicit import statements for those exact class/function names so ktlint
no-wildcard-imports rule is satisfied.
- Around line 68-109: MultiSelectContent에서 SurveyGridOptionButton의 클릭 콜백에 현재
options.indexOf(option)을 전달하고 있어 isSelected(선택 비교에 사용하는 option.code)와 불일치합니다;
SurveyGridOptionButton의 onClick에는 항목의 인덱스 대신 option.code를 전달하도록 변경하고, 이에 따라
ViewModel의 updateAnswer와 toggleQ7Answer 내부에서 받는 값에 대한 인덱스 보정(+1 등) 로직을 제거해 전달값을
서버 코드에서 직접 사용하도록 일관되게 맞추세요 (참조 심볼: MultiSelectContent, SurveyGridOptionButton,
option.code, onOptionToggled, selectedOptions, updateAnswer, toggleQ7Answer).
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyResultScreen.kt`:
- Around line 3-6: The current imports use wildcards (e.g.,
androidx.compose.foundation.layout.*, androidx.compose.material3.*,
androidx.compose.runtime.*); replace each wildcard import with explicit imports
for only the symbols actually used in SurveyResultScreen.kt (for example Column,
Row, Spacer, Box, Modifier, background, padding, Text, Button, Surface,
MaterialTheme, remember, LaunchedEffect, etc.) so the file no longer relies on
wildcard imports and conforms to ktlint's no-wildcard-imports rule; update the
import block at the top of SurveyResultScreen.kt to list each required symbol
explicitly and remove the wildcard lines.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyMainScreen.kt`:
- Around line 3-11: Replace the wildcard import "import
com.gildongmu.dduru.presentation.feature.register.survey.screen.*" with explicit
imports for each symbol the file uses from that package (import each composable,
class, or object by name as referenced in SurveyMainScreen.kt); remove the .*
entry and add one import line per referenced symbol to satisfy the
ktlint_standard_no-wildcard-imports rule enforced by .editorconfig.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt`:
- Around line 135-156: The loadMySurveyResult() function is defined but unused;
either invoke it from the appropriate lifecycle or action handler (for example
call SurveyViewModel.loadMySurveyResult() from the ViewModel init block, from
the Fragment/Activity when the survey screen is shown, or from an existing
method like refreshSurvey() / onViewCreated) or remove the function if it's not
needed. Locate the function by its name loadMySurveyResult and update the caller
(e.g., add viewModel.loadMySurveyResult() in the UI layer or call it inside the
SurveyViewModel init) or delete the method and any related unused imports to
keep the codebase clean.
🧹 Nitpick comments (15)
app/src/main/java/com/gildongmu/dduru/data/model/SurvayState.kt (1)
32-32: Q7 리스트 변환 시 순서 고정 권장
Set의toList()는 순서가 비결정적입니다. 서버가 순서에 민감하거나 테스트 스냅샷 비교가 있다면 정렬로 순서를 고정하는 편이 안전합니다.♻️ 정렬로 순서 고정
- q7 = q7.toList(), + q7 = q7.sorted(),app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/BackButton.kt (1)
26-49: Modifier 체인 순서 검토현재 Modifier 순서에서
.size(48.dp)가.shadow()뒤에 적용되어 있습니다. Compose에서 Modifier는 순서대로 적용되므로, shadow가 size 이전에 적용되면 예상치 못한 그림자 경계가 발생할 수 있습니다.또한
.clickable(onClick = onClick)은 기본 ripple 효과를 사용합니다. 이 PR에서 추가된noRippleClickable확장 함수와의 일관성을 고려해보세요.♻️ 제안된 Modifier 순서 조정
Box( modifier = modifier + .size(48.dp) .shadow( elevation = 16.dp, spotColor = DduruTheme.colors.blackAlpha10, - ambientColor = DduruTheme.colors.blackAlpha10 + ambientColor = DduruTheme.colors.blackAlpha10, + shape = RoundedCornerShape(size = 99.dp) ) .shadow( elevation = 8.dp, spotColor = DduruTheme.colors.blackAlpha05, - ambientColor = DduruTheme.colors.blackAlpha05 + ambientColor = DduruTheme.colors.blackAlpha05, + shape = RoundedCornerShape(size = 99.dp) ) - .size(48.dp) .border(app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/QuestionTitle.kt (1)
19-21: lineHeight 매직 넘버 개선 고려
(18 * 1.8).sp계산식의 의도가 명확하지 않습니다. 가독성을 위해 상수나 주석으로 의도를 명시하거나, 디자인 시스템의 lineHeight 값을 활용하는 것을 고려해보세요.app/src/main/java/com/gildongmu/dduru/data/remote/dto/response/SurveyResponse.kt (1)
6-19: DTO 속성 명명 및 일관성 검토
r,w,s,p속성명이 의미를 파악하기 어렵습니다. 도메인 의미를 명확히 하는 이름(예:relaxScore,workScore등)이나 최소한 KDoc 주석 추가를 권장합니다.
SurveyRequest는@SerialName을 사용하지만, 이 DTO는 사용하지 않습니다. JSON 키와 속성명이 동일하더라도 명시적인@SerialName사용은 일관성과 리팩토링 안전성을 높여줍니다.서버에서 null 응답 시 역직렬화 실패 가능성이 있습니다. 선택적 필드에 대해 기본값 설정을 고려해보세요.
app/src/main/java/com/gildongmu/dduru/data/remote/dto/response/SurveyQuestionResponse.kt (1)
13-24:type필드 타입 안전성 고려
type이String으로 정의되어 있습니다. 질문 유형이 한정적이라면(예: "single", "multi"), sealed class나 enum으로 정의하면 타입 안전성과 컴파일 타임 검증이 가능합니다.현재 구현도 유효하며, 서버에서 새로운 타입 추가 시 유연성을 제공합니다. 추후 도메인 모델로 매핑 시 enum 변환을 고려해볼 수 있습니다.
app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/SurveyOptionButton.kt (1)
46-52: Text 컴포저블의 들여쓰기가 일관성이 없습니다.
style파라미터(line 48)와 이후 파라미터들(lines 49-51)의 들여쓰기가 불일치합니다. 코드는 동작하지만 가독성을 위해 정렬을 맞춰주세요.♻️ 들여쓰기 수정 제안
Text( text = text, style = DduruTheme.typography.Title1, - color = if (isSelected) DduruTheme.colors.primary - else DduruTheme.colors.gray700, - textAlign = TextAlign.Center, - ) + color = if (isSelected) DduruTheme.colors.primary + else DduruTheme.colors.gray700, + textAlign = TextAlign.Center + )app/src/main/java/com/gildongmu/dduru/data/di/NetworkModule.kt (1)
36-39: 인터셉터 순서: 로깅 인터셉터가 인증 헤더를 로깅합니다.
HttpLoggingInterceptor가AuthInterceptor앞에 추가되어 있어, 인증 토큰이 로그에 출력됩니다. 디버그 빌드에서는 괜찮지만, 로그가 외부로 유출되지 않도록 주의하세요.app/src/main/java/com/gildongmu/dduru/data/remote/interceptor/AuthInterceptor.kt (1)
14-17: OkHttp 인터셉터에서runBlocking으로 DataStore 접근 시 네트워크 스레드 차단
TokenLocalDataSource가 Android DataStore를 사용하므로,runBlocking으로 토큰을 동기적으로 읽을 때 OkHttp의 디스패처 스레드가 차단됩니다. DataStore는 디스크 기반 저장소로, 특히 앱 시작 후 첫 요청에서 I/O 지연이 발생할 수 있습니다.토큰을 메모리에 캐싱하고 Flow로 동기화하거나, 토큰 갱신 시점에 미리 로드하는 방식으로 개선할 수 있습니다. 현재 패턴도 일반적이지만, 네트워크 성능이 중요한 상황에서는 리팩토링을 권장합니다.
app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyResultScreen.kt (1)
21-26: 분석 단계 상태를 구성 변경에서도 유지하는 것을 권장합니다.회전/프로세스 복원 시
isAnalyzing이 초기화되어 다시 분석 화면이 보일 수 있습니다.rememberSaveable로 상태를 보존하는 편이 UX에 유리합니다.♻️ 제안 수정
- var isAnalyzing by remember { mutableStateOf(true) } + var isAnalyzing by rememberSaveable { mutableStateOf(true) }app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyMainScreen.kt (3)
32-33: 스마트 캐스트 후 중복 캐스트를 제거하세요.
is검사 후 Kotlin은 자동으로 스마트 캐스트를 적용하므로 명시적 캐스트가 불필요합니다.when표현식을 사용하면 더 깔끔합니다.♻️ 스마트 캐스트 활용
- uiState is SurveyUiState.Success -> { - val data = (uiState as SurveyUiState.Success).result + is SurveyUiState.Success -> { + val data = uiState.result참고:
when블록 내에서uiState를 subject로 사용하면 스마트 캐스트가 자동 적용됩니다:when (uiState) { is SurveyUiState.Success -> { val data = uiState.result // ... } // ... }
122-128: 테마 색상을 사용하고 스마트 캐스트를 활용하세요.
Color.Red직접 참조 대신DduruTheme.colors에서 에러 색상을 사용하는 것이 일관성에 좋습니다.- Line 124에서도 스마트 캐스트 활용이 가능합니다.
♻️ 제안
- if (uiState is SurveyUiState.Error) { + (uiState as? SurveyUiState.Error)?.let { errorState -> Text( - text = (uiState as SurveyUiState.Error).message, + text = errorState.message, modifier = Modifier .align(Alignment.BottomCenter) .padding(20.dp), - color = androidx.compose.ui.graphics.Color.Red + color = DduruTheme.colors.error // 또는 적절한 테마 색상 ) }
71-83: selectedOption 매핑 로직을 간소화하는 것을 고려하세요.현재
when블록이 각 질문 ID를 개별적으로 처리하고 있어 q7을 제외한 모든 질문에 대해 반복적입니다.SurveyState에 맵 기반 접근자를 추가하면 이 코드를 단순화할 수 있습니다.app/src/main/java/com/gildongmu/dduru/data/repositoryimpl/SurveyRepositoryImpl.kt (1)
21-27: 원본 예외를 보존하여 디버깅을 용이하게 하세요.
IOException을 포함한 예외들이 원본 예외 정보 없이 새Exception으로 래핑되고 있습니다. 원본 예외를cause로 전달하면 스택 트레이스가 보존되어 디버깅에 도움이 됩니다.♻️ 예외 체이닝 적용
} catch (e: retrofit2.HttpException) { - Result.failure(Exception("서버 오류: ${e.code()} - ${e.message()}")) + Result.failure(Exception("서버 오류: ${e.code()} - ${e.message()}", e)) } catch (e: java.io.IOException) { - Result.failure(Exception("네트워크 오류: 인터넷 연결을 확인해주세요.")) + Result.failure(Exception("네트워크 오류: 인터넷 연결을 확인해주세요.", e)) } catch (e: Exception) { - Result.failure(Exception("알 수 없는 오류: ${e.message}")) + Result.failure(Exception("알 수 없는 오류: ${e.message}", e)) }이 수정은
getMySurveyResult()(Lines 40-50)와getSurveyQuestions()(Lines 61-67)에도 동일하게 적용하세요.app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt (2)
5-8: ViewModel이 Data Layer DTO에 직접 의존하고 있습니다.Google 권장 아키텍처에 따르면, Presentation Layer(ViewModel)는 Domain Layer의 모델만 의존해야 합니다. 현재
QuestionItem과SurveyResponse가data.remote.dto패키지에서 직접 임포트되고 있어 레이어 분리 원칙을 위반합니다.Domain Layer에 별도의 모델을 정의하고 Mapper를 통해 변환하는 것을 권장합니다.
As per coding guidelines: "레이어 분리 준수: presentation(ViewModel/Ui) ↔ data(repository/datasource) 의존 방향이 단방향인지 확인" 및 "Retrofit DTO/Room Entity를 그대로 UI에 노출하지 않는지 확인"
145-152: 문자열 기반 에러 타입 판별은 취약한 패턴입니다.
error.message?.contains("찾을 수 없습니다")로 에러 타입을 판별하는 것은 메시지 변경 시 쉽게 깨질 수 있습니다. 타입화된 에러(sealed class 또는 커스텀 Exception)를 사용하는 것이 더 안정적입니다.♻️ 타입화된 에러 사용 예시
// Domain layer에 정의 sealed class SurveyError : Exception() { object NotFound : SurveyError() data class ServerError(override val message: String) : SurveyError() object NetworkError : SurveyError() } // ViewModel에서 사용 result.fold( onSuccess = { ... }, onFailure = { error -> when (error) { is SurveyError.NotFound -> _uiState.value = SurveyUiState.Idle else -> _uiState.value = SurveyUiState.Error(error.message ?: "오류 발생") } } )
app/src/main/java/com/gildongmu/dduru/data/model/SurveyState.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/gildongmu/dduru/data/model/SurveyState.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/gildongmu/dduru/data/remote/service/SurveyService.kt
Show resolved
Hide resolved
app/src/main/java/com/gildongmu/dduru/data/repositoryimpl/SurveyRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/gildongmu/dduru/domain/repository/SurveyRepository.kt
Outdated
Show resolved
Hide resolved
.../com/gildongmu/dduru/presentation/feature/register/survey/components/SurveyQuestionLayout.kt
Show resolved
Hide resolved
...java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyQuestionScreen.kt
Outdated
Show resolved
Hide resolved
...java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyQuestionScreen.kt
Show resolved
Hide resolved
...n/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyResultScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyMainScreen.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/androidTest/java/gildong/dduru/ExampleInstrumentedTest.kt`:
- Line 1: The androidTest package path does not match its package declaration:
the file ExampleInstrumentedTest.kt declares package com.gildongmu.dduru but
lives under gildong/dduru; move the file to the correct directory structure or
update the package declaration so they match—specifically relocate
ExampleInstrumentedTest.kt into app/src/androidTest/java/com/gildongmu/dduru/
(or change its package line to match the actual folder) so the Android test
runner can discover the test.
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt`:
- Around line 97-107: The nextQuestion() method allows _currentQuestion to reach
_questions.value.size (off-by-one) which can cause index errors; update the
guard in nextQuestion() to only increment when _currentQuestion.value <
(_questions.value.size - 1) (and handle empty list safely) so the maximum index
remains size - 1; refer to nextQuestion(), _currentQuestion and _questions to
locate and fix this condition.
🧹 Nitpick comments (6)
app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/components/SurveyGridOptionButton.kt (1)
18-57: 전반적으로 잘 구현된 Composable 컴포넌트입니다.상태를 외부에서 주입받는 Stateless Composable 패턴을 따르고 있어 재사용성과 테스트 용이성이 좋습니다. Modifier 체인 순서도 적절하여 padding 영역을 포함한 전체 Box가 클릭 가능합니다.
개선 제안 (선택사항):
- 접근성:
noRippleClickable사용 시 스크린 리더 지원을 위해semanticsmodifier 추가를 고려해 주세요.- 텍스트 오버플로우:
maxLines = 2설정 시overflow = TextOverflow.Ellipsis추가를 권장합니다.♻️ 접근성 및 텍스트 오버플로우 처리 제안
import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp.noRippleClickable(onClick = onClick) + .semantics { role = Role.Button } .padding(vertical = 20.dp, horizontal = 8.dp),textAlign = TextAlign.Center, - maxLines = 2 + maxLines = 2, + overflow = TextOverflow.Ellipsis )app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt (1)
135-156: 에러 메시지 문자열 매칭은 취약합니다.
문구 변경/다국어 처리 시 오동작 가능성이 있으니, Repository에서 에러 타입/코드로 분기하도록 구조화하는 편이 안전합니다.app/src/main/java/com/gildongmu/dduru/data/remote/interceptor/AuthInterceptor.kt (1)
14-17:runBlocking사용이 OkHttp 스레드를 블로킹합니다.
intercept()내에서runBlocking을 사용하면 OkHttp 스레드 풀이 블로킹됩니다. DataStore 접근은 일반적으로 빠르지만, 첫 접근 시 또는 부하 상황에서 지연될 수 있습니다.권장 개선 방향:
- 토큰을 메모리에 캐싱하여 동기적으로 접근
- 또는
TokenLocalDataSource에 동기 접근 메서드 추가현재 구현도 동작하지만, 고부하 상황에서 잠재적 성능 이슈가 있을 수 있습니다.
♻️ 메모리 캐싱을 활용한 개선 예시
class AuthInterceptor `@Inject` constructor( private val tokenLocalDataSource: TokenLocalDataSource ) : Interceptor { `@Volatile` private var cachedToken: String? = null init { // CoroutineScope에서 토큰 변경 구독 CoroutineScope(Dispatchers.IO).launch { tokenLocalDataSource.accessToken.collect { token -> cachedToken = token } } } override fun intercept(chain: Interceptor.Chain): Response { val token = cachedToken // ... 나머지 로직 } }app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyQuestionScreen.kt (1)
69-69:QuestionOptionimport 추가를 고려해 주세요.
com.gildongmu.dduru.data.remote.dto.response.QuestionOption을 파일 상단에 import하면 코드 가독성이 향상됩니다.♻️ Import 추가 제안
import com.gildongmu.dduru.core.designsystem.theme.DduruTheme -import com.gildongmu.dduru.data.remote.dto.response.QuestionItem +import com.gildongmu.dduru.data.remote.dto.response.QuestionItem +import com.gildongmu.dduru.data.remote.dto.response.QuestionOption import com.gildongmu.dduru.presentation.feature.register.survey.components.SurveyGridOptionButton그 후 함수 파라미터에서:
- options: List<com.gildongmu.dduru.data.remote.dto.response.QuestionOption>, + options: List<QuestionOption>,app/src/main/java/com/gildongmu/dduru/data/repositoryimpl/SurveyRepositoryImpl.kt (1)
23-24: 원본 예외를 cause로 보존하세요.
IOException을 catch할 때 원본 예외 정보가 손실됩니다. 디버깅을 위해 cause로 전달하는 것이 좋습니다. 이 패턴은 lines 46, 63에도 동일하게 적용됩니다.♻️ 예외 cause 보존 제안
} catch (e: java.io.IOException) { - Result.failure(Exception("네트워크 오류: 인터넷 연결을 확인해주세요.")) + Result.failure(Exception("네트워크 오류: 인터넷 연결을 확인해주세요.", e)) } catch (e: Exception) { - Result.failure(Exception("알 수 없는 오류: ${e.message}")) + Result.failure(Exception("알 수 없는 오류: ${e.message}", e)) }동일한 수정을
getMySurveyResult()(lines 46-49)와getSurveyQuestions()(lines 63-66)에도 적용해 주세요.app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyResultScreen.kt (1)
30-30: DTO를 UI 레이어에서 직접 사용하고 있습니다.
SurveyResponse는data.remote.dto.response패키지의 DTO입니다. 코딩 가이드라인에 따르면 UI(Composable)는 DTO/Entity를 직접 노출받지 않고 UiState 또는 도메인 모델만 관찰해야 합니다.도메인 모델 또는 UI 전용 모델로 변환하여 레이어 분리 원칙을 준수해 주세요.
♻️ 권장 리팩토링 예시
// 1. UI용 모델 정의 (presentation layer) data class SurveyResultUiModel( val avatarName: String, val avatarDescription: String, val personality: String, val strength: String, val tip: String ) // 2. ViewModel 또는 UseCase에서 매핑 fun SurveyResponse.toUiModel() = SurveyResultUiModel( avatarName = avatarName, avatarDescription = avatarDescription, personality = personality, strength = strength, tip = tip ) // 3. Screen에서 UI 모델 사용 `@Composable` fun SurveyResultScreen( result: SurveyResultUiModel, // DTO 대신 UI 모델 onConfirm: () -> Unit, modifier: Modifier = Modifier )As per coding guidelines: "UI(Composable)는 UiState만 관찰하고, 로직/네트워크/상태 변경이 직접 발생하지 않는지 확인" 및 "Retrofit DTO/Room Entity를 그대로 UI에 노출" 금지.
Also applies to: 34-36
app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/main/MainActivity.kt`:
- Around line 42-58: The currentScreen state is only remembered in-memory and is
overwritten by LaunchedEffect(startScreen) on every recomposition/config change;
change the state declaration from remember { mutableStateOf(Screen.Splash) } to
a saved state (e.g., rememberSaveable { mutableStateOf(Screen.Splash) }) or add
a guard so LaunchedEffect(startScreen) only sets currentScreen on initial entry
(check if currentScreen == Screen.Splash before assigning) to prevent
rotation/process-restart from jumping to Home/Login; update references to
currentScreen, Screen and the LaunchedEffect(startScreen) block accordingly.
🧹 Nitpick comments (5)
app/src/main/java/com/gildongmu/dduru/domain/entity/SurveyQuestions.kt (2)
3-7: SurveyQuestions.count 중복 위험 완화 권장
count가questions.size와 불일치할 가능성이 있어 데이터 일관성 문제가 생길 수 있습니다. 최소한 불일치 시 조기 감지하도록 검증을 두는 걸 권장합니다.🔧 예시 수정안
data class SurveyQuestions( val version: Int, val count: Int, val questions: List<Question> -) +) { + init { + require(count == questions.size) { + "count($count) must match questions.size(${questions.size})" + } + } +}
9-19: Question 선택 범위 유효성 검증 추가 권장
minSelect/maxSelect와options크기의 조합이 잘못 들어오면 UI/로직에서 예외가 날 수 있습니다. 모델 생성 시 기본적인 범위 검증을 두면 안정성이 올라갑니다.🔧 예시 수정안
data class Question( val id: String, val displayOrder: Int, val questionText: String, val imageUrl: String?, val type: QuestionType, val required: Boolean, val minSelect: Int, val maxSelect: Int, val options: List<QuestionOption> -) +) { + init { + require(minSelect >= 0) { "minSelect must be >= 0" } + require(maxSelect >= minSelect) { "maxSelect must be >= minSelect" } + require(maxSelect <= options.size) { "maxSelect must be <= options.size" } + } +}app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt (2)
5-5: 데이터 레이어 모델이 프레젠테이션 레이어에 직접 노출됩니다.
SurveyState가data.model패키지에 위치하지만 ViewModel에서 직접 사용되고 있습니다. Google 권장 아키텍처에 따르면 프레젠테이션 레이어는 데이터 레이어 모델에 직접 의존하지 않아야 합니다.개선 방안:
SurveyState를presentation.feature.register.survey패키지로 이동하거나- 프레젠테이션 전용 상태 모델을 별도로 정의
66-81:updateAnswer의 문자열 기반 분기가 취약합니다.
questionId를 하드코딩된 문자열로 비교하는 방식은 오타에 취약하고 컴파일 타임 검증이 불가능합니다. enum 또는 sealed class 사용을 권장합니다.♻️ enum 기반 개선 예시
enum class QuestionId { Q1, Q2, Q3, Q4, Q5, Q6, Q7, Q8, Q9, Q10, Q11 } fun updateAnswer(questionId: QuestionId, answer: Int) { val serverAnswer = answer + 1 _surveyState.value = when (questionId) { QuestionId.Q1 -> _surveyState.value.copy(q1 = serverAnswer) QuestionId.Q2 -> _surveyState.value.copy(q2 = serverAnswer) // ... } }app/src/main/java/com/gildongmu/dduru/presentation/feature/main/MainActivity.kt (1)
71-94: 주석 처리된 로그인 분기 블록은 정리 권장.현재 로직이 확정됐다면 주석 블록을 삭제해 가독성을 높이는 편이 좋습니다.
app/src/main/java/com/gildongmu/dduru/presentation/feature/main/MainActivity.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt`:
- Around line 134-155: The method loadMySurveyResult() in SurveyViewModel is
unused and should be removed or integrated; either delete the entire
loadMySurveyResult() function (including its coroutine block and references to
_uiState and surveyRepository.getMySurveyResult()) to eliminate dead code, or if
intended, add calls to it from the UI (e.g., SurveyMainScreen or the screen's
init/LaunchedEffect) where survey results must be loaded and ensure the View
calls SurveyViewModel.loadMySurveyResult() so the method is actually exercised;
pick one approach and update related imports/tests accordingly.
🧹 Nitpick comments (2)
app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt (2)
16-21: 단발성 이벤트(Error) 처리 개선 고려현재
SurveyUiState.Error가StateFlow로 관리되어 있어, 화면 회전 등 configuration change 시 에러 메시지가 재표시될 수 있습니다.resetUiState()가 있지만, 단발성 이벤트는Channel또는SharedFlow로 분리하면 더 안전합니다.현재 구현도 동작하지만, 향후 개선 시 고려해주세요.
Also applies to: 31-32
144-151: 에러 메시지 기반 분기 처리 취약성
error.message?.contains("찾을 수 없습니다")로 특정 에러를 구분하는 방식은 메시지 변경에 취약합니다. Repository에서 sealed class나 custom exception을 사용하여 에러 유형을 명시적으로 전달하면 더 안정적입니다.현재 구현도 동작하지만, 유지보수성 향상을 위해 고려해주세요.
| fun loadMySurveyResult() { | ||
| viewModelScope.launch { | ||
| _uiState.value = SurveyUiState.Loading | ||
|
|
||
| val result = surveyRepository.getMySurveyResult() | ||
|
|
||
| result.fold( | ||
| onSuccess = { surveyResult -> | ||
| _uiState.value = SurveyUiState.Success(surveyResult) | ||
| }, | ||
| onFailure = { error -> | ||
| if (error.message?.contains("찾을 수 없습니다") == true) { | ||
| _uiState.value = SurveyUiState.Idle | ||
| } else { | ||
| _uiState.value = SurveyUiState.Error( | ||
| error.message ?: "설문 결과 조회 중 오류가 발생했습니다." | ||
| ) | ||
| } | ||
| } | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for loadMySurveyResult usages throughout the codebase
echo "=== Searching for loadMySurveyResult() usages ==="
rg -n "loadMySurveyResult" --type=kt
echo ""
echo "=== Search in related UI/Screen files ==="
rg -n "loadMySurveyResult" -C 3Repository: ddu-ru/ddu-ru-android
Length of output: 1019
🏁 Script executed:
#!/bin/bash
# Search for loadMySurveyResult without file type restriction
echo "=== Searching for loadMySurveyResult() in all files ==="
rg "loadMySurveyResult" -C 3
echo ""
echo "=== Check all Kotlin files in presentation directory ==="
find . -name "*.kt" -path "*/presentation/*" -type f | head -20
echo ""
echo "=== Search specifically in SurveyViewModel and related files ==="
find . -name "*Survey*" -type f -name "*.kt"Repository: ddu-ru/ddu-ru-android
Length of output: 4595
🏁 Script executed:
#!/bin/bash
# Check SurveyMainScreen.kt for loadMySurveyResult usage
echo "=== SurveyMainScreen.kt content ==="
cat app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyMainScreen.kt
echo ""
echo "=== SurveyResultScreen.kt content ==="
cat app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/screen/SurveyResultScreen.kt
echo ""
echo "=== Check for any ViewModel method calls in screen files ==="
rg "viewModel\." app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/ -C 2Repository: ddu-ru/ddu-ru-android
Length of output: 18354
loadMySurveyResult() 미사용 함수 제거 필요
해당 함수는 코드베이스 전체에서 호출되지 않고 있습니다. SurveyMainScreen.kt 및 관련 화면에서는 submitSurvey(), nextQuestion() 등 다른 ViewModel 메서드만 호출되고 있으며, loadMySurveyResult()는 어디에서도 사용되지 않습니다. 이전 리뷰에서 "Addressed"로 표시되었으나 실제로는 미사용 상태가 유지되고 있으므로, 함수를 제거하거나 필요한 곳에서 호출하도록 수정해야 합니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/gildongmu/dduru/presentation/feature/register/survey/SurveyViewModel.kt`
around lines 134 - 155, The method loadMySurveyResult() in SurveyViewModel is
unused and should be removed or integrated; either delete the entire
loadMySurveyResult() function (including its coroutine block and references to
_uiState and surveyRepository.getMySurveyResult()) to eliminate dead code, or if
intended, add calls to it from the UI (e.g., SurveyMainScreen or the screen's
init/LaunchedEffect) where survey results must be loaded and ensure the View
calls SurveyViewModel.loadMySurveyResult() so the method is actually exercised;
pick one approach and update related imports/tests accordingly.
choi-day
left a comment
There was a problem hiding this comment.
너무너무 수고하셨습니다~~~! 현재 설문조사 기획이 바뀌는 중인데 질문 개수가 변하는 등 기획이 달라진다면 다시 수정이 필요할 것 같네요ㅜㅜ 다같이 화이팅합시다...
|
|
||
| @Serializable | ||
| data class SurveyQuestionsResponse( | ||
| @SerialName("version") val version: Int, |
There was a problem hiding this comment.
@SerialName를 하나하나 붙여준 이유가 무엇인가요? 별다른 이유가 없고 추후에 변수명 변경 가능성이 적다면 가독성을 위해 없어도 되지 않을까 싶습니당!
| } | ||
| } | ||
|
|
||
| // 화면 전환 로직 |
There was a problem hiding this comment.
화면 전환기능은 추후에 navigation으로 분리시킵시다!! 제가 navigation얼른 해볼게요 ㅎㅎ...
| import com.gildongmu.dduru.core.designsystem.theme.DduruTheme | ||
|
|
||
| @Composable | ||
| fun SurveyGridOptionButton( |
There was a problem hiding this comment.
SurveyOptionButton과 거의 동일한 컴포넌트같아요 제가 발견하지 못한 크게 다른 점이 있을까요? 없다면 하나로 합칠 수 있을 것 같아요!
There was a problem hiding this comment.
설문 이미지를 서버에서 받아온다고 들었는데 모바일에서도 이미지를 따로 받아둔 이유가 있나요?
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.composed | ||
|
|
||
| fun Modifier.noRippleClickable(onClick: () -> Unit): Modifier = composed { |
✨ 작업 내용
사용자 맞춤형 여행 추천을 위한 '여행 성향 설문조사(온보딩)'의 전체 UI 및 API을 구현
시작 화면(QuestionStart)부터 총 11개 문항(Q1~Q11)에 대한 응답 상태 관리, 화면 전환, 그리고 서버 전송을 위한 데이터 구조 설계를 완료
📄 상세 내용
1. 설문조사 UI 및 공통 컴포넌트 구현
시작 화면: 캐릭터와 말풍선 등을 활용한 온보딩 진입 화면(QuestionStart) 구현
공통 컴포넌트: 질문 카드(QuestionCard), 단일/다중 선택 버튼 등 재사용 가능한 컴포넌트 개발
화면 구현: 피그마 디자인에 맞춰 11가지 문항의 개별 화면(Question1Screen ~ Question11Screen) 구현
2. ViewModel 및 상태(State) 관리
SurveyViewModel을 통해 1~11번 문항의 답변 상태(SurveyState)를 통합 관리하도록 구현했습니다.
Q7 문항의 경우 Set을 활용하여 다중 선택(최대 3개) 로직을 처리했습니다.
UI 상태(SurveyUiState)를 정의하여 로딩(Loading), 성공(Success), 에러(Error) 처리를 구분했습니다.
Screen_recording_20260214_234315.mp4
📎 관련 이슈
📷 스크린샷
✅ 체크리스트
Summary by CodeRabbit
새로운 기능
잡일(Chores)