From 327685124f1af9db93162d717c6667b81e1119c3 Mon Sep 17 00:00:00 2001 From: Pascal Wachowski <186315683+Pascal-SAPUI5@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:21:58 +0200 Subject: [PATCH 1/2] Tell the user when a project cannot be loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changing a project's password on the server left PayForMe showing empty lists and no explanation. The same happened for a deleted project, an unreachable server, or an address that is not a Cospend or iHateMoney instance at all. The cause was structural. Both load publishers declared `Never` as their failure type, so every failure had to become a value before it could leave NetworkService: an HTTP status other than 200 was dropped by a `compactMap`, and a URLError was turned into `[]` by `replaceError`. An empty list is exactly what a project with no bills looks like, so nothing upstream could tell the two apart — and nothing did. Both publishers now fail with a `LoadError`, ProjectManager keeps the last failure, and ContentView shows it. The message names what happened: a rejected password suggests adding the project again with the new one, which is the action the reporter needed and could not guess. The type is called LoadError rather than ServerError because NetworkService already nests a `ServerError` of its own, and a second one under the same name resolves differently inside and outside the class. The two existing tests that pinned the old behaviour are rewritten rather than deleted — one of them already carried the note "should [emit] when proper feedback is implemented". Co-Authored-By: Claude Opus 5 --- PayForMe.xcodeproj/project.pbxproj | 4 + PayForMe/Services/NetworkService.swift | 76 ++++++++-- PayForMe/Services/ProjectManager.swift | 14 +- PayForMe/Strings/cs.lproj/Localizable.strings | 7 + PayForMe/Strings/de.lproj/Localizable.strings | 7 + PayForMe/Strings/en.lproj/Localizable.strings | 7 + PayForMe/Strings/es.lproj/Localizable.strings | 7 + PayForMe/Strings/fr.lproj/Localizable.strings | 7 + PayForMe/Strings/ru.lproj/Localizable.strings | 7 + PayForMe/Views/ContentView.swift | 22 +++ PayForMeTests/LoadErrorTests.swift | 137 ++++++++++++++++++ PayForMeTests/NetworkRequestTests.swift | 67 +++++---- PayForMeTests/TestHelpers.swift | 4 + 13 files changed, 322 insertions(+), 44 deletions(-) create mode 100644 PayForMeTests/LoadErrorTests.swift diff --git a/PayForMe.xcodeproj/project.pbxproj b/PayForMe.xcodeproj/project.pbxproj index e5acab8..cf7dad7 100644 --- a/PayForMe.xcodeproj/project.pbxproj +++ b/PayForMe.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + C34C21CC3AD72833B6C06C9D /* LoadErrorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD167A293B2F0746C0A09C2E /* LoadErrorTests.swift */; }; 481FB4FB23E964C8003BD108 /* ProjectManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 481FB4FA23E964C8003BD108 /* ProjectManager.swift */; }; 481FB4FD23EAD78F003BD108 /* AddProjectManualViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 481FB4FC23EAD78F003BD108 /* AddProjectManualViewModel.swift */; }; 489995DA23F6EC6F008B7E38 /* OnboardingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 489995D923F6EC6F008B7E38 /* OnboardingView.swift */; }; @@ -88,6 +89,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + CD167A293B2F0746C0A09C2E /* LoadErrorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadErrorTests.swift; sourceTree = ""; }; 0865D2DAF541B24078D1DC41 /* TestHelpers.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TestHelpers.swift; sourceTree = ""; }; 143F41EE78329905894C8A29 /* BalanceCalculationTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = BalanceCalculationTests.swift; sourceTree = ""; }; 316FBA7C0CE4ABB51EC2A121 /* NetworkRequestTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = NetworkRequestTests.swift; sourceTree = ""; }; @@ -228,6 +230,7 @@ isa = PBXGroup; children = ( 654754E32528B29F00A82EB6 /* AddProjectManuallyTests.swift */, + CD167A293B2F0746C0A09C2E /* LoadErrorTests.swift */, 654754E52528B29F00A82EB6 /* Info.plist */, 6523FC4B25580EEF00BCD843 /* UrlExtensionsTests.swift */, 0865D2DAF541B24078D1DC41 /* TestHelpers.swift */, @@ -544,6 +547,7 @@ buildActionMask = 2147483647; files = ( 654754E42528B29F00A82EB6 /* AddProjectManuallyTests.swift in Sources */, + C34C21CC3AD72833B6C06C9D /* LoadErrorTests.swift in Sources */, 6523FC4C25580EEF00BCD843 /* UrlExtensionsTests.swift in Sources */, 57BC8D25B91CE40F4669650C /* TestHelpers.swift in Sources */, D279295CFA25D9CC69C6F27C /* BillTests.swift in Sources */, diff --git a/PayForMe/Services/NetworkService.swift b/PayForMe/Services/NetworkService.swift index 42ba278..09cf79a 100644 --- a/PayForMe/Services/NetworkService.swift +++ b/PayForMe/Services/NetworkService.swift @@ -8,6 +8,49 @@ import Combine import Foundation +/// Why loading a project's data failed. +/// +/// The load publishers used to declare `Never` as their failure type, which +/// left everything upstream unable to tell "this project is empty" from "the +/// server rejected us". Changing a project's password on the server therefore +/// showed empty lists and no explanation at all (#37). +enum LoadError: Error, Equatable, Identifiable { + /// Credentials rejected. Usually the project password was changed on the + /// server after it was added here. + case unauthorized + case notFound + case http(Int) + /// No usable connection, or a response that was not HTTP at all. + case connection + /// The server answered, but not with something we could decode. + case invalidResponse + + var id: String { String(describing: self) } + + init(statusCode: Int) { + switch statusCode { + case 401, 403: + self = .unauthorized + case 404: + self = .notFound + default: + self = .http(statusCode) + } + } + + init(_ error: Error) { + switch error { + case let serverError as LoadError: + self = serverError + case is URLError: + self = .connection + default: + // Decoding failed: the server answered, we just could not read it. + self = .invalidResponse + } + } +} + class NetworkService { static let shared = NetworkService() @@ -34,16 +77,25 @@ class NetworkService { let networkActivityPublisher = PassthroughSubject() - func loadBillsPublisher(_ project: Project) -> AnyPublisher<[Bill], Never> { + /// Turns a response into its body or into a typed failure. Both cases used + /// to collapse into "no value emitted", which is precisely what made a + /// rejected password indistinguishable from an empty project. + private static func validate(_ data: Data, _ response: URLResponse) throws -> Data { + guard let httpResponse = response as? HTTPURLResponse else { + throw LoadError.connection + } + guard httpResponse.statusCode == 200 else { + throw LoadError(statusCode: httpResponse.statusCode) + } + return data + } + + func loadBillsPublisher(_ project: Project) -> AnyPublisher<[Bill], LoadError> { let request = buildURLRequest("bills", params: [:], project: project) return URLSession.shared.dataTaskPublisher(for: request) - .compactMap { data, response -> Data? in - guard let httpResponse = response as? HTTPURLResponse else { print("Network Error"); return nil } - guard httpResponse.statusCode == 200 else { print("Network Error: Status code: \(httpResponse.statusCode) \(httpResponse.description)"); return nil } - return data - } + .tryMap { data, response in try NetworkService.validate(data, response) } .decode(type: [Bill].self, decoder: decoder) - .replaceError(with: []) + .mapError { LoadError($0) } .map { $0.sorted { if let l1 = $0.lastchanged, @@ -57,16 +109,12 @@ class NetworkService { .eraseToAnyPublisher() } - func loadMembersPublisher(_ project: Project) -> AnyPublisher<[Int: Person], Never> { + func loadMembersPublisher(_ project: Project) -> AnyPublisher<[Int: Person], LoadError> { let request = buildURLRequest("members", params: [:], project: project) return URLSession.shared.dataTaskPublisher(for: request) - .compactMap { data, response -> Data? in - guard let httpResponse = response as? HTTPURLResponse else { print("Network Error"); return nil } - guard httpResponse.statusCode == 200 else { print("Network Error: Status code: \(httpResponse.statusCode) \(httpResponse.description)"); return nil } - return data - } + .tryMap { data, response in try NetworkService.validate(data, response) } .decode(type: [Person].self, decoder: decoder) - .replaceError(with: []) + .mapError { LoadError($0) } .map { members in let filtered = members.filter { diff --git a/PayForMe/Services/ProjectManager.swift b/PayForMe/Services/ProjectManager.swift index a8b474c..ace5144 100644 --- a/PayForMe/Services/ProjectManager.swift +++ b/PayForMe/Services/ProjectManager.swift @@ -26,6 +26,11 @@ class ProjectManager: ObservableObject { @Published var openedByURL: URL? + /// The last failure while loading bills and members, or nil after a load + /// that worked. Set so the UI can say something instead of showing lists + /// that are empty for no visible reason (#37). + @Published var loadingError: LoadError? + private init() { print("init") projects = storageService.loadProjects() @@ -80,8 +85,15 @@ class ProjectManager: ObservableObject { .receive(on: DispatchQueue.main) .handleEvents(receiveCancel: invokeCompletionOnce) .sink( - receiveCompletion: { _ in invokeCompletionOnce() }, + receiveCompletion: { [weak self] result in + if case let .failure(error) = result { + self?.loadingError = error + } + invokeCompletionOnce() + }, receiveValue: { [weak self] project in + // A load that worked clears whatever went wrong last time. + self?.loadingError = nil self?.currentProject = project invokeCompletionOnce() } diff --git a/PayForMe/Strings/cs.lproj/Localizable.strings b/PayForMe/Strings/cs.lproj/Localizable.strings index 7bbb911..8672b08 100644 --- a/PayForMe/Strings/cs.lproj/Localizable.strings +++ b/PayForMe/Strings/cs.lproj/Localizable.strings @@ -150,3 +150,10 @@ "member_error_forbidden" = "To obvykle vyžaduje práva správce projektu. Sdílené heslo projektu může umožňovat jen přístup účastníka."; "member_error_network" = "Není spojení se serverem. Zkontrolujte síť a zkuste to znovu."; "member_error_generic" = "Server odmítl požadavek (HTTP %d)."; + +"Could not load project" = "Projekt se nepodařilo načíst"; +"load_error_unauthorized" = "Server odmítl uložené heslo. Pokud se heslo projektu změnilo, přidej projekt znovu s novým heslem."; +"load_error_not_found" = "Server tento projekt nezná. Možná byl smazán nebo je špatné ID projektu."; +"load_error_connection" = "Žádné spojení se serverem. Zkontroluj síť a zkus to znovu."; +"load_error_invalid_response" = "Odpověď serveru nelze přečíst. Možná nejde o server Cospend ani iHateMoney."; +"load_error_generic" = "Server projekt neodeslal (HTTP %d)."; diff --git a/PayForMe/Strings/de.lproj/Localizable.strings b/PayForMe/Strings/de.lproj/Localizable.strings index 0c4cfdb..8d685d3 100644 --- a/PayForMe/Strings/de.lproj/Localizable.strings +++ b/PayForMe/Strings/de.lproj/Localizable.strings @@ -154,3 +154,10 @@ "member_error_forbidden" = "Für den Erfolg sind meist Maintainer-Rechte am Projekt nötig. Du kannst diese Berechtigungen jederzeit erteilen, indem du die Einstellungen des Freigabelinks in Cospend (deiner Nextcloud-App) änderst."; "member_error_network" = "Keine Verbindung zum Server. Bitte Netzwerk prüfen und erneut versuchen."; "member_error_generic" = "Der Server hat die Anfrage abgelehnt (HTTP %d)."; + +"Could not load project" = "Projekt konnte nicht geladen werden"; +"load_error_unauthorized" = "Der Server hat das gespeicherte Passwort abgelehnt. Wurde das Projektpasswort geändert, füge das Projekt mit dem neuen Passwort erneut hinzu."; +"load_error_not_found" = "Der Server kennt dieses Projekt nicht. Möglicherweise wurde es gelöscht oder die Projekt-ID stimmt nicht."; +"load_error_connection" = "Keine Verbindung zum Server. Bitte prüfe dein Netzwerk und versuche es erneut."; +"load_error_invalid_response" = "Die Antwort des Servers war nicht lesbar. Möglicherweise ist es kein Cospend- oder iHateMoney-Server."; +"load_error_generic" = "Der Server hat das Projekt nicht herausgegeben (HTTP %d)."; diff --git a/PayForMe/Strings/en.lproj/Localizable.strings b/PayForMe/Strings/en.lproj/Localizable.strings index 8fc28df..5c73bef 100644 --- a/PayForMe/Strings/en.lproj/Localizable.strings +++ b/PayForMe/Strings/en.lproj/Localizable.strings @@ -157,3 +157,10 @@ "member_error_forbidden" = "This usually requires maintainer rights for the project. You can grant those at any time by changing the share-link's settings in Cospend (your Nextcloud-App)."; "member_error_network" = "No connection to the server. Please check your network and try again."; "member_error_generic" = "The server rejected the request (HTTP %d)."; + +"Could not load project" = "Could not load project"; +"load_error_unauthorized" = "The server rejected the stored password. If the project password was changed, add the project again with the new one."; +"load_error_not_found" = "The server does not know this project. It may have been deleted, or the project id may be wrong."; +"load_error_connection" = "No connection to the server. Please check your network and try again."; +"load_error_invalid_response" = "The server's answer could not be read. It may not be a Cospend or iHateMoney server."; +"load_error_generic" = "The server refused to send the project (HTTP %d)."; diff --git a/PayForMe/Strings/es.lproj/Localizable.strings b/PayForMe/Strings/es.lproj/Localizable.strings index c297e8d..fd50c45 100644 --- a/PayForMe/Strings/es.lproj/Localizable.strings +++ b/PayForMe/Strings/es.lproj/Localizable.strings @@ -157,3 +157,10 @@ "Enter invite link" = "Introducir el enlace de invitación"; "invite_link_hint" = "Pega el enlace de invitación completo. El servidor y el proyecto se detectan automáticamente."; "Server returned an invalid HTTP response" = "El servidor devolvió una respuesta HTTP no válida"; + +"Could not load project" = "No se pudo cargar el proyecto"; +"load_error_unauthorized" = "El servidor rechazó la contraseña guardada. Si se cambió la contraseña del proyecto, añádelo de nuevo con la nueva."; +"load_error_not_found" = "El servidor no conoce este proyecto. Puede que se haya eliminado o que el identificador sea incorrecto."; +"load_error_connection" = "Sin conexión con el servidor. Comprueba tu red e inténtalo de nuevo."; +"load_error_invalid_response" = "No se pudo leer la respuesta del servidor. Puede que no sea un servidor Cospend o iHateMoney."; +"load_error_generic" = "El servidor se negó a enviar el proyecto (HTTP %d)."; diff --git a/PayForMe/Strings/fr.lproj/Localizable.strings b/PayForMe/Strings/fr.lproj/Localizable.strings index 3f52860..11677eb 100644 --- a/PayForMe/Strings/fr.lproj/Localizable.strings +++ b/PayForMe/Strings/fr.lproj/Localizable.strings @@ -157,3 +157,10 @@ "Enter invite link" = "Saisir le lien d'invitation"; "invite_link_hint" = "Collez le lien d'invitation complet. Le serveur et le projet sont détectés automatiquement."; "Server returned an invalid HTTP response" = "Le serveur a renvoyé une réponse HTTP non valide"; + +"Could not load project" = "Impossible de charger le projet"; +"load_error_unauthorized" = "Le serveur a refusé le mot de passe enregistré. Si le mot de passe du projet a changé, ajoutez à nouveau le projet avec le nouveau."; +"load_error_not_found" = "Le serveur ne connaît pas ce projet. Il a peut-être été supprimé, ou l'identifiant est incorrect."; +"load_error_connection" = "Pas de connexion au serveur. Vérifiez votre réseau et réessayez."; +"load_error_invalid_response" = "La réponse du serveur est illisible. Ce n'est peut-être pas un serveur Cospend ou iHateMoney."; +"load_error_generic" = "Le serveur a refusé d'envoyer le projet (HTTP %d)."; diff --git a/PayForMe/Strings/ru.lproj/Localizable.strings b/PayForMe/Strings/ru.lproj/Localizable.strings index 985ec73..04d276e 100644 --- a/PayForMe/Strings/ru.lproj/Localizable.strings +++ b/PayForMe/Strings/ru.lproj/Localizable.strings @@ -156,3 +156,10 @@ "member_error_forbidden" = "Обычно для этого нужны права сопровождающего проекта. Пароль общего доступа может давать только права участника."; "member_error_network" = "Нет соединения с сервером. Проверьте сеть и повторите попытку."; "member_error_generic" = "Сервер отклонил запрос (HTTP %d)."; + +"Could not load project" = "Не удалось загрузить проект"; +"load_error_unauthorized" = "Сервер отклонил сохранённый пароль. Если пароль проекта изменился, добавьте проект заново с новым паролем."; +"load_error_not_found" = "Сервер не знает этот проект. Возможно, он удалён или указан неверный идентификатор."; +"load_error_connection" = "Нет соединения с сервером. Проверьте сеть и попробуйте снова."; +"load_error_invalid_response" = "Не удалось прочитать ответ сервера. Возможно, это не сервер Cospend или iHateMoney."; +"load_error_generic" = "Сервер отказался передать проект (HTTP %d)."; diff --git a/PayForMe/Views/ContentView.swift b/PayForMe/Views/ContentView.swift index 829ff1e..f75e80a 100644 --- a/PayForMe/Views/ContentView.swift +++ b/PayForMe/Views/ContentView.swift @@ -46,6 +46,28 @@ struct ContentView: View { manager.loadBillsAndMembers() } } + // Without this the app just shows empty lists when the server turns us + // away — the complaint behind #37. + .alert(item: $manager.loadingError) { error in + Alert(title: Text("Could not load project"), + message: Text(Self.message(for: error)), + dismissButton: .default(Text("OK"))) + } + } + + static func message(for error: LoadError) -> String { + switch error { + case .unauthorized: + return NSLocalizedString("load_error_unauthorized", comment: "Loading failed because the credentials were rejected") + case .notFound: + return NSLocalizedString("load_error_not_found", comment: "Loading failed because the project does not exist on the server") + case .connection: + return NSLocalizedString("load_error_connection", comment: "Loading failed because the server could not be reached") + case .invalidResponse: + return NSLocalizedString("load_error_invalid_response", comment: "Loading failed because the server's answer could not be read") + case let .http(code): + return String(format: NSLocalizedString("load_error_generic", comment: "Loading failed with an HTTP status code"), code) + } } var tabBar: some View { diff --git a/PayForMeTests/LoadErrorTests.swift b/PayForMeTests/LoadErrorTests.swift new file mode 100644 index 0000000..d5fcd6f --- /dev/null +++ b/PayForMeTests/LoadErrorTests.swift @@ -0,0 +1,137 @@ +// +// LoadErrorTests.swift +// PayForMeTests +// +// Changing a project's password on the server left the app showing empty +// lists and no explanation — #37. The cause was structural: both load +// publishers declared `Never` as their failure type, so every failure had to +// be turned into a value before it could leave NetworkService. An HTTP 401 was +// dropped by a `compactMap` and a URLError became `[]`, which is exactly what +// a project with no members looks like. +// +// These tests pin down that a failure now reaches the caller as a failure. +// + +import Combine +import XCTest +@testable import PayForMe + +final class LoadErrorTests: XCTestCase { + + private var subscriptions = Set() + private var savedProject: Project! + + override func setUp() { + super.setUp() + URLProtocol.registerClass(MockURLProtocol.self) + MockURLProtocol.reset() + savedProject = ProjectManager.shared.currentProject + } + + override func tearDown() { + URLProtocol.unregisterClass(MockURLProtocol.self) + subscriptions.removeAll() + ProjectManager.shared.currentProject = savedProject + super.tearDown() + } + + /// Runs a load against a stubbed response and returns how it ended. + private func failure(loading load: (Project) -> AnyPublisher<[Bill], LoadError>, + status: Int, + body: String = "[]", + file: StaticString = #filePath, + line: UInt = #line) -> LoadError? { + MockURLProtocol.requestHandler = { request in + (.status(status, for: request.url!), Data(body.utf8)) + } + + var received: LoadError? + let exp = expectation(description: "publisher finished") + load(Project.makeCospend()) + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } + exp.fulfill() + }, receiveValue: { _ in }) + .store(in: &subscriptions) + waitForExpectations(timeout: 2) + return received + } + + // MARK: The case from the issue + + /// The password was changed on the server: Cospend answers 401, and until + /// now that was indistinguishable from a project with no bills. + func testRejectedCredentialsReachTheCaller() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, status: 401), + .unauthorized) + } + + func testForbiddenCountsAsRejectedCredentials() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, status: 403), + .unauthorized) + } + + // MARK: The other outcomes + + func testAnUnexpectedStatusIsPassedThroughVerbatim() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, status: 500), + .http(500)) + } + + /// A 200 whose body is not what we expect — pointing the app at something + /// that is not a Cospend or iHateMoney server. + func testAnUnreadableBodyIsAnInvalidResponse() { + XCTAssertEqual(failure(loading: NetworkService.shared.loadBillsPublisher, + status: 200, body: "hello"), + .invalidResponse) + } + + func testMembersFailTheSameWay() { + MockURLProtocol.requestHandler = { request in + (.status(401, for: request.url!), Data("[]".utf8)) + } + + var received: LoadError? + let exp = expectation(description: "publisher finished") + NetworkService.shared.loadMembersPublisher(Project.makeCospend()) + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } + exp.fulfill() + }, receiveValue: { _ in + XCTFail("no members should arrive on 401") + }) + .store(in: &subscriptions) + waitForExpectations(timeout: 2) + XCTAssertEqual(received, .unauthorized) + } + + // MARK: Status-code mapping + + func testStatusCodesMapToTheRightCase() { + XCTAssertEqual(LoadError(statusCode: 401), .unauthorized) + XCTAssertEqual(LoadError(statusCode: 403), .unauthorized) + XCTAssertEqual(LoadError(statusCode: 404), .notFound) + XCTAssertEqual(LoadError(statusCode: 503), .http(503)) + } + + // MARK: What the user is told + + /// Every case needs a message, and it has to be a translated one. A missing + /// entry in Localizable.strings makes NSLocalizedString hand back the key + /// itself, which would ship as "load_error_unauthorized" on screen. + func testEveryErrorHasATranslatedMessage() { + let errors: [LoadError] = [.unauthorized, .notFound, .connection, .invalidResponse, .http(500)] + var messages = Set() + + for error in errors { + let message = ContentView.message(for: error) + XCTAssertFalse(message.isEmpty, "\(error) has no message") + XCTAssertFalse(message.hasPrefix("load_error_"), + "\(error) falls back to the raw key — the string is missing from Localizable.strings") + messages.insert(message) + } + + XCTAssertEqual(messages.count, errors.count, + "each error needs its own message, otherwise they are not worth distinguishing") + } +} diff --git a/PayForMeTests/NetworkRequestTests.swift b/PayForMeTests/NetworkRequestTests.swift index 848687a..c95f3a3 100644 --- a/PayForMeTests/NetworkRequestTests.swift +++ b/PayForMeTests/NetworkRequestTests.swift @@ -77,7 +77,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -98,7 +98,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadMembersPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -116,7 +116,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -136,7 +136,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -156,7 +156,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -176,7 +176,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -194,7 +194,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -264,40 +264,49 @@ class NetworkRequestTests: XCTestCase { // MARK: - loadBills() — graceful empty result on error - func testLoadBills_on404_publisherCompletesWithoutEmittingBills() { + func testLoadBills_on404_failsWithNotFound() { let project = Project.makeCospend() MockURLProtocol.requestHandler = { req in (.notFound(for: req.url!), Data()) } - var didReceiveBills = false - let exp = expectation(description: "publisher completes without emitting (status quo, not ideal)") + var received: LoadError? + let exp = expectation(description: "publisher fails") NetworkService.shared.loadBillsPublisher(project) - .handleEvents(receiveCompletion: { _ in exp.fulfill() }) - .sink { _ in didReceiveBills = true } + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } + exp.fulfill() + }, receiveValue: { _ in + XCTFail("no bills should arrive on 404") + }) .store(in: &subscriptions) waitForExpectations(timeout: 2) - XCTAssertFalse(didReceiveBills, - "loadBillsPublisher must NOT emit on 404 — but should when proper feedback is implemented") + XCTAssertEqual(received, .notFound, + "a missing project has to reach the UI, not vanish into an empty list") } - func testLoadMembers_returnsEmptyDictOnNetworkFailure() { + /// This used to emit `[:]`, which the UI could not tell apart from a project + /// that genuinely has no members. + func testLoadMembers_onNetworkFailure_failsWithConnection() { let project = Project.makeCospend() MockURLProtocol.requestHandler = { _ in throw URLError(.timedOut) } - let exp = expectation(description: "empty members received") + var received: LoadError? + let exp = expectation(description: "publisher fails") NetworkService.shared.loadMembersPublisher(project) - .sink { members in - XCTAssertTrue(members.isEmpty, - "loadMembers must return [:] on network failure, not crash") + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { received = error } exp.fulfill() - } + }, receiveValue: { _ in + XCTFail("no members should arrive when the connection failed") + }) .store(in: &subscriptions) waitForExpectations(timeout: 2) + XCTAssertEqual(received, .connection) } // MARK: - HTTP methods @@ -308,7 +317,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadBillsPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -321,7 +330,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.loadMembersPublisher(project) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -337,7 +346,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -353,7 +362,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -369,7 +378,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -387,7 +396,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.postBillPublisher(bill: .make()) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -430,7 +439,7 @@ class NetworkRequestTests: XCTestCase { let bill = Bill.make(id: 42) let exp = expectation(description: "request intercepted") NetworkService.shared.updateBillPublisher(bill: bill) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -449,7 +458,7 @@ class NetworkRequestTests: XCTestCase { let bill = Bill.make(id: 7) let exp = expectation(description: "request intercepted") NetworkService.shared.deleteBillPublisher(bill: bill) - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) @@ -469,7 +478,7 @@ class NetworkRequestTests: XCTestCase { let exp = expectation(description: "request intercepted") NetworkService.shared.createMemberPublisher(name: "Alice") - .sink { _ in exp.fulfill() } + .sink(receiveCompletion: { _ in exp.fulfill() }, receiveValue: { _ in }) .store(in: &subscriptions) waitForExpectations(timeout: 2) diff --git a/PayForMeTests/TestHelpers.swift b/PayForMeTests/TestHelpers.swift index 76bdddf..7a256ac 100644 --- a/PayForMeTests/TestHelpers.swift +++ b/PayForMeTests/TestHelpers.swift @@ -147,4 +147,8 @@ extension HTTPURLResponse { static func notFound(for url: URL) -> HTTPURLResponse { HTTPURLResponse(url: url, statusCode: 404, httpVersion: nil, headerFields: nil)! } + + static func status(_ code: Int, for url: URL) -> HTTPURLResponse { + HTTPURLResponse(url: url, statusCode: code, httpVersion: nil, headerFields: nil)! + } } From 6f663b7bfa2c312efdf7f0081ad972c1c10df176 Mon Sep 17 00:00:00 2001 From: Pascal Wachowski <186315683+Pascal-SAPUI5@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:29:30 +0200 Subject: [PATCH 2/2] CI: allow manual dispatch on this verification branch (never merged) --- .github/workflows/ios.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index aa2d288..14b7866 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -5,6 +5,7 @@ on: branches: [ "main" ] pull_request: branches: [ "main" ] + workflow_dispatch: jobs: build: