아이템 수동 등록 사용성 개선 #63
아이템 수동 등록 사용성 개선 #63
Conversation
WalkthroughUpdated Android SDK to 35, bumped app version, added paging dependency, introduced paging infrastructure across repositories and services, refactored many DTOs/serial names, reworked auth interceptor with device-info handling and token refresh, split upload flows into Parsing/Manual with multi-image support, and adjusted numerous UI components and view models. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/MyScreen.kt (1)
256-274: Minor:isEmailMatchedis never set totrue, making the dismiss condition unreachable.When the email matches at line 260,
deleteAccount()is called butisEmailMatchedis never set totrue. This makes thedismissOnConfirmcondition at line 274 (isEmailMatched == true) unreachable for the Withdraw case.While this works in practice because
deleteAccount()navigates away from this screen, the logic would be cleaner ifisEmailMatchedwere set totruebefore callingdeleteAccount():DialogData.Withdraw -> { val isMatched = withdrawalEmailInput.value.isNotBlank() && withdrawalEmailInput.value.trimEnd() == uiModel.userInfo.email if (isMatched) { + isEmailMatched = true deleteAccount() } else { isEmailMatched = false } }app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/ProfileEditScreen.kt (1)
137-141: Consider removing the arbitrary focus delay.The 300ms delay before requesting focus seems arbitrary. While LaunchedEffect should cancel properly on screen exit, this pattern can be fragile. Consider requesting focus without delay or using a more deterministic trigger.
val focusRequester = remember { FocusRequester() } LaunchedEffect(Unit) { - delay(300L) focusRequester.requestFocus() }app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/SignViewModel.kt (1)
135-164: Potential stuck Loading state if FCM token fetch fails silently.If
initFCMTokenfails (line 141), the callback never executes, leavingcheckVerificationCodeStatusstuck inLoadingstate indefinitely. TheinitFCMTokenmethod only logs the error but doesn't notify the caller.Consider adding failure handling to
initFCMToken:- private fun initFCMToken(onSuccess: (String) -> Unit) { + private fun initFCMToken(onSuccess: (String) -> Unit, onFailure: () -> Unit = {}) { FirebaseMessaging.getInstance().token.addOnCompleteListener { task -> if (task.isSuccessful) { val fcmToken = task.result onSuccess(fcmToken) Timber.d(fcmToken) } else { Timber.e("Fetching FCM registration token failed", task.exception) + onFailure() } } }Then handle the failure in
signInEmail:initFCMToken( onSuccess = { fcmToken -> /* existing logic */ }, onFailure = { _uiModel.update { it.copy(checkVerificationCodeStatus = WishBoardState.Failure) } updateSnackbarMessage(SnackbarMessage.DEFAULT) } )
🧹 Nitpick comments (47)
app/src/main/java/com/hyeeyoung/wishboard/domain/model/wish/ParsedWishItem.kt (1)
8-13: Confirm JSON key rename impact and consider isolating serialization concerns.Changing
@SerialNamevalues to"itemImageUrl","itemName", and"itemPrice"will alter the JSON contract for this type. If any existing backend responses, stored JSON (e.g., cache, DB), or other services still use the old keys, these fields will now deserialize asnull(even though the object itself won’t fail due to defaults).Please verify:
- All producers/consumers of
ParsedWishItemJSON have been updated to use the new keys.- There is no persisted data using the old keys that still needs to be read, or that you have a migration/compat strategy if there is.
Separately, if
ParsedWishItemis meant to be a pure domain model, you might consider moving@SerialNameto a DTO in the data layer and mapping into this type, to keep serialization details out of the domain. This is optional but can improve layering clarity.app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/component/ShopLinkModalContent.kt (1)
56-56: LGTM! Button label improved.The label change from "Complete" to "Add" better reflects the action of adding a shop link. This aligns with the PR's goal of improving wording across upload modals.
Optional future improvement: Consider renaming
onClickCompletetoonClickAddoronAddLinkfor consistency with the button label. This would require updating parent components, so it's outside the current scope.app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignUpPasswordScreen.kt (1)
6-12: Scrollable, keyboard-safe layout change looks goodUsing
fillMaxSize() + verticalScroll(rememberScrollState()) + imePadding()is a solid pattern here and should improve usability on small screens and with the keyboard open. The combination withpadding(top = paddingValues.calculateTopPadding(), bottom = 16.dp, …)keeps the top inset from theScaffoldwhile giving consistent side/bottom spacing.If you want to micro‑tune behavior later, an optional tweak is to pull
rememberScrollState()into avalat the top of the composable for readability and reuse, but functionally this is already correct and consistent with other sign screens.Also applies to: 109-116
app/src/main/java/com/hyeeyoung/wishboard/presentation/calendar/CalendarViewModel.kt (1)
28-42: Consider simplifying the refresh mechanism.The current flow routes through the Screen unnecessarily: EventBus → trigger → Screen →
fetchSchedule(). Since the Screen's only action is callingfetchSchedule(), you can invoke it directly inrefreshSchedule()and eliminate the trigger indirection.- private val _refreshScheduleTrigger = Channel<Unit>() - val refreshScheduleTrigger = _refreshScheduleTrigger.receiveAsFlow() - init { fetchSchedule() refreshSchedule() } private fun refreshSchedule() { viewModelScope.launch { WishBoardEventBus.onWishItemChanged.collect { - _refreshScheduleTrigger.send(Unit) + fetchSchedule() } } }This removes the need for
CalendarScreento subscribe to the trigger and call back to the ViewModel.app/src/main/java/com/hyeeyoung/wishboard/presentation/onboarding/OnboardingScreen.kt (1)
68-81: Consider decoupling WishBoardIndicator from PagerState and clarifying parameter naming
WishBoardIndicatoris functionally correct, but you might make it more reusable by:
- Accepting
currentPage: Int(and optionallypageCount: Int) instead of aPagerState, so the indicator isn’t tied to a specific pager implementation.- Renaming
sizeto something likepageCountfor clearer intent at the call site.These are optional design tweaks; the current implementation works as-is.
app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/MyScreen.kt (1)
340-361: Consider renamingisIncorrectEmailfor clarity.The parameter
isIncorrectEmailreceives the value ofisEmailMatched, which makes the naming counterintuitive—when the email is incorrect,isIncorrectEmailisfalse(becauseisEmailMatchedisfalse). The logic works correctly, butisEmailMatchedoremailValidationResultwould be clearer parameter names.app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/wish/WishItemIdDto.kt (1)
6-6: TODO comment flagged.The TODO comment indicates that this data class structure needs to be changed. Consider tracking this as a separate issue if it's not already being addressed.
Would you like me to open an issue to track this technical debt?
app/src/main/java/com/hyeeyoung/wishboard/presentation/main/MainViewModel.kt (1)
58-72: Consider usingcollectinstead ofcollectLatestor taking only the first match.The
collectLatestcall will continue collecting indefinitely. After resettingautoLoginExpiryInfoto(false, "")on line 68, the flow will immediately emit again, causing the lambda to re-execute (though theifguard will prevent action). This keeps a coroutine running continuously.Consider one of these alternatives:
Option 1: Use
firstto handle only the first matching event:fun checkAuthLoginState(moveToStartRoute: () -> Unit) { viewModelScope.launch { - GlobalState.autoLoginExpiryInfo.collectLatest { autoLoginInfo -> - if (autoLoginInfo.first) { - moveToStartRoute() - sendSnackbarChannel( - WishBoardSnackbarVisuals( - message = autoLoginInfo.second, - ), - ) - GlobalState.autoLoginExpiryInfo.value = false to "" - } - } + GlobalState.autoLoginExpiryInfo + .first { it.first } + .let { autoLoginInfo -> + moveToStartRoute() + sendSnackbarChannel( + WishBoardSnackbarVisuals( + message = autoLoginInfo.second, + ), + ) + GlobalState.autoLoginExpiryInfo.value = false to "" + } } }Option 2: Use
collectif continuous monitoring is intended:fun checkAuthLoginState(moveToStartRoute: () -> Unit) { viewModelScope.launch { - GlobalState.autoLoginExpiryInfo.collectLatest { autoLoginInfo -> + GlobalState.autoLoginExpiryInfo.collect { autoLoginInfo -> if (autoLoginInfo.first) { moveToStartRoute() sendSnackbarChannel( WishBoardSnackbarVisuals( message = autoLoginInfo.second, ), ) GlobalState.autoLoginExpiryInfo.value = false to "" } } } }app/src/main/java/com/hyeeyoung/wishboard/designsystem/util/PriceVisualTransformation.kt (1)
9-56: LGTM: Complex offset mapping logic appears correct.The
PriceVisualTransformationimplementation correctly formats numeric input as KRW currency with thousand separators. TheOffsetMappinglogic for cursor position translation is complex but appears sound.Consider adding unit tests to verify edge cases:
- Empty input
- Single digit: "5" → "₩ 5"
- Around comma boundaries: "999" → "₩ 999", "1000" → "₩ 1,000"
- Large numbers: "1234567890" → "₩ 1,234,567,890"
- Cursor positions at comma boundaries
If tests don't already exist, I can help generate them.
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/topbar/WishBoardMainTopBar.kt (1)
18-18: Legacy icon button usage in preview looks fine; consider a11y if reusedSwitching the preview’s endComponent to
LegacyWishBoardIconButtonmatches the new design-system API and should behave the same as before. If this pattern is ever reused in production UI (not just preview), consider wiring acontentDescriptionfor TalkBack users.Also applies to: 51-53
app/src/main/java/com/hyeeyoung/wishboard/presentation/cart/CartScreen.kt (1)
38-39: LegacyWishBoardIconButton swap is fine; consider content descriptions for iconsUsing
LegacyWishBoardIconButtonfor delete and count +/- actions keeps behavior and click handling unchanged. To improve accessibility, especially in these non-decorative controls, consider passing meaningfulcontentDescriptionvalues (e.g., “삭제”, “수량 감소”, “수량 증가”) so TalkBack users can understand and operate these buttons.Also applies to: 150-156, 178-181
app/src/main/java/com/hyeeyoung/wishboard/data/util/extension/StringExt.kt (1)
3-8: Time conversion logic is reasonable; consider logging and layering tweaksThe
toInstantToLocalDateTimeimplementation correctly treats the string as aLocalDateTimeinTimeUtil.koreaTimeZoneand converts it to the system default zone, which fits the “Korea server time → device local time” use case.A couple of refinements to consider:
Logging:
Timber.e("$e")loses the stack trace context. Prefer passing the throwable:} catch (e: Exception) { Timber.e(e, "Failed to parse LocalDateTime from '$this'") Clock.System.now().toLocalDateTime(TimeZone.currentSystemDefault()) }This makes diagnosing malformed date strings much easier.
Fallback strategy: Returning
Clock.System.now()on parse failure is user‑friendly but can hide data issues (the UI shows “now” even when the server value is invalid or unexpected). If distinguishing “invalid/unknown date” is important, consider a nullable return type or a sentinel value at the call site instead.Layering:
StringExtunderdata.utilnow depends oncom.hyeeyoung.wishboard.presentation.util.TimeUtil. If you want to keep a clean data→presentation separation, it might be worth movingTimeUtil(or just thekoreaTimeZonedefinition) into a shared core/util package used by both layers.Also applies to: 12-13, 20-27
app/src/main/java/com/hyeeyoung/wishboard/config/GlobalState.kt (1)
5-9: Consider replacing Pair<Boolean, String> with a dedicated data classUsing
Pair<Boolean, String>forautoLoginExpiryInfoworks but is not very self‑documenting (what theStringrepresents, etc.), and there’s already a TODO about changing the data type. A small data class like:data class AutoLoginExpiryInfo( val isExpired: Boolean, val message: String, )would improve readability and reduce misuse at call sites.
app/src/main/java/com/hyeeyoung/wishboard/domain/util/SafeEnumUtil.kt (1)
1-13: safeValueOf implementation is fine; clarify intent / detekt behaviorThis is a reasonable implementation of a “safe” enum lookup: it explicitly treats invalid names as
null. The swallowedIllegalArgumentExceptionis intentional here, so the detekt warning can be silenced for this function or replaced with arunCatching { enumValueOf<T>(type) }.getOrNull()variant if you prefer:inline fun <reified T : Enum<T>> safeValueOf(type: String?): T? = type?.let { runCatching { enumValueOf<T>(it) }.getOrNull() }Either way, a short KDoc note that “invalid or null input returns null” will make the behavior clear to callers and future reviewers.
app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderDetailViewModel.kt (1)
34-44: Consider usingSharingStarted.WhileSubscribed()instead ofEagerly.The current implementation uses
SharingStarted.Eagerly, which keeps the flow active even when there are no collectors. For better resource management, consider usingSharingStarted.WhileSubscribed()to start collection only when the UI is observing.Apply this diff if appropriate for your use case:
.cachedIn(viewModelScope) - .stateIn(viewModelScope, SharingStarted.Eagerly, PagingData.empty()) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), PagingData.empty())Note: Only apply this if the eager start behavior is not required for your specific use case.
app/src/main/java/com/hyeeyoung/wishboard/domain/model/noti/NotiType.kt (1)
16-25: Consider safer error handling for invalid labels.The
fromLabelmethod throwsIllegalArgumentExceptionfor invalid inputs, which could crash the app if unexpected data is received from an API or user input. Consider returning a default value or using a Result type for safer error handling.Option 1: Return a default value:
-fun fromLabel(label: String) = +fun fromLabel(label: String): NotiType = when (label) { RESTOCK.label -> RESTOCK OPEN.label -> OPEN PREORDER.label -> PREORDER SALE_END.label -> SALE_END SALE_START.label -> SALE_START REMINDER.label -> REMINDER - else -> throw IllegalArgumentException("유효하지 않은 알림 유형입니다.") + else -> OPEN // or another sensible default }Option 2: Return nullable:
-fun fromLabel(label: String) = +fun fromLabel(label: String): NotiType? = when (label) { RESTOCK.label -> RESTOCK OPEN.label -> OPEN PREORDER.label -> PREORDER SALE_END.label -> SALE_END SALE_START.label -> SALE_START REMINDER.label -> REMINDER - else -> throw IllegalArgumentException("유효하지 않은 알림 유형입니다.") + else -> null }app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/wish/WishItemImage.kt (1)
6-12: Clarify dual image fields and finalize the TODO
WishItemImagekeeps bothurl(itemImageUrl) and legacyitemImgwhile onlyurlis used in new mappings. Because both are non-nullable, deserialization will fail if the backend ever drops one of them before the client is updated. Either:
- guarantee the backend always sends both until
itemImgis removed, or- make the transitional field nullable and/or explicitly deprecate it to document the migration plan.
Once the server stops sending
itemImg, please remove the property to avoid carrying dead API surface.app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/user/UserInfoDto.kt (1)
11-14: AlignisPushAllowednullability with API semantics
isPushAllowedis nullable in the DTO but always defaulted tofalseintoDomain(). If the backend never omitspushStateand there is no semantic difference betweennullandfalse, consider making the DTO field non-null with a default:@SerialName("pushState") val isPushAllowed: Boolean = falseand then passing it through directly in
toDomain. This tightens the contract and avoids treating a potentially “unknown” state as “false” by accident.Also applies to: 20-20
app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiViewModel.kt (1)
28-42: Re-evaluate Channel usage and error handling behaviorUsing an unbuffered
Channel<Unit>forrefreshNotiListTriggermeans_refreshNotiListTrigger.send(Unit)will suspend if there is no active collector, potentially blockingWishBoardEventBus.onWishItemChanged.collect. For a one-to-many UI signal, aMutableSharedFlow<Unit>(or aChannel(capacity = Channel.CONFLATED)if you want to keep Channel) tends to be safer and avoids unexpected backpressure.Also, the simplified
onFailurepath now treats all errors the same; if the API previously used specific codes (e.g., 404 to indicate “no notifications”), double‑check that this behavior change—showing a generic snackbar and leaving the list as-is—is intended.Also applies to: 50-52
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardLabelTextField.kt (1)
29-88: Minor cleanups inWishBoardLabelTextField
isFocusedis written inonFocusChangedbut never read; unless you plan to style based on focus soon, it can be removed to avoid dead state.- You currently pass
modifierto bothTextFieldLabelandBasicTextField, while theColumnitself has no modifier. In many Compose APIs the pattern isColumn(modifier = modifier) { … }and then child modifiers are separate (e.g.,Modifier.fillMaxWidth()for the field). That avoids duplicating padding/size constraints on both label and field.Refactoring along these lines will make the composable a bit cleaner and more predictable to use.
app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/noti/NotiItemDto.kt (1)
3-5: Tidy up naming and image handling inNotiItemDtoThe DTO and mapping look consistent, but there are a couple of small polish opportunities:
itemImageis aList<WishItemImage>but named singular; renaming toitemImages(and updating call sites) would better match both the JSON key and its type.- In
toDomain(), you pickitemImage.firstOrNull()?.url. That’s fine if the UI only ever shows one image, but if the backend starts sending multiple, consider whether the domain model should eventually expose the full list instead of just the first entry.These are non-blocking, but aligning names and future behavior now will reduce surprises later.
Also applies to: 12-25, 29-35
app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/GuestAuthService.kt (1)
3-8: Check layering and serialization ofTokeninGuestAuthService
GuestAuthServicenow usescom.hyeeyoung.wishboard.domain.model.auth.Tokendirectly as the Retrofit body and response type. This couples the data layer to the domain layer and also assumesTokenis correctly set up for your Retrofit converter (e.g.,@Serializableor the right JSON annotations).If you want to keep a clean architecture boundary, consider introducing a
TokenDtoin the data/remote model package and mapping to/from the domainToken. At minimum, verify that the currentTokentype serializes/deserializes as expected for theauth/refreshendpoint.Also applies to: 12-15
app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/ItemService.kt (1)
3-6: Verify path consistency and consider decoupling from presentation models
updateWishItemstill targets@PUT("item/{item_id}")with@Path("item_id") itemId: Long, while the other item endpoints useitem/{itemId}. If the backend path has been normalized toitem/{itemId}, this one may be left behind—worth double‑checking against the API spec.
fetchWishListreturnsBaseResponse<PagedResponse<WishItemDto>>butWishItemDtolives underpresentation.upload.model. Having the remote service indata.remote.servicedepend on a presentation‑layer DTO inverts the usual dependency direction and can make refactoring harder. Long‑term, it would be cleaner to keep a remote DTO in the data layer and map that to whatever presentation model you need.The multipart shapes for
uploadWishItem/updateWishItemand the switch toBaseResponseNoDatafor write endpoints all look reasonable once confirmed against the backend contract.Also applies to: 21-27, 33-48, 49-57, 58-62
app/src/main/java/com/hyeeyoung/wishboard/data/remote/interceptor/AuthInterceptor.kt (1)
77-88: Side effect in request builder: consider extracting device ID initialization.Generating and persisting the device ID inside
newAuthBuilder()introduces a side effect during request construction. While functional, this could lead to race conditions if multiple requests are made simultaneously before a device ID is set.Consider initializing the device ID during interceptor construction or in a dedicated method:
+ private fun getOrCreateDeviceId(): String { + return localStorage.deviceId.ifEmpty { + val deviceId = UUID.randomUUID().toString() + localStorage.deviceId = deviceId + deviceId + } + } + private fun Request.newAuthBuilder(): Request { return this.newBuilder() .addHeader(AUTHORIZATION, "$TOKEN_PREF${localStorage.accessToken}") - .addHeader( - DEVICE_INFO_HEADER_NAME, - localStorage.deviceId.ifEmpty { - val deviceId = UUID.randomUUID().toString() - localStorage.deviceId = deviceId - deviceId - }, - ).build() + .addHeader(DEVICE_INFO_HEADER_NAME, getOrCreateDeviceId()) + .build() }app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/WishItemDto.kt (1)
12-13: Address the TODO: test behavior when image list is empty.The TODO comment indicates uncertainty about empty image list handling. The
toDomain()method handles this correctly withfirstOrNull()?.urlreturningnull, but the comment should be addressed or removed after verification.Would you like me to open an issue to track testing for the empty image list case, or can this TODO be removed if the behavior has been verified?
app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/FolderRepositoryImpl.kt (1)
36-38: Minor: Redundantitreference in nested map.The nested
it.map { it.toDomain() }usesitfor both the outerPagingDataand inner item, which can be confusing.Consider using an explicit parameter name for clarity:
).flow.map { - it.map { it.toDomain() } + it.map { folder -> folder.toDomain() } }app/src/main/java/com/hyeeyoung/wishboard/data/remote/paging/GeneralPagingSource.kt (1)
23-25: Consider logging exceptions for debugging purposes.The exception is caught and returned as
LoadResult.Error, but logging would help diagnose paging failures in production.+import timber.log.Timber + } catch (e: Exception) { + Timber.e(e, "Paging load failed for page $currentPage") LoadResult.Error(e) }app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardTextField.kt (2)
226-243: Unconditional Row rendering may cause layout inconsistency.Unlike the other overloads which conditionally render the bottom Row based on focus state or
bottomEndComponent, this overload always renders the Row withpadding(top = 6.dp). This creates 6dp of extra vertical space even when neither error message norbottomEndComponentis displayed.Consider aligning the conditional rendering logic with the first overload:
- Row( - modifier = Modifier - .fillMaxWidth() - .padding(top = 6.dp), - ) { + if (isFocused || bottomEndComponent != null) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(top = 6.dp), + ) { + if (isFocused) { + TextFieldErrorMessage( + errorHidingStrategy = errorHidingStrategy, + isFocused = isFocused, + isError = isError, + errorMsg = errorMsg, + ) + } + + Spacer(modifier = Modifier.weight(1f)) + + bottomEndComponent?.let { + bottomEndComponent() + } + } + }
105-107: Useit()for cleaner lambda invocation.The
letblock capturesbottomEndComponentbut then invokes it by name rather than usingit. While functionally correct, usingit()is more idiomatic.- bottomEndComponent?.let { - bottomEndComponent() - } + bottomEndComponent?.invoke()Or alternatively:
bottomEndComponent?.let { it() }Also applies to: 240-242
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
16-53: LGTM! Clear separation of legacy and new icon button implementations.The refactor properly separates concerns:
LegacyWishBoardIconButtonpreserves existing behavior for backward compatibilityWishBoardIconButtonintroduces a fixed 48dp touch target that meets accessibility guidelinesThe TODO comment on Line 42 is noted for future cleanup.
Would you like me to open an issue to track the TODO for adding the dimension resource?
app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/UserService.kt (1)
23-26: Consider renaming parameter to match the part name for clarity.The parameter is named
nicknamebut the@Partannotation specifies"request"as the part name. This mismatch may cause confusion during maintenance.suspend fun updateUserInfo( - @Part("request") nickname: RequestBody, + @Part("request") request: RequestBody, @Part profileImg: MultipartBody.Part?, ): BaseResponseNoDataAlternatively, if the server expects
"request"but the content is specifically a nickname, consider adding a brief comment for clarity.app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderDetailScreen.kt (1)
50-55: Use appropriate log level for non-error messages.
Timber.e(error level) is used for a routine refresh event log. This should useTimber.d(debug) orTimber.i(info) instead to avoid polluting error logs.LaunchedEffect(Unit) { viewModel.refreshFolderDetailTrigger.collectLatest { - Timber.e("폴더 상세 리프레시") + Timber.d("폴더 상세 리프레시") wishList.refresh() } }app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/ItemRepositoryImpl.kt (3)
52-54: Avoid shadowed variable names for clarity.Using
ittwice in nested lambdas (it.map { it.toDomain() }) shadows the outer variable and reduces readability.- ).flow.map { - it.map { it.toDomain() } + ).flow.map { pagingData -> + pagingData.map { it.toDomain() } }
128-140: Clarify the intent of thejsonserializer marked for removal.The
TODO 제거 예정(to be removed) comment suggests this serializer is temporary, but it's actively used inupdateWishItem. Additionally,prettyPrint = trueincreases payload size unnecessarily for network requests.Consider:
- Removing
prettyPrint = truefor production payloads.- Updating the TODO with context on when/why removal is planned.
- Centralizing the
Jsoninstance if it's used across multiple files.val json = Json { isLenient = true - prettyPrint = true + prettyPrint = false explicitNulls = true ignoreUnknownKeys = true coerceInputValues = true
70-82: Consider extracting duplicate image conversion logic.The
ImageTypetoMultipartBody.Partmapping is duplicated betweenuploadWishItemandupdateWishItem. Extracting this to a helper function would improve maintainability.private fun List<ImageType>?.toMultipartParts(): List<MultipartBody.Part>? = this?.mapNotNull { imageType -> when (imageType) { is ImageType.DownloadImage -> MultipartBody.Part.createFormData( FORM_DATA_IMAGE_KEY, imageType.file.name, imageType.file.asRequestBody("image/jpeg".toMediaTypeOrNull()), ) is ImageType.Picture -> imageType.image } }Also applies to: 95-107
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardMiniSingleTextField.kt (1)
37-42: Address the paste handling TODO.The current implementation silently discards input exceeding
maxLength, which could confuse users when pasting longer text. Consider trimming tomaxLengthinstead of rejecting entirely.onValueChange = { - // TODO 붙여넣기 예외처리 - if (it.text.length <= maxLength) { - onTextChange(it) - } + if (it.text.length <= maxLength) { + onTextChange(it) + } else { + // Trim pasted text to maxLength, preserving cursor at end + val trimmedText = it.text.take(maxLength) + onTextChange(it.copy(text = trimmedText, selection = TextRange(trimmedText.length))) + } },Note: This requires adding
import androidx.compose.ui.text.TextRange.app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/FolderService.kt (1)
38-45: Inconsistent path parameter naming.
updateNameuses@Path("folder_id")(snake_case) whiledeleteFolderuses@Path("folderId")(camelCase). If this reflects actual backend endpoint differences, consider adding a comment for clarity. Otherwise, align the naming for consistency.app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/LinkSharingWishUploadActivity.kt (1)
52-58: Consider a more descriptive exception.Line 55 throws a generic
NullPointerException. Consider usingIllegalStateExceptionorIllegalArgumentExceptionwith a more descriptive message to better indicate the contract violation.- url = intent.getStringExtra(Intent.EXTRA_TEXT) ?: throw NullPointerException("Url is null") + url = intent.getStringExtra(Intent.EXTRA_TEXT) + ?: throw IllegalArgumentException("EXTRA_TEXT is required for ACTION_SEND")app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderViewModel.kt (2)
29-34: Consider makinggetFoldersUseCaseprivate.The constructor parameter
getFoldersUseCaselacks theprivatemodifier, making it publicly accessible. Unless there's a specific reason (e.g., testing), use-case dependencies are typically kept private.class FolderViewModel @Inject constructor( - getFoldersUseCase: GetFoldersUseCase, + private val getFoldersUseCase: GetFoldersUseCase,Note: If you make it private val, you'll need to adjust line 38 accordingly since the property is initialized inline.
38-43: Address the TODO comment.There's a
TODO fetchStatecomment indicating incomplete work. Consider either implementing fetch state tracking or creating an issue to track this work.Do you want me to open an issue to track implementing fetch state handling for the folders paging flow?
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.kt (4)
282-291: Avoidremember+mutableStateOffor purely derivedisEnabledSave
isEnabledSaveis fully derived fromuiModel.itemName,uiModel.itemPrice, anduiModel.images, but is stored in aremember { mutableStateOf(...) }with keys. This adds unnecessary state allocation and indirection.You can simplify to a plain derived value:
val isEnabledSave = uiModel.itemName.text.isNotBlank() && uiModel.itemPrice.text.isNotBlank() && uiModel.images.isNotEmpty()This keeps recompositions predictable and avoids extra mutable state.
365-377: Centralize image count limit and avoid magic error stringThe add‑image lambda correctly enforces
uiModel.images.size < MAX_IMAGE_COUNTbefore launching the picker, but the error message when exceeding the limit is an inline hard‑coded string:updateSnackbarMessage("앗, 이미지는 최대 10장까지만 등록할 수 있어요!")For consistency and easier localization, consider moving this into a string resource and, if possible, formatting it using
MAX_IMAGE_COUNTrather than duplicating10in the string. That keeps the UX aligned if the limit changes later.
584-673: StabilizeLazyRowitem identity and simplifyselectedImageCount
ItemImageRowworks well functionally, but there are a couple of small improvements:
selectedImageCountis always passed asuiModel.images.size, which can be derived fromimagesinside this composable. The parameter is redundant.LazyRow(items(images) { ... })currently uses index keys. Since eachUploadImagealready has a stableid(used for delete), you can use it as the item key to avoid unnecessary recomposition when images are added/removed.Example:
LazyRow( modifier = Modifier .fillMaxWidth() .padding(top = 18.dp), horizontalArrangement = Arrangement.spacedBy(8.dp), ) { // camera tile... items( items = images, key = { image -> image.id }, ) { image -> // image tile... } }And compute the counter via
images.sizerather than a separate parameter.
850-893: Consider applying layout modifier to clickable container instead of children
ItemFieldWithDetailIconappliesModifier.noRippleClickableto theColumn, while the providedmodifier(e.g., padding) is applied separately to the labelTextand the innerRow:Column( modifier = Modifier.noRippleClickable { onClick() }, ... ) { Text(modifier = modifier, ...) Row(modifier = modifier.fillMaxWidth(), ...) { ... } }This works, but slightly complicates hit testing and layout because the padding is not visually associated with the clickable container. For clearer layout and easier reuse, consider:
Column( modifier = modifier .fillMaxWidth() .noRippleClickable { onClick() }, ... ) { ... }and then use
Modifier.fillMaxWidth()on the Row only. This keeps the entire padded area consistently clickable.app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt (3)
236-284: Folder fetch per upload type is correct; consider login behavior for MANUAL
getFolders(uploadType)now:
- For PARSING: early‑returns when not logged in and guards repeated fetches via
folderFetchState.- For MANUAL: skips the login check and only guards on
folderFetchState.Depending on your product rules, you might also want to block MANUAL folder fetch when not logged in to avoid unnecessary failing calls:
WishItemUploadType.MANUAL -> { if (!localStorage.isLogin) return if (_manualUploadUiModel.value.folderFetchState is WishBoardState.Loading) return _manualUploadUiModel.update { it.copy(folderFetchState = WishBoardState.Loading) } }If manual upload is only reachable when logged in, current behavior is fine; otherwise, aligning PARSING and MANUAL here would make error handling more predictable.
383-399: Avoid double price sanitization between UI and ViewModel
onItemPriceChangedalready normalizes input via:val refinedPrice = price.copy(text = price.text.makeValidPriceStr() ?: "")However, in
WishUploadScreenyou also callmakeValidPriceStr()in the UI before forwarding the value. This results in sanitizing twice and slightly scatters the business rule across layers.Consider:
- Doing all normalization in the ViewModel (keep UI “dumb”), or
- Doing it only in the UI and making the ViewModel a pass‑through for price formatting.
Centralizing the normalization in one layer (preferably the ViewModel) will simplify maintenance.
401-441: Image addition logic: consider enforcingMAX_IMAGE_COUNTin ViewModel as well
addItemImageUrl(uris: List<Uri>)simply appends all URIs:_manualUploadUiModel.update { it.copy(images = it.images + uris.map { uri -> UploadImage.Local(uri) }) }The UI (
WishUploadScreen) enforcesMAX_IMAGE_COUNTbefore launching the picker, so under normal use you won’t exceed the limit. But if this function is reused from another screen or the UI logic changes, the ViewModel won’t guard against overshootingMAX_IMAGE_COUNT.Consider adding a simple cap here as a safety net, e.g.:
_manualUploadUiModel.update { val newImages = it.images + uris.map { uri -> UploadImage.Local(uri) } it.copy(images = newImages.take(MAX_IMAGE_COUNT)) }(Using the same constant that the UI relies on.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
app/build.gradle.kts(2 hunks)app/src/main/AndroidManifest.xml(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/config/GlobalState.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/config/navigation/navhost/WishBoardNavHost.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/config/navigation/screen/MainScreen.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/local/WishBoardPreference.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/interceptor/AuthInterceptor.kt(4 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/auth/AuthResponseDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/auth/RefreshTokenDto.kt(0 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/base/BaseResponse.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/base/BaseResponseNoData.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/base/PagedResponse.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/common/PageSize.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/folder/FolderItemDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/folder/FolderNameDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/folder/FolderSummaryDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/noti/NotiItemDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/user/NicknameRequestDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/user/UserInfoDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/wish/WishItemDetailDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/wish/WishItemIdDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/wish/WishItemImage.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/wish/WishItemUploadInfoDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/paging/GeneralPagingSource.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/AuthRepositoryImpl.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/FolderRepositoryImpl.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/ItemRepositoryImpl.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/NotiRepositoryImpl.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/UserRepositoryImpl.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/AuthService.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/FolderService.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/GuestAuthService.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/ItemService.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/NotiService.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/SystemService.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/remote/service/UserService.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/util/ContentUriRequestBody.kt(0 hunks)app/src/main/java/com/hyeeyoung/wishboard/data/util/extension/StringExt.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/WishboardSnackbar.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardWideButton.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/dialog/screen/WishBoardModal.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/dialog/temp/ModalTitle.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/dialog/temp/WishBoardModal.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardLabelTextField.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardMiniSingleTextField.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardSimpleTextField.kt(0 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardTextField.kt(6 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/topbar/WishBoardMainTopBar.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/topbar/WishBoardTopBar.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/util/LegacyPriceTransformation.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/designsystem/util/PriceVisualTransformation.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/model/noti/NotiType.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/model/wish/ParsedWishItem.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/model/wish/WishItemDetail.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/model/wish/WishItemUploadInfo.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/repository/FolderRepository.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/repository/ItemRepository.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/folder/GetFolderDetailUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/folder/GetFolderSummariesUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/folder/GetFoldersUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/folder/PostNewFolderUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/item/GetParsedItemInfoUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/item/GetWishItemDetailUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/usecase/item/GetWishListUseCase.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/util/JsonUtil.kt(0 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/util/SafeEnumUtil.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/domain/util/WishBoardDateFormat.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/calendar/CalendarViewModel.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/calendar/component/CalendarHeader.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/calendar/screen/CalendarScreen.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/cart/CartScreen.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt(0 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/common/WebviewScreen.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderDetailScreen.kt(5 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderDetailViewModel.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderListModalContent.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderScreen.kt(8 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderViewModel.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/model/FolderTabUiModel.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/main/MainActivity.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/main/MainViewModel.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/my/MyViewModel.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/my/model/MyUiModel.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/MyScreen.kt(10 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/PasswordChangeScreen.kt(4 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/ProfileEditScreen.kt(7 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiModalContent.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiScreen.kt(4 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiViewModel.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/onboarding/OnboardingScreen.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/SignViewModel.kt(5 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/model/auth/SignUiModel.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignInEmailScreen.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignInScreen.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignInVerificationCodeScreen.kt(7 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignUpEmailScreen.kt(3 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignUpPasswordScreen.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt(6 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/component/ShopLinkModalContent.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/ManualUploadItemUiModel.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/ParsingUploadItemUiModel.kt(2 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/UploadImage.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/WishItemDto.kt(1 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/LinkSharingWishUploadActivity.kt(5 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/LinkSharingWishUploadScreen.kt(8 hunks)app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.kt(12 hunks)
⛔ Files not processed due to max files limit (19)
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/TestVariable.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/TimeUtil.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/WishBoardEventBus.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/WishBoardPullToRefreshBox.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/BitmapUtil.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/ContextExt.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/LocalDateTimeExt.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/ModifierExt.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/NavControllerExt.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/WishListViewModel.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/model/WishItemDetailUiModel.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/model/WishListUiModel.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/screen/WishIistScreen.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/screen/WishItemDetailScreen.kt
- app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/screen/WishItemViewModel.kt
- app/src/main/res/drawable/ic_delete_small.xml
- app/src/main/res/values/dimes.xml
- app/src/main/res/values/strings.xml
- gradle/libs.versions.toml
💤 Files with no reviewable changes (5)
- app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt
- app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/auth/RefreshTokenDto.kt
- app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardSimpleTextField.kt
- app/src/main/java/com/hyeeyoung/wishboard/data/util/ContentUriRequestBody.kt
- app/src/main/java/com/hyeeyoung/wishboard/domain/util/JsonUtil.kt
🧰 Additional context used
🧬 Code graph analysis (26)
app/src/main/java/com/hyeeyoung/wishboard/presentation/main/MainViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt (1)
sendSnackbarChannel(474-478)
app/src/main/java/com/hyeeyoung/wishboard/presentation/cart/CartScreen.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderDetailViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt (1)
updateSnackbarMessage(18-35)
app/src/main/java/com/hyeeyoung/wishboard/domain/model/noti/NotiType.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/domain/util/SafeEnumUtil.kt (1)
safeValueOf(6-13)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/topbar/WishBoardTopBar.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardLabelTextField.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/util/SpanUtil.kt (1)
buildStringWithSpans(8-21)
app/src/main/java/com/hyeeyoung/wishboard/presentation/calendar/CalendarViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt (1)
updateSnackbarMessage(18-35)
app/src/main/java/com/hyeeyoung/wishboard/presentation/main/MainActivity.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/WishboardSnackbar.kt (1)
WishBoardSnackbarMessage(33-50)
app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/ProfileEditScreen.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/my/MyViewModel.kt (1)
onNicknameChange(169-173)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/topbar/WishBoardMainTopBar.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/dialog/screen/WishBoardModal.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderDetailScreen.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/util/TestVariable.kt (1)
getFakePagingData(11-28)
app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/screen/SignInScreen.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/WishboardSnackbar.kt (1)
WishBoardGlobalSnackbarMessage(57-71)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/WebviewScreen.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt (1)
updateSnackbarMessage(18-35)
app/src/main/java/com/hyeeyoung/wishboard/presentation/my/MyViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt (1)
updateSnackbarMessage(18-35)
app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiScreen.kt (2)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/WishboardSnackbar.kt (1)
WishBoardGlobalSnackbarMessage(57-71)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/presentation/sign/SignViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt (1)
updateSnackbarMessage(18-35)
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/LinkSharingWishUploadScreen.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.kt (4)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardLabelTextField.kt (1)
WishBoardLabelTextField(29-89)app/src/main/java/com/hyeeyoung/wishboard/presentation/folder/FolderUploadModalContent.kt (1)
FolderUploadModalContent(24-69)app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/FolderRepositoryImpl.kt (1)
createFolder(62-64)app/src/main/java/com/hyeeyoung/wishboard/domain/repository/FolderRepository.kt (1)
createFolder(15-15)
app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiViewModel.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/common/BaseViewModel.kt (1)
updateSnackbarMessage(18-35)
app/src/main/java/com/hyeeyoung/wishboard/presentation/calendar/component/CalendarHeader.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardTextField.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/button/WishBoardIconButton.kt (1)
LegacyWishBoardIconButton(16-31)
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/WishItemDto.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/wish/component/WishItem.kt (1)
WishItem(22-72)
app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/noti/NotiItemDto.kt (1)
app/src/main/java/com/hyeeyoung/wishboard/presentation/noti/NotiScreen.kt (1)
NotiItem(151-204)
app/src/main/java/com/hyeeyoung/wishboard/presentation/my/screen/MyScreen.kt (3)
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/divider/WishBoardThickDivider.kt (1)
WishBoardThickDivider(10-15)app/src/main/java/com/hyeeyoung/wishboard/presentation/my/MyViewModel.kt (1)
deleteAccount(158-167)app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardTextField.kt (3)
WishBoardTextField(38-111)WishBoardTextField(113-173)WishBoardTextField(175-245)
🪛 detekt (1.23.8)
app/src/main/java/com/hyeeyoung/wishboard/domain/util/SafeEnumUtil.kt
[warning] 10-10: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR Checker
| return if (snackbarMessage == null) { | ||
| updateRefreshToken(chain) ?: run { | ||
| // handleAutoLoginExpiration() | ||
| getUnauthorizedExceptionResponse(originRequest = originRequest) | ||
| } | ||
| } else { | ||
| handleAutoLoginExpiration(snackbarMessage) | ||
| getUnauthorizedExceptionResponse(originRequest = originRequest, snackbarMessage = snackbarMessage) | ||
| } |
There was a problem hiding this comment.
Clarify the commented-out handleAutoLoginExpiration() call.
Line 36 has handleAutoLoginExpiration() commented out. If the token refresh fails without a specific error message, the user's session expires but they won't be notified or logged out properly. This could lead to inconsistent app state.
Either uncomment the call or add a TODO explaining why it's disabled:
return if (snackbarMessage == null) {
updateRefreshToken(chain) ?: run {
-// handleAutoLoginExpiration()
+ handleAutoLoginExpiration()
getUnauthorizedExceptionResponse(originRequest = originRequest)
}
} else {📝 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.
| return if (snackbarMessage == null) { | |
| updateRefreshToken(chain) ?: run { | |
| // handleAutoLoginExpiration() | |
| getUnauthorizedExceptionResponse(originRequest = originRequest) | |
| } | |
| } else { | |
| handleAutoLoginExpiration(snackbarMessage) | |
| getUnauthorizedExceptionResponse(originRequest = originRequest, snackbarMessage = snackbarMessage) | |
| } | |
| return if (snackbarMessage == null) { | |
| updateRefreshToken(chain) ?: run { | |
| handleAutoLoginExpiration() | |
| getUnauthorizedExceptionResponse(originRequest = originRequest) | |
| } | |
| } else { | |
| handleAutoLoginExpiration(snackbarMessage) | |
| getUnauthorizedExceptionResponse(originRequest = originRequest, snackbarMessage = snackbarMessage) | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/data/remote/interceptor/AuthInterceptor.kt
around lines 34 to 42, the call to handleAutoLoginExpiration() is commented out
which leaves the user unnotified and not logged out when refreshToken fails
without a snackbarMessage; either re-enable the call so session expiration is
handled (uncomment handleAutoLoginExpiration()) ensuring it runs before
returning getUnauthorizedExceptionResponse, or if it must stay disabled add a
clear TODO comment explaining why it’s disabled, what conditions prevent its
use, and link to the issue/PR that will address it so future maintainers know
why user logout/notification is omitted.
| data class AuthResponseDto( | ||
| /** 임시 닉네임, 수정 이력이 있는 유저는 null로 내려옴 */ | ||
| val tempNickname: String? = null, | ||
| val temporaryNickname: String? = null, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for references to the old field name and verify the new code
# 1. Find all references to tempNickname
echo "=== References to old field name (tempNickname) ==="
rg -n 'tempNickname' --type=kt
echo ""
echo "=== References to new field name (temporaryNickname) ==="
rg -n 'temporaryNickname' --type=kt
echo ""
echo "=== AuthResponseDto file content ==="
fd 'AuthResponseDto.kt' --exec cat -n {}Repository: hyeeyoung/wishboard-android-compose
Length of output: 680
🏁 Script executed:
#!/bin/bash
# Search without type restriction to find all references
echo "=== AuthRepositoryImpl references ==="
rg -n 'AuthRepositoryImpl' -A5 -B5
echo ""
echo "=== AuthResponseDto usage ==="
rg -n 'AuthResponseDto' -A3 -B3
echo ""
echo "=== All references to temporaryNickname (broader search) ==="
rg -i 'temporary' -n
echo ""
echo "=== Check test files for field usage ==="
fd 'Test.kt' -o -name '*Test*.kt' | head -20Repository: hyeeyoung/wishboard-android-compose
Length of output: 8145
🏁 Script executed:
#!/bin/bash
# Check for @SerialName usage patterns and backend API info
echo "=== Check for @SerialName usage in project ==="
rg -n '@SerialName' -B2 -A2
echo ""
echo "=== Full AuthRepositoryImpl.kt to see how temporaryNickname is used ==="
fd 'AuthRepositoryImpl.kt' --exec cat -n {}
echo ""
echo "=== Check other DTOs for serialization patterns ==="
fd '.*Dto.kt' | head -10 | xargs grep -l 'SerialName' 2>/dev/null | head -5Repository: hyeeyoung/wishboard-android-compose
Length of output: 15304
Add @SerialName annotation to maintain project convention and ensure API compatibility.
The field rename from tempNickname to temporaryNickname breaks the established project pattern. All similar DTOs in the codebase (WishItemDto, NotiItemDto, FolderItemDto, UserInfoDto, WishItemDetailDto) consistently use @SerialName to map Kotlin property names to JSON field names. Without this annotation, the code expects the backend to send "temporaryNickname" in the response.
Add @SerialName("tempNickname") unless the backend API has been confirmed to send "temporaryNickname". Additionally, the token field should also have @SerialName("token") to align with project conventions.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/auth/AuthResponseDto.kt
around line 9, the property temporaryNickname was renamed from tempNickname but
lacks the @SerialName mapping and the token property also lacks the
project-standard @SerialName; add @SerialName("tempNickname") to the
temporaryNickname property (unless backend confirmed to send
"temporaryNickname") and add @SerialName("token") to the token property, and
import kotlinx.serialization.SerialName so the DTO maps to the API fields
consistently.
| data class FolderSummaryDto( | ||
| @SerialName("id") | ||
| val id: Long, | ||
| @SerialName("folderName") | ||
| var name: String = "", | ||
| @SerialName("folderThumbnail") | ||
| val thumbnail: String = "", | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Prefer immutable fields in DTOs.
The name field is declared as var, making it mutable, while DTOs should typically be immutable to prevent unintended modifications and ensure data integrity. Unless there's a specific serialization or business requirement for mutability, this field should be declared with val.
Apply this diff to make the field immutable:
@SerialName("folderName")
- var name: String = "",
+ val name: String = "",📝 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.
| data class FolderSummaryDto( | |
| @SerialName("id") | |
| val id: Long, | |
| @SerialName("folderName") | |
| var name: String = "", | |
| @SerialName("folderThumbnail") | |
| val thumbnail: String = "", | |
| ) { | |
| data class FolderSummaryDto( | |
| @SerialName("id") | |
| val id: Long, | |
| @SerialName("folderName") | |
| val name: String = "", | |
| @SerialName("folderThumbnail") | |
| val thumbnail: String = "", | |
| ) { |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/data/remote/model/folder/FolderSummaryDto.kt
around lines 8 to 15 the DTO declares `name` as a mutable `var`; change it to an
immutable `val` (keeping the SerialName annotation and default value) so the
data class is immutable, and then update any callers that currently modify
`name` to instead create a new FolderSummaryDto (or use copy()) so no code
relies on in-place mutation.
| item = WishItemUploadInfoDto.fromDomain(itemInfo).toJson().toRequestBody( | ||
| "application/json".toMediaTypeOrNull(), | ||
| ), |
There was a problem hiding this comment.
Inconsistent JSON serialization between uploadWishItem and updateWishItem.
uploadWishItem uses WishItemUploadInfoDto.fromDomain(itemInfo).toJson() (line 67), while updateWishItem uses json.encodeToString(WishItemUploadInfoDto.fromDomain(itemInfo)) (line 92). If toJson() uses a different Json configuration than the companion json, this could cause serialization inconsistencies.
Unify the approach—either use toJson() in both places or use the companion json.encodeToString() consistently:
item = WishItemUploadInfoDto.fromDomain(itemInfo).toJson().toRequestBody(
- "application/json".toMediaTypeOrNull(),
+ "application/json".toMediaTypeOrNull(), // TODO 상수화 필요
),Or for updateWishItem:
- item = json.encodeToString(WishItemUploadInfoDto.fromDomain(itemInfo)).toRequestBody(
+ item = WishItemUploadInfoDto.fromDomain(itemInfo).toJson().toRequestBody(Also applies to: 92-94
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/data/remote/repository/ItemRepositoryImpl.kt
around lines 67-69 and 92-94, serialization is inconsistent: uploadWishItem
calls WishItemUploadInfoDto.fromDomain(itemInfo).toJson() while updateWishItem
uses the companion json.encodeToString(...). Pick one approach and apply it to
both call sites so the same Json configuration is always used; e.g., replace the
toJson() usage with
json.encodeToString(WishItemUploadInfoDto.fromDomain(itemInfo)) (and ensure the
companion json is imported/accessible), then create the request body from that
string with the same media type, or alternatively make both call toJson() if
toJson() delegates to the same companion json—whichever maintains a single Json
instance across the file.
| visualTransformation: VisualTransformation = VisualTransformation.None, | ||
| errorHidingStrategy: Int = View.GONE, | ||
| endComponent: WishBoardTextFieldComponent = WishBoardTextFieldComponent.DeleteButton, | ||
| bottomEndComponent: (@Composable () -> Unit)? = null, |
There was a problem hiding this comment.
Inconsistency: bottomEndComponent parameter missing from String input overload.
The bottomEndComponent parameter is added to the TextFieldValue overload (Line 54) and MutableState<String> overload (Line 191), but is missing from the String input overload (Lines 113-129). This inconsistency may cause issues if callers expect uniform behavior across all overloads.
Consider adding the bottomEndComponent parameter to the String input overload for API consistency:
fun WishBoardTextField(
modifier: Modifier = Modifier,
input: String,
label: String? = null,
errorMsg: String? = null,
placeholder: String,
onTextChange: (String) -> Unit = {},
isError: Boolean = false,
singleLine: Boolean = true,
maxLength: Int = Int.MAX_VALUE,
keyboardOptions: KeyboardOptions = KeyboardOptions.Default,
keyboardActions: KeyboardActions = KeyboardActions.Default,
visualTransformation: VisualTransformation = VisualTransformation.None,
errorHidingStrategy: Int = View.GONE,
endComponent: WishBoardTextFieldComponent = WishBoardTextFieldComponent.DeleteButton,
+ bottomEndComponent: (@Composable () -> Unit)? = null,
)Also applies to: 113-129
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/designsystem/component/textfield/WishBoardTextField.kt
around lines 113 to 129, the String-typed overload is missing the
bottomEndComponent: (@Composable () -> Unit)? = null parameter (present in the
TextFieldValue and MutableState<String> overloads); add that parameter to the
String overload's signature, thread it through when creating/forwarding to the
internal TextFieldValue/MutableState implementation (or when calling the other
overload), and ensure any calls to the internal function pass bottomEndComponent
along so behavior and API are consistent across all overloads.
| sealed class UploadImage(val id: String = UUID.randomUUID().toString()) { | ||
| data class Remote(val url: String) : UploadImage() | ||
| data class Local(val uri: Uri) : UploadImage() | ||
| } |
There was a problem hiding this comment.
Random UUID breaks data class equality semantics.
Each UploadImage instance receives a random UUID, which means two Remote or Local instances with identical content will not be equal:
Remote("https://example.com") != Remote("https://example.com") // different UUIDsThis violates expected data class behavior and can cause bugs in collections, comparisons, or state management.
Consider one of these solutions:
Solution 1 (recommended): Make id deterministic based on content
-sealed class UploadImage(val id: String = UUID.randomUUID().toString()) {
- data class Remote(val url: String) : UploadImage()
- data class Local(val uri: Uri) : UploadImage()
+sealed class UploadImage {
+ abstract val id: String
+ data class Remote(val url: String) : UploadImage() {
+ override val id: String = url.hashCode().toString()
+ }
+ data class Local(val uri: Uri) : UploadImage() {
+ override val id: String = uri.toString().hashCode().toString()
+ }
}Solution 2: Remove id from equality check
-sealed class UploadImage(val id: String = UUID.randomUUID().toString()) {
- data class Remote(val url: String) : UploadImage()
- data class Local(val uri: Uri) : UploadImage()
+sealed class UploadImage {
+ val id: String = UUID.randomUUID().toString()
+ data class Remote(val url: String) : UploadImage()
+ data class Local(val uri: Uri) : UploadImage()
}Solution 3: Accept id as a parameter when needed
-sealed class UploadImage(val id: String = UUID.randomUUID().toString()) {
- data class Remote(val url: String) : UploadImage()
- data class Local(val uri: Uri) : UploadImage()
+sealed class UploadImage {
+ abstract val id: String
+ data class Remote(val url: String, override val id: String = UUID.randomUUID().toString()) : UploadImage()
+ data class Local(val uri: Uri, override val id: String = UUID.randomUUID().toString()) : UploadImage()
}🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/model/UploadImage.kt
around lines 6-9, the current default random UUID breaks data class equality
because each instance gets a different id; change the design so id is
deterministic (recommended): remove the default UUID generation from the sealed
superclass and derive id from each subtype's content (e.g., use the URL string
for Remote and the URI string for Local) so two instances with identical content
compare equal; alternatively, if unique runtime identity is required, accept id
as an explicit constructor parameter or exclude id from equality by overriding
equals/hashCode, but prefer the deterministic-id approach.
| viewModel.updateWishItem(context = context, itemId = itemDetail?.id) { | ||
| val detailRoute = "${MainScreen.WishItemDetail.route}/${itemDetail?.id}" | ||
| keyboardController?.hide() | ||
| navController.safePopBackStack() | ||
| navController.navigate(detailRoute) { | ||
| popUpTo(detailRoute) { inclusive = true } | ||
| launchSingleTop = true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the navigation graph file(s) and search for WishItemDetail route definition
fd -t f -e ".kt" | xargs rg -l "WishItemDetail|navGraph|NavGraph" | head -20Repository: hyeeyoung/wishboard-android-compose
Length of output: 1785
🏁 Script executed:
#!/bin/bash
# Read the ItemDetailNavGraph to see how WishItemDetail is registered
cat -n app/src/main/java/com/hyeeyoung/wishboard/config/navigation/navgraph/ItemDetailNavGraph.ktRepository: hyeeyoung/wishboard-android-compose
Length of output: 1292
🏁 Script executed:
#!/bin/bash
# Read MainScreen.kt to see route definitions
cat -n app/src/main/java/com/hyeeyoung/wishboard/config/navigation/screen/MainScreen.ktRepository: hyeeyoung/wishboard-android-compose
Length of output: 3853
🏁 Script executed:
#!/bin/bash
# Read WishUploadScreen.kt around lines 174-180
sed -n '165,190p' app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.ktRepository: hyeeyoung/wishboard-android-compose
Length of output: 1109
🏁 Script executed:
#!/bin/bash
# Search for popUpTo usages across the codebase
rg -n "popUpTo" -A 2 -B 2 app/src/main/java/com/hyeeyoung/wishboard/Repository: hyeeyoung/wishboard-android-compose
Length of output: 6201
🏁 Script executed:
#!/bin/bash
# Check context around the WishUploadScreen navigation code for error handling
sed -n '170,185p' app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.ktRepository: hyeeyoung/wishboard-android-compose
Length of output: 696
Based on my verification, I can now provide the rewritten review comment.
Fix popUpTo() to use the route pattern, not the concrete route with ID
In the edit flow at lines 174-180, popUpTo(detailRoute) receives a concrete route like "wishItemDetail/123", but the Navigation Graph registers the destination with the pattern "wishItemDetail/{wishItemId}". Navigation Compose expects popUpTo() to match against the registered pattern route, not a concrete route with arguments. This mismatch will cause the back stack pop to silently fail.
Change to:
navController.navigate("${MainScreen.WishItemDetail.route}/${itemDetail?.id}") {
popUpTo(MainScreen.WishItemDetail.route) { inclusive = true }
launchSingleTop = true
}All other popUpTo() calls in the codebase correctly use base routes or graph IDs; this is the only anomaly.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.kt
around lines 174-180, the popUpTo call is using a concrete route with the item
ID (e.g., "wishItemDetail/123") which does not match the registered navigation
pattern and causes the pop to fail; update the popUpTo to reference the
registered route pattern (MainScreen.WishItemDetail.route) instead of the
concrete detailRoute while keeping the navigate target as the concrete path and
preserving launchSingleTop and inclusive behavior.
| @Composable | ||
| private fun FolderList( | ||
| modifier: Modifier = Modifier, | ||
| folders: List<FolderItem>, | ||
| selectedFolder: FolderItem?, | ||
| showFolderDetail: () -> Unit, | ||
| onClickFolder: (FolderItem) -> Unit, | ||
| onClickNewFolder: () -> Unit, | ||
| ) { | ||
| val itemShape = RoundedCornerShape(16.dp) | ||
| val itemPadding = PaddingValues(vertical = 6.dp, horizontal = 10.dp) | ||
| val density = LocalDensity.current | ||
| val textMeasure = rememberTextMeasurer() | ||
| val textStyle = WishBoardTheme.typography.suitB5 | ||
| val folderItemHeight = density.run { | ||
| textMeasure.measure( | ||
| "폴더", | ||
| textStyle, | ||
| ).size.width.toDp() | ||
| } + itemPadding.calculateTopPadding() + itemPadding.calculateBottomPadding() | ||
|
|
There was a problem hiding this comment.
Fix folder gradient height calculation using text height instead of width
folderItemHeight is computed using measure("폴더", textStyle).size.width.toDp(), then padding is added. Width is used as a proxy for height, which is likely a typo and can produce incorrect gradient height:
val folderItemHeight = density.run {
textMeasure.measure("폴더", textStyle).size.width.toDp()
} + itemPadding.calculateTopPadding() + itemPadding.calculateBottomPadding()This should be based on size.height:
val folderItemHeight = with(density) {
textMeasure.measure("폴더", textStyle).size.height.toDp()
} + itemPadding.calculateTopPadding() + itemPadding.calculateBottomPadding()That will better match the actual chip height and align the right‑side gradient correctly.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/screen/WishUploadScreen.kt
around lines 685 to 705, the folderItemHeight calculation uses
textMeasure.measure(...).size.width which incorrectly uses the measured text
width as a proxy for height; change it to use .size.height (and you can switch
to with(density) for clarity) and keep adding the top and bottom padding as
before so the gradient height matches the actual chip height.
| fun uploadWishItemForManual( | ||
| context: Context, | ||
| afterSuccess: (Long) -> Unit, | ||
| ) { | ||
| if (_manualUploadUiModel.value.wishItemUploadState is WishBoardState.Loading || | ||
| _manualUploadUiModel.value.wishItemUploadState is WishBoardState.Success | ||
| ) { | ||
| return | ||
| } | ||
|
|
||
| _manualUploadUiModel.update { it.copy(wishItemUploadState = WishBoardState.Loading) } | ||
|
|
||
| viewModelScope.launch { | ||
| postWishItemUseCase( | ||
| uploadType = WishItemUploadType.MANUAL, | ||
| itemInfo = _manualUploadUiModel.value.toDomain( | ||
| itemImage = _manualUploadUiModel.value.images.toImageType( | ||
| context = context, | ||
| ), | ||
| ), | ||
| ).onSuccess { id -> | ||
| _manualUploadUiModel.update { it.copy(wishItemUploadState = WishBoardState.Success(Unit)) } | ||
| updateSnackbarMessage(ITEM_UPLOAD_SUCCESS_MESSAGE) | ||
| WishBoardEventBus.notifyWishItemChanged() | ||
| afterSuccess(id) | ||
| }.onFailure { exception, errorCode, _ -> | ||
| _manualUploadUiModel.update { it.copy(wishItemUploadState = WishBoardState.Failure) } | ||
| when (errorCode) { | ||
| 409 -> updateSnackbarMessage(message = "변경사항을 저장하지 못했어요!\n잠시 후 다시 시도해주세요.", exception = exception) | ||
| else -> updateSnackbarMessage(message = SnackbarMessage.DEFAULT, exception = exception) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for implementations of heavy image helpers
rg -n "fun compressImageToMaxSize" -A 10
rg -n "fun toBitmap" -A 10
rg -n "fun toFile" -A 10
rg -n "fun toImageType" -A 15Repository: hyeeyoung/wishboard-android-compose
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Explore project structure
find app/src -type f -name "*.kt" | grep -i image | head -20
# Search for toImageType more broadly
rg "toImageType" -n -A 5 -B 2
# Search for compressImageToMaxSize, toBitmap, toFile more broadly
rg "compressImageToMaxSize|toBitmap|toFile" -n -A 3 -B 1Repository: hyeeyoung/wishboard-android-compose
Length of output: 10499
🏁 Script executed:
#!/bin/bash
# Read the compressImageToMaxSize implementation
cat -n app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/ContextExt.kt | head -100
# Read the BitmapUtil implementations
cat -n app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/BitmapUtil.kt
# Read the toImageType function implementation completely
sed -n '480,510p' app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.ktRepository: hyeeyoung/wishboard-android-compose
Length of output: 9223
🏁 Script executed:
#!/bin/bash
# Read the full uploadWishItemForManual function to see how toImageType is called
sed -n '141,175p' app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.ktRepository: hyeeyoung/wishboard-android-compose
Length of output: 1606
🏁 Script executed:
#!/bin/bash
# Look more carefully at the context around line 157
sed -n '150,165p' app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt
# Check toDomain signature to understand parameter types
rg "fun.*toDomain" -A 5 -B 2 app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/Repository: hyeeyoung/wishboard-android-compose
Length of output: 3937
🏁 Script executed:
#!/bin/bash
# Confirm the exact location where toImageType is called in the coroutine
sed -n '152,165p' app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt
# Check if compressImageToMaxSize is called anywhere with IO dispatcher
rg "compressImageToMaxSize" -B 3 -A 3 app/src/main/java/com/hyeeyoung/wishboard/Repository: hyeeyoung/wishboard-android-compose
Length of output: 5353
Based on my verification, I have confirmed the issues identified in the review comment. Let me generate the final rewritten comment:
Manual upload flow: Heavy image processing occurs on main thread without IO dispatcher
uploadWishItemForManual uses images.toImageType(context) to build the list of ImageType, which performs file creation and JPEG compression for both local and remote images. However, the implementation has blocking IO operations on the main thread:
- Local images:
compressImageToMaxSize(ContextExt.kt:51-82) performs blocking IO—decoding bitmaps, resizing, compressing, and writing to disk without any dispatcher. - Remote images:
toBitmapcorrectly useswithContext(Dispatchers.IO), buttoFile(BitmapUtil.kt:52-62) is a non-suspend function that performs blocking FileOutputStream writes on the main thread.
Both compressImageToMaxSize and toFile must offload to Dispatchers.IO. Wrap the heavy operations explicitly:
viewModelScope.launch {
val images = withContext(Dispatchers.IO) {
_manualUploadUiModel.value.images.toImageType(context)
}
postWishItemUseCase(
uploadType = WishItemUploadType.MANUAL,
itemInfo = _manualUploadUiModel.value.toDomain(itemImage = images),
)
// ...
}Alternatively, make compressImageToMaxSize and toFile suspend functions with internal withContext(Dispatchers.IO) wrapping.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt
around lines 141-174, the call to
_manualUploadUiModel.value.images.toImageType(context) runs heavy image
processing (file IO, bitmap decoding/compression) on the main thread; move that
work onto Dispatchers.IO by computing the ImageType list inside a
withContext(Dispatchers.IO) block (or make the image helpers suspend and
internally use withContext(Dispatchers.IO)) before calling postWishItemUseCase
so all blocking disk/bitmap operations run on IO dispatcher.
| private suspend fun List<UploadImage>.toImageType(context: Context): List<ImageType> { | ||
| return this.map { | ||
| when (it) { | ||
| is UploadImage.Local -> { | ||
| val file = it.uri.let { uri -> | ||
| context.compressImageToMaxSize(uri) | ||
| } | ||
| val requestBody = file?.asRequestBody("image/jpeg".toMediaTypeOrNull()) | ||
|
|
||
| requestBody?.let { | ||
| ImageType.Picture( | ||
| MultipartBody.Part.createFormData("itemImages", file.name, requestBody), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| is UploadImage.Remote -> { | ||
| val bitmap = it.url.toBitmap() | ||
| val file = bitmap?.toFile(localStorage.accessToken, context = context) | ||
| file?.let { ImageType.DownloadImage(file = it) } | ||
| } | ||
| } | ||
| }.filterNotNull() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Dispatchers.IO usage in the codebase
rg -n "Dispatchers\.IO" -S
echo "---"
# Search for compressImageToMaxSize function
rg -n "compressImageToMaxSize" -S -A 5
echo "---"
# Search for toBitmap function
rg -n "toBitmap" -S -A 5
echo "---"
# Search for toFile function
rg -n "toFile" -S -A 5Repository: hyeeyoung/wishboard-android-compose
Length of output: 11309
🏁 Script executed:
#!/bin/bash
# Get full implementation of compressImageToMaxSize
cat -n app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/ContextExt.kt | sed -n '51,100p'
echo "---"
# Get full implementation of toFile
cat -n app/src/main/java/com/hyeeyoung/wishboard/presentation/util/extension/BitmapUtil.kt | sed -n '52,65p'
echo "---"
# Find where toImageType is called
rg -n "toImageType" -S -B 3 -A 3Repository: hyeeyoung/wishboard-android-compose
Length of output: 5389
🏁 Script executed:
#!/bin/bash
# Get context around where toImageType is called (line 157 and 193)
cat -n app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt | sed -n '145,165p'
echo "---"
# Get the second call context around line 193
cat -n app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt | sed -n '180,205p'
echo "---"
# Check if these are inside viewModelScope.launch
rg -n "viewModelScope.launch" app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt -A 30 | head -60Repository: hyeeyoung/wishboard-android-compose
Length of output: 5092
The review comment's concerns are valid; both compressImageToMaxSize and toFile are heavy blocking operations that must be offloaded to Dispatchers.IO.
The code calls toImageType() within viewModelScope.launch (lines 153, 189) which defaults to the Main thread. Inside toImageType():
compressImageToMaxSize()performs synchronous bitmap decoding, resizing, and file I/O without any dispatcher context (lines 56–81 in ContextExt.kt)toBitmap()is properly wrapped withwithContext(Dispatchers.IO)✓toFile()performs synchronous bitmap compression and file I/O without any dispatcher context (lines 52–62 in BitmapUtil.kt)
When multiple images are selected, these blocking operations on the Main thread risk ANR and UI jank. The inconsistency with toBitmap() highlights the issue further.
Required fix: Wrap both blocking operations in withContext(Dispatchers.IO) within toImageType():
- Line 485:
context.compressImageToMaxSize(uri)→ wrap withwithContext(Dispatchers.IO) - Line 498:
bitmap?.toFile(...)→ wrap withwithContext(Dispatchers.IO)
Alternatively, make these helper functions suspend and offload internally.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hyeeyoung/wishboard/presentation/upload/WishItemUploadViewModel.kt
around lines 480 to 503, the calls to context.compressImageToMaxSize(uri) and
bitmap?.toFile(...) perform blocking bitmap decode/encode and file I/O on the
default coroutine context (Main via viewModelScope), risk ANR and must be
offloaded; update to wrap those calls in withContext(Dispatchers.IO) so the
heavy work runs on IO dispatcher (or alternatively refactor those helpers into
suspend functions that internally switch to Dispatchers.IO).
What is this PR? 🔍
Key Changes 🔑
To Reviewers 📢
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.