mirror of
https://codeberg.org/secana/Forji.git
synced 2026-06-16 05:13:55 -07:00
fix: settle merged pagination and drop duplicate rows (#47)
PaginationState computed hasMore as `fetched.count >= pageSize`, but merged
views return the combined, de-duplicated result of N instances, so the merged
count cleared the single-source threshold and kept hasMore true past the real
end (extra empty fetches). loadMore also appended without de-duping against
already-loaded items, so overlapping involvement queries and shifting pages
could produce duplicate TaggedItem.id entries in the ForEach.
Add an optional dedupeKey to PaginationState that de-duplicates across pages via
a running seen-set, and base hasMore on whether a page contributed new items
when dedupeKey is set. Single-source pagination keeps the page-size heuristic
unchanged. Merged Issues/PRs, Repositories, and Notifications views opt in with
dedupeKey = { $0.id }.
This commit is contained in:
parent
66fe573cb6
commit
bec427d7da
4 changed files with 35 additions and 4 deletions
|
|
@ -15,6 +15,13 @@ final class PaginationState<Item> {
|
||||||
let pageSize: Int
|
let pageSize: Int
|
||||||
var cacheName: String?
|
var cacheName: String?
|
||||||
|
|
||||||
|
/// Optional key extractor enabling cross-page de-duplication. Merged views set
|
||||||
|
/// this because they combine paginated results from several instances, where
|
||||||
|
/// the same item can resurface on a later page (overlapping involvement
|
||||||
|
/// queries) and the merged page size no longer matches `pageSize`.
|
||||||
|
@ObservationIgnored var dedupeKey: ((Item) -> AnyHashable)?
|
||||||
|
@ObservationIgnored private var seenKeys = Set<AnyHashable>()
|
||||||
|
|
||||||
init(pageSize: Int = 20) {
|
init(pageSize: Int = 20) {
|
||||||
self.pageSize = pageSize
|
self.pageSize = pageSize
|
||||||
}
|
}
|
||||||
|
|
@ -42,8 +49,10 @@ final class PaginationState<Item> {
|
||||||
do {
|
do {
|
||||||
let fetched = try await fetch(1, pageSize)
|
let fetched = try await fetch(1, pageSize)
|
||||||
guard !Task.isCancelled else { return }
|
guard !Task.isCancelled else { return }
|
||||||
items = fetched
|
if dedupeKey != nil { seenKeys.removeAll() }
|
||||||
hasMore = fetched.count >= pageSize
|
let newItems = deduped(fetched)
|
||||||
|
items = newItems
|
||||||
|
hasMore = computeHasMore(fetched: fetched, newItems: newItems)
|
||||||
currentPage = 2
|
currentPage = 2
|
||||||
hasLoaded = true
|
hasLoaded = true
|
||||||
} catch is CancellationError {
|
} catch is CancellationError {
|
||||||
|
|
@ -72,8 +81,9 @@ final class PaginationState<Item> {
|
||||||
do {
|
do {
|
||||||
let fetched = try await fetch(currentPage, pageSize)
|
let fetched = try await fetch(currentPage, pageSize)
|
||||||
guard !Task.isCancelled else { return }
|
guard !Task.isCancelled else { return }
|
||||||
items.append(contentsOf: fetched)
|
let newItems = deduped(fetched)
|
||||||
hasMore = fetched.count >= pageSize
|
items.append(contentsOf: newItems)
|
||||||
|
hasMore = computeHasMore(fetched: fetched, newItems: newItems)
|
||||||
self.currentPage = currentPage + 1
|
self.currentPage = currentPage + 1
|
||||||
} catch is CancellationError {
|
} catch is CancellationError {
|
||||||
// Ignore cancellation
|
// Ignore cancellation
|
||||||
|
|
@ -95,6 +105,24 @@ final class PaginationState<Item> {
|
||||||
func invalidate() {
|
func invalidate() {
|
||||||
hasLoaded = false
|
hasLoaded = false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Filters out items already seen on a previous page when `dedupeKey` is set,
|
||||||
|
/// recording the new keys. Without a `dedupeKey` the page passes through
|
||||||
|
/// unchanged so single-source pagination keeps its existing behavior.
|
||||||
|
private func deduped(_ fetched: [Item]) -> [Item] {
|
||||||
|
guard let dedupeKey else { return fetched }
|
||||||
|
return fetched.filter { seenKeys.insert(dedupeKey($0)).inserted }
|
||||||
|
}
|
||||||
|
|
||||||
|
/// For merged sources the de-duplicated, multi-instance page size no longer
|
||||||
|
/// matches `pageSize`, so "more pages exist" means "this page contributed new
|
||||||
|
/// items." Single sources keep the page-size heuristic.
|
||||||
|
private func computeHasMore(fetched: [Item], newItems: [Item]) -> Bool {
|
||||||
|
if dedupeKey != nil {
|
||||||
|
return !newItems.isEmpty
|
||||||
|
}
|
||||||
|
return fetched.count >= pageSize
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Disk cache
|
// MARK: - Disk cache
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@ struct MergedNotificationsOverviewView: View {
|
||||||
self.manager = manager
|
self.manager = manager
|
||||||
let state = PaginationState<TaggedItem<NotificationThread>>()
|
let state = PaginationState<TaggedItem<NotificationThread>>()
|
||||||
state.cacheName = "merged_notifications"
|
state.cacheName = "merged_notifications"
|
||||||
|
state.dedupeKey = { $0.id }
|
||||||
state.loadFromCache()
|
state.loadFromCache()
|
||||||
state.rehydrate(from: manager)
|
state.rehydrate(from: manager)
|
||||||
_pagination = State(initialValue: state)
|
_pagination = State(initialValue: state)
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,7 @@ struct MergedRepositoryListView: View {
|
||||||
self.manager = manager
|
self.manager = manager
|
||||||
let state = PaginationState<TaggedItem<Repository>>()
|
let state = PaginationState<TaggedItem<Repository>>()
|
||||||
state.cacheName = "merged_repos"
|
state.cacheName = "merged_repos"
|
||||||
|
state.dedupeKey = { $0.id }
|
||||||
state.loadFromCache()
|
state.loadFromCache()
|
||||||
state.rehydrate(from: manager)
|
state.rehydrate(from: manager)
|
||||||
_pagination = State(initialValue: state)
|
_pagination = State(initialValue: state)
|
||||||
|
|
|
||||||
|
|
@ -68,6 +68,7 @@ struct MergedSearchableOverviewView<Row: View, Detail: View, CreateView: View>:
|
||||||
|
|
||||||
let state = PaginationState<TaggedItem<Issue>>()
|
let state = PaginationState<TaggedItem<Issue>>()
|
||||||
state.cacheName = "merged_\(config.issueType)"
|
state.cacheName = "merged_\(config.issueType)"
|
||||||
|
state.dedupeKey = { $0.id }
|
||||||
state.loadFromCache()
|
state.loadFromCache()
|
||||||
state.rehydrate(from: manager)
|
state.rehydrate(from: manager)
|
||||||
_pagination = State(initialValue: state)
|
_pagination = State(initialValue: state)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue