fix: 마이페이지 및 피드소소 QA 반영 #802
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough다수의 피드 관련 로직과 UI를 리팩터링합니다. 다이얼로그 바인딩과 다이얼로그 레이아웃 추가, FeedDetail에서 새로고침 래퍼 제거, FeedOrder 열거형 및 MyFeedFilter API 변경, FeedRepository 캐시 구조 재설계와 FeedViewModel의 상태/페이징 로직 추가 등이 포함됩니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant UI as FeedScreen / UI
participant VM as FeedViewModel
participant Repo as FeedRepository
participant API as FeedApi (네트워크)
participant DB as LibraryLocalDataSource
UI->>VM: fetchNextPage(feedId?)
VM->>Repo: fetchFeeds(params...)
Repo->>API: GET /feeds (쿼리: lastFeedId,size,...)
API-->>Repo: FeedsResponse
Repo->>DB: syncNovelEntities(removed/updated)
DB-->>Repo: OK
Repo-->>VM: FeedsEntity(merged 캐시)
VM-->>UI: uiState 업데이트 (feeds, loading, isLoadable)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/feed/src/main/java/com/into/websoso/feature/feed/FeedScreen.kt (1)
231-234:⚠️ Potential issue | 🟡 Minor디버그 로그를 제거해주세요.
Log.d("123123", tab.toString())는 개발 중 디버깅 목적으로 추가된 것으로 보입니다. 프로덕션 코드에서는 제거하거나 의미 있는 태그로 변경해야 합니다.🗑️ 디버그 로그 제거
onClick = { - Log.d("123123", tab.toString()) onTabClick(tab) },
🤖 Fix all issues with AI agents
In `@app/src/main/res/layout/dialog_editing_cancel_feed.xml`:
- Around line 11-19: The ImageView with id iv_editing_cancel_feed is missing
accessibility info; decide if it's decorative or meaningful: if decorative, add
android:contentDescription="@null" and android:importantForAccessibility="no" on
iv_editing_cancel_feed to hide it from screen readers; if it conveys meaning,
add a descriptive contentDescription referencing a string resource (e.g.,
`@string/`...), and ensure it is not set focusable or clickable for unintended
traversal.
In
`@feature/feed/src/main/java/com/into/websoso/feature/feed/component/MyFeedFilterModal.kt`:
- Around line 138-140: 비공개글 토글의 상태 반전 로직이 역으로 되어 있습니다; in MyFeedFilterModal
(variable tempIsUnVisible) update the onClick handler to mirror 공개 로직: keep
isSelected = tempIsUnVisible == true, and change the onClick assignment from
using `tempIsUnVisible == false` to check `tempIsUnVisible == true` so that
clicking sets tempIsUnVisible = if (tempIsUnVisible == true) null else true
(i.e., select -> true, deselect -> null).
- Around line 156-158: onApplyFilterClick 호출에서 MyFeedFilter 생성자에 비공개 글 필터 값이
누락되어 있으니 .debouncedClickable 블록 내에서 MyFeedFilter(tempGenres, tempIsVisible) 대신
MyFeedFilter에 세 번째 인자로 tempIsUnVisible을 추가해 호출하도록 수정하세요; 대상 심볼은
onApplyFilterClick, MyFeedFilter, tempIsUnVisible, tempGenres, tempIsVisible입니다.
In `@feature/feed/src/main/java/com/into/websoso/feature/feed/FeedViewModel.kt`:
- Line 68: Remove the debug Log.d call introduced for development; locate the
Log.d("123123 카테고리", category.toString()) inside FeedViewModel (the method where
category is handled) and delete that statement (or replace it with a proper
production-level logger if persistent logging is required, e.g., using Timber or
a structured logger and an appropriate tag/message).
🧹 Nitpick comments (4)
app/src/main/res/layout/dialog_editing_cancel_feed.xml (1)
36-70: 버튼 터치 피드백(리플) 추가를 권장합니다.
현재 TextView 버튼에 시각적 클릭 피드백이 없어 UX가 약할 수 있습니다.✨ 제안 수정
<TextView android:id="@+id/tv_editing_cancel_feed_cancel_button" + android:foreground="?attr/selectableItemBackground" + android:clickable="true" + android:focusable="true" android:layout_width="wrap_content" android:layout_height="wrap_content" ... /> <TextView android:id="@+id/tv_editing_cancel_feed_alert_cancel_button" + android:foreground="?attr/selectableItemBackground" + android:clickable="true" + android:focusable="true" android:layout_width="wrap_content" android:layout_height="wrap_content" ... />app/src/main/res/layout/activity_feed_detail.xml (1)
69-74: WebsosoLoadingLayout에 constraint가 누락되었습니다.ConstraintLayout 내에서
WebsosoLoadingLayout이 constraint 없이match_parent로 설정되어 있습니다. ConstraintLayout에서는0dp와 함께 constraint를 명시하는 것이 권장됩니다. 현재 동작에는 문제가 없을 수 있지만, 명시적인 constraint 추가를 고려해 주세요.🔧 제안된 수정
<com.into.websoso.core.common.ui.custom.WebsosoLoadingLayout android:id="@+id/wll_feed" - android:layout_width="match_parent" - android:layout_height="match_parent" + android:layout_width="0dp" + android:layout_height="0dp" android:elevation="100dp" - android:visibility="invisible" /> + android:visibility="invisible" + app:layout_constraintBottom_toBottomOf="parent" + app:layout_constraintEnd_toEndOf="parent" + app:layout_constraintStart_toStartOf="parent" + app:layout_constraintTop_toTopOf="parent" />app/src/main/java/com/into/websoso/ui/feedDetail/FeedDetailActivity.kt (1)
579-579: 사용되지 않는 상수LOTTIE_IMAGE제거를 권장합니다.이 상수는 파일 내에서 전혀 사용되지 않으므로, pull-to-refresh 제거 시 함께 삭제하는 것이 좋습니다.
feature/feed/src/main/java/com/into/websoso/feature/feed/model/MyFeedFilter.kt (1)
5-6:isUnVisible네이밍이 일관적이지 않습니다.
isVisible과 쌍을 이루는 속성으로isUnVisible대신isInvisible또는isHidden이 더 자연스러운 네이밍입니다. "UnVisible"은 표준 영어 단어가 아닙니다.💡 네이밍 개선 제안
data class MyFeedFilter( val selectedGenres: Set<NovelCategory> = emptySet(), val isVisible: Boolean? = null, - val isUnVisible: Boolean? = null, + val isInvisible: Boolean? = null, )
Comment on lines
+11
to
+19
| <ImageView | ||
| android:id="@+id/iv_editing_cancel_feed" | ||
| android:layout_width="46dp" | ||
| android:layout_height="46dp" | ||
| android:layout_marginHorizontal="102dp" | ||
| android:src="@drawable/ic_novel_detail_warning" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent" /> |
There was a problem hiding this comment.
경고 아이콘 접근성 설정이 필요합니다.
장식용 이미지라면 스크린리더 노출을 막고, 의미가 있다면 설명을 추가하세요.
✅ 제안 수정
<ImageView
android:id="@+id/iv_editing_cancel_feed"
+ android:contentDescription="@null"
+ android:importantForAccessibility="no"
android:layout_width="46dp"
android:layout_height="46dp"📝 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.
Suggested change
| <ImageView | |
| android:id="@+id/iv_editing_cancel_feed" | |
| android:layout_width="46dp" | |
| android:layout_height="46dp" | |
| android:layout_marginHorizontal="102dp" | |
| android:src="@drawable/ic_novel_detail_warning" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintStart_toStartOf="parent" | |
| app:layout_constraintTop_toTopOf="parent" /> | |
| <ImageView | |
| android:id="@+id/iv_editing_cancel_feed" | |
| android:contentDescription="@null" | |
| android:importantForAccessibility="no" | |
| android:layout_width="46dp" | |
| android:layout_height="46dp" | |
| android:layout_marginHorizontal="102dp" | |
| android:src="@drawable/ic_novel_detail_warning" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintStart_toStartOf="parent" | |
| app:layout_constraintTop_toTopOf="parent" /> |
🤖 Prompt for AI Agents
In `@app/src/main/res/layout/dialog_editing_cancel_feed.xml` around lines 11 - 19,
The ImageView with id iv_editing_cancel_feed is missing accessibility info;
decide if it's decorative or meaningful: if decorative, add
android:contentDescription="@null" and android:importantForAccessibility="no" on
iv_editing_cancel_feed to hide it from screen readers; if it conveys meaning,
add a descriptive contentDescription referencing a string resource (e.g.,
`@string/`...), and ensure it is not set focusable or clickable for unintended
traversal.
feature/feed/src/main/java/com/into/websoso/feature/feed/component/MyFeedFilterModal.kt
Show resolved
Hide resolved
feature/feed/src/main/java/com/into/websoso/feature/feed/component/MyFeedFilterModal.kt
Show resolved
Hide resolved
feature/feed/src/main/java/com/into/websoso/feature/feed/FeedViewModel.kt
Outdated
Show resolved
Hide resolved
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/network/src/main/java/com/into/websoso/core/network/datasource/user/UserApi.kt (1)
18-26:⚠️ Potential issue | 🟡 Minor쿼리 파라미터 키와 변수명 불일치를 변수명 변경으로 개선하세요.
Line 22의
@Query("genreNames")어노테이션과genres변수명이 일치하지 않습니다. Retrofit이 올바르게 "genreNames"를 백엔드에 전송하므로 기능상 문제는 없지만, 가독성과 유지보수성을 위해 변수명을genreNames로 변경하는 것을 권장합니다. 이렇게 하면 쿼리 파라미터와 변수명이 일치하여 코드의 의도가 더 명확해집니다.
🤖 Fix all issues with AI agents
In
`@data/feed/src/main/java/com/into/websoso/data/feed/repository/FeedRepository.kt`:
- Line 13: Remove the lingering TODO comment in FeedRepository.kt ("// TODO: 이
부분 확인 후 수정 후 주석 모두 제거"): inspect the surrounding code in the FeedRepository
class or relevant methods, implement the intended fix or confirmation (or
create/attach a tracking ticket if work remains), run tests/validation, then
delete the TODO line; if the change is non-trivial, replace the TODO with a
clear actionable comment referencing the ticket ID and owner instead of leaving
a generic TODO.
- Around line 22-23: The two mutable lists _cachedFeeds and
_cachedRecommendedFeeds are not thread-safe in the `@Singleton` FeedRepository;
add a Mutex (e.g., cacheMutex = Mutex()) and wrap every read/write access to
_cachedFeeds and _cachedRecommendedFeeds with cacheMutex.withLock to serialize
concurrent coroutine access, or alternatively convert them into
MutableStateFlow<List<FeedEntity>> and update via immutable copies to ensure
atomic updates; ensure all repository methods that currently touch
_cachedFeeds/_cachedRecommendedFeeds (reads, adds, removes, clears) are updated
to use the chosen synchronization approach so no unsynchronized access remains.
- Around line 89-91: The onFailure block inside FeedRepository.saveRemovedFeed
currently swallows exceptions and logs with Log.d; change it to propagate the
error (either rethrow the exception or change saveRemovedFeed to return a
Result/Outcome type) so callers (ViewModel) can handle deletion failures, and
replace Log.d with Log.e (or include throwable) for proper error logging; update
callers to handle the returned Result or catch the rethrown exception.
In
`@feature/feed/src/main/java/com/into/websoso/feature/feed/component/FeedSection.kt`:
- Around line 188-195: Remove the temporary TODO comments around the
onFirstItemClick and onSecondItemClick invocations in FeedSection.kt; ensure the
calls pass feed.id and feed.isMyFeed as currently implemented
(onFirstItemClick(feed.id, feed.isMyFeed) and onSecondItemClick(feed.id,
feed.isMyFeed)), delete both TODO lines and any related explanatory comments
left for reviewers, and keep only the necessary code invoking these handlers.
In `@feature/feed/src/main/java/com/into/websoso/feature/feed/FeedViewModel.kt`:
- Around line 241-259: The issue is that FeedRepository.saveRemovedFeed swallows
API errors so FeedViewModel's runCatching always treats the call as success and
removes the UI item; fix by making the failure propagate and updating the
ViewModel to only mutate _uiState on real success: either change
FeedRepository.saveRemovedFeed to rethrow the exception (or return a
Result/boolean indicating success) instead of just logging, and in FeedViewModel
(inside viewModelScope.launch where runCatching calls saveRemovedFeed) rely on
the thrown exception or the returned Result to enter onFailure before calling
_uiState.update; ensure the symbols involved are FeedRepository.saveRemovedFeed,
FeedViewModel.runCatching (the lambda in viewModelScope.launch), and
_uiState.update so the UI removal happens exclusively in the confirmed-success
path.
🧹 Nitpick comments (1)
feature/feed/src/main/java/com/into/websoso/feature/feed/FeedViewModel.kt (1)
79-86: 빠른 연속 호출 시 중복 데이터 가능성이 있습니다.
loading가드와_uiState.update { it.copy(loading = true) }사이에 경합 조건이 존재합니다. 두 번의 빠른fetchNextPage()호출이 둘 다 가드를 통과할 수 있습니다.♻️ 원자적 가드 패턴 적용
fun fetchNextPage(feedId: Long? = null) { - val state = uiState.value - val current = state.currentData - val lastFeedId = feedId ?: current.lastId - - if (state.loading || !current.isLoadable) return - - _uiState.update { it.copy(loading = true) } + val snapshot = _uiState.updateAndGet { state -> + if (state.loading || !state.currentData.isLoadable) return + state.copy(loading = true) + } + val lastFeedId = feedId ?: snapshot.currentData.lastId또는 별도의
AtomicBoolean플래그를 사용하여 동기화할 수 있습니다.
data/feed/src/main/java/com/into/websoso/data/feed/repository/FeedRepository.kt
Show resolved
Hide resolved
Comment on lines
22
to
23
| private val _cachedFeeds = mutableListOf<FeedEntity>() | ||
| private val _cachedRecommendedFeeds = mutableListOf<FeedEntity>() |
There was a problem hiding this comment.
@singleton에서 mutableListOf 사용은 스레드 안전하지 않습니다.
여러 코루틴이 동시에 _cachedFeeds와 _cachedRecommendedFeeds에 접근할 수 있습니다. mutableListOf는 스레드 안전하지 않아 동시 수정 시 데이터 불일치나 ConcurrentModificationException이 발생할 수 있습니다.
🔒 스레드 안전한 구조로 변경
- private val _cachedFeeds = mutableListOf<FeedEntity>()
- private val _cachedRecommendedFeeds = mutableListOf<FeedEntity>()
+ private val _cachedFeeds = Collections.synchronizedList(mutableListOf<FeedEntity>())
+ private val _cachedRecommendedFeeds = Collections.synchronizedList(mutableListOf<FeedEntity>())또는 Mutex를 사용한 동기화나 StateFlow/MutableSharedFlow 기반 구조를 고려해주세요.
📝 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.
Suggested change
| private val _cachedFeeds = mutableListOf<FeedEntity>() | |
| private val _cachedRecommendedFeeds = mutableListOf<FeedEntity>() | |
| private val _cachedFeeds = Collections.synchronizedList(mutableListOf<FeedEntity>()) | |
| private val _cachedRecommendedFeeds = Collections.synchronizedList(mutableListOf<FeedEntity>()) |
🤖 Prompt for AI Agents
In
`@data/feed/src/main/java/com/into/websoso/data/feed/repository/FeedRepository.kt`
around lines 22 - 23, The two mutable lists _cachedFeeds and
_cachedRecommendedFeeds are not thread-safe in the `@Singleton` FeedRepository;
add a Mutex (e.g., cacheMutex = Mutex()) and wrap every read/write access to
_cachedFeeds and _cachedRecommendedFeeds with cacheMutex.withLock to serialize
concurrent coroutine access, or alternatively convert them into
MutableStateFlow<List<FeedEntity>> and update via immutable copies to ensure
atomic updates; ensure all repository methods that currently touch
_cachedFeeds/_cachedRecommendedFeeds (reads, adds, removes, clears) are updated
to use the chosen synchronization approach so no unsynchronized access remains.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Comment on lines
+89
to
91
| }.onFailure { | ||
| Log.d("FeedRepository", "saveRemovedFeed 함수 failed : ${it.message}") | ||
| } |
There was a problem hiding this comment.
삭제 실패 시 에러가 전파되지 않습니다.
onFailure에서 로그만 남기고 예외를 무시하면, 호출자(ViewModel)는 삭제 실패 여부를 알 수 없습니다. UI에서는 피드가 삭제된 것처럼 보이지만 실제로는 삭제되지 않은 상태가 될 수 있습니다.
또한 Log.d는 에러 로깅에 적합하지 않습니다.
🐛 에러 전파 및 로그 레벨 수정
}.onFailure {
- Log.d("FeedRepository", "saveRemovedFeed 함수 failed : ${it.message}")
+ Log.e("FeedRepository", "saveRemovedFeed 함수 failed : ${it.message}", it)
+ throw it
}또는 Result 타입을 반환하여 호출자가 에러를 처리할 수 있도록 해주세요.
🤖 Prompt for AI Agents
In
`@data/feed/src/main/java/com/into/websoso/data/feed/repository/FeedRepository.kt`
around lines 89 - 91, The onFailure block inside FeedRepository.saveRemovedFeed
currently swallows exceptions and logs with Log.d; change it to propagate the
error (either rethrow the exception or change saveRemovedFeed to return a
Result/Outcome type) so callers (ViewModel) can handle deletion failures, and
replace Log.d with Log.e (or include throwable) for proper error logging; update
callers to handle the returned Result or catch the rethrown exception.
feature/feed/src/main/java/com/into/websoso/feature/feed/component/FeedSection.kt
Show resolved
Hide resolved
feature/feed/src/main/java/com/into/websoso/feature/feed/FeedViewModel.kt
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
Summary by CodeRabbit
릴리스 노트
새 기능
버그 수정 / 동작 개선
스타일
✏️ Tip: You can customize this high-level summary in your review settings.