Support backward pagination refresh#147
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR extends bidirectional cursor-based pagination across Timeline, Explore, and Bookmarks screens. GraphQL operations now accept forward and backward pagination parameters; repository methods conditionally build queries based on cursor direction; and viewmodels manage a separate "newer posts" StateFlow that prepends fetched items and updates the top cursor. ChangesBidirectional Pagination & Load Newer Posts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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. 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dec90a52a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/src/main/java/pub/hackers/android/ui/screens/explore/ExploreScreen.kt (1)
190-336: ⚡ Quick winExtract the
PostCardwiring to remove duplicated rendering for newer/paging items.The new
items(newerPosts...)block (lines 191-264) duplicates ~70 lines ofPostCardcallback wiring that already exists in the pagingitems(...)block (lines 266-336). The two blocks differ only in how they obtainpostand the LazyColumn key prefix; the entirePostCard(...)invocation includingonClick,onShareClick,onBookmarkClick,onExternalShareClick, etc. is byte-for-byte identical.This will drift the next time a callback is added or changed. Extract a private
@Composablehelper (e.g.ExplorePostItem(post, isLoggedIn, viewModel, …)) and call it from bothitemsblocks.♻️ Sketch of the extracted helper
`@Composable` private fun ExplorePostItem( post: Post, isLoggedIn: Boolean, colors: AppColors, context: Context, bookmarkedMessage: String, bookmarkRemovedMessage: String, onPostClick: (String) -> Unit, onProfileClick: (String) -> Unit, onReplyClick: (String) -> Unit, onQuoteClick: (String) -> Unit, viewModel: ExploreViewModel, ) { PostCard( post = post, onClick = { onPostClick(post.sharedPost?.id ?: post.id) }, onProfileClick = onProfileClick, onReplyClick = if (isLoggedIn) { { onReplyClick(post.sharedPost?.id ?: post.id) } } else null, // …rest identical to current wiring… onQuotedPostClick = onPostClick, ) HorizontalDivider( color = colors.divider, thickness = 1.dp, modifier = Modifier.padding(horizontal = 16.dp), ) }Then both
itemsblocks shrink to a singleExplorePostItem(post = …, …)call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/pub/hackers/android/ui/screens/explore/ExploreScreen.kt` around lines 190 - 336, The two LazyColumn item blocks duplicate the PostCard wiring; extract a private `@Composable` helper (e.g. ExplorePostItem) that takes a Post, isLoggedIn, colors, context, bookmarkedMessage, bookmarkRemovedMessage, onPostClick, onProfileClick, onReplyClick, onQuoteClick, viewModel, etc., and contains the PostCard invocation plus the HorizontalDivider; then replace both usages (the newerPosts items loop and the paging items loop) with a single call to ExplorePostItem(post = ..., ...) while preserving the keys ("newer-${post.sharedPost?.id ?: post.id}" for newerPosts and items.itemKey for paging) and passing the same viewModel and callbacks so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/pub/hackers/android/ui/screens/bookmarks/BookmarksScreen.kt`:
- Around line 67-68: The screen currently bases refresh and empty-state logic
only on items.itemCount but also renders a separate prepended stream newerPosts;
update all checks (where empty state is shown, pull-to-refresh fallback, and
conditional branches that use items.itemCount — e.g., the logic around
items.refresh()) to compute a combined visible count = items.itemCount +
newerPosts.size (or newerPosts.count()) and use that combined count for deciding
when to call items.refresh(), show empty state, or render other branches; locate
references to items.itemCount, newerPosts, selectedTab and replace the
single-source checks with the combined-visible-count check so prepended posts
prevent incorrect empty-state or refresh behavior.
In
`@app/src/main/java/pub/hackers/android/ui/screens/bookmarks/BookmarksViewModel.kt`:
- Around line 101-107: selectTab currently resets state but does not cancel
in-flight loadNewerPosts coroutines, allowing stale responses to call
prependNewerPosts and override topCursor/_newerPosts; fix by introducing a
cancellation/version guard: add a MutableStateFlow or incremental requestId
(e.g., newerRequestId) and/or keep the Job for loadNewerPosts, cancel it in
selectTab (alongside setting topCursor = null and _newerPosts = emptyList()),
and modify loadNewerPosts/prependNewerPosts to check the current selected tab or
requestId before applying results (ignore results whose requestId != current or
whose tab differs from _selectedTab). Ensure both the loadNewerPosts call sites
and prependNewerPosts reference this guard so stale responses are dropped.
In
`@app/src/main/java/pub/hackers/android/ui/screens/explore/ExploreViewModel.kt`:
- Around line 101-126: The coroutine in loadNewerPosts() must ignore results
from a previous tab: capture the requested tab (already done) and before
applying any state changes (calls to prependNewerPosts, assignments to
topCursor, _newerPosts.value, and any _uiState.update for error or success)
check that _selectedTab.value == tab; if not, return/ignore the result. Ensure
the guard is applied both in the onSuccess branch (before calling
prependNewerPosts or updating topCursor/_newerPosts) and in the onFailure branch
(before setting _uiState.error) so stale responses cannot overwrite the new
tab's state.
---
Nitpick comments:
In `@app/src/main/java/pub/hackers/android/ui/screens/explore/ExploreScreen.kt`:
- Around line 190-336: The two LazyColumn item blocks duplicate the PostCard
wiring; extract a private `@Composable` helper (e.g. ExplorePostItem) that takes a
Post, isLoggedIn, colors, context, bookmarkedMessage, bookmarkRemovedMessage,
onPostClick, onProfileClick, onReplyClick, onQuoteClick, viewModel, etc., and
contains the PostCard invocation plus the HorizontalDivider; then replace both
usages (the newerPosts items loop and the paging items loop) with a single call
to ExplorePostItem(post = ..., ...) while preserving the keys
("newer-${post.sharedPost?.id ?: post.id}" for newerPosts and items.itemKey for
paging) and passing the same viewModel and callbacks so behavior remains
identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63a6b610-44e7-4a98-861c-0d7010424b3b
📒 Files selected for processing (10)
app/src/main/graphql/pub/hackers/android/operations.graphqlapp/src/main/java/pub/hackers/android/data/paging/CursorPagingSource.ktapp/src/main/java/pub/hackers/android/data/repository/HackersPubRepository.ktapp/src/main/java/pub/hackers/android/domain/model/Models.ktapp/src/main/java/pub/hackers/android/ui/screens/bookmarks/BookmarksScreen.ktapp/src/main/java/pub/hackers/android/ui/screens/bookmarks/BookmarksViewModel.ktapp/src/main/java/pub/hackers/android/ui/screens/explore/ExploreScreen.ktapp/src/main/java/pub/hackers/android/ui/screens/explore/ExploreViewModel.ktapp/src/main/java/pub/hackers/android/ui/screens/timeline/TimelineScreen.ktapp/src/main/java/pub/hackers/android/ui/screens/timeline/TimelineViewModel.kt
Add newer-item refresh handling for timelines, bookmarks, notifications, search, and profile lists. Signed-off-by: Haze Lee <hazelee@realignist.me>
0dec90a to
b35ab65
Compare
This PR implements newer-item refresh handling for timelines, bookmarks, notifications, search, and profile lists. Which is related with hackers-pub/hackerspub#296 .
And This PR was assisted by Codex / GPT-5.5.