[feat/#30] 게시물 상세페이지 API #31
Conversation
WalkthroughThis PR implements community post detail retrieval functionality across the network, data, domain, and presentation layers. It introduces new DTOs for post details and nested comments, adds corresponding API endpoints and repository methods, refactors the post screen from static rendering to state-driven UI, and updates navigation to use Int instead of String for postId type. Changes
Sequence DiagramsequenceDiagram
actor User
participant Nav as Navigation
participant Screen as CommunityPostScreen
participant VM as CommunityViewModel
participant Repo as CommunityRepository
participant API as CommunityPostService
User->>Nav: Navigate to post detail (postId)
Nav->>Screen: CommunityPostRoute(postId)
Screen->>Screen: LaunchedEffect triggered
Screen->>VM: getPostDetail(postId)
VM->>VM: Update postDetailState to Loading
VM->>Repo: getCommunityDetail(postId)
Repo->>API: GET /posts/{postId}
API-->>Repo: Response<CommunityDetailResponseDto>
alt Success
Repo-->>VM: Result.Success
VM->>VM: Update postDetailState to Success(post)
else Error
Repo-->>VM: Result.Failure
VM->>VM: Update postDetailState to Error(message)
end
VM-->>Screen: postDetailState collected
Screen->>Screen: Render based on state
alt Loading State
Screen->>Screen: Show CircularProgressIndicator
else Error State
Screen->>Screen: Show error message
else Success State
Screen->>Screen: Render post header, content & comments
end
Screen-->>User: Display post detail
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/com/hsLink/hslink/domain/repository/community/CommunityRepository.kt (1)
15-15: Domain layer exposes data layer DTO.This interface method returns
CommunityDetailResponseDto, a data-layer type. As noted in the implementation review, this violates clean architecture by creating a dependency from domain to data layer.
🧹 Nitpick comments (4)
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt (1)
29-31: Good addition to address timeout issues.The timeout configuration correctly addresses the issue mentioned in the PR description. Setting all three timeouts to 30 seconds is reasonable for mobile applications with varying network conditions.
However, consider whether all API operations require the same timeout. For example:
- Quick read operations (like fetching post details) might work well with shorter timeouts
- File upload/download operations might need longer timeouts in the future
For now, this uniform configuration is acceptable, but you may want to implement per-endpoint timeouts if you encounter specific performance requirements later.
app/src/main/java/com/hsLink/hslink/data/repositoryimpl/CommunityRepositoryImpl.kt (1)
36-44: Consider introducing a domain model instead of exposing data layer DTOs.The repository implementation directly returns
CommunityDetailResponseDto(a data-layer DTO) to the domain layer. This creates a dependency from the domain layer on the data layer, which violates clean architecture principles. Consider introducing a domain model (e.g.,CommunityPostDetail) and mapping the DTO to this model within the repository implementation.Note: The existing
createCommunityPostmethod follows the same pattern, so this may be a broader architectural decision for the codebase.app/src/main/java/com/hsLink/hslink/presentation/community/viewmodel/CommunityViewModel.kt (1)
58-69: Consider externalizing the error message.The error message "게시물 상세 조회 실패" is hardcoded in Korean. For better maintainability and localization support, consider externalizing this string to a resource file.
app/src/main/java/com/hsLink/hslink/presentation/community/screen/post/CommunityPostScreen.kt (1)
17-18: Prefer lifecycle-aware state collection.
collectAsState()keeps collecting whenever the composable stays in the composition, even if the user navigates away but the destination is still on the back stack. Switching tocollectAsStateWithLifecycle()lets the flow respect the Lifecycle state and stops extra work when the screen is not resumed.Apply this diff:
-import androidx.compose.runtime.collectAsState +import androidx.lifecycle.compose.collectAsStateWithLifecycle ... - val postDetailState by viewModel.postDetailState.collectAsState() + val postDetailState by viewModel.postDetailState.collectAsStateWithLifecycle()Also applies to: 66-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/community/CommunityDetailResponseDto.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/CommunityRepositoryImpl.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/service/commuunity/CommunityPostService.kt(2 hunks)app/src/main/java/com/hsLink/hslink/domain/repository/community/CommunityRepository.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/navigation/post/CommunityPostNavigation.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/screen/post/CommunityPostScreen.kt(5 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/state/CommunityContract.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/community/viewmodel/CommunityViewModel.kt(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/src/main/java/com/hsLink/hslink/presentation/community/navigation/post/CommunityPostNavigation.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/community/screen/post/CommunityPostScreen.kt (1)
CommunityPostRoute(54-74)
app/src/main/java/com/hsLink/hslink/presentation/community/screen/post/CommunityPostScreen.kt (7)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)app/src/main/java/com/hsLink/hslink/presentation/community/component/post/PostHeader.kt (1)
PostHeader(15-52)app/src/main/java/com/hsLink/hslink/presentation/community/component/post/PostContent.kt (1)
PostContent(13-37)app/src/main/java/com/hsLink/hslink/presentation/community/component/post/EmptyComment.kt (1)
EmptyComment(15-38)app/src/main/java/com/hsLink/hslink/presentation/community/component/main/CommentItem.kt (1)
CommentItem(24-79)app/src/main/java/com/hsLink/hslink/presentation/community/component/post/CommentInput.kt (1)
CommentInput(23-79)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkDialog.kt (1)
HsLinkDialog(22-95)
🔇 Additional comments (5)
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt (1)
18-18: LGTM!The TimeUnit import is necessary for the timeout configurations below.
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt (1)
68-70: LGTM! Type migration from String to Int is consistent.The parameter type change from
StringtoIntforpostIdaligns with the API layer changes and improves type safety.app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt (1)
54-54: LGTM! Navigation wiring updated correctly.The navigation callback now correctly passes
postIdas anInt, consistent with the updatednavigateToCommunityPostsignature.app/src/main/java/com/hsLink/hslink/data/service/commuunity/CommunityPostService.kt (1)
26-29: LGTM! API endpoint properly defined.The new endpoint for fetching post details is correctly configured with
@Pathparameter binding and appropriate return type.app/src/main/java/com/hsLink/hslink/presentation/community/state/CommunityContract.kt (1)
15-19: LGTM! Well-structured state definition.The
CommunityDetailStatesealed interface properly models the three states of post detail loading with appropriate data for each case.
| composable<CommunityPost> { backStackEntry -> | ||
|
|
||
| val postId = backStackEntry.arguments?.getInt("postId") ?: -1 | ||
|
|
||
| CommunityPostRoute( | ||
| postId = postId, | ||
| paddingValues = padding, | ||
| navigateUp = navigateUp | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify error handling for invalid postId fallback.
The code extracts postId from backStackEntry.arguments with a fallback value of -1 (Line 25). If the navigation arguments are missing or malformed, -1 will be passed to CommunityPostRoute. Ensure that downstream code (particularly the API call and UI) properly handles this invalid ID case, or consider throwing an error/navigating back if the postId is missing.
Run the following script to check how -1 (or invalid postId) is handled in the detail flow:
🏁 Script executed:
#!/bin/bash
# Description: Check error handling for invalid postId values in the detail flow
# Search for postId validation or error handling in CommunityPostRoute and related components
ast-grep --pattern $'fun getPostDetail(postId: Int) {
$$$
}'
# Search for API error handling when postId is invalid
rg -n -A 5 -B 2 "getCommunityDetail|getPostDetail" --type kotlinLength of output: 6512
Let me verify how the invalid postId is handled in the UI and whether there are guards in place:
Now let me check the CommunityPostScreen to see if there's any UI handling for invalid postId:
Let me search for the actual CommunityPostScreen implementation to see the full picture:
Based on the script output and web research, I can now make a definitive assessment. The review comment raises a valid concern about error handling for invalid postId values. Let me verify the complete error handling flow by examining how the error state is handled in the screen:
Analysis Summary:
From the script execution results, I found:
- Navigation layer (CommunityPostNavigation.kt:25): Extracts postId with fallback value
-1 - ViewModel (CommunityViewModel.kt:58-69): Calls
repository.getCommunityDetail(postId)with.onFailureerror handler - Repository (CommunityRepositoryImpl.kt:36-43): Wraps API call in
runCatchingfor exception handling - API Service (CommunityPostService.kt:27): Makes HTTP GET request with postId as path parameter
- Screen (CommunityPostScreen.kt:63): Calls
viewModel.getPostDetail(postId)inLaunchedEffect(postId)
Current State: Error handling exists downstream (ViewModel's .onFailure handler), but there is no early validation for the invalid -1 postId before making the API call. Kotlin best practices recommend throwing exceptions for logic errors with conditions like invalid IDs, and Jetpack Compose documentation notes that providing default values and covering edge cases makes code safer.
Issues identified:
- API call is made with
-1(wasteful network request for known invalid ID) - No explicit validation guard in CommunityPostScreen using
LaunchedEffectcondition - Missing early error check before ViewModel invocation
Recommendation: Add validation for postId != -1 before calling viewModel.getPostDetail() to prevent unnecessary API calls and provide faster user feedback.
Add postId validation in CommunityPostScreen before API call.
Include a guard condition in LaunchedEffect to check if postId is valid (-1 indicates missing argument), and navigate back or show error immediately without making an unnecessary API request:
LaunchedEffect(postId) {
if (postId == -1) {
navigateUp() // or show error UI
return@LaunchedEffect
}
viewModel.getPostDetail(postId)
}This prevents wasteful API calls and provides faster error feedback when navigation arguments are missing.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/community/navigation/post/CommunityPostNavigation.kt
lines 23-31 the navigation extracts postId with a fallback of -1 but downstream
screens immediately call the ViewModel which triggers unnecessary API calls for
an invalid ID; update the CommunityPostScreen to validate postId before invoking
viewModel.getPostDetail by adding a guard in the LaunchedEffect (if postId == -1
then call navigateUp() or set an error state and return) so you avoid wasteful
network requests and provide immediate user feedback.
ISSUE
❗ WORK DESCRIPTION
📸 SCREENSHOT
Android.Emulator.-.google_5554.2025-11-12.21-42-36.mp4