[API] 내 팔로잉 목록 조회 API 연동 #82
Conversation
Walkthrough내 팔로잉 조회 API 연동을 위해 UserService·DTO·UserRepository·MySubscriptionViewModel을 추가하고 Feed 및 MySubscription 화면을 ViewModel 기반으로 전환했으며 DI 제공자, 내비게이션 경로, AuthorHeader 배지 색상 파라미터 및 hexToColor 유틸을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant FeedUI as FeedScreen
participant SubUI as MySubscriptionScreen
participant VM as MySubscriptionViewModel
participant Repo as UserRepository
participant Svc as UserService
participant API as Backend
FeedUI->>VM: 구독 바용 uiState 수집
FeedUI->>FeedUI: onNavigateToMySubscription() 호출 -> 네비게이션
SubUI->>VM: 초기 진입 / 스크롤 끝 감지 -> fetchMyFollowings(cursor)
VM->>Repo: getMyFollowings(cursor, size)
Repo->>Svc: Retrofit GET /users/my-followings?size=&cursor=
Svc->>API: HTTP 요청
API-->>Svc: BaseResponse<MyFollowingsResponse>
Svc-->>Repo: 응답 전달
Repo-->>VM: Result<MyFollowingsResponse?>
VM-->>SubUI: uiState 업데이트 (followings/total/isLast/isLoading/showToast)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 없음 — 연결된 이슈 목적 범위를 벗어나는 기능성 코드 변경은 발견되지 않았습니다.) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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: 1
🔭 Outside diff range comments (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
65-114: 하드코딩된 mock 데이터 제거 필요프로덕션 코드에 테스트용 mock 데이터가 남아있습니다. 이 데이터는 사용되지 않으므로 제거해야 합니다.
- 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 - ) - )
🧹 Nitpick comments (13)
app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (1)
104-111: 문자열 연결(i18n) 지양 — 하나의 문자열 리소스로 포맷 통합 제안서로 다른 stringResource를 +로 연결하면 현지화 시 어순/띄어쓰기 문제와 번역 누락 위험이 큽니다. 하나의 문자열 리소스에서 포맷 플레이스홀더로 처리해 주세요.
Kotlin 변경 제안:
- Text( - text = stringResource(R.string.thip_num, thipNum)+ stringResource(R.string.thip_ing), + Text( + text = stringResource(R.string.thip_num_with_suffix, thipNum), style = typography.view_r400_s11_h20, color = colors.White )strings.xml 추가(예시):
<string name="thip_num_with_suffix">%1$d띱 진행중</string>문구는 기존 thip_num/thip_ing 의미에 맞게 조정해 주세요.
app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt (1)
31-33: 네이밍 일관성 제안: providesRoomsService → provideRoomsService다른 provider들은 provideXxx를 쓰는데, Rooms만 provides로 시작합니다. 일관성을 위해 아래처럼 정리하면 가독성이 좋아집니다. (기능 변화 없음)
- fun providesRoomsService(retrofit: Retrofit): RoomsService = + fun provideRoomsService(retrofit: Retrofit): RoomsService = retrofit.create(RoomsService::class.java)app/src/main/java/com/texthip/thip/data/service/UserService.kt (1)
10-16: 기본 페이지 크기 상수화로 의미 부여 및 재사용성 향상 제안매직 넘버(10)를 상수로 추출하면 의미가 드러나고, 서비스/레포지토리/뷰모델에서 동일 상수를 재사용하기 쉽습니다.
interface UserService { + companion object { + const val DEFAULT_PAGE_SIZE = 10 + } @GET("users/my-followings") suspend fun getMyFollowings( - @Query("size") size: Int = 10, + @Query("size") size: Int = DEFAULT_PAGE_SIZE, @Query("cursor") cursor: String? = null ): BaseResponse<MyFollowingsResponse>app/src/main/java/com/texthip/thip/data/repository/UserRepository.kt (2)
10-10: Repository 클래스 이름 뒤 공백 추가 필요
UserRepository와@Inject사이에 공백이 누락되어 있습니다.-class UserRepository@Inject constructor( +class UserRepository @Inject constructor(
13-20: API 호출 파라미터 순서 일관성 개선서비스 호출 시 파라미터 순서가 함수 시그니처와 다릅니다. 가독성을 위해 일관된 순서를 유지하는 것이 좋습니다.
- userService.getMyFollowings(cursor = cursor, size = size) + userService.getMyFollowings(size = size, cursor = cursor)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt (6)
43-45: 주석 및 빈 줄 정리한국어 주석을 영어로 변경하고 불필요한 빈 줄을 제거하는 것이 좋습니다.
init { - //ViewModel 생성될 때 첫 페이지 데이터를 로드 + // Load first page data when ViewModel is created fetchMyFollowings(isInitial = true) } - -
60-60: 주석 언어 일관성코드베이스의 일관성을 위해 주석을 영어로 작성하는 것이 좋습니다.
- //data가 null이 아닐 경우에만 상태 업데이트 + // Update state only if data is not null
83-84: 주석 및 오류 메시지 언어 일관성영어로 변경하여 일관성을 유지하세요.
- // 데이터가 null인 경우 (성공했지만 내용이 없는 케이스) - it.copy(isLoading = false, errorMessage = "응답 데이터가 비어있습니다.") + // Data is null (successful but empty response) + it.copy(isLoading = false, errorMessage = "Response data is empty")
87-87: 주석 언어 일관성영어로 변경하세요.
- // 실패 시 예외 처리 + // Handle failure exception
94-95: 빈 줄 제거불필요한 빈 줄을 제거하세요.
} - - fun toggleFollow(userId: Int, followedMessage: String, unfollowedMessage: String) {
97-97: TODO 주석 처리 필요실제 팔로우/언팔로우 API 호출 로직이 구현되어야 합니다. 현재는 UI 상태만 변경하고 있어 서버와 동기화되지 않습니다.
실제 API 호출 로직을 구현하도록 새로운 이슈를 생성하시겠습니까?
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (2)
52-52: 주석 언어 일관성영어로 변경하세요.
- //잘못된 형식이면 기본 색 + // Return default color for invalid format
74-78: 페이지네이션 트리거 로직 개선 고려현재 마지막 아이템이 보일 때 다음 페이지를 로드하는데, 사용자 경험 향상을 위해 마지막에서 2-3번째 아이템이 보일 때 미리 로드하는 것이 좋습니다.
LaunchedEffect(isScrolledToEnd) { if (isScrolledToEnd && !uiState.isLoading && !uiState.isLastPage) { viewModel.fetchMyFollowings() } }더 나은 방법:
val shouldLoadMore by remember { derivedStateOf { val layoutInfo = lazyListState.layoutInfo if (layoutInfo.totalItemsCount == 0) return@derivedStateOf false val lastVisibleItemIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 // Trigger when 3 items before the end lastVisibleItemIndex >= layoutInfo.totalItemsCount - 3 } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt(2 hunks)app/src/main/java/com/texthip/thip/data/model/users/MyFollowingsResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/UserRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/service/UserService.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(7 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt(2 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/MySubscriptionListScreen.kt (1)
MySubscriptionScreen(56-91)
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (4)
app/src/main/java/com/texthip/thip/ui/common/modal/ToastWithDate.kt (1)
ToastWithDate(24-60)app/src/main/java/com/texthip/thip/ui/common/topappbar/DefaultTopAppBar.kt (1)
DefaultTopAppBar(22-65)app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (1)
AuthorHeader(34-118)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (1)
MySubscribeBarlist(38-123)
🔇 Additional comments (10)
app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (3)
85-86: badgeTextColor 적용 LGTM배지 텍스트 색상을 파라미터로 분리해 재사용성과 스타일 유연성이 좋아졌습니다.
136-137: Preview에서 커스터마이징 예시 적절미리보기에서 colors.Yellow로 전달해 사용성을 확인할 수 있어 좋습니다.
40-41: AuthorHeader 호출부 점검 완료 – 포지셔널 호출 없음, 컴파일 오류 없음
스캔 결과 모든AuthorHeader(호출부가 명명(named) 인자를 사용하고 있어,badgeTextColor를 중간에 추가해도 기존 호출에 영향이 없습니다.app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt (1)
25-27: 새 라우트 연동 LGTMFeedRoutes.MySubscription에 대한 composable 엔트리 추가가 적절하고, 화면은 MySubscriptionScreen으로 정상 연결됩니다.
app/src/main/java/com/texthip/thip/ui/navigator/routes/FeedRoutes.kt (1)
11-11: 타입드 라우트 정의 적절@serializable data object MySubscription 추가로 타입 안전한 네비게이션 구성이 잘 이어졌습니다.
app/src/main/java/com/texthip/thip/data/di/ServiceModule.kt (1)
34-38: UserService DI 바인딩 LGTMRetrofit 기반의 다른 서비스들과 동일한 패턴으로 제공되고, 스코프도 @singleton으로 일관됩니다.
app/src/main/java/com/texthip/thip/data/service/UserService.kt (1)
11-16: 내 팔로잉 조회 API 시그니처 적절쿼리 파라미터 기본값과 null 시 자동 생략 동작(Retrofit) 모두 의도에 부합합니다. BaseResponse 반환도 기존 레이어와 잘 맞습니다.
app/src/main/java/com/texthip/thip/data/model/users/MyFollowingsResponse.kt (1)
1-23: 데이터 모델 구조가 깔끔하게 구현되었습니다Gson과 Kotlinx serialization을 모두 지원하도록 적절히 어노테이션이 적용되어 있고, nullable 타입도 올바르게 정의되어 있습니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (2)
56-57: ViewModel 의존성 주입이 적절히 구현되었습니다Hilt를 사용한 ViewModel 주입이 올바르게 구현되어 있습니다.
215-224: aliasColor 색상 값 처리 확인 필요aliasColor를
colors.White로 하드코딩하고 있는데, 실제 API에서 반환하는 색상 값을 사용해야 하지 않나요? MySubscriptionListScreen에서는hexToColor를 사용하여 처리하고 있습니다.API 응답의 aliasColor 값이 hex 형식인지 확인하고, 필요하다면 hexToColor 함수를 사용하도록 수정해야 합니다:
+import com.texthip.thip.ui.feed.screen.hexToColor ... val subscriptionsForBar = subscriptionUiState.followings.map { user -> MySubscriptionData( profileImageUrl = user.profileImageUrl, nickname = user.nickname, role = user.aliasName, - roleColor = colors.White, + roleColor = hexToColor(user.aliasColor), subscriberCount = 0, isSubscribed = user.isFollowing ) }
| //TODO util 함수로 빼기 | ||
| fun hexToColor(hex: String): Color { | ||
| return try { | ||
| Color(android.graphics.Color.parseColor(hex)) | ||
| } catch (e: IllegalArgumentException) { | ||
| //잘못된 형식이면 기본 색 | ||
| Color.White | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
hexToColor 함수를 공통 유틸리티로 이동
TODO 주석에 명시된 대로 이 함수는 공통 유틸리티로 이동해야 합니다. 다른 화면에서도 색상 변환이 필요할 수 있습니다.
ui/common/utils/ColorUtils.kt 파일을 생성하여 이 함수를 이동시키는 작업을 도와드릴까요?
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt
around lines 47–55 the local hexToColor function (with try/catch returning
Color.White on IllegalArgumentException) should be moved to a shared utility:
create a new file
app/src/main/java/com/texthip/thip/ui/common/utils/ColorUtils.kt with the
appropriate package, expose the hexToColor function (public top-level or object)
preserving the same behavior, remove the duplicate from
MySubscriptionListScreen.kt and replace its usage with the new utility import,
and delete the TODO comment; ensure any other screens that need color parsing
import and use the new utility.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (1)
55-59: px/dp 단위 혼용으로 maxVisibleCount 계산이 잘못될 수 있습니다maxWidthPx와 imageWithSpacingPx는 px 단위인데, 36f는 dp 변환 없이 px로 간주되어 밀도에 따라 오차/오동작이 발생할 수 있습니다. dp를 px로 변환해 사용하고, 음수 방지를 위해 coerceAtLeast(0)도 권장합니다.
val maxWidthPx = with(density) { maxWidth.toPx() } val imageWithSpacingPx = with(density) { imageWidthWithSpacing.toPx() } - - val maxVisibleCount = ((maxWidthPx - 36f) / imageWithSpacingPx).toInt() + // 우측 아이콘/여백 예약치를 dp→px로 변환하여 사용 + val chevronReservePx = with(density) { 36.dp.toPx() } + val maxVisibleCount = ((maxWidthPx - chevronReservePx) / imageWithSpacingPx) + .toInt() + .coerceAtLeast(0)
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (3)
109-110: 텍스트 가운데 정렬은 fillMaxWidth로 간결하게 유지하세요부모 Column이 이미 width(36.dp)를 가지고 있으므로 Text에는 width(36.dp) 대신 fillMaxWidth()만 주면 충분합니다. 불필요한 고정 폭을 줄여 가독성과 향후 확장성을 높일 수 있습니다.
- modifier = Modifier.width(36.dp), + modifier = Modifier.fillMaxWidth(), textAlign = TextAlign.Center
165-165: 프리뷰 닉네임 문자열 오타로 보입니다 — 의도 아니라면 정정 권장"닉네임임$it"는 오타로 보입니다. 의도한 경우가 아니라면 기존 형태로 되돌리는 것이 좋습니다. 또는 다양한 길이의 문자열로 Ellipsis 동작을 테스트해도 좋습니다.
- nickname = "닉네임임$it", + nickname = "닉네임$it",
51-54: 중첩 clickable 중복 — 하나로 통일 권장상위 BoxWithConstraints와 내부 Row 모두 clickable을 갖고 있어 제스처 중복/리플 중첩 가능성이 있습니다. 상위 하나로만 처리하는 것이 안전합니다.
Row( modifier = Modifier .fillMaxWidth() - .height(58.dp) - .clickable { onClick() }, + .height(58.dp), verticalAlignment = Alignment.CenterVertically ) {Also applies to: 84-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt(3 hunks)
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (1)
27-27: TextAlign import 추가 적절합니다도입한 가운데 정렬 사용 목적에 부합하며, 실제 사용 라인과 일관됩니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (1)
45-45: 공통 유틸리티 사용으로 중복 코드 해결이전 리뷰 코멘트에서 제기된
hexToColor함수 중복 문제가 해결되었습니다. 이제 공통 유틸리티를 import하여 사용하고 있어 코드 중복이 제거되었습니다.
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (1)
57-70: 무한 스크롤 구현 로직 검토무한 스크롤 구현이 적절해 보이지만 성능상 고려사항이 있습니다:
derivedStateOf를 사용한 상태 계산이 매번 composition에서 실행될 수 있습니다LaunchedEffect가isScrolledToEnd변경 시마다 트리거됩니다성능 최적화를 위해 다음과 같이 개선을 고려해보세요:
- val isScrolledToEnd by remember { - derivedStateOf { - val layoutInfo = lazyListState.layoutInfo - if (layoutInfo.totalItemsCount == 0) return@derivedStateOf false - val lastVisibleItemIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 - lastVisibleItemIndex >= layoutInfo.totalItemsCount - 1 - } - } - - LaunchedEffect(isScrolledToEnd) { - if (isScrolledToEnd && !uiState.isLoading && !uiState.isLastPage) { - viewModel.fetchMyFollowings() - } - } + LaunchedEffect(lazyListState) { + snapshotFlow { + val layoutInfo = lazyListState.layoutInfo + val lastVisibleItemIndex = layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 + lastVisibleItemIndex >= layoutInfo.totalItemsCount - 3 // 3개 정도 여유분 두기 + } + .distinctUntilChanged() + .collect { isNearEnd -> + if (isNearEnd && !uiState.isLoading && !uiState.isLastPage) { + viewModel.fetchMyFollowings() + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (5)
app/src/main/java/com/texthip/thip/ui/common/modal/ToastWithDate.kt (1)
ToastWithDate(24-60)app/src/main/java/com/texthip/thip/ui/common/topappbar/DefaultTopAppBar.kt (1)
DefaultTopAppBar(22-65)app/src/main/java/com/texthip/thip/ui/common/header/AuthorHeader.kt (1)
AuthorHeader(34-118)app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
hexToColor(5-12)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
🔇 Additional comments (7)
app/src/main/java/com/texthip/thip/utils/color/HexToColor.kt (1)
5-12: 중복 코드 제거 완료 - 우수한 리팩터링이전 리뷰에서 지적된 중복 함수 문제가 해결되었습니다.
MySubscriptionListScreen.kt에 있던 로컬hexToColor함수를 공통 유틸리티로 성공적으로 이동시켰네요. 구현도 정확하고 예외 처리도 적절합니다.app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (6)
49-52: ViewModel 기반 아키텍처로 개선함수 시그니처가 ViewModel을 주입받도록 변경되어 더 나은 아키텍처 패턴을 따르고 있습니다.
hiltViewModel()을 사용한 DI도 적절합니다.
84-91: 컴포저블 분리로 테스트 가능성 향상
MySubscriptionContent를 별도 컴포저블로 분리한 것은 좋은 설계입니다. 테스트 가능성과 재사용성이 향상되었습니다.
145-170: 효율적인 리스트 렌더링 구현
itemsIndexed를 사용하여 키 기반 렌더링과 조건부 구분선 처리가 잘 구현되어 있습니다.user.userId를 키로 사용하여 리스트 아이템의 재사용성도 보장됩니다.
154-154: 색상 변환 함수 사용 확인
hexToColor(user.aliasColor)를 통해 새로 생성된 공통 유틸리티 함수를 올바르게 사용하고 있습니다. 색상 파싱 실패 시Color.White로 안전하게 fallback됩니다.
172-185: 로딩 상태 UI 처리 개선로딩 인디케이터 표시 조건이 적절합니다.
!uiState.isLastPage조건을 통해 마지막 페이지에서는 로딩 인디케이터를 표시하지 않도록 처리되어 있습니다.
194-203: 프리뷰 데이터 구성 개선mock 데이터가 현실적으로 구성되어 있고, 다양한 색상과 배지 타입을 테스트할 수 있도록 설정되어 있습니다.
Nico1eKim
left a comment
Nico1eKim
left a comment
There was a problem hiding this comment.
전반적으로 screen과 content를 나눠주고 ThipTheme 붙은거 수정하고 navigation도 파일 만들어서 작성해주시면 될거같습니당 ~
그리고 코드 정렬 습관화 해주면 좋을거같아용 코드 정렬이 잘 안돼있는 파일이 많은거가타요
수고하셨습니당 ~
|
|
||
| @Serializable | ||
| data class MyFollowingsResponse( | ||
| @SerializedName("followings") val followings: List<FollowingDto>, |
There was a problem hiding this comment.
FollowingDto보다는 FollowingList라던가 Followings라는 이름으로 바꾸면 좋을거같아용
| selectedTabIndex: Int = 0, | ||
| followerProfileImageUrls: List<String> = emptyList() | ||
| followerProfileImageUrls: List<String> = emptyList(), | ||
| viewModel: MySubscriptionViewModel = hiltViewModel() |
There was a problem hiding this comment.
이거 screen이랑 content랑 분리해서 screen에서 viewmodel을 주입받고 content에서는 ui만 다루도록 수정해주세여 !
| subscriptions = mySubscriptions, | ||
| subscriptions = subscriptionsForBar, | ||
| onClick = { | ||
| navController?.navigate(FeedRoutes.MySubscription) |
There was a problem hiding this comment.
navController말고 해당 화면의 navigation 파일을 생성해서 연결해주세요!
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt
Outdated
Show resolved
Hide resolved
| data class MySubscriptionUiState( | ||
| val isLoading: Boolean = false, | ||
| val followings: List<MyFollowingUiModel> = emptyList(), | ||
| val totalCount: Int = 0, | ||
| val isLastPage: Boolean = false, | ||
| val errorMessage: String? = null, | ||
| val showToast: Boolean = false, | ||
| val toastMessage: String = "" | ||
| ) | ||
|
|
||
| data class MyFollowingUiModel( | ||
| val userId: Int, | ||
| val profileImageUrl: String?, | ||
| val nickname: String, | ||
| val aliasName: String, | ||
| val aliasColor: String, | ||
| var isFollowing: Boolean | ||
| ) |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt (6)
55-61: 성공 시 이전 에러 메시지 정리 필요성공 응답 처리에서 이전에 남아있던 errorMessage를 null로 초기화하지 않아 UI가 오래된 에러 상태를 유지할 수 있습니다.
적용 diff:
_uiState.update { currentState -> currentState.copy( isLoading = false, followings = if (isInitial) newFollowings else currentState.followings + newFollowings, totalCount = it.totalFollowingCount, - isLastPage = it.isLast + isLastPage = it.isLast, + errorMessage = null ) }
66-69: 에러 메시지 null 안전성과 사용자 친화적 메시지 보강exception.message가 null일 수 있어 UI에 표시할 메시지가 사라질 수 있습니다. 기본 메시지를 제공하세요.
적용 diff:
}.onFailure { exception -> _uiState.update { - it.copy(isLoading = false, errorMessage = exception.message) + it.copy( + isLoading = false, + errorMessage = exception.message ?: "팔로잉 목록을 불러오지 못했습니다. 잠시 후 다시 시도해주세요." + ) } }
93-95: 토스트 숨김 시 메시지도 정리토스트 플래그만 false로 바꾸면, 이후 상태 재사용/재합성 시 이전 메시지가 남을 수 있습니다. 메시지도 초기화하세요.
적용 diff:
fun hideToast() { - _uiState.update { it.copy(showToast = false) } + _uiState.update { it.copy(showToast = false, toastMessage = "") } }
16-16: UI 상태에 데이터 계층 DTO 노출 — UI 모델로 디커플링 권장UI가 data-layer DTO(FollowingDto)에 직접 의존하면 계층 간 결합도가 높아집니다. 간단한 UI 전용 모델(필요 필드만 노출)로 매핑해 사용하면 테스트/변경에 유리합니다.
예:
data class FollowingUi( val id: Int, val name: String, val isFollowing: Boolean, val avatarUrl: String? ) // Repository -> ViewModel에서 DTO -> UI 모델로 변환하여 상태에 저장
20-21: 단발성 이벤트(showToast/toastMessage)는 StateFlow 상태 대신 이벤트 스트림으로 분리 권장State 기반으로 토스트를 관리하면 구성 변경/재구성 시 재소비 문제가 발생할 수 있습니다. SharedFlow/Channel 기반의 일회성 이벤트로 분리하는 것이 안전합니다.
예:
sealed interface UiEvent { data class ShowToast(val message: String): UiEvent } private val _events = MutableSharedFlow<UiEvent>() val events = _events.asSharedFlow() // 사용 _events.emit(UiEvent.ShowToast(toastMsg))
38-71: 단위 테스트 보강 제안: 페이지네이션/에러/중복 호출 가드 검증다음 시나리오에 대한 단위 테스트가 있으면 회귀 방지에 유익합니다.
- 초기 로딩 후 nextCursor 반영 및 isLastPage=true일 때 추가 요청 차단
- 에러 응답 시 errorMessage 설정 및 isLoading=false 복귀
- 연속 호출 시(isLoading=true) 중복 요청 방지
- isInitial=true(풀 리프레시)에서 목록 교체 및 nextCursor 재설정
원하시면 Turbine을 사용한 StateFlow 단위 테스트 템플릿을 작성해 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt
🔇 Additional comments (2)
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt (2)
24-33: 전체 구조와 DI/StateFlow 사용 적절 — 초기 로딩과 페이지네이션 상태 관리가 명확합니다Hilt 주입, StateFlow 기반 UI 상태 관리, nextCursor를 이용한 페이지네이션 플로우 구성이 깔끔합니다. 초기 로딩을 init에서 트리거하는 것도 현재 요구사항에 부합해 보입니다.
47-48: getMyFollowings 호출에 size 기본값(10)이 이미 지정되어 있습니다
UserRepository.getMyFollowings및UserService.getMyFollowings시그니처에 이미size: Int = 10기본값이 선언되어 있어, 뷰모델에서 명시적으로size = 10을 전달하지 않아도 기본값이 적용됩니다. 따라서 요약과 실제 코드 사이에 불일치가 없습니다—변경 불필요합니다.Likely an incorrect or invalid review comment.
| fun fetchMyFollowings(isInitial: Boolean = false) { | ||
| if (_uiState.value.isLoading || (!isInitial && _uiState.value.isLastPage)) { | ||
| return | ||
| } | ||
|
|
||
| viewModelScope.launch { | ||
| _uiState.update { it.copy(isLoading = true) } | ||
| val cursorToFetch = if (isInitial) null else nextCursor | ||
|
|
||
| val result = userRepository.getMyFollowings(cursor = cursorToFetch) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시 호출 시 중복 로딩 가능성: isLoading 설정 타이밍을 launch 밖으로 이동하세요
빠른 스크롤/중복 트리거 시, 가드 체크 이후 isLoading을 코루틴 내부에서 true로 설정하기 때문에 두 번 이상의 요청이 거의 동시에 발사될 수 있습니다. isLoading을 launch 전에 변경해 레이스를 차단하세요. 또한 nextCursor가 없는 상태에서의 비초기 로딩도 가드로 차단하면 안전합니다.
적용 diff:
- if (_uiState.value.isLoading || (!isInitial && _uiState.value.isLastPage)) {
+ if (_uiState.value.isLoading || (!isInitial && (_uiState.value.isLastPage || nextCursor == null))) {
return
}
- viewModelScope.launch {
- _uiState.update { it.copy(isLoading = true) }
- val cursorToFetch = if (isInitial) null else nextCursor
-
- val result = userRepository.getMyFollowings(cursor = cursorToFetch)
+ val cursorToFetch = if (isInitial) null else nextCursor
+ _uiState.update { it.copy(isLoading = true) }
+ viewModelScope.launch {
+ val result = userRepository.getMyFollowings(cursor = cursorToFetch)추가로, 멀티 스레드에서의 절대적 보장을 원한다면 Mutex를 도입하는 방법도 있습니다(선택사항).
예: 아래 코드를 파일 상단 import와 클래스 필드로 추가하고, fetch 본문을 withLock으로 감싸는 방식
// imports
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
// class field
private val loadMutex = Mutex()🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt
around lines 38 to 48, the isLoading flag is set inside the coroutine which
allows race conditions and duplicate requests; move the guard and
_uiState.update { it.copy(isLoading = true) } to before viewModelScope.launch so
the check+set is atomic from the caller thread, and add a guard that if not
isInitial and nextCursor == null return immediately to avoid non-initial
empty-cursor fetches; optionally, for stronger concurrency guarantees, introduce
a Mutex as a class field and wrap the fetch body in withLock().
| fun toggleFollow(userId: Int, followedMessage: String, unfollowedMessage: String) { | ||
| var toastMsg = "" | ||
| _uiState.update { currentState -> | ||
| val updatedList = currentState.followings.map { user -> | ||
| if (user.userId == userId) { | ||
| val isNowFollowing = !user.isFollowing | ||
| toastMsg = if (isNowFollowing) followedMessage else unfollowedMessage | ||
| user.copy(isFollowing = isNowFollowing) | ||
| } else { | ||
| user | ||
| } | ||
| } | ||
| currentState.copy( | ||
| followings = updatedList, | ||
| showToast = true, | ||
| toastMessage = toastMsg | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
사용자 미존재 시에도 토스트가 뜨는 버그: 매칭 실패 시 토스트를 띄우지 않도록 수정
현재 userId가 목록에 없더라도 showToast=true로 설정되어 비어있는 메시지 토스트가 뜰 수 있습니다. 매칭 여부에 따라 토스트 플래그/메시지를 설정하세요.
적용 diff:
fun toggleFollow(userId: Int, followedMessage: String, unfollowedMessage: String) {
- var toastMsg = ""
+ var toastMsg = ""
+ var found = false
_uiState.update { currentState ->
val updatedList = currentState.followings.map { user ->
if (user.userId == userId) {
+ found = true
val isNowFollowing = !user.isFollowing
toastMsg = if (isNowFollowing) followedMessage else unfollowedMessage
user.copy(isFollowing = isNowFollowing)
} else {
user
}
}
currentState.copy(
followings = updatedList,
- showToast = true,
- toastMessage = toastMsg
+ showToast = found,
+ toastMessage = if (found) toastMsg else currentState.toastMessage
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun toggleFollow(userId: Int, followedMessage: String, unfollowedMessage: String) { | |
| var toastMsg = "" | |
| _uiState.update { currentState -> | |
| val updatedList = currentState.followings.map { user -> | |
| if (user.userId == userId) { | |
| val isNowFollowing = !user.isFollowing | |
| toastMsg = if (isNowFollowing) followedMessage else unfollowedMessage | |
| user.copy(isFollowing = isNowFollowing) | |
| } else { | |
| user | |
| } | |
| } | |
| currentState.copy( | |
| followings = updatedList, | |
| showToast = true, | |
| toastMessage = toastMsg | |
| ) | |
| } | |
| } | |
| fun toggleFollow(userId: Int, followedMessage: String, unfollowedMessage: String) { | |
| var toastMsg = "" | |
| var found = false | |
| _uiState.update { currentState -> | |
| val updatedList = currentState.followings.map { user -> | |
| if (user.userId == userId) { | |
| found = true | |
| val isNowFollowing = !user.isFollowing | |
| toastMsg = if (isNowFollowing) followedMessage else unfollowedMessage | |
| user.copy(isFollowing = isNowFollowing) | |
| } else { | |
| user | |
| } | |
| } | |
| currentState.copy( | |
| followings = updatedList, | |
| showToast = found, | |
| toastMessage = if (found) toastMsg else currentState.toastMessage | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt
around lines 74 to 92, the current toggleFollow always sets showToast=true and
toastMessage even when no user matched the provided userId; change the update
logic to detect if any item was actually toggled (e.g., track a boolean matched
or compare list change), and only set showToast=true and toastMessage when a
match occurred (otherwise preserve currentState.showToast and
currentState.toastMessage or set showToast=false and empty message); ensure
toastMsg is only populated when matched and that the state copy uses the
conditional values so no toast appears for non-existent users.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
51-59: Screen/Content 분리 및 navController 제거 제안 (이전 리뷰 반영 필요)이 화면에서 ViewModel을 직접 주입하고 UI까지 함께 관리하고 있습니다. 이전 리뷰(라인 58)에서 요청된 대로 Screen에서만 ViewModel을 주입하고, Content는 순수 UI로 분리해 주세요. 또한 onNavigateToMySubscription 콜백을 도입하셨으므로 navController 파라미터는 제거 가능합니다.
- 이점:
- Preview/테스트 용이
- 재사용성/모듈성 향상
- 네비게이션과 UI 관심사 분리
구조 예시:
- FeedRoute(...) = hiltViewModel() 주입 + 상태 수집 + 콜백 바인딩
- FeedContent(...) = 순수 UI, 상태/콜백 파라미터만 사용
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (1)
46-46: 기존 리뷰 반영 확인: hexToColor 공통 유틸 이동 LGTMhexToColor를 com.texthip.thip.utils.color로 이동하여 재사용성을 확보한 점 좋습니다. 이전 리뷰 피드백이 잘 반영되었습니다.
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/ui/navigator/extensions/FeedNavigationExtensions.kt (1)
12-15: 중복 진입 방지: navigate 옵션에 launchSingleTop 적용 제안같은 화면으로 빠르게 반복 클릭 시 같은 목적지가 스택에 중복으로 쌓일 수 있습니다. 단순 네비게이션이면 launchSingleTop 옵션을 켜 중복 푸시를 방지하는 편이 안전합니다.
아래처럼 변경을 제안합니다:
fun NavHostController.navigateToMySubscription() { - navigate(FeedRoutes.MySubscription) + navigate(FeedRoutes.MySubscription) { + launchSingleTop = true + } }app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
216-225: 불필요 재계산 최소화: map 계산 hoist매 recomposition마다 LazyColumn 내부에서 followings -> UI 모델 매핑이 다시 수행됩니다. 비용이 크지는 않지만, 상태 변화가 잦을 경우 잦은 할당이 발생할 수 있어 바깥으로 hoist하고 remember로 메모이즈하는 편이 좋습니다.
아래처럼 이 블록을 제거하고:
- val subscriptionsForBar = subscriptionUiState.followings.map { user -> - MySubscriptionData( - profileImageUrl = user.profileImageUrl, - nickname = user.nickname, - role = user.aliasName, - roleColor = colors.White, - subscriberCount = 0, - isSubscribed = user.isFollowing - ) - }해당 위치(예: Line 116 아래)에서 한 번만 계산하도록 추가해 주세요:
// FeedScreen 상단(collect 이후)에서 메모이즈 val subscriptionsForBar = remember(subscriptionUiState.followings) { subscriptionUiState.followings.map { user -> MySubscriptionData( profileImageUrl = user.profileImageUrl, nickname = user.nickname, role = user.aliasName, roleColor = colors.White, subscriberCount = 0, isSubscribed = user.isFollowing ) } }app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (1)
58-65: 무한 스크롤 트리거 안정성 개선(선택 사항)derivedStateOf 기반 isScrolledToEnd는 대부분의 케이스에서 동작하지만, 부분 가시 상태에서도 true가 될 수 있습니다. snapshotFlow + distinctUntilChanged로 안정적으로 트리거를 제어하는 방법을 고려해 주세요.
예시:
import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.snapshotFlow LaunchedEffect(lazyListState) { snapshotFlow { val layout = lazyListState.layoutInfo val last = layout.visibleItemsInfo.lastOrNull() // 마지막 아이템이 완전히 보일 때만 true last != null && last.index == layout.totalItemsCount - 1 && last.offset + last.size <= layout.viewportEndOffset - layout.afterContentPadding } .distinctUntilChanged() .filter { it && !uiState.isLoading && !uiState.isLastPage } .collect { viewModel.fetchMyFollowings() } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/texthip/thip/data/model/users/MyFollowingsResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.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(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/com/texthip/thip/ui/feed/viewmodel/MySubscriptionViewModel.kt
- app/src/main/java/com/texthip/thip/data/model/users/MyFollowingsResponse.kt
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/FeedNavigation.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (1)
MySubscribeBarlist(39-125)
🔇 Additional comments (3)
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
228-229: 내 띱 목록 네비게이션 연동 OKMySubscribeBarlist onClick에 onNavigateToMySubscription을 그대로 위임한 점 좋습니다. UI-네비게이션 분리에 부합합니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/MySubscriptionListScreen.kt (2)
67-71: 중복 호출 방지 가드 OKisScrolledToEnd에 대한 LaunchedEffect에서 isLoading/isLastPage 가드로 중복 호출을 방지하는 로직이 깔끔합니다. ViewModel에서 isLoading 플래그 전환만 정확히 이뤄지면 재호출 이슈는 없겠습니다.
ViewModel.fetchMyFollowings()가 호출 시점에 isLoading을 즉시 true로 설정하는지(낙관 락)만 재확인 부탁드립니다. 로컬 상태 전환이 지연되면 드물게 중복 호출이 발생할 수 있습니다.
151-160: AuthorHeader의 profileImage는 String? 타입으로 URL 문자열을 직접 받도록 설계되어 있습니다
–AuthorHeader(profileImage: String?, …)시그니처 확인 완료
현재user.profileImageUrl을 넘기는 코드는 의도된 동작이며 별도 조치가 필요 없습니다.
| roleColor = colors.SocialScience | ||
| ) | ||
| ) | ||
| val subscriptionUiState by viewModel.uiState.collectAsState() |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
수명 주기 인식 상태 수집으로 전환 권장: collectAsStateWithLifecycle
수명 주기와 동기화된 상태 수집으로 전환하면 백그라운드에서 불필요한 수집을 방지할 수 있습니다.
아래 변경을 제안합니다:
-import androidx.compose.runtime.collectAsState
+import androidx.lifecycle.compose.collectAsStateWithLifecycle-val subscriptionUiState by viewModel.uiState.collectAsState()
+val subscriptionUiState by viewModel.uiState.collectAsStateWithLifecycle()Gradle 의존성에 lifecycle-runtime-compose가 포함되어 있는지 확인해 주세요. 아래 스크립트로 점검할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# lifecycle-runtime-compose 의존성 존재 여부 확인
rg -n "lifecycle-runtime-compose" -A 2Length of output: 38
다음 스크립트로 Gradle 파일 내에 lifecycle-runtime-compose 의존성이 있는지 다시 한번 확인해주세요.
🏁 Script executed:
#!/bin/bash
# Gradle 파일 (*.gradle, *.gradle.kts) 내 lifecycle-runtime-compose 의존성 검색
rg -n "lifecycle-runtime-compose" --glob "*.gradle*" -A2Length of output: 56
수명 주기 인식 상태 수집으로 전환 및 의존성 추가 필요
아래와 같이 collectAsState를 collectAsStateWithLifecycle로 교체하고, 관련 Compose-용 Lifecycle 의존성을 Gradle에 추가해주세요.
위치: app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (116행)
제안 변경사항:
-import androidx.compose.runtime.collectAsState
+import androidx.lifecycle.compose.collectAsStateWithLifecycle-val subscriptionUiState by viewModel.uiState.collectAsState()
+val subscriptionUiState by viewModel.uiState.collectAsStateWithLifecycle()추가로, 프로젝트의 build.gradle 또는 build.gradle.kts에 다음 의존성을 선언해야 합니다.
(스크립트 확인 결과 이미 선언된 항목이 없어 직접 추가가 필요합니다.)
implementation "androidx.lifecycle:lifecycle-runtime-compose:<최신_버전>"<최신_버전>은 기존androidx.lifecycle의존성과 맞추거나 Maven Central에서 확인하세요.- 선언 후 프로젝트 Sync를 실행해 의존성이 정상 반영되는지 확인해주세요.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt around line
116, replace usage of viewModel.uiState.collectAsState() with
viewModel.uiState.collectAsStateWithLifecycle() and add the corresponding import
for the lifecycle Compose extension; then add the lifecycle runtime compose
dependency to the project build file (build.gradle or build.gradle.kts):
implementation "androidx.lifecycle:lifecycle-runtime-compose:<latest_version>"
where <latest_version> matches your existing androidx.lifecycle versions or the
latest on Maven Central, sync the project, and verify the new symbol resolves.
2 participants
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
리스트바 중앙정렬 수정했습니다~
bandicam.2025-08-13.21-52-02-057.mp4
😢 해결하지 못한 과제
📢 리뷰어들에게
Summary by CodeRabbit