[API] Feed 메인 화면 API 연결#93
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. Walkthrough전체/내 피드 및 피드 상세 API 모델·서비스·레포지토리 메서드가 추가되고, FeedViewModel에 탭별 상태·페이징·리프레시 로직이 도입되며 UI는 URL 기반 이미지(AsyncImage)로 전환되고 피드 댓글 라우트가 추가됨. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FeedScreen
participant FeedViewModel
participant FeedRepository
participant FeedService
participant Backend
User->>FeedScreen: 진입/새로고침/스크롤 하단 근접
FeedScreen->>FeedViewModel: refreshCurrentTab() / loadMoreFeeds()
alt 탭 = 전체
FeedViewModel->>FeedRepository: getAllFeeds(cursor)
FeedRepository->>FeedService: GET /feeds?cursor=...
else 탭 = 내피드
FeedViewModel->>FeedRepository: getMyFeeds(cursor)
FeedRepository->>FeedService: GET /feeds/mine?cursor=...
end
FeedService->>Backend: 요청
Backend-->>FeedService: BaseResponse<...>
FeedService-->>FeedRepository: 응답
FeedRepository-->>FeedViewModel: Result<...>
FeedViewModel-->>FeedScreen: uiState 업데이트(리스트/로딩/커서)
FeedScreen-->>User: 리스트 렌더링 / 로딩 인디케이터
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (10)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (19)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedOthersScreen.kt (1)
142-142: 미리보기용 URL 전환 OK — Preview 네트워크 의존 최소화 권장URL 전환은 전체 PR의 방향과 일치합니다. 다만 Compose Preview에서 외부 네트워크 의존이 생겨 느려질 수 있으니, 필요 시 Coil의 placeholder/error 또는 로컬 URI(mock 서버/asset)를 고려해 주세요.
Also applies to: 154-154
app/src/main/java/com/texthip/thip/ui/feed/component/ImageViewerModal.kt (2)
72-77: 이미지 로딩 경험 개선(placeholder/crossfade) 및 접근성 보완 제안
- 네트워크 지연 시 깜빡임을 줄이려면 crossfade/placeholder/error를 지정하세요.
- 접근성 측면에서 장식용이 아니라면 contentDescription 설정을 권장합니다. 장식용이면 빈 문자열("")이 더 적절합니다.
다음과 같이 개선할 수 있습니다:
- AsyncImage( - model = imageUrls[page], - contentDescription = null, - contentScale = ContentScale.Fit, // 원본 비율 유지하면서 화면에 맞춤 - modifier = Modifier.fillMaxSize() - ) + AsyncImage( + model = imageUrls[page], + // 장식용이면 "", 의미가 있다면 적절한 문자열/리소스를 지정 + contentDescription = "", + contentScale = ContentScale.Fit, + modifier = Modifier.fillMaxSize() + )추가로 crossfade/placeholder가 필요하면 다음과 같이 확장 가능합니다:
// 필요한 경우 상단에 추가 import androidx.compose.ui.platform.LocalContext import coil.request.ImageRequest AsyncImage( model = ImageRequest.Builder(LocalContext.current) .data(imageUrls[page]) .crossfade(true) .build(), contentDescription = "", contentScale = ContentScale.Fit, modifier = Modifier.fillMaxSize() )
34-41: 빈 리스트 및 초기 인덱스 방어 로직 추가 제안FeedCommentScreen에서
images.take(3)로 잘라낸 리스트에 원본selectedImageIndex를 그대로 넘기면, 선택된 인덱스가 0~2 범위를 벗어날 위험이 있습니다. 공용 컴포저블로서 더 안전하게 처리하기 위해 다음 사항을 반영해 주세요.· 주의할 호출부
- app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt:458
imageUrls = images.take(3), initialIndex = selectedImageIndex다음과 같이 빈 리스트 처리 및 인덱스 범위(coerceIn) 방어 코드를 추가할 것을 권장합니다:
@Composable fun ImageViewerModal( - imageUrls: List<String>, + imageUrls: List<String>, initialIndex: Int = 0, onDismiss: () -> Unit ) { + if (imageUrls.isEmpty()) { + // 빈 목록일 땐 즉시 닫기 + onDismiss() + return + } val pagerState = rememberPagerState( - initialPage = initialIndex, + initialPage = initialIndex.coerceIn(0, imageUrls.lastIndex), pageCount = { imageUrls.size } )app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (2)
159-172: 썸네일 이미지 a11y·로딩 경험 개선 제안
- 장식용 이미지라면 contentDescription을 빈 문자열로 두고, 그렇지 않다면 의미 있는 설명을 제공하세요.
- crossfade/placeholder/error 지정으로 UX를 개선할 수 있습니다.
가능한 최소 변경 예시는 아래와 같습니다:
- AsyncImage( - model = imageUrl, - contentDescription = null, + AsyncImage( + model = imageUrl, + contentDescription = "", modifier = Modifier .padding(end = 16.dp) .size(200.dp) .clickable { selectedImageIndex = index showImageViewer = true }, contentScale = ContentScale.Crop )
457-462: 이미지 뷰어에는 전체 목록 전달 권장(현재는 최대 3장으로 제한됨)썸네일은 3장으로 제한하더라도, 뷰어에서는 전체 이미지를 넘겨주는 편이 자연스러운 UX입니다. 현재 index는 0..2 범위이므로 전체 목록으로 바꿔도 초기 인덱스 정합성에는 문제가 없습니다.
- ImageViewerModal( - imageUrls = images.take(3), - initialIndex = selectedImageIndex, - onDismiss = { showImageViewer = false } - ) + ImageViewerModal( + imageUrls = images, + initialIndex = selectedImageIndex, + onDismiss = { showImageViewer = false } + )app/src/main/java/com/texthip/thip/data/model/feeds/response/MyFeedResponse.kt (1)
15-27: 디코딩 안전성 향상: 기본값을 부여해 서버 누락 값에 견고하게 대응하는 것을 권장합니다.
서버에서 특정 필드가 간헐적으로 누락되면 직렬화가 실패할 수 있습니다. contentUrls/likeCount/commentCount/isPublic/isWriter 등에 기본값을 부여하면 복원력이 좋아집니다. (도메인 변환 레이어에서 처리할 수도 있으므로, 선택 사항입니다.)다음과 같이 기본값을 제안드립니다:
data class MyFeedItem( @SerialName("feedId") val feedId: Int, @SerialName("postDate") val postDate: String, @SerialName("isbn") val isbn: String, @SerialName("bookTitle") val bookTitle: String, @SerialName("bookAuthor") val bookAuthor: String, @SerialName("contentBody") val contentBody: String, - @SerialName("contentUrls") val contentUrls: List<String>, - @SerialName("likeCount") val likeCount: Int, - @SerialName("commentCount") val commentCount: Int, - @SerialName("isPublic") val isPublic: Boolean, - @SerialName("isWriter") val isWriter: Boolean + @SerialName("contentUrls") val contentUrls: List<String> = emptyList(), + @SerialName("likeCount") val likeCount: Int = 0, + @SerialName("commentCount") val commentCount: Int = 0, + @SerialName("isPublic") val isPublic: Boolean = true, + @SerialName("isWriter") val isWriter: Boolean = false )추가로, postDate를 도메인 레이어에서 시간 타입(예: Instant/LocalDateTime)으로 변환해 두면 정렬/표시 포맷 일관성 관리가 용이합니다.
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
134-139: 에러 처리 흐름 명확. 중복 최소화를 위한 작은 공통화 제안
runCatching → handleBaseResponse → getOrThrow 체인이 두 메서드에서 동일합니다. 공통 헬퍼로 감싸면 중복이 줄고 테스트 용이성이 좋아집니다. (선택 사항)다음과 같은 헬퍼를 추가하고 메서드를 간결화할 수 있습니다:
// 파일 내 임의 위치(예: 클래스 하단)에 추가 private suspend fun <T> fetch(block: suspend () -> com.texthip.thip.data.model.base.BaseResponse<T>): Result<T?> = runCatching { block().handleBaseResponse().getOrThrow() }그리고 아래처럼 교체:
/** 전체 피드 목록 조회 */ -suspend fun getAllFeeds(cursor: String? = null): Result<AllFeedResponse?> = runCatching { - feedService.getAllFeeds(cursor) - .handleBaseResponse() - .getOrThrow() -} +suspend fun getAllFeeds(cursor: String? = null): Result<AllFeedResponse?> = + fetch { feedService.getAllFeeds(cursor) }app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (2)
66-83: 네트워크 이미지 UX 개선: crossfade/클립 적용 제안
로딩/전환 품질을 위해 crossfade를 켜고, 이미지에 라운드 클립을 적용하면 전체 카드 톤과도 잘 어울립니다. Coil에서 ImageRequest로 교체해도 영향 범위가 작습니다.다음 변경을 제안드립니다:
- feedItem.imageUrls.take(3).forEach { imageUrl -> - AsyncImage( - model = imageUrl, - contentDescription = null, - modifier = Modifier - .padding(end = 10.dp) - .size(100.dp), - contentScale = ContentScale.Crop - ) - } + feedItem.imageUrls.take(3).forEach { imageUrl -> + AsyncImage( + model = coil.request.ImageRequest.Builder(androidx.compose.ui.platform.LocalContext.current) + .data(imageUrl) + .crossfade(true) + .build(), + contentDescription = null, + modifier = Modifier + .padding(end = 10.dp) + .size(100.dp) + .clip(androidx.compose.foundation.shape.RoundedCornerShape(8.dp)), + contentScale = ContentScale.Crop + ) + }추가 import 필요:
- androidx.compose.ui.draw.clip
- androidx.compose.foundation.shape.RoundedCornerShape
- androidx.compose.ui.platform.LocalContext
- coil.request.ImageRequest
103-103: 접근성: 클릭 가능한 아이콘에 contentDescription을 부여해 주세요.
보조기기 사용자를 위해 댓글 아이콘도 의미 있는 설명이 필요합니다. stringResource를 이용한 현지화된 설명을 권장합니다.예: contentDescription = stringResource(R.string.cd_open_comments) (리소스 키는 프로젝트 컨벤션에 맞게 추가)
app/src/main/java/com/texthip/thip/data/model/feeds/response/AllFeedResponse.kt (1)
15-33: ignoreUnknownKeys 설정이 이미 활성성화되어 있습니다
app/src/main/java/com/texthip/thip/data/di/NetworkModule.kt에서ignoreUnknownKeys = true가 확인되었으므로, 서버에서 새로운 필드가 추가되더라도 파싱 오류가 발생하지 않습니다.
필드가 아예 누락될 경우에 대비해 기본값을 두는 것은 여전히 선택적(refactor 권장)입니다. 필요 시 아래와 같이 적용하세요:data class AllFeedItem( @SerialName("feedId") val feedId: Int, @SerialName("creatorId") val creatorId: Int, @SerialName("creatorNickname") val creatorNickname: String, @SerialName("creatorProfileImageUrl") val creatorProfileImageUrl: String?, @SerialName("aliasName") val aliasName: String, @SerialName("aliasColor") val aliasColor: String, @SerialName("postDate") val postDate: String, @SerialName("isbn") val isbn: String, @SerialName("bookTitle") val bookTitle: String, @SerialName("bookAuthor") val bookAuthor: String, @SerialName("contentBody") val contentBody: String, - @SerialName("contentUrls") val contentUrls: List<String>, - @SerialName("likeCount") val likeCount: Int, - @SerialName("commentCount") val commentCount: Int, - @SerialName("isSaved") val isSaved: Boolean, - @SerialName("isLiked") val isLiked: Boolean, - @SerialName("isWriter") val isWriter: Boolean + @SerialName("contentUrls") val contentUrls: List<String> = emptyList(), + @SerialName("likeCount") val likeCount: Int = 0, + @SerialName("commentCount") val commentCount: Int = 0, + @SerialName("isSaved") val isSaved: Boolean = false, + @SerialName("isLiked") val isLiked: Boolean = false, + @SerialName("isWriter") val isWriter: Boolean = false )app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (4)
98-110: 풀투리프레시 중 무한스크롤 트리거 방지 및 재조합 안정성 개선isRefreshing 상태에서도 shouldLoadMore가 true가 될 수 있어 리프레시와 로드모어가 경쟁할 수 있습니다. 또한 remember 키에 listState를 포함해 파생 상태 객체 재생성을 안정화하는 것이 안전합니다.
- val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { + val shouldLoadMore by remember(listState, feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { derivedStateOf { val layoutInfo = listState.layoutInfo val totalItems = layoutInfo.totalItemsCount val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 feedUiState.canLoadMoreCurrentTab && !feedUiState.isLoadingMore && + !feedUiState.isRefreshing && feedUiState.currentTabFeeds.isNotEmpty() && totalItems > 0 && lastVisibleIndex >= totalItems - 3 } }
95-95: 탭 전환 시 스크롤 위치가 유지되지 않습니다 — 탭별 LazyListState 분리 권장현재 두 탭이 하나의 listState를 공유하여 탭 전환 시 스크롤 위치가 섞이며, 전환 직후 의도치 않은 loadMore 트리거가 발생할 수 있습니다. 탭별 상태를 분리해 UX를 개선하세요.
- val listState = rememberLazyListState() + // 탭별 스크롤 상태 분리 + val allListState = rememberLazyListState() + val myListState = rememberLazyListState() + val listState = if (feedUiState.selectedTabIndex == 0) allListState else myListState- state = listState, + state = listState,Also applies to: 214-214
85-89: 사용되지 않는 로컬 상태 제거feedStateList는 어디에도 사용되지 않습니다. 과거 props 기반 렌더링 잔재로 보이니 제거해 혼란을 줄이세요. 함수 파라미터 feeds/totalFeedCount/selectedTabIndex 역시 사용처가 없다면 후속 PR에서 시그니처 정리 권장합니다.
- val feedStateList = remember { - mutableStateListOf<FeedItem>().apply { - addAll(feeds) - } - }
384-393: 좋아요/북마크 클릭 시 낙관적 업데이트 고려TODO 주석만 있고 UI 즉시 반영이 없습니다. 네트워크 왕복 전에 낙관적 토글로 피드백을 주고 실패 시 롤백하는 패턴을 권장합니다. 별도 이슈를 열어드릴까요?
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (2)
51-57: 프로필 이미지 Fallback 전략 재검토 제안현재 null 시 외부 URL(https://example.com/…)을 프리폴백으로 사용합니다. 네트워크 실패/지연 시 플레이스홀더 플리커가 발생할 수 있어 로컬 placeholder 리소스 사용을 권장합니다. 프로젝트에 공용 플레이스홀더가 있다면 그걸로 교체하세요.
92-101: 접근성: 썸네일 contentDescription 고려현재 AsyncImage의 contentDescription이 null입니다. 시각적 의미가 있다면 간단한 설명(예: “피드 이미지”)을 제공하세요. 의미가 전무하다면 null 유지로 스크린리더 제외가 맞습니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (3)
28-39: 무한스크롤 가능 여부에서 리프레시 상태 고려리프레시 중엔 로드모어를 비활성화하는 편이 안전합니다. canLoadMoreAllFeeds / canLoadMoreMyFeeds에 isRefreshing 체크를 추가하세요.
- val canLoadMoreAllFeeds: Boolean get() = !isLoading && !isLoadingMore && !isLastPageAllFeeds - val canLoadMoreMyFeeds: Boolean get() = !isLoading && !isLoadingMore && !isLastPageMyFeeds + val canLoadMoreAllFeeds: Boolean get() = !isLoading && !isLoadingMore && !isRefreshing && !isLastPageAllFeeds + val canLoadMoreMyFeeds: Boolean get() = !isLoading && !isLoadingMore && !isRefreshing && !isLastPageMyFeeds
111-115: 글로벌 로딩 플래그의 경쟁 상태 가능성 — 탭별 로딩 상태 분리 고려isLoading/isLoadingMore가 탭 간 공유됩니다. 두 탭에서 거의 동시에 호출될 경우 먼저 끝난 쪽의 finally 블록이 다른 탭의 로딩 인디케이터를 끌 수 있습니다. 탭별(isLoadingAllFeedsUi, isLoadingMoreAllFeedsUi, isLoadingMyFeedsUi, isLoadingMoreMyFeedsUi)로 UI 상태를 갖는 방향을 제안합니다.
Also applies to: 152-156
30-35: currentTabFeeds의 타입 안정성currentTabFeeds를 List로 노출하면 추후 사용처에서 캐스팅 위험이 있습니다. UI에서 isEmpty() 정도만 본다면 문제없지만, 가능하면 sealed UI 모델(예: UiFeedItem)로 공통화하거나 currentTabFeedsCount(Int) 같은 파생값으로 대체하는 방안을 고려해보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
app/src/main/java/com/texthip/thip/data/model/feeds/response/AllFeedResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/feeds/response/MyFeedResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/service/FeedService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/ImageViewerModal.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt(8 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt(8 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedOthersScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(12 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt(8 hunks)app/src/main/java/com/texthip/thip/ui/mypage/mock/FeedItem.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedFeedViewModel.kt(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
app/src/main/java/com/texthip/thip/ui/common/header/ProfileBar.kt (1)
ProfileBar(29-101)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (1)
MyFeedCard(29-124)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
SavedFeedCard(33-142)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)
🔇 Additional comments (11)
app/src/main/java/com/texthip/thip/ui/mypage/mock/FeedItem.kt (2)
18-18: imageUrls에 대한 null 체크 코드가 없습니다 — 추가 삭제 불필요현재 코드베이스 내에
imageUrls == null또는!= null비교 로직,List<Int>정의/초기화 사용 흔적이 전혀 발견되지 않았습니다. null 체크는 이미 모두 제거된 상태이므로 추가 작업이 필요 없습니다.
5-5: 프로필 이미지 타입 전환(Int → String URL) — 호출부 잔여 확인 완료
rg 검색 결과:
- userProfileImage에 R.drawable(Int) 전달 사례 없음
- FeedItem.userProfileImage가 Int로 선언된 흔적 없음
- ImageViewerModal(imageUrls: List) 호출부 모두 URL 리스트 사용
컴파일/런타임 오류를 일으킬 잔여 호출부는 발견되지 않아 안전합니다.
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/SavedFeedViewModel.kt (1)
14-14: URL 기반 목데이터로의 정렬 완료
- userProfileImage를 URL로 통일
- imageUrls를 List(non-null)로 정리
UI 로직과 일치하며, isNotEmpty() 분기 사용 중이라 동작에 문제 없어 보입니다.
Also applies to: 28-28, 39-39, 43-43, 59-59, 69-69, 73-73
app/src/main/java/com/texthip/thip/data/model/feeds/response/MyFeedResponse.kt (1)
8-12: 페이징 nextCursor null/빈 값 처리 검증 필요
ViewModel 전반에서isLast플래그로만 더 이상 페이지 여부를 판단하고,nextCursor값은 그대로 API 호출 파라미터로 전달합니다. 빈 문자열 반환 시 별도 분기 로직이 없어 잠재적 오류가 발생할 수 있으니, 서버 계약상 페이지가 없을 때 반드시nextCursor=null만 내려오는지 확인해주세요.확인이 필요한 주요 지점:
- FeedViewModel (
getAllFeeds()/getMyFeeds())- MySubscriptionViewModel (
getMyFollowings())- OthersSubscriptionViewModel (
getOthersFollowers())필요 시, API 호출 전에
nextCursor.takeIf { !it.isNullOrBlank() }등으로 빈 문자열을 null로 처리하는 로직을 추가해 주세요.app/src/main/java/com/texthip/thip/data/service/FeedService.kt (1)
30-40: 엔드포인트 정의 및 쿼리 파라미터 처리 적절합니다.
AuthInterceptor.kt → NetworkModule 에
addInterceptor(authInterceptor)로 등록되어 있어Authorization헤더가 자동 주입됨을 확인했습니다.백엔드에서 실제로 쿼리 파라미터명을
cursor로 사용 중인지(예:pageToken/nextCursor등과 혼동 없음) 한 번 더 확인 부탁드립니다.app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
141-146: 무한스크롤 중복 피드 제거 로직 확인 요청실행 결과, 코드 전체에서
distinctBy { it.feedId }같은 중복 제거 로직이 발견되지 않았습니다. 탭 전환·새로고침 경계에서 피드가 중복 삽입될 수 있으니, 아래 사항을 검토 부탁드립니다:
loadMoreFeeds/refreshCurrentTab등 피드 로드 후 기존 리스트에 새 데이터를 병합할 때distinctBy { it.feedId }또는 유사 로직 적용 여부fetch { … }패턴으로 공통화한 이후에도 중복 제거 기능이 정상 동작하는지 테스트app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (2)
36-37: 텍스트 줄 수 제어 로직 명확합니다.
이미지 존재 여부에 따라 3/8줄로 나누는 규칙이 UI 의도와 맞습니다.
129-160: 네트워크 보안 구성 검토 완료
- app/src/main/res/xml/network_security_config.xml에서
3.37.87.117도메인에 대해서만cleartextTrafficPermitted="true"로 허용 중- 사용자 프로필·이미지 URL(
example.com)은 HTTPS 사용으로 차단 이슈 없음- 다른 외부 리소스 도메인도 HTTPS 전용 제공 여부 추가 확인 권장
app/src/main/java/com/texthip/thip/data/model/feeds/response/AllFeedResponse.kt (1)
8-12: 전체 피드 응답 래퍼 구조 적절합니다.
페이징에 필요한 최소 정보만 담아 간결합니다.app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
276-283: 피드 응답 모델에 전체 개수 제공 항목 없음 확인
AllFeedResponse.kt 및 MyFeedResponse.kt에 totalCount/total/size 관련 필드가 존재하지 않으므로, 현재myFeeds.size를 사용하는 방식이 적절합니다. 변경 없이 유지해주세요.app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
70-104: URL 기반 썸네일 전환 및 클릭 처리 LGTM리소스 ID → URL 로 전환, 최대 3장 썸네일 렌더링, 본문 클릭 핸들러 연결 모두 적절합니다. Coil 기본 디스크 캐시가 적용되어 성능도 무난할 것으로 보입니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
98-129: 파일명 충돌 가능성(중대) + 예외 처리 중복 + PII 로그 위험
- 파일명 생성이 System.currentTimeMillis()만 사용되어, 여러 이미지를 같은 ms에 처리할 경우 파일명이 충돌해 앞선 파일이 덮어써질 수 있습니다. 이 경우 여러 파트가 동일 파일을 참조하여 잘못된 이미지가 업로드되는 치명적 버그로 이어질 수 있습니다.
- 함수 내부 runCatching이 외부 runCatching과 중복되어 에러 흐름이 분산됩니다. 상위에서 한 번에 처리하는 편이 가독성과 제어에 유리합니다.
- 예외 메시지에 URI 전체를 포함하면 로그를 통해 PII/민감정보(content:// 경로) 노출 가능성이 있습니다.
아래와 같이 고유 식별자(UUID)로 파일명 충돌을 제거하고, 함수 내부에서는 예외를 던지도록 단순화하며, 예외 메시지에서 구체 URI를 제거하는 것을 권장합니다.
- private fun uriToMultipartBodyPart(uri: Uri, paramName: String, tempFiles: MutableList<File>): MultipartBody.Part? { - return runCatching { + private fun uriToMultipartBodyPart(uri: Uri, paramName: String, tempFiles: MutableList<File>): MultipartBody.Part { // MIME 타입 확인 val mimeType = context.contentResolver.getType(uri) ?: "image/jpeg" val extension = when (mimeType) { "image/png" -> "png" "image/gif" -> "gif" "image/jpeg", "image/jpg" -> "jpg" else -> "jpg" // 기본값 } // 파일명 생성 - val fileName = "feed_image_${System.currentTimeMillis()}.$extension" + val fileName = "feed_image_${System.currentTimeMillis()}_${java.util.UUID.randomUUID()}.$extension" val tempFile = File(context.cacheDir, fileName) // 임시 파일 목록에 추가 tempFiles.add(tempFile) // InputStream을 use 블록으로 안전하게 관리 context.contentResolver.openInputStream(uri)?.use { inputStream -> FileOutputStream(tempFile).use { outputStream -> inputStream.copyTo(outputStream) } - } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") + } ?: throw IllegalStateException("Failed to open input stream for image URI") // MultipartBody.Part 생성 val requestFile = tempFile.asRequestBody(mimeType.toMediaType()) - MultipartBody.Part.createFormData(paramName, fileName, requestFile) - }.onFailure { e -> - e.printStackTrace() - }.getOrNull() + return MultipartBody.Part.createFormData(paramName, fileName, requestFile) }참고:
- 위 변경과 함께, 윗부분(라인 75-87)에서 제안한 “부분 실패 시 전체 실패 처리”와 외부 runCatching 로깅을 적용해 일관성을 맞춰 주세요.
- 로깅은 Timber 사용 시 Timber.e(e, "..."), 미사용 시 현재처럼 android.util.Log.e 사용을 권장합니다(단, URI 원문 노출은 지양).
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
148-154: 파일 삭제 실패(false 반환) 케이스 로깅 필요
- file.delete()는 예외를 던지지 않고 false를 반환하는 경우가 있습니다. 현 구현은 예외만 잡고 false는 무시합니다.
- 추적 용이성을 위해 false 반환시 경고 로그를 남기는 것을 권장합니다.
아래와 같이 보완해 주세요.
tempFiles.forEach { file -> runCatching { if (file.exists()) { - file.delete() + val deleted = file.delete() + if (!deleted) { + android.util.Log.w("FeedRepository", "임시 파일 삭제 실패: ${file.absolutePath}") + } } }.onFailure { e -> - e.printStackTrace() + android.util.Log.e("FeedRepository", "임시 파일 삭제 중 예외 발생", e) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(4 hunks)
🔇 Additional comments (4)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (4)
9-10: 새 응답 모델 import 적절 (LGTM).신규 응답 타입(AllFeedResponse, MyFeedResponse) 노출 및 레이어 경계가 명확합니다.
88-96: 네트워크 호출 후 finally로 임시 파일 정리 — 패턴 적절 (LGTM).예외 발생 여부와 무관하게 temp 파일을 정리하는 구조가 안정적입니다.
131-137: 커서 기반 전체 피드 조회 래핑 구조 적절 (LGTM).runCatching + handleBaseResponse + getOrThrow 조합으로 예외가 Result로 안전히 귀속됩니다.
138-143: 커서 기반 내 피드 조회 래핑 구조 적절 (LGTM).상단 메서드와 일관된 에러 처리 및 반환 타입을 유지하고 있습니다.
| // 이미지 파일들을 MultipartBody.Part로 변환 | ||
| val imageParts = if (imageUris.isNotEmpty()) { | ||
| withContext(Dispatchers.IO) { | ||
| imageUris.mapNotNull { uri -> | ||
| runCatching { | ||
| uriToMultipartBodyPart(uri, "images", tempFiles) | ||
| }.getOrNull() | ||
| } | ||
| } else { | ||
| null | ||
| } | ||
| } else { | ||
| null | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
이미지 파트 생성: 실패를 조용히 무시 + 이중 runCatching + null/empty 의미 모호성
- 현재 mapNotNull + getOrNull로 이미지 변환 실패가 조용히 무시됩니다. 일부 이미지만 업로드되는 상황을 UI가 인지하지 못해 사용자 관점에서 “이미지 일부 유실”로 보일 수 있습니다.
- uriToMultipartBodyPart 내부와 외부에서 runCatching을 중복 사용하고 있어 에러 처리 흐름이 분산됩니다.
- imageUris.isNotEmpty()인데 변환에 모두 실패해 빈 리스트가 생성돼도, 네트워크 호출은 진행됩니다(침묵 실패). 의도라면 주석/로그로 명확히, 아니라면 실패로 처리하는 것이 안전합니다.
아래와 같이 “부분 실패 시 전체 업로드 실패”로 명확화하고(권장), 예외 로깅은 외부에서 단일 책임으로 처리해 주세요. 이 변경은 아래 uriToMultipartBodyPart 개선 제안(예외는 던지고 상위에서 처리)과 함께 적용되는 것을 전제합니다.
적용 제안(해당 범위 내 diff):
- // 이미지 파일들을 MultipartBody.Part로 변환
- val imageParts = if (imageUris.isNotEmpty()) {
- withContext(Dispatchers.IO) {
- imageUris.mapNotNull { uri ->
- runCatching {
- uriToMultipartBodyPart(uri, "images", tempFiles)
- }.getOrNull()
- }
- }
- } else {
- null
- }
+ // 이미지 파일들을 MultipartBody.Part로 변환
+ val imageParts: List<MultipartBody.Part>? = if (imageUris.isNotEmpty()) {
+ val parts = withContext(Dispatchers.IO) {
+ imageUris.mapNotNull { uri ->
+ runCatching {
+ uriToMultipartBodyPart(uri, "images", tempFiles)
+ }.onFailure { e ->
+ android.util.Log.e("FeedRepository", "이미지 파트 생성 실패", e) // 프로젝트에 Timber 사용 시 Timber.e로 교체 권장
+ }.getOrNull()
+ }
+ }
+ // 부분 실패 방지: 입력 개수와 생성된 파트 개수가 다르면 실패 처리
+ if (parts.size != imageUris.size) {
+ throw IllegalStateException("일부 이미지 처리에 실패했습니다. 다시 시도해 주세요.")
+ }
+ parts
+ } else {
+ null
+ }검증 필요사항:
- feedService.createFeed의 이미지 파라미터가 null 허용인지 확인 바랍니다(허용되지 않으면 빈 리스트 전달 또는 오버로드/DTO 조정 필요).
검증 스크립트:
🏁 Script executed:
#!/bin/bash
# FeedService 시그니처와 Timber 사용 여부 확인
set -euo pipefail
# FeedService 정의 및 관련 메서드 확인
rg -n -C3 -g '!**/build/**' --type=kt -P '(interface|object)\s+FeedService\b|@Multipart|@POST\(|createFeed\s*\(|getAllFeeds\s*\(|getMyFeeds\s*\('
# handleBaseResponse 시그니처 확인
rg -n -C2 -g '!**/build/**' --type=kt 'fun\s+handleBaseResponse\('
# Timber 사용 여부 확인
rg -n -C1 -g '!**/build/**' --type=kt '\bTimber\b'Length of output: 194
rg가 --type=kt를 인식하지 못해 파일 패턴을 수정하여 재확인합니다. 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# FeedService 정의 및 createFeed 시그니처 확인
rg -n -C3 -g '!**/build/**' -g '*.kt' -P '(interface|object)\s+FeedService\b|@Multipart|@POST\(|createFeed\s*\('
# handleBaseResponse 시그니처 확인
rg -n -C2 -g '!**/build/**' -g '*.kt' -P 'fun\s+handleBaseResponse\('
# Timber 사용 여부 확인
rg -n -C1 -g '!**/build/**' -g '*.kt' -P '\bTimber\b'Length of output: 14184
handleBaseResponse 확장 함수 정의를 찾지 못했습니다. 아래 스크립트를 실행해 위치를 확인해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# handleBaseResponse 확장 함수 검색
rg -n -C2 -g '!**/build/**' -g '*.kt' 'handleBaseResponse'Length of output: 25793
uriToMultipartBodyPart 함수 정의 내에서 runCatching이 중복 사용되는지 확인합니다. 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# uriToMultipartBodyPart 정의 및 내부 예외 처리 흐름 확인
rg -n -C5 -g '!**/build/**' -g '*.kt' 'fun\s+uriToMultipartBodyPart'Length of output: 1307
uriToMultipartBodyPart 함수 내부에서 예외 처리 흐름(중복 runCatching 여부)을 전체 확인하기 위해 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# uriToMultipartBodyPart 전체 구현 내용 출력 (90~140라인)
sed -n '90,140p' app/src/main/java/com/texthip/thip/data/repository/FeedRepository.ktLength of output: 2041
부분 실패 시 전체 업로드 실패 처리 및 로깅 중앙화 필요
검증 결과:
- FeedService.createFeed의
images: List<MultipartBody.Part>?파라미터는null허용이 확인되었습니다. uriToMultipartBodyPart내부에서 이미runCatching { … }.onFailure { e.printStackTrace() }로 예외를 흘려보내고, 외부 호출부에서도runCatching { … }.getOrNull()을 사용해 중복 처리되고 있습니다.- 코드베이스 내
Timber는 사용되지 않고 있어, 로깅은android.util.Log.e또는 프로젝트 표준 로거로 일원화하는 것이 좋습니다.
아래와 같이 “부분 실패 시 전체 실패 처리” 로직을 적용해 주세요:
- // 이미지 파일들을 MultipartBody.Part로 변환
- val imageParts = if (imageUris.isNotEmpty()) {
- withContext(Dispatchers.IO) {
- imageUris.mapNotNull { uri ->
- runCatching {
- uriToMultipartBodyPart(uri, "images", tempFiles)
- }.getOrNull()
- }
- }
- } else {
- null
- }
+ // 이미지 파일들을 MultipartBody.Part로 변환
+ val imageParts: List<MultipartBody.Part>? = if (imageUris.isNotEmpty()) {
+ val parts = withContext(Dispatchers.IO) {
+ imageUris.mapNotNull { uri ->
+ runCatching {
+ uriToMultipartBodyPart(uri, "images", tempFiles)
+ }.onFailure { e ->
+ android.util.Log.e(
+ "FeedRepository",
+ "이미지 파트 생성 실패 for URI=$uri",
+ e
+ )
+ }.getOrNull()
+ }
+ }
+ // 변환 개수와 URI 개수가 다를 경우 전체 업로드 실패 처리
+ if (parts.size != imageUris.size) {
+ throw IllegalStateException("일부 이미지 처리에 실패했습니다. 다시 시도해 주세요.")
+ }
+ parts
+ } else {
+ null
+ }uriToMultipartBodyPart내부의runCatching은 제거하고, 예외는 상위(위 코드)에서만 처리하도록 단일 책임을 부여합니다.- 로깅은
android.util.Log.e또는 Timber 사용 시Timber.e로 교체해 주세요.
📝 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.
| // 이미지 파일들을 MultipartBody.Part로 변환 | |
| val imageParts = if (imageUris.isNotEmpty()) { | |
| withContext(Dispatchers.IO) { | |
| imageUris.mapNotNull { uri -> | |
| runCatching { | |
| uriToMultipartBodyPart(uri, "images", tempFiles) | |
| }.getOrNull() | |
| } | |
| } else { | |
| null | |
| } | |
| } else { | |
| null | |
| } | |
| // 이미지 파일들을 MultipartBody.Part로 변환 | |
| val imageParts: List<MultipartBody.Part>? = if (imageUris.isNotEmpty()) { | |
| val parts = withContext(Dispatchers.IO) { | |
| imageUris.mapNotNull { uri -> | |
| runCatching { | |
| uriToMultipartBodyPart(uri, "images", tempFiles) | |
| }.onFailure { e -> | |
| android.util.Log.e( | |
| "FeedRepository", | |
| "이미지 파트 생성 실패 for URI=$uri", | |
| e | |
| ) | |
| }.getOrNull() | |
| } | |
| } | |
| // 변환 개수와 URI 개수가 다를 경우 전체 업로드 실패 처리 | |
| if (parts.size != imageUris.size) { | |
| throw IllegalStateException("일부 이미지 처리에 실패했습니다. 다시 시도해 주세요.") | |
| } | |
| parts | |
| } else { | |
| null | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt around
lines 75 to 87, remove the per-item runCatching/getOrNull pattern and instead
let uriToMultipartBodyPart throw on error (remove its internal runCatching),
then perform the entire imageUris.map transformation inside a single try/catch
in this scope so any item failure aborts the whole upload; in the catch log the
exception centrally using android.util.Log.e (or Timber.e if project uses
Timber) with a clear message and return null/propagate as appropriate so partial
failures are treated as full failures and logging is centralized.
There was a problem hiding this comment.
Pull Request Overview
This PR implements API integration for the main feed screen with network-based image loading and infinite scroll functionality. It replaces mock data with actual server API calls for both all feeds and user's own feeds, while transitioning from resource-based to URL-based image handling.
- Updates data models to support server API responses with pagination
- Implements infinite scroll and pull-to-refresh functionality for feeds
- Converts all image handling from drawable resources to network URLs
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| SavedFeedViewModel.kt | Updates mock data to use URL-based profile and content images |
| FeedItem.kt | Changes image fields from resource IDs to URL strings |
| SavedFeedCard.kt | Replaces Image composable with AsyncImage for network loading |
| FeedViewModel.kt | Implements feed API integration with pagination and state management |
| FeedScreen.kt | Adds pull-to-refresh and infinite scroll with API data binding |
| FeedOthersScreen.kt | Updates preview data to use URL-based images |
| FeedCommentScreen.kt | Converts image handling to use AsyncImage |
| MyFeedCard.kt | Replaces Image with AsyncImage for network images |
| ImageViewerModal.kt | Updates to handle URL-based images instead of Painters |
| FeedService.kt | Adds API endpoints for feed list retrieval |
| FeedRepository.kt | Implements feed list API calls with error handling |
| MyFeedResponse.kt | Defines data models for user's feed API response |
| AllFeedResponse.kt | Defines data models for all feeds API response |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| ) | ||
| } | ||
| allFeedsNextCursor = data.nextCursor | ||
| } ?: run { |
There was a problem hiding this comment.
[nitpick] The ?.let { } ?: run { } pattern can be simplified to a single null check with early return or direct assignment. Consider using a more straightforward null handling approach.
| } ?: run { | |
| } else { |
| myFeedsNextCursor = data.nextCursor | ||
| } ?: run { | ||
| updateState { it.copy(isLastPageMyFeeds = true) } | ||
| } |
There was a problem hiding this comment.
Duplicate null handling pattern. The same ?.let { } ?: run { } structure is repeated multiple times in this file. Consider extracting this into a reusable helper function.
| } | |
| response.ifLet( | |
| orElse = { | |
| updateState { it.copy(isLastPageMyFeeds = true) } | |
| }, | |
| block = { data -> | |
| val currentList = if (isInitial) emptyList() else _uiState.value.myFeeds | |
| updateState { | |
| it.copy( | |
| myFeeds = currentList + data.feedList, | |
| error = null, | |
| isLastPageMyFeeds = data.isLast | |
| ) | |
| } | |
| myFeedsNextCursor = data.nextCursor | |
| } | |
| ) |
| inputStream.copyTo(outputStream) | ||
| } | ||
| } ?: return null | ||
| } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") |
There was a problem hiding this comment.
The error message should be more descriptive. Consider including more context about what operation failed and potential causes for debugging purposes.
| } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") | |
| } ?: throw IllegalStateException( | |
| "Failed to open input stream for URI: $uri (paramName: $paramName, mimeType: $mimeType). " + | |
| "Possible causes: file not found, permission denied, or invalid URI." | |
| ) |
| if (imageUrls.size > 1) { | ||
| Text( | ||
| text = stringResource(id = R.string.tag_count, images.size, 3), | ||
| text = stringResource(id = R.string.tag_count, pagerState.currentPage + 1, imageUrls.size), |
There was a problem hiding this comment.
The string resource R.string.tag_count appears to be incorrectly used for image counter display. This should likely use a dedicated image counter string resource instead of a tag-related resource.
| text = stringResource(id = R.string.tag_count, pagerState.currentPage + 1, imageUrls.size), | |
| text = stringResource(id = R.string.image_counter, pagerState.currentPage + 1, imageUrls.size), |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
76-87: 이미지 파트 생성: 부분 실패를 침묵 처리 + 중복 runCatching 제거 필요
- mapNotNull + 내부/외부 runCatching 조합으로 “일부 실패 시 침묵 무시”가 발생합니다.
- 동일 지적이 선행 리뷰에 존재합니다. 로깅 중앙화 + 부분 실패를 전체 실패로 처리하는 쪽이 안전합니다.
아래처럼 수정하면 중복 예외 처리 제거, 부분 실패 방지, 로깅 중앙화가 됩니다.
- // 이미지 파일들을 MultipartBody.Part로 변환 - val imageParts = if (imageUris.isNotEmpty()) { - withContext(Dispatchers.IO) { - imageUris.mapNotNull { uri -> - runCatching { - uriToMultipartBodyPart(uri, "images", tempFiles) - }.getOrNull() - } - } - } else { - null - } + // 이미지 파일들을 MultipartBody.Part로 변환 + val imageParts: List<MultipartBody.Part>? = if (imageUris.isNotEmpty()) { + try { + val parts = withContext(Dispatchers.IO) { + imageUris.map { uri -> + uriToMultipartBodyPart(uri, "images", tempFiles) // 실패 시 예외 발생 + } + } + parts + } catch (e: Exception) { + android.util.Log.e("FeedRepository", "이미지 파트 생성 실패", e) + throw e + } + } else { + null + }- private fun uriToMultipartBodyPart(uri: Uri, paramName: String, tempFiles: MutableList<File>): MultipartBody.Part? { - return runCatching { + private fun uriToMultipartBodyPart(uri: Uri, paramName: String, tempFiles: MutableList<File>): MultipartBody.Part { // MIME 타입 확인 val mimeType = context.contentResolver.getType(uri) ?: "image/jpeg" val extension = when (mimeType) { "image/png" -> "png" "image/gif" -> "gif" "image/jpeg", "image/jpg" -> "jpg" else -> "jpg" // 기본값 } // 파일명 생성 val fileName = "feed_image_${System.currentTimeMillis()}.$extension" val tempFile = File(context.cacheDir, fileName) // 임시 파일 목록에 추가 tempFiles.add(tempFile) // InputStream을 use 블록으로 안전하게 관리 context.contentResolver.openInputStream(uri)?.use { inputStream -> FileOutputStream(tempFile).use { outputStream -> inputStream.copyTo(outputStream) } - } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") + } ?: throw IllegalStateException( + "Failed to open input stream for URI: $uri (paramName=$paramName, mimeType=$mimeType)" + ) // MultipartBody.Part 생성 val requestFile = tempFile.asRequestBody(mimeType.toMediaType()) - MultipartBody.Part.createFormData(paramName, fileName, requestFile) - }.onFailure { e -> - e.printStackTrace() - }.getOrNull() + return MultipartBody.Part.createFormData(paramName, fileName, requestFile) - } + }Also applies to: 99-130
122-122: 에러 메시지 상세화이 라인의 예외 메시지에 paramName, mimeType 등 컨텍스트를 포함하면 디버깅 효율이 올라갑니다. 위 제안 diff에 반영했습니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
86-86: 탭 인덱스 이원화 제거(단일 소스 유지)로컬 selectedIndex와 ViewModel의 feedUiState.selectedTabIndex를 혼용하면 UI 불일치가 납니다. 이전 리뷰 피드백과 동일합니다. ViewModel 상태 하나로 통일하세요.
- val selectedIndex = rememberSaveable { mutableIntStateOf(feedUiState.selectedTabIndex) } ... - onTabSelected = { - selectedIndex.intValue = it - feedViewModel.onTabSelected(it) - } + onTabSelected = feedViewModel::onTabSelected ... - if (selectedIndex.value == 1) { + if (feedUiState.selectedTabIndex == 1) {Also applies to: 223-226, 273-273
🧹 Nitpick comments (13)
app/src/main/java/com/texthip/thip/data/service/FeedService.kt (1)
35-36: cursor 토큰 인코딩 방식 BE 계약 확인 제안cursor 값이 Base64/서명 토큰처럼 이미 인코딩된 문자열일 경우 Retrofit의 기본 인코딩으로 인해 값이 변형될 수 있습니다. 그런 경우에 한해
encoded = true옵션을 고려해주세요. 적용은 BE와 계약 확인 후 진행이 안전합니다.적용이 필요한 경우의 변경 예시는 아래와 같습니다:
- @Query("cursor") cursor: String? = null + @Query(value = "cursor", encoded = true) cursor: String? = nullAlso applies to: 41-42
app/src/main/java/com/texthip/thip/data/model/feed/response/FeedDetailResponse.kt (1)
19-20: 리스트 기본값으로 역직렬화 견고성 보완 권장BE에서 필드 누락이 발생해도 크래시를 피하려면 리스트 필드에 기본값을 두는 것이 안전합니다. 선택사항이지만 장기 유지보수에 유리합니다.
- @SerialName("contentUrls") val contentUrls: List<String>, + @SerialName("contentUrls") val contentUrls: List<String> = emptyList(), ... - @SerialName("tagList") val tagList: List<String> + @SerialName("tagList") val tagList: List<String> = emptyList()Also applies to: 25-25
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt (1)
32-56: 실패 시 기존 데이터 유지 고려(사용자 경험 개선)현재 onFailure에서 feedDetail을 null로 덮어써 UI가 완전히 비게 됩니다. 직전 성공 데이터는 유지하고 에러 배지/스낵바만 노출하는 쪽이 UX에 유리합니다. 예:
- 실패 시: isLoading=false, error=…, feedDetail=기존 값 유지
원한다면 간단한 diff 제안 드리겠습니다.
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
132-151: runCatching이 CancellationException을 삼킵니다(취소 전파 깨짐)세 메서드 모두 runCatching으로 감싸 취소(CancellationException)까지 실패로 포장됩니다. 코루틴 취소는 반드시 재던져야 합니다. 아래처럼 try/catch로 대체해 주세요.
- suspend fun getAllFeeds(cursor: String? = null): Result<AllFeedResponse?> = runCatching { - feedService.getAllFeeds(cursor) - .handleBaseResponse() - .getOrThrow() - } + suspend fun getAllFeeds(cursor: String? = null): Result<AllFeedResponse?> { + return try { + val data = feedService.getAllFeeds(cursor) + .handleBaseResponse() + .getOrThrow() + Result.success(data) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (t: Throwable) { + Result.failure(t) + } + } - suspend fun getMyFeeds(cursor: String? = null): Result<MyFeedResponse?> = runCatching { - feedService.getMyFeeds(cursor) - .handleBaseResponse() - .getOrThrow() - } + suspend fun getMyFeeds(cursor: String? = null): Result<MyFeedResponse?> { + return try { + val data = feedService.getMyFeeds(cursor) + .handleBaseResponse() + .getOrThrow() + Result.success(data) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (t: Throwable) { + Result.failure(t) + } + } - suspend fun getFeedDetail(feedId: Int): Result<FeedDetailResponse?> = runCatching { - feedService.getFeedDetail(feedId) - .handleBaseResponse() - .getOrThrow() - } + suspend fun getFeedDetail(feedId: Int): Result<FeedDetailResponse?> { + return try { + val data = feedService.getFeedDetail(feedId) + .handleBaseResponse() + .getOrThrow() + Result.success(data) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (t: Throwable) { + Result.failure(t) + } + }
156-163: printStackTrace 대신 로깅 API 사용printStackTrace는 무소음/비구조적입니다. Log/Tiber 등 프로젝트 표준 로거를 사용해 주세요.
- }.onFailure { e -> - e.printStackTrace() - } + }.onFailure { e -> + android.util.Log.e("FeedRepository", "임시 파일 삭제 실패: ${file.absolutePath}", e) + }app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (4)
76-77: Preview 실패 가능: 기본값에서 hiltViewModel() 호출 제거 권장@Preview에서 기본 파라미터로 hiltViewModel()을 호출하면 미리보기 크래시가 납니다. viewModel 주입은 Route 컴포저블에서 처리하고, Screen은 의존성을 주입받도록 분리하세요.
- commentList: SnapshotStateList<FeedCommentItem>? = null, - viewModel: FeedDetailViewModel = hiltViewModel() + commentList: SnapshotStateList<FeedCommentItem>? = null, + viewModel: FeedDetailViewModel추가로, 아래 Route를 새로 두고 실제 화면 진입 시 Hilt로 주입하세요(새 파일 또는 동일 파일 하단에 추가):
@Composable fun FeedCommentRoute( feedId: Int, onNavigateBack: () -> Unit = {}, // 기타 파라미터는 필요 시 전달 ) { val vm: FeedDetailViewModel = hiltViewModel() FeedCommentScreen( feedId = feedId, onNavigateBack = onNavigateBack, viewModel = vm ) }
129-129: 미사용 상태 제거feedDetail을 다시 래핑한 feed 상태는 사용되지 않습니다. 혼동을 줄이기 위해 제거하세요.
- val feed = remember { mutableStateOf(feedDetail) }
123-123: 변수 네이밍 컨벤션 준수: CommentList → lowerCamelCaseKotlin 컨벤션상 지역 변수는 lowerCamelCase입니다. CommentList를 comments 등으로 변경 권장합니다. 광범위 수정이므로 지금은 제안만 남깁니다.
Also applies to: 241-241, 264-266, 291-324, 392-397, 403-457
429-447: 닉네임 기반 부모 댓글 탐색은 충돌 위험대댓글 부모 탐색을 nickName 비교로 수행하면 동명이인/닉변 케이스에서 오동작합니다. commentId 등 불변 식별자로 매칭하세요.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (4)
99-112: 무한스크롤 트리거 계산의 remember 키에 listState 포함listState 의존을 명시적으로 포함해 의도치 않은 재생성을 줄이고 안정성을 높이는 것이 좋습니다.
- val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { + val shouldLoadMore by remember(listState, feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { derivedStateOf { val layoutInfo = listState.layoutInfo val totalItems = layoutInfo.totalItemsCount val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 feedUiState.canLoadMoreCurrentTab && !feedUiState.isLoadingMore && feedUiState.currentTabFeeds.isNotEmpty() && totalItems > 0 && lastVisibleIndex >= totalItems - 3 } }
82-83: Preview 친화성: hiltViewModel 기본값 제거 + Route 분리 권장FeedCommentScreen과 동일하게 Preview 환경에서 기본 파라미터의 hiltViewModel() 호출이 크래시를 유발합니다. Route/Screen 분리 패턴을 적용하세요.
- feedViewModel: FeedViewModel = hiltViewModel(), + feedViewModel: FeedViewModel,추가(외부 코드):
@OptIn(ExperimentalMaterial3Api::class) @Composable fun FeedRoute( onNavigateToMySubscription: () -> Unit = {}, onNavigateToFeedWrite: () -> Unit = {}, onNavigateToFeedComment: (Int) -> Unit = {}, // ... ) { val vm: FeedViewModel = hiltViewModel() val mySubVm: MySubscriptionViewModel = hiltViewModel() FeedScreen( onNavigateToMySubscription = onNavigateToMySubscription, onNavigateToFeedWrite = onNavigateToFeedWrite, onNavigateToFeedComment = onNavigateToFeedComment, feedViewModel = vm, mySubscriptionViewModel = mySubVm ) }
87-91: 사용되지 않는 상태 제거: feedStateList입력 파라미터 feeds를 복사한 feedStateList가 이후 사용되지 않습니다. 제거해 불필요한 메모리/리컴포지션을 줄이세요.
- val feedStateList = remember { - mutableStateListOf<FeedItem>().apply { - addAll(feeds) - } - }
189-191: 사용되지 않는 subscriptionUiState 제거collect만 하고 사용하지 않아 불필요한 리컴포지션 트리거가 됩니다. 제거를 권장합니다.
- val subscriptionUiState by mySubscriptionViewModel.uiState.collectAsState() -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
app/src/main/java/com/texthip/thip/data/model/feed/response/FeedDetailResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(4 hunks)app/src/main/java/com/texthip/thip/data/service/FeedService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt(12 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(11 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/FeedNavigationExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/FeedRoutes.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (1)
FeedCommentScreen(63-510)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedCommentScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/buttons/ActionBookButton.kt (1)
ActionBookButton(27-71)app/src/main/java/com/texthip/thip/ui/common/buttons/OptionChipButton.kt (1)
OptionChipButton(30-92)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (1)
MyFeedCard(29-124)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
SavedFeedCard(33-142)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)
🔇 Additional comments (9)
app/src/main/java/com/texthip/thip/data/service/FeedService.kt (1)
32-48: 엔드포인트 추가 전반 LGTM
- 전체/내 피드 목록, 피드 상세 조회 시그니처가 명확하고, Retrofit 어노테이션 사용이 적절합니다.
- cursor 파라미터를 nullable 로 둔 처리도 페이징 초기 진입 시 유용합니다.
app/src/main/java/com/texthip/thip/data/model/feed/response/FeedDetailResponse.kt (1)
6-26: 응답 모델 정의 적절
- 명시적 필드 매핑과 Nullable 처리(creatorProfileImageUrl) 모두 합리적입니다.
- UI 단(FeedCommentScreen)에서 바로 사용하기에 충분히 풍부한 스키마입니다.
app/src/main/java/com/texthip/thip/ui/navigator/extensions/FeedNavigationExtensions.kt (2)
20-21: 누락된 중괄호 보완 LGTM
navigateToFeedWrite종료 중괄호가 명확해졌습니다.
22-25: 댓글 화면 네비게이션 확장 함수 추가 LGTM타입드 라우트(FeedRoutes.Comment)와 일관된 사용입니다.
app/src/main/java/com/texthip/thip/ui/navigator/routes/FeedRoutes.kt (1)
10-10: Feed 댓글 라우트 추가 LGTM
@Serializable data class Comment(val feedId: Int)추가로 타입드 네비게이션 경로가 명확해졌습니다. 상위 그래프와의 결합도 낮아집니다.app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt (2)
6-6: 필요한 import 추가 LGTM
FeedCommentScreen,navigateToFeedCommentimport가 적절히 추가되었습니다.Also applies to: 10-10
38-40: Feed → 댓글 화면 네비게이션 연결 LGTM
onNavigateToFeedComment을 확장 함수로 위임해 그래프 외부에서 호출점이 단순해졌습니다.app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt (2)
14-18: UI 상태 모델 구성 깔끔합니다로딩/데이터/에러를 한곳에서 관리하는 구조가 명확하고, 기본값 설정도 적절합니다.
25-30: StateFlow 노출 패턴이 올바릅니다MutableStateFlow를 내부에 캡슐화하고 asStateFlow로 공개하는 패턴이 잘 적용되어 있습니다.
| composable<FeedRoutes.Comment> { backStackEntry -> | ||
| val route = backStackEntry.arguments?.let { | ||
| FeedRoutes.Comment(it.getInt("feedId")) | ||
| } ?: return@composable | ||
|
|
||
| FeedCommentScreen( | ||
| feedId = route.feedId, | ||
| onNavigateBack = { | ||
| navController.popBackStack() | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
타입드 네비게이션 활용해 안전하게 route 파싱하기
현재 arguments?.getInt("feedId") 접근은 키 누락 시 0이 들어갈 수 있어(기본값) 잘못된 feedId로 화면을 띄울 위험이 있습니다. 타입드 네비게이션의 toRoute<T>()를 사용하면 파싱 실패 시 즉시 오류로 드러나고, 키/타입 불일치를 방지할 수 있습니다.
아래처럼 단순화하는 것을 권장합니다:
- composable<FeedRoutes.Comment> { backStackEntry ->
- val route = backStackEntry.arguments?.let {
- FeedRoutes.Comment(it.getInt("feedId"))
- } ?: return@composable
-
- FeedCommentScreen(
- feedId = route.feedId,
- onNavigateBack = {
- navController.popBackStack()
- }
- )
- }
+ composable<FeedRoutes.Comment> { backStackEntry ->
+ val route: FeedRoutes.Comment = backStackEntry.toRoute()
+ FeedCommentScreen(
+ feedId = route.feedId,
+ onNavigateBack = { navController.popBackStack() }
+ )
+ }toRoute 사용을 위한 import 추가(파일 상단):
import androidx.navigation.toRoute
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
219-221: 탭 상태 단일 소스로 통일한 점 좋습니다이전 리뷰에서 지적된 로컬 selectedIndex vs ViewModel 상태 이원화가 해소되었습니다. Header와 본문이 동일한 상태를 바라봐 UI 일관성이 개선되었습니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (3)
64-75: 탭 재선택 시 초기 로드 중복 호출 — 리스트가 비어있을 때만 초기 로드탭을 다시 눌렀을 때도 매번 초기 로드를 트리거합니다. 이미 데이터가 있으면 스킵하세요.
fun onTabSelected(index: Int) { updateState { it.copy(selectedTabIndex = index) } when (index) { - 0 -> { - loadAllFeeds(isInitial = true) - } - 1 -> { - loadMyFeeds(isInitial = true) - } + 0 -> if (_uiState.value.allFeeds.isEmpty()) { + loadAllFeeds(isInitial = true) + } + 1 -> if (_uiState.value.myFeeds.isEmpty()) { + loadMyFeeds(isInitial = true) + } } }
77-116: loadAllFeeds: isInitial=true에서도 재진입 가능 + 리프레시와 병행 실행 위험
- isInitial=true일 때 로딩 가드가 동작하지 않아 빠르게 재호출 시 병렬 실행됩니다.
- 리프레시 중에도 로드가 가능해 중첩 실행 위험이 있습니다.
아래처럼 가드를 통일하고, 마지막에 isLoading을 집계 방식으로 갱신하세요.
private fun loadAllFeeds(isInitial: Boolean = true) { - if (isLoadingAllFeeds && !isInitial) return - if (_uiState.value.isLastPageAllFeeds && !isInitial) return + if (isLoadingAllFeeds) return + if (_uiState.value.isRefreshing) return + if (!isInitial && _uiState.value.isLastPageAllFeeds) return + if (isInitial && _uiState.value.allFeeds.isNotEmpty()) return viewModelScope.launch { try { isLoadingAllFeeds = true if (isInitial) { updateState { it.copy(isLoading = true, allFeeds = emptyList(), isLastPageAllFeeds = false) } allFeedsNextCursor = null } else { updateState { it.copy(isLoadingMore = true) } } val cursor = if (isInitial) null else allFeedsNextCursor feedRepository.getAllFeeds(cursor).onSuccess { response -> if (response != null) { val currentList = if (isInitial) emptyList() else _uiState.value.allFeeds updateState { it.copy( allFeeds = currentList + response.feedList, error = null, isLastPageAllFeeds = response.isLast ) } allFeedsNextCursor = response.nextCursor } else { updateState { it.copy(isLastPageAllFeeds = true) } } }.onFailure { exception -> updateState { it.copy(error = exception.message) } } } finally { isLoadingAllFeeds = false - updateState { it.copy(isLoading = false, isLoadingMore = false) } + updateState { it.copy( + isLoading = isLoadingAllFeeds || isLoadingMyFeeds, + isLoadingMore = false + ) } } } }
118-157: loadMyFeeds: 동일한 중복 호출/병행 실행 위험 — 같은 가드 적용allFeeds와 대칭으로 동일한 방어 로직을 적용하세요.
private fun loadMyFeeds(isInitial: Boolean = true) { - if (isLoadingMyFeeds && !isInitial) return - if (_uiState.value.isLastPageMyFeeds && !isInitial) return + if (isLoadingMyFeeds) return + if (_uiState.value.isRefreshing) return + if (!isInitial && _uiState.value.isLastPageMyFeeds) return + if (isInitial && _uiState.value.myFeeds.isNotEmpty()) return viewModelScope.launch { try { isLoadingMyFeeds = true if (isInitial) { updateState { it.copy(isLoading = true, myFeeds = emptyList(), isLastPageMyFeeds = false) } myFeedsNextCursor = null } else { updateState { it.copy(isLoadingMore = true) } } val cursor = if (isInitial) null else myFeedsNextCursor feedRepository.getMyFeeds(cursor).onSuccess { response -> if (response != null) { val currentList = if (isInitial) emptyList() else _uiState.value.myFeeds updateState { it.copy( myFeeds = currentList + response.feedList, error = null, isLastPageMyFeeds = response.isLast ) } myFeedsNextCursor = response.nextCursor } else { updateState { it.copy(isLastPageMyFeeds = true) } } }.onFailure { exception -> updateState { it.copy(error = exception.message) } } } finally { isLoadingMyFeeds = false - updateState { it.copy(isLoading = false, isLoadingMore = false) } + updateState { it.copy( + isLoading = isLoadingAllFeeds || isLoadingMyFeeds, + isLoadingMore = false + ) } } } }
🧹 Nitpick comments (8)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (7)
111-116: 탭 전환 시 하단 위치 유지로 즉시 loadMore 유발 가능 — 탭 변경 시 스크롤 Top으로 이동 권장한 개의 listState를 탭 간에 공유하므로, 탭을 바꿨을 때도 스크롤 위치가 유지되어 곧바로 끝 근처로 인식되고 loadMore가 발동될 수 있습니다. 탭 전환 시 상단으로 스크롤하는 가드를 추가하면 깔끔합니다.
LaunchedEffect(shouldLoadMore) { if (shouldLoadMore) { feedViewModel.loadMoreFeeds() } } + + // 탭 변경 시 스크롤 위치 초기화 + LaunchedEffect(feedUiState.selectedTabIndex) { + listState.scrollToItem(0) + }
84-88: 사용되지 않는 로컬 상태 제거feedStateList는 어디에서도 사용되지 않습니다. 혼란을 줄이기 위해 제거하세요.
- val feedStateList = remember { - mutableStateListOf<FeedItem>().apply { - addAll(feeds) - } - }
74-81: 사용되지 않는 매개변수(feeds/totalFeedCount/selectedTabIndex) 및 프리뷰 인자 정리현재 화면은 ViewModel의 상태만 사용합니다. 아래 파라미터는 더 이상 사용되지 않으므로 제거하고 프리뷰 호출부도 정리하세요.
- 함수 시그니처 정리:
nickname: String = "", userRole: String = "", - feeds: List<FeedItem> = emptyList(), - totalFeedCount: Int = 0, - selectedTabIndex: Int = 0, followerProfileImageUrls: List<String> = emptyList(),
- Preview 1 호출부 정리:
FeedScreen( nickname = "ThipUser01", userRole = "문학 칭호", - selectedTabIndex = 1, - feeds = mockFeeds, - totalFeedCount = mockFeeds.size, followerProfileImageUrls = mockFollowerImages, onNavigateToFeedWrite = { } )
- Preview 2 호출부 정리:
FeedScreen( nickname = "ThipUser01", userRole = "문학 칭호", - selectedTabIndex = 0, - feeds = mockFeeds, - totalFeedCount = mockFeeds.size, followerProfileImageUrls = mockFollowerImages, onNavigateToFeedWrite = { } )Also applies to: 474-481, 495-502
80-82: Composable 시그니처에서 hiltViewModel 기본값 사용 지양 — Route/Screen 분리 추천Preview에서 hiltViewModel() 사용은 실패할 가능성이 높고, 테스트·재사용성이 떨어집니다. FeedRoute에서 ViewModel을 주입하고, FeedScreen은 순수 UI + 콜백으로 받는 패턴으로 분리하는 것을 권장합니다.
예시:
@Composable fun FeedRoute(...){ val feedVm: FeedViewModel = hiltViewModel() val subVm: MySubscriptionViewModel = hiltViewModel() FeedScreen(..., feedViewModel = feedVm, mySubscriptionViewModel = subVm) }
136-185: 사용되지 않는 더미 mySubscriptions 제거하드코딩된 목록이 실제 UI 어디에도 사용되지 않습니다. 남겨두면 오해를 유발하니 삭제하세요.
186-186: 미사용 상태 구독 제거subscriptionUiState가 사용되지 않습니다. 불필요한 recomposition을 줄이기 위해 제거하세요.
396-407: 좋아요/북마크 TODO는 ViewModel 액션으로 연결하세요UI에서만 TODO로 남기지 말고 ViewModel에 토글 액션(optimistic update + 실패 롤백)을 추가해 연결하는 것을 권장합니다. 필요 시 구현 도와드릴 수 있습니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (1)
55-57: StateFlow 업데이트의 원자성 향상여러 코루틴에서 연속 업데이트 시 덮어쓰기 위험을 줄이기 위해 MutableStateFlow.update 확장을 사용하세요.
- private fun updateState(update: (FeedUiState) -> FeedUiState) { - _uiState.value = update(_uiState.value) - } + private fun updateState(update: (FeedUiState) -> FeedUiState) { + _uiState.update(update) + }필요한 import:
+import kotlinx.coroutines.flow.update
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(12 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (1)
MyFeedCard(29-124)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
SavedFeedCard(33-142)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
189-200: 초기 로딩 인디케이터 처리 LGTM탭별 초기 로딩 시 컨텐츠를 숨기고 중앙 로딩만 노출하는 UX가 명확합니다.
| val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { | ||
| derivedStateOf { | ||
| val layoutInfo = listState.layoutInfo | ||
| val totalItems = layoutInfo.totalItemsCount | ||
| val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 | ||
|
|
||
| feedUiState.canLoadMoreCurrentTab && | ||
| !feedUiState.isLoadingMore && | ||
| feedUiState.currentTabFeeds.isNotEmpty() && | ||
| totalItems > 0 && | ||
| lastVisibleIndex >= totalItems - 3 | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
리프레시 중에도 무한스크롤 트리거 가능 — isRefreshing 가드 및 remember 단순화 제안
현재 shouldLoadMore는 isRefreshing을 고려하지 않아 풀투리프레시 중에도 loadMore가 중첩 호출될 수 있습니다. 또한 remember 키로 feedUiState.*를 명시했지만, derivedStateOf 내부에서 이미 해당 상태와 listState.layoutInfo를 읽고 있으므로 키를 생략해도 정상 동작합니다.
아래처럼 isRefreshing 가드를 추가하고 remember를 단순화하세요.
- val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) {
- derivedStateOf {
+ val shouldLoadMore by remember {
+ derivedStateOf {
val layoutInfo = listState.layoutInfo
val totalItems = layoutInfo.totalItemsCount
val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0
- feedUiState.canLoadMoreCurrentTab &&
- !feedUiState.isLoadingMore &&
+ feedUiState.canLoadMoreCurrentTab &&
+ !feedUiState.isLoadingMore &&
+ !feedUiState.isRefreshing &&
feedUiState.currentTabFeeds.isNotEmpty() &&
totalItems > 0 &&
lastVisibleIndex >= totalItems - 3
- }
- }
+ }
+ }📝 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 shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { | |
| derivedStateOf { | |
| val layoutInfo = listState.layoutInfo | |
| val totalItems = layoutInfo.totalItemsCount | |
| val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 | |
| feedUiState.canLoadMoreCurrentTab && | |
| !feedUiState.isLoadingMore && | |
| feedUiState.currentTabFeeds.isNotEmpty() && | |
| totalItems > 0 && | |
| lastVisibleIndex >= totalItems - 3 | |
| } | |
| } | |
| val shouldLoadMore by remember { | |
| derivedStateOf { | |
| val layoutInfo = listState.layoutInfo | |
| val totalItems = layoutInfo.totalItemsCount | |
| val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 | |
| feedUiState.canLoadMoreCurrentTab && | |
| !feedUiState.isLoadingMore && | |
| !feedUiState.isRefreshing && | |
| feedUiState.currentTabFeeds.isNotEmpty() && | |
| totalItems > 0 && | |
| lastVisibleIndex >= totalItems - 3 | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt around lines
97 to 109, add a guard to prevent triggering loadMore while a pull-to-refresh is
in progress and simplify the remember usage: include !feedUiState.isRefreshing
in the boolean expression so loadMore won't fire during refreshing, and remove
the explicit keys from remember by using remember { derivedStateOf { ... } }
(letting derivedStateOf observe the needed state reads inside).
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt
Outdated
Show resolved
Hide resolved
| viewModelScope.launch { | ||
| updateState { it.copy(isRefreshing = true) } | ||
|
|
||
| when (_uiState.value.selectedTabIndex) { | ||
| 0 -> refreshAllFeeds() | ||
| 1 -> refreshMyFeeds() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
refreshCurrentTab: 진행 중 로딩/리프레시와 중복 실행 가능 — 가드 추가
리프레시 중복 실행 또는 로딩과의 병행 실행을 방지하세요.
fun refreshCurrentTab() {
- viewModelScope.launch {
+ if (_uiState.value.isRefreshing || isLoadingAllFeeds || isLoadingMyFeeds) return
+ viewModelScope.launch {
updateState { it.copy(isRefreshing = true) }
when (_uiState.value.selectedTabIndex) {
0 -> refreshAllFeeds()
1 -> refreshMyFeeds()
}
}
}📝 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 { | |
| updateState { it.copy(isRefreshing = true) } | |
| when (_uiState.value.selectedTabIndex) { | |
| 0 -> refreshAllFeeds() | |
| 1 -> refreshMyFeeds() | |
| } | |
| } | |
| } | |
| fun refreshCurrentTab() { | |
| if (_uiState.value.isRefreshing || isLoadingAllFeeds || isLoadingMyFeeds) return | |
| viewModelScope.launch { | |
| updateState { it.copy(isRefreshing = true) } | |
| when (_uiState.value.selectedTabIndex) { | |
| 0 -> refreshAllFeeds() | |
| 1 -> refreshMyFeeds() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt around
lines 160 to 168, the refreshCurrentTab coroutine can be run while a refresh or
loading is already in progress; add a guard that checks state flags (e.g.,
isRefreshing or isLoading) and returns early if a refresh/load is active, then
set isRefreshing = true before performing the tab-specific refresh and reset it
to false in a finally block to ensure the flag is cleared on success or error;
keep the existing when on selectedTabIndex (0 -> refreshAllFeeds, 1 ->
refreshMyFeeds).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (2)
96-109: 리프레시 중에도 무한스크롤 트리거 가능 — isRefreshing 가드 추가 및 remember 단순화풀투리프레시 중에도 loadMore가 중첩될 수 있습니다. 이전 코멘트와 동일한 지적입니다. isRefreshing을 가드에 포함하고, remember 키는 생략해도 derivedStateOf 내부의 상태 접근만으로 추적됩니다.
아래처럼 수정 제안드립니다.
- val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { - derivedStateOf { + val shouldLoadMore by remember { + derivedStateOf { val layoutInfo = listState.layoutInfo val totalItems = layoutInfo.totalItemsCount val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 - feedUiState.canLoadMoreCurrentTab && - !feedUiState.isLoadingMore && - feedUiState.currentTabFeeds.isNotEmpty() && - totalItems > 0 && - lastVisibleIndex >= totalItems - 3 + feedUiState.canLoadMoreCurrentTab && + !feedUiState.isLoadingMore && + !feedUiState.isRefreshing && + feedUiState.currentTabFeeds.isNotEmpty() && + totalItems > 0 && + lastVisibleIndex >= totalItems - 3 - } - } + } + }
218-221: 탭 상태 단일 소스 사용 LGTMHeaderMenuBarTab과 본문 분기가 모두 feedUiState.selectedTabIndex를 사용합니다. 이전 이슈(탭 인덱스 이원화) 해소 확인했습니다.
🧹 Nitpick comments (14)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (6)
76-83: 긴 텍스트는 말줄임표로 처리해 가독성 개선하세요현재는 기본 overflow(Clip)라 텍스트가 갑자기 잘립니다. Ellipsis 적용을 권장합니다.
적용 diff:
Text( text = feedItem.content, style = typography.feedcopy_r400_s14_h20, color = colors.White, maxLines = maxLines, modifier = Modifier .fillMaxWidth() .padding(vertical = 16.dp) + , + overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis )추가: 필요 시 import를 추가해 주세요.
import androidx.compose.ui.text.style.TextOverflow
85-101: 썸네일 간격은 Row의 spacedBy로 처리하고 per-item padding 제거 권장현재 각 이미지마다 end padding을 주어 마지막에도 여백이 남습니다. Row 수준에서 간격을 관리하면 마지막 여백이 생기지 않고 코드도 간결해집니다.
적용 diff:
- Row( - modifier = Modifier - .fillMaxWidth() - .padding(bottom = 16.dp), - verticalAlignment = Alignment.CenterVertically, - ) { - feedItem.imageUrls.take(3).forEach { imageUrl -> - AsyncImage( - model = imageUrl, - contentDescription = null, - modifier = Modifier - .padding(end = 10.dp) - .size(100.dp), - contentScale = ContentScale.Crop - ) - } - } + Row( + modifier = Modifier + .fillMaxWidth() + .padding(bottom = 16.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(10.dp) + ) { + feedItem.imageUrls.take(3).forEach { imageUrl -> + AsyncImage( + model = imageUrl, + contentDescription = null, + modifier = Modifier.size(100.dp), + contentScale = ContentScale.Crop + ) + } + }
50-56: 프로필 이미지 null 대체로 외부 URL을 쓰는 것은 지양 — 컴포넌트 내부 placeholder/error로 처리 권장네트워크 실패/지연 시에도 안정적인 UI를 위해, 기본 이미지는 로컬 리소스 placeholder로 처리하는 편이 안전합니다. 여기에서 임의의 외부 URL을 기본값으로 넘기기보다, 빈 문자열을 넘기고 ProfileBar에서 placeholder/error를 처리하는 방식으로 개선을 제안합니다.
적용 diff(본 파일):
- profileImage = feedItem.userProfileImage ?: "https://example.com/image1.jpg", + profileImage = feedItem.userProfileImage.orEmpty(),추가 제안(ProfileBar.kt 내부, 별도 변경 필요): AsyncImage에 placeholder/error/fallback painter를 지정해 주세요. 리소스 식별자는 프로젝트에 있는 플레이스홀더로 맞춰 주시면 됩니다.
69-74: 본문 클릭 영역 범위 확인 필요 — 이미지까지 포함되어 탐색 동작이 과도하게 확장될 수 있음현재 Column 자체가 clickable이라 텍스트 뿐 아니라 썸네일까지 onContentClick이 트리거됩니다. 의도대로라면 텍스트만 이동, 이미지는 이미지 뷰어로 분리되는 게 자연스러울 수 있습니다.
텍스트만 클릭 가능하도록 변경하는 예시:
- Column ( - modifier = Modifier - .clickable { onContentClick() }, - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally - ) { + Column ( + modifier = Modifier, + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally + ) { Text( text = feedItem.content, style = typography.feedcopy_r400_s14_h20, color = colors.White, maxLines = maxLines, modifier = Modifier .fillMaxWidth() - .padding(vertical = 16.dp) + .padding(vertical = 16.dp) + .clickable { onContentClick() } )또는 썸네일에는 별도의 onImageClick 콜백을 두어 이미지 뷰어로 연결하는 패턴도 고려해 주세요.
Also applies to: 80-83
105-107: 하단 여백이 중첩될 가능성 — 카드 외부 레이아웃과 합쳐지면 과도한 공간이 생길 수 있음카드 전체에 20.dp 패딩이 이미 있고, ActionBarButton에도 bottom=20.dp가 더해져 리스트 맥락에서 여백이 두 배가 될 수 있습니다. 의도된 디자인인지 확인 바랍니다. 필요 시 아래처럼 top padding으로 조정 가능합니다.
적용 diff:
- ActionBarButton( - modifier = Modifier.padding(bottom = 20.dp), + ActionBarButton( + modifier = Modifier.padding(top = 16.dp),
121-153: @Preview에서 네트워크 이미지는 개발 경험 저하 가능 — 검사 모드에선 로컬/빈 모델 사용 권장Preview 렌더링 시 원격 URL 요청이 발생하면 느려지거나 실패 로그가 쌓일 수 있습니다. LocalInspectionMode를 사용해 프리뷰 모드에서는 로컬 placeholder나 빈 모델을 쓰도록 분기하는 것을 권장합니다.
예시(아이디어):
import androidx.compose.ui.platform.LocalInspectionMode @Composable private fun SavedFeedCardPrev() { val inPreview = LocalInspectionMode.current val feed1 = FeedItem( // ... userProfileImage = if (inPreview) "" else "https://example.com/profile1.jpg", imageUrls = if (inPreview) emptyList() else listOf(/* ... */) ) // ... }리소스 기반 플레이스홀더를 사용한다면 Preview의 안정성이 더 높아집니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (8)
94-94: 탭 전환 시 스크롤 상태 공유 — UX 의도에 따라 초기화 또는 탭별 상태 분리 고려현재 하나의 LazyListState를 두 탭이 공유합니다. 탭 전환 시 스크롤 위치를 유지할지/초기화할지 명확히 하는 편이 좋습니다. 간단히는 전환 시 최상단으로 스크롤하거나, 고급하게는 탭별로 LazyListState를 분리하세요.
전환 시 초기화 예시:
LaunchedEffect(shouldLoadMore) { if (shouldLoadMore) { feedViewModel.loadMoreFeeds() } } + // 탭 전환 시 스크롤 위치 초기화(선택) + LaunchedEffect(feedUiState.selectedTabIndex) { + listState.scrollToItem(0) + }Also applies to: 111-116
84-88: 미사용 상태(feedStateList) 정리feedStateList는 어디에서도 사용되지 않습니다. 유지하면 혼동만 주므로 제거하세요. (연쇄적으로 feeds/totalFeedCount/selectedTabIndex 매개변수도 실제로는 미사용이므로 후속 PR에서 제거/Deprecated 처리 권장)
- val feedStateList = remember { - mutableStateListOf<FeedItem>().apply { - addAll(feeds) - } - }
136-185: 미사용 더미 데이터(mySubscriptions) 제거실사용되지 않는 하드코딩 리스트입니다. 유지 시 유지보수 비용/혼란만 증가합니다. 안전하게 제거하세요.
- val mySubscriptions = listOf( - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ) - )
186-186: 미사용 구독 UI 상태 수집 제거subscriptionUiState가 사용되지 않습니다. 불필요한 재구성 트리거를 막기 위해 제거하세요.
- val subscriptionUiState by mySubscriptionViewModel.uiState.collectAsState()
80-82: 미사용 MySubscriptionViewModel 파라미터 제거mySubscriptionViewModel은 본문에서 사용되지 않습니다. 시그니처를 단순화하세요. (이 변경 시 import 정리 필요할 수 있음)
- feedViewModel: FeedViewModel = hiltViewModel(), - mySubscriptionViewModel: MySubscriptionViewModel = hiltViewModel() + feedViewModel: FeedViewModel = hiltViewModel()
89-89: 불필요한 rememberCoroutineScope 제거LaunchedEffect 내부에서 다시 scope.launch를 사용할 이유가 없습니다. 아래 리팩터와 함께 scope를 제거하세요.
- val scope = rememberCoroutineScope()
123-133: LaunchedEffect 내 불필요한 중첩 코루틴 제거LaunchedEffect 자체가 코루틴 컨텍스트이므로 scope.launch로 중첩 생성할 필요가 없습니다. 단순화하면 가독성과 제어가 좋아집니다.
- scope.launch { - progress.animateTo( - targetValue = 1f, - animationSpec = tween(durationMillis = 1000, easing = LinearEasing) - ) - delay(500) - if (showProgressBar) { - showProgressBar = false - } - } + progress.animateTo( + targetValue = 1f, + animationSpec = tween(durationMillis = 1000, easing = LinearEasing) + ) + delay(500) + if (showProgressBar) { + showProgressBar = false + }
446-486: Preview에서 hiltViewModel 기본 파라미터로 인한 렌더 실패 가능성 점검Android Studio Preview 환경에선 hiltViewModel() 기본값이 종종 동작하지 않습니다. 로컬 Preview 렌더 확인을 부탁드립니다. 문제가 있다면:
- Route/Screen 분리(FeedRoute에서 hilt 주입, FeedScreen은 uiState/콜백만 받는 순수 컴포저블) 또는
- Preview에서 사용할 Stub/Fake ViewModel을 명시 주입
중 하나로 정리하는 것을 권장합니다.Also applies to: 489-508
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(12 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (1)
MyFeedCard(29-124)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
SavedFeedCard(31-117)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (2)
app/src/main/java/com/texthip/thip/ui/common/header/ProfileBar.kt (1)
ProfileBar(29-101)app/src/main/java/com/texthip/thip/ui/common/buttons/ActionBarButton.kt (1)
ActionBarButton(24-99)
🔇 Additional comments (4)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (2)
41-43: 이미지 존재 여부 기반 maxLines 분기 처리, 적절합니다이미지가 있을 때 3줄, 없을 때 8줄로 제한하는 단순 규칙이 UX에 잘 맞습니다. 나중에 디자인 요구가 바뀌어도 이 한 줄로 쉽게 조정 가능합니다.
107-114: 액션 바 바인딩 적절합니다좋아요/댓글/북마크 상태 및 카운트, onCommentClick 전달까지 기대 동작에 맞게 연결되었습니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (2)
189-200: 초기 로딩 분기/스피너 처리 LGTM빈 목록 + 최초 로딩 시 전체 화면 인디케이터로 빠르게 반환하는 UX가 명확합니다.
421-436: 하단 로딩 인디케이터 배치 적절isLoadingMore 시 리스트 하단에 인디케이터를 노출하는 접근 괜찮습니다. shouldLoadMore 가드 보완(리프레시 중 차단)과 함께 사용하면 중복 호출을 잘 막을 수 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (2)
97-109: 무한스크롤 트리거 가드 보완 및 remember 단순화ViewModel의 canLoadMoreCurrentTab가 isRefreshing을 포함하므로 현재도 동작은 안전하지만, UI 단에서도 명시적으로 isRefreshing 가드를 추가하면 의도 전달이 명확하고 방어적입니다. 또한 remember 키는 생략해도 derivedStateOf가 내부 읽기를 추적합니다.
적용 패치:
- val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { - derivedStateOf { + val shouldLoadMore by remember { + derivedStateOf { val layoutInfo = listState.layoutInfo val totalItems = layoutInfo.totalItemsCount val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 feedUiState.canLoadMoreCurrentTab && !feedUiState.isLoadingMore && + !feedUiState.isRefreshing && feedUiState.currentTabFeeds.isNotEmpty() && totalItems > 0 && lastVisibleIndex >= totalItems - 3 - } - } + } + }
219-221: 단일 소스의 탭 상태 사용으로 일관성 확보 LGTMHeaderMenuBarTab과 본문 렌더링 모두 feedUiState.selectedTabIndex를 참조하도록 정리된 점 좋습니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (3)
77-81: 초기 로드 중복 실행 가능 — isInitial이어도 로딩 가드 필요isLoadingAllFeeds 체크가 isInitial=false에만 적용되어 초기 로딩이 병렬로 중복 실행될 수 있습니다. 리프레시 중에도 실행되지 않도록 가드하세요.
적용 패치:
- private fun loadAllFeeds(isInitial: Boolean = true) { - if (isLoadingAllFeeds && !isInitial) return - if (_uiState.value.isLastPageAllFeeds && !isInitial) return + private fun loadAllFeeds(isInitial: Boolean = true) { + if (isLoadingAllFeeds) return + if (_uiState.value.isRefreshing) return + if (_uiState.value.isLastPageAllFeeds && !isInitial) return
118-121: 내 피드도 동일 — 초기/리프레시 가드 추가 필요loadMyFeeds 역시 동일한 문제가 있습니다.
적용 패치:
- private fun loadMyFeeds(isInitial: Boolean = true) { - if (isLoadingMyFeeds && !isInitial) return - if (_uiState.value.isLastPageMyFeeds && !isInitial) return + private fun loadMyFeeds(isInitial: Boolean = true) { + if (isLoadingMyFeeds) return + if (_uiState.value.isRefreshing) return + if (_uiState.value.isLastPageMyFeeds && !isInitial) return
159-168: refreshCurrentTab 중복 실행 및 로딩과 병행 실행 가능리프레시 중복 실행 또는 로딩과 병행 실행을 방지하는 가드를 추가하세요.
적용 패치:
- fun refreshCurrentTab() { - viewModelScope.launch { + fun refreshCurrentTab() { + if (_uiState.value.isRefreshing || isLoadingAllFeeds || isLoadingMyFeeds) return + viewModelScope.launch { updateState { it.copy(isRefreshing = true) } when (_uiState.value.selectedTabIndex) { 0 -> refreshAllFeeds() 1 -> refreshMyFeeds() } } }
🧹 Nitpick comments (13)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (5)
76-83: 텍스트 잘림 처리 보강: Ellipsis 적용 권장maxLines로 줄 수를 제한하면서 overflow가 기본값(Clip)이라 사용자 입장에서 갑작스런 절단처럼 보일 수 있습니다. Ellipsis를 추가해 가독성을 개선하세요.
Text( text = feedItem.content, style = typography.feedcopy_r400_s14_h20, color = colors.White, maxLines = maxLines, + overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis, modifier = Modifier .fillMaxWidth() .padding(vertical = 16.dp) )참고: 필요 시 import를 추가하세요.
- import androidx.compose.ui.text.style.TextOverflow
84-102: 썸네일 간격 처리 개선 및 불필요한 여백 제거각 썸네일에 end padding을 주면 마지막 항목 오른쪽에도 불필요한 공백이 생깁니다. Row의 spacedBy를 사용해 간격을 일관성 있게 관리하는 편이 좋습니다.
if (hasImages) { Row( modifier = Modifier .fillMaxWidth() .padding(bottom = 16.dp), - verticalAlignment = Alignment.CenterVertically, + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(10.dp) ) { feedItem.imageUrls.take(3).forEach { imageUrl -> AsyncImage( model = imageUrl, contentDescription = null, - modifier = Modifier - .padding(end = 10.dp) - .size(100.dp), + modifier = Modifier.size(100.dp), contentScale = ContentScale.Crop ) } } }추가 제안:
- 로딩/에러 placeholder, crossfade 적용을 고려해 주세요. 예시:
위 예시는 다음 import가 필요합니다:AsyncImage( model = ImageRequest.Builder(LocalContext.current) .data(imageUrl) .crossfade(true) .build(), placeholder = painterResource(R.drawable.img_placeholder), error = painterResource(R.drawable.img_placeholder), ... )
- import androidx.compose.ui.platform.LocalContext
- import coil.request.ImageRequest
- import androidx.compose.ui.res.painterResource
69-74: 중앙 정렬 제거 권장 (레이아웃 일관성)본문 텍스트가 fillMaxWidth로 확장되어 있어 Column의 중앙 정렬 설정이 효과가 없거나 의도치 않게 보일 수 있습니다. 기본 정렬(Top/Start)이 일반적인 피드 레이아웃에 적합합니다.
- Column ( - modifier = Modifier - .clickable { onContentClick() }, - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally - ) { + Column( + modifier = Modifier.clickable { onContentClick() } + ) {
49-56: 기본 프로필 이미지를 외부 URL로 두는 것은 지양기본값으로 외부 네트워크 URL을 사용하면 네트워크 실패 시 빈 화면/지연이 생기고, 불필요한 트래픽이 발생합니다. 앱 내 로컬 placeholder(예: R.drawable.ic_profile_placeholder)를 ProfileBar 내부에서 처리하거나, 여기서는 빈 값만 전달하고 내부에서 fallback을 적용하는 방식을 권장합니다.
- profileImage = feedItem.userProfileImage ?: "https://example.com/image1.jpg", + profileImage = feedItem.userProfileImage ?: "",확인 요청:
- ProfileBar가 빈 문자열/nullable 입력에 대해 로컬 placeholder로 안전하게 처리하는지 확인 부탁드립니다. 필요 시 ProfileBar 쪽에 placeholder 로직을 이식하는 것이 재사용성 측면에서 더 적절합니다.
121-152: Preview에서 네트워크 이미지는 미리보기 실패를 유발할 수 있음Android Studio Preview 환경에서는 네트워크 호출이 제한되어 썸네일/프로필 이미지가 로드되지 않을 수 있습니다. Preview 전용으로 LocalInspectionMode.current를 사용해 로컬 리소스 placeholder를 쓰도록 분기하는 것을 고려해 주세요.
예시:
val inPreview = LocalInspectionMode.current val sampleProfile = if (inPreview) R.drawable.ic_profile_placeholder else feedItem.userProfileImage필요 import:
- import androidx.compose.ui.platform.LocalInspectionMode
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (6)
84-88: 미사용 상태/목데이터 제거로 간결화
- feedStateList(라인 84-88), mySubscriptions(라인 136-185), subscriptionUiState(라인 186-187)가 사용되지 않습니다. 제거해 빌드 경고를 없애고 가독성을 높이세요. 또한 mySubscriptionViewModel 자체도 현재 파일에서 사용되지 않으므로 파라미터 제거를 고려해 주세요.
적용 패치:
- val feedStateList = remember { - mutableStateListOf<FeedItem>().apply { - addAll(feeds) - } - }- val mySubscriptions = listOf( - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image1.jpg", - nickname = "abcabcabcabc", - role = "문학가", - roleColor = colors.SocialScience - ), - MySubscriptionData( - profileImageUrl = "https://example.com/image.jpg", - nickname = "aaaaaaa", - role = "작가", - roleColor = colors.SocialScience - ) - )- val subscriptionUiState by mySubscriptionViewModel.uiState.collectAsState()Also applies to: 136-185, 186-187
203-206: 풀투리프레시 연결은 적절하나, ViewModel 가드 보완 필요UI 측 연결은 좋습니다. 다만 FeedViewModel.refreshCurrentTab()에 중복 실행 가드가 없어 로딩/리프레시 중 재실행 가능성이 있습니다. 아래 ViewModel에 대한 코멘트를 참고해 가드를 추가해 주세요.
318-339: UI 내 데이터 매핑 제거하고 Mapper로 이전 제안Composable 내부에서 MyFeedItem → FeedItem 매핑을 매 항목마다 수행하고 있습니다. ViewModel 혹은 별도 mapper로 이전하면 재구성 비용을 줄이고 재사용성을 높일 수 있습니다.
적용 패치(해당 블록 단순화):
- // MyFeedItem을 FeedItem으로 변환 - val feedItem = FeedItem( - id = myFeed.feedId, - userProfileImage = null, - userName = "", // 내 피드이므로 고정값 - userRole = "", // 내 피드이므로 고정값 - bookTitle = myFeed.bookTitle, - authName = myFeed.bookAuthor, - timeAgo = myFeed.postDate, - content = myFeed.contentBody, - likeCount = myFeed.likeCount, - commentCount = myFeed.commentCount, - isLiked = false, // 내 피드는 좋아요 개념 없음 - isSaved = false, // 내 피드는 저장 개념 없음 - isLocked = !myFeed.isPublic, // isPublic의 반대값 - tags = emptyList(), - imageUrls = myFeed.contentUrls - ) + val feedItem = myFeed.toFeedItem()추가(별도 파일/위치): Mapper 확장 함수 예시
// app/src/main/java/com/texthip/thip/ui/feed/mapper/FeedMappers.kt package com.texthip.thip.ui.feed.mapper import com.texthip.thip.data.model.feeds.response.MyFeedItem import com.texthip.thip.data.model.feeds.response.AllFeedItem import com.texthip.thip.ui.mypage.mock.FeedItem fun MyFeedItem.toFeedItem() = FeedItem( id = feedId, userProfileImage = null, userName = "", userRole = "", bookTitle = bookTitle, authName = bookAuthor, timeAgo = postDate, content = contentBody, likeCount = likeCount, commentCount = commentCount, isLiked = false, isSaved = false, isLocked = !isPublic, tags = emptyList(), imageUrls = contentUrls ) fun AllFeedItem.toFeedItem() = FeedItem( id = feedId, userProfileImage = creatorProfileImageUrl, userName = creatorNickname, userRole = aliasName, bookTitle = bookTitle, authName = bookAuthor, timeAgo = postDate, content = contentBody, likeCount = likeCount, commentCount = commentCount, isLiked = isLiked, isSaved = isSaved, isLocked = false, tags = emptyList(), imageUrls = contentUrls )
376-395: AllFeedItem 매핑도 Mapper로 이전 권장위 코멘트와 동일한 맥락으로 AllFeedItem → FeedItem 매핑도 확장 함수 사용으로 간결화하세요.
적용 패치:
- val feedItem = FeedItem( - id = allFeed.feedId, - userProfileImage = allFeed.creatorProfileImageUrl, - userName = allFeed.creatorNickname, - userRole = allFeed.aliasName, - bookTitle = allFeed.bookTitle, - authName = allFeed.bookAuthor, - timeAgo = allFeed.postDate, - content = allFeed.contentBody, - likeCount = allFeed.likeCount, - commentCount = allFeed.commentCount, - isLiked = allFeed.isLiked, - isSaved = allFeed.isSaved, - isLocked = false, - tags = emptyList(), - imageUrls = allFeed.contentUrls - ) + val feedItem = allFeed.toFeedItem()
401-413: 좋아요/북마크 TODO 구현 시 주의사항 및 도와드릴 수 있어요
- API 호출 실패 시 롤백 가능한 낙관적 업데이트 패턴을 권장합니다.
- 중복 탭(피드/내 피드) 간 상태 일관성을 위해 단일 소스(ViewModel)에서 토글 처리하세요.
- 호출 중 중복 클릭 방지(로딩 가드)도 필요합니다.
원하시면 FeedViewModel에 toggleLike(feedId)/toggleBookmark(feedId)와 optimistic UI 업데이트 구현을 제안 드릴게요.
66-82: Preview 친화적 구조로 개선 권장(hiltViewModel 의존 제거)현재 기본 파라미터에서 hiltViewModel()를 호출하므로 Preview에서 크래시가 납니다. 파라미터를 nullable로 두고 내부에서 null일 때만 hiltViewModel()를 호출하는 패턴으로 바꾸면 Preview가 안전해집니다.
예시:
@Composable fun FeedScreen( ..., feedViewModel: FeedViewModel? = null, mySubscriptionViewModel: MySubscriptionViewModel? = null ) { val vm = feedViewModel ?: hiltViewModel<FeedViewModel>() val mySubVm = mySubscriptionViewModel ?: hiltViewModel<MySubscriptionViewModel>() val feedUiState by vm.uiState.collectAsState() ... }Preview에선 가짜 ViewModel이나 목 상태를 주입할 수 있습니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
30-39: currentTabFeeds의 타입 안정성 개선 제안currentTabFeeds가 List로 노출되어 타입 안전성이 떨어집니다. UI에서 공통 모델(예: FeedItem)로 통합해 내보내거나 size/isEmpty 등 파생 속성(currentTabCount, isCurrentTabEmpty)을 제공하는 방식을 검토해 주세요. View에서 불필요한 캐스팅/매핑을 줄일 수 있습니다.
64-75: 탭 재선택 시 중복 초기 로드 방지 가드 권장onTabSelected에서 매번 초기 로드를 트리거하면, 이미 로드된 상태나 로딩 중에도 다시 초기 로드가 겹칠 수 있습니다. 데이터가 비어있을 때만 초기 로드를 수행하도록 가드하면 불필요한 호출을 줄일 수 있습니다.
예시 패치:
fun onTabSelected(index: Int) { updateState { it.copy(selectedTabIndex = index) } when (index) { - 0 -> { - loadAllFeeds(isInitial = true) - } - 1 -> { - loadMyFeeds(isInitial = true) - } + 0 -> if (_uiState.value.allFeeds.isEmpty() && !isLoadingAllFeeds) { + loadAllFeeds(isInitial = true) + } + 1 -> if (_uiState.value.myFeeds.isEmpty() && !isLoadingMyFeeds) { + loadMyFeeds(isInitial = true) + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(12 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (1)
MyFeedCard(29-124)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
SavedFeedCard(31-116)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
app/src/main/java/com/texthip/thip/ui/common/buttons/ActionBarButton.kt (1)
ActionBarButton(24-99)
🔇 Additional comments (7)
app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (3)
41-43: imageUrls의 null 가능성 점검 필요FeedItem.imageUrls가 nullable이라면 .isNotEmpty()에서 NPE가 발생합니다. 타입이 non-null로 보장되지 않았다면 안전 연산으로 처리해 주세요.
nullable일 경우 아래처럼 변경을 권장합니다:
- val hasImages = feedItem.imageUrls.isNotEmpty() + val hasImages = feedItem.imageUrls?.isNotEmpty() == true val maxLines = if (hasImages) 3 else 8
35-40: 컴포저블 API 확장 방향 좋습니다
- onCommentClick 추가로 상위 네비게이션까지 이벤트 전달이 명확해졌습니다.
- bottomTextColor 기본값과 onContentClick 기본 구현으로 기존 호출부와의 호환성도 유지됩니다.
105-114: ActionBarButton 호출 시 modifier 기본값 존재 — 변경 불필요
ActionBarButton선언부를 확인해 보니 modifier 파라미터에 이미= Modifier기본값이 지정되어 있습니다.
따라서 호출부에서 modifier를 생략해도 컴파일 오류가 발생하지 않으며, 제안된 수정은 불필요합니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (2)
188-201: 초기 로딩 스피너 처리 LGTM초기 로딩 + 빈 리스트 조건에서 전체 화면 인디케이터로 반환하는 패턴이 명확하고 사용자 경험에 적합합니다.
424-439: 하단 로딩 인디케이터 배치 적절isLoadingMore 시 리스트 하단에 인디케이터를 표시하는 UX가 직관적입니다.
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
28-39: 리프레시 중 로드 방지 가드 포함 LGTMcanLoadMoreAllFeeds/MyFeeds에 !isRefreshing을 포함하고, canLoadMoreCurrentTab로 일관되게 노출한 점 좋습니다.
236-243: loadMore 가드 LGTMcanLoadMoreCurrentTab와 isRefreshing 체크로 안전하게 방어되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
99-134: 중복 runCatching 제거 및 예외 전파로 책임 일원화함수 내부
runCatching{...}.getOrNull()로 예외가 삼켜져 상위에서 실패를 감지하기 어렵습니다. 내부에서는 예외를 던지고, 상위 호출부에서 로깅/처리를 일원화하세요.- ): MultipartBody.Part? { - return runCatching { + ): MultipartBody.Part { // MIME 타입 확인 val mimeType = context.contentResolver.getType(uri) ?: "image/jpeg" val extension = when (mimeType) { "image/png" -> "png" "image/gif" -> "gif" "image/jpeg", "image/jpg" -> "jpg" else -> "jpg" // 기본값 } // 파일명 생성 val fileName = "feed_image_${System.currentTimeMillis()}.$extension" val tempFile = File(context.cacheDir, fileName) // 임시 파일 목록에 추가 tempFiles.add(tempFile) // InputStream을 use 블록으로 안전하게 관리 context.contentResolver.openInputStream(uri)?.use { inputStream -> FileOutputStream(tempFile).use { outputStream -> inputStream.copyTo(outputStream) } - } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") + } ?: throw IllegalStateException("Failed to open input stream for URI: $uri") // MultipartBody.Part 생성 val requestFile = tempFile.asRequestBody(mimeType.toMediaType()) - MultipartBody.Part.createFormData(paramName, fileName, requestFile) - }.onFailure { e -> - e.printStackTrace() - }.getOrNull() + return MultipartBody.Part.createFormData(paramName, fileName, requestFile) - } + }상위(
createFeed)에서 try/catch로 로깅하고 사용자 메시지는 리소스 기반으로 반환하는 구성을 권장합니다.
76-87: 이미지 파트 생성: 부분 실패를 침묵 무시하지 말고 전체 실패로 처리현재
mapNotNull + getOrNull()로 일부 URI 변환 실패가 조용히 무시됩니다. 사용자 입장에선 “이미지 일부 유실”로 보일 수 있어 신뢰성 저하가 큽니다. 입력 개수와 생성된 파트 개수가 다르면 실패로 처리하고 로깅을 중앙화하세요.- val imageParts = if (imageUris.isNotEmpty()) { - withContext(Dispatchers.IO) { - imageUris.mapNotNull { uri -> - runCatching { - uriToMultipartBodyPart(uri, "images", tempFiles) - }.getOrNull() - } - } - } else { - null - } + val imageParts: List<MultipartBody.Part>? = if (imageUris.isNotEmpty()) { + val parts = withContext(Dispatchers.IO) { + imageUris.map { uri -> + uriToMultipartBodyPart(uri, "images", tempFiles) + } + } + // 안전장치: 생성 실패 시 예외 발생(위에서 throw되며 여기로 전파) + parts + } else { + null + }app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
97-109: 무한스크롤 트리거 가드 간소화 및 리프레시 중 중첩 방지
derivedStateOf는 내부에서 읽는 상태에 반응하므로remember키를 생략해도 됩니다. 또한 리프레시 중 중첩 호출 방지를 위해 가드를 함께 두는 편이 안전합니다(현재canLoadMoreCurrentTab이 리프레시를 반영하도록 구현되었지만 UI 레벨에서도 방어 가능).- val shouldLoadMore by remember(feedUiState.canLoadMoreCurrentTab, feedUiState.isLoadingMore) { - derivedStateOf { + val shouldLoadMore by remember { + derivedStateOf { val layoutInfo = listState.layoutInfo val totalItems = layoutInfo.totalItemsCount val lastVisibleIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 - feedUiState.canLoadMoreCurrentTab && - !feedUiState.isLoadingMore && - feedUiState.currentTabFeeds.isNotEmpty() && - totalItems > 0 && - lastVisibleIndex >= totalItems - 3 + feedUiState.canLoadMoreCurrentTab && + !feedUiState.isLoadingMore && + !feedUiState.isRefreshing && + feedUiState.currentTabFeeds.isNotEmpty() && + totalItems > 0 && + lastVisibleIndex >= totalItems - 3 } }app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (3)
80-85: 초기 로드에도 로딩 가드 적용
isInitial여부와 무관하게 중복 호출을 막아야 합니다. 탭 재선택/중복 트리거 시 병렬 호출 가능성이 있습니다. 리프레시 중에는 호출을 금지하세요.- private fun loadAllFeeds(isInitial: Boolean = true) { - if (isLoadingAllFeeds && !isInitial) return - if (_uiState.value.isLastPageAllFeeds && !isInitial) return + private fun loadAllFeeds(isInitial: Boolean = true) { + if (isLoadingAllFeeds) return + if (_uiState.value.isRefreshing) return + if (_uiState.value.isLastPageAllFeeds && !isInitial) return
127-133: 내 피드 로딩도 동일한 가드 적용
loadMyFeeds에도 동일한 중복 실행 방지/리프레시 가드를 적용해 주세요.- private fun loadMyFeeds(isInitial: Boolean = true) { - if (isLoadingMyFeeds && !isInitial) return - if (_uiState.value.isLastPageMyFeeds && !isInitial) return + private fun loadMyFeeds(isInitial: Boolean = true) { + if (isLoadingMyFeeds) return + if (_uiState.value.isRefreshing) return + if (_uiState.value.isLastPageMyFeeds && !isInitial) return
174-183: 리프레시 중복 실행/로딩 병행 방지 가드 추가현재 로딩/리프레시와 병행 실행될 수 있습니다. 시작 전에 가드하고 finally에서 플래그를 복구하세요.
- fun refreshCurrentTab() { - viewModelScope.launch { - updateState { it.copy(isRefreshing = true) } + fun refreshCurrentTab() { + if (_uiState.value.isRefreshing || isLoadingAllFeeds || isLoadingMyFeeds) return + viewModelScope.launch { + updateState { it.copy(isRefreshing = true) } when (_uiState.value.selectedTabIndex) { 0 -> refreshAllFeeds() 1 -> refreshMyFeeds() } } }
🧹 Nitpick comments (10)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (3)
47-52: 성공 시 이전 에러 메시지 초기화 제안성공 경로에서
errorMessage를 명시적으로null로 초기화하면 이전 실패 상태가 UI에 잔존하는 것을 방지할 수 있습니다.적용 예시:
updateState { it.copy( categories = response?.categoryList ?: emptyList(), - isLoadingCategories = false + isLoadingCategories = false, + errorMessage = null ) }
176-183: 매직 넘버(3) 상수화 및 재사용이미지 최대 개수(3)를 상수로 추출하면 가독성과 유지보수성이 향상됩니다. 향후 정책 변경에도 안전합니다.
- val availableSlots = 3 - currentState.imageUris.size + val availableSlots = MAX_IMAGE_COUNT - currentState.imageUris.size val imagesToAdd = newImageUris.take(availableSlots) updateState { - it.copy(imageUris = currentState.imageUris + imagesToAdd) + it.copy(imageUris = currentState.imageUris + imagesToAdd) }추가(파일 외부 범위):
companion object { private const val MAX_IMAGE_COUNT = 3 }
261-265: 원시 예외 메시지 노출 자제
onError(exception.message ?: …)는 내부 구현/민감 정보를 노출할 수 있어 UX와 보안 측면에서 바람직하지 않습니다. 사용자 메시지는 리소스 기반으로 통일하고, 예외는 로그로 남기는 방식 권장합니다.- }.onFailure { exception -> - onError( - exception.message - ?: stringResourceProvider.getString(R.string.error_network_error) - ) - } + }.onFailure { exception -> + // TODO: 로깅 프레임워크 사용 시 교체 (예: Timber.e(exception)) + android.util.Log.e("FeedWriteViewModel", "createFeed failed", exception) + onError(stringResourceProvider.getString(R.string.error_network_error)) + }원하시면 로거 도입/일원화까지 패치 제안 드리겠습니다.
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (2)
50-55: 불필요한 정렬 속성 제거 제안해당 Column은 텍스트가
fillMaxWidth로 배치되고 이미지 Row도 별도 배치되어 있어verticalArrangement/horizontalAlignment는 실효성이 낮습니다. 간결화를 고려해 주세요.- Column( - modifier = Modifier - .clickable { onContentClick() }, - verticalArrangement = Arrangement.Center, - horizontalAlignment = Alignment.CenterHorizontally - ) { + Column( + modifier = Modifier.clickable { onContentClick() } + ) {
66-83: 이미지 로딩 UX 향상: 플레이스홀더/크로스페이드/오류 처리
AsyncImage에 플레이스홀더, 오류 이미지, 크로스페이드 등을 추가하면 깜빡임과 레이아웃 점프를 줄일 수 있습니다. 또한 접근성을 위해contentDescription을 의미 있게 제공하는 것도 고려해 주세요(장식적 이미지라면 null 유지).적용 예시(리소스 존재 시):
- AsyncImage( - model = imageUrl, - contentDescription = null, - modifier = Modifier - .padding(end = 10.dp) - .size(100.dp), - contentScale = ContentScale.Crop - ) + AsyncImage( + model = imageUrl, + contentDescription = null, // 또는 stringResource(R.string.feed_image) + modifier = Modifier + .padding(end = 10.dp) + .size(100.dp), + contentScale = ContentScale.Crop + // placeholder = painterResource(R.drawable.img_placeholder), + // error = painterResource(R.drawable.img_placeholder) + )리소스가 없다면 최소한 Coil crossfade만 적용하는 방법도 있습니다(ImageRequest 사용).
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (2)
39-51: 카테고리 정렬 기준 상수화 및 도메인화 제안하드코딩된 문자열 매핑은 오타/다국어/서버 정책 변경에 취약합니다. 맵 상수로 추출하거나 서버가 정렬 우선순위를 제공하면 더 안전합니다.
추가(파일 외부 범위):
private val CATEGORY_ORDER_MAP = mapOf( "문학" to 0, "과학·IT" to 1, "사회과학" to 2, "인문학" to 3, "예술" to 4, ) private fun orderOf(category: String): Int = CATEGORY_ORDER_MAP[category] ?: Int.MAX_VALUE적용:
- val orderedCategories = response?.categoryList?.sortedBy { category -> - when (category.category) { - "문학" -> 0 - "과학·IT" -> 1 - "사회과학" -> 2 - "인문학" -> 3 - "예술" -> 4 - else -> 999 - } - } ?: emptyList() + val orderedCategories = response?.categoryList + ?.sortedBy { category -> orderOf(category.category) } + ?: emptyList()
157-168: 파일 정리 로깅 개선
printStackTrace()대신 표준 로거 사용 권장합니다. 배포 빌드에서의 제어 및 필터링이 용이합니다.- }.onFailure { e -> - e.printStackTrace() - } + }.onFailure { e -> + android.util.Log.e("FeedRepository", "Failed to delete temp file: ${file.absolutePath}", e) + }추가(파일 외부 범위):
import android.util.Logapp/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
268-356: 내 피드 매핑/노출 정책 확인 요청
isLiked = false,isSaved = false로 고정,isLocked = !isPublic은 정책에 부합하는지 PO/백엔드와 한번 더 정합성 확인을 권장합니다. 특히 내 피드에서도 좋아요/댓글 수 노출은 있으나 좋아요 상태 고정은 UX 혼선을 줄 수 있습니다.app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
16-42: currentTabFeeds 타입 안정성 개선
List<Any>는 다운스트림 사용 시 캐스팅 이슈를 야기할 수 있습니다. 공통 인터페이스/시일드 타입을 도입하거나 UI에서 크기/공통 필드만 참조하도록 보장하는 래퍼를 고려해 주세요.
66-78: 탭 전환 시 불필요한 재초기 로드 방지이미 데이터가 로드된 탭을 다시 선택한 경우 초기 로드를 생략하면 API 트래픽과 깜빡임을 줄일 수 있습니다.
fun onTabSelected(index: Int) { updateState { it.copy(selectedTabIndex = index) } when (index) { 0 -> { - loadAllFeeds(isInitial = true) + if (_uiState.value.allFeeds.isEmpty()) { + loadAllFeeds(isInitial = true) + } } 1 -> { - loadMyFeeds(isInitial = true) + if (_uiState.value.myFeeds.isEmpty()) { + loadMyFeeds(isInitial = true) + } } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt(8 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt(7 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt
- app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt
- app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt
🧰 Additional context used
🧬 Code Graph Analysis (3)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (2)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt (1)
updateState(28-30)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (1)
updateState(34-36)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (2)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedDetailViewModel.kt (1)
updateState(28-30)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (1)
updateState(57-59)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (7)
app/src/main/java/com/texthip/thip/ui/common/topappbar/LogoTopAppBar.kt (1)
LogoTopAppBar(24-73)app/src/main/java/com/texthip/thip/ui/common/header/HeaderMenuBarTab.kt (1)
HeaderMenuBarTab(23-69)app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (1)
AuthorHeader(34-118)app/src/main/java/com/texthip/thip/ui/feed/component/FeedSubscribelistBar.kt (1)
FeedSubscribeBarlist(35-101)app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (1)
MyFeedCard(29-124)app/src/main/java/com/texthip/thip/ui/mypage/component/SavedFeedCard.kt (1)
SavedFeedCard(31-116)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(6-13)
🔇 Additional comments (10)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedWriteViewModel.kt (3)
55-61: 실패 시 사용자 친화적 에러 메시지 추가: 좋습니다네트워크 오류 시 리소스 기반 공통 메시지로 설정한 점 👍
199-205: 카테고리 변경 시 태그 초기화: 적절합니다카테고리 전환 시 종속 상태(태그)를 초기화하는 방향 맞습니다.
223-225: 태그 제거 로직 간결화: 좋습니다불변 리스트 연산으로 간단하고 안전하게 처리되어 있습니다.
app/src/main/java/com/texthip/thip/ui/feed/component/MyFeedCard.kt (2)
36-38: null 불가 리스트로의 전환: LGTM
imageUrls.isNotEmpty()기반 분기와maxLines계산이 명확해졌습니다.
103-103: 댓글 아이콘 클릭 시 콘텐츠 네비게이션 연결: 적절합니다댓글 접근 경로 통일에 도움이 됩니다.
app/src/main/java/com/texthip/thip/data/repository/FeedRepository.kt (1)
136-155: 피드 조회 API 래핑: LGTM
handleBaseResponse().getOrThrow()를runCatching으로 감싼 구조 적절합니다.app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (3)
188-200: 초기 로딩 인디케이터 분기: 적절합니다탭별 데이터 비어있음 + 로딩 조합에 대한 깔끔한 early return 처리 👍
378-399: 전체 피드 도메인 → UI 매핑: LGTM키 설정(
key = feedId), 색상 변환(hexToColor), URL 기반 이미지/프로필 적용 흐름이 일관됩니다.
428-441: 하단 로딩 인디케이터: 적절합니다
isLoadingMore시 리스트 푸터에 로더 출력하는 UX 패턴 문제 없습니다.app/src/main/java/com/texthip/thip/ui/feed/viewmodel/FeedViewModel.kt (1)
251-258: loadMoreFeeds 가드: 적절합니다
canLoadMoreCurrentTab및isRefreshing가드로 중복/중첩 호출을 예방하는 구조 좋습니다.
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
2025-08-17.3.27.39.mov
😢 해결하지 못한 과제
[] 나머지 API는 오늘 내로 연결하겠습니다
📢 리뷰어들에게
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores