[Feature/#131] 제보하기 #139
[Feature/#131] 제보하기 #139
Conversation
…to feature/#131-course-item-ui
Walkthrough지도 오류 제보 기능(타입 선택·내용·첨부 이미지·프리사인드 업로드)과 관련 DTO·엔티티·레포지토리·원격 API를 추가했고, 맵스 UI에 제보 다이얼로그 및 상태/인텐트를 도입했습니다. 디자인시스템에 고정 높이 스크롤 텍스트필드와 탭-아웃사이드 포커스 해제 Modifier도 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant MS as MapsScreen
participant D as ReportPlaceDialog
participant VM as MapsViewModel
participant Repo as MapsRepository
participant RDS as MapsRemoteDataSource
participant S as MapsService
participant S3 as ObjectStorage
U->>MS: 장소 상세에서 오류 제보 클릭
MS->>D: 다이얼로그 표시
U->>D: 유형 선택 / 내용 입력 / 이미지 첨부
D->>VM: ReportWrongPlace(selectedFilesName)
VM->>VM: 첨부 유무 판단
alt 첨부 없음
VM->>Repo: postReportWrongPlace(placeId, ReportRequestEntity(images=[]))
Repo->>RDS: postReportWrongPlace(...)
RDS->>S: POST /api/places/{id}/reports
S-->>RDS: ReportWrongPlaceResponseDto
RDS-->>Repo: ReportResponseEntity
Repo-->>VM: 성공
VM-->>D: 완료 상태 갱신(로티 → 닫기)
else 첨부 있음
VM->>Repo: postPresignedUrl(PresignedUrlsRequestEntity(files))
Repo->>RDS: postPresignedUrl(...)
RDS->>S: POST /files/presigned-urls
S-->>RDS: PresignedUrlsResponseDto
RDS-->>Repo: PresignedUrlsResponseEntity
Repo-->>VM: presignedUrls
par 병렬 업로드
VM->>S3: PUT presignedUrl (파일 스트림 업로드)
VM->>S3: PUT presignedUrl (파일 스트림 업로드)
end
VM->>Repo: postReportWrongPlace(placeId, ReportRequestEntity(imageKeys=temps))
Repo->>RDS: postReportWrongPlace(...)
RDS->>S: POST /api/places/{id}/reports
S-->>RDS: ReportWrongPlaceResponseDto
RDS-->>Repo: ReportResponseEntity
Repo-->>VM: 성공
VM-->>D: 완료 상태 갱신(로티 → 닫기)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 10
🧹 Nitpick comments (27)
feature/maps/build.gradle.kts (1)
14-15: Lottie core 미사용으로implementation(libs.lottie)제거 가능
- feature/maps 모듈 내에 LottieAnimationView, LottieDrawable, LottieCompositionFactory 등 core API 호출이 없습니다.
lottie-compose가 core 라이브러리(libs.lottie)를 transitive로 포함하므로 중복 의존성 제거가 가능합니다.- implementation(libs.lottie) implementation(libs.lottie.compose)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (2)
359-377: 스크롤 애니메이션 최적화 가능현재 커서 위치에 따른 자동 스크롤이 매 텍스트 레이아웃마다 실행됩니다. 이는 불필요한 애니메이션을 트리거할 수 있습니다.
다음과 같이 스크롤이 실제로 필요한 경우에만 실행되도록 최적화할 수 있습니다:
onTextLayout = { layout -> if (viewportHeightPx == 0) return@BasicTextField val caret = layout.getCursorRect(value.length) val extraPadding = 16f val currentTop = scrollState.value.toFloat() val currentBottom = currentTop + viewportHeightPx + var targetScroll: Int? = null when { caret.top < currentTop -> { - val target = (caret.top - extraPadding).coerceAtLeast(0f).toInt() - coroutineScope.launch { scrollState.animateScrollTo(target) } + targetScroll = (caret.top - extraPadding).coerceAtLeast(0f).toInt() } caret.bottom > currentBottom -> { - val target = (caret.bottom - viewportHeightPx + extraPadding).coerceAtLeast(0f).toInt() - coroutineScope.launch { scrollState.animateScrollTo(target) } + targetScroll = (caret.bottom - viewportHeightPx + extraPadding).coerceAtLeast(0f).toInt() } } + + targetScroll?.let { target -> + if (scrollState.value != target) { + coroutineScope.launch { scrollState.animateScrollTo(target) } + } + } }
303-324: 호출부에서 textAlignment·singleLine 값 명시
SolplyRenameCourseTextField에 추가된 두 파라미터가 기본값으로 적용됩니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/RenameCourseBottomSheet.kt의 121행, 134행 호출부를 검토해 의도한 Alignment 및 singleLine 값을 전달하세요.domain/maps/src/main/java/com/teamsolply/solply/maps/model/ReportEntity.kt (1)
9-12: 상태 값에 대한 enum 사용 고려
ReportResponseEntity의status필드가 문자열로 정의되어 있어 타입 안정성이 부족합니다.상태 값을 enum으로 정의하여 타입 안정성을 높이는 것이 좋습니다:
enum class ReportStatus { PENDING, IN_PROGRESS, RESOLVED, REJECTED } data class ReportResponseEntity( val reportId: Long, val status: ReportStatus )domain/maps/src/main/java/com/teamsolply/solply/maps/repository/MapsRepository.kt (1)
44-52: 제보 API 메소드명 일관성 개선
postPresignedUrl과postReportWrongPlace메소드명이 일관성이 부족합니다. 또한 한글 주석보다는 영문 주석을 사용하는 것이 좋습니다.다음과 같이 개선하는 것을 제안합니다:
- // 제보하기 + // Report submission - suspend fun postPresignedUrl( + suspend fun requestPresignedUrls( presignedUrlsRequestEntity: PresignedUrlsRequestEntity ): Result<PresignedUrlsResponseEntity> - suspend fun postReportWrongPlace( + suspend fun submitPlaceReport( placeId: Long, reportRequestEntity: ReportRequestEntity ): Result<ReportResponseEntity>remote/maps/src/main/java/com/teamsolply/solply/maps/service/MapsService.kt (1)
91-95: Presigned URL 엔드포인트 네이밍 일관성
presignedUrls메소드명이 다른 메소드들과 네이밍 컨벤션이 다릅니다. 동사로 시작하는 일관된 네이밍이 필요합니다.- suspend fun presignedUrls( + suspend fun requestPresignedUrls( @Body presignedUrlsRequestDto: PresignedUrlsRequestDto ): BaseResponse<PresignedUrlsResponseDto>data/maps/src/main/java/com/teamsolply/solply/maps/source/MapsRemoteDataSource.kt (1)
45-53: 메서드명 복수형으로 정렬 제안 (postPresignedUrl → postPresignedUrls)요청/응답 DTO가 모두 복수형(PresignedUrls...)이라 메서드명도 복수형이 일관성에 더 맞습니다. 레포/구현/서비스 계층과 함께 리네이밍을 고려해주세요.
적용 예시(diff):
- suspend fun postPresignedUrl( + suspend fun postPresignedUrls( presignedUrlsRequestDto: PresignedUrlsRequestDto ): PresignedUrlsResponseDtodata/maps/src/main/java/com/teamsolply/solply/maps/dto/response/ReportWrongPlaceResponseDto.kt (1)
6-12: status 문자열 대신 enum 사용 고려서버 스펙이 고정된 상태/코드 집합이라면 enum(@serializable)으로 모델링하면 오타/비정상 값에 대한 타입 안전성이 올라갑니다. 또한 필요 시 @SerialName는 JSON 키와 동일하면 생략 가능해 보입니다(팀 컨벤션에 따라 유지도 OK).
data/maps/src/main/java/com/teamsolply/solply/maps/dto/response/PresignedUrlsResponseDto.kt (1)
12-22: 이름 충돌 방지: 내부 아이템 타입에 Dto 접미사 고려도메인 계층에도 동일한 이름(PresignedUrlInfo)이 존재해 사용처에서 FQN/별칭이 필요합니다. DTO 측 아이템을 PresignedUrlInfoDto로 바꾸면 충돌/가독성 이슈가 줄어듭니다. (대상 범위가 넓어지는 변경이라 추후 리팩터로도 충분)
domain/maps/src/main/java/com/teamsolply/solply/maps/model/PresignedUrlEntity.kt (1)
3-5: 도메인 클래스명 File 충돌 우려 → UploadFile 등으로 변경 권장java.io.File 등과의 네이밍 충돌/혼동 가능성이 높습니다. 의미가 드러나는 이름(UploadFile/PresignedFile)으로 바꾸면 안전합니다.
예시(diff):
data class PresignedUrlsRequestEntity( - val files: List<File> + val files: List<UploadFile> ) -data class File( +data class UploadFile( val fileName: String )주의: 호출부 매핑도 함께 수정 필요.
Also applies to: 7-9
data/maps/src/main/java/com/teamsolply/solply/maps/repository/MapsRepositoryImpl.kt (4)
15-18: FQN 제거: import 별칭으로 가독성 개선도메인/DTO 동명 타입 충돌로 FQN을 사용 중입니다. import alias를 쓰면 읽기 쉬워집니다.
적용 예시(diff):
@@ import com.teamsolply.solply.maps.model.PresignedUrlsRequestEntity import com.teamsolply.solply.maps.model.PresignedUrlsResponseEntity import com.teamsolply.solply.maps.model.ReportRequestEntity import com.teamsolply.solply.maps.model.ReportResponseEntity +import com.teamsolply.solply.maps.model.PresignedUrlInfo as PresignedUrlInfoEntity @@ - com.teamsolply.solply.maps.model.PresignedUrlInfo( + PresignedUrlInfoEntity( originalFileName = dto.originalFileName, tempFileKey = dto.tempFileKey, presignedUrl = dto.presignedUrl, expirationSeconds = dto.expirationSeconds )Also applies to: 110-114
102-120: DTO↔Entity 매핑은 Mapper로 분리레포지토리 내부에 매핑 로직이 직접 들어오고 있어 향후 중복·변경 비용이 커집니다. 기존 패턴(toEntity 등)에 맞춰 PresignedUrlsResponseDto → Entity 매퍼 확장함수로 추출을 권장합니다.
102-109: 메서드명 복수형 정렬 제안 (postPresignedUrl → postPresignedUrls)요청/응답이 복수형이므로 레포지토리 메서드명도 복수형이 더 자연스럽습니다. 인터페이스·데이터소스·서비스와 일괄 정렬을 고려해주세요.
122-140: Report 매핑도 Mapper로 추출ReportWrongPlaceResponseDto → ReportResponseEntity 변환도 확장함수로 추출하면 레이어 간 의존성/변환 위치가 명확해지고 테스트가 쉬워집니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/util/ReportImageUrlUtil.kt (2)
13-19: ContentResolver.length() 실패 시 더 명확한 처리 필요
ContentResolver.length()함수가-1L을 반환할 때, Line 44에서require로 예외를 발생시키지만, 사용자에게 더 명확한 에러 메시지를 제공하는 것이 좋을 것 같습니다. 파일을 읽을 수 없거나 권한이 없는 경우를 구분하여 처리하면 디버깅이 용이할 것입니다.private fun ContentResolver.length(uri: Uri): Long { - return query(uri, arrayOf(OpenableColumns.SIZE), null, null, null) - ?.use { c -> - val idx = c.getColumnIndex(OpenableColumns.SIZE) - if (idx >= 0 && c.moveToFirst()) c.getLong(idx) else -1L - } ?: -1L + return try { + query(uri, arrayOf(OpenableColumns.SIZE), null, null, null) + ?.use { c -> + val idx = c.getColumnIndex(OpenableColumns.SIZE) + if (idx >= 0 && c.moveToFirst()) c.getLong(idx) else -1L + } ?: -1L + } catch (e: SecurityException) { + // 권한 문제 + -2L + } }그리고 Line 44에서:
- require(len >= 0) { "Content length unknown; presigned PUT may reject chunked transfer" } + require(len >= 0) { + when (len) { + -2L -> "파일 접근 권한이 없습니다" + else -> "파일 크기를 확인할 수 없습니다" + } + }
52-57: OkHttpClient 인스턴스 재사용 권장매번 새로운
OkHttpClient인스턴스를 생성하는 것은 비효율적입니다. 싱글톤 패턴이나 의존성 주입을 통해 재사용하는 것이 좋습니다.파일 상단에 companion object를 추가하여 client를 재사용하세요:
companion object { private val httpClient = OkHttpClient.Builder() .connectTimeout(30, TimeUnit.SECONDS) .writeTimeout(30, TimeUnit.SECONDS) .readTimeout(30, TimeUnit.SECONDS) .build() }그리고 Line 52에서:
- OkHttpClient().newCall(req).execute().use { resp -> + httpClient.newCall(req).execute().use { resp ->feature/maps/src/main/java/com/teamsolply/solply/maps/MapsContract.kt (1)
104-122: Intent 네이밍 일관성 개선 제안
ReportWrongPlace를 제외한 다른 Intent들은 모두 동작을 나타내는 동사형으로 되어 있습니다. 일관성을 위해SubmitWrongPlaceReport나SendWrongPlaceReport와 같은 이름을 고려해보세요.- data class ReportWrongPlace( + data class SubmitWrongPlaceReport( val selectedFilesName: List<String> ) : MapsIntentfeature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (1)
506-522: 빈 이미지 리스트 처리 로직 확인이미지가 없는 경우와 있는 경우를 분기 처리하는 로직이 명확합니다. 다만, 실패 처리에 대한 TODO 주석이 있는데, 사용자 피드백을 위한 구체적인 처리가 필요합니다.
실패 처리를 위한 스낵바나 다이얼로그 표시 코드를 구현하시겠습니까? 사용자에게 명확한 피드백을 제공하는 것이 중요합니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/PlaceDetailBottomSheet.kt (1)
413-417: 오류 제보 UI에 전용 아이콘 사용 필요
PlaceDetailBottomSheet.kt(라인 413–417)에서ic_snackbar_notification대신 오류 제보용 전용 아이콘으로 교체하세요.feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (4)
118-137: 탭-아웃 포커스 해제 로직 중복 제거 제안
clearFocusOnTapOutside와 직접 구현한pointerInput제스처가 동일 목적(포커스 해제/키보드 숨김)으로 중복됩니다. 유지보수와 이벤트 충돌을 줄이기 위해 하나만 유지하세요. 아래처럼pointerInput블록을 제거하는 쪽이 깔끔합니다.- .pointerInput(Unit) { - awaitEachGesture { - awaitFirstDown( - requireUnconsumed = false, - pass = PointerEventPass.Final - ) - blankFocus.requestFocus() - keyboard?.hide() - waitForUpOrCancellation() - } - }
437-448: Lottie 완료 타이밍 신뢰성 개선(고정 delay 제거, 1회 재생으로 전환)고정 2.5초 delay는 애니메이션 길이/디바이스 성능에 따라 어긋날 수 있습니다. 1회 재생 후 진행률로 완료를 감지하도록 바꾸면 안정적입니다.
- LaunchedEffect(Unit) { - delay(2500) - onFinished() - } - - val composition by rememberLottieComposition(LottieCompositionSpec.RawRes(R.raw.finish_onboarding)) - val progress by animateLottieCompositionAsState(composition) + val composition by rememberLottieComposition(LottieCompositionSpec.RawRes(R.raw.finish_onboarding)) + val progress by animateLottieCompositionAsState(composition, iterations = 1) + LaunchedEffect(progress) { + if (progress >= 1f) onFinished() + }Also applies to: 454-467
356-431: 이미지 제거(삭제) UX 부재선택한 이미지를 해제할 방법이 없습니다. 각 썸네일에 제거 버튼(예: 우상단 X)을 제공해 사용자가 첨부 취소 가능하도록 하는 것을 권장합니다.
470-483: 파일명 유틸의 배치와 보강 제안UI 파일 내부의
ContentResolver.getFileName확장은 재사용 가능 유틸입니다. core/ui 또는 core/common 유틸 모듈로 이동을 권장합니다. 추가로 file:// 스킴(DocumentFile) 처리 보강을 고려해 주세요.feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/RenameCourseBottomSheet.kt (4)
103-113: 닫기 아이콘은 onDismissRequest를 직접 호출하도록 변경닫기 동작에서 포커스/키보드 정리 로직이 보장되도록
onStartRenameCourseClick()대신onDismissRequest()호출이 더 적합합니다.- Icon( + Icon( painter = painterResource(R.drawable.ic_close), modifier = Modifier .align(Alignment.CenterEnd) .minimumInteractiveComponentSize() .customClickable(rippleEnabled = false) { - onStartRenameCourseClick() + onDismissRequest() }, contentDescription = "close_rename_bottomsheet" )
139-149: 완료 버튼 유효성/상태 연동입력 검증과 버튼 상태가 분리되어 있습니다. 최소한 공백이 아닌 입력 여부를 버튼 selected와 onClick 가드에 반영해 주세요.
- SolplyBasicButton( + // 간단한 유효성 예시: 필요 시 길이/패턴 검증으로 확장 + val isValid = newCourseName.isNotBlank() && newCourseIntroduction.isNotBlank() + SolplyBasicButton( text = "완료", onClick = { - onStartRenameCourseClick() + if (!isValid) return@SolplyBasicButton + onStartRenameCourseClick() onStartEditCourseClick() }, modifier = Modifier .padding(start = 20.dp, end = 20.dp, bottom = 36.dp), textPadding = PaddingValues(vertical = 21.dp), - enabledBackgroundColor = SolplyTheme.colors.gray900 + enabledBackgroundColor = SolplyTheme.colors.gray900, + selected = isValid )
75-83: 고정 높이(670.dp) 사용 지양 → 가변 레이아웃/IME 대응고정 높이는 다양한 화면/키보드(IME) 상황에서 잘리는 문제가 생깁니다.
fillMaxHeight(0.9f)또는heightIn(max = 670.dp)+imePadding()조합으로 대응하세요.
79-90: 포커스 해제 로직 중복
clearFocusOnTapOutside와pointerInput제스처가 중복입니다. 하나만 유지해 불필요한 이벤트 처리/충돌을 줄이세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt(7 hunks)core/ui/src/main/java/com/teamsolply/solply/ui/extension/Modifier.kt(3 hunks)data/maps/src/main/java/com/teamsolply/solply/maps/dto/request/PresignedUrlsRequestDto.kt(1 hunks)data/maps/src/main/java/com/teamsolply/solply/maps/dto/request/ReportWrongPlaceRequestDto.kt(1 hunks)data/maps/src/main/java/com/teamsolply/solply/maps/dto/response/PresignedUrlsResponseDto.kt(1 hunks)data/maps/src/main/java/com/teamsolply/solply/maps/dto/response/ReportWrongPlaceResponseDto.kt(1 hunks)data/maps/src/main/java/com/teamsolply/solply/maps/repository/MapsRepositoryImpl.kt(3 hunks)data/maps/src/main/java/com/teamsolply/solply/maps/source/MapsRemoteDataSource.kt(3 hunks)domain/maps/src/main/java/com/teamsolply/solply/maps/model/PresignedUrlEntity.kt(1 hunks)domain/maps/src/main/java/com/teamsolply/solply/maps/model/ReportEntity.kt(1 hunks)domain/maps/src/main/java/com/teamsolply/solply/maps/repository/MapsRepository.kt(2 hunks)feature/maps/build.gradle.kts(1 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/MapsContract.kt(3 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/MapsScreen.kt(5 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt(3 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/EditCourseBottomSheet.kt(0 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/PlaceDetailBottomSheet.kt(5 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/RenameCourseBottomSheet.kt(1 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt(1 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/model/ReportType.kt(1 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/util/ReportImageUrlUtil.kt(1 hunks)remote/maps/src/main/java/com/teamsolply/solply/maps/datasource/MapsRemoteDataSourceImpl.kt(2 hunks)remote/maps/src/main/java/com/teamsolply/solply/maps/service/MapsService.kt(2 hunks)
💤 Files with no reviewable changes (1)
- feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/EditCourseBottomSheet.kt
🧰 Additional context used
🧬 Code graph analysis (4)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (2)
core/ui/src/main/java/com/teamsolply/solply/ui/base/BaseViewModel.kt (1)
reduce(44-46)feature/maps/src/main/java/com/teamsolply/solply/maps/util/ReportImageUrlUtil.kt (1)
uploadToPresignedUrl(36-58)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (4)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/button/SolplyButton.kt (1)
SolplyBasicButton(63-89)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/chip/CheckedCircle.kt (1)
CheckedCircle(17-34)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (1)
SolplyFixedReportTextField(327-390)core/ui/src/main/java/com/teamsolply/solply/ui/image/AdaptationImage.kt (1)
AdaptationImage(15-69)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsScreen.kt (2)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (1)
ReportPlaceDialog(75-229)feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/RenameCourseBottomSheet.kt (1)
RenameCourseBottomSheet(40-152)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/RenameCourseBottomSheet.kt (2)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (1)
SolplyRenameCourseTextField(302-325)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/button/SolplyButton.kt (1)
SolplyBasicButton(63-89)
🔇 Additional comments (13)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (2)
327-390: 새로운 제보 텍스트 필드 구현 확인
SolplyFixedReportTextField구현이 잘 되어 있습니다. 200자 제한, 자동 스크롤, 플레이스홀더 처리가 적절합니다.
105-107: Modifier 체인 조건부 적용 로직 확인
fillMaxHeight()를 조건부로 적용하는 로직이 적절합니다. 단일 라인이 아닌 경우에만 높이를 채우도록 설정되어 있습니다.core/ui/src/main/java/com/teamsolply/solply/ui/extension/Modifier.kt (1)
68-74: 이벤트 처리 방식 차이 확인 후 함수 통합/역할 분리 결정
addFocusCleaner(detectTapGestures)와 clearFocusOnTapOutside(awaitEachGesture)는 키보드 숨김 여부뿐 아니라 제스처 처리 방식도 다릅니다. 두 방식의 동작 차이를 검토한 뒤, keyboard 옵션을 포함한 단일 함수(awaitEachGesture 기반 권장)로 합치거나 각 함수의 역할을 명확히 분리하세요.remote/maps/src/main/java/com/teamsolply/solply/maps/service/MapsService.kt (1)
92-92: 엔드포인트 경로 확인 필요
"files/presigned-urls"경로가 다른 API 경로들과 다르게api/접두사가 없습니다. 의도적인 것인지 확인이 필요합니다.백엔드 API 문서나 스펙을 확인하여 경로가 올바른지 검증하시기 바랍니다.
data/maps/src/main/java/com/teamsolply/solply/maps/dto/request/ReportWrongPlaceRequestDto.kt (1)
6-14: ReportWrongPlaceRequestDto ↔ ReportRequestEntity 매핑 일관성 확인
MapsRepositoryImpl에서 reportType, content, imageKeys가 모두 동일하게 매핑되어 있습니다.remote/maps/src/main/java/com/teamsolply/solply/maps/datasource/MapsRemoteDataSourceImpl.kt (1)
65-75: 신규 API 위임 구현 LGTMService 호출을 그대로 위임하고 .data를 반환하는 기존 패턴과 일관적입니다. 문제 없어 보입니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/model/ReportType.kt (1)
3-11: LGTM! 잘 구조화된 enum 클래스입니다.신고 유형을 명확하게 정의한 enum 클래스로, 각 유형에 적절한 한국어 설명이 포함되어 있습니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/PlaceDetailBottomSheet.kt (1)
399-443: 오류 제보 UI 추가 구현 확인오류 제보 기능을 위한 새로운 UI 블록이 잘 구현되었습니다. 사용자가 잘못된 정보를 발견했을 때 쉽게 제보할 수 있는 직관적인 인터페이스입니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsContract.kt (1)
39-43: 신고 관련 상태 필드 추가 확인제보하기 기능을 위한 새로운 상태 필드들이 적절하게 추가되었습니다.
ReportType,Uri리스트, 콘텐츠 문자열 등 필요한 모든 상태를 관리하고 있습니다.feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (1)
535-550: 비동기 업로드 처리 잘 구현됨
coroutineScope와async/awaitAll을 활용한 병렬 업로드 처리가 효율적으로 구현되었습니다. 모든 업로드가 완료될 때까지 기다리는 로직이 적절합니다.feature/maps/src/main/java/com/teamsolply/solply/maps/MapsScreen.kt (3)
308-334: ReportPlaceDialog 연동 LGTM상태(타입/내용/이미지/Lottie)와 인텐트 연결이 일관적이며, dismiss 흐름도 정상입니다.
278-284: 리포트 다이얼로그 표시 토글 배선 LGTM
PlaceDetailBottomSheet에changeReportPlaceDialogVisibility: (Boolean) -> Unit파라미터가 올바르게 추가된 것을 확인했습니다.
603-605: changeReportPlaceDialogVisibility 콜백이 “오류 제보하기” 버튼에 연결되어 있습니다
PlaceDetailBottomSheet 내customClickable { changeReportPlaceDialogVisibility(true) }로 정상 호출됨 확인했습니다.
| fun Modifier.clearFocusOnTapOutside( | ||
| focusManager: FocusManager, | ||
| keyboard: SoftwareKeyboardController? | ||
| ) = pointerInput(Unit) { | ||
| awaitEachGesture { | ||
| awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Final) | ||
| focusManager.clearFocus(force = true) | ||
| keyboard?.hide() | ||
| waitForUpOrCancellation() | ||
| } | ||
| } |
There was a problem hiding this comment.
키보드 숨기기 로직 개선 필요
현재 구현에서는 터치 다운 시점에 즉시 포커스를 해제하고 키보드를 숨기는데, 이로 인해 사용자가 텍스트 필드 내부를 탭했을 때도 키보드가 닫힐 수 있습니다. 또한 requireUnconsumed = false와 PointerEventPass.Final 설정으로 인해 모든 터치 이벤트에서 동작할 가능성이 있습니다.
다음과 같이 수정하여 텍스트 필드 외부 탭에만 반응하도록 개선하는 것을 권장합니다:
fun Modifier.clearFocusOnTapOutside(
focusManager: FocusManager,
keyboard: SoftwareKeyboardController?
) = pointerInput(Unit) {
awaitEachGesture {
- awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Final)
- focusManager.clearFocus(force = true)
- keyboard?.hide()
- waitForUpOrCancellation()
+ val down = awaitFirstDown(requireUnconsumed = true, pass = PointerEventPass.Main)
+ val up = waitForUpOrCancellation()
+ if (up != null) {
+ focusManager.clearFocus(force = true)
+ keyboard?.hide()
+ }
}
}이렇게 하면:
requireUnconsumed = true로 이미 처리된 이벤트는 무시PointerEventPass.Main으로 일반적인 이벤트 순서 유지- 터치 업/취소 시점에 동작하여 더 자연스러운 UX 제공
📝 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.
| fun Modifier.clearFocusOnTapOutside( | |
| focusManager: FocusManager, | |
| keyboard: SoftwareKeyboardController? | |
| ) = pointerInput(Unit) { | |
| awaitEachGesture { | |
| awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Final) | |
| focusManager.clearFocus(force = true) | |
| keyboard?.hide() | |
| waitForUpOrCancellation() | |
| } | |
| } | |
| fun Modifier.clearFocusOnTapOutside( | |
| focusManager: FocusManager, | |
| keyboard: SoftwareKeyboardController? | |
| ) = pointerInput(Unit) { | |
| awaitEachGesture { | |
| val down = awaitFirstDown(requireUnconsumed = true, pass = PointerEventPass.Main) | |
| val up = waitForUpOrCancellation() | |
| if (up != null) { | |
| focusManager.clearFocus(force = true) | |
| keyboard?.hide() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In core/ui/src/main/java/com/teamsolply/solply/ui/extension/Modifier.kt around
lines 76–86, the current gesture handler clears focus and hides the keyboard on
pointer down with requireUnconsumed = false and PointerEventPass.Final, which
can close the keyboard when tapping inside a text field; change it to
awaitFirstDown(requireUnconsumed = true, pass = PointerEventPass.Main) and move
the focusManager.clearFocus(...) and keyboard?.hide() calls to run only after a
successful waitForUpOrCancellation() (i.e., when up != null), so the handler
ignores already-consumed events, follows the normal event pass ordering, and
only clears focus/hides the keyboard on an actual outside-tap completion.
| data class PresignedUrlsRequestDto( | ||
| @SerialName("files") | ||
| val files: List<FileDto> | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
빈 리스트에 대한 처리 확인
files 리스트가 비어있는 경우에 대한 처리가 명확하지 않습니다.
다음 스크립트로 빈 리스트 검증 로직이 있는지 확인합니다:
🏁 Script executed:
#!/bin/bash
# PresignedUrlsRequestDto 사용 시 빈 리스트 검증 확인
rg -n -B3 -A3 "PresignedUrlsRequestDto" --type kotlin | rg -C3 "isEmpty|isNotEmpty|require|check"Length of output: 0
🏁 Script executed:
#!/bin/bash
# PresignedUrlsRequestDto 인스턴스 생성 지점 및 빈 리스트 처리 로직 검색
rg -nC3 "PresignedUrlsRequestDto\s*\(" --type kotlin
rg -nC3 "files\.(isEmpty|isNotEmpty)" --type kotlin
rg -nC3 "require\(.+files" --type kotlinLength of output: 1811
빈 files 리스트 검증 로직 추가
PresignedUrlsRequestDto 생성 시 files.isNotEmpty() 확인 또는 DTO init 블록에 require(files.isNotEmpty()) 적용 필요.
🤖 Prompt for AI Agents
In
data/maps/src/main/java/com/teamsolply/solply/maps/dto/request/PresignedUrlsRequestDto.kt
around lines 7 to 10, the DTO currently allows an empty files list; add
validation to enforce non-empty input by adding an init block with
require(files.isNotEmpty()) (or equivalent validation) to throw an
IllegalArgumentException when files is empty, ensuring callers cannot construct
the DTO with an empty list.
| @Serializable | ||
| data class FileDto( | ||
| @SerialName("fileName") | ||
| val fileName: String | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
파일명 검증 로직 추가 고려
FileDto의 fileName에 대한 검증이 없어 잘못된 파일명이나 경로 탐색 공격에 취약할 수 있습니다.
파일명 검증을 추가하는 것을 권장합니다:
@Serializable
data class FileDto(
@SerialName("fileName")
val fileName: String
-)
+) {
+ init {
+ require(fileName.isNotBlank()) { "File name cannot be blank" }
+ require(!fileName.contains("..")) { "Invalid file name" }
+ require(fileName.matches(Regex("^[a-zA-Z0-9._-]+$"))) {
+ "File name contains invalid characters"
+ }
+ }
+}📝 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.
| @Serializable | |
| data class FileDto( | |
| @SerialName("fileName") | |
| val fileName: String | |
| ) | |
| @Serializable | |
| data class FileDto( | |
| @SerialName("fileName") | |
| val fileName: String | |
| ) { | |
| init { | |
| require(fileName.isNotBlank()) { "File name cannot be blank" } | |
| require(!fileName.contains("..")) { "Invalid file name" } | |
| require(fileName.matches(Regex("^[a-zA-Z0-9._-]+$"))) { | |
| "File name contains invalid characters" | |
| } | |
| } | |
| } |
| data class ReportRequestEntity( | ||
| val reportType: String, | ||
| val content: String, | ||
| val imageKeys: List<String> | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
엔티티 검증 로직 추가 필요
ReportRequestEntity에서 입력 데이터에 대한 검증이 없습니다. 빈 문자열이나 잘못된 reportType 값이 전달될 수 있습니다.
다음과 같이 검증 로직을 추가하는 것을 권장합니다:
data class ReportRequestEntity(
val reportType: String,
val content: String,
val imageKeys: List<String>
-)
+) {
+ init {
+ require(reportType.isNotBlank()) { "Report type cannot be blank" }
+ require(content.isNotBlank()) { "Content cannot be blank" }
+ require(imageKeys.size <= 10) { "Maximum 10 images allowed" }
+ }
+}📝 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 ReportRequestEntity( | |
| val reportType: String, | |
| val content: String, | |
| val imageKeys: List<String> | |
| ) | |
| data class ReportRequestEntity( | |
| val reportType: String, | |
| val content: String, | |
| val imageKeys: List<String> | |
| ) { | |
| init { | |
| require(reportType.isNotBlank()) { "Report type cannot be blank" } | |
| require(content.isNotBlank()) { "Content cannot be blank" } | |
| require(imageKeys.size <= 10) { "Maximum 10 images allowed" } | |
| } | |
| } |
🤖 Prompt for AI Agents
In domain/maps/src/main/java/com/teamsolply/solply/maps/model/ReportEntity.kt
around lines 3 to 7, the ReportRequestEntity currently has no input validation;
add an init block (or factory/companion validation method) that enforces:
reportType is one of the allowed values (use an enum or whitelist and validate
against it), content is not blank, and imageKeys is non-null with only non-blank
strings (and optionally a max count). On validation failure throw a clear
exception (e.g., IllegalArgumentException) with a descriptive message so callers
get immediate feedback.
| text = "코스 정보 수정", | ||
| color = SolplyTheme.colors.black, | ||
| style = SolplyTheme.typography.head16M, | ||
| modifier = Modifier.align(Alignment.Center) | ||
| ) |
There was a problem hiding this comment.
하드코딩 텍스트를 string resource로 이전
"코스 정보 수정", "코스 이름", "코스 한 줄 소개", "완료" 등의 문자열은 리소스로 분리해 i18n/테마/접근성 대응을 보장해 주세요.
Also applies to: 115-120, 128-133, 140-141
🤖 Prompt for AI Agents
In
feature/maps/src/main/java/com/teamsolply/solply/maps/component/bottomsheet/RenameCourseBottomSheet.kt
around lines 98-102 (also apply to 115-120, 128-133, 140-141), several UI
strings are hardcoded ("코스 정보 수정", "코스 이름", "코스 한 줄 소개", "완료"); extract each
literal into res/values/strings.xml with meaningful keys (e.g.,
rename_course_title, rename_course_label_name, rename_course_label_description,
action_done), then replace the hardcoded text in the Compose UI with
stringResource(R.string.<key>) calls (or appropriate localized retrieval) and
ensure any formatting or accessibility/ARIA content uses those resources as
well; add entries to strings.xml (and other locale files as needed) and run a
build to verify no remaining hardcoded literals.
| Text( | ||
| text = "제보하기", | ||
| color = SolplyTheme.colors.black, | ||
| style = SolplyTheme.typography.head16M | ||
| ) |
There was a problem hiding this comment.
사용자 노출 문자열 하드코딩 → string resource로 이전 필요
다국어/테마/접근성(번역·RTL) 대응을 위해 모든 UI 문자열은 string resource로 관리하세요. 해당 위치의 하드코딩된 텍스트를 리소스로 이동해 주세요: 제목("제보하기"), 본문 지시문, 선택 표시("(선택)"), 버튼("다음"), 감사 메시지 등.
Also applies to: 192-194, 307-312, 320-331, 461-466
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt
Show resolved
Hide resolved
| val resolver = context.contentResolver | ||
| val uris = uiState.value.selectedReportUris | ||
| val byName = uris.associateBy { resolver.getFileName(it) } | ||
|
|
There was a problem hiding this comment.
파일명 매칭 로직의 안정성 확인 필요
getFileName()으로 가져온 파일명과 selectedFilesName의 파일명이 일치한다고 가정하고 있습니다. 파일명이 일치하지 않는 경우에 대한 처리가 필요합니다.
val byName = uris.associateBy { resolver.getFileName(it) }
+
+// 파일명 매칭 검증
+selectedFilesName.forEach { fileName ->
+ if (!byName.containsKey(fileName)) {
+ // 파일명 불일치 에러 처리
+ reduce {
+ copy(reportLottieVisibility = false)
+ }
+ // TODO: 사용자에게 에러 알림
+ return@launch
+ }
+}📝 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.
| val resolver = context.contentResolver | |
| val uris = uiState.value.selectedReportUris | |
| val byName = uris.associateBy { resolver.getFileName(it) } | |
| val resolver = context.contentResolver | |
| val uris = uiState.value.selectedReportUris | |
| val byName = uris.associateBy { resolver.getFileName(it) } | |
| // 파일명 매칭 검증 | |
| selectedFilesName.forEach { fileName -> | |
| if (!byName.containsKey(fileName)) { | |
| // 파일명 불일치 에러 처리 | |
| reduce { | |
| copy(reportLottieVisibility = false) | |
| } | |
| // TODO: 사용자에게 에러 알림 | |
| return@launch | |
| } | |
| } |
🤖 Prompt for AI Agents
In feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt around
lines 531 to 534, the code assumes resolver.getFileName(uri) always returns a
name that matches entries in selectedFilesName; update to handle cases where
getFileName returns null or a non-matching name by: 1) safely handling nulls
(skip or provide a fallback like using the lastPathSegment or querying
OpenableColumns.DISPLAY_NAME), 2) normalizing both names before comparing (trim,
case-insensitive, strip path components and common prefixes), 3) handle
duplicates by grouping URIs by normalized name and selecting the intended one or
logging/collecting ambiguous matches, and 4) add logging or error handling for
URIs that cannot be matched so callers can react rather than silently failing.
| }.onFailure { e -> | ||
| // TODO. 업로드 실패 | ||
| } | ||
| }.onFailure { | ||
| // TODO. presigned url 발급 실패 | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 구현 필요
업로드 실패와 presigned URL 발급 실패에 대한 TODO 주석이 있습니다. 사용자 경험을 위해 구체적인 에러 처리가 필요합니다.
}.onFailure { e ->
- // TODO. 업로드 실패
+ reduce {
+ copy(
+ reportLottieVisibility = false
+ )
+ }
+ postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar)
}
}.onFailure {
- // TODO. presigned url 발급 실패
+ reduce {
+ copy(
+ reportLottieVisibility = false
+ )
+ }
+ postSideEffect(MapsSideEffect.ShowReportUrlErrorSnackBar)
}MapsSideEffect에 다음 추가:
data object ShowReportUploadErrorSnackBar : MapsSideEffect
data object ShowReportUrlErrorSnackBar : MapsSideEffect🤖 Prompt for AI Agents
In feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt around
lines 565 to 570, the failure handlers are left as TODOs for upload failure and
presigned URL issuance failure; replace those TODOs by dispatching the
corresponding MapsSideEffect instances and optionally logging the exception.
Specifically, in the .onFailure { e -> ... } handler for the upload failure call
the ViewModel’s side effect emitter with
MapsSideEffect.ShowReportUploadErrorSnackBar and include the exception in a
debug/error log, and in the outer .onFailure { ... } handler for presigned URL
issuance call the emitter with MapsSideEffect.ShowReportUrlErrorSnackBar and log
the error; ensure you import/use the correct emitter method and keep UI-visible
error reporting minimal (snackbar side-effects) while preserving the exception
for logs.
| override fun writeTo(sink: BufferedSink) { | ||
| context.contentResolver.openInputStream(uri)?.use { input -> | ||
| input.copyTo(sink.outputStream()) | ||
| } | ||
| } |
There was a problem hiding this comment.
스트림 복사 시 예외 처리 보완 필요
writeTo 메서드에서 openInputStream()이 null을 반환하는 경우는 처리하고 있지만, 스트림 복사 중 발생할 수 있는 IOException은 처리하지 않고 있습니다.
override fun writeTo(sink: BufferedSink) {
- context.contentResolver.openInputStream(uri)?.use { input ->
- input.copyTo(sink.outputStream())
- }
+ context.contentResolver.openInputStream(uri)?.use { input ->
+ try {
+ input.copyTo(sink.outputStream())
+ } catch (e: IOException) {
+ throw IOException("파일 업로드 중 오류 발생: ${e.message}", e)
+ }
+ } ?: throw IllegalStateException("파일을 열 수 없습니다: $uri")
}📝 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.
| override fun writeTo(sink: BufferedSink) { | |
| context.contentResolver.openInputStream(uri)?.use { input -> | |
| input.copyTo(sink.outputStream()) | |
| } | |
| } | |
| override fun writeTo(sink: BufferedSink) { | |
| context.contentResolver.openInputStream(uri)?.use { input -> | |
| try { | |
| input.copyTo(sink.outputStream()) | |
| } catch (e: IOException) { | |
| throw IOException("파일 업로드 중 오류 발생: ${e.message}", e) | |
| } | |
| } ?: throw IllegalStateException("파일을 열 수 없습니다: $uri") | |
| } |
🤖 Prompt for AI Agents
In
feature/maps/src/main/java/com/teamsolply/solply/maps/util/ReportImageUrlUtil.kt
around lines 29 to 33, the writeTo method currently opens the InputStream and
calls input.copyTo(...) but does not handle IOExceptions during the copy and
silently does nothing if openInputStream returns null; update it to throw an
IOException when openInputStream(uri) returns null and wrap the copy operation
in a try/catch that catches IOException and rethrows a new IOException (or
propagate) with additional context (e.g., include the uri) so callers receive a
clear error when stream reading/writing fails.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (2)
503-573: Dispatchers.IO에서 직접 UI 상태 변경(reduce/postSideEffect) — 메인 전환 권장현재
viewModelScope.launch(Dispatchers.IO)내부에서reduce/postSideEffect/sendIntent를 호출합니다. Flow/State 자체는 스레드 세이프하나, 일관성을 위해 UI 상태 변경은 Main에서 수행하는 것이 안전합니다.권장: 네트워크/IO는
withContext(Dispatchers.IO)로 감싸고, 상태 변경은 기본 디스패처(Main)에서 실행. 예:viewModelScope.launch { val result = withContext(Dispatchers.IO) { /* 네트워크 실행 */ } reduce { /* UI 상태 변경 */ } }
543-547: OkHttpClient 매 호출 생성 비용 — 재사용/주입 고려
uploadToPresignedUrl이 매 업로드마다OkHttpClient()를 새로 생성합니다(참조: ReportImageUrlUtil.kt). 첨부가 최대 3장이라도 커넥션 풀/스레드 재사용을 위해 싱글턴 클라이언트 주입을 권장합니다.원하시면 core 네트워킹 모듈에서 클라이언트를 제공하도록 DI 바인딩 예시를 드리겠습니다.
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (2)
73-73: abs 사용을 위한 import (선택)위 패치 적용 시
kotlin.math.abs를 명시적으로 import하면 가독성이 좋아집니다.+import kotlin.math.abs
466-479: UI 파일에 배치된 ContentResolver 확장 함수의 계층 결합해당 확장은 VM에서도 사용됩니다. UI 컴포저블 파일이 아닌 공용 유틸(예: core/ui 또는 core/common)로 이동해 계층 결합을 낮추는 것을 권장합니다.
원하시면 파일 분리/패키지 이동 패치를 제안해 드립니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt(3 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (4)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/button/SolplyButton.kt (1)
SolplyBasicButton(63-89)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/chip/CheckedCircle.kt (1)
CheckedCircle(17-34)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (1)
SolplyFixedReportTextField(327-390)core/ui/src/main/java/com/teamsolply/solply/ui/image/AdaptationImage.kt (1)
AdaptationImage(15-69)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (2)
core/ui/src/main/java/com/teamsolply/solply/ui/base/BaseViewModel.kt (2)
reduce(44-46)sendIntent(38-42)feature/maps/src/main/java/com/teamsolply/solply/maps/util/ReportImageUrlUtil.kt (1)
uploadToPresignedUrl(36-58)
🔇 Additional comments (4)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (2)
151-161: 하드코딩 문자열을 string resource로 이전다국어/접근성 대응을 위해 문자열 및 contentDescription은 모두 string resource로 관리하세요.
예시 패치:
- Text( - text = "제보하기", + Text( + text = stringResource(id = R.string.report_title), color = SolplyTheme.colors.black, style = SolplyTheme.typography.head16M )- text = when (pagerState.currentPage) { - 0 -> "다음" - else -> "완료" - }, + text = when (pagerState.currentPage) { + 0 -> stringResource(id = R.string.next) + else -> stringResource(id = R.string.done) + },contentDescription도 동일하게
stringResource로 교체해 주세요.Also applies to: 190-193, 307-312, 320-329, 373-376, 394-397, 458-462
189-223: 비활성 시에도 onClick 실행(회귀) — 즉시 반환 가드 추가버튼이 비활성처럼 보이지만 onClick은 항상 동작합니다. 접근성/UX 일치화를 위해 클릭 초기에 가드를 두세요.
- onClick = { - when (pagerState.currentPage) { - 0 -> if (selectedReportType != ReportType.EMPTY) { - coroutineScope.launch { - pagerState.animateScrollToPage(1) - } - } - - 1 -> { - if (reportContent.isNotEmpty()) { - reportWrongPlace( - selectedUris.map { - context.contentResolver.getFileName( - uri = it - ) - } - ) - } - } - } - }, + onClick = { + val canProceed = when (pagerState.currentPage) { + 0 -> selectedReportType != ReportType.EMPTY + else -> reportContent.isNotEmpty() + } + if (!canProceed) return@SolplyBasicButton + when (pagerState.currentPage) { + 0 -> coroutineScope.launch { pagerState.animateScrollToPage(1) } + 1 -> reportWrongPlace( + selectedUris.map { uri -> + context.contentResolver.getFileName(uri = uri) + } + ) + } + },feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (2)
507-525: 이미지 없이 신고 실패 시에도 에러 피드백 필요현재 onFailure에서 5초 대기 후 다이얼로그를 닫기만 합니다. 사용자에게 실패 원인/재시도를 알려야 합니다. 로티도 즉시 끄는 편이 자연스럽습니다.
적용 예:
- ).onSuccess { + ).onSuccess { delay(2500) sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) - }.onFailure { - // TODO. 신고 실패 처리 - delay(5000) - sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) - } + }.onFailure { e -> + reduce { copy(reportLottieVisibility = false) } + postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar) + }
569-571: presigned URL 발급 실패 TODO 해소 필요발급 실패 시 사용자 피드백과 로티 해제가 누락되어 있습니다. 기존 리뷰에서 제안된 스낵바를 사용하세요.
- }.onFailure { - // TODO. presigned url 발급 실패 - } + }.onFailure { e -> + reduce { copy(reportLottieVisibility = false) } + postSideEffect(MapsSideEffect.ShowReportUrlErrorSnackBar) + }
| fun ContentResolver.getFileName(uri: Uri): String { | ||
| query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor -> | ||
| val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) | ||
| if (idx >= 0 && cursor.moveToFirst()) { | ||
| val name = cursor.getString(idx) | ||
| if (!name.isNullOrBlank()) return name | ||
| } | ||
| } | ||
| uri.path?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') } | ||
| ?.let { return it } | ||
| val mime = getType(uri) | ||
| val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" | ||
| return "image_${System.currentTimeMillis()}.$ext" | ||
| } |
There was a problem hiding this comment.
파일명 fallback이 비결정적(System.currentTimeMillis) — 동일 URI라도 재호출 시 달라짐
시간 기반 이름은 VM 단계의 매칭 실패를 유발할 수 있습니다. URI 기반 해시 등 결정적 규칙으로 변경하세요.
-fun ContentResolver.getFileName(uri: Uri): String {
+fun ContentResolver.getFileName(uri: Uri): String {
query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor ->
val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)
if (idx >= 0 && cursor.moveToFirst()) {
val name = cursor.getString(idx)
if (!name.isNullOrBlank()) return name
}
}
- uri.path?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') }
- ?.let { return it }
- val mime = getType(uri)
- val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg"
- return "image_${System.currentTimeMillis()}.$ext"
+ // 경로 마지막 세그먼트(확장자 포함) 우선
+ uri.lastPathSegment?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') }
+ ?.let { return it }
+ // MIME 기반 확장자 + URI 해시로 결정적 이름 생성
+ val mime = getType(uri)
+ val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg"
+ return "image_${kotlin.math.abs(uri.toString().hashCode())}.$ext"
}📝 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.
| fun ContentResolver.getFileName(uri: Uri): String { | |
| query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor -> | |
| val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) | |
| if (idx >= 0 && cursor.moveToFirst()) { | |
| val name = cursor.getString(idx) | |
| if (!name.isNullOrBlank()) return name | |
| } | |
| } | |
| uri.path?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') } | |
| ?.let { return it } | |
| val mime = getType(uri) | |
| val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" | |
| return "image_${System.currentTimeMillis()}.$ext" | |
| } | |
| fun ContentResolver.getFileName(uri: Uri): String { | |
| query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor -> | |
| val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) | |
| if (idx >= 0 && cursor.moveToFirst()) { | |
| val name = cursor.getString(idx) | |
| if (!name.isNullOrBlank()) return name | |
| } | |
| } | |
| // 경로 마지막 세그먼트(확장자 포함) 우선 | |
| uri.lastPathSegment | |
| ?.substringAfterLast('/') | |
| ?.takeIf { it.isNotBlank() && it.contains('.') } | |
| ?.let { return it } | |
| // MIME 기반 확장자 + URI 해시로 결정적 이름 생성 | |
| val mime = getType(uri) | |
| val ext = MimeTypeMap.getSingleton() | |
| .getExtensionFromMimeType(mime) | |
| ?: "jpg" | |
| return "image_${kotlin.math.abs(uri.toString().hashCode())}.$ext" | |
| } |
🤖 Prompt for AI Agents
In
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt
around lines 466-479, the fallback filename uses System.currentTimeMillis(),
which is non-deterministic; replace that with a deterministic name derived from
the URI so repeated calls for the same URI produce the same filename (for
example, compute a stable hash (SHA-1/SHA-256 or similar) of uri.toString() or
uri.lastPathSegment and append the resolved extension), ensure you still fall
back to a sensible default extension when MIME lookup fails, and return the
composed deterministic filename instead of time-based one.
| mapsRepository.postPresignedUrl( | ||
| presignedUrlsRequestEntity = PresignedUrlsRequestEntity( | ||
| files = selectedFilesName.map { File(it) } | ||
| ) | ||
| ).onSuccess { response -> | ||
| val infos = response.presignedUrlInfos | ||
| val resolver = context.contentResolver | ||
| val uris = uiState.value.selectedReportUris | ||
| val byName = uris.associateBy { resolver.getFileName(it) } | ||
|
|
There was a problem hiding this comment.
파일명 재계산으로 URI 매칭 실패 가능성(치명적) — 시간기반 fallback 이름과 불일치
UI에서 전달한 selectedFilesName을 다시 ContentResolver.getFileName(uri)로 재계산해 매칭하고 있어, DISPLAY_NAME이 없어서 fallback(현재 구현은 System.currentTimeMillis 기반) 이름이 생성된 경우 VM에서 다시 계산한 이름과 불일치가 발생할 수 있습니다. 그 결과 URI for {originalFileName} not found로 업로드 전 단계에서 실패합니다. 중복 파일명(동일 이름 여러 URI)도 현재 Map 덮어쓰기 이슈가 있습니다.
selectedFilesName과 현재 selectedReportUris의 인덱스 순서를 그대로 사용하여 결정적 매핑을 구성하세요(이름 재계산 금지).
적용 예:
- val resolver = context.contentResolver
- val uris = uiState.value.selectedReportUris
- val byName = uris.associateBy { resolver.getFileName(it) }
+ val uris = uiState.value.selectedReportUris
+ // UI에서 생성해 전달된 파일명과 URI를 인덱스 순서로 안정적으로 매핑
+ val byName = selectedFilesName.zip(uris).toMap()
+ // 안전 가드: 응답 크기와 매핑 크기가 다르면 업로드 중단
+ if (byName.size != infos.size) {
+ reduce { copy(reportLottieVisibility = false) }
+ postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar)
+ return@onSuccess
+ }| val result = runCatching { | ||
| coroutineScope { | ||
| infos.map { info -> | ||
| val uri = byName[info.originalFileName] | ||
| ?: error("URI for ${info.originalFileName} not found") | ||
| async { | ||
| uploadToPresignedUrl( | ||
| context = context, | ||
| uri = uri, | ||
| presignedUrl = info.presignedUrl | ||
| ) | ||
| info.tempFileKey | ||
| } | ||
| }.awaitAll() | ||
| } | ||
| } |
There was a problem hiding this comment.
업로드 실패 시 사용자 피드백·상태 복구 onFailure 누락
runCatching { ... }의 실패 분기 처리가 없습니다. 업로드 중 한건이라도 실패하면 로티가 계속 표시된 채로 사용자가 진행불가 상태에 빠질 수 있습니다.
아래처럼 실패 분기를 추가해 로티를 끄고 에러 스낵바를 노출하세요.
val result = runCatching {
coroutineScope {
infos.map { info ->
val uri = byName[info.originalFileName]
?: error("URI for ${info.originalFileName} not found")
async {
uploadToPresignedUrl(
context = context,
uri = uri,
presignedUrl = info.presignedUrl
)
info.tempFileKey
}
}.awaitAll()
}
}
- result.onSuccess { tempKeys ->
+ result.onSuccess { tempKeys ->
mapsRepository.postReportWrongPlace(
placeId = uiState.value.placeDetailInfo.placeId,
reportRequestEntity = ReportRequestEntity(
content = uiState.value.reportContent,
reportType = uiState.value.selectedReportType.name,
imageKeys = tempKeys
)
).onSuccess {
delay(2500)
sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false))
- }.onFailure {
- delay(5000)
- sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false))
- }
+ }.onFailure { e ->
+ reduce { copy(reportLottieVisibility = false) }
+ postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar)
+ }
}
+ result.onFailure { e ->
+ reduce { copy(reportLottieVisibility = false) }
+ postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar)
+ }📝 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.
| val result = runCatching { | |
| coroutineScope { | |
| infos.map { info -> | |
| val uri = byName[info.originalFileName] | |
| ?: error("URI for ${info.originalFileName} not found") | |
| async { | |
| uploadToPresignedUrl( | |
| context = context, | |
| uri = uri, | |
| presignedUrl = info.presignedUrl | |
| ) | |
| info.tempFileKey | |
| } | |
| }.awaitAll() | |
| } | |
| } | |
| val result = runCatching { | |
| coroutineScope { | |
| infos.map { info -> | |
| val uri = byName[info.originalFileName] | |
| ?: error("URI for ${info.originalFileName} not found") | |
| async { | |
| uploadToPresignedUrl( | |
| context = context, | |
| uri = uri, | |
| presignedUrl = info.presignedUrl | |
| ) | |
| info.tempFileKey | |
| } | |
| }.awaitAll() | |
| } | |
| } | |
| result.onSuccess { tempKeys -> | |
| mapsRepository.postReportWrongPlace( | |
| placeId = uiState.value.placeDetailInfo.placeId, | |
| reportRequestEntity = ReportRequestEntity( | |
| content = uiState.value.reportContent, | |
| reportType = uiState.value.selectedReportType.name, | |
| imageKeys = tempKeys | |
| ) | |
| ).onSuccess { | |
| delay(2500) | |
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | |
| }.onFailure { e -> | |
| reduce { copy(reportLottieVisibility = false) } | |
| postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar) | |
| } | |
| } | |
| result.onFailure { e -> | |
| reduce { copy(reportLottieVisibility = false) } | |
| postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar) | |
| } |
| ).onSuccess { | ||
| delay(2500) | ||
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | ||
| }.onFailure { | ||
| delay(5000) | ||
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | ||
| } |
There was a problem hiding this comment.
최종 신고 API 실패 시 UX 개선(스낵바/로티 해제)
성공 시엔 로티 후 닫힘, 실패 시엔 5초 대기 후 닫힘으로 사용자 혼란을 유발합니다. 실패 시에는 로티를 즉시 끄고 에러 스낵바만 노출하는 패턴으로 통일하세요.
- }.onFailure {
- delay(5000)
- sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false))
- }
+ }.onFailure { e ->
+ reduce { copy(reportLottieVisibility = false) }
+ postSideEffect(MapsSideEffect.ShowReportUploadErrorSnackBar)
+ }🤖 Prompt for AI Agents
In feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt around
lines 561–567, the failure branch waits 5s before hiding the report dialog
animation which confuses users; change the onFailure flow to immediately send
ChangeReportPlaceDialogVisibility(visible = false) to stop the Lottie/ dialog,
then show the error snackbar intent (no long delay). Keep the onSuccess branch
as-is (delay 2500 then hide), but onFailure remove delay(5000) and instead
perform sendIntent(ChangeReportPlaceDialogVisibility(visible = false))
immediately followed by the intent that displays the error snackbar.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt(3 hunks)feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (2)
core/ui/src/main/java/com/teamsolply/solply/ui/base/BaseViewModel.kt (2)
reduce(44-46)sendIntent(38-42)feature/maps/src/main/java/com/teamsolply/solply/maps/util/ReportImageUrlUtil.kt (1)
uploadToPresignedUrl(36-58)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (4)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/button/SolplyButton.kt (1)
SolplyBasicButton(63-89)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/chip/CheckedCircle.kt (1)
CheckedCircle(17-34)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (1)
SolplyFixedReportTextField(327-390)core/ui/src/main/java/com/teamsolply/solply/ui/image/AdaptationImage.kt (1)
AdaptationImage(15-69)
🔇 Additional comments (4)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (2)
157-161: 하드코딩된 문자열 리소스화 필요
189-220: 비활성 상태에서의 버튼 클릭 동작 불일치 수정feature/maps/src/main/java/com/teamsolply/solply/maps/MapsViewModel.kt (2)
533-536: 파일명 매칭 로직의 안전성 확인 필요
519-523: 에러 처리 구현 필요Also applies to: 565-571
| fun ContentResolver.getFileName(uri: Uri): String { | ||
| query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor -> | ||
| val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) | ||
| if (idx >= 0 && cursor.moveToFirst()) { | ||
| val name = cursor.getString(idx) | ||
| if (!name.isNullOrBlank()) return name | ||
| } | ||
| } | ||
| uri.path?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') } | ||
| ?.let { return it } | ||
| val mime = getType(uri) | ||
| val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" | ||
| return "image_${System.currentTimeMillis()}.$ext" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ContentResolver.getFileName 안전성 개선 필요
현재 구현은 예외 상황에서 안전하지 않습니다. ContentResolver query가 null을 반환하거나 cursor가 비어있을 수 있습니다. 또한 파일이 실제로 존재하지 않거나 Google Drive 등에서 동적으로 생성되는 콘텐츠일 수 있습니다.
다음 개선사항을 적용하세요:
fun ContentResolver.getFileName(uri: Uri): String {
+ return try {
query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor ->
- val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)
- if (idx >= 0 && cursor.moveToFirst()) {
- val name = cursor.getString(idx)
- if (!name.isNullOrBlank()) return name
+ if (cursor.moveToFirst()) {
+ val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME)
+ if (idx >= 0) {
+ val name = cursor.getString(idx)
+ if (!name.isNullOrBlank()) return@try name
+ }
}
- }
+ null
+ }
+ } catch (e: Exception) {
+ null
+ } ?: run {
uri.path?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') }
- ?.let { return it }
+ ?: run {
+ val mime = getType(uri)
+ val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg"
+ "image_${System.currentTimeMillis()}.$ext"
+ }
+ }
- val mime = getType(uri)
- val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg"
- return "image_${System.currentTimeMillis()}.$ext"
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt
around lines 464 to 477, make getFileName robust by explicitly handling
null/empty cursors and any exceptions from ContentResolver.query: wrap the query
in a try/catch, verify cursor.moveToFirst() before accessing columns, and only
use the display name if non-blank and safe (sanitize remove path traversal and
limit length); if the cursor path/name is unavailable fall back to checking
DocumentsContract/URI authority patterns, attempt a safe openInputStream to
verify content existence, and finally derive an extension from getType with
null-checking and defaulting to "jpg" — ensure all resources are closed and do
not throw on unexpected URI types.
| viewModelScope.launch(Dispatchers.IO) { | ||
| if (uiState.value.selectedReportUris.isEmpty()) { | ||
| mapsRepository.postReportWrongPlace( | ||
| placeId = uiState.value.placeDetailInfo.placeId, | ||
| reportRequestEntity = ReportRequestEntity( | ||
| content = uiState.value.reportContent, | ||
| reportType = uiState.value.selectedReportType.name, | ||
| imageKeys = emptyList() | ||
| ) | ||
| ).onSuccess { | ||
| delay(2500) | ||
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | ||
| }.onFailure { | ||
| // TODO. 신고 실패 처리 | ||
| delay(5000) | ||
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | ||
| } | ||
| return@launch | ||
| } |
There was a problem hiding this comment.
비동기 작업에서의 UI 상태 접근 안정성 개선 필요
IO Dispatcher에서 uiState.value에 직접 접근하고 있습니다. 이는 메인 스레드가 아닌 곳에서 UI 상태에 접근하는 것으로 동시성 문제가 발생할 수 있습니다.
UI 상태는 함수 시작 시점에 캡처하여 사용하세요:
private fun presignedUrl(
selectedFilesName: List<String>
) {
viewModelScope.launch(Dispatchers.IO) {
- if (uiState.value.selectedReportUris.isEmpty()) {
+ val currentState = uiState.value
+ if (currentState.selectedReportUris.isEmpty()) {
mapsRepository.postReportWrongPlace(
- placeId = uiState.value.placeDetailInfo.placeId,
+ placeId = currentState.placeDetailInfo.placeId,
reportRequestEntity = ReportRequestEntity(
- content = uiState.value.reportContent,
- reportType = uiState.value.selectedReportType.name,
+ content = currentState.reportContent,
+ reportType = currentState.selectedReportType.name,
imageKeys = emptyList()
)
).onSuccess {또한 Line 534, 554-559에서도 동일하게 적용하세요:
+ val reportUris = currentState.selectedReportUris
+ val reportContent = currentState.reportContent
+ val reportType = currentState.selectedReportType.name
+ val placeId = currentState.placeDetailInfo.placeId
// ... 이후 코드에서 uiState.value 대신 캡처된 값들 사용📝 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.
| viewModelScope.launch(Dispatchers.IO) { | |
| if (uiState.value.selectedReportUris.isEmpty()) { | |
| mapsRepository.postReportWrongPlace( | |
| placeId = uiState.value.placeDetailInfo.placeId, | |
| reportRequestEntity = ReportRequestEntity( | |
| content = uiState.value.reportContent, | |
| reportType = uiState.value.selectedReportType.name, | |
| imageKeys = emptyList() | |
| ) | |
| ).onSuccess { | |
| delay(2500) | |
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | |
| }.onFailure { | |
| // TODO. 신고 실패 처리 | |
| delay(5000) | |
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | |
| } | |
| return@launch | |
| } | |
| viewModelScope.launch(Dispatchers.IO) { | |
| val currentState = uiState.value | |
| if (currentState.selectedReportUris.isEmpty()) { | |
| mapsRepository.postReportWrongPlace( | |
| placeId = currentState.placeDetailInfo.placeId, | |
| reportRequestEntity = ReportRequestEntity( | |
| content = currentState.reportContent, | |
| reportType = currentState.selectedReportType.name, | |
| imageKeys = emptyList() | |
| ) | |
| ).onSuccess { | |
| delay(2500) | |
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | |
| }.onFailure { | |
| // TODO. 신고 실패 처리 | |
| delay(5000) | |
| sendIntent(MapsIntent.ChangeReportPlaceDialogVisibility(visible = false)) | |
| } | |
| return@launch | |
| } |
…to feature/#131-course-item-ui
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (1)
124-134: 포커스 해제 로직 중복(gesture vs clearFocusOnTapOutside)clearFocusOnTapOutside가 이미 외부 탭 포커스 해제/키보드 숨김을 처리한다면 pointerInput 블록은 중복일 수 있습니다. 유지가 필요 없다면 제거해 단순화하세요.
- .pointerInput(Unit) { - awaitEachGesture { - awaitFirstDown( - requireUnconsumed = false, - pass = PointerEventPass.Final - ) - blankFocus.requestFocus() - keyboard?.hide() - waitForUpOrCancellation() - } - } + // clearFocusOnTapOutside로 충분하다면 아래 제스처 블록은 제거 가능
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (4)
core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/button/SolplyButton.kt (1)
SolplyBasicButton(63-89)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/chip/CheckedCircle.kt (1)
CheckedCircle(17-34)core/designsystem/src/main/java/com/teamsolply/solply/designsystem/component/textfield/SolplyTextField.kt (1)
SolplyFixedReportTextField(327-390)core/ui/src/main/java/com/teamsolply/solply/ui/image/AdaptationImage.kt (1)
AdaptationImage(15-69)
🔇 Additional comments (9)
feature/maps/src/main/java/com/teamsolply/solply/maps/component/dialog/ReportPlaceDialog.kt (9)
151-161: 하드코딩된 텍스트/콘텐츠 설명(i18n) → string resource로 이전헤더 타이틀과 뒤로가기 아이콘 contentDescription을 리소스로 전환하세요. 디자인시스템 R과 충돌을 피하려면 maps 모듈 R을 FQCN으로 참조하세요.
- Icon( + Icon( painter = painterResource(R.drawable.ic_back_long_arrow), - contentDescription = "back_arrow", + contentDescription = stringResource(com.teamsolply.solply.maps.R.string.cd_back_arrow), modifier = Modifier.padding(end = 8.dp), tint = SolplyTheme.colors.gray900 ) - Text( - text = "제보하기", + Text( + text = stringResource(com.teamsolply.solply.maps.R.string.report_place_title), color = SolplyTheme.colors.black, style = SolplyTheme.typography.head16M )추가로 상단 import에 stringResource를 추가하세요:
import androidx.compose.ui.res.stringResource
189-222: 비활성 상태에서도 onClick 동작 → 가드 추가 및 중복 조건 제거버튼 색만 바뀌고 클릭은 항상 수행됩니다. canProceed 가드로 즉시 반환하고, 텍스트도 string resource로 이전하세요.
- SolplyBasicButton( - text = when (pagerState.currentPage) { - 0 -> "다음" - else -> "완료" - }, - onClick = { - when (pagerState.currentPage) { - 0 -> if (selectedReportType != ReportType.EMPTY) { - coroutineScope.launch { - pagerState.animateScrollToPage(1) - } - } - - 1 -> { - if (reportContent.isNotEmpty()) { - reportWrongPlace( - selectedUris.map { - context.contentResolver.getFileName( - uri = it - ) - } - ) - } - } - } - }, + SolplyBasicButton( + text = when (pagerState.currentPage) { + 0 -> stringResource(com.teamsolply.solply.maps.R.string.common_next) + else -> stringResource(com.teamsolply.solply.maps.R.string.common_done) + }, + onClick = { + val canProceed = when (pagerState.currentPage) { + 0 -> selectedReportType != ReportType.EMPTY + 1 -> reportContent.isNotEmpty() + else -> false + } + if (!canProceed) return@SolplyBasicButton + + when (pagerState.currentPage) { + 0 -> coroutineScope.launch { pagerState.animateScrollToPage(1) } + 1 -> reportWrongPlace( + selectedUris.map { uri -> + context.contentResolver.getFileName(uri = uri) + } + ) + } + }, modifier = Modifier.padding(start = 20.dp, end = 20.dp, bottom = 72.dp), selected = when (pagerState.currentPage) { 0 -> selectedReportType != ReportType.EMPTY - else -> reportContent.isNotEmpty() + else -> reportContent.isNotEmpty() }, enabledBackgroundColor = SolplyTheme.colors.gray900, disabledBackgroundColor = SolplyTheme.colors.gray300 )
307-312: 하드코딩 텍스트(i18n) → string resource로 이전입력 안내 문구는 리소스로 관리하세요.
- Text( - text = "잘못된 정보에 대한 설명을 입력해주세요", + Text( + text = stringResource(com.teamsolply.solply.maps.R.string.report_content_instruction), modifier = Modifier.padding(bottom = 12.dp), color = SolplyTheme.colors.black, style = SolplyTheme.typography.body16R )
320-329: 하드코딩 텍스트(i18n) → string resource로 이전첨부 안내와 “선택” 표기는 리소스로 전환하세요.
- Text( - text = "관련 자료가 있다면 첨부해주세요", + Text( + text = stringResource(com.teamsolply.solply.maps.R.string.report_attachment_title), modifier = Modifier.padding(end = 4.dp), color = SolplyTheme.colors.black, style = SolplyTheme.typography.body16M ) - Text( - text = "(선택)", + Text( + text = stringResource(com.teamsolply.solply.maps.R.string.common_optional), color = SolplyTheme.colors.gray500, style = SolplyTheme.typography.body16M )
371-376: 이미지 contentDescription 하드코딩(i18n/접근성)미리보기 이미지의 설명도 리소스로 전환하세요. 반복 항목이면 인덱스 포함 또는 의미 없는 경우 null 고려.
- AdaptationImage( - imageUrl = selectedUris[index].toString(), - contentDescription = "selected image", + AdaptationImage( + imageUrl = selectedUris[index].toString(), + contentDescription = stringResource(com.teamsolply.solply.maps.R.string.cd_selected_image), modifier = Modifier.fillMaxSize(), contentScale = ContentScale.Crop )
393-397: 아이콘 contentDescription 하드코딩(i18n/접근성)“사진 추가” 아이콘 라벨을 리소스로 이전하세요.
- Icon( + Icon( painter = painterResource(R.drawable.ic_cross), - contentDescription = "add_picture", + contentDescription = stringResource(com.teamsolply.solply.maps.R.string.cd_add_picture), tint = SolplyTheme.colors.gray400 )
457-462: 하드코딩 텍스트(i18n) → string resource로 이전감사 메시지를 리소스로 전환하세요.
- Text( - text = "소중한 제보 감사합니다!", + Text( + text = stringResource(com.teamsolply.solply.maps.R.string.report_submit_thanks), modifier = Modifier.graphicsLayer { alpha = textAlpha }, color = SolplyTheme.colors.black, style = SolplyTheme.typography.display20Sb )
466-479: 파일명 fallback 비결정성 및 안전성 보완 (시간 기반 → 결정적, 예외 안전)System.currentTimeMillis() 기반 이름은 동일 URI 재호출 시 달라집니다. URI 기반 결정적 규칙과 예외 안전 처리를 적용하세요.
-fun ContentResolver.getFileName(uri: Uri): String { - query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor -> - val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) - if (idx >= 0 && cursor.moveToFirst()) { - val name = cursor.getString(idx) - if (!name.isNullOrBlank()) return name - } - } - uri.path?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') } - ?.let { return it } - val mime = getType(uri) - val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" - return "image_${System.currentTimeMillis()}.$ext" -} +fun ContentResolver.getFileName(uri: Uri): String { + return try { + query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor -> + if (cursor.moveToFirst()) { + val idx = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) + if (idx >= 0) { + val name = cursor.getString(idx) + if (!name.isNullOrBlank()) return@try name + } + } + } + // 경로 세그먼트 기반 우선 + uri.lastPathSegment?.substringAfterLast('/')?.takeIf { it.isNotBlank() && it.contains('.') } + ?: run { + val mime = getType(uri) + val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" + // 동일 URI에 대해 동일 결과를 보장하는 결정적 이름 + "image_${kotlin.math.abs(uri.toString().hashCode())}.$ext" + } + } catch (e: Exception) { + val mime = getType(uri) + val ext = MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" + "image_${kotlin.math.abs(uri.toString().hashCode())}.$ext" + } +}
163-168: 수평 페이저 비활성 스크롤 구성 적절userScrollEnabled=false로 단계적 흐름 제어가 명확합니다. LGTM.
📌 PR 요약
🌱 작업한 내용
🌱 PR 포인트
📸 스크린샷
📮 관련 이슈
Summary by CodeRabbit
신기능
개선
의존성