Conversation
- Category, Question 모델을 하위 패키지로 이동 - 전체 프로젝트의 import 경로 업데이트 - Filter 모델 import 경로 업데이트 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- ExamQuestion: 시험 문제 모델 - Exams: 시험 목록 관리 모델 - Exam: 시험 정보 모델 - ExamDetail: 시험 상세 정보 모델 - ExamHistory: 시험 이력 모델 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
Request: - ExamRpcRequest: 시험 생성 요청 - ExamItemRpcRequest: 시험 항목 요청 - ExamDetailRpcRequest: 시험 상세 요청 - ExamQuestionRequest: 시험 문제 요청 Response: - ExamQuestionResponse: 시험 문제 응답 - ExamDetailResponse: 시험 상세 응답 - ExamHistoryResponse: 시험 이력 응답 - CategoryCountResponse: 카테고리별 문제 수 응답 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- getCategoryCount: 카테고리별 문제 수 조회 - createExam: 시험 생성 - getExamHistories: 시험 이력 목록 조회 - getExamDetail: 시험 상세 조회 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- RemoteExamDataSource 인터페이스 정의 - DefaultRemoteExamDataSource 구현 - Supabase RPC를 통한 시험 관련 데이터 조회/생성 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- ExamRepository 인터페이스 구현 - RemoteExamDataSource를 통한 데이터 조회/생성 - Response를 Domain 모델로 변환 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- getExamQuestions: 시험용 문제 목록 조회 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- getExamQuestions RPC 호출 구현 - fetchCategoryCount RPC 호출 구현 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- getExamQuestions 메서드 구현 - QuestionResponse에 toExamQuestion 변환 메서드 추가 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- RemoteExamDataSource 바인딩 - ExamRepository 바인딩 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- Category, Question 패키지 경로 변경 반영 - ExamQuestion 관련 테스트 픽스처 추가 - FakeRemoteQuestionDataSource에 누락된 메서드 구현 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- ExamQuestionCard: 시험 문제 카드 컴포넌트 추가 - ConfirmDialog: 확인 다이얼로그 개선 - ConfirmationDialog: 기존 파일 제거 - 색상 테마 업데이트 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- 시험 생성, 진행, 완료, 상세 화면 관련 문자열 추가 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- ExamMain: 시험 메인 화면 라우트 - ExamProgress: 시험 진행 화면 라우트 - ExamDetail: 시험 상세 화면 라우트 - ExamComplete: 시험 완료 화면 라우트 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- 시험 생성 탭: 카테고리별 문제 선택 및 시험 생성 - 시험 이력 탭: 과거 시험 기록 목록 조회 - ExamViewModel: 비즈니스 로직 처리 - ExamUiState: UI 상태 관리 - CategorySelector, HistoryList 컴포넌트 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- 시험 문제 진행 화면 - 문제 답변 선택 및 다음/이전 문제 이동 - 진행 상황 표시 - ExamProgressViewModel: 답변 상태 관리 - ExamProgressUiState: UI 상태 관리 - Navigation 설정 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- 완료된 시험의 상세 결과 조회 - 문제별 정답/오답 표시 - 정답률 통계 - ExamDetailViewModel: 시험 상세 데이터 로딩 - ExamDetailUiState: UI 상태 관리 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- 시험 완료 화면 - 총점 및 정답률 표시 - 시험 결과 저장 - ExamCompleteViewModel: 시험 결과 처리 - ExamCompleteUiState: UI 상태 관리 - Navigation 설정 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- BottomNavigationType에 EXAM 추가 - MainScreen에 Exam 탭 통합 - MainViewModel 추가 - Navigation 설정 업데이트 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- ExamViewModel, ExamProgressViewModel 등록 - ExamDetailViewModel, ExamCompleteViewModel 등록 - MainViewModel 등록 - Navigation Contributor 등록 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- Exam 기능 관련 의존성 및 설정 추가 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
- HistoryScreen: Exam 기능으로 대체 - TestScreen: 테스트용 화면 제거 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough시험(Exam) 기능을 추가·통합하는 대규모 변경입니다. 도메인에 Exam 관련 모델(Exam, Exams, ExamQuestion, ExamDetail, ExamHistory 등)과 Category 패키지 재구성, 리포지토리 인터페이스(ExamRepository) 및 QuestionRepository/RemoteQuestionDataSource 확장이 추가되었습니다. 데이터 계층에 RPC 요청/응답 모델과 원격 데이터소스(DefaultRemoteExamDataSource) 및 DefaultExamRepository가 도입되었습니다. UI에서는 시험 메인(ExamScreen), 생성/기록 탭, 진행(ExamProgressScreen), 상세(ExamDetailScreen), 완료(ExamCompleteScreen) 화면과 여러 컴포넌트 및 프리뷰가 추가되었고 네비게이션 경로와 DI(ViewModel/Navigation) 등록이 확장되었습니다. 기존 Test/Profile/History 스크린 일부는 제거되고 MainScreen 및 하단 내비게이션이 Exam 흐름으로 변경되었습니다. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
- ProfileScreen 파일 및 디렉토리 삭제 - BottomNavigationType에서 PROFILE 제거 - NavRoutes에서 Profile 라우트 제거 - MainScreen에서 ProfileScreen 사용 제거 - 관련 문자열 리소스 제거 🤖 Generated with [Firebender](https://firebender.com) Co-Authored-By: Firebender <help@firebender.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composeApp/src/commonMain/kotlin/com/peto/droidmorning/questions/detail/component/AnswerCard.kt (1)
102-107: 삭제 버튼 설명이 ‘취소’로 읽혀 접근성 혼선이 생깁니다.왜 문제인가: 삭제 아이콘에 “취소” 라벨이 붙으면 스크린리더 사용자에게 동작이 잘못 전달됩니다.
어떻게 개선: 삭제/제거에 해당하는 리소스로 맞춰주세요.✅ 제안 수정안
-import com.peto.droidmorning.designsystem.generated.resources.cancel +import com.peto.droidmorning.designsystem.generated.resources.remove ... - contentDescription = stringResource(DesignRes.string.cancel), + contentDescription = stringResource(DesignRes.string.remove),
🤖 Fix all issues with AI agents
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/complete/vm/ExamCompleteViewModel.kt`:
- Around line 19-28: The loadExamDetail function currently only handles
onSuccess and never updates loading/error state; modify loadExamDetail (inside
viewModelScope.launch where examRepository.fetchExamDetail is called) to set a
loading flag in ExamCompleteUiState before the call, handle both onSuccess and
onFailure (use onFailure to update an error field/message in
ExamCompleteUiState), and ensure the loading flag is cleared in both branches
(or finally) when updating _uiState via _uiState.update; add loading/error
fields to ExamCompleteUiState if they don't exist.
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/detail/vm/ExamDetailViewModel.kt`:
- Around line 19-27: loadExamDetail currently only handles
fetchExamDetail().onSuccess so failures leave the UI unchanged; add an onFailure
handler to catch errors from fetchExamDetail(examId) inside
viewModelScope.launch and update the _uiState (ExamDetailUiState) to reflect the
error state (e.g., set an error message, isLoading=false, and a retryable flag)
or emit an error event via the ViewModel event channel; also ensure you log the
exception. Modify ExamDetailUiState (or expose an error event) so loadExamDetail
can set a visible error and support retry, and update callers to handle that new
state/event.
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/navigation/ExamProgressNavGraphContributor.kt`:
- Around line 26-39: The problem is that ExamProgressNavGraphContributor reads
saved state from previousBackStackEntry (the ExamProgressGraph entry) so it
misses values Main stored on currentBackStackEntry; update the retrieval to read
from navController.currentBackStackEntry?.savedStateHandle (matching where
MainNavGraphContributor set "questionCount" and "categories") in the composable
for NavRoutes.ExamProgress.route (replace usages of
previousBackStackEntry?.savedStateHandle?.get<Int>("questionCount") and
?.get<Array<String>>("categories") with
currentBackStackEntry?.savedStateHandle?.get...), or alternatively implement
route arguments on NavRoutes.ExamProgress or a shared ViewModel/global state if
you prefer that approach.
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/vm/ExamProgressViewModel.kt`:
- Around line 61-72: submitExam currently only handles onSuccess and ignores
failures; update the call to examRepository.submitExam(...) in submitExam
(inside viewModelScope.launch) to handle failures by adding an onFailure handler
that surfaces the error to the UI (e.g., sendEvent with a new or existing error
event such as ExamProgressUiEvent.ShowError or NavigateToError, and/or log the
throwable). Ensure you reference examRepository.submitExam, sendEvent, and
ExamProgressUiEvent.NavigateToComplete and create or reuse an error event
variant so users receive feedback on network/server errors.
In
`@data/src/commonMain/kotlin/com/peto/droidmorning/data/model/request/ExamRpcRequest.kt`:
- Around line 9-15: ExamRpcRequest is currently serializing the domain Category
enum (categories: List<Category>), which breaks DTO/domain separation and risks
serialization mismatch; change the DTO to use categories: List<String> instead
and update wherever ExamRpcRequest is constructed (notably the toRequest()
converter) to map each Category to its string representation (e.g.,
category.name or a defined string value) before building the ExamRpcRequest,
ensuring all code paths creating ExamRpcRequest use the string list rather than
the Category enum.
In
`@designsystem/src/commonMain/kotlin/com/peto/droidmorning/designsystem/component/ConfirmDialog.kt`:
- Around line 45-46: The default parameter values for confirmText and cancelText
in ConfirmDialog call stringResource (a `@Composable`), which must not be used as
compile-time defaults; change the parameters to nullable Strings (confirmText:
String? = null, cancelText: String? = null) or accept `@StringRes` Int IDs, then
inside the ConfirmDialog composable resolve them via stringResource (e.g., val
resolvedConfirmText = confirmText ?: stringResource(DesignRes.string.remove) and
val resolvedCancelText = cancelText ?: stringResource(DesignRes.string.cancel))
and use resolvedConfirmText/resolvedCancelText in the UI; this removes
stringResource from parameter defaults and keeps resolution inside the
Composition.
In
`@domain/src/commonMain/kotlin/com/peto/droidmorning/domain/model/category/Category.kt`:
- Around line 11-17: The current Category.from(value: String) uses error(...)
and will crash on unknown inputs; change it to a safe fallback by either (A)
adding an Unknown entry to the Category enum and returning that from
Category.from instead of calling error (update UNKNOWN_CATEGORY_EXCEPTION usage
accordingly), or (B) provide a nullable alternative like
Category.fromOrNull(value: String): Category? that returns null when not found
and keep from(...) throwing if you want to preserve strict behavior; modify call
sites that rely on from to handle the new Unknown or nullable return.
🟡 Minor comments (14)
data/src/commonMain/kotlin/com/peto/droidmorning/data/model/response/QuestionResponse.kt-3-4 (1)
3-4: Mapper의 에러 처리 전략 개선 필요도메인 모델 패키지 구조 변경에 맞춰 import 경로와
Category.from(category)사용이 일관성 있게 업데이트되었습니다. 다만 Data 모듈 리뷰 가이드의 null safety 요구사항을 고려했을 때, 현재 구현에는 개선이 필요합니다.
Category.from()메서드는 일치하는 카테고리를 찾지 못하면 예외를 던집니다(error("Unknown category: $value")). 서버에서 예상치 못한 카테고리 문자열이 전달되면 애플리케이션이 런타임에 크래시될 수 있다는 의미입니다.개선 방안:
- 미지의 카테고리에 대한 기본값 설정 (예: 첫 번째 enum 값)
Category?또는Result<Category>같은 Optional 타입 사용으로 명시적 에러 처리- 서버 응답 검증 로직 추가
현재 구현처럼 예외 던지기는 유효한 에러 처리 방식이지만, 외부 데이터 소스(Remote)에서 받은 데이터를 처리하는 mapper 계층에서는 더욱 견고한 에러 핸들링이 권장됩니다.
composeApp/src/commonMain/composeResources/values/strings.xml-68-68 (1)
68-68: “문제 시험” 문구의 자연스러움 확인 부탁드립니다.왜: 수량과 결합될 경우 “10 문제 시험”처럼 어색하게 보일 수 있습니다.
어떻게: UX 카피 기준에 맞게 “문제/문항” 등으로 조정하는 방향을 고려해도 좋을까요?composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/component/ExamAnswerInput.kt-20-71 (1)
20-71: 테마별 색상 적용이 필요합니다.왜:
ExamSelected는 직접 임포트되어 다크 테마에서도 라이트 색상으로 고정됩니다. 현재ExamSelectedDark가 정의되어 있지만 사용되지 않고 있습니다.어떻게:
MaterialTheme.extendedColors.examSelected를 사용하면 테마에 따라 올바른 색상이 자동으로 적용됩니다. 다만examUnSelected는 아직ExtendedColors에 포함되지 않았으므로, 먼저ExamUnSelectedDark색상을 정의하고ExtendedColors에examUnSelected속성을 추가해야 합니다.🔧 제안 수정안
-import com.peto.droidmorning.designsystem.theme.ExamSelected -import com.peto.droidmorning.designsystem.theme.ExamUnSelected +import com.peto.droidmorning.designsystem.theme.AppTheme @@ - val borderColor = - if (isFocused) { - ExamSelected - } else { - ExamUnSelected - } + val borderColor = + if (isFocused) { + MaterialTheme.extendedColors.examSelected + } else { + MaterialTheme.extendedColors.examUnSelected + }단,
designsystem모듈의ExtendedColors에 먼저examUnSelected속성을 추가하고 다크 테마 색상을 정의해야 합니다.designsystem/src/commonMain/kotlin/com/peto/droidmorning/designsystem/component/ExamQuestionCard.kt-142-148 (1)
142-148:overflow만 있고maxLines가 없어 말줄임이 적용되지 않습니다.
Line 142-148에서TextOverflow.Ellipsis는maxLines가 없으면 무시되어 긴 답변이 카드 높이를 과도하게 늘릴 수 있습니다.
maxLines를 지정해 UI 안정성을 높이는 것을 권장합니다.✅ 수정 예시
Text( text = examDetail.userAnswer, style = MaterialTheme.typography.bodyMedium, fontWeight = FontWeight.Medium, color = MaterialTheme.colorScheme.onSurface, + maxLines = 3, overflow = TextOverflow.Ellipsis, )composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/component/ExamCreateTab.kt-49-175 (1)
49-175: 스크롤 Column에서weight()수정자는 의도한 대로 동작하지 않습니다.Line 129의
Spacer(modifier = Modifier.weight(1f))는verticalScroll()내에서 무시됩니다. 그 이유는verticalScroll()이 자식에게 제한되지 않은(unbounded) 높이를 제공하기 때문에,weight()는 남은 공간을 계산할 수 없기 때문입니다. 현재 구조에서는 버튼이 스크롤 콘텐츠의 일부가 되어 하단에 고정되지 않을 수 있습니다.스크롤 영역과 버튼을 분리하는 구조로 개선하면 문제를 해결할 수 있습니다:
✅ 개선 방향 예시
- Column( - modifier = - modifier - .fillMaxSize() - .verticalScroll(rememberScrollState()) - .padding(16.dp), - verticalArrangement = Arrangement.spacedBy(24.dp), - ) { + Column( + modifier = modifier.fillMaxSize().padding(16.dp), + ) { + Column( + modifier = Modifier.weight(1f).verticalScroll(rememberScrollState()), + verticalArrangement = Arrangement.spacedBy(24.dp), + ) { ... - Spacer(modifier = Modifier.weight(1f)) - + } Box( ... ) { ... } }실제 기기에서 버튼의 배치가 의도한 대로 나타나는지 확인해 주실 수 있을까요? 특히 콘텐츠가 적은 경우 버튼이 항상 화면 하단에 고정되어야 한다면, 이 구조가 필요합니다.
composeApp/src/commonMain/kotlin/com/peto/droidmorning/main/vm/MainViewModel.kt-12-18 (1)
12-18:valueOf에서 예외 발생 가능성이 있습니다.
BottomNavigationType.valueOf(it)는 저장된 문자열이 현재 enum 값과 일치하지 않을 경우IllegalArgumentException을 발생시킵니다. 앱 업데이트로 enum 값이 변경되거나 제거된 경우, 기존 사용자의 저장된 상태로 인해 앱이 크래시될 수 있습니다.
runCatching이나entries.find를 사용하여 안전하게 파싱하는 것이 좋습니다.🛡️ 안전한 enum 파싱 제안
private val _selectedTab = MutableStateFlow( savedStateHandle .get<String>(SELECTED_TAB_KEY) - ?.let { BottomNavigationType.valueOf(it) } + ?.let { name -> + BottomNavigationType.entries.find { it.name == name } + } ?: BottomNavigationType.QUESTION, )composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/detail/ExamDetailScreen.kt-137-139 (1)
137-139: 문항 수 텍스트 포맷을 더 나은 지역화 방식으로 개선해 보세요현재 코드는
questionCount와 리소스 문자열을 단순 연결하고 있습니다. 이 방식은 한국어에서는 작동하지만, 다른 언어로 확장할 때 문제가 될 수 있습니다. 예를 들어, 영어에서는 "Question 5" 형식이 필요할 수 있고, 복수형 처리가 필요할 수도 있습니다.리소스에
%d같은 플레이스홀더를 추가하여stringResource(Res.string.exam_question_count_format, questionCount)로 포맷을 맡기는 방식을 고려해 보세요. 이렇게 하면 각 언어별로 문자와 숫자의 순서를 다르게 배치할 수 있습니다.<!-- strings.xml --> <string name="exam_question_count_format">%d문제</string>- text = "$questionCount${stringResource(Res.string.exam_question_count_format)}", + text = stringResource(Res.string.exam_question_count_format, questionCount),이렇게 개선하면 지역화가 더욱 유연해집니다.
data/src/commonMain/kotlin/com/peto/droidmorning/data/model/response/ExamDetailResponse.kt-26-35 (1)
26-35:Category.from()호출 시 에러 처리 일관성을 검토해 주세요.
ExamHistoryResponse.toDomain()에서는runCatching { Category.from(categoryString) }.getOrNull()을 사용하여 잘못된 카테고리 값을 안전하게 처리하고 있습니다. 하지만 이 파일에서는Category.from(questionCategory)를 직접 호출하고 있어, 유효하지 않은 카테고리 문자열이 들어오면 예외가 발생할 수 있습니다.데이터 레이어의 에러 처리 전략을 일관되게 유지하는 것이 좋습니다. 두 가지 접근 방식 중 하나를 선택해 주세요:
- 방어적 처리:
runCatching을 사용하고 기본값 또는 null 처리- 빠른 실패: 유효하지 않은 데이터는 즉시 예외 발생 (현재 방식)
현재 구현 의도가 "단일 상세 조회에서는 카테고리가 반드시 유효해야 한다"라면 괜찮지만, 일관성 측면에서 확인이 필요합니다.
🔧 방어적 처리 예시 (선택 사항)
fun ExamDetailResponse.toDomain(): ExamDetail = ExamDetail( examItemId = examItemId, examId = examId, questionId = questionId, userAnswer = userAnswer, questionTitle = questionTitle, - questionCategory = Category.from(questionCategory), + questionCategory = runCatching { Category.from(questionCategory) } + .getOrDefault(Category.Unknown), // 또는 적절한 기본값 questionSourceUrl = questionSourceUrl, )data/src/commonMain/kotlin/com/peto/droidmorning/data/datasource/question/remote/RemoteQuestionDataSource.kt-7-19 (1)
7-19:FakeRemoteQuestionDataSource에서 메서드 오버로드 구현이 누락되었습니다.좋은 소식부터:
fetchQuestions()메서드가 실제로 이름이 변경되지 않았습니다. 도메인 레이어(QuestionRepository)는 기존의fetchQuestions()메서드를 유지하고 있으며, 데이터 레이어에서 세부 구현(RemoteQuestionDataSource)이 확장되었습니다. 이는 레이어 간 역할 분리가 잘 이루어진 좋은 설계입니다.하지만 주의할 점이 있습니다:
FakeRemoteQuestionDataSource가RemoteQuestionDataSource인터페이스의 두 번째fetchExamQuestions(category: List<String>, count: Int)오버로드를 구현하지 않았습니다.DefaultRemoteQuestionDataSource와 도메인 레이어에서 이 메서드를 사용하고 있으므로, 테스트 코드에서 컴파일 오류가 발생하거나 예상치 못한 동작이 발생할 수 있습니다.다음과 같이
FakeRemoteQuestionDataSource에 누락된 메서드를 추가해 주세요:override suspend fun fetchExamQuestions( category: List<String>, count: Int, ): List<ExamQuestionResponse> { // 테스트용 구현 (예: 입력된 개수만큼 필터링된 응답 반환) return emptyList() }또한
fetchCategoryCount()메서드도 구현되어 있지 않습니다.composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/vm/ExamViewModel.kt-103-118 (1)
103-118: 삭제 실패 시 사용자에게 피드백이 없습니다.
deleteExam()의onFailure에서 삭제 확인 다이얼로그만 닫고 에러 메시지를 표시하지 않습니다. 사용자는 삭제가 왜 실패했는지 알 수 없습니다.🛠️ 에러 이벤트 추가 제안
.onFailure { _uiState.update { it.hideDeleteConfirmation() } + _uiEvent.send(ExamUiEvent.ShowDeleteFailureMessage) }composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/vm/ExamViewModel.kt-49-74 (1)
49-74: 데이터 로드 실패 시 에러 처리가 누락되었습니다.
loadCategoryCounts()와loadExamHistory()모두onSuccess만 처리하고onFailure가 없습니다. 네트워크 오류 등의 상황에서 사용자에게 피드백이 전혀 없어 앱이 정상 동작하지 않는 것처럼 보일 수 있습니다.
ExamUiState에 이미error필드가 있으니, 이를 활용하여 에러 상태를 표시하는 것이 좋을까요?🛠️ 에러 처리 추가 제안
private fun loadCategoryCounts() { viewModelScope.launch { questionRepository .fetchAllCategoryCount() .onSuccess { countMap -> _uiState.update { examUiState -> examUiState.updateCategoryCounts(countMap) } + }.onFailure { throwable -> + _uiState.update { it.copy(error = "카테고리 정보를 불러오는데 실패했습니다.") } } } }composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/vm/ExamProgressViewModel.kt-38-42 (1)
38-42: 문제 로드 실패 시 사용자 피드백이 없습니다.
loadExamQuestions의onFailure에서 로딩 상태만 해제하고 에러 메시지를 표시하지 않습니다. 사용자는 왜 문제가 로드되지 않았는지 알 수 없게 됩니다.🛠️ 에러 이벤트 추가 제안
.onFailure { _uiState.update { it.loading(false) } + sendEvent(ExamProgressUiEvent.ShowError("문제를 불러오는데 실패했습니다.")) }composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/complete/navigation/ExamCompleteNavGraphContributor.kt-23-44 (1)
23-44: 심화된 안전성을 위해 popBackStack의 반환값 확인 및 fallback 처리를 추가하세요.왜:
popBackStack(NavRoutes.Main.route, inclusive = false)은 Main이 백 스택에 없으면 false를 반환하고 현재 화면에 그대로 머물게 됩니다(예: 딥링크를 통한 진입). 반환값을 확인하지 않으면 네비게이션이 실패해도 사용자에게 피드백이 없습니다.어떻게: 반환값을 확인하고, false 시 Main으로 navigate하는 fallback을 추가해 볼까요?
🧭 제안 수정안
ExamCompleteScreen( examId = args.examId, onNavigateToQuestions = { - navController.popBackStack(NavRoutes.Main.route, inclusive = false) + val popped = navController.popBackStack(NavRoutes.Main.route, inclusive = false) + if (!popped) { + navController.navigate(NavRoutes.Main.route) { + launchSingleTop = true + } + } }, )data/src/commonMain/kotlin/com/peto/droidmorning/data/datasource/question/remote/DefaultRemoteQuestionDataSource.kt-23-31 (1)
23-31: 범주가 비어있거나 문항 수가 0 이하면 불필요한 RPC 호출이 발생합니다.현재 코드는 서버에 요청을 보낸 후 오류가 발생하면 Repository의
runCatching에서 처리합니다. 하지만 입력값이 명백히 유효하지 않다면 요청 전에 조기 차단하는 것이 효율적입니다.데이터 소스 레이어에서 간단한 precondition으로 검증하면, 불필요한 네트워크 호출과 예외 처리를 줄일 수 있습니다.
🛠️ 제안 수정안
override suspend fun fetchExamQuestions( category: List<String>, count: Int, ): List<ExamQuestionResponse> { + require(category.isNotEmpty()) { "category must not be empty" } + require(count > 0) { "count must be positive" } val params = ExamQuestionRequest(category, count) return postgrest .rpc(RPC_GENERATE_EXAM_QUESTIONS, params) .decodeList<ExamQuestionResponse>() }
🧹 Nitpick comments (23)
composeApp/src/commonMain/kotlin/com/peto/droidmorning/questions/list/model/QuestionUiState.kt (1)
67-81: 시험 기능을 위한 상태 업데이트 로직이 잘 구현되었습니다.
isLiked와isSolved상태를 함께 업데이트할 수 있도록 확장된 점이 좋습니다. 기존toggleQuestionLike과 유사한 패턴을 사용하면서도, 명시적인 값 설정이 필요한 시험 완료 후 상태 동기화 시나리오에 적합합니다.참고로,
toggleQuestionLike(55-65줄)과 유사한 매핑 로직이 반복되고 있습니다. 현재 규모에서는 문제없지만, 향후 업데이트 로직이 더 추가된다면 공통 헬퍼 함수 추출을 고려해볼 수 있습니다:private fun updateQuestionById( questionId: Long, transform: (Question) -> Question ): QuestionUiState { val updatedList = allQuestions.toList().map { question -> if (question.id == questionId) transform(question) else question } return copy(allQuestions = Questions(updatedList)) }designsystem/src/commonMain/kotlin/com/peto/droidmorning/designsystem/theme/Color.kt (1)
81-93: Exam 관련 색상 상수 추가 확인 ✓시험 기능을 위한 색상 상수들이 기존 네이밍 컨벤션(PascalCase, Dark 접미사)을 잘 따르고 있습니다. Light/Dark 모드 변형이 적절히 정의되어 있네요.
한 가지 확인 사항:
ExamUnSelected에 대한 다크 모드 변형(ExamUnSelectedDark)이 누락된 것 같습니다. 다크 모드에서도ExamUnSelected색상을 그대로 사용할 계획인지, 아니면 별도의 다크 모드 색상이 필요한지 검토해 주세요. 기존 패턴을 보면 대부분의 색상에 Dark 변형이 있어서 일관성을 위해 추가를 고려해 볼 수 있습니다.🎨 다크 모드 색상 추가 제안 (선택 사항)
val ExamSelected = Color(0xFFFF9800) val ExamUnSelected = Color(0xFFFCEEDA) val ExamSelectedDark = Color(0xFFFFB74D) +val ExamUnSelectedDark = Color(0xFF3D3530) // 다크 모드에서 적절한 unselected 색상composeApp/src/commonMain/kotlin/com/peto/droidmorning/questions/detail/component/AddAnswerBottomSheet.kt (1)
111-121: 저장 검증 조건이 두 곳에서 다르게 유지됩니다.왜 문제인가: 지금은 enabled가 막아주지만, 향후 로직이 바뀌면 공백 입력이 통과될 여지가 있습니다.
어떻게 개선: 동일한 predicate를 공유하도록 정리하는 게 안전하지 않을까요?♻️ 제안 리팩터
- TextButton( + val isValid = draftAnswer.trim().isNotEmpty() + TextButton( onClick = { - if (draftAnswer.isNotEmpty()) { + if (isValid) { scope.launch { onSave(draftAnswer) sheetState.hide() onDismiss() } } }, - enabled = draftAnswer.trim().isNotEmpty(), + enabled = isValid, ) {composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/model/ExamUiEvent.kt (1)
6-9:categories파라미터에ImmutableList사용을 고려해 볼 수 있습니다.현재
List<Category>를 사용하고 있는데, 프로젝트의 다른 부분(예:QuestionFilterChips의ImmutableSet<Category>)에서는 kotlinx.collections.immutable을 활용하고 있습니다. 일관성을 위해ImmutableList<Category>사용을 고려해 볼 수 있습니다.다만, 이 이벤트는 한 번 소비되고 버려지는 특성이 있어 현재 구현도 실질적인 문제는 없습니다.
♻️ 선택적 개선안
+import kotlinx.collections.immutable.ImmutableList + sealed interface ExamUiEvent { data class NavigateToExamProgress( val questionCount: Int, - val categories: List<Category>, + val categories: ImmutableList<Category>, ) : ExamUiEventcomposeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/component/ExamQuestionHeader.kt (1)
17-38: 잘 구현된 Composable 컴포넌트입니다!
ExamQuestionHeader가 Compose 권장 패턴을 잘 따르고 있습니다:
modifier파라미터를 마지막 위치에 기본값과 함께 제공- 단일 책임 원칙 준수
- CategoryBadge 재사용으로 일관된 UI 유지
한 가지 개선 제안:
8.dp매직 넘버를Dimen상수로 추출하면 디자인 시스템 일관성이 더 좋아질 수 있습니다. 예를 들어Dimen.spacingSm또는Dimen.spacingBase를 사용하는 것은 어떨까요?♻️ Dimen 상수 활용 제안
`@Composable` fun ExamQuestionHeader( category: Category, questionContent: String, modifier: Modifier = Modifier, ) { Column( modifier = modifier, - verticalArrangement = Arrangement.spacedBy(8.dp), + verticalArrangement = Arrangement.spacedBy(Dimen.spacingBase), ) {designsystem/src/commonMain/kotlin/com/peto/droidmorning/designsystem/theme/AppTheme.kt (2)
16-34: ExtendedColors에 미선택 색상도 포함시키는 게 좋습니다.왜: 선택/미선택 색상을 컴포넌트가 직접 상수로 참조하면 테마 전환 시 불일치가 생길 수 있어요.
어떻게:examUnSelected를 확장 팔레트에 포함해 라이트/다크 모두 주입하도록 해주세요.♻️ 제안 수정안
data class ExtendedColors( val examSelected: Color = ExamSelected, + val examUnSelected: Color = ExamUnSelected, val examCorrect: Color = ExamCorrect, val examCorrectBackground: Color = ExamCorrectBackground, ) @@ private val LightExtendedColors = ExtendedColors( examSelected = ExamSelected, + examUnSelected = ExamUnSelected, examCorrect = ExamCorrect, examCorrectBackground = ExamCorrectBackground, ) @@ private val DarkExtendedColors = ExtendedColors( examSelected = ExamSelectedDark, + examUnSelected = ExamUnSelectedDark, examCorrect = ExamCorrectDark, examCorrectBackground = ExamCorrectBackgroundDark, )
38-55: 커스텀colorScheme전달 시 확장 컬러 불일치 가능성이 있습니다.왜:
extendedColors가useDarkTheme만 기준으로 결정되어, 호출부에서colorScheme를 직접 넘기면 팔레트가 어긋날 수 있어요.
어떻게:extendedColors를 파라미터로 분리하거나colorScheme/테마 상태에 맞춰 계산하도록 조정하는 게 안전합니다. 사용처에 커스텀 스킴 전달 케이스가 있는지 확인해주실 수 있을까요?domain/src/commonMain/kotlin/com/peto/droidmorning/domain/model/exam/ExamQuestion.kt (1)
5-9: LGTM! 깔끔한 도메인 모델 설계입니다.불변
data class로 설계되어 도메인 모듈 가이드라인(Android 의존성 없음, 불변 객체)을 잘 따르고 있습니다. 필요한 속성만 간결하게 포함하고 있어 Single Responsibility Principle도 잘 지켜지고 있습니다.선택적 제안:
questionId가 양수여야 하거나content가 비어있지 않아야 하는 비즈니스 규칙이 있다면,init블록에서 유효성 검증을 추가하는 것을 고려해볼 수 있습니다. 다만 이는 비즈니스 요구사항에 따라 결정하시면 됩니다.composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/component/QuestionCountChip.kt (1)
33-81: 선택 상태가 접근성 서비스에 노출되지 않고, 색상이 하드코딩되어 테마 대응이 약합니다.왜 문제인가:
clickable만 사용하면 스크린리더가 선택 여부를 알기 어렵고,Color.White는 커스텀 테마/다크 모드에서 대비 이슈가 날 수 있습니다.
어떻게 개선할까:selectable로 선택 상태/Role을 제공하고, 텍스트 색상은onPrimary등 테마 색을 사용해 주세요.♻️ 개선 제안
+import androidx.compose.foundation.selection.selectable +import androidx.compose.ui.semantics.Role ... - ).clickable(onClick = onClick) + ).selectable( + selected = selected, + onClick = onClick, + role = Role.RadioButton, + ) ... - Color.White + MaterialTheme.colorScheme.onPrimary ... - Color.White + MaterialTheme.colorScheme.onPrimarycomposeApp/src/commonMain/kotlin/com/peto/droidmorning/main/BottomNavigationType.kt (1)
22-25: EXAM과 리소스 키명이 불일치해 유지보수 혼동 가능성이 있습니다.왜 문제인가: enum 이름은 EXAM인데 라벨 리소스가
bottom_nav_test라 의미가 엇갈려 보입니다.
어떻게 개선할까:bottom_nav_exam같은 키로 정리하거나, 의도적으로 유지한 것이라면 주석/컨벤션으로 설명해 주실 수 있을까요?composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/component/EmptyHistoryState.kt (1)
32-36: 아이콘이 의미를 전달한다면 contentDescription 제공을 고려해 주세요.왜 문제인가: Line 32-36에서 contentDescription = null이라 스크린리더가 아이콘 의미를 읽지 못합니다.
어떻게 개선하나: 아이콘이 상태 의미를 전달한다면 문자열 리소스로 설명을 제공해 주세요. 장식용이라면 현재처럼 null 유지가 맞습니다. 의도는 어떤가요?📝 예시 수정안
- Icon( - imageVector = Icons.Default.CheckCircle, - contentDescription = null, + Icon( + imageVector = Icons.Default.CheckCircle, + contentDescription = stringResource(Res.string.exam_history_empty_title), modifier = Modifier.size(80.dp), tint = MaterialTheme.colorScheme.outlineVariant, )composeApp/src/commonMain/kotlin/com/peto/droidmorning/main/navigation/MainNavGraphContributor.kt (2)
46-62: 타입 안전 네비게이션에서navArgument()제거 권장
@Serializable어노테이션이 적용된NavRoutes.ExamDetail을 사용하고 있으므로, 타입 안전한 라우팅 패턴을 완전히 활용하기 위해 lines 48-53의navArgument()선언을 제거해도 됩니다.Navigation Compose 2.8+에서는 타입 안전 라우팅을 사용할 때,
toRoute<T>()만으로 충분합니다:composable( route = NavRoutes.ExamDetail.ROUTE, ) { backStackEntry -> val args = backStackEntry.toRoute<NavRoutes.ExamDetail>() ExamDetailScreen( examId = args.examId, onNavigateBack = { navController.popBackStack(NavRoutes.Main.route, inclusive = false) }, ) }현재 코드는 정상 동작하지만,
navArgument()정의가 중복되면 유지보수 시 두 곳을 수정해야 합니다. 타입 정보의 단일 출처(source of truth)를 유지하는 것이 더 깔끔합니다.
32-38:savedStateHandle을 통한 데이터 전달 방식에서 타입 안전성 개선이 가능합니다.현재 구현을 살펴보니,
categories를 문자열 배열로 전달한 후ExamProgressNavGraphContributor에서Category.from(name)으로 다시 복원하고 있습니다.Categoryenum에 case-insensitive 매칭을 지원하는from()함수가 있어 현재는 정상 작동합니다.다만 이 방식은 다음과 같은 잠재적 문제가 있습니다:
- enum 값 변경 시 런타임 에러: 카테고리 이름이 변경되면
from()함수에서 error를 발생시킵니다.- 문자열 기반 매칭의 취약성: 리팩토링 시 enum 이름을 변경하면 해당 변경사항을 모든 통신 지점에서 추적해야 합니다.
Navigation Compose의 Type-safe navigation 을 사용하여
@Serializableroute 클래스에Category타입을 직접 포함시키는 방식이 장기적으로 더 안전합니다. 이렇게 하면 컴파일 타임에 타입 검사가 가능해져 runtime 에러를 방지할 수 있습니다.현재 구현은 동작하므로 문제 없지만, 향후 리팩토링 시 이러한 접근 방식을 검토해 보시길 권장합니다.
domain/src/commonMain/kotlin/com/peto/droidmorning/domain/model/exam/Exams.kt (1)
3-23: LGTM! 불변 도메인 모델로 잘 설계되었습니다.
Exams클래스가 불변 객체로 설계되어 도메인 모델 가이드라인을 잘 따르고 있습니다.operator fun get을 통한 편리한 접근과updateAnswer의 불변 업데이트 패턴이 적절합니다.♻️ (선택 사항) 리스트 업데이트를 더 간결하게 표현할 수 있습니다
mapIndexed를 사용하면 중간 mutableList 생성 없이 더 선언적으로 표현할 수 있습니다:fun updateAnswer( questionId: Long, answer: String, ): Exams { val existingExamIndex = values.indexOfFirst { it.questionId == questionId } return if (existingExamIndex >= 0) { - val updatedValues = - values.toMutableList().apply { - set(existingExamIndex, Exam(questionId, answer)) - } - copy(values = updatedValues) + copy(values = values.mapIndexed { index, exam -> + if (index == existingExamIndex) Exam(questionId, answer) else exam + }) } else { copy(values = values + Exam(questionId, answer)) } }composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/component/ExamHistoryTab.kt (1)
36-46: 삭제 가능한 리스트는 key 지정이 안전합니다
items에 key가 없으면 삭제/정렬 시 항목 상태가 섞일 수 있습니다 → 고유 id를 key로 지정해 Compose가 항목을 안정적으로 추적하도록 해 주세요.♻️ 제안 수정
- LazyColumn( + LazyColumn( modifier = Modifier.fillMaxSize(), verticalArrangement = Arrangement.spacedBy(12.dp), contentPadding = PaddingValues(16.dp), ) { - items(state.histories) { history -> + items(state.histories, key = { it.id }) { history -> ExamHistoryCard( uiModel = history, onClick = { onOpenExamHistory(history.id) }, onDeleteClick = { onDeleteExam(history.id) }, ) } }composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/complete/ExamCompleteScreen.kt (2)
131-147: 하드코딩된 색상 값을 테마 색상으로 교체하는 것을 권장합니다.현재
Color(0xFFB8E6D5)와Color(0xFF34A853)가 직접 하드코딩되어 있습니다. 이렇게 하면 다크 테마 지원 시 일관성이 깨질 수 있고, 디자인 시스템 변경 시 여러 곳을 수정해야 합니다.테마의
ExtendedColors나MaterialTheme.colorScheme에 성공/완료 관련 색상을 정의하고 사용하는 것이 유지보수성 측면에서 더 좋습니다. 어떻게 생각하시나요?♻️ 권장 수정 예시
Box( modifier = Modifier .size(80.dp) .background( - color = Color(0xFFB8E6D5), + color = MaterialTheme.colorScheme.primaryContainer, shape = CircleShape, ), contentAlignment = Alignment.Center, ) { Icon( imageVector = Icons.Default.Check, contentDescription = null, modifier = Modifier.size(32.dp), - tint = Color(0xFF34A853), + tint = MaterialTheme.colorScheme.primary, ) }
47-63: 로딩 및 에러 상태 처리를 추가하면 UX가 개선됩니다.현재
ExamCompleteScreen은 데이터 로딩 중이나 에러 발생 시 사용자에게 피드백을 제공하지 않습니다.LaunchedEffect로 데이터를 불러오는 동안 빈 화면이 표시될 수 있습니다.
ExamCompleteUiState에isLoading,error상태를 추가하고, UI에서 이를 처리하면 사용자 경험이 향상됩니다. 현재 PR 범위를 고려해 추후 개선 사항으로 검토해 주세요.composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/vm/ExamProgressViewModel.kt (1)
74-84:cancelExam과sendEvent에서 불필요한 중첩 코루틴이 있습니다.
cancelExam()은 이미viewModelScope.launch를 사용하고 있고, 내부에서 호출하는sendEvent()도 다시viewModelScope.launch를 사용합니다. 이는 불필요한 중첩입니다.
cancelExam의 경우 코루틴 없이 직접sendEvent를 호출하거나,sendEvent에서 코루틴을 제거하는 방향으로 개선할 수 있습니다.♻️ 코루틴 중첩 제거 제안
fun cancelExam() { - viewModelScope.launch { - sendEvent(ExamProgressUiEvent.NavigateBack) - } + sendEvent(ExamProgressUiEvent.NavigateBack) } private fun sendEvent(event: ExamProgressUiEvent) { viewModelScope.launch { _uiEvent.send(event) } }또는
Channel.trySend를 사용하면 코루틴 없이도 가능합니다:private fun sendEvent(event: ExamProgressUiEvent) { - viewModelScope.launch { - _uiEvent.send(event) - } + _uiEvent.trySend(event) }composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/ExamProgressScreen.kt (2)
63-65:LaunchedEffect(Unit)사용에 대해 의도를 확인해주세요.현재
LaunchedEffect(Unit)을 사용하여 최초 컴포지션 시에만 문제를 로드합니다. 만약questionCount나categories가 변경될 가능성이 있다면, 이들을 키로 추가하는 것이 좋습니다.현재 설계가 의도적이라면 문제없습니다. 다만, 향후 재사용성을 고려하면 파라미터 변경에도 반응하도록 할 수 있습니다.
// 파라미터 변경 시에도 재로드가 필요한 경우: LaunchedEffect(questionCount, categories) { viewModel.loadExamQuestions(questionCount, categories) }
116-121: 문제 목록이 비어있을 때 "1/0" 표시를 개선할 수 있습니다.
questions.isEmpty()일 때"${uiState.currentQuestionIndex + 1}/${uiState.questions.size}"가 "1/0"으로 표시됩니다. 로딩 중일 때는 이 영역을 숨기거나 적절한 placeholder를 표시하는 것이 사용자 경험에 더 좋을 수 있습니다.♻️ 조건부 표시 제안
actions = { - Text( - text = - "${uiState.currentQuestionIndex + 1}/${uiState.questions.size}", - style = MaterialTheme.typography.titleMedium, - modifier = Modifier.padding(end = 16.dp), - ) + if (uiState.questions.isNotEmpty()) { + Text( + text = "${uiState.currentQuestionIndex + 1}/${uiState.questions.size}", + style = MaterialTheme.typography.titleMedium, + modifier = Modifier.padding(end = 16.dp), + ) + } },composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/vm/ExamViewModel.kt (1)
34-39: HISTORY 탭 선택 시 매번 데이터를 리로드하는 것이 의도적인가요?
selectTab(ExamTab.HISTORY)호출 시마다loadExamHistory()가 실행됩니다. 데이터가 변경되지 않았더라도 매번 네트워크 요청이 발생하여 불필요한 로딩 표시나 깜빡임이 발생할 수 있습니다.사용자가 탭을 자주 전환하는 경우를 고려하면, 캐시나 조건부 리프레시를 도입하는 것도 방법입니다. 물론 항상 최신 데이터를 보여주려는 의도라면 현재 구현도 괜찮습니다.
composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/component/ExamNavigationButtons.kt (1)
132-177: 활성 상태 텍스트/아이콘 색상을 onPrimary로 맞추면 테마 대비가 안정적입니다.왜:
Color.White고정은 밝은 primary나 커스텀 테마에서 대비가 부족할 수 있습니다.
어떻게:MaterialTheme.colorScheme.onPrimary로 전환하면 테마 대비가 자동 보장됩니다.🎨 제안 수정안
- Text( + Text( text = if (isLastQuestion) { stringResource(Res.string.exam_button_submit) } else { stringResource(Res.string.exam_button_next) }, style = MaterialTheme.typography.titleMedium, fontWeight = FontWeight.Bold, - color = if (isEnabled) Color.White else MaterialTheme.colorScheme.onSurfaceVariant, + color = if (isEnabled) MaterialTheme.colorScheme.onPrimary else MaterialTheme.colorScheme.onSurfaceVariant, ) Icon( imageVector = if (isLastQuestion) { Icons.Default.Send } else { Icons.AutoMirrored.Filled.ArrowForward }, contentDescription = null, - tint = if (isEnabled) Color.White else MaterialTheme.colorScheme.onSurfaceVariant, + tint = if (isEnabled) MaterialTheme.colorScheme.onPrimary else MaterialTheme.colorScheme.onSurfaceVariant, )composeApp/src/commonMain/kotlin/com/peto/droidmorning/main/MainScreen.kt (1)
130-139: 프리뷰에서 Koin 미초기화로 크래시가 날 수 있어요.왜: Preview 환경에서는 DI가 초기화되지 않아
koinViewModel()이 실패할 수 있습니다.
어떻게: 프리뷰에서는MainContent를 직접 호출하거나, 프리뷰 전용 뷰모델을 주입하도록 분리해 보는 건 어떨까요?🧪 제안 수정안
`@Preview` `@Composable` fun MainScreenPreview() { AppTheme { - MainScreen( - onNavigateToQuestionDetail = {}, - onNavigateToExamProgress = { _, _ -> }, - onNavigateToExamResult = {}, - ) + MainContent( + selectedTab = BottomNavigationType.QUESTION, + onNavigateToQuestionDetail = {}, + onNavigateToExamProgress = { _, _ -> }, + onNavigateToExamResult = {}, + savedStateHandle = null, + ) } }
| fun loadExamDetail(examId: Long) { | ||
| viewModelScope.launch { | ||
| examRepository | ||
| .fetchExamDetail(examId) | ||
| .onSuccess { examDetails -> | ||
| _uiState.update { | ||
| it.updateExamDetails(examDetails) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
실패/로딩 처리가 없어서 네트워크 오류 시 화면이 조용히 멈출 수 있습니다.
왜 문제인가: onSuccess만 처리하면 실패 시 UI 상태가 갱신되지 않아 사용자가 원인을 알기 어렵습니다.
어떻게 개선할까: 로딩 플래그와 오류 상태를 ExamCompleteUiState에 반영해 onFailure를 처리하고, 성공/실패 모두에서 로딩을 해제해 주세요. (필드가 없다면 추가를 고려해 주세요.)
🤖 Prompt for AI Agents
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/complete/vm/ExamCompleteViewModel.kt`
around lines 19 - 28, The loadExamDetail function currently only handles
onSuccess and never updates loading/error state; modify loadExamDetail (inside
viewModelScope.launch where examRepository.fetchExamDetail is called) to set a
loading flag in ExamCompleteUiState before the call, handle both onSuccess and
onFailure (use onFailure to update an error field/message in
ExamCompleteUiState), and ensure the loading flag is cleared in both branches
(or finally) when updating _uiState via _uiState.update; add loading/error
fields to ExamCompleteUiState if they don't exist.
| fun loadExamDetail(examId: Long) { | ||
| viewModelScope.launch { | ||
| examRepository | ||
| .fetchExamDetail(examId) | ||
| .onSuccess { examDetail -> | ||
| _uiState.update { | ||
| it.updateExamQuestions(examDetail) | ||
| } | ||
| } |
There was a problem hiding this comment.
실패 처리 누락으로 상세 화면이 빈 상태로 남을 수 있어요
fetchExamDetail 실패 시 onFailure 처리가 없어 UI 상태가 갱신되지 않습니다 → 사용자 입장에선 빈 리스트만 보이거나 로딩이 멈춘 것으로 보일 수 있어요. 실패 상태/재시도 가능 여부를 ExamDetailUiState에 반영하거나 에러 이벤트를 노출하도록 보완해 주세요. 실패 UI/로그 처리를 다른 곳에서 하고 계신가요?
🤖 Prompt for AI Agents
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/detail/vm/ExamDetailViewModel.kt`
around lines 19 - 27, loadExamDetail currently only handles
fetchExamDetail().onSuccess so failures leave the UI unchanged; add an onFailure
handler to catch errors from fetchExamDetail(examId) inside
viewModelScope.launch and update the _uiState (ExamDetailUiState) to reflect the
error state (e.g., set an error message, isLoading=false, and a retryable flag)
or emit an error event via the ViewModel event channel; also ensure you log the
exception. Modify ExamDetailUiState (or expose an error event) so loadExamDetail
can set a visible error and support retry, and update callers to handle that new
state/event.
| composable(NavRoutes.ExamProgress.route) { backStackEntry -> | ||
| val questionCount = | ||
| navController.previousBackStackEntry | ||
| ?.savedStateHandle | ||
| ?.get<Int>("questionCount") ?: 5 | ||
|
|
||
| val categoriesArray = | ||
| navController.previousBackStackEntry | ||
| ?.savedStateHandle | ||
| ?.get<Array<String>>("categories") ?: emptyArray() | ||
|
|
||
| ExamProgressScreen( | ||
| questionCount = questionCount, | ||
| categories = categoriesArray.map { name -> Category.from(name) }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the file structure and find related navigation files
find composeApp/src -name "*ExamProgress*" -type f | head -20Repository: chanho0908/DroidMorning
Length of output: 680
🏁 Script executed:
# Search for where ExamProgress route is defined and used
rg -n "ExamProgress\.(route|Graph)" --type ktRepository: chanho0908/DroidMorning
Length of output: 93
🏁 Script executed:
# Find where savedStateHandle is set for questionCount and categories
rg -n "savedStateHandle.*questionCount|savedStateHandle.*categories" --type ktRepository: chanho0908/DroidMorning
Length of output: 93
🏁 Script executed:
# Search for navigation to ExamProgress to see how values are passed
rg -n "NavigateToExamProgress|navController.*navigate.*ExamProgress" --type ktRepository: chanho0908/DroidMorning
Length of output: 93
🏁 Script executed:
# Read the full ExamProgressNavGraphContributor.kt file to understand the context
cat -n composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/navigation/ExamProgressNavGraphContributor.ktRepository: chanho0908/DroidMorning
Length of output: 2446
🏁 Script executed:
# Search for savedStateHandle usage without type restriction
rg -n "savedStateHandle.*questionCount|savedStateHandle.*categories|questionCount.*savedStateHandle"Repository: chanho0908/DroidMorning
Length of output: 49
🏁 Script executed:
# Search for navigation to ExamProgress
rg -n "NavigateToExamProgress|navigate.*ExamProgress"Repository: chanho0908/DroidMorning
Length of output: 1918
🏁 Script executed:
# Search for where questionCount and categories are set in navigation context
rg -n "previousBackStackEntry|savedStateHandle" -A 3 -B 3Repository: chanho0908/DroidMorning
Length of output: 14698
🏁 Script executed:
# Check the MainNavGraphContributor to see where values are set
cat -n composeApp/src/commonMain/kotlin/com/peto/droidmorning/main/navigation/MainNavGraphContributor.ktRepository: chanho0908/DroidMorning
Length of output: 3180
🏁 Script executed:
# Search for NavRoutes definitions to understand the graph structure
rg -n "object NavRoutes|sealed class NavRoutes" -A 20Repository: chanho0908/DroidMorning
Length of output: 2378
🏁 Script executed:
# Check if route arguments are defined for ExamProgress
rg -n "ExamProgress|ExamProgressGraph" composeApp/src/commonMain/kotlin/com/peto/droidmorning/navigation/Repository: chanho0908/DroidMorning
Length of output: 357
🏁 Script executed:
# Let's verify the back stack entry structure by checking if there's any custom handling
# or if the graph entry even has the savedStateHandle values
rg -n "backStackEntry|savedStateHandle" composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/navigation/ExamProgressNavGraphContributor.kt -B 2 -A 2Repository: chanho0908/DroidMorning
Length of output: 647
🏁 Script executed:
# Let's check what the complete NavRoutes file looks like to understand all routes
cat -n composeApp/src/commonMain/kotlin/com/peto/droidmorning/navigation/NavRoutes.ktRepository: chanho0908/DroidMorning
Length of output: 2109
🏁 Script executed:
# Check if ExamProgressGraph is navigated to from other places
rg -n "ExamProgressGraph|navigate.*ExamProgress" --type-not=rustRepository: chanho0908/DroidMorning
Length of output: 562
🏁 Script executed:
# Check if there are any tests or other navigation flows
rg -n "onNavigateToExamProgress|NavigateToExamProgress" -B 3 -A 3Repository: chanho0908/DroidMorning
Length of output: 8958
🏁 Script executed:
# Let's trace the actual flow: where does NavigateToExamProgress event come from?
cat -n composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/main/ExamScreen.kt | head -60Repository: chanho0908/DroidMorning
Length of output: 3379
중첩 그래프에서 사용자의 선택값이 무시되고 있습니다
ExamProgressGraph로 이동할 때 값이 올바르게 전달되지 않습니다. MainNavGraphContributor에서 Main 엔트리의 savedStateHandle에 값을 설정하지만, ExamProgressNavGraphContributor에서는 previousBackStackEntry(= ExamProgressGraph 엔트리)에서 읽으려고 합니다. 결과적으로 값을 찾을 수 없어 기본값(questionCount: 5, categories: empty)으로 진행되어 사용자의 선택이 무시됩니다.
해결 방안:
- QuestionDetail처럼 route arguments로
questionCount와categories를 정의하고 전달하거나 - 또는 공유 ViewModel이나 전역 상태 관리를 통해 데이터를 공유하세요
현재 코드 흐름
// MainNavGraphContributor.kt:33-37
navController.currentBackStackEntry?.savedStateHandle?.apply {
set("questionCount", questionCount) // ← Main 엔트리에 저장
set("categories", categories.map { it.name }.toTypedArray())
}
navController.navigate(NavRoutes.ExamProgressGraph.route)
// ExamProgressNavGraphContributor.kt:28-35
navController.previousBackStackEntry // ← ExamProgressGraph 엔트리를 가리킴
?.savedStateHandle
?.get<Int>("questionCount") ?: 5 // ← 값이 없음 → 기본값 사용
🤖 Prompt for AI Agents
In
`@composeApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/navigation/ExamProgressNavGraphContributor.kt`
around lines 26 - 39, The problem is that ExamProgressNavGraphContributor reads
saved state from previousBackStackEntry (the ExamProgressGraph entry) so it
misses values Main stored on currentBackStackEntry; update the retrieval to read
from navController.currentBackStackEntry?.savedStateHandle (matching where
MainNavGraphContributor set "questionCount" and "categories") in the composable
for NavRoutes.ExamProgress.route (replace usages of
previousBackStackEntry?.savedStateHandle?.get<Int>("questionCount") and
?.get<Array<String>>("categories") with
currentBackStackEntry?.savedStateHandle?.get...), or alternatively implement
route arguments on NavRoutes.ExamProgress or a shared ViewModel/global state if
you prefer that approach.
...oseApp/src/commonMain/kotlin/com/peto/droidmorning/exam/progress/vm/ExamProgressViewModel.kt
Show resolved
Hide resolved
| data class ExamRpcRequest( | ||
| val uid: String, | ||
| @SerialName("total_questions") | ||
| val questionsCount: Int, | ||
| val categories: List<Category>, | ||
| val items: List<ExamItemRpcRequest>, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the Category definition
find . -name "*.kt" -type f | xargs rg -l "class Category|data class Category" | head -20Repository: chanho0908/DroidMorning
Length of output: 331
🏁 Script executed:
# Look at the complete ExamRpcRequest file
cat -n data/src/commonMain/kotlin/com/peto/droidmorning/data/model/request/ExamRpcRequest.ktRepository: chanho0908/DroidMorning
Length of output: 953
🏁 Script executed:
# Search for Category definition in the data module
rg "class Category|data class Category" data/ -A 3Repository: chanho0908/DroidMorning
Length of output: 525
🏁 Script executed:
# Check if there are mapping functions related to ExamRpcRequest
rg "toRequest\|ExamRpcRequest\|toRpcRequest" data/ -B 2 -A 5Repository: chanho0908/DroidMorning
Length of output: 49
🏁 Script executed:
# Look for any Serializer implementations for Category
rg "Serializer.*Category|CategorySerializer" . -A 5Repository: chanho0908/DroidMorning
Length of output: 49
🏁 Script executed:
# Examine the Category domain model definition
cat -n domain/src/commonMain/kotlin/com/peto/droidmorning/domain/model/category/Category.ktRepository: chanho0908/DroidMorning
Length of output: 644
🏁 Script executed:
# Check if Category is imported/defined as `@Serializable`
rg "@Serializable.*Category|Category.*@Serializable" domain/ -B 2 -A 5Repository: chanho0908/DroidMorning
Length of output: 49
🏁 Script executed:
# Look for how other request DTOs handle domain models
rg "@Serializable" data/src/commonMain/kotlin/com/peto/droidmorning/data/model/request/ -A 10 | head -60Repository: chanho0908/DroidMorning
Length of output: 6169
🏁 Script executed:
# Check ExamQuestionRequest to see the correct pattern
cat -n data/src/commonMain/kotlin/com/peto/droidmorning/data/model/request/ExamQuestionRequest.ktRepository: chanho0908/DroidMorning
Length of output: 455
🏁 Script executed:
# Verify if there's any serialization setup for Category enum elsewhere
rg "Category" data/src/commonMain/kotlin/com/peto/droidmorning/data/model/request/ -B 1 -A 1Repository: chanho0908/DroidMorning
Length of output: 1135
RPC 요청 DTO에서 도메인 Category 모델의 직렬화 책임 분리 필요합니다
ExamRpcRequest가 도메인 모델 Category enum을 직렬화하려 하는데, 이는 두 가지 문제를 야기합니다:
- 아키텍처 원칙 위반: 코딩 가이드라인에 따르면 DTO와 도메인 모델은 명확히 분리되어야 합니다. 현재
ExamQuestionRequest는List<String>으로 올바르게 구현했는데,ExamRpcRequest만 도메인 모델을 직렬화하고 있습니다. - 직렬화 불일치 위험:
Categoryenum에는@Serializable어노테이션이 없어서 직렬화 동작이 명확하지 않으며, 서버 스키마가 문자열을 기대한다면 요청이 실패할 수 있습니다.
개선 방법: toRequest() 함수에서 Category를 문자열로 매핑하도록 변경하세요:
적용 예시
`@Serializable`
data class ExamRpcRequest(
val uid: String,
`@SerialName`("total_questions")
val questionsCount: Int,
- val categories: List<Category>,
+ val categories: List<String>,
val items: List<ExamItemRpcRequest>,
)
fun Exams.toRequest(
uid: String,
categories: List<Category>,
): ExamRpcRequest =
ExamRpcRequest(
uid = uid,
questionsCount = values.size,
- categories = categories,
+ categories = categories.map { it.name },
items = values.map { it.toRequest() },
)🤖 Prompt for AI Agents
In
`@data/src/commonMain/kotlin/com/peto/droidmorning/data/model/request/ExamRpcRequest.kt`
around lines 9 - 15, ExamRpcRequest is currently serializing the domain Category
enum (categories: List<Category>), which breaks DTO/domain separation and risks
serialization mismatch; change the DTO to use categories: List<String> instead
and update wherever ExamRpcRequest is constructed (notably the toRequest()
converter) to map each Category to its string representation (e.g.,
category.name or a defined string value) before building the ExamRpcRequest,
ensuring all code paths creating ExamRpcRequest use the string list rather than
the Category enum.
...gnsystem/src/commonMain/kotlin/com/peto/droidmorning/designsystem/component/ConfirmDialog.kt
Show resolved
Hide resolved
| companion object { | ||
| fun from(value: String): Category = | ||
| entries | ||
| .find { it.name.equals(value, ignoreCase = true) } | ||
| ?: error("$UNKNOWN_CATEGORY_EXCEPTION $value") | ||
|
|
||
| private const val UNKNOWN_CATEGORY_EXCEPTION = "Unknown category:" |
There was a problem hiding this comment.
알 수 없는 카테고리 입력 시 앱이 바로 크래시될 수 있습니다.
왜 문제인가: 서버에서 신규 카테고리가 추가되거나 오탈자/공백이 오면 error()로 앱이 종료됩니다.
어떻게 개선: 안전한 fallback(예: Unknown)을 두거나 null 반환 후 상위에서 처리하도록 바꾸는 게 좋지 않을까요?
💡 예시 수정안 (Unknown 추가)
enum class Category {
Kotlin,
Coroutine,
Android,
Compose,
OOP,
+ Unknown,
;
companion object {
fun from(value: String): Category =
entries
- .find { it.name.equals(value, ignoreCase = true) }
- ?: error("$UNKNOWN_CATEGORY_EXCEPTION $value")
-
- private const val UNKNOWN_CATEGORY_EXCEPTION = "Unknown category:"
+ .find { it.name.equals(value.trim(), ignoreCase = true) }
+ ?: Unknown
}
}🤖 Prompt for AI Agents
In
`@domain/src/commonMain/kotlin/com/peto/droidmorning/domain/model/category/Category.kt`
around lines 11 - 17, The current Category.from(value: String) uses error(...)
and will crash on unknown inputs; change it to a safe fallback by either (A)
adding an Unknown entry to the Category enum and returning that from
Category.from instead of calling error (update UNKNOWN_CATEGORY_EXCEPTION usage
accordingly), or (B) provide a nullable alternative like
Category.fromOrNull(value: String): Category? that returns null when not found
and keep from(...) throwing if you want to preserve strict behavior; modify call
sites that rely on from to handle the new Unknown or nullable return.
1 participant
이슈 번호
Closes #35
작업내용
시험(Exam) 기능을 전체적으로 구현했습니다.
주요 기능
시험 생성
시험 진행
시험 완료
시험 상세
시험 이력
아키텍처
리팩토링
category,question,exam하위 패키지)커밋 구조
총 22개의 세분화된 커밋으로 구성:
🤖 Generated with Firebender