fix(offline): manual mode never skipped live reads, storage/banner cleanup
The main fix: CachingOutlineAPIClient.cachedFetch (documentInfo, listCollections, listDocuments, etc.) never actually checked manual Offline Mode — only the write path did. With a real connection still up, turning Offline Mode on did nothing for reads: the sidebar kept fetching live, showing collections/documents beyond what was actually cached. Reads now skip `live` entirely under manual offline mode, same as writes already did — the sidebar and document lists are now genuinely limited to whatever's cached, and only those documents are openable, once the toggle is on. 1 new regression test (54/54). Settings: removed the cache-size readout from Offline & Sync (storage management is Advanced-only now, per the "one place that can touch the cache" design) and added it next to Advanced's Clear All Cache instead, where it was missing. Sidebar: replaced the old passive "Offline — showing cached content" banner for a real dropped connection with an actionable prompt (OfflineConnectionPromptBanner, styled like the existing RemoteChangesBanner) offering to turn Offline Mode on — doesn't change any behavior on its own, same as RemoteChangesBanner never auto- refreshes. The informational banner still shows once Offline Mode is actually on (manually, or via this prompt).
This commit is contained in:
@@ -409,6 +409,17 @@ public actor CachingOutlineAPIClient: OutlineAPIClient {
|
|||||||
// MARK: - Helpers
|
// MARK: - Helpers
|
||||||
|
|
||||||
private func cachedFetch<T: Codable>(key: String, fetch: () async throws -> T) async throws -> T {
|
private func cachedFetch<T: Codable>(key: String, fetch: () async throws -> T) async throws -> T {
|
||||||
|
// Manual offline mode means "skip the network entirely," not just
|
||||||
|
// "prefer it" — without this check, a read would still hit `live`
|
||||||
|
// (and succeed, showing content beyond whatever's cached) any time
|
||||||
|
// the device actually had a connection, defeating the point of
|
||||||
|
// deliberately testing/working as if offline.
|
||||||
|
if isManualOfflineModeEnabled {
|
||||||
|
if let data = await cache.load(forKey: key), let cached = try? decoder.decode(T.self, from: data) {
|
||||||
|
return cached
|
||||||
|
}
|
||||||
|
throw OutlineAPIError.transport(URLError(.notConnectedToInternet))
|
||||||
|
}
|
||||||
do {
|
do {
|
||||||
let result = try await fetch()
|
let result = try await fetch()
|
||||||
if let data = try? encoder.encode(result) {
|
if let data = try? encoder.encode(result) {
|
||||||
|
|||||||
@@ -326,6 +326,39 @@ final class CachingOutlineAPIClientTests: XCTestCase {
|
|||||||
XCTAssertTrue(pin.id.hasPrefix("pending-"))
|
XCTAssertTrue(pin.id.hasPrefix("pending-"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression test: manual offline mode used to only gate writes —
|
||||||
|
/// reads (listCollections, documentInfo, etc.) still hit `live` first
|
||||||
|
/// any time the device actually had a connection, silently defeating
|
||||||
|
/// "skip the network entirely" for the one thing that mattered most:
|
||||||
|
/// the sidebar showing more than what was actually cached.
|
||||||
|
func testManualOfflineModeSkipsLiveForReadsToo() async throws {
|
||||||
|
let defaults = UserDefaults(suiteName: "CachingOutlineAPIClientTests.manualOfflineReads")!
|
||||||
|
defaults.removePersistentDomain(forName: "CachingOutlineAPIClientTests.manualOfflineReads")
|
||||||
|
|
||||||
|
let stub = StubOutlineAPIClient()
|
||||||
|
let cachedCollection = makeCollection(id: "col-1")
|
||||||
|
var liveCallCount = 0
|
||||||
|
stub.listCollectionsHandler = { _, _ in
|
||||||
|
liveCallCount += 1
|
||||||
|
return [cachedCollection]
|
||||||
|
}
|
||||||
|
let cache = try makeCache()
|
||||||
|
let sut = CachingOutlineAPIClient(live: stub, cache: cache, defaults: defaults)
|
||||||
|
|
||||||
|
// Online first — populates the cache normally.
|
||||||
|
let firstResult = try await sut.listCollections(offset: 0, limit: 25)
|
||||||
|
XCTAssertEqual(firstResult, [cachedCollection])
|
||||||
|
XCTAssertEqual(liveCallCount, 1)
|
||||||
|
|
||||||
|
// Flip manual offline mode on, still "connected" (stub would happily
|
||||||
|
// answer) — the live call must not be attempted at all.
|
||||||
|
defaults.set(true, forKey: CachingOutlineAPIClient.offlineModeDefaultsKey)
|
||||||
|
let secondResult = try await sut.listCollections(offset: 0, limit: 25)
|
||||||
|
|
||||||
|
XCTAssertEqual(secondResult, [cachedCollection])
|
||||||
|
XCTAssertEqual(liveCallCount, 1, "listCollections should have served entirely from cache")
|
||||||
|
}
|
||||||
|
|
||||||
func testCacheStorageSummaryReflectsCachedItems() async throws {
|
func testCacheStorageSummaryReflectsCachedItems() async throws {
|
||||||
let stub = StubOutlineAPIClient()
|
let stub = StubOutlineAPIClient()
|
||||||
stub.documentInfoHandler = { _ in self.makeDocument() }
|
stub.documentInfoHandler = { _ in self.makeDocument() }
|
||||||
|
|||||||
@@ -159,12 +159,6 @@ struct SettingsView: View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
Divider()
|
|
||||||
.frame(maxWidth: 480)
|
|
||||||
|
|
||||||
storageRow
|
|
||||||
.frame(maxWidth: 480, alignment: .leading)
|
|
||||||
|
|
||||||
Divider()
|
Divider()
|
||||||
.frame(maxWidth: 480)
|
.frame(maxWidth: 480)
|
||||||
|
|
||||||
@@ -173,25 +167,6 @@ struct SettingsView: View {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private var storageRow: some View {
|
|
||||||
VStack(alignment: .leading, spacing: 6) {
|
|
||||||
Text("Cache Storage")
|
|
||||||
.font(.subheadline.weight(.medium))
|
|
||||||
if let storageSummary {
|
|
||||||
Text("\(storageSummary.itemCount) items — \(formattedBytes(storageSummary.totalBytes))")
|
|
||||||
.font(.caption)
|
|
||||||
.foregroundStyle(.secondary)
|
|
||||||
} else {
|
|
||||||
Text("—")
|
|
||||||
.font(.caption)
|
|
||||||
.foregroundStyle(.secondary)
|
|
||||||
}
|
|
||||||
Text("Clearing the cache is in Advanced Options.")
|
|
||||||
.font(.caption2)
|
|
||||||
.foregroundStyle(.secondary)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
private var pendingOperationsRow: some View {
|
private var pendingOperationsRow: some View {
|
||||||
VStack(alignment: .leading, spacing: 8) {
|
VStack(alignment: .leading, spacing: 8) {
|
||||||
HStack {
|
HStack {
|
||||||
@@ -309,6 +284,11 @@ struct SettingsView: View {
|
|||||||
.buttonStyle(.borderedProminent)
|
.buttonStyle(.borderedProminent)
|
||||||
.tint(.red)
|
.tint(.red)
|
||||||
}
|
}
|
||||||
|
if let storageSummary {
|
||||||
|
Text("\(storageSummary.itemCount) items — \(formattedBytes(storageSummary.totalBytes))")
|
||||||
|
.font(.caption)
|
||||||
|
.foregroundStyle(.secondary)
|
||||||
|
}
|
||||||
Text("Deletes every cached collection and document, including anything Full Local Sync built, and anything still waiting to sync. Doesn't touch the server. This is the only place that can — it works even while offline, which is exactly why it's behind this toggle.")
|
Text("Deletes every cached collection and document, including anything Full Local Sync built, and anything still waiting to sync. Doesn't touch the server. This is the only place that can — it works even while offline, which is exactly why it's behind this toggle.")
|
||||||
.font(.caption2)
|
.font(.caption2)
|
||||||
.foregroundStyle(.secondary)
|
.foregroundStyle(.secondary)
|
||||||
|
|||||||
@@ -51,8 +51,11 @@ struct ContentView_macOS: View {
|
|||||||
VStack(spacing: 0) {
|
VStack(spacing: 0) {
|
||||||
SidebarSearchField(text: $globalSearchQuery)
|
SidebarSearchField(text: $globalSearchQuery)
|
||||||
Divider()
|
Divider()
|
||||||
if isOfflineModeEnabled || !session.networkMonitor.isOnline {
|
if isOfflineModeEnabled {
|
||||||
OfflineBanner(isManual: isOfflineModeEnabled)
|
OfflineBanner(isManual: true)
|
||||||
|
Divider()
|
||||||
|
} else if !session.networkMonitor.isOnline {
|
||||||
|
OfflineConnectionPromptBanner(onEnableOfflineMode: { isOfflineModeEnabled = true })
|
||||||
Divider()
|
Divider()
|
||||||
}
|
}
|
||||||
sidebar
|
sidebar
|
||||||
|
|||||||
@@ -0,0 +1,30 @@
|
|||||||
|
#if os(macOS)
|
||||||
|
import SwiftUI
|
||||||
|
|
||||||
|
/// Shown in the sidebar when the network is actually down and the user
|
||||||
|
/// hasn't turned on Offline Mode themselves yet. Deliberately doesn't change
|
||||||
|
/// any fetch behavior on its own — reads still try live and fall back to
|
||||||
|
/// cache the normal way (see `CachingOutlineAPIClient`) until the user
|
||||||
|
/// actually taps the button here, same as `RemoteChangesBanner` never
|
||||||
|
/// auto-refreshes on their behalf either.
|
||||||
|
struct OfflineConnectionPromptBanner: View {
|
||||||
|
let onEnableOfflineMode: () -> Void
|
||||||
|
|
||||||
|
var body: some View {
|
||||||
|
HStack(spacing: 8) {
|
||||||
|
Image(systemName: "wifi.slash")
|
||||||
|
.foregroundStyle(.orange)
|
||||||
|
Text("No connection")
|
||||||
|
.font(.callout)
|
||||||
|
Spacer(minLength: 8)
|
||||||
|
Button("Turn On Offline Mode", action: onEnableOfflineMode)
|
||||||
|
.buttonStyle(.borderedProminent)
|
||||||
|
.tint(.orange)
|
||||||
|
.controlSize(.small)
|
||||||
|
}
|
||||||
|
.padding(.horizontal, 12)
|
||||||
|
.padding(.vertical, 8)
|
||||||
|
.background(Color.orange.opacity(0.12))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
#endif
|
||||||
Reference in New Issue
Block a user