Merged
Conversation
chanho0908
changed the title
chanho0908
changed the base branch from
develop
to
feat/#88-stats-detail-feature
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
feature/main/src/main/java/com/twix/stats/StatsViewModel.kt (1)
27-27: 월 캐시에 상한을 두면 메모리 안정성이 더 좋아집니다.현재 캐시는 월 이동 횟수만큼 계속 커질 수 있습니다. LRU 형태로 최대 보관 개수를 제한하는 방향을 고려해볼까요?
♻️ 제안 수정안
- private val inProgressStatsCache = mutableMapOf<YearMonth, Stats>() + private val inProgressStatsCache = + object : LinkedHashMap<YearMonth, Stats>(MAX_CACHE_SIZE, 0.75f, true) { + override fun removeEldestEntry( + eldest: MutableMap.MutableEntry<YearMonth, Stats>?, + ): Boolean = size > MAX_CACHE_SIZE + } companion object { private const val DEBOUNCE_INTERVAL = 300L + private const val MAX_CACHE_SIZE = 24 }Also applies to: 133-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/main/src/main/java/com/twix/stats/StatsViewModel.kt` at line 27, The inProgressStatsCache (mutableMapOf<YearMonth, Stats>) can grow unbounded; replace it with a bounded LRU map to limit entries and improve memory stability (e.g., use a LinkedHashMap/evicting map or a small in-memory cache library) and apply the same change to the other cache instances referenced around the StatsViewModel (the maps used for per-month stats at lines near 133-135). Ensure the replacement preserves key type YearMonth and value type Stats, enforces a configurable max size, and evicts least-recently-used entries on access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.kt`:
- Line 16: selectedDate parsing in StatsMapper uses
LocalDate.parse(selectedDate) which fails for yyyy-MM responses from the
YearMonth-based StatsService; update the mapping in StatsMapper to try multiple
parsing strategies (e.g., attempt LocalDate.parse with yyyy-MM-dd, fall back to
parsing YearMonth with DateTimeFormatter.ofPattern("yyyy-MM") and convert to a
LocalDate (e.g., first day of month), and handle DateTimeParseException by
logging or providing a safe default) so the selectedDate assignment is resilient
to both "yyyy-MM-dd" and "yyyy-MM" inputs.
In `@data/src/main/java/com/twix/data/repository/DefaultStatsRepository.kt`:
- Around line 28-33: The TODO in DefaultStatsRepository.fetchStatsDetail
currently throws NotImplementedError and causes crashes; implement the method to
call the data source (StatsService) to fetch details and convert the response
via the existing mapper into a domain StatsDetail, returning
AppResult.success(...) on success and AppResult.failure(...) (or the module's
error wrapper) on any exception or non-2xx response; ensure you use the same
function signature (suspend fun fetchStatsDetail(goalId: Long, date: YearMonth):
AppResult<StatsDetail>), handle HTTP/IO errors with try/catch, map service DTOs
to domain via the repository's mapper, and avoid throwing — return a safe
AppResult instead.
In `@domain/src/main/java/com/twix/domain/model/enums/StatsStatus.kt`:
- Line 13: The current StatsStatus.fromApi uses runCatching { valueOf(value)
}.getOrElse { IN_PROGRESS } which masks unknown/typo/back-end-added enum values
by mapping them to IN_PROGRESS; change this to either return a new safe enum
constant (e.g., add UNKNOWN to StatsStatus and map failures to UNKNOWN) or
propagate the failure to the caller (throw an IllegalArgumentException) so
callers can handle unexpected values; update fromApi to use valueOf(value) with
explicit handling (return UNKNOWN or rethrow) and ensure any code paths using
fromApi handle the new UNKNOWN or exception accordingly.
In `@feature/main/src/main/java/com/twix/stats/StatsViewModel.kt`:
- Around line 63-68: The async response can overwrite the UI when navigating
months quickly: in the launchResult success handler for launchResult/block
calling statsRepository.fetchStats (which currently writes to
inProgressStatsCache[date] and calls reduce { copy(inProgressStats = it) }), add
a guard that only updates the cache and calls reduce if the request's date/month
matches the current selected month in the ViewModel's state (compare the date
parameter to the ViewModel's selected-month state field, e.g.,
selectedDate/selectedMonth); keep using inProgressStatsCache and
reduce/inProgressStats but skip updates when they differ.
In
`@feature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt`:
- Around line 47-56: Restore the initial stats fetch and ensure month-change
triggers call fetchStatsDetail: uncomment or re-enable the initial
fetchStatsDetail(...) call in the init block (using argDate -> LocalDate.parse
-> YearMonth fallback to YearMonth.now()), and inside the collectMonthChangeFlow
subscription (where monthChangeFlow is collected) call
fetchStatsDetail(yearMonth) whenever a new YearMonth is emitted; keep
reduceNavArguments() and collectMonthChangeFlow() but ensure
fetchStatsDetail(...) is invoked both initially and on each monthChangeFlow
emission.
---
Nitpick comments:
In `@feature/main/src/main/java/com/twix/stats/StatsViewModel.kt`:
- Line 27: The inProgressStatsCache (mutableMapOf<YearMonth, Stats>) can grow
unbounded; replace it with a bounded LRU map to limit entries and improve memory
stability (e.g., use a LinkedHashMap/evicting map or a small in-memory cache
library) and apply the same change to the other cache instances referenced
around the StatsViewModel (the maps used for per-month stats at lines near
133-135). Ensure the replacement preserves key type YearMonth and value type
Stats, enforces a configurable max size, and evicts least-recently-used entries
on access.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
core/network/src/main/java/com/twix/network/di/ApiServiceModule.ktcore/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.ktcore/network/src/main/java/com/twix/network/model/response/stats/model/StatsResponse.ktcore/network/src/main/java/com/twix/network/service/StatsService.ktdata/src/main/java/com/twix/data/di/RepositoryModule.ktdata/src/main/java/com/twix/data/repository/DefaultStatsRepository.ktdata/src/main/java/com/twix/data/repository/FakeStatsRepository.ktdomain/src/main/java/com/twix/domain/model/enums/StatsStatus.ktdomain/src/main/java/com/twix/domain/repository/StatsRepository.ktfeature/main/src/main/java/com/twix/stats/StatsScreen.ktfeature/main/src/main/java/com/twix/stats/StatsViewModel.ktfeature/main/src/main/java/com/twix/stats/contract/StatsUiState.ktfeature/main/src/main/java/com/twix/stats/preview/StatsUiStatePreviewProvider.ktfeature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt
💤 Files with no reviewable changes (1)
- data/src/main/java/com/twix/data/repository/FakeStatsRepository.kt
core/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.kt
Show resolved
Hide resolved
feature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt
Show resolved
Hide resolved
📝 WalkthroughWalkthrough이 변경사항은 통계 데이터 조회 기능을 실제 네트워크 API 호출로 전환하는 종합적인 구조 개선입니다. 네트워크 서비스 계층에서 새로운 Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 주요 검토 포인트 및 제안1. 날짜 타입 전환에 따른 일관성 검증현황: 도메인 계층에서 확인 필요 사항:
2. StatsViewModel의 캐싱 및 요청 중복 제거 로직강점: 개선 제안:
3. StatsDetailViewModel의 주석 처리된 코드주의: 질문:
4. StatsMapper의 안정성강점: 검토 사항:
5. StatsRepository 인터페이스 변경의 영향도주의: 이 변경은 공개 인터페이스의 시그니처 변경입니다. 확인 필요:
6. 속성 이름 변경 (endStats → completedStats)긍정 평가: 이름이 더 명확하고 의도를 잘 드러냅니다. 변경 범위도 잘 관리되었습니다. 최종 확인: 모든 관련 UI 컴포넌트와 테스트가 이 변경을 반영했는지 재확인 부탁합니다. 종합 의견이 PR은 가짜 데이터에서 실제 네트워크 API로의 전환이라는 중요한 마이그레이션을 담고 있습니다. 레이어 간 책임 분리가 명확하고, 캐싱과 요청 중복 제거 로직도 잘 설계되었습니다. 다만 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
domain/src/main/java/com/twix/domain/model/enums/StatsStatus.kt (1)
12-14:⚠️ Potential issue | 🟠 Major알 수 없는 API 상태값이
IN_PROGRESS로 무음 폴백됩니다.이 문제는 이전 리뷰에서 이미 언급되었으며, 코드에 아직 반영되지 않아 재언급합니다. 알 수 없는
value가 들어왔을 때IN_PROGRESS로 강제 매핑하면, 백엔드에서 새로운 상태를 추가하거나 오타가 발생했을 때 "진행중" 상태로 오분류되어 도메인 의미가 왜곡됩니다.개선 방향:
UNKNOWN값을 추가하거나, 최소한 로그를 남겨 운영 중 이슈를 추적할 수 있도록 하는 것을 권장합니다.🛠️ 개선 예시
enum class StatsStatus { NOT_STARTED, IN_PROGRESS, COMPLETED, DELETED, + UNKNOWN, ; fun toApi(): String = name companion object { - fun fromApi(value: String): StatsStatus = runCatching { valueOf(value) }.getOrElse { IN_PROGRESS } + fun fromApi(value: String): StatsStatus = + runCatching { valueOf(value) }.getOrElse { + Log.w("StatsStatus", "Unknown StatsStatus: $value, falling back to UNKNOWN") + UNKNOWN + } } }As per coding guidelines [Domain 모듈 리뷰 가이드] "비즈니스 로직이 명확히 표현되어 있는가?" 규칙에 따라, 알 수 없는 상태를 특정 정상 상태(
IN_PROGRESS)로 강제 매핑하는 것은 도메인 의미를 흐립니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@domain/src/main/java/com/twix/domain/model/enums/StatsStatus.kt` around lines 12 - 14, The fromApi function in companion object of StatsStatus currently swallows unknown API values by mapping them to IN_PROGRESS; change it to either return a dedicated UNKNOWN enum value (add UNKNOWN to StatsStatus) or throw/log the unexpected value instead of defaulting to IN_PROGRESS so domain meaning isn't corrupted; specifically update the companion object method fromApi to detect missing value (valueOf failure) and map to StatsStatus.UNKNOWN or call process/logger with the raw value and return a safe sentinel, ensuring references to fromApi, StatsStatus, IN_PROGRESS are adjusted accordingly.feature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt (1)
47-56:⚠️ Potential issue | 🔴 Critical초기/월 변경 상세 조회가 비활성화되어 화면 데이터가 갱신되지 않습니다.
왜 문제인가: 초기 진입(Line 48-53)과 월 변경 수집(Line 63-65)에서
fetchStatsDetail(...)호출이 주석 처리되어, 상태의 월만 바뀌고 실제 조회가 발생하지 않습니다.
어떻게 개선하나:init에서 초기 1회 조회를 복구하고,monthChangeFlow수집부에서fetchStatsDetail(yearMonth)를 다시 호출해 주세요.argDate가 있으면LocalDate.parse(argDate)→YearMonth.from(...)로 시작 월을 계산하는 방식이 안전합니다.As per coding guidelines, [Feature 모듈 - MVI 패턴 리뷰 가이드]의
단방향 데이터 플로우(Intent → ViewModel → State → View)가 유지되는가?규칙을 근거로 한 코멘트입니다.Also applies to: 63-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt` around lines 47 - 56, Restore the disabled initial and month-change detail fetches: un-comment and call fetchStatsDetail(...) once in the init block (using argDate?.let { LocalDate.parse(it) }?.let(YearMonth::from) ?: YearMonth.now() to compute the start YearMonth) and also invoke fetchStatsDetail(yearMonth) inside the monthChangeFlow collector in collectMonthChangeFlow()/where monthChangeFlow is collected so that reduceNavArguments(), collectMonthChangeFlow(), and state updates trigger an actual data load on init and on month changes.data/src/main/java/com/twix/data/repository/DefaultStatsRepository.kt (1)
28-33:⚠️ Potential issue | 🔴 Critical
fetchStatsDetail의 TODO는 런타임 크래시를 유발합니다.왜 문제인가: 현재 구현은 호출 시
NotImplementedError로 종료되어 상세 화면 조회 플로우가 즉시 깨집니다.
어떻게 개선하나: TODO를 제거하고 실제 서비스 호출 + 매핑을safeApiCall로 감싸AppResult를 안전하게 반환해 주세요. 혹시 상세 API 시그니처가 이미 정해졌다면, 그 시그니처 기준으로 바로 연결해도 될까요?As per coding guidelines, [Data 모듈 리뷰 가이드]의
Repository 구현 - Domain 인터페이스를 정확히 구현하는가?규칙을 근거로 한 코멘트입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/twix/data/repository/DefaultStatsRepository.kt` around lines 28 - 33, Replace the TODO in DefaultStatsRepository.fetchStatsDetail with a real implementation that wraps the remote call and mapping in safeApiCall and returns an AppResult<StatsDetail>; specifically, call the repository's stats API/client (the same client used elsewhere in this class), invoke the detail endpoint for the given goalId and date, map the response to the domain StatsDetail (using the existing mapper function, e.g., toStatsDetail() or equivalent), and return the result from safeApiCall so errors are captured as AppResult instead of throwing NotImplementedError.
🧹 Nitpick comments (4)
feature/main/src/main/java/com/twix/stats/preview/StatsUiStatePreviewProvider.kt (1)
56-56:completedStats리네임이 두 프리뷰 상태 모두에 반영되었습니다! 👍한 가지 선택적 제안:
defaultState()에서도completedStats를emptyList()로만 두면 종료된 통계 탭의 UI 렌더링을 Compose Preview에서 직접 확인하기 어렵습니다.EndStatsContent의 UI를 프리뷰로 검증하고 싶다면defaultState에 샘플 데이터를 추가하는 것을 고려해보세요.Also applies to: 66-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/main/src/main/java/com/twix/stats/preview/StatsUiStatePreviewProvider.kt` at line 56, Update the preview data so Compose Preview shows the closed-stats UI: in StatsUiStatePreviewProvider, populate completedStats with representative sample items instead of emptyList() in the preview states (e.g., inside defaultState() and the other preview state at the same file). Create a small list of sample completed stat objects that match the StatsUiState/EndStatsContent model and assign it to completedStats so EndStatsContent renders meaningfully in the preview.core/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.kt (1)
38-40:valueOf파싱 실패 시 로그 없이 기본값으로 폴백되어 API 불일치를 추적하기 어렵습니다.왜 문제가 되나요?
StampType.valueOf(this)와StampColor.valueOf(this)는 대소문자를 구분합니다. API가"clover"나"green400"같이 다른 케이싱으로 응답하면runCatching이 예외를 삼키고 조용히 기본값을 반환합니다. 이렇게 되면 실제 API 불일치가 앱에서 전혀 드러나지 않아 디버깅이 어려워집니다.어떻게 개선할 수 있을까요? 최소한 폴백 발생 시 로그를 남겨 추적 가능하도록 하는 것을 권장합니다.
🛠️ 개선 예시
+import android.util.Log -private fun String.toStampType(): StampType = runCatching { StampType.valueOf(this) }.getOrElse { StampType.CLOVER } +private fun String.toStampType(): StampType = + runCatching { StampType.valueOf(this) }.getOrElse { + Log.w("StatsMapper", "Unknown StampType: $this, falling back to CLOVER") + StampType.CLOVER + } -private fun String.toStampColor(): StampColor = runCatching { StampColor.valueOf(this) }.getOrElse { StampColor.GREEN400 } +private fun String.toStampColor(): StampColor = + runCatching { StampColor.valueOf(this) }.getOrElse { + Log.w("StatsMapper", "Unknown StampColor: $this, falling back to GREEN400") + StampColor.GREEN400 + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.kt` around lines 38 - 40, The String extension functions toStampType and toStampColor silently swallow valueOf failures via runCatching and return defaults, making API casing mismatches invisible; update both functions to first normalize the input (e.g., this.uppercase()) before calling StampType.valueOf / StampColor.valueOf and add a catch block that logs a warning including the original value and the default chosen (use the project's logger instance) so any fallback is observable; keep the default fallback behavior but ensure the log mentions the function name (toStampType/toStampColor), the raw input, and the default returned.core/network/src/main/java/com/twix/network/model/response/stats/model/StatsResponse.kt (1)
9-9:statsGoals와stampColors리스트 필드에 null 안전성 보완을 고려해보세요.왜 문제가 되나요? 현재 두 필드 모두 non-nullable
List로 선언되어 있습니다. API가 해당 필드를null로 반환하거나 필드 자체를 누락하면, kotlinx.serialization은SerializationException을 던져 런타임에 크래시가 발생합니다.어떻게 개선할 수 있을까요? 서버 계약이 항상 non-null을 보장한다면 현재 구조로 충분하지만, 방어적으로 nullable + 기본값 처리를 고려해볼 수 있습니다.
`@Serializable` data class StatsResponse( `@SerialName`("selectedDate") val selectedDate: String, - `@SerialName`("statsGoals") val statsGoals: List<StatsGoalResponse>, + `@SerialName`("statsGoals") val statsGoals: List<StatsGoalResponse> = emptyList(), ) `@Serializable` data class ParticipantStatsResponse( `@SerialName`("nickname") val nickname: String, `@SerialName`("endCount") val endCount: Int, - `@SerialName`("stampColors") val stampColors: List<String>, + `@SerialName`("stampColors") val stampColors: List<String> = emptyList(), )서버 스펙이 null을 절대 반환하지 않음이 확실하다면 현 구조를 유지해도 무방합니다. 다만 스펙이 바뀌거나 오류 응답에서 필드가 누락될 경우를 대비해 방어 처리 여부를 팀과 논의해보시면 좋겠습니다.
Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/network/src/main/java/com/twix/network/model/response/stats/model/StatsResponse.kt` at line 9, Change the collection fields in StatsResponse to be defensive against missing/null payloads: update the declarations for statsGoals and stampColors to nullable types with a safe default (e.g., val statsGoals: List<StatsGoalResponse>? = emptyList() and val stampColors: List<StampColorResponse>? = emptyList()) so kotlinx.serialization won't throw when the fields are omitted or null; locate these fields in the StatsResponse class and apply the same pattern to both.core/network/src/main/java/com/twix/network/service/StatsService.kt (1)
10-13: 쿼리 파라미터로YearMonth를 직접 전달하는 방식은 명시성 측면에서 개선할 수 있습니다.Ktorfit의
@Query파라미터는String이 아닌 타입을toString()으로 직렬화합니다. 현재 코드에서는YearMonth.toString()이"2024-01"형식의 문자열을 반환하고, 이것이 API 쿼리 파라미터로 전송됩니다. API가 이 형식을 수용하고 있으므로 기능상 문제는 없지만, 직렬화 형식을 코드에서 명시적으로 표현하면 의도를 더 명확하게 드러낼 수 있습니다.호출부(
DefaultStatsRepository)에서 포맷팅된String을 전달하는 방식이 더 명확하고 예측 가능합니다:interface StatsService { `@GET`("api/v1/stats") suspend fun fetchStats( - `@Query`("selectedDate") selectedDate: YearMonth, + `@Query`("selectedDate") selectedDate: String, `@Query`("status") status: String, ): StatsResponse }호출부에서:
statsService.fetchStats( selectedDate = yearMonth.toString(), // "yyyy-MM" 형식 명시 status = status.toApi() )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/network/src/main/java/com/twix/network/service/StatsService.kt` around lines 10 - 13, Change the fetchStats signature to accept a formatted String instead of YearMonth so the query serialization is explicit: update the suspend fun fetchStats(...) declaration in StatsService (replace YearMonth parameter with String named selectedDate) and update callers such as DefaultStatsRepository to pass yearMonth.toString() (or a dedicated formatter producing "yyyy-MM") and leave status mapping (status.toApi()) as-is; ensure tests/usage sites are adjusted to the new parameter type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@feature/main/src/main/java/com/twix/stats/StatsViewModel.kt`:
- Around line 86-92: Move the freshness checks so the cache is only updated
after verifying the response is the latest and for the current month: compute
isLatestRequest (requestId == latestInProgressRequestId) and isCurrentMonth
(YearMonth.from(currentState.currentDate) == date) first, then if both are true
call inProgressStatsCache[date] = stats and reduce { copy(inProgressStats =
stats) } (or update only the cache when not updating current state if intended),
otherwise skip writing to inProgressStatsCache to avoid older responses
overwriting newer data.
---
Duplicate comments:
In `@data/src/main/java/com/twix/data/repository/DefaultStatsRepository.kt`:
- Around line 28-33: Replace the TODO in DefaultStatsRepository.fetchStatsDetail
with a real implementation that wraps the remote call and mapping in safeApiCall
and returns an AppResult<StatsDetail>; specifically, call the repository's stats
API/client (the same client used elsewhere in this class), invoke the detail
endpoint for the given goalId and date, map the response to the domain
StatsDetail (using the existing mapper function, e.g., toStatsDetail() or
equivalent), and return the result from safeApiCall so errors are captured as
AppResult instead of throwing NotImplementedError.
In `@domain/src/main/java/com/twix/domain/model/enums/StatsStatus.kt`:
- Around line 12-14: The fromApi function in companion object of StatsStatus
currently swallows unknown API values by mapping them to IN_PROGRESS; change it
to either return a dedicated UNKNOWN enum value (add UNKNOWN to StatsStatus) or
throw/log the unexpected value instead of defaulting to IN_PROGRESS so domain
meaning isn't corrupted; specifically update the companion object method fromApi
to detect missing value (valueOf failure) and map to StatsStatus.UNKNOWN or call
process/logger with the raw value and return a safe sentinel, ensuring
references to fromApi, StatsStatus, IN_PROGRESS are adjusted accordingly.
In
`@feature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt`:
- Around line 47-56: Restore the disabled initial and month-change detail
fetches: un-comment and call fetchStatsDetail(...) once in the init block (using
argDate?.let { LocalDate.parse(it) }?.let(YearMonth::from) ?: YearMonth.now() to
compute the start YearMonth) and also invoke fetchStatsDetail(yearMonth) inside
the monthChangeFlow collector in collectMonthChangeFlow()/where monthChangeFlow
is collected so that reduceNavArguments(), collectMonthChangeFlow(), and state
updates trigger an actual data load on init and on month changes.
---
Nitpick comments:
In
`@core/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.kt`:
- Around line 38-40: The String extension functions toStampType and toStampColor
silently swallow valueOf failures via runCatching and return defaults, making
API casing mismatches invisible; update both functions to first normalize the
input (e.g., this.uppercase()) before calling StampType.valueOf /
StampColor.valueOf and add a catch block that logs a warning including the
original value and the default chosen (use the project's logger instance) so any
fallback is observable; keep the default fallback behavior but ensure the log
mentions the function name (toStampType/toStampColor), the raw input, and the
default returned.
In
`@core/network/src/main/java/com/twix/network/model/response/stats/model/StatsResponse.kt`:
- Line 9: Change the collection fields in StatsResponse to be defensive against
missing/null payloads: update the declarations for statsGoals and stampColors to
nullable types with a safe default (e.g., val statsGoals:
List<StatsGoalResponse>? = emptyList() and val stampColors:
List<StampColorResponse>? = emptyList()) so kotlinx.serialization won't throw
when the fields are omitted or null; locate these fields in the StatsResponse
class and apply the same pattern to both.
In `@core/network/src/main/java/com/twix/network/service/StatsService.kt`:
- Around line 10-13: Change the fetchStats signature to accept a formatted
String instead of YearMonth so the query serialization is explicit: update the
suspend fun fetchStats(...) declaration in StatsService (replace YearMonth
parameter with String named selectedDate) and update callers such as
DefaultStatsRepository to pass yearMonth.toString() (or a dedicated formatter
producing "yyyy-MM") and leave status mapping (status.toApi()) as-is; ensure
tests/usage sites are adjusted to the new parameter type.
In
`@feature/main/src/main/java/com/twix/stats/preview/StatsUiStatePreviewProvider.kt`:
- Line 56: Update the preview data so Compose Preview shows the closed-stats UI:
in StatsUiStatePreviewProvider, populate completedStats with representative
sample items instead of emptyList() in the preview states (e.g., inside
defaultState() and the other preview state at the same file). Create a small
list of sample completed stat objects that match the
StatsUiState/EndStatsContent model and assign it to completedStats so
EndStatsContent renders meaningfully in the preview.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
core/network/src/main/java/com/twix/network/di/ApiServiceModule.ktcore/network/src/main/java/com/twix/network/model/response/stats/mapper/StatsMapper.ktcore/network/src/main/java/com/twix/network/model/response/stats/model/StatsResponse.ktcore/network/src/main/java/com/twix/network/service/StatsService.ktdata/src/main/java/com/twix/data/di/RepositoryModule.ktdata/src/main/java/com/twix/data/repository/DefaultStatsRepository.ktdata/src/main/java/com/twix/data/repository/FakeStatsRepository.ktdomain/src/main/java/com/twix/domain/model/enums/StatsStatus.ktdomain/src/main/java/com/twix/domain/repository/StatsRepository.ktfeature/main/src/main/java/com/twix/stats/StatsScreen.ktfeature/main/src/main/java/com/twix/stats/StatsViewModel.ktfeature/main/src/main/java/com/twix/stats/contract/StatsUiState.ktfeature/main/src/main/java/com/twix/stats/preview/StatsUiStatePreviewProvider.ktfeature/stats/detail/src/main/java/com/twix/stats/detail/StatsDetailViewModel.kt
💤 Files with no reviewable changes (1)
- data/src/main/java/com/twix/data/repository/FakeStatsRepository.kt
dogmania
approved these changes
Feb 25, 2026
Member
There was a problem hiding this comment.
확실히 캐시를 두고 쓰는 게 불러오는 속도가 빨라서 좋네요 근데 지금 캐시 최대 사이즈 같은 정책이 따로 없어서 캐시 사이즈가 늘어날 수도 있을 것 같아요 나중에 최대 사이즈를 정하거나 제한을 추가해주세요
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
이슈 번호
#88
작업내용
StatsService와StatsResponse/StatsMapper를 추가StatsRepository를YearMonth + StatsStatus기반 시그니처로 변경DefaultStatsRepository로 교체하고, 기존FakeStatsRepository는 제거StatsViewModel을 신규 인터페이스에 맞춰 정리하고, 월 이동 시YearMonth단위 캐싱/디바운스로 불필요한 호출을 줄였습니다.StatsDetailViewModel도YearMonth기반 상세 조회 하도록 수정결과물
리뷰어에게 추가로 요구하는 사항 (선택)