인증샷 업로드 기능 구현

[chanho0908]

이슈 번호

#52

리뷰/머지 희망 기한 (선택)

작업내용

결과물

리뷰어에게 추가로 요구하는 사항 (선택)

• 이벤트 버스 현수 따라서 만들어봤는데 이렇게 사용하는게 맞는지 확인 부탁해 🙃
• 인증샷 업로드 API 호출 완료되기까지 약 5초..? 좀 오래 걸리는 것 같은데 이거 서버랑 이야기 해봐야할듯 !

[chanho0908]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

feature/task-certification/src/main/java/com/twix/task_certification/detail/TaskCertificationDetail.kt (1)

101-124: ⚠️ Potential issue | 🟡 Minor

권한 거부 시 토스트 메시지가 두 경우 모두 동일합니다 — 의도된 것인가요?

shouldShowRationale == true인 경우(단순 거부)와 shouldShowRationale == false인 경우(영구 거부)에 동일한 문자열 task_certification_camera_permission_denied_permanently를 사용하고 있습니다.

단순 거부 시에는 "카메라 권한이 필요합니다"와 같은 안내 메시지가 더 적절하고, 영구 거부 시에만 설정 이동 안내를 보여주는 것이 UX 관점에서 바람직합니다. 현재 구조상 메시지만 다르게 하면 되므로 변경 비용이 낮습니다.

💡 개선 제안

             if (!shouldShowRationale) {
                 toastManager.tryShow(
                     ToastData(
                         currentContext.getString(
                             R.string.task_certification_camera_permission_denied_permanently,
                         ),
                         ToastType.ERROR,
                         action =
                             ToastAction(
                                 label = currentContext.getString(R.string.move_to_setting),
                                 onClick = { openAppSettings(currentContext) },
                             ),
                     ),
                 )
             } else {
                 toastManager.tryShow(
                     ToastData(
                         currentContext.getString(
-                            R.string.task_certification_camera_permission_denied_permanently,
+                            R.string.task_certification_camera_permission_denied, // 단순 거부용 별도 문자열
                         ),
                         ToastType.ERROR,
                     ),
                 )
             }

feature/task-certification/src/main/java/com/twix/task_certification/detail/TaskCertificationDetailViewModel.kt (1)

53-68: ⚠️ Potential issue | 🔴 Critical

🚨 fetchPhotolog()의 onSuccess 블록이 주석 처리되어 있어 UI 상태가 업데이트되지 않습니다.

이것은 기능적 결함입니다. launchResult의 onSuccess에서 reduce { copy(photoLogs = ...) }가 주석 처리되어 있기 때문에, API 호출이 성공하더라도 UI 상태(uiState)에 photolog 데이터가 반영되지 않습니다. 결과적으로 화면에 인증샷 데이터가 표시되지 않게 됩니다.

이 상태로 머지하면 인증샷 상세 화면이 빈 상태로 남게 됩니다. 의도적인 WIP(작업 중)라면 TODO 주석을 남겨주시고, 실수라면 주석을 해제해주세요.

🐛 수정 제안

     private fun fetchPhotolog() {
         launchResult(
             block = { photologRepository.fetchPhotoLogs(goalId) },
             onSuccess = {
-                // reduce { copy(photoLogs = it.toUiModel(), goalTitle = goalTitle) }
+                reduce { copy(photoLogs = it.toUiModel(), goalTitle = goalTitle) }
             },
             onError = {

🤖 Fix all issues with AI agents

In `@core/ui/src/main/java/com/twix/ui/extension/Context.kt`:
- Line 19: Update the KDoc `@return` description in Context.kt to match the actual
behavior: the function returns a non-null ByteArray and on failure returns an
empty byteArrayOf() rather than null; locate the KDoc for the method in
Context.kt and change the return text to state it returns a JPEG-compressed
image as a ByteArray and returns an empty byte array on failure.
- Around line 21-34: Context.uriToByteArray에서 contentResolver.openInputStream과
BitmapFactory.decodeStream 결과를 안전하게 처리하도록 InputStream을 use { }로 감싸서 자동으로 닫고,
decodeStream 결과인 bitmap에 대해 null 체크를 추가해 bitmap이 null일 경우 NPE를 방지하며
bitmap.recycle()로 네이티브 메모리 즉시 해제하도록 수정하세요; 또한 outputStream.toByteArray()는
non-null이므로 불필요한 엘비스 연산자를 제거하고, 실패 시 빈 ByteArray를 반환할지 또는 null을 반환할지 결정해 함수
시그니처/ KDoc(현재 "실패 시 null")과 동작을 일치시키도록 KDoc을 업데이트하세요 (참조 심볼:
Context.uriToByteArray, contentResolver.openInputStream,
BitmapFactory.decodeStream, bitmap.compress, bitmap.recycle,
ByteArrayOutputStream.toByteArray).

In
`@feature/task-certification/src/main/java/com/twix/task_certification/certification/TaskCertificationScreen.kt`:
- Around line 126-129: GetImageFromUri handling currently performs potentially
blocking I/O on the main thread by calling currentContext.uriToByteArray inside
the side-effect handler; move the URI → ByteArray conversion into the ViewModel
(e.g., add a method like handleTryUpload(uri: Uri) that runs on Dispatchers.IO
within viewModelScope) so the UI only dispatches the URI, and change the
side-effect to dispatch something like TaskCertificationIntent.TryUpload(uri)
instead of reading bytes in the UI; additionally, ensure null-safe behavior by
checking for a null byte array in the ViewModel before dispatching
TaskCertificationIntent.Upload(byteArray) or emitting a failure side-effect
(avoid calling viewModel.dispatch(TaskCertificationIntent.Upload(image)) from
the UI when image may be null).

In
`@feature/task-certification/src/main/java/com/twix/task_certification/certification/TaskCertificationViewModel.kt`:
- Around line 103-120: In both upload(...) and uploadPhotoLog(...) change the
onError callbacks passed to launchResult so they call emitSideEffect(...) with
the TaskCertificationSideEffect.ShowToast instance instead of merely
constructing it; specifically, inside upload use
emitSideEffect(TaskCertificationSideEffect.ShowToast(R.string.task_certification_upload_fail,
ToastType.ERROR)) in the onError suspend lambda, and apply the same pattern in
uploadPhotoLog's onError to ensure errors actually emit the side effect.
- Around line 122-145: The onError block in uploadPhotoLog currently constructs
a TaskCertificationSideEffect.ShowToast but never emits it; change the onError
to call tryEmitSideEffect(TaskCertificationSideEffect.ShowToast(...)) so the
toast side effect is actually emitted (match the pattern used in the onSuccess
block where tryEmitSideEffect(TaskCertificationSideEffect.NavigateToDetail) is
called); update the onError in uploadPhotoLog to invoke tryEmitSideEffect with
the ShowToast instance.

🧹 Nitpick comments (6)

feature/task-certification/src/main/java/com/twix/task_certification/certification/model/TaskCertificationSideEffect.kt (2)

8-13: ShowImageCaptureFailToast와 ShowToast의 역할 중복 가능성 검토

기존 ShowImageCaptureFailToast는 특정 실패 상황에 대한 토스트인데, 새로 추가된 ShowToast(message, type)가 메시지와 타입을 파라미터로 받아 범용적으로 사용할 수 있으므로, ShowImageCaptureFailToast도 ShowToast로 통합할 수 있지 않을까요?

통합하면 SideEffect 종류가 줄어들어 핸들링 코드가 단순해지고, 추후 새로운 토스트가 필요할 때마다 별도 클래스를 추가하지 않아도 됩니다.

♻️ 통합 예시

 sealed interface TaskCertificationSideEffect : SideEffect {
-    data object ShowImageCaptureFailToast : TaskCertificationSideEffect
-
     data class ShowToast(
         val message: Int,
         val type: ToastType,
     ) : TaskCertificationSideEffect

ViewModel에서 기존 ShowImageCaptureFailToast 발행부를 ShowToast(R.string.image_capture_fail, ToastType.Error) 형태로 변경하면 됩니다.

---

15-17: GetImageFromUri — SideEffect 네이밍에 대한 제안

현재 이름이 "URI로부터 이미지를 가져와라"라는 명령형으로 읽히는데, SideEffect는 보통 View가 수행해야 할 동작을 나타내므로 의미 전달에는 문제없습니다. 다만, 다른 SideEffect들이 Show~, NavigateTo~ 등 View 동작 중심 네이밍인 것과 비교하면, ConvertImageFromUri 혹은 ProcessImageUri 같은 이름도 고려해볼 수 있습니다.

이 부분은 팀 컨벤션에 따라 현재 이름을 유지해도 괜찮습니다 — 단순히 일관성 관점에서의 제안입니다.

core/util/src/main/java/com/twix/util/bus/TaskCertificationRefreshBus.kt (1)

7-16: 이벤트 버스 구현이 적절합니다 ✅

PR 작성자분이 이벤트 버스 사용법에 대해 확인을 요청하셨는데, 전반적으로 잘 구현되어 있습니다. replay = 0, extraBufferCapacity = 1 조합은 이벤트 신호용으로 적합한 패턴입니다.

한 가지 알아두시면 좋은 점: onBufferOverflow가 기본값 BufferOverflow.SUSPEND인 상태에서 tryEmit을 사용하면, 버퍼가 가득 찬 경우 이벤트가 조용히 드롭됩니다. 인증샷 업로드는 빈번하게 발생하지 않으므로 실질적으로 문제가 되진 않지만, 명시적으로 onBufferOverflow = BufferOverflow.DROP_OLDEST를 지정하면 의도가 더 명확해집니다.

💡 의도를 명시적으로 표현하는 제안

     private val _events =
         MutableSharedFlow<Unit>(
             replay = 0,
             extraBufferCapacity = 1,
+            onBufferOverflow = kotlinx.coroutines.channels.BufferOverflow.DROP_OLDEST,
         )

feature/task-certification/src/main/java/com/twix/task_certification/certification/model/TaskCertificationIntent.kt (1)

30-45: Upload Intent에 ByteArray를 직접 담는 것이 적절한지 검토가 필요합니다.

ByteArray는 대용량 데이터일 수 있어 Intent 채널을 통해 전달하기에는 부담이 될 수 있습니다. 또한 data class에서 ByteArray를 사용하면 equals/hashCode를 직접 구현해야 하는 불편함이 따릅니다 (현재 잘 구현해주셨지만요).

대안으로 ViewModel 내부에서 직접 URI → ByteArray 변환 후 업로드하는 방식도 고려해볼 수 있을까요? 그러면 Intent는 가벼운 사용자 액션 단위로 유지되고, 이미지 데이터는 ViewModel 내부에서만 처리됩니다.

현재 구조가 동작상 문제는 없지만, MVI에서 Intent는 가능한 가볍게 유지하는 것이 권장됩니다.

feature/task-certification/src/main/java/com/twix/task_certification/certification/TaskCertificationViewModel.kt (1)

21-30: SavedStateHandle 파라미터 네이밍이 관례와 다릅니다.

saveStateHandle → savedStateHandle가 Android 공식 문서 및 관례적인 네이밍입니다. 작은 부분이지만 일관성을 위해 참고해주세요.

또한, goalId를 찾지 못할 때 IllegalStateException을 던지는 것은 적절합니다. 네비게이션 인자가 반드시 존재해야 하는 상황이니까요. 👍

feature/task-certification/src/main/java/com/twix/task_certification/certification/TaskCertificationScreen.kt (1)

304-321: Preview가 제공되어 있습니다.

기본 상태로 Preview가 구성되어 있어 UI 확인이 가능합니다. 다만, 다양한 상태(캡처 완료, 코멘트 에러 등)에 대한 Preview도 추가하면 더 좋을 것 같습니다. 필요시 나중에 추가해도 괜찮습니다.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)

• main
• develop

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/#52-task-certification-save

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}