mirror of
https://codeberg.org/secana/Forji.git
synced 2026-06-16 05:13:55 -07:00
fix: de-duplicate involved-scope results across pages in single-instance overviews (#70)
**Why.** The Issues and Pull Requests overviews fire one search request per involvement flag (created/assigned/mentioned/review-requested) and merge the results, so the same issue can resurface on a later page — created on page 1, assigned on page 2. The merged multi-instance overview already de-duplicates across pages with a pagination dedupe key, but the single-instance overview never set one, so loading more appended duplicate rows with duplicate Identifiable ids. **What changed.** `SearchableOverviewView.init` sets the same `dedupeKey` the merged overview sets (`MergedSearchableOverviewView` line 71), routing pages through `PaginationState`'s existing cross-page dedupe. The `dedupeKey` doc comment is updated to match. `hasMore` switches to the contributed-new-items heuristic, which fits combined per-flag sources whose merged page size never matches the request limit. **Verification.** New `PaginationStateDedupeTests` pin the keyed path: a key repeated across pages is filtered out on the later page, `hasMore` follows `!newItems.isEmpty` when `dedupeKey` is set (including a full page of already-seen keys ending pagination), and a reload resets the seen keys. Full `ForjiTests` suite passes (0 failed, iPhone 17 Pro, iOS 26.5). SwiftLint clean with the repo config. I grant Stefan Hausotte an irrevocable, worldwide, royalty-free license to use, sublicense, and distribute my contribution, including through Apple's App Store under the project's App Store exception. Reviewed-on: https://codeberg.org/secana/Forji/pulls/70
This commit is contained in:
parent
617e687e7b
commit
100d7c412c
3 changed files with 52 additions and 4 deletions
|
|
@ -16,10 +16,10 @@ final class PaginationState<Item> {
|
|||
let pageSize: Int
|
||||
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`.
|
||||
/// Optional key extractor enabling cross-page de-duplication. Views that
|
||||
/// combine several paginated sources set this (merged multi-instance lists,
|
||||
/// or the per-flag involvement queries), because the same item can resurface
|
||||
/// on a later page and the combined page size no longer matches `pageSize`.
|
||||
@ObservationIgnored var dedupeKey: ((Item) -> AnyHashable)?
|
||||
@ObservationIgnored private var seenKeys = Set<AnyHashable>()
|
||||
|
||||
|
|
|
|||
|
|
@ -59,6 +59,7 @@ struct SearchableOverviewView<Row: View, Detail: View, CreateView: View>: View {
|
|||
|
||||
let state = PaginationState<Issue>()
|
||||
state.cacheName = issueType
|
||||
state.dedupeKey = { $0.id }
|
||||
state.loadFromCache()
|
||||
_pagination = State(initialValue: state)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -475,3 +475,50 @@ struct PaginationStateLoadMoreTests {
|
|||
#expect(!pagination.isLoading)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Keyed de-duplication
|
||||
|
||||
struct PaginationStateDedupeTests {
|
||||
|
||||
@Test @MainActor func loadMoreFiltersKeysSeenOnEarlierPages() async {
|
||||
let pagination = PaginationState<String>(pageSize: 2)
|
||||
pagination.dedupeKey = { $0 }
|
||||
await pagination.reload { _, _ in ["a", "b"] }.value
|
||||
|
||||
// "b" resurfaces on page 2 — only "c" is new
|
||||
await pagination.loadMore { _, _ in ["b", "c"] }
|
||||
#expect(pagination.items == ["a", "b", "c"])
|
||||
#expect(pagination.hasMore) // the page contributed a new item
|
||||
}
|
||||
|
||||
@Test @MainActor func hasMoreFollowsNewItemsNotPageSize() async {
|
||||
let pagination = PaginationState<String>(pageSize: 5)
|
||||
pagination.dedupeKey = { $0 }
|
||||
await pagination.reload { _, _ in ["a", "b"] }.value
|
||||
#expect(pagination.hasMore) // 2 < pageSize 5, but the page contributed new items
|
||||
}
|
||||
|
||||
@Test @MainActor func hasMoreFalseWhenFullPageIsAllDuplicates() async {
|
||||
let pagination = PaginationState<String>(pageSize: 2)
|
||||
pagination.dedupeKey = { $0 }
|
||||
await pagination.reload { _, _ in ["a", "b"] }.value
|
||||
#expect(pagination.hasMore)
|
||||
|
||||
// 2 >= pageSize 2, but every key was already seen -> pagination ends
|
||||
await pagination.loadMore { _, _ in ["a", "b"] }
|
||||
#expect(pagination.items == ["a", "b"])
|
||||
#expect(!pagination.hasMore)
|
||||
}
|
||||
|
||||
@Test @MainActor func reloadResetsSeenKeys() async {
|
||||
let pagination = PaginationState<String>(pageSize: 2)
|
||||
pagination.dedupeKey = { $0 }
|
||||
await pagination.reload { _, _ in ["a", "b"] }.value
|
||||
await pagination.loadMore { _, _ in ["b", "c"] }
|
||||
#expect(pagination.items == ["a", "b", "c"])
|
||||
|
||||
// A fresh reload must accept keys seen in the previous cycle
|
||||
await pagination.reload { _, _ in ["a", "b"] }.value
|
||||
#expect(pagination.items == ["a", "b"])
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue