From e5f571a63e07b8762652eee2de0f9c4a82d4ab60 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Sat, 13 Jun 2026 15:32:26 -0400 Subject: [PATCH] fix: show a generic "App Blocked" shield instead of mis-attributing a rule The shield titled itself with the first enabled rule whose selection contained the app, regardless of whether that rule was the one actually blocking. When several rules covered the same app (e.g. a dormant schedule + the active limit) it named the wrong rule. Drop the rule-name lookup entirely: every shield now carries a generic "App Blocked" title. Open-limit shields keep their functional detail (running count + "Open (N left)" button) beneath that title. The text-only decision moves into a pure, unit-tested ShieldPresentation in Shared/, and the now-dead ShieldLookup.snapshot(containingApplication:) is removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ShieldConfigurationExtension.swift | 56 +++++++------------ .../ShieldPresentationTests.swift | 53 ++++++++++++++++++ Shared/ShieldLookup.swift | 12 ---- Shared/ShieldPresentation.swift | 47 ++++++++++++++++ docs/RULES_FEATURE_SPEC.md | 11 +++- 5 files changed, 127 insertions(+), 52 deletions(-) create mode 100644 OpenAppLockTests/ShieldPresentationTests.swift create mode 100644 Shared/ShieldPresentation.swift diff --git a/OpenAppLockShieldConfig/ShieldConfigurationExtension.swift b/OpenAppLockShieldConfig/ShieldConfigurationExtension.swift index d69b7ec..2e75860 100644 --- a/OpenAppLockShieldConfig/ShieldConfigurationExtension.swift +++ b/OpenAppLockShieldConfig/ShieldConfigurationExtension.swift @@ -9,28 +9,29 @@ import UIKit /// Customizes the system shield. Apps under an open-limit rule show how many /// opens were used and offer an "Open" secondary button while opens remain; -/// everything else gets a plain blocked shield named after its rule. +/// everything else gets a plain blocked shield. Shields never name a rule — the +/// text is decided by the pure `ShieldPresentation`. final class ShieldConfigurationExtension: ShieldConfigurationDataSource { override func configuration(shielding application: Application) -> ShieldConfiguration { - guard let token = application.token else { return blockedConfiguration(ruleName: nil) } + guard let token = application.token else { return configuration(for: .blocked) } return configuration(forApplicationToken: token) } override func configuration( shielding application: Application, in category: ActivityCategory ) -> ShieldConfiguration { - guard let token = application.token else { return blockedConfiguration(ruleName: nil) } + guard let token = application.token else { return configuration(for: .blocked) } return configuration(forApplicationToken: token) } override func configuration(shielding webDomain: WebDomain) -> ShieldConfiguration { - blockedConfiguration(ruleName: nil) + configuration(for: .blocked) } override func configuration( shielding webDomain: WebDomain, in category: ActivityCategory ) -> ShieldConfiguration { - blockedConfiguration(ruleName: nil) + configuration(for: .blocked) } private func configuration( @@ -41,45 +42,26 @@ final class ShieldConfigurationExtension: ShieldConfigurationDataSource { let snapshot = ShieldLookup.openLimitSnapshot( containingApplication: token, in: snapshots) else { - let name = ShieldLookup.snapshot(containingApplication: token, in: snapshots)?.name - return blockedConfiguration(ruleName: name) + return configuration(for: .blocked) } - let usage = UsageLedger().usage(for: snapshot.id, onDayContaining: .now) - let remaining = max(0, snapshot.maxOpens - usage.opensUsed) - guard remaining > 0 else { - return ShieldConfiguration( - backgroundBlurStyle: .systemMaterial, - title: .init(text: snapshot.name, color: .label), - subtitle: .init( - text: "No opens left today — the block lifts tomorrow.", - color: .secondaryLabel), - primaryButtonLabel: .init(text: "OK", color: .white), - primaryButtonBackgroundColor: .systemBlue - ) - } - return ShieldConfiguration( - backgroundBlurStyle: .systemMaterial, - title: .init(text: snapshot.name, color: .label), - subtitle: .init( - text: "Opened \(usage.opensUsed) of \(snapshot.maxOpens) times today. " - + "Each open lasts \(MonitoringPlan.openSessionMinutes) minutes.", - color: .secondaryLabel), - primaryButtonLabel: .init(text: "OK", color: .white), - primaryButtonBackgroundColor: .systemBlue, - secondaryButtonLabel: .init( - text: remaining == 1 ? "Open (1 left)" : "Open (\(remaining) left)", - color: .systemBlue) - ) + return configuration( + for: .openLimit( + opensUsed: usage.opensUsed, + maxOpens: snapshot.maxOpens, + sessionMinutes: MonitoringPlan.openSessionMinutes)) } - private func blockedConfiguration(ruleName: String?) -> ShieldConfiguration { + private func configuration(for presentation: ShieldPresentation) -> ShieldConfiguration { ShieldConfiguration( backgroundBlurStyle: .systemMaterial, - title: .init(text: ruleName ?? "Blocked", color: .label), - subtitle: .init(text: "This app is blocked by OpenAppLock.", color: .secondaryLabel), + title: .init(text: presentation.title, color: .label), + subtitle: .init(text: presentation.subtitle, color: .secondaryLabel), primaryButtonLabel: .init(text: "OK", color: .white), - primaryButtonBackgroundColor: .systemBlue + primaryButtonBackgroundColor: .systemBlue, + secondaryButtonLabel: presentation.secondaryButton.map { + .init(text: $0, color: .systemBlue) + } ) } } diff --git a/OpenAppLockTests/ShieldPresentationTests.swift b/OpenAppLockTests/ShieldPresentationTests.swift new file mode 100644 index 0000000..6b1e469 --- /dev/null +++ b/OpenAppLockTests/ShieldPresentationTests.swift @@ -0,0 +1,53 @@ +// +// ShieldPresentationTests.swift +// OpenAppLockTests +// + +import Testing + +@testable import OpenAppLock + +@Suite("Shield presentation") +struct ShieldPresentationTests { + @Test("Fully-blocked shield is generic and names no rule") + func blockedIsGeneric() { + let presentation = ShieldPresentation.blocked + #expect(presentation.title == "App Blocked") + #expect(presentation.subtitle == "This app is blocked by OpenAppLock.") + #expect(presentation.secondaryButton == nil) + } + + @Test("Open-limit shield with opens remaining shows counts and an Open button") + func openLimitWithOpensRemaining() { + let presentation = ShieldPresentation.openLimit( + opensUsed: 2, maxOpens: 5, sessionMinutes: 15) + // Title is generic — never the rule name. + #expect(presentation.title == "App Blocked") + #expect(presentation.subtitle.contains("Opened 2 of 5 times today")) + #expect(presentation.subtitle.contains("15 minutes")) + #expect(presentation.secondaryButton == "Open (3 left)") + } + + @Test("A single remaining open reads in the singular") + func openLimitSingularRemaining() { + let presentation = ShieldPresentation.openLimit( + opensUsed: 4, maxOpens: 5, sessionMinutes: 15) + #expect(presentation.secondaryButton == "Open (1 left)") + } + + @Test("Spent open-limit shield drops the Open button and reads as blocked") + func openLimitExhausted() { + let presentation = ShieldPresentation.openLimit( + opensUsed: 5, maxOpens: 5, sessionMinutes: 15) + #expect(presentation.title == "App Blocked") + #expect(presentation.subtitle == "No opens left today — the block lifts tomorrow.") + #expect(presentation.secondaryButton == nil) + } + + @Test("Over-spending never yields a negative or non-nil Open button") + func openLimitOverSpent() { + let presentation = ShieldPresentation.openLimit( + opensUsed: 9, maxOpens: 5, sessionMinutes: 15) + #expect(presentation.secondaryButton == nil) + } +} diff --git a/Shared/ShieldLookup.swift b/Shared/ShieldLookup.swift index 7e9f675..81bfc0e 100644 --- a/Shared/ShieldLookup.swift +++ b/Shared/ShieldLookup.swift @@ -29,16 +29,4 @@ enum ShieldLookup { .categoryTokens.contains(token) } } - - /// Any enabled rule whose selection includes the application (for naming - /// plain blocked shields). - static func snapshot( - containingApplication token: ApplicationToken, in snapshots: [RuleSnapshot] - ) -> RuleSnapshot? { - snapshots.first { snapshot in - guard snapshot.isEnabled else { return false } - return AppSelectionCodec.decode(snapshot.selectionData) - .applicationTokens.contains(token) - } - } } diff --git a/Shared/ShieldPresentation.swift b/Shared/ShieldPresentation.swift new file mode 100644 index 0000000..87b94f1 --- /dev/null +++ b/Shared/ShieldPresentation.swift @@ -0,0 +1,47 @@ +// +// ShieldPresentation.swift +// OpenAppLock +// + +/// The text a shield shows, decided independently of any UI type so it can be +/// unit tested. Shields never name the rule that blocks an app: when several +/// rules cover the same app the responsible rule can't be told apart, so every +/// shield carries the same generic "App Blocked" title. Open-limit shields add +/// their functional detail (remaining opens + an "Open" button) beneath it. +struct ShieldPresentation: Equatable { + let title: String + let subtitle: String + /// Label for the secondary "Open" button, or `nil` when the shield offers + /// no way through (a full block, or a spent open-limit budget). + let secondaryButton: String? + + static let blockedTitle = "App Blocked" + + /// A plain, fully-blocked app: no counts, no way through. + static let blocked = ShieldPresentation( + title: blockedTitle, + subtitle: "This app is blocked by OpenAppLock.", + secondaryButton: nil + ) + + /// An open-limit shield. While opens remain it shows the running count and + /// an "Open (N left)" button; once spent it reads like a plain block. + static func openLimit( + opensUsed: Int, maxOpens: Int, sessionMinutes: Int + ) -> ShieldPresentation { + let remaining = max(0, maxOpens - opensUsed) + guard remaining > 0 else { + return ShieldPresentation( + title: blockedTitle, + subtitle: "No opens left today — the block lifts tomorrow.", + secondaryButton: nil + ) + } + return ShieldPresentation( + title: blockedTitle, + subtitle: "Opened \(opensUsed) of \(maxOpens) times today. " + + "Each open lasts \(sessionMinutes) minutes.", + secondaryButton: remaining == 1 ? "Open (1 left)" : "Open (\(remaining) left)" + ) + } +} diff --git a/docs/RULES_FEATURE_SPEC.md b/docs/RULES_FEATURE_SPEC.md index 9afa742..50d5d3b 100644 --- a/docs/RULES_FEATURE_SPEC.md +++ b/docs/RULES_FEATURE_SPEC.md @@ -485,9 +485,14 @@ Xh", "Xh left") is **derived**, not stored. `OpenSessionStore` reports a still-running granted open for it — so the foreground loop establishes the turnstile for newly created rules and never re-locks an app mid-session. -- **`OpenAppLockShieldConfig`** (ShieldConfiguration extension): open-limit - shields show "Opened X of N times today" with an "Open (Y left)" secondary - button; other shields show the blocking rule's name. +- **`OpenAppLockShieldConfig`** (ShieldConfiguration extension): every shield + carries the same generic **"App Blocked"** title — rule names are never shown, + since the rule a shield is attributed to cannot be determined reliably when + several rules cover the same app. Open-limit shields keep their functional + detail under that title ("Opened X of N times today" with an "Open (Y left)" + secondary button while opens remain); all other shields just read "This app is + blocked by OpenAppLock." The text-only decision lives in the pure, unit-tested + `ShieldPresentation` (in `Shared/`). - **`OpenAppLockShieldAction`** (ShieldAction extension): the Open press spends one open in the ledger, lifts the rule's shield, records the session expiry in `OpenSessionStore`, and starts the ~15-minute one-shot session