From 5f49b57d813ebbc95175e245f6c9a1b1268f1c2b Mon Sep 17 00:00:00 2001 From: Pascal Wachowski <186315683+Pascal-SAPUI5@users.noreply.github.com> Date: Sun, 23 Aug 2026 01:04:55 +0000 Subject: [PATCH] Keep a bill's category and payment mode when it is updated Editing any bill in a Cospend project silently moved it to "uncategorised" and reset its payment mode on the server. Two things combined to cause it. `Bill` never decoded `categoryid` or `paymentmode`, so both values were dropped the moment a bill arrived from the server; and `paramsFor(_:)` then hardcoded `"categoryid": "0"` and `"paymentmode": "n"` into every request. Changing only a bill's amount was therefore enough to destroy its category, and Cospend's own category statistics degraded a little with every edit made from the app. Both fields are now decoded and round-tripped, falling back to the documented defaults for a bill that genuinely has none. `BillDetailViewModel.createBill()` carries them across the edit form, which has no field for either. iHateMoney is unaffected: it has no such concepts and neither key is sent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017Xgcu26TW3shuDwi9VENji --- PayForMe.xcodeproj/project.pbxproj | 4 + PayForMe/Model/Bill.swift | 13 +- .../BillDetail/BillDetailViewModel.swift | 13 +- .../BillCategoryPreservationTests.swift | 123 ++++++++++++++++++ 4 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 PayForMeTests/BillCategoryPreservationTests.swift diff --git a/PayForMe.xcodeproj/project.pbxproj b/PayForMe.xcodeproj/project.pbxproj index e5acab8..b8e00eb 100644 --- a/PayForMe.xcodeproj/project.pbxproj +++ b/PayForMe.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + CF5E3E2FB8C03CEFE2577197 /* BillCategoryPreservationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8B9248AB9EA3BF11E8FC22AC /* BillCategoryPreservationTests.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 */ + 8B9248AB9EA3BF11E8FC22AC /* BillCategoryPreservationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BillCategoryPreservationTests.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 = ""; }; @@ -227,6 +229,7 @@ 654754E22528B29F00A82EB6 /* PayForMeTests */ = { isa = PBXGroup; children = ( + 8B9248AB9EA3BF11E8FC22AC /* BillCategoryPreservationTests.swift */, 654754E32528B29F00A82EB6 /* AddProjectManuallyTests.swift */, 654754E52528B29F00A82EB6 /* Info.plist */, 6523FC4B25580EEF00BCD843 /* UrlExtensionsTests.swift */, @@ -543,6 +546,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + CF5E3E2FB8C03CEFE2577197 /* BillCategoryPreservationTests.swift in Sources */, 654754E42528B29F00A82EB6 /* AddProjectManuallyTests.swift in Sources */, 6523FC4C25580EEF00BCD843 /* UrlExtensionsTests.swift in Sources */, 57BC8D25B91CE40F4669650C /* TestHelpers.swift in Sources */, diff --git a/PayForMe/Model/Bill.swift b/PayForMe/Model/Bill.swift index e4b5651..e472563 100644 --- a/PayForMe/Model/Bill.swift +++ b/PayForMe/Model/Bill.swift @@ -16,6 +16,11 @@ struct Bill: Codable, Identifiable, Hashable { var owers: [Person] var `repeat`: String? var lastchanged: Int? + /// Cospend category id. Built-in categories are negative, custom ones + /// positive, `0`/`nil` means uncategorised. iHateMoney never sends it. + var categoryid: Int? + /// Cospend payment mode ("n" = none, "c" = card, "b" = cash, ...). + var paymentmode: String? func paramsFor(_ backend: ProjectBackend) -> [String: Any] { var dict: [String: Any] = [ @@ -26,8 +31,12 @@ struct Bill: Codable, Identifiable, Hashable { ] if backend == .cospend { dict["payed_for"] = owers.map { $0.id.description }.joined(separator: ",") - dict["paymentmode"] = "n" - dict["categoryid"] = "0" + // Round-trip whatever the server told us instead of resetting to the + // defaults. Updating a bill previously wiped its category and + // payment mode, because the fields were never decoded in the first + // place and were hardcoded back to the defaults on every PUT. + dict["paymentmode"] = paymentmode ?? "n" + dict["categoryid"] = (categoryid ?? 0).description if let rep = self.repeat { dict["repeat"] = rep diff --git a/PayForMe/Views/BillDetail/BillDetailViewModel.swift b/PayForMe/Views/BillDetail/BillDetailViewModel.swift index e8c7ca4..ea76dd5 100644 --- a/PayForMe/Views/BillDetail/BillDetailViewModel.swift +++ b/PayForMe/Views/BillDetail/BillDetailViewModel.swift @@ -68,7 +68,18 @@ class BillDetailViewModel: ObservableObject { let actualOwers = povm.actualOwers() - return Bill(id: billID, amount: doubleAmount, what: topic, date: date, payer_id: selectedPayer, owers: actualOwers, repeat: currentProject.backend == .cospend ? "n" : nil, lastchanged: 0) + // The edit form has no category or payment-mode field, so both are + // carried over from the bill being edited rather than dropped. + return Bill(id: billID, + amount: doubleAmount, + what: topic, + date: date, + payer_id: selectedPayer, + owers: actualOwers, + repeat: currentProject.backend == .cospend ? "n" : nil, + lastchanged: 0, + categoryid: currentBill.categoryid, + paymentmode: currentBill.paymentmode) } func prefillData() { diff --git a/PayForMeTests/BillCategoryPreservationTests.swift b/PayForMeTests/BillCategoryPreservationTests.swift new file mode 100644 index 0000000..9ec92d8 --- /dev/null +++ b/PayForMeTests/BillCategoryPreservationTests.swift @@ -0,0 +1,123 @@ +// +// BillCategoryPreservationTests.swift +// PayForMeTests +// +// Regression tests for a data-loss bug: editing any bill in a Cospend project +// reset its category and payment mode on the server. +// +// Two things went wrong together. `Bill` never decoded `categoryid` or +// `paymentmode`, so the values were lost the moment a bill arrived; and +// `paramsFor(_:)` hardcoded `"categoryid": "0"` and `"paymentmode": "n"` into +// every PUT. Changing a bill's amount therefore also silently moved it to +// "uncategorised" — and the category statistics in Cospend degraded a little +// more with each edit. +// + +import XCTest +@testable import PayForMe + +final class BillCategoryPreservationTests: XCTestCase { + + private let member = Person(id: 1, weight: 1, name: "Alice", activated: true) + private var testDate: Date { DateFormatter.cospend.date(from: "2026-05-14")! } + + private func decoder() -> JSONDecoder { + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .formatted(DateFormatter.cospend) + return decoder + } + + // MARK: Decoding + + func testCategoryAndPaymentModeAreDecoded() throws { + let json = """ + [{"id":42,"amount":12.5,"what":"Grocery run","date":"2026-05-14","payer_id":1, + "owers":[],"repeat":"n","categoryid":-1,"paymentmode":"c"}] + """.data(using: .utf8)! + + let bills = try decoder().decode([Bill].self, from: json) + XCTAssertEqual(bills.first?.categoryid, -1) + XCTAssertEqual(bills.first?.paymentmode, "c") + } + + /// iHateMoney never sends either field, and older Cospend payloads may not + /// either — their absence must not fail the whole response. + func testPayloadWithoutTheFieldsStillDecodes() throws { + let json = """ + [{"id":7,"amount":12.5,"what":"Tea","date":"2026-05-04","payer_id":1, + "owers":[],"repeat":"n"}] + """.data(using: .utf8)! + + let bills = try decoder().decode([Bill].self, from: json) + XCTAssertEqual(bills.count, 1) + XCTAssertNil(bills.first?.categoryid) + XCTAssertNil(bills.first?.paymentmode) + } + + // MARK: Request parameters + + func testParamsPreserveCategoryAndPaymentMode() { + let bill = Bill(id: 9, amount: 5, what: "Rent", date: testDate, payer_id: 1, + owers: [member], repeat: "n", lastchanged: nil, + categoryid: -3, paymentmode: "c") + + let params = bill.paramsFor(.cospend) + XCTAssertEqual(params["categoryid"] as? String, "-3", + "editing a bill must not move it to 'uncategorised'") + XCTAssertEqual(params["paymentmode"] as? String, "c") + } + + /// A brand new bill has neither, and the server expects the documented + /// defaults rather than an empty value. + func testParamsFallBackToCospendDefaults() { + let bill = Bill(id: -1, amount: 5, what: "New", date: testDate, payer_id: 1, + owers: [member], repeat: "n") + + let params = bill.paramsFor(.cospend) + XCTAssertEqual(params["categoryid"] as? String, "0") + XCTAssertEqual(params["paymentmode"] as? String, "n") + } + + /// iHateMoney has no such concepts; sending them would be noise. + func testIHateMoneyParamsCarryNeitherField() { + let bill = Bill(id: 9, amount: 5, what: "Rent", date: testDate, payer_id: 1, + owers: [member], repeat: "n", lastchanged: nil, + categoryid: -3, paymentmode: "c") + + let params = bill.paramsFor(.iHateMoney) + XCTAssertNil(params["categoryid"]) + XCTAssertNil(params["paymentmode"]) + } + + // MARK: The edit path + + /// The regression itself: load a categorised bill, edit it through the + /// detail view model, and the outgoing parameters must still carry the + /// original category. + func testEditingABillKeepsItsCategory() throws { + let json = """ + [{"id":42,"amount":12.5,"what":"Grocery run","date":"2026-05-14","payer_id":1, + "owers":[],"repeat":"n","categoryid":-1,"paymentmode":"c"}] + """.data(using: .utf8)! + let original = try decoder().decode([Bill].self, from: json)[0] + + let viewModel = BillDetailViewModel(currentBill: original) + let project = Project(name: "test", password: "pw", token: "tok", + backend: .cospend, url: URL(string: "https://test.de")!, + projectId: "test") + project.members = [member.id: member] + viewModel.currentProject = project + viewModel.selectedPayer = member.id + viewModel.topic = "Grocery run" + // The user changes only the amount. + viewModel.amount = "18.00" + viewModel.povm.members = [member] + viewModel.povm.isOwing = [true] + + let edited = try XCTUnwrap(viewModel.createBill()) + XCTAssertEqual(edited.amount, 18.0, accuracy: 0.001, "the edit itself still applies") + XCTAssertEqual(edited.categoryid, -1, "the category must survive the edit") + XCTAssertEqual(edited.paymentmode, "c", "the payment mode must survive the edit") + XCTAssertEqual(edited.paramsFor(.cospend)["categoryid"] as? String, "-1") + } +}