From 8044e27a87bdbc8fa328aec9e5219f8a5cdf8c37 Mon Sep 17 00:00:00 2001 From: t Date: Thu, 3 Sep 2026 10:51:00 -0400 Subject: [PATCH] fix(support): problem reports arrived with a blank Library line (PP-5078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `generateBody` rendered `Library: \(accountsManager.currentAccount?.name ?? "")`. `currentAccount` resolves through the library registry and is nil until that registry has loaded the selected account, so a patron who reports a problem before it settles sends a report whose Library line is empty — while the patron ID, a separate per-library lookup, resolves normally. Real ticket 18864 (1 Sep 2026, app 3.2.3) reached support as: Palace Version: 3.2.3 Library: Patron ID: 21467001510417 The agent triaging it recorded the summary as "Sign in prompt - no library?" — the blank actively implied the patron had no library configured. The patron had written that they were a member of Park Ridge Public Library. The line is now produced by `libraryFieldValue(name:uuid:)`, which yields three outcomes a reader can tell apart: * a real name — the common case, passed through verbatim * the identifier — the app knows WHICH library but cannot name it yet; support can resolve a UUID, and cannot resolve a blank * "(none selected)" — the app genuinely has no library A blank could equally have meant the line was lost in mail transit. None of these can. Whitespace-only names fall back too, since a space renders in the email exactly like nothing at all. Intermittent, not universal: of the recent problem reports sampled in HelpSpot, most carried a library name and a minority did not, which is why this has been easy to miss. **Scope:** the Library line in the problem-report body, plus its tests. The patron-ID resolution, the report's other fields, and registry loading are all untouched. **Not done — the XCTest suite was NOT run.** It could not be: on this branch `xcodebuild` refuses with "an out-of-date resolved file was detected at Package.resolved", a pre-existing condition of main's committed lockfile rather than anything this change introduces, and forcing resolution instead pulls a newer swift-toolkit that conflicts on sqlite.swift. CI runs on a clean environment and is the verification of record here. What WAS verified locally is the decision itself, executed standalone against all four cells: the old expression returns an empty string for a nil name (the shipped defect, reproduced), and each new cell returns what it should. That proves the logic, not the wiring into `generateBody`. **Not done:** no build-number bump. This is deliberate — bumping now would presume this ships alone, and un-bumping after a merge is worse than adding it. Bump `CURRENT_PROJECT_VERSION` (492 on main today) before cutting the build. **Deferred:** develop has since extracted this into `AccountsManager+ProblemReportContext`, where the name still comes from `currentAccount?.name` while the patron ID comes from the requested library — so develop additionally mislabels a report filed about library A while B is selected. The forward-port needs to carry the fallback into that extraction rather than apply this diff verbatim; the file's own header comment records the current behaviour as an intentional replication of the pre-extraction code. Co-Authored-By: Claude Opus 5 (1M context) --- Palace/ErrorHandling/ProblemReportEmail.swift | 36 +++++++++++- PalaceTests/ProblemReportEmailTests.swift | 57 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/Palace/ErrorHandling/ProblemReportEmail.swift b/Palace/ErrorHandling/ProblemReportEmail.swift index c07da6835..4eac46b32 100644 --- a/Palace/ErrorHandling/ProblemReportEmail.swift +++ b/Palace/ErrorHandling/ProblemReportEmail.swift @@ -65,6 +65,40 @@ import UIKit presentingViewController.present(mailComposeViewController, animated: true) } + /// The value rendered after `Library:` in a problem report. Never empty. + /// + /// PP-5078. This previously read `currentAccount?.name ?? ""`, so a nil + /// account emitted the bare line `Library:`. `currentAccount` resolves + /// through the library registry and is nil until that registry has loaded + /// the selected account — so a patron who reports a problem before it + /// settles sends a report with no library on it, while the patron ID (a + /// separate, per-library lookup) resolves normally. + /// + /// Real ticket 18864, app 3.2.3: `Library:` blank, `Patron ID:` populated. + /// The agent's triage summary read "Sign in prompt - no library?" — the + /// blank implied a patron with no library configured, and the patron had + /// written that they were a member of Park Ridge Public Library. + /// + /// Three outcomes, deliberately distinguishable by whoever reads the email: + /// - a real name — the common case, passed through verbatim + /// - the identifier — the app knows WHICH library but cannot name + /// it; support can resolve a UUID, not a blank + /// - "(none selected)" — the app genuinely has no library + /// + /// A blank could equally mean the line was lost in mail transit. None of + /// these can. + static func libraryFieldValue(name: String?, uuid: String?) -> String { + if let name = name?.trimmingCharacters(in: .whitespacesAndNewlines), + !name.isEmpty { + return name + } + if let uuid = uuid?.trimmingCharacters(in: .whitespacesAndNewlines), + !uuid.isEmpty { + return "(name unavailable — \(uuid))" + } + return "(none selected)" + } + func generateBody(book: TPPBook?, patronIdentifier: String? = nil, accountsManager: AccountsManager = AppContainer.production().accountsManager) -> String { let nativeHeight = UIScreen.main.nativeBounds.height let systemVersion = UIDevice.current.systemVersion @@ -91,7 +125,7 @@ import UIKit } let appVersion = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "1.0" - var body = "\n\n---\nIdiom: \(idiom)\nPlatform: iOS\nOS: \(systemVersion)\nHeight: \(nativeHeight)\nPalace Version: \(appVersion)\nLibrary: \(accountsManager.currentAccount?.name ?? "")" + var body = "\n\n---\nIdiom: \(idiom)\nPlatform: iOS\nOS: \(systemVersion)\nHeight: \(nativeHeight)\nPalace Version: \(appVersion)\nLibrary: \(Self.libraryFieldValue(name: accountsManager.currentAccount?.name, uuid: accountsManager.currentAccountId))" if let patronIdentifier = patronIdentifier { body += "\nPatron ID: \(patronIdentifier)" diff --git a/PalaceTests/ProblemReportEmailTests.swift b/PalaceTests/ProblemReportEmailTests.swift index e021a0ae2..7f6bcb59d 100644 --- a/PalaceTests/ProblemReportEmailTests.swift +++ b/PalaceTests/ProblemReportEmailTests.swift @@ -157,4 +157,61 @@ final class ProblemReportEmailTests: XCTestCase { XCTAssertFalse(body.contains(sentinel), "Sanity check that the body does not contain unrelated identifier-shaped text") } + + // MARK: - PP-5078: the Library line must never arrive blank + + /// The reported defect. `generateBody` rendered + /// `Library: \(accountsManager.currentAccount?.name ?? "")`, so whenever + /// `currentAccount` was nil the line went out as `Library:` with nothing + /// after it. + /// + /// Real ticket 18864 (1 Sep 2026, app 3.2.3) reached support as: + /// + /// Palace Version: 3.2.3 + /// Library: + /// Patron ID: 21467001510417 + /// + /// The patron ID resolved and the library name did not, because they come + /// from different sources — the ID is looked up per-library, the name is + /// read from `currentAccount`, which is nil until the library registry has + /// loaded that account. The agent triaging it recorded the summary as + /// "Sign in prompt - no library?", so the blank actively implied the patron + /// had no library configured; the patron had written that they were a + /// member of Park Ridge Public Library. + func testPP5078_libraryFieldValue_withNoNameAndNoUUID_isNotBlank() { + let value = ProblemReportEmail.libraryFieldValue(name: nil, uuid: nil) + XCTAssertFalse(value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty, + "Must render a placeholder, never an empty string. A blank cannot be " + + "distinguished by support from a line lost in email transit.") + } + + /// The case that actually produced ticket 18864: the app knows WHICH + /// library, it just cannot name it yet. Emitting the identifier turns a + /// dead end into something support can look up. + func testPP5078_libraryFieldValue_withUUIDButNoName_carriesTheIdentifier() { + let uuid = "urn:uuid:99d6a227-910c-484b-aaed-e323e247e959" + let value = ProblemReportEmail.libraryFieldValue(name: nil, uuid: uuid) + XCTAssertTrue(value.contains(uuid), + "When the registry cannot name the library, the report must still say " + + "which one it was — support can resolve a UUID, but not a blank.") + } + + /// Whitespace is a blank line wearing a hat: identical in the email, + /// equally useless to the agent reading it. + func testPP5078_libraryFieldValue_withWhitespaceName_fallsBack() { + for candidate in ["", " ", " ", "\n", " \t "] { + let value = ProblemReportEmail.libraryFieldValue(name: candidate, uuid: "urn:uuid:abc") + XCTAssertTrue(value.contains("urn:uuid:abc"), + "Whitespace-only name \(candidate.debugDescription) must fall back, not pass through.") + } + } + + /// The fix must not damage the common case — most reports do carry a name. + func testPP5078_libraryFieldValue_withRealName_isPassedThroughUnchanged() { + XCTAssertEqual( + ProblemReportEmail.libraryFieldValue(name: "Park Ridge Public Library", + uuid: "urn:uuid:whatever"), + "Park Ridge Public Library", + "A real library name must reach support verbatim, with no decoration.") + } }