기념품 조회 수정 삭제 API 변경 및 Top10 국가별 조회 API 교체, 유저메모리 캐시 삭제#18
Open
기념품 조회 수정 삭제 API 변경 및 Top10 국가별 조회 API 교체, 유저메모리 캐시 삭제#18
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough이 PR은 사용자 토큰 캐싱 메커니즘을 메모리 기반(UserMemoryCache)에서 DataStore 기반 지속성으로 마이그레이션하며, 동시에 Discovery API 엔드포인트를 재구성하고 가격 모델 구조를 이중화(oldPrice/newPrice)하는 포괄적인 리팩토링입니다. 또한 인증 인터셉터의 토큰 검색 로직을 업데이트하고, UI 모델 계층을 정리하여 가격 상태 표현을 분리합니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 이유:
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
domain/src/main/java/com/swyp/souzip/domain/repository/DiscoveryRepository.kt (1)
10-14:⚠️ Potential issue | 🟡 Minor함수 시그니처와 KDoc 파라미터 설명이 불일치합니다.
Line 11-12의
@param countryCode는 현재 함수 시그니처(무인자)와 맞지 않아 오해를 유발할 수 있습니다.수정 예시
/** * 나라별 추천 기념품 TOP 10 조회 - * `@param` countryCode 국가 코드 (예: "JP", "US") * `@return` 추천 기념품 목록 */ suspend fun getDiscoveryTopByCountry(): SouZipResult<List<DiscoveryTopByCountry>>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@domain/src/main/java/com/swyp/souzip/domain/repository/DiscoveryRepository.kt` around lines 10 - 14, The KDoc for getDiscoveryTopByCountry() mentions a `@param` countryCode but the function has no parameters; either remove the `@param` line from the KDoc or add the countryCode parameter to the function signature and update its return/behavior accordingly; locate the KDoc block above the suspend fun getDiscoveryTopByCountry() declaration and update either the comment or the function signature to keep them consistent (e.g., change to suspend fun getDiscoveryTopByCountry(countryCode: String): SouZipResult<List<DiscoveryTopByCountry>> if the parameter is required, or delete the `@param` countryCode tag if not).data/src/main/java/com/swyp/souzip/data/remote/repository/DiscoveryRepositoryImpl.kt (1)
21-55:⚠️ Potential issue | 🟠 Major코루틴 취소 예외가
Throwable캐치에서 흡수됩니다.현재 구조에서는
CancellationException도SouZipResult.Failure로 변환되어 취소 전파가 끊깁니다. Line 29, Line 41, Line 53 경로에 취소 예외 재전파 분기를 추가해 주세요.수정 예시
import com.swyp.souzip.domain.util.SouZipException import com.swyp.souzip.domain.util.SouZipResult +import kotlinx.coroutines.CancellationException import retrofit2.HttpException import java.io.IOException @@ try { val response = discoveryDataSource.getDiscoveryTopByCountry() SouZipResult.Success(response.map { it.toDomain() }) + } catch (e: CancellationException) { + throw e } catch (e: IOException) { SouZipResult.Failure(SouZipException.NetworkError(cause = e)) } catch (e: HttpException) { SouZipResult.Failure(e.toSouZipException()) } catch (e: Throwable) { SouZipResult.Failure(SouZipException.UnknownError(cause = e)) } @@ try { val response = discoveryDataSource.getDiscoveryByCategory(categoryName) SouZipResult.Success(response.map { it.toDomain() }) + } catch (e: CancellationException) { + throw e } catch (e: IOException) { SouZipResult.Failure(SouZipException.NetworkError(cause = e)) } catch (e: HttpException) { SouZipResult.Failure(e.toSouZipException()) } catch (e: Throwable) { SouZipResult.Failure(SouZipException.UnknownError(cause = e)) } @@ try { val response = discoveryDataSource.getMonthlyCountryRanking() SouZipResult.Success(response.toDomain()) + } catch (e: CancellationException) { + throw e } catch (e: IOException) { SouZipResult.Failure(SouZipException.NetworkError(cause = e)) } catch (e: HttpException) { SouZipResult.Failure(e.toSouZipException()) } catch (e: Throwable) { SouZipResult.Failure(SouZipException.UnknownError(cause = e)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/swyp/souzip/data/remote/repository/DiscoveryRepositoryImpl.kt` around lines 21 - 55, The catch-all Throwable handlers in getDiscoveryTopByCountry, getDiscoveryByCategory, and getMonthlyCountryRanking are swallowing CancellationException; update each Throwable catch to rethrow if the caught Throwable is a CancellationException (or call throw when e is CancellationException) so coroutine cancellations propagate, otherwise wrap into SouZipException.UnknownError as before; reference the methods getDiscoveryTopByCountry, getDiscoveryByCategory, and getMonthlyCountryRanking and ensure CancellationException from kotlinx.coroutines is checked and rethrown before creating SouZipResult.Failure.presentation/src/main/java/com/swyp/souzip/presentation/discovery/DiscoveryViewModel.kt (1)
33-54:⚠️ Potential issue | 🟠 MajorOrbit MVI에서 중첩
intent {}를 사용하면 로딩 상태가 실제 완료 전에 해제될 수 있습니다.현재
loadInitialData()와handleRefresh()함수에서intent {}블록 내부에 다시intent {}를 반환하는 로더 함수들(loadTopSouvenirsByCountry(),loadTrendingSouvenirsByCategory(),loadCountryRanking())을 호출하고 있습니다. 이는 Orbit MVI 아키텍처에서 명시적으로 권장하지 않는 패턴입니다.구체적인 문제:
- 로더 함수들이 별도의
intent {}컨텍스트에서 실행되면,isLoading/isRefreshing플래그가 실제 비동기 작업 완료 전에false로 내려갑니다.- 이는 상태 관리의 일관성을 해치고 UI에서 부정확한 로딩 상태를 표시할 수 있습니다.
해결 방법:
로더 함수들을 일반suspend함수로 변경하여 같은intent컨텍스트 내에서 순차 실행되도록 구조를 개선해야 합니다.수정 예시
- private fun loadTopSouvenirsByCountry() = - intent { + private suspend fun loadTopSouvenirsByCountry() { when (val result = discoveryRepository.getDiscoveryTopByCountry()) { is SouZipResult.Success -> { val uiModels = result.data.map { it.toUiModel() } val initialCountryCode = uiModels.firstOrNull()?.countryCode ?: "" reduce { state.copy( topByCountryData = uiModels, selectedCountryCode = initialCountryCode, errorMessage = null, ) } } is SouZipResult.Failure -> { reduce { state.copy(errorMessage = result.exception.message) } } } - } + } - private fun loadTrendingSouvenirsByCategory(categoryCode: String) = - intent { + private suspend fun loadTrendingSouvenirsByCategory(categoryCode: String) { when (val result = discoveryRepository.getDiscoveryByCategory(categoryCode)) { is SouZipResult.Success -> { val uiModels = result.data.map { it.toUiModelWithoutCountry() } reduce { state.copy(trendingSouvenirsByCategory = uiModels, errorMessage = null) } } is SouZipResult.Failure -> { reduce { state.copy(errorMessage = result.exception.message) } } } - } + } - private fun loadCountryRanking() = - intent { + private suspend fun loadCountryRanking() { when (val result = discoveryRepository.getMonthlyCountryRanking()) { is SouZipResult.Success -> { val uiModels = result.data.take(3).mapIndexed { index, ranking -> val flagImage = localCountryRepository.getFlagImageByCode(ranking.countryCode) ?: "" ranking.toUiModel(index, flagImage) } reduce { state.copy(topCountriesRanking = uiModels, errorMessage = null) } } is SouZipResult.Failure -> { reduce { state.copy(errorMessage = result.exception.message) } } } - } + }적용 범위: 33-54줄의
loadInitialData(), 86-93줄의handleRefresh(), 105-114줄의handleCategoryFilterSelected()등에서 로더 함수를 호출하는 모든intent블록이 영향을 받습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/swyp/souzip/presentation/discovery/DiscoveryViewModel.kt` around lines 33 - 54, The intent blocks (e.g., loadInitialData, handleRefresh, handleCategoryFilterSelected) currently call loader helpers that themselves start new intent { } contexts (loadTopSouvenirsByCountry, loadTrendingSouvenirsByCategory, loadCountryRanking), causing isLoading/isRefreshing to be cleared prematurely; refactor those loader helpers into plain suspend functions (remove their internal intent { } usage) and call them directly from the surrounding intent { } so their work runs sequentially inside the same Orbit intent context, keeping loading flags true until the suspend calls complete (update function signatures for loadTopSouvenirsByCountry, loadTrendingSouvenirsByCategory, loadCountryRanking and change their callers accordingly).
🧹 Nitpick comments (10)
gradle/libs.versions.toml (1)
250-250: LeakCanary 항목의 섹션 위치는 Debug 쪽이 더 명확합니다.현재
# Test블록 아래에 있어 실제 용도(디버그 메모리 진단)와 분류가 달라 보입니다. 유지보수 혼선을 줄이기 위해# Debug섹션으로 이동을 권장드립니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gradle/libs.versions.toml` at line 250, The leakcanary dependency entry named "leakcanary-android" (version.ref = "leakcanaryAndroid") is placed under the # Test section but belongs in the # Debug section; move the line `leakcanary-android = { module = "com.squareup.leakcanary:leakcanary-android", version.ref = "leakcanaryAndroid" }` out of the Test block and insert it under the # Debug block so the dependency classification matches its runtime use.presentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/LocationState.kt (1)
8-8: 기본countryCode의"US"하드코딩은 오동작 여지가 있습니다.위치 미선택 상태에서도 국가가 미국으로 채워져 downstream 로직(표시/요청)에 잘못 전달될 수 있습니다.
초기값은 빈 값(또는 null)로 두고, 실제 위치 선택 시 설정하는 편이 안전합니다.수정 예시
-data class LocationState( - val countryCode: String = "US", +data class LocationState( + val countryCode: String = "",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/LocationState.kt` at line 8, LocationState의 countryCode가 기본값으로 "US"로 하드코딩되어 있어 위치 미선택 시에도 downstream에 잘못 전달될 수 있으니 countryCode를 빈 문자열 또는 nullable(String?)로 초기화하고 실제 사용자는 위치 선택 시에만 값을 할당하도록 변경하세요; 참조 대상: data class LocationState 및 해당 프로퍼티 countryCode를 수정해 초기값 제거(혹은 null 허용)하고, 위치 선택 로직(setter/함수)에서만 값을 설정하도록 관련 호출부를 검토/수정하세요.domain/src/main/java/com/swyp/souzip/domain/repository/LocalUserRepository.kt (1)
19-21: 문서 주석 업데이트가 필요합니다.
setGuestUser()주석에서 "메모리 캐시의 토큰을 초기화합니다"라고 되어 있는데, 이제 메모리 캐시를 사용하지 않으므로 문서를 수정해주시면 좋겠습니다.📝 문서 수정 제안
/** * 현재 사용자를 게스트 모드로 설정합니다. - * 메모리 캐시의 토큰을 초기화합니다. + * DataStore에 저장된 토큰을 초기화합니다. */ suspend fun setGuestUser()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@domain/src/main/java/com/swyp/souzip/domain/repository/LocalUserRepository.kt` around lines 19 - 21, setGuestUser()의 KDoc에서 더이상 사용하지 않는 "메모리 캐시의 토큰을 초기화합니다" 문구를 삭제하고, 현재 동작을 정확히 설명하도록 주석을 갱신하세요; 예를 들어 "현재 사용자를 게스트로 설정합니다"와 (필요하면) "영구 저장소(또는 관련 토큰 저장소)에서 토큰을 제거합니다"처럼 실제 구현과 일치하는 한 줄로 수정해 주세요. 참고 심볼: setGuestUser().data/src/main/java/com/swyp/souzip/data/remote/interceptor/TokenAuthenticator.kt (1)
26-27: 문서 주석의 "메모리 캐시" 언급 수정이 필요합니다.6번 항목에서 "로컬 저장소와 메모리 캐시 모두 업데이트"라고 되어 있는데, 이제 메모리 캐시를 사용하지 않으므로 문서를 수정해주시면 좋겠습니다.
📝 문서 수정 제안
- * 6. 성공 시: 로컬 저장소와 메모리 캐시 모두 업데이트, 새 AccessToken으로 요청 재시도 + * 6. 성공 시: 로컬 저장소(DataStore) 업데이트, 새 AccessToken으로 요청 재시도🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/swyp/souzip/data/remote/interceptor/TokenAuthenticator.kt` around lines 26 - 27, Update the documentation comment in TokenAuthenticator.kt to remove the obsolete "memory cache" reference: change item 6 to state that on success the local storage (e.g., SharedPreferences or whatever persistent store is used) is updated and the request is retried with the new access token, and ensure item 7 remains unchanged; locate the class TokenAuthenticator and the authenticate/handleTokenRefresh comment block to make this text-only edit.data/src/main/java/com/swyp/souzip/data/remote/interceptor/AuthInterceptor.kt (2)
10-18: 문서 주석이 현재 구현과 일치하지 않습니다.이제
UserMemoryCache를 사용하지 않고 DataStore 기반으로 변경되었는데, 주석에서는 여전히 "메모리 캐시 (UserMemoryCache)"를 언급하고 있어요. 혼란을 방지하기 위해 문서를 업데이트해주시면 좋겠습니다.📝 문서 수정 제안
/** * 모든 API 요청에 인증 헤더를 자동으로 추가하는 Interceptor * - * 토큰 조회 우선순위: - * 1. 메모리 캐시 (UserMemoryCache) - 빠름 - * 2. 디스크 (DataStore via LocalUserRepository) - 메모리 캐시가 비어있을 때 fallback - * - * 앱 시작 직후 메모리 캐시가 비어있는 경우에도 디스크에서 토큰을 가져와 정상 동작 + * DataStore (via LocalUserRepository)에서 토큰을 조회하여 헤더에 추가합니다. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/swyp/souzip/data/remote/interceptor/AuthInterceptor.kt` around lines 10 - 18, Update the KDoc on AuthInterceptor to reflect the current implementation: remove references to UserMemoryCache and describe that tokens are retrieved from DataStore via LocalUserRepository (or other disk-backed source) as the primary/fallback mechanism; mention that AuthInterceptor will fetch tokens from LocalUserRepository/DataStore on startup when no in-memory cache exists and add any relevant behavior (e.g., synchronous vs. suspend call) so the comment matches the code paths in AuthInterceptor, LocalUserRepository and DataStore.
40-58: DataStore 접근을 메모리 캐시로 전환 검토 추천현재 구현은 기능적으로 정상 동작하며,
runBlocking사용도 OkHttp Interceptor의 자체 스레드 풀에서 실행되므로 문제없습니다. 다만 매 API 요청마다 DataStore에 접근하는 패턴은 다음 개선을 권장합니다:DataStore는 첫 읽기 후 메모리에 캐시하지만, 반복적인 접근은 여전히 Flow 변환/코루틴 스케줄링 비용이 누적됩니다. 특히 API 호출이 빈번한 경우, 앱 시작 시 DataStore에서 1회만 로드한 뒤
StateFlow같은 메모리 캐시로 관리하고 Interceptor는 메모리 값만 조회하는 구조로 변경하면 지연을 줄일 수 있습니다. 토큰 변경(갱신/로그아웃) 시점에만 DataStore를 업데이트하는 방식이 Interceptor 최적화 패턴입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/swyp/souzip/data/remote/interceptor/AuthInterceptor.kt` around lines 40 - 58, Replace per-request DataStore reads inside AuthInterceptor's runBlocking by reading values from an in-memory cache (e.g., a StateFlow or simple volatile holder) that is initialized/updated from localUserRepository.get() once on app start or whenever tokens change; modify the code paths that currently call repository.getUserId() and repository.getAccessToken() inside runBlocking to instead read cachedUserId and cachedAccessToken, and ensure localUserRepository (or a token manager) updates those cached values on login, refresh, or logout so that X_USER_ID and AUTHORIZATION (BEARER) headers are populated from memory-only lookups in AuthInterceptor.data/src/main/java/com/swyp/souzip/data/local/repository/LocalUserRepositoryImpl.kt (1)
30-50: 예외를 조용히 무시하면 디버깅이 어려울 수 있습니다.
CancellationException을 올바르게 다시 던지는 것은 좋습니다. 하지만 다른 예외들을 조용히null로 처리하면, 실제 문제가 발생했을 때 원인 파악이 어려울 수 있어요. 최소한 에러 로그를 남기는 것을 권장드립니다.📝 로깅 추가 제안
override suspend fun getAccessToken(): String? = withContext(Dispatchers.IO) { try { userPreferenceDataSource.accessToken.firstOrNull() } catch (e: CancellationException) { throw e - } catch (_: Exception) { + } catch (e: Exception) { + Timber.w(e, "Failed to get access token from DataStore") null } }다른 getter 메서드들(
getUserId,getRefreshToken,getOAuthToken,getFcmDeviceToken)에도 동일하게 적용하시면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/swyp/souzip/data/local/repository/LocalUserRepositoryImpl.kt` around lines 30 - 50, The getters getUserId and getAccessToken (and similarly getRefreshToken, getOAuthToken, getFcmDeviceToken) in LocalUserRepositoryImpl are swallowing non-cancellation exceptions and returning null silently; update each with logging so exceptions are recorded before returning null. Inside the withContext(Dispatchers.IO) try/catch blocks around userPreferenceDataSource.*.firstOrNull(), catch CancellationException and rethrow as-is, but in the generic catch log the exception via your logger (e.g., processLogger or a class logger) with a clear message like "Failed to read <tokenName>/<userId> from preferences" and then return null; apply the same pattern to userPreferenceDataSource.userId, .accessToken, .refreshToken, .oauthToken, and .fcmDeviceToken to ensure errors are visible while preserving behavior.presentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CategoryFilterUiModel.kt (1)
9-13:code와categoryType이중 소스는 불일치 위험이 있습니다.두 필드가 서로 다른 값을 가지면 필터 선택/비교 로직에서 미묘한 버그가 생길 수 있어, 한쪽을 단일 진실 공급원으로 두는 쪽을 권장드립니다.
제안 코드
`@Parcelize` data class CategoryFilterUiModel( - val code: String, // "FOOD_SNACK", "BEAUTY_HEALTH" 등 (CategoryType과 매핑) val displayName: String, // "하자.간식", "뷰티.헬스" 등 val categoryType: CategoryType, // Domain의 CategoryType -) : Parcelable +) : Parcelable { + val code: String get() = categoryType.name +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CategoryFilterUiModel.kt` around lines 9 - 13, CategoryFilterUiModel currently stores both code and categoryType which can diverge; make one the single source of truth (prefer keeping categoryType) and derive code from it rather than storing both: remove the val code property from CategoryFilterUiModel and provide a computed/derived code accessor (e.g., a getCode() or val code get() = categoryType.code) or add a factory that maps from code to CategoryType when constructing the model; update all call sites that construct or serialize/deserialize CategoryFilterUiModel (and Parcelable implementation) to use the chosen single source (CategoryFilterUiModel.categoryType) and derive the code when needed so there is no duplicated state.domain/src/main/java/com/swyp/souzip/domain/model/Souvenir.kt (1)
43-48:PriceInfo = Pair<Int, String>는 필드 의미가 약해 실수 여지가 큽니다.
first/second사용은 금액/통화 순서 실수를 만들기 쉬워서, 명시적 타입으로 바꾸는 쪽이 유지보수에 안전합니다.변경 제안
-typealias PriceInfo = Pair<Int, String> // amount, symbol +data class PriceInfo( + val amount: Int, + val symbol: String, +) data class SouvenirPriceInfoNew( val original: PriceInfo, // 사용자가 입력한 원본 가격 val converted: PriceInfo, // 등록 시점 환율로 변환된 가격 )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@domain/src/main/java/com/swyp/souzip/domain/model/Souvenir.kt` around lines 43 - 48, Replace the ambiguous typealias PriceInfo with a descriptive data class (e.g., Money or AmountCurrency) that exposes named properties like amount: Int and currency: String, and update SouvenirPriceInfoNew to use that new class for both original and converted fields; change all usages of PriceInfo/first/second to the new property names (amount, currency) to avoid order-related mistakes and improve readability (refer to PriceInfo, SouvenirPriceInfoNew, original, converted).data/src/main/java/com/swyp/souzip/data/remote/dto/response/RecommendationResponse.kt (1)
6-6: 불필요한 import 제거를 권장드립니다.
kotlin.collections.map은 Kotlin 표준 라이브러리에 포함되어 있어 명시적 import가 필요하지 않습니다. IDE에서 자동 정리하시면 됩니다.♻️ 제안하는 수정
-import kotlin.collections.map🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/com/swyp/souzip/data/remote/dto/response/RecommendationResponse.kt` at line 6, Remove the unnecessary explicit import "import kotlin.collections.map" from RecommendationResponse.kt — it's part of the Kotlin stdlib and not needed; simply delete that import line (or run the IDE's "Optimize Imports"/"Organize Imports" action) so the file compiles with no redundant imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/src/main/java/com/swyp/souzip/data/remote/api/SouvenirApi.kt`:
- Around line 47-51: The updateSouvenir API declaration is missing the image
upload parameter, so modify the suspend function updateSouvenir to support
multipart uploads by annotating the method with `@Multipart` and adding a `@Part`
parameter for the image files (e.g., files: List<MultipartBody.Part>) while
keeping the existing `@Part`("souvenir") souvenir: RequestBody and return type
ApiResponse<SouvenirDetailResponse>; ensure the Retrofit annotations remain
`@PUT`("/api/v2/souvenirs/{id}") and `@Path`("id") id: Int and use the exact symbol
names updateSouvenir, souvenir, and files when updating the signature.
In `@data/src/main/java/com/swyp/souzip/data/remote/di/RetrofitModule.kt`:
- Around line 115-117: Add header redaction to the logging interceptor and
remove debug-only Timber logs in the auth interceptor: in RetrofitModule's
providesLoggingInterceptor() (or wherever HttpLoggingInterceptor is created)
call redactHeader("Authorization") before setting level to BODY so the Bearer
token is not logged, and ensure the loggingInterceptor variable used in the
OkHttpClient builder includes that change; in AuthInterceptor.intercept(Chain)
remove any Timber.d() debug logs that print request URL, method, response code,
or token values and only add headers (X_USER_ID and Authorization with "$BEARER
$accessToken") to the request builder, preserving S3 bypass logic and
runBlocking repository access as currently implemented.
In `@domain/src/main/java/com/swyp/souzip/domain/model/Souvenir.kt`:
- Around line 101-102: The Souvenir data class currently allows price:Int? and
currency:String? to be set independently, permitting invalid partial states;
introduce a single value type (e.g., data class Money(amount: Int, currency:
String)) and replace the two nullable fields with one nullable Money (val price:
Money?) in Souvenir (or make Money non-nullable if price must always exist).
Update all constructors, serializers/deserializers, mappers, DB/entity
conversion code and any references to Souvenir.price and Souvenir.currency to
use the new Money type so partial states cannot be represented.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/common/util/DecimalFormat.kt`:
- Around line 5-12: The shared DecimalFormat instance decimalFormatter is not
thread-safe; replace it so each call gets its own formatter (e.g., add a private
createFormatter() that returns a new DecimalFormat("#,##0") and update
Int.toDecimalFormat, Long.toDecimalFormat and Number.toDecimalFormat to call
createFormatter().format(this)) or, if you prefer to avoid allocation while
keeping thread-safety, wrap DecimalFormat in a ThreadLocal and have each
extension use threadLocal.get().format(this); update references to
decimalFormatter accordingly.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CountryRankingUiModel.kt`:
- Around line 30-35: The when-block mapping index to CountryRank incorrectly
defaults 4th+ ranks to CountryRank.GOLD; update the mapping in
CountryRankingUiModel (the when(index) block) so indices >=3 are mapped to an
appropriate non-GOLD rank (e.g., a TOP_TEN/OTHER/UNRANKED enum value) or use
range branches (e.g., 3..9 -> CountryRank.TOP_TEN, else -> CountryRank.UNRANKED)
instead of else -> CountryRank.GOLD so ranks beyond third are represented
correctly.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/DiscoveryTopByCountryUiModel.kt`:
- Around line 9-26: The mapping passes countryNameKr from
DiscoveryTopByCountry.toUiModel into souvenirs.map { it.toUiModel(countryNameKr)
} but DiscoverySouvenir.toUiModel and DiscoverySouvenirUiModel don’t use that
value; either add a countryNameKr field to DiscoverySouvenirUiModel and assign
it in DiscoverySouvenir.toUiModel (update DiscoverySouvenirUiModel and the
internal fun DiscoverySouvenir.toUiModel(countryNameKr: String) to set
countryNameKr), or remove the unused parameter by changing
DiscoveryTopByCountry.toUiModel to call souvenirs.map { it.toUiModel() } and
update the signature of internal fun DiscoverySouvenir.toUiModel() accordingly
(and remove any references to countryNameKr in that function).
In
`@presentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/LocationState.kt`:
- Around line 14-15: hasLocation currently treats any 0.0 coordinate as "no
location", which incorrectly excludes valid points like latitude=0.0; fix by
making the coordinate properties nullable and base presence on null-checks:
change latitude and longitude to Double? and update the hasLocation getter in
LocationState to "get() = latitude != null && longitude != null" (or, if you
must keep non-null Doubles, change the logic to a separate flag like
hasCoordinates or use Optional/Boxed types and check that flag); update usages
of latitude/longitude accordingly to handle the new nullable types.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/upload/navigation/EditModeData.kt`:
- Around line 39-41: The price mapping in EditModeData is only reading from
priceStateOld so posts that only have priceStateNew end up with null prices;
update the assignments for localPrice, krwPrice, and currencySymbol to fall back
to priceStateNew when priceStateOld is null (e.g., use priceStateOld ?:
priceStateNew for each), referencing the properties localPrice, krwPrice,
currencySymbol and the source objects priceStateOld and priceStateNew in
EditModeData.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/upload/UploadViewModel.kt`:
- Around line 149-160: When building SouvenirParam, ensure currency is never
null when a price is provided: in the SouvenirParam construction (symbols:
currencyCode, state.isPriceUnknown, state.isKrw, SouvenirParam) change the
currency expression so that if state.isPriceUnknown is false you either set
"KRW" when state.isKrw is true or require a non-null currencyCode (e.g.,
fail/return early or throw) instead of passing null; also update the subsequent
validation block (the check around lines 175-179) to validate currency presence
whenever a price is supplied so the request cannot proceed with price but
missing currency.
---
Outside diff comments:
In
`@data/src/main/java/com/swyp/souzip/data/remote/repository/DiscoveryRepositoryImpl.kt`:
- Around line 21-55: The catch-all Throwable handlers in
getDiscoveryTopByCountry, getDiscoveryByCategory, and getMonthlyCountryRanking
are swallowing CancellationException; update each Throwable catch to rethrow if
the caught Throwable is a CancellationException (or call throw when e is
CancellationException) so coroutine cancellations propagate, otherwise wrap into
SouZipException.UnknownError as before; reference the methods
getDiscoveryTopByCountry, getDiscoveryByCategory, and getMonthlyCountryRanking
and ensure CancellationException from kotlinx.coroutines is checked and rethrown
before creating SouZipResult.Failure.
In
`@domain/src/main/java/com/swyp/souzip/domain/repository/DiscoveryRepository.kt`:
- Around line 10-14: The KDoc for getDiscoveryTopByCountry() mentions a `@param`
countryCode but the function has no parameters; either remove the `@param` line
from the KDoc or add the countryCode parameter to the function signature and
update its return/behavior accordingly; locate the KDoc block above the suspend
fun getDiscoveryTopByCountry() declaration and update either the comment or the
function signature to keep them consistent (e.g., change to suspend fun
getDiscoveryTopByCountry(countryCode: String):
SouZipResult<List<DiscoveryTopByCountry>> if the parameter is required, or
delete the `@param` countryCode tag if not).
In
`@presentation/src/main/java/com/swyp/souzip/presentation/discovery/DiscoveryViewModel.kt`:
- Around line 33-54: The intent blocks (e.g., loadInitialData, handleRefresh,
handleCategoryFilterSelected) currently call loader helpers that themselves
start new intent { } contexts (loadTopSouvenirsByCountry,
loadTrendingSouvenirsByCategory, loadCountryRanking), causing
isLoading/isRefreshing to be cleared prematurely; refactor those loader helpers
into plain suspend functions (remove their internal intent { } usage) and call
them directly from the surrounding intent { } so their work runs sequentially
inside the same Orbit intent context, keeping loading flags true until the
suspend calls complete (update function signatures for
loadTopSouvenirsByCountry, loadTrendingSouvenirsByCategory, loadCountryRanking
and change their callers accordingly).
---
Nitpick comments:
In
`@data/src/main/java/com/swyp/souzip/data/local/repository/LocalUserRepositoryImpl.kt`:
- Around line 30-50: The getters getUserId and getAccessToken (and similarly
getRefreshToken, getOAuthToken, getFcmDeviceToken) in LocalUserRepositoryImpl
are swallowing non-cancellation exceptions and returning null silently; update
each with logging so exceptions are recorded before returning null. Inside the
withContext(Dispatchers.IO) try/catch blocks around
userPreferenceDataSource.*.firstOrNull(), catch CancellationException and
rethrow as-is, but in the generic catch log the exception via your logger (e.g.,
processLogger or a class logger) with a clear message like "Failed to read
<tokenName>/<userId> from preferences" and then return null; apply the same
pattern to userPreferenceDataSource.userId, .accessToken, .refreshToken,
.oauthToken, and .fcmDeviceToken to ensure errors are visible while preserving
behavior.
In
`@data/src/main/java/com/swyp/souzip/data/remote/dto/response/RecommendationResponse.kt`:
- Line 6: Remove the unnecessary explicit import "import kotlin.collections.map"
from RecommendationResponse.kt — it's part of the Kotlin stdlib and not needed;
simply delete that import line (or run the IDE's "Optimize Imports"/"Organize
Imports" action) so the file compiles with no redundant imports.
In
`@data/src/main/java/com/swyp/souzip/data/remote/interceptor/AuthInterceptor.kt`:
- Around line 10-18: Update the KDoc on AuthInterceptor to reflect the current
implementation: remove references to UserMemoryCache and describe that tokens
are retrieved from DataStore via LocalUserRepository (or other disk-backed
source) as the primary/fallback mechanism; mention that AuthInterceptor will
fetch tokens from LocalUserRepository/DataStore on startup when no in-memory
cache exists and add any relevant behavior (e.g., synchronous vs. suspend call)
so the comment matches the code paths in AuthInterceptor, LocalUserRepository
and DataStore.
- Around line 40-58: Replace per-request DataStore reads inside
AuthInterceptor's runBlocking by reading values from an in-memory cache (e.g., a
StateFlow or simple volatile holder) that is initialized/updated from
localUserRepository.get() once on app start or whenever tokens change; modify
the code paths that currently call repository.getUserId() and
repository.getAccessToken() inside runBlocking to instead read cachedUserId and
cachedAccessToken, and ensure localUserRepository (or a token manager) updates
those cached values on login, refresh, or logout so that X_USER_ID and
AUTHORIZATION (BEARER) headers are populated from memory-only lookups in
AuthInterceptor.
In
`@data/src/main/java/com/swyp/souzip/data/remote/interceptor/TokenAuthenticator.kt`:
- Around line 26-27: Update the documentation comment in TokenAuthenticator.kt
to remove the obsolete "memory cache" reference: change item 6 to state that on
success the local storage (e.g., SharedPreferences or whatever persistent store
is used) is updated and the request is retried with the new access token, and
ensure item 7 remains unchanged; locate the class TokenAuthenticator and the
authenticate/handleTokenRefresh comment block to make this text-only edit.
In `@domain/src/main/java/com/swyp/souzip/domain/model/Souvenir.kt`:
- Around line 43-48: Replace the ambiguous typealias PriceInfo with a
descriptive data class (e.g., Money or AmountCurrency) that exposes named
properties like amount: Int and currency: String, and update
SouvenirPriceInfoNew to use that new class for both original and converted
fields; change all usages of PriceInfo/first/second to the new property names
(amount, currency) to avoid order-related mistakes and improve readability
(refer to PriceInfo, SouvenirPriceInfoNew, original, converted).
In
`@domain/src/main/java/com/swyp/souzip/domain/repository/LocalUserRepository.kt`:
- Around line 19-21: setGuestUser()의 KDoc에서 더이상 사용하지 않는 "메모리 캐시의 토큰을 초기화합니다" 문구를
삭제하고, 현재 동작을 정확히 설명하도록 주석을 갱신하세요; 예를 들어 "현재 사용자를 게스트로 설정합니다"와 (필요하면) "영구 저장소(또는
관련 토큰 저장소)에서 토큰을 제거합니다"처럼 실제 구현과 일치하는 한 줄로 수정해 주세요. 참고 심볼: setGuestUser().
In `@gradle/libs.versions.toml`:
- Line 250: The leakcanary dependency entry named "leakcanary-android"
(version.ref = "leakcanaryAndroid") is placed under the # Test section but
belongs in the # Debug section; move the line `leakcanary-android = { module =
"com.squareup.leakcanary:leakcanary-android", version.ref = "leakcanaryAndroid"
}` out of the Test block and insert it under the # Debug block so the dependency
classification matches its runtime use.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CategoryFilterUiModel.kt`:
- Around line 9-13: CategoryFilterUiModel currently stores both code and
categoryType which can diverge; make one the single source of truth (prefer
keeping categoryType) and derive code from it rather than storing both: remove
the val code property from CategoryFilterUiModel and provide a computed/derived
code accessor (e.g., a getCode() or val code get() = categoryType.code) or add a
factory that maps from code to CategoryType when constructing the model; update
all call sites that construct or serialize/deserialize CategoryFilterUiModel
(and Parcelable implementation) to use the chosen single source
(CategoryFilterUiModel.categoryType) and derive the code when needed so there is
no duplicated state.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/LocationState.kt`:
- Line 8: LocationState의 countryCode가 기본값으로 "US"로 하드코딩되어 있어 위치 미선택 시에도
downstream에 잘못 전달될 수 있으니 countryCode를 빈 문자열 또는 nullable(String?)로 초기화하고 실제 사용자는
위치 선택 시에만 값을 할당하도록 변경하세요; 참조 대상: data class LocationState 및 해당 프로퍼티 countryCode를
수정해 초기값 제거(혹은 null 허용)하고, 위치 선택 로직(setter/함수)에서만 값을 설정하도록 관련 호출부를 검토/수정하세요.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
app/build.gradle.ktsdata/src/main/java/com/swyp/souzip/data/cache/UserMemoryCache.ktdata/src/main/java/com/swyp/souzip/data/local/datasource/UserPreferenceDataSource.ktdata/src/main/java/com/swyp/souzip/data/local/repository/LocalCountryRepositoryImpl.ktdata/src/main/java/com/swyp/souzip/data/local/repository/LocalUserRepositoryImpl.ktdata/src/main/java/com/swyp/souzip/data/remote/api/DiscoveryApi.ktdata/src/main/java/com/swyp/souzip/data/remote/api/RecommendationApi.ktdata/src/main/java/com/swyp/souzip/data/remote/api/SouvenirApi.ktdata/src/main/java/com/swyp/souzip/data/remote/datasource/DiscoveryDataSource.ktdata/src/main/java/com/swyp/souzip/data/remote/datasource/RecommendationDataSource.ktdata/src/main/java/com/swyp/souzip/data/remote/datasource/SouvenirRemoteDataSource.ktdata/src/main/java/com/swyp/souzip/data/remote/di/RetrofitModule.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/request/SouvenirRequest.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/CountryRankingResponse.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/DiscoveryResponse.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/DiscoverySouvenirDto.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/DiscoveryTopByCategoryResponse.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/DiscoveryTopByCountryResponse.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/RecommendationResponse.ktdata/src/main/java/com/swyp/souzip/data/remote/dto/response/SouvenirDetailResponse.ktdata/src/main/java/com/swyp/souzip/data/remote/interceptor/AuthInterceptor.ktdata/src/main/java/com/swyp/souzip/data/remote/interceptor/TokenAuthenticator.ktdata/src/main/java/com/swyp/souzip/data/remote/repository/DiscoveryRepositoryImpl.ktdata/src/main/java/com/swyp/souzip/data/remote/repository/SouvenirRepositoryImpl.ktdomain/src/main/java/com/swyp/souzip/domain/model/DiscoveryTopByCountry.ktdomain/src/main/java/com/swyp/souzip/domain/model/Souvenir.ktdomain/src/main/java/com/swyp/souzip/domain/repository/DiscoveryRepository.ktdomain/src/main/java/com/swyp/souzip/domain/repository/LocalCountryRepository.ktdomain/src/main/java/com/swyp/souzip/domain/repository/LocalUserRepository.ktgradle/libs.versions.tomlpresentation/build.gradle.ktspresentation/src/main/java/com/swyp/souzip/presentation/common/design/DiscoverySouvenirGrid.ktpresentation/src/main/java/com/swyp/souzip/presentation/common/design/PriceRow.ktpresentation/src/main/java/com/swyp/souzip/presentation/common/design/SouvenirSummaryItem.ktpresentation/src/main/java/com/swyp/souzip/presentation/common/model/DiscoverySouvenirUiModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/common/model/PriceState.ktpresentation/src/main/java/com/swyp/souzip/presentation/common/model/PriceUiState.ktpresentation/src/main/java/com/swyp/souzip/presentation/common/util/DecimalFormat.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/DiscoveryViewModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/component/DiscoverySouvenirRow.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/component/TopSouvenirsByCountrySection.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/component/TrendingSouvenirsByCategorySection.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CategoryFilterUiModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CountryRankingUiModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/DiscoveryTopByCountryUiModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/DiscoveryUiState.ktpresentation/src/main/java/com/swyp/souzip/presentation/discovery/contract/PreviewData.ktpresentation/src/main/java/com/swyp/souzip/presentation/map/component/SouvenirMarker.ktpresentation/src/main/java/com/swyp/souzip/presentation/map/component/bottombanner/SouvenirInfoCard.ktpresentation/src/main/java/com/swyp/souzip/presentation/map/component/bottomsheet/MapboxBottomSheetContent.ktpresentation/src/main/java/com/swyp/souzip/presentation/map/contract/MapboxUiState.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/PostDetailScreen.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/PostDetailScreenContent.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/PostDetailViewModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/LocationState.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/PostDetailSideEffect.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/PostDetailUiState.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/SouvenirUiState.ktpresentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/UploaderState.ktpresentation/src/main/java/com/swyp/souzip/presentation/recommendation/RecommendationScreenContent.ktpresentation/src/main/java/com/swyp/souzip/presentation/recommendation/RecommendationViewModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/recommendation/component/PreferCategorySection.ktpresentation/src/main/java/com/swyp/souzip/presentation/recommendation/component/SimilarRecommendationSection.ktpresentation/src/main/java/com/swyp/souzip/presentation/recommendation/contract/RecommendationUiState.ktpresentation/src/main/java/com/swyp/souzip/presentation/upload/UploadViewModel.ktpresentation/src/main/java/com/swyp/souzip/presentation/upload/navigation/EditModeData.kt
💤 Files with no reviewable changes (3)
- data/src/main/java/com/swyp/souzip/data/remote/dto/response/DiscoveryResponse.kt
- data/src/main/java/com/swyp/souzip/data/cache/UserMemoryCache.kt
- presentation/src/main/java/com/swyp/souzip/presentation/common/model/PriceState.kt
Comment on lines
+115
to
+117
| addInterceptor(authInterceptor) | ||
| // if (DEBUG) addInterceptor(loggingInterceptor) | ||
| if (BuildConfig.DEBUG) addInterceptor(prettyLoggingInterceptor) | ||
| if (DEBUG) addInterceptor(loggingInterceptor) | ||
| // if (BuildConfig.DEBUG) addInterceptor(prettyLoggingInterceptor) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 파일 전체 확인
cat -n data/src/main/java/com/swyp/souzip/data/remote/di/RetrofitModule.ktRepository: souzip/souzip-android
Length of output: 6812
🏁 Script executed:
#!/bin/bash
# 1) PrettyLoggingInterceptor 찾기
fd 'PrettyLoggingInterceptor\.kt$' -t f
# 2) AuthInterceptor 찾기
fd 'AuthInterceptor\.kt$' -t fRepository: souzip/souzip-android
Length of output: 229
🏁 Script executed:
#!/bin/bash
# PrettyLoggingInterceptor 확인
echo "=== PrettyLoggingInterceptor.kt ==="
cat -n data/src/main/java/com/swyp/souzip/data/remote/interceptor/PrettyLoggingInterceptor.kt
echo ""
echo "=== AuthInterceptor.kt ==="
cat -n data/src/main/java/com/swyp/souzip/data/remote/interceptor/AuthInterceptor.ktRepository: souzip/souzip-android
Length of output: 7317
HttpLoggingInterceptor에 Authorization 헤더 마스킹을 추가하세요.
AuthInterceptor에서 Bearer 토큰을 Authorization 헤더에 추가하고 있는데, 현재 HttpLoggingInterceptor는 Level.BODY로 설정되어 모든 헤더와 응답 본문을 로그에 남기고 있습니다. 디버그 빌드라도 토큰이 로그 시스템에 남을 수 있어 보안 위험입니다.
또한 AuthInterceptor에 배포 시 불필요한 Timber.d() 디버그 로그가 남아 있습니다(요청 URL, Method, 응답 코드 등). 해당 로그들도 함께 제거하시기 바랍니다.
개선 예시
// RetrofitModule.kt - providesLoggingInterceptor()
`@Provides`
`@Singleton`
fun providesLoggingInterceptor(): HttpLoggingInterceptor =
HttpLoggingInterceptor().apply {
redactHeader("Authorization")
level = HttpLoggingInterceptor.Level.BODY
}// AuthInterceptor.kt - intercept() 메서드의 불필요한 로그 제거
override fun intercept(chain: Interceptor.Chain): Response {
val originalRequest = chain.request()
val requestUrl = originalRequest.url.encodedPath
if (isS3Request(originalRequest.url.host)) {
return chain.proceed(originalRequest)
}
val requestBuilder = originalRequest.newBuilder()
runBlocking {
val repository = localUserRepository.get()
val userId = repository.getUserId()
if (userId != null) {
requestBuilder.addHeader(X_USER_ID, userId)
}
val accessToken = repository.getAccessToken()
if (!accessToken.isNullOrBlank()) {
requestBuilder.addHeader(AUTHORIZATION, "$BEARER $accessToken")
}
}
val newRequest = requestBuilder.build()
return chain.proceed(newRequest)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@data/src/main/java/com/swyp/souzip/data/remote/di/RetrofitModule.kt` around
lines 115 - 117, Add header redaction to the logging interceptor and remove
debug-only Timber logs in the auth interceptor: in RetrofitModule's
providesLoggingInterceptor() (or wherever HttpLoggingInterceptor is created)
call redactHeader("Authorization") before setting level to BODY so the Bearer
token is not logged, and ensure the loggingInterceptor variable used in the
OkHttpClient builder includes that change; in AuthInterceptor.intercept(Chain)
remove any Timber.d() debug logs that print request URL, method, response code,
or token values and only add headers (X_USER_ID and Authorization with "$BEARER
$accessToken") to the request builder, preserving S3 bypass logic and
runBlocking repository access as currently implemented.
presentation/src/main/java/com/swyp/souzip/presentation/common/util/DecimalFormat.kt
Outdated
Show resolved
Hide resolved
...ation/src/main/java/com/swyp/souzip/presentation/discovery/contract/CountryRankingUiModel.kt
Show resolved
Hide resolved
...rc/main/java/com/swyp/souzip/presentation/discovery/contract/DiscoveryTopByCountryUiModel.kt
Show resolved
Hide resolved
presentation/src/main/java/com/swyp/souzip/presentation/postdetail/contract/LocationState.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+39
to
+41
| localPrice = this.priceStateOld?.localPrice, | ||
| krwPrice = this.priceStateOld?.krwPrice, | ||
| currencySymbol = this.priceStateOld?.currencySymbol, |
There was a problem hiding this comment.
신규 가격(priceStateNew)만 있는 게시물은 수정 모드에서 가격이 비어질 수 있습니다.
현재 매핑이 priceStateOld만 참조해서, 신 API 응답에서 priceStateOld == null인 경우 가격 정보가 유실됩니다. priceStateNew로의 fallback을 함께 두는 편이 안전합니다.
수정 제안
- localPrice = this.priceStateOld?.localPrice,
- krwPrice = this.priceStateOld?.krwPrice,
- currencySymbol = this.priceStateOld?.currencySymbol,
+ localPrice = this.priceStateOld?.localPrice ?: this.priceStateNew?.originalPrice,
+ krwPrice = this.priceStateOld?.krwPrice ?: this.priceStateNew?.convertedPrice,
+ currencySymbol = this.priceStateOld?.currencySymbol ?: this.priceStateNew?.originalSymbol,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| localPrice = this.priceStateOld?.localPrice, | |
| krwPrice = this.priceStateOld?.krwPrice, | |
| currencySymbol = this.priceStateOld?.currencySymbol, | |
| localPrice = this.priceStateOld?.localPrice ?: this.priceStateNew?.originalPrice, | |
| krwPrice = this.priceStateOld?.krwPrice ?: this.priceStateNew?.convertedPrice, | |
| currencySymbol = this.priceStateOld?.currencySymbol ?: this.priceStateNew?.originalSymbol, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/upload/navigation/EditModeData.kt`
around lines 39 - 41, The price mapping in EditModeData is only reading from
priceStateOld so posts that only have priceStateNew end up with null prices;
update the assignments for localPrice, krwPrice, and currencySymbol to fall back
to priceStateNew when priceStateOld is null (e.g., use priceStateOld ?:
priceStateNew for each), referencing the properties localPrice, krwPrice,
currencySymbol and the source objects priceStateOld and priceStateNew in
EditModeData.
Comment on lines
+149
to
+160
| val currencyCode = | ||
| state.countryCode?.let { | ||
| localCountryRepository.getCurrencyCode(it) | ||
| } | ||
| // SouvenirParam 생성 | ||
| val param = | ||
| SouvenirParam( | ||
| name = state.souvenirName, | ||
| localPrice = if (!state.isKrw && !state.isPriceUnknown) state.price.toIntOrNull() else null, | ||
| currencySymbol = | ||
| price = if (!state.isPriceUnknown) state.price.toIntOrNull() else null, | ||
| currency = | ||
| if (!state.isPriceUnknown) { | ||
| if (state.isKrw) "₩" else state.currencySymbol | ||
| if (state.isKrw) "KRW" else currencyCode |
There was a problem hiding this comment.
비KRW 저장 시 currency가 null로 전송될 수 있습니다
Line 149-160에서 currencyCode가 null일 때도 price만 유효하면 통과하고, Line 175-179 검증은 price만 확인해서 잘못된 요청이 내려갈 수 있습니다.
수정 예시
val currencyCode =
state.countryCode?.let {
localCountryRepository.getCurrencyCode(it)
}
+ val currency =
+ if (!state.isPriceUnknown) {
+ if (state.isKrw) "KRW" else currencyCode
+ } else {
+ null
+ }
+
+ if (!state.isPriceUnknown && state.price.toIntOrNull() == null) {
+ reduce { state.copy(isLoading = false) }
+ postSideEffect(UploadSideEffect.ShowSnackBar(context.getString(R.string.upload_price_invalid)))
+ return@intent
+ }
+
+ if (!state.isPriceUnknown && currency.isNullOrBlank()) {
+ reduce { state.copy(isLoading = false) }
+ postSideEffect(UploadSideEffect.ShowSnackBar(context.getString(R.string.upload_error)))
+ return@intent
+ }
val param =
SouvenirParam(
name = state.souvenirName,
price = if (!state.isPriceUnknown) state.price.toIntOrNull() else null,
- currency =
- if (!state.isPriceUnknown) {
- if (state.isKrw) "KRW" else currencyCode
- } else {
- null
- },
+ currency = currency,Also applies to: 175-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/com/swyp/souzip/presentation/upload/UploadViewModel.kt`
around lines 149 - 160, When building SouvenirParam, ensure currency is never
null when a price is provided: in the SouvenirParam construction (symbols:
currencyCode, state.isPriceUnknown, state.isKrw, SouvenirParam) change the
currency expression so that if state.isPriceUnknown is false you either set
"KRW" when state.isKrw is true or require a non-null currencyCode (e.g.,
fail/return early or throw) instead of passing null; also update the subsequent
validation block (the check around lines 175-179) to validate currency presence
whenever a price is supplied so the request cannot proceed with price but
missing currency.
CategoryType에서 불러옴
DiscoverySouvenirUiModel 사용처 정확히 명시
서버에서 항상 위치 정보 넘겨주므로, nullable이나 hasLocation 같은 확인 생략하도록 변경
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
1 participant
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.
Summary by CodeRabbit
릴리스 노트