From 3611d941247a7163fce9c61d8f75a81ddedd90b7 Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 22:38:22 -0500 Subject: fix: start a new account at zero unread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An absent view record read as unread, so on first sign-in every thread a list had ever carried was unread. On a busy list that is thousands of threads the user never intended to read, inflating the Home dashboard, the needs-attention snapshot, and the widget with a number that means nothing. Record a baseline when an account is activated: mail that predates it is read, mail after it is not. activate() is the single funnel for launch validation, account switching, and adding an account, so one call covers every sign-in path. Accounts that already carry read state have been in use, so they get a distantPast baseline and keep every unread thread they had — upgrading must not silently mark a real backlog as read. markUnread now records an explicit distantPast marker instead of deleting the entry. Deleting would drop the thread back to the baseline rule, so marking an old thread unread would appear to do nothing. --- Hutch/App/AppState.swift | 3 + Hutch/Models/Inbox.swift | 45 +++++++++++++-- HutchTests/InboxViewModelTests.swift | 104 +++++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 4 deletions(-) diff --git a/Hutch/App/AppState.swift b/Hutch/App/AppState.swift index 479f610..ad31c5c 100644 --- a/Hutch/App/AppState.swift +++ b/Hutch/App/AppState.swift @@ -561,6 +561,9 @@ final class AppState { UserDefaults.standard.set(session.account.id, forKey: AppStorageKeys.activeAccountID) ActiveAccountContextStore.save(session.account.id) ContributionWidgetContextStore.saveActor(session.user.canonicalName, accountID: session.account.id) + // Every sign-in path funnels through here, so this is where an account + // first learns which mail predates it. + InboxReadStateStore.establishBaselineIfNeeded(defaults: session.defaults) authStatusMessage = "Connecting…" } diff --git a/Hutch/Models/Inbox.swift b/Hutch/Models/Inbox.swift index be2e105..c8e41bb 100644 --- a/Hutch/Models/Inbox.swift +++ b/Hutch/Models/Inbox.swift @@ -170,6 +170,7 @@ struct InboxPatchPreview: Decodable, Sendable, Hashable { enum InboxReadStateStore { private static let key = "InboxThreadLastViewed" + private static let baselineKey = "InboxUnreadBaseline" static func lastViewedAt(for threadID: String, defaults: UserDefaults = .standard) -> Date? { guard let dictionary = defaults.dictionary(forKey: key) as? [String: TimeInterval], @@ -185,16 +186,52 @@ enum InboxReadStateStore { defaults.set(dictionary, forKey: key) } + /// Records an explicit unread marker rather than forgetting the thread. + /// + /// Deleting the entry would drop the thread back to the baseline rule below, + /// which would call anything older than the baseline read — so marking an old + /// thread unread would appear to do nothing. `distantPast` always compares as + /// older than the thread's activity, so the thread reads as unread. static func markUnread(for threadID: String, defaults: UserDefaults = .standard) { var dictionary = defaults.dictionary(forKey: key) as? [String: TimeInterval] ?? [:] - dictionary.removeValue(forKey: threadID) + dictionary[threadID] = Date.distantPast.timeIntervalSince1970 defaults.set(dictionary, forKey: key) } + /// Mail that arrived before this is treated as already read. + static func baseline(defaults: UserDefaults = .standard) -> Date? { + guard let timestamp = defaults.object(forKey: baselineKey) as? TimeInterval else { + return nil + } + return Date(timeIntervalSince1970: timestamp) + } + + /// Sets the point from which mail counts as unread. Called once per account, + /// when the account is activated. + /// + /// Without this, every thread a list has ever carried is unread on first + /// login, because an absent view record reads as unread. On a busy list that + /// is thousands of threads, none of which the user has any intention of + /// reading. + /// + /// An account that already has read state has been in use, so it keeps the + /// old behavior — a baseline of `distantPast` leaves every existing unread + /// thread unread rather than silently marking a real backlog as read. + static func establishBaselineIfNeeded(now: Date = .now, defaults: UserDefaults = .standard) { + guard defaults.object(forKey: baselineKey) == nil else { return } + + let hasExistingReadState = !((defaults.dictionary(forKey: key) as? [String: TimeInterval])?.isEmpty ?? true) + let baseline = hasExistingReadState ? Date.distantPast : now + defaults.set(baseline.timeIntervalSince1970, forKey: baselineKey) + } + static func isUnread(threadID: String, lastActivityAt: Date, defaults: UserDefaults = .standard) -> Bool { - guard let lastViewedAt = lastViewedAt(for: threadID, defaults: defaults) else { - return true + if let lastViewedAt = lastViewedAt(for: threadID, defaults: defaults) { + return lastActivityAt > lastViewedAt + } + if let baseline = baseline(defaults: defaults), lastActivityAt <= baseline { + return false } - return lastActivityAt > lastViewedAt + return true } } diff --git a/HutchTests/InboxViewModelTests.swift b/HutchTests/InboxViewModelTests.swift index 7bfc199..92f380a 100644 --- a/HutchTests/InboxViewModelTests.swift +++ b/HutchTests/InboxViewModelTests.swift @@ -41,6 +41,110 @@ struct InboxViewModelTests { #expect(!InboxReadStateStore.isUnread(threadID: threadID, lastActivityAt: lastActivity, defaults: defaults)) } + @Test + func freshAccountTreatsExistingMailAsRead() { + let suiteName = "InboxViewModelTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + let signIn = Date(timeIntervalSince1970: 5_000) + InboxReadStateStore.establishBaselineIfNeeded(now: signIn, defaults: defaults) + + // Years of list history should not land on a new user as unread. + #expect( + !InboxReadStateStore.isUnread( + threadID: "list#old", + lastActivityAt: Date(timeIntervalSince1970: 4_000), + defaults: defaults + ) + ) + } + + @Test + func mailArrivingAfterSignInIsUnread() { + let suiteName = "InboxViewModelTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + InboxReadStateStore.establishBaselineIfNeeded(now: Date(timeIntervalSince1970: 5_000), defaults: defaults) + + #expect( + InboxReadStateStore.isUnread( + threadID: "list#new", + lastActivityAt: Date(timeIntervalSince1970: 6_000), + defaults: defaults + ) + ) + } + + @Test + func mailExactlyAtTheBaselineIsRead() { + let suiteName = "InboxViewModelTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + let signIn = Date(timeIntervalSince1970: 5_000) + InboxReadStateStore.establishBaselineIfNeeded(now: signIn, defaults: defaults) + + #expect(!InboxReadStateStore.isUnread(threadID: "list#edge", lastActivityAt: signIn, defaults: defaults)) + } + + @Test + func baselineIsEstablishedOnceAndNotMovedBySubsequentSignIns() { + let suiteName = "InboxViewModelTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + InboxReadStateStore.establishBaselineIfNeeded(now: Date(timeIntervalSince1970: 5_000), defaults: defaults) + // A later launch must not silently mark the backlog read. + InboxReadStateStore.establishBaselineIfNeeded(now: Date(timeIntervalSince1970: 9_000), defaults: defaults) + + #expect( + InboxReadStateStore.isUnread( + threadID: "list#since", + lastActivityAt: Date(timeIntervalSince1970: 6_000), + defaults: defaults + ) + ) + } + + @Test + func existingAccountsKeepTheirUnreadBacklog() { + let suiteName = "InboxViewModelTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + // An account already carrying read state has been in use, so upgrading + // must not retroactively mark everything it had not read as read. + InboxReadStateStore.markViewed(Date(timeIntervalSince1970: 1_000), for: "list#seen", defaults: defaults) + InboxReadStateStore.establishBaselineIfNeeded(now: Date(timeIntervalSince1970: 5_000), defaults: defaults) + + #expect( + InboxReadStateStore.isUnread( + threadID: "list#unseen", + lastActivityAt: Date(timeIntervalSince1970: 4_000), + defaults: defaults + ) + ) + } + + @Test + func markingAnOldThreadUnreadSurvivesTheBaseline() { + let suiteName = "InboxViewModelTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + InboxReadStateStore.establishBaselineIfNeeded(now: Date(timeIntervalSince1970: 5_000), defaults: defaults) + let oldActivity = Date(timeIntervalSince1970: 4_000) + + #expect(!InboxReadStateStore.isUnread(threadID: "list#old", lastActivityAt: oldActivity, defaults: defaults)) + + // Explicitly marking it unread must stick, rather than falling back to the + // baseline rule and reading as read again. + InboxReadStateStore.markUnread(for: "list#old", defaults: defaults) + #expect(InboxReadStateStore.isUnread(threadID: "list#old", lastActivityAt: oldActivity, defaults: defaults)) + } + @Test func normalizesThreadSubjectsForDisplay() { let summary = InboxThreadSummary( -- cgit v1.2.3 From 91023a6572354af7770309715d1d77e978298b0d Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 22:51:41 -0500 Subject: fix: let pull to refresh actually refresh Home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New mail showed as unread inside a mailing list but not on Home. The list view reads threads with client.execute, so it always sees the server. Home reads them with executeCached under cacheFirstThenRefresh, which returns the cached value and only schedules a background fetch. loadDashboard had no way to bypass that, so pulling to refresh could not surface mail that arrived after the last fetch — it returned the same cache it already had and Home stayed a cycle behind. Thread forceRefresh through loadDashboard to the cached reads, and use it from the pull to refresh on Home and Work. Matches loadSystemStatus(forceRefresh:), which MoreView already refreshes this way. The staleness predates the unread baseline. It was invisible while every thread counted as unread, because Home always had a large number to show; now that Home correctly ignores mail older than sign-in, a stale cache reads as a truthful 0. --- Hutch/Views/Home/HomeView.swift | 2 +- Hutch/Views/Home/HomeViewModel.swift | 61 +++++++++++++++++++----------------- Hutch/Views/Work/WorkView.swift | 2 +- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/Hutch/Views/Home/HomeView.swift b/Hutch/Views/Home/HomeView.swift index 409d095..3e95f0b 100644 --- a/Hutch/Views/Home/HomeView.swift +++ b/Hutch/Views/Home/HomeView.swift @@ -74,7 +74,7 @@ struct HomeView: View { .listStyle(.insetGrouped) .listSectionSpacing(.compact) .refreshable { - await viewModel.loadDashboard() + await viewModel.loadDashboard(forceRefresh: true) } .connectivityOverlay(hasContent: hasHomeContent(viewModel)) { await viewModel.loadDashboard() diff --git a/Hutch/Views/Home/HomeViewModel.swift b/Hutch/Views/Home/HomeViewModel.swift index 4436f0c..98dd968 100644 --- a/Hutch/Views/Home/HomeViewModel.swift +++ b/Hutch/Views/Home/HomeViewModel.swift @@ -343,7 +343,12 @@ final class HomeViewModel { self.accountID = accountID } - func loadDashboard() async { + /// Loads the dashboard. + /// + /// `forceRefresh` bypasses the cache. Without it, a pull to refresh returns + /// whatever is already cached and only schedules a background fetch, so new + /// mail cannot show up on the first pull. + func loadDashboard(forceRefresh: Bool = false) async { isLoadingProjects = true isLoadingAssignedTickets = true isLoadingRecentBuilds = true @@ -354,11 +359,11 @@ final class HomeViewModel { isShowingStaleSystemStatus = false systemStatusErrorMessage = nil - async let projectsTask = loadProjects() - async let jobsTask = loadRecentJobs() - async let assignedTicketsTask = loadAssignedTickets() - async let inboxUnreadTask = loadInboxUnreadSnapshot() - async let systemStatusTask = loadSystemStatusSnapshot() + async let projectsTask = loadProjects(forceRefresh: forceRefresh) + async let jobsTask = loadRecentJobs(forceRefresh: forceRefresh) + async let assignedTicketsTask = loadAssignedTickets(forceRefresh: forceRefresh) + async let inboxUnreadTask = loadInboxUnreadSnapshot(forceRefresh: forceRefresh) + async let systemStatusTask = loadSystemStatusSnapshot(forceRefresh: forceRefresh) let projectsResult = await projectsTask switch projectsResult { @@ -676,7 +681,7 @@ final class HomeViewModel { persistNeedsAttentionSnapshot() } - private func loadProjects() async -> Result<[Project], Error> { + private func loadProjects(forceRefresh: Bool) async -> Result<[Project], Error> { do { return .success(try await projectService.fetchProjects()) } catch { @@ -684,7 +689,7 @@ final class HomeViewModel { } } - private func loadRecentJobs() async -> Result<[HomeJobPayload], Error> { + private func loadRecentJobs(forceRefresh: Bool) async -> Result<[HomeJobPayload], Error> { do { let cached = try await client.executeCached( service: .builds, @@ -693,7 +698,7 @@ final class HomeViewModel { cacheKey: APICacheKeys.homeJobs(actor: currentUser.canonicalName), resourceType: .buildList, ttl: APICacheTTLs.homeDashboard, - policy: .cacheFirstThenRefresh + policy: forceRefresh ? .refreshIgnoringCache : .cacheFirstThenRefresh ) return .success(cached.value.jobs.results) } catch { @@ -701,15 +706,15 @@ final class HomeViewModel { } } - private func loadInboxUnreadSnapshot() async -> HomeInboxUnreadSnapshot? { + private func loadInboxUnreadSnapshot(forceRefresh: Bool) async -> HomeInboxUnreadSnapshot? { do { - return try await fetchUnreadInboxSnapshot() + return try await fetchUnreadInboxSnapshot(forceRefresh: forceRefresh) } catch { return nil } } - private func loadSystemStatusSnapshot() async -> Result, Error> { + private func loadSystemStatusSnapshot(forceRefresh: Bool) async -> Result, Error> { do { return .success(try await systemStatusRepository.snapshotResult()) } catch { @@ -717,8 +722,8 @@ final class HomeViewModel { } } - private func fetchUnreadInboxSnapshot() async throws -> HomeInboxUnreadSnapshot { - let mailingLists = try await fetchInboxMailingLists() + private func fetchUnreadInboxSnapshot(forceRefresh: Bool) async throws -> HomeInboxUnreadSnapshot { + let mailingLists = try await fetchInboxMailingLists(forceRefresh: forceRefresh) guard !mailingLists.isEmpty else { return HomeInboxUnreadSnapshot(unreadCount: 0, threads: []) } var startIndex = mailingLists.startIndex @@ -737,7 +742,7 @@ final class HomeViewModel { for mailingList in batch { group.addTask { do { - return .success(try await self.fetchUnreadThreadSnapshot(for: mailingList)) + return .success(try await self.fetchUnreadThreadSnapshot(for: mailingList, forceRefresh: forceRefresh)) } catch { return .failure(error) } @@ -776,7 +781,7 @@ final class HomeViewModel { ) } - private func fetchInboxMailingLists() async throws -> [InboxMailingListReference] { + private func fetchInboxMailingLists(forceRefresh: Bool) async throws -> [InboxMailingListReference] { var subscriptions: [HomeInboxSubscription] = [] var cursor: String? @@ -794,7 +799,7 @@ final class HomeViewModel { cacheKey: APICacheKeys.inboxSubscriptions(cursor: cursor), resourceType: .ticketList, ttl: APICacheTTLs.inboxSummary, - policy: .cacheFirstThenRefresh + policy: forceRefresh ? .refreshIgnoringCache : .cacheFirstThenRefresh ) let response = cached.value @@ -809,7 +814,7 @@ final class HomeViewModel { return subscriptions.compactMap(\.list).filter { seen.insert($0.rid).inserted } } - private func fetchUnreadThreadSnapshot(for mailingList: InboxMailingListReference) async throws -> HomeInboxUnreadSnapshot { + private func fetchUnreadThreadSnapshot(for mailingList: InboxMailingListReference, forceRefresh: Bool) async throws -> HomeInboxUnreadSnapshot { var unreadCount = 0 var cursor: String? var unreadThreads: [InboxThreadSummary] = [] @@ -828,7 +833,7 @@ final class HomeViewModel { cacheKey: APICacheKeys.inboxThreads(listRid: mailingList.rid, cursor: cursor), resourceType: .ticketList, ttl: APICacheTTLs.inboxSummary, - policy: .cacheFirstThenRefresh + policy: forceRefresh ? .refreshIgnoringCache : .cacheFirstThenRefresh ) let response = cached.value @@ -871,10 +876,10 @@ final class HomeViewModel { ) } - private func loadAssignedTickets() async -> Result<[HomeAssignedTicket], Error> { + private func loadAssignedTickets(forceRefresh: Bool) async -> Result<[HomeAssignedTicket], Error> { do { - let trackers = try await fetchAllTrackers() - let tickets = try await fetchAssignedTickets(for: trackers) + let trackers = try await fetchAllTrackers(forceRefresh: forceRefresh) + let tickets = try await fetchAssignedTickets(for: trackers, forceRefresh: forceRefresh) .sorted(by: Self.sortAssignedTicketsForTriage) return .success(tickets) } catch { @@ -882,7 +887,7 @@ final class HomeViewModel { } } - private func fetchAllTrackers() async throws -> [TrackerSummary] { + private func fetchAllTrackers(forceRefresh: Bool) async throws -> [TrackerSummary] { var allTrackers: [TrackerSummary] = [] var cursor: String? @@ -900,7 +905,7 @@ final class HomeViewModel { cacheKey: APICacheKeys.trackers(cursor: cursor), resourceType: .ticketList, ttl: APICacheTTLs.ticketList, - policy: .cacheFirstThenRefresh + policy: forceRefresh ? .refreshIgnoringCache : .cacheFirstThenRefresh ) let response = cached.value @@ -914,7 +919,7 @@ final class HomeViewModel { return allTrackers } - private func fetchAssignedTickets(for trackers: [TrackerSummary]) async throws -> [HomeAssignedTicket] { + private func fetchAssignedTickets(for trackers: [TrackerSummary], forceRefresh: Bool) async throws -> [HomeAssignedTicket] { guard !trackers.isEmpty else { return [] } var assignedTickets: [HomeAssignedTicket] = [] @@ -927,7 +932,7 @@ final class HomeViewModel { let batchTickets = try await withThrowingTaskGroup(of: [HomeAssignedTicket].self) { group in for tracker in batch { group.addTask { - try await self.fetchAssignedTickets(for: tracker) + try await self.fetchAssignedTickets(for: tracker, forceRefresh: forceRefresh) } } @@ -945,7 +950,7 @@ final class HomeViewModel { return assignedTickets } - private func fetchAssignedTickets(for tracker: TrackerSummary) async throws -> [HomeAssignedTicket] { + private func fetchAssignedTickets(for tracker: TrackerSummary, forceRefresh: Bool) async throws -> [HomeAssignedTicket] { let cached = try await client.executeCached( service: .todo, query: Self.trackerTicketsQuery, @@ -964,7 +969,7 @@ final class HomeViewModel { ), resourceType: .ticketList, ttl: APICacheTTLs.ticketList, - policy: .cacheFirstThenRefresh + policy: forceRefresh ? .refreshIgnoringCache : .cacheFirstThenRefresh ) let response = cached.value diff --git a/Hutch/Views/Work/WorkView.swift b/Hutch/Views/Work/WorkView.swift index 6ba1d0c..23acdfb 100644 --- a/Hutch/Views/Work/WorkView.swift +++ b/Hutch/Views/Work/WorkView.swift @@ -72,7 +72,7 @@ struct WorkView: View { .themedList() .listStyle(.insetGrouped) .refreshable { - await viewModel.loadDashboard() + await viewModel.loadDashboard(forceRefresh: true) } .connectivityOverlay(hasContent: hasWorkContent(viewModel)) { await viewModel.loadDashboard() -- cgit v1.2.3 From 97e8b49fcb56751fa0e80a332484db8252753417 Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 23:18:20 -0500 Subject: fix: derive inbox activity from mail, not thread.updated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thread.updated is the root email's insert time. It never advances when a reply arrives, despite the name and despite the schema describing threads as ordered "most recently bumped". sr.ht returns updated seven seconds after root.date on a thread carrying four replies: subject: "Re: test post" updated: 2026-07-16T03:44:55Z replies: 4 root: { date: 2026-07-15T22:44:48-05:00 } The app fed that field into lastActivityAt everywhere, so a reply to an old thread was invisible: it did not mark the thread unread, did not raise it in the list, and did not update the timestamp beside it. This was inert until the unread baseline landed, because an absent view record returned unread unconditionally and the date was never compared to anything. The baseline did not break unread; it was the first code to depend on that field meaning what it says. Use MailingList.emails instead, which is real reverse-chronological arrival data, keyed by thread root id. The scan stops once it is older than the cutoff, so a quiet list costs one page and a busy one costs only what is new. Threads with nothing in the window keep the root timestamp and stay read, which they already were. Failure returns empty activity rather than throwing: unread is a decoration and should not take the thread list down with it. Uses Email.received rather than Email.date — received is server-side and non-null, date comes from the sender's header and is neither. --- Hutch/Networking/MailingListActivity.swift | 135 ++++++++++++++++++++++ Hutch/Views/Home/HomeViewModel.swift | 13 ++- Hutch/Views/Projects/ProjectMailingListView.swift | 21 +++- HutchTests/MailingListActivityTests.swift | 77 ++++++++++++ 4 files changed, 240 insertions(+), 6 deletions(-) create mode 100644 Hutch/Networking/MailingListActivity.swift create mode 100644 HutchTests/MailingListActivityTests.swift diff --git a/Hutch/Networking/MailingListActivity.swift b/Hutch/Networking/MailingListActivity.swift new file mode 100644 index 0000000..c52a4d8 --- /dev/null +++ b/Hutch/Networking/MailingListActivity.swift @@ -0,0 +1,135 @@ +import Foundation + +// MARK: - Response types (file-private to avoid @MainActor Decodable issues) + +private struct ListEmailsResponse: Decodable, Sendable { + let list: ListEmailsPayload? +} + +private struct ListEmailsPayload: Decodable, Sendable { + let emails: ListEmailPage +} + +private struct ListEmailPage: Decodable, Sendable { + let results: [ListEmailPayload] + let cursor: String? +} + +private struct ListEmailPayload: Decodable, Sendable { + /// When sr.ht received the mail. Unlike `date`, which comes from the sender's + /// Date: header and is both nullable and not to be trusted, this is + /// server-authoritative. + let received: Date + let thread: ListEmailThread +} + +private struct ListEmailThread: Decodable, Sendable { + let root: ListEmailThreadRoot +} + +private struct ListEmailThreadRoot: Decodable, Sendable { + let id: Int +} + +// MARK: - Activity + +/// When each thread on a mailing list last received mail. +/// +/// `Thread.updated` cannot answer this. Despite its name, and despite the schema +/// describing threads as ordered "most recently bumped", it is the root email's +/// insert time and never advances when a reply arrives — sr.ht reports `updated` +/// seven seconds after `root.date` on a thread carrying four replies. Anything +/// built on it silently treats thread creation as activity. +/// +/// `MailingList.emails` is reverse-chronological arrival data, so it can. +struct MailingListActivity: Sendable { + private let newestByRootEmailID: [Int: Date] + + init(newestByRootEmailID: [Int: Date] = [:]) { + self.newestByRootEmailID = newestByRootEmailID + } + + /// The newest arrival in the thread rooted at `rootEmailID`. + /// + /// Falls back to `fallback` for threads with nothing inside the scanned + /// window, which are by definition older than the cutoff and therefore read. + func lastActivity(rootEmailID: Int, fallback: Date) -> Date { + guard let newest = newestByRootEmailID[rootEmailID] else { return fallback } + return max(newest, fallback) + } +} + +enum MailingListActivityLoader { + + private static let listEmailsQuery = """ + query listActivity($rid: ID!, $cursor: Cursor) { + list(rid: $rid) { + emails(cursor: $cursor) { + results { + received + thread { root { id } } + } + cursor + } + } + } + """ + + /// Scans the list's mail newest-first and stops once it is older than + /// `cutoff`, so a quiet list costs a single page and a busy one costs only + /// what has arrived since. + /// + /// `maxPages` bounds the scan. An account carrying pre-existing read state has + /// a `distantPast` cutoff, which would otherwise walk the entire archive; + /// threads beyond the window keep their fallback date and stay read, which is + /// what they already were. + /// + /// Returns empty activity on failure rather than throwing: unread is a + /// decoration, and losing it should not fail the thread list around it. + static func load( + client: SRHTClient, + listRID: String, + since cutoff: Date, + maxPages: Int = 3 + ) async -> MailingListActivity { + var newest: [Int: Date] = [:] + var cursor: String? + var pagesFetched = 0 + + while pagesFetched < maxPages { + var variables: [String: any Sendable] = ["rid": listRID] + if let cursor { + variables["cursor"] = cursor + } + + let response: ListEmailsResponse + do { + response = try await client.execute( + service: .lists, + query: listEmailsQuery, + variables: variables, + responseType: ListEmailsResponse.self + ) + } catch { + return MailingListActivity(newestByRootEmailID: newest) + } + + guard let page = response.list?.emails else { break } + pagesFetched += 1 + + for email in page.results { + let rootID = email.thread.root.id + if let existing = newest[rootID], existing >= email.received { continue } + newest[rootID] = email.received + } + + // Reverse chronological, so once a page ends older than the cutoff + // nothing further back can matter. + if let oldest = page.results.map(\.received).min(), oldest <= cutoff { break } + guard let next = page.cursor, !next.isEmpty else { break } + cursor = next + } + + return MailingListActivity(newestByRootEmailID: newest) + } +} diff --git a/Hutch/Views/Home/HomeViewModel.swift b/Hutch/Views/Home/HomeViewModel.swift index 98dd968..a3aed30 100644 --- a/Hutch/Views/Home/HomeViewModel.swift +++ b/Hutch/Views/Home/HomeViewModel.swift @@ -819,6 +819,14 @@ final class HomeViewModel { var cursor: String? var unreadThreads: [InboxThreadSummary] = [] + // thread.updated is the root email's insert time and never advances when a + // reply lands, so activity has to come from the list's mail feed. + let activity = await MailingListActivityLoader.load( + client: client, + listRID: mailingList.rid, + since: InboxReadStateStore.baseline(defaults: defaults) ?? .distantPast + ) + while true { var variables: [String: any Sendable] = ["rid": mailingList.rid] if let cursor { @@ -838,6 +846,7 @@ final class HomeViewModel { let response = cached.value let unreadThreadSummaries = response.list.threads.results.compactMap { thread -> InboxThreadSummary? in + let lastActivityAt = activity.lastActivity(rootEmailID: thread.root.id, fallback: thread.updated) let summary = InboxThreadSummary( rootEmailID: thread.root.id, rootMessageID: thread.root.messageID, @@ -849,13 +858,13 @@ final class HomeViewModel { listOwner: mailingList.owner, subject: thread.subject, latestSender: thread.sender, - lastActivityAt: thread.updated, + lastActivityAt: lastActivityAt, messageCount: thread.replies + 1, repo: InboxThreadUtilities.deriveRepositoryName(from: mailingList.name), containsPatch: thread.root.patch != nil || thread.subject.localizedCaseInsensitiveContains("[patch"), isUnread: InboxReadStateStore.isUnread( threadID: "\(mailingList.rid)#\(InboxThreadSummary.normalizationKey(for: thread.subject))", - lastActivityAt: thread.updated, + lastActivityAt: lastActivityAt, defaults: defaults ) ) diff --git a/Hutch/Views/Projects/ProjectMailingListView.swift b/Hutch/Views/Projects/ProjectMailingListView.swift index d466121..e4c9df4 100644 --- a/Hutch/Views/Projects/ProjectMailingListView.swift +++ b/Hutch/Views/Projects/ProjectMailingListView.swift @@ -84,8 +84,14 @@ final class MailingListDetailViewModel { responseType: ProjectMailingListThreadsResponse.self ) + let activity = await MailingListActivityLoader.load( + client: client, + listRID: mailingList.rid, + since: InboxReadStateStore.baseline(defaults: defaults) ?? .distantPast + ) + threads = deduplicateThreads( - response.list.threads.results.map(makeSummary(from:)) + response.list.threads.results.map { makeSummary(from: $0, activity: activity) } ) } catch { self.error = "Failed to load mailing list" @@ -138,7 +144,10 @@ final class MailingListDetailViewModel { NeedsAttentionSnapshotStore.adjustUnreadInboxThreads(by: -unreadThreads.count, accountID: accountID) } - private func makeSummary(from thread: ProjectMailingListThreadPayload) -> InboxThreadSummary { + private func makeSummary( + from thread: ProjectMailingListThreadPayload, + activity: MailingListActivity + ) -> InboxThreadSummary { let normalizedSubject = thread.subject .replacingOccurrences(of: #"\s+"#, with: " ", options: .regularExpression) .trimmingCharacters(in: .whitespacesAndNewlines) @@ -146,6 +155,10 @@ final class MailingListDetailViewModel { .lowercased() let threadID = "\(mailingList.rid)#\(normalizedSubject)" + // thread.updated is the root email's insert time and never advances when a + // reply lands, so activity has to come from the list's mail feed. + let lastActivityAt = activity.lastActivity(rootEmailID: thread.root.id, fallback: thread.updated) + return InboxThreadSummary( rootEmailID: thread.root.id, rootMessageID: thread.root.messageID, @@ -157,11 +170,11 @@ final class MailingListDetailViewModel { listOwner: mailingList.owner, subject: thread.subject, latestSender: thread.sender, - lastActivityAt: thread.updated, + lastActivityAt: lastActivityAt, messageCount: thread.replies + 1, repo: nil, containsPatch: thread.root.patch != nil || thread.subject.localizedCaseInsensitiveContains("[patch"), - isUnread: InboxReadStateStore.isUnread(threadID: threadID, lastActivityAt: thread.updated, defaults: defaults) + isUnread: InboxReadStateStore.isUnread(threadID: threadID, lastActivityAt: lastActivityAt, defaults: defaults) ) } diff --git a/HutchTests/MailingListActivityTests.swift b/HutchTests/MailingListActivityTests.swift new file mode 100644 index 0000000..46208cb --- /dev/null +++ b/HutchTests/MailingListActivityTests.swift @@ -0,0 +1,77 @@ +import Foundation +import Testing +@testable import Hutch + +struct MailingListActivityTests { + + @Test + func usesTheNewestArrivalOverTheRootTimestamp() { + // The case that started this: sr.ht reports thread.updated seven seconds + // after the root email on a thread carrying four replies, so the fallback + // must lose to real arrival data. + let rootInsert = Date(timeIntervalSince1970: 1_000) + let newestReply = Date(timeIntervalSince1970: 5_000) + let activity = MailingListActivity(newestByRootEmailID: [42: newestReply]) + + #expect(activity.lastActivity(rootEmailID: 42, fallback: rootInsert) == newestReply) + } + + @Test + func fallsBackForThreadsOutsideTheScannedWindow() { + // Threads with nothing new are absent from the feed scan; they keep the + // root timestamp, which is older than any cutoff and so reads as read. + let rootInsert = Date(timeIntervalSince1970: 1_000) + let activity = MailingListActivity(newestByRootEmailID: [:]) + + #expect(activity.lastActivity(rootEmailID: 42, fallback: rootInsert) == rootInsert) + } + + @Test + func neverGoesBackwardsFromTheFallback() { + // A root inserted after the newest scanned reply must not age the thread + // backwards. + let rootInsert = Date(timeIntervalSince1970: 9_000) + let staleReply = Date(timeIntervalSince1970: 5_000) + let activity = MailingListActivity(newestByRootEmailID: [42: staleReply]) + + #expect(activity.lastActivity(rootEmailID: 42, fallback: rootInsert) == rootInsert) + } + + @Test + func tracksThreadsIndependently() { + let activity = MailingListActivity(newestByRootEmailID: [ + 1: Date(timeIntervalSince1970: 5_000), + 2: Date(timeIntervalSince1970: 7_000) + ]) + let fallback = Date(timeIntervalSince1970: 1_000) + + #expect(activity.lastActivity(rootEmailID: 1, fallback: fallback) == Date(timeIntervalSince1970: 5_000)) + #expect(activity.lastActivity(rootEmailID: 2, fallback: fallback) == Date(timeIntervalSince1970: 7_000)) + #expect(activity.lastActivity(rootEmailID: 3, fallback: fallback) == fallback) + } + + @Test + func newMailInAnOldThreadReadsAsUnread() { + let suiteName = "MailingListActivityTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + let signIn = Date(timeIntervalSince1970: 5_000) + InboxReadStateStore.establishBaselineIfNeeded(now: signIn, defaults: defaults) + + // A thread rooted long before sign-in, with a reply after it. Keyed on + // thread.updated this reads as read, which was the bug. + let rootInsert = Date(timeIntervalSince1970: 1_000) + let replyAfterSignIn = Date(timeIntervalSince1970: 6_000) + let activity = MailingListActivity(newestByRootEmailID: [42: replyAfterSignIn]) + let lastActivityAt = activity.lastActivity(rootEmailID: 42, fallback: rootInsert) + + #expect( + InboxReadStateStore.isUnread( + threadID: "list#old thread", + lastActivityAt: lastActivityAt, + defaults: defaults + ) + ) + } +} -- cgit v1.2.3