From 9abb03ee4faf0f292181d1fa2c77615f542d83d6 Mon Sep 17 00:00:00 2001 From: tajniak81 <13187254+tajniak81@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:26:56 +0200 Subject: [PATCH] Service history: 0 km is a reading, not a blank MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A car collected new sits at 0 km, and every km calculation in the app quietly refused to work for it. ComputeDerived only filled NextServiceKm when Km > 0, so a service record entered at 0 produced no next-due distance at all — the date side worked, because it guards on IsZero(), which is genuine absence rather than a number that happens to be low. The same conflation had been copied outward from there. The reminder's km signal wanted currentKm > 0 before it would count anything down, the web badge and the service-life ring tested the odometer for truthiness, formatKm printed an em dash for zero, and fuel and charging rejected a 0 km entry as "odometer (km) is required" — which is the first charge of an EV on the driveway on delivery day. The phone app carried its own copy of each. Editing such a car offered an empty odometer box, since the forms only prefilled a reading above zero. Everywhere the odometer is a measurement, absence is now tested as absence: null in the clients, negative on the server, and the required fields check that the box was filled rather than that the number cleared zero. Fuel and charging validate Km < 0 instead, and their inputs drop min="1". Completing a repeating km reminder rolls from the car's actual reading in every case; the old fallback to the previous target existed to keep an untracked car off a due date in the past, but CurrentKm + RepeatKm is ahead of the car by construction, so it could not have happened. Left as it was: dueKm, repeatKm and the service intervals, where zero really does encode "no trigger" and "use the default", and the liters and kwh checks, since a zero fill is not a fill. Maintenance is the exception. Its odometer is the one that is genuinely optional, so zero there still has to mean "not recorded" and those three sites keep the truthiness test, commented. Fixing that properly wants a nullable field rather than an int, which is a schema change and its own commit — the same shape of problem as the latency em dash in 3c4eba8. Co-Authored-By: Claude Opus 5 --- API Server/internal/api/charging.go | 4 +- API Server/internal/api/fuel.go | 4 +- API Server/internal/api/records_test.go | 27 ++++++++ API Server/internal/api/reminders.go | 14 ++--- API Server/internal/models/models.go | 12 +++- API Server/internal/models/service_test.go | 61 +++++++++++++++++++ Phone App/lib/format.dart | 5 +- Phone App/lib/screens/car_detail_screen.dart | 15 +++-- Phone App/lib/screens/car_form_sheet.dart | 4 +- Phone App/lib/screens/dashboard_screen.dart | 3 +- Phone App/lib/screens/record_form_sheets.dart | 10 +-- Web App/web/src/components/CarFormModal.vue | 4 +- .../web/src/components/ChargingFormModal.vue | 2 +- Web App/web/src/components/FuelFormModal.vue | 2 +- .../web/src/components/ReminderFormModal.vue | 2 +- .../web/src/components/ServiceFormModal.vue | 6 +- Web App/web/src/lib/format.js | 12 +++- Web App/web/src/views/Dashboard.vue | 3 +- 18 files changed, 150 insertions(+), 40 deletions(-) create mode 100644 API Server/internal/api/records_test.go create mode 100644 API Server/internal/models/service_test.go diff --git a/API Server/internal/api/charging.go b/API Server/internal/api/charging.go index 4260ecd..2b73e69 100644 --- a/API Server/internal/api/charging.go +++ b/API Server/internal/api/charging.go @@ -222,8 +222,8 @@ func validateChargingSession(c models.ChargingSession) string { return "car is required" case c.Date.IsZero(): return "date is required" - case c.Km <= 0: - return "odometer (km) is required" + case c.Km < 0: + return "odometer (km) cannot be negative" case c.Kwh <= 0: return "kwh must be greater than zero" case c.Cost < 0: diff --git a/API Server/internal/api/fuel.go b/API Server/internal/api/fuel.go index cafe234..7c30b29 100644 --- a/API Server/internal/api/fuel.go +++ b/API Server/internal/api/fuel.go @@ -220,8 +220,8 @@ func validateFuelEntry(f models.FuelEntry) string { return "car is required" case f.Date.IsZero(): return "date is required" - case f.Km <= 0: - return "odometer (km) is required" + case f.Km < 0: + return "odometer (km) cannot be negative" case f.Liters <= 0: return "liters must be greater than zero" case f.Cost < 0: diff --git a/API Server/internal/api/records_test.go b/API Server/internal/api/records_test.go new file mode 100644 index 0000000..b030fbb --- /dev/null +++ b/API Server/internal/api/records_test.go @@ -0,0 +1,27 @@ +package api + +import ( + "testing" + "time" + + "drivervault/apiserver/internal/models" +) + +// A car collected new can be fuelled or charged at 0 km on the forecourt; only +// a negative reading is impossible. +func TestZeroKmPassesRecordValidation(t *testing.T) { + now := time.Now() + + if got := validateFuelEntry(models.FuelEntry{Car: "c1", Date: now, Km: 0, Liters: 40}); got != "" { + t.Errorf("fuel at 0 km rejected: %s", got) + } + if got := validateFuelEntry(models.FuelEntry{Car: "c1", Date: now, Km: -1, Liters: 40}); got == "" { + t.Error("fuel at -1 km accepted") + } + if got := validateChargingSession(models.ChargingSession{Car: "c1", Date: now, Km: 0, Kwh: 12}); got != "" { + t.Errorf("charge at 0 km rejected: %s", got) + } + if got := validateChargingSession(models.ChargingSession{Car: "c1", Date: now, Km: -1, Kwh: 12}); got == "" { + t.Error("charge at -1 km accepted") + } +} diff --git a/API Server/internal/api/reminders.go b/API Server/internal/api/reminders.go index 4d70342..8025e45 100644 --- a/API Server/internal/api/reminders.go +++ b/API Server/internal/api/reminders.go @@ -317,14 +317,12 @@ func (s *Server) handleCompleteReminder(w http.ResponseWriter, r *http.Request) // Roll from where the car actually is: the work was done now, so the // next one is due RepeatKm from this reading, whether it was done // early or late. Rolling from the old target instead would let an - // early completion drift the schedule forward for good. The target is - // only a fallback for a car whose odometer is untracked (0), where - // rolling from zero would put the next due date in the past. - base := car.CurrentKm - if base <= 0 { - base = m.DueKm - } - payload["due_km"] = base + m.RepeatKm + // early completion drift the schedule forward for good. + // + // A reading of 0 rolls from 0 like any other: a car collected new is + // genuinely there, and CurrentKm + RepeatKm is ahead of the car by + // construction, so this cannot land a target in the past. + payload["due_km"] = car.CurrentKm + m.RepeatKm } payload["done"] = false payload["done_at"] = "" diff --git a/API Server/internal/models/models.go b/API Server/internal/models/models.go index 14ed78c..36f711c 100644 --- a/API Server/internal/models/models.go +++ b/API Server/internal/models/models.go @@ -469,6 +469,11 @@ type Session struct { // ComputeDerived fills NextServiceDate / NextServiceKm from the car's intervals, // reproducing the spreadsheet formulas. Intervals of 0 fall back to the // spreadsheet defaults (365 days, 15000 km). +// +// Note the asymmetry in what counts as "no reading": a zero date is genuinely +// absent, but a zero odometer is a reading. A car collected new sits at 0 km +// and its first service is still due 15000 km later, so 0 has to produce a +// next-due figure like any other number would. func (r *ServiceRecord) ComputeDerived(c *Car) { days := c.ServiceIntervalDays if days <= 0 { @@ -482,7 +487,7 @@ func (r *ServiceRecord) ComputeDerived(c *Car) { d := r.Date.AddDate(0, 0, days) r.NextServiceDate = &d } - if r.Km > 0 { + if r.Km >= 0 { n := r.Km + km r.NextServiceKm = &n } @@ -841,7 +846,10 @@ func (r *Reminder) ComputeReminderDerived(now time.Time, currentKm int) { } } - if r.DueKm > 0 && currentKm > 0 { + // currentKm == 0 is a real odometer, not a missing one — see ComputeDerived. + // Without this a brand-new car's first service reminder shows a due date but + // never the distance left to run. + if r.DueKm > 0 && currentKm >= 0 { left := r.DueKm - currentKm r.KmLeft = &left switch { diff --git a/API Server/internal/models/service_test.go b/API Server/internal/models/service_test.go new file mode 100644 index 0000000..7d99fef --- /dev/null +++ b/API Server/internal/models/service_test.go @@ -0,0 +1,61 @@ +package models + +import ( + "testing" + "time" +) + +// A car collected new sits at 0 km; its first service is still due a full +// interval later, and the reminder must show the distance left to run. +func TestZeroKmStillYieldsNextService(t *testing.T) { + car := &Car{ServiceIntervalDays: 365, ServiceIntervalKm: 15000, CurrentKm: 0} + pickup := time.Date(2026, 8, 10, 0, 0, 0, 0, time.UTC) + rec := ServiceRecord{Date: pickup, Km: 0} + rec.ComputeDerived(car) + + if rec.NextServiceDate == nil || !rec.NextServiceDate.Equal(pickup.AddDate(0, 0, 365)) { + t.Fatalf("next service date = %v", rec.NextServiceDate) + } + if rec.NextServiceKm == nil { + t.Fatal("next service km is nil for a 0 km car") + } + if *rec.NextServiceKm != 15000 { + t.Fatalf("next service km = %d, want 15000", *rec.NextServiceKm) + } + + rem := Reminder{DueKm: *rec.NextServiceKm} + rem.ComputeReminderDerived(time.Now(), car.CurrentKm) + if rem.KmLeft == nil || *rem.KmLeft != 15000 { + t.Fatalf("km left = %v, want 15000", rem.KmLeft) + } + if rem.Status != "upcoming" { + t.Fatalf("status = %q, want upcoming", rem.Status) + } +} + +// A repeating km reminder completed on a car that reads 0 rolls from 0, not +// from the old target — the odometer is where the car is, not a missing value. +func TestZeroKmReminderStatus(t *testing.T) { + for _, tc := range []struct { + name string + currentKm int + dueKm int + wantLeft int + wantState string + }{ + {"collected new", 0, 15000, 15000, "upcoming"}, + {"nearly due", 14500, 15000, 500, "due_soon"}, + {"overdue", 15500, 15000, -500, "overdue"}, + } { + t.Run(tc.name, func(t *testing.T) { + rem := Reminder{DueKm: tc.dueKm} + rem.ComputeReminderDerived(time.Now(), tc.currentKm) + if rem.KmLeft == nil || *rem.KmLeft != tc.wantLeft { + t.Fatalf("km left = %v, want %d", rem.KmLeft, tc.wantLeft) + } + if rem.Status != tc.wantState { + t.Fatalf("status = %q, want %q", rem.Status, tc.wantState) + } + }) + } +} diff --git a/Phone App/lib/format.dart b/Phone App/lib/format.dart index 9a10d6e..ba0634e 100644 --- a/Phone App/lib/format.dart +++ b/Phone App/lib/format.dart @@ -37,7 +37,8 @@ String formatDate(DateTime? d) { return DateFormat(pattern, _locale).format(d); } -String formatKm(int? km) => (km == null || km == 0) ? "—" : "${_num(km)} km"; +// 0 km is a reading — a car collected new — not a blank. See format.js. +String formatKm(int? km) => km == null ? "—" : "${_num(km)} km"; // The fuel figures. The server sends null for anything it could not derive (a // window with a missed fill, a first-ever tank), which reads as "—" rather than @@ -109,7 +110,7 @@ Status _dateSignal(DateTime? nextDate) { } Status _kmSignal(int currentKm, int? nextKm) { - if (currentKm == 0 || nextKm == null) return Status(StatusKey.unknown, t("status.noKm")); + if (nextKm == null) return Status(StatusKey.unknown, t("status.noKm")); final remaining = nextKm - currentKm; if (remaining < 0) return Status(StatusKey.overdue, t("status.serviceOverdueKm", params: {"km": _num(remaining.abs())})); if (remaining <= _kmSoon) return Status(StatusKey.soon, t("status.inKm", params: {"km": _num(remaining)})); diff --git a/Phone App/lib/screens/car_detail_screen.dart b/Phone App/lib/screens/car_detail_screen.dart index 7943daf..77afeb2 100644 --- a/Phone App/lib/screens/car_detail_screen.dart +++ b/Phone App/lib/screens/car_detail_screen.dart @@ -234,7 +234,7 @@ class _CarDetailScreenState extends State { } Future _editOdometer(Car car) async { - final controller = TextEditingController(text: car.currentKm > 0 ? "${car.currentKm}" : ""); + final controller = TextEditingController(text: "${car.currentKm}"); final saved = await showDialog( context: context, builder: (ctx) => AlertDialog( @@ -986,7 +986,7 @@ class _MaintenanceTile extends StatelessWidget { context, [ formatDate(entry.date), - if (entry.km > 0) formatKm(entry.km), + if (entry.km > 0) formatKm(entry.km), // optional on maintenance _typeLabels[entry.type] ?? entry.type, _statusLabels[entry.status] ?? entry.status, ].join(" · ")), @@ -1623,7 +1623,7 @@ class _ServiceSheetState extends State<_ServiceSheet> { super.initState(); final r = widget.record; _date = r?.date ?? DateTime.now(); - _km = TextEditingController(text: (r != null && r.km > 0) ? "${r.km}" : ""); + _km = TextEditingController(text: r == null ? "" : "${r.km}"); _notes = TextEditingController(text: r?.notes ?? ""); _oil = r?.changedOil ?? true; _engine = r?.changedEngineAirFilter ?? false; @@ -1638,6 +1638,11 @@ class _ServiceSheetState extends State<_ServiceSheet> { } Future _save() async { + final km = int.tryParse(_km.text.trim()) ?? 0; + if (_km.text.trim().isEmpty || km < 0) { + setState(() => _error = "Odometer is required."); + return; + } setState(() { _saving = true; _error = null; @@ -1645,7 +1650,7 @@ class _ServiceSheetState extends State<_ServiceSheet> { final payload = { "car": widget.carId, "date": _date.toUtc().toIso8601String(), - "km": int.tryParse(_km.text.trim()) ?? 0, + "km": km, "changedOil": _oil, "changedEngineAirFilter": _engine, "changedCabinAirFilter": _cabin, @@ -1713,7 +1718,7 @@ class _ServiceSheetState extends State<_ServiceSheet> { child: TextField( controller: _km, keyboardType: TextInputType.number, - decoration: const InputDecoration(labelText: "Odometer (km)", border: OutlineInputBorder()), + decoration: const InputDecoration(labelText: "Odometer (km) *", border: OutlineInputBorder()), ), ), ], diff --git a/Phone App/lib/screens/car_form_sheet.dart b/Phone App/lib/screens/car_form_sheet.dart index c85487c..4e418ad 100644 --- a/Phone App/lib/screens/car_form_sheet.dart +++ b/Phone App/lib/screens/car_form_sheet.dart @@ -46,8 +46,8 @@ class _CarFormSheetState extends State { "differentialOilSpec": TextEditingController(text: car?.differentialOilSpec ?? ""), "brakeFluidSpec": TextEditingController(text: car?.brakeFluidSpec ?? ""), "coolantSpec": TextEditingController(text: car?.coolantSpec ?? ""), - "currentKm": - TextEditingController(text: (car != null && car.currentKm > 0) ? "${car.currentKm}" : ""), + // 0 is a reading, so it prints; only a car that does not exist yet is blank. + "currentKm": TextEditingController(text: car == null ? "" : "${car.currentKm}"), "serviceIntervalDays": TextEditingController(text: "${car?.serviceIntervalDays ?? 365}"), "serviceIntervalKm": TextEditingController(text: "${car?.serviceIntervalKm ?? 15000}"), "technicalCheckIntervalDays": TextEditingController( diff --git a/Phone App/lib/screens/dashboard_screen.dart b/Phone App/lib/screens/dashboard_screen.dart index af9fc71..2136a61 100644 --- a/Phone App/lib/screens/dashboard_screen.dart +++ b/Phone App/lib/screens/dashboard_screen.dart @@ -263,7 +263,8 @@ class _CarCard extends StatelessWidget { final interval = row.car.serviceIntervalKm; final nextKm = row.latest?.nextServiceKm ?? 0; final currentKm = row.car.currentKm; - if (interval <= 0 || nextKm <= 0 || currentKm <= 0) return const []; + // A new car reads 0 and belongs at 0% of its interval, not hidden entirely. + if (interval <= 0 || nextKm <= 0 || currentKm < 0) return const []; final remaining = nextKm - currentKm; final pct = (100 * (1 - remaining / interval)).clamp(0, 100).round(); final tone = status.fg(DriverVault.isDark(context)); diff --git a/Phone App/lib/screens/record_form_sheets.dart b/Phone App/lib/screens/record_form_sheets.dart index c65f165..3d276a1 100644 --- a/Phone App/lib/screens/record_form_sheets.dart +++ b/Phone App/lib/screens/record_form_sheets.dart @@ -354,7 +354,7 @@ class _FuelSheetState extends State { super.initState(); final e = widget.entry; _date = e?.date ?? DateTime.now(); - _km = TextEditingController(text: (e != null && e.km > 0) ? "${e.km}" : ""); + _km = TextEditingController(text: e == null ? "" : "${e.km}"); _liters = TextEditingController(text: (e != null && e.liters > 0) ? "${e.liters}" : ""); _cost = TextEditingController(text: (e != null && e.cost > 0) ? "${e.cost}" : ""); // A full tank is the common case and the one that makes the entry count @@ -383,7 +383,7 @@ class _FuelSheetState extends State { Future _save() async { final km = _int(_km); final liters = _num(_liters); - if (km <= 0) { + if (_km.text.trim().isEmpty || km < 0) { setState(() => _error = "Odometer is required."); return; } @@ -559,6 +559,8 @@ class _MaintenanceSheetState extends State { _type = e?.type ?? "repair"; _status = e?.status ?? "completed"; _warrantyUntil = e?.warrantyUntil; + // Maintenance is the one record whose odometer is optional, so 0 still + // reads as "not recorded" here. _km = TextEditingController(text: (e != null && e.km > 0) ? "${e.km}" : ""); _workshop = TextEditingController(text: e?.workshop ?? ""); _location = TextEditingController(text: e?.location ?? ""); @@ -946,9 +948,7 @@ class _ReminderSheetState extends State { @override Widget build(BuildContext context) { - final atKm = widget.car.currentKm > 0 - ? " The car is at ${formatKm(widget.car.currentKm)} now." - : ""; + final atKm = " The car is at ${formatKm(widget.car.currentKm)} now."; return _SheetScaffold( title: _isEdit ? "Edit reminder" : "Add reminder", error: _error, diff --git a/Web App/web/src/components/CarFormModal.vue b/Web App/web/src/components/CarFormModal.vue index e535579..adcd9ed 100644 --- a/Web App/web/src/components/CarFormModal.vue +++ b/Web App/web/src/components/CarFormModal.vue @@ -29,7 +29,7 @@ const form = ref({ serviceIntervalDays: props.car?.serviceIntervalDays || 365, serviceIntervalKm: props.car?.serviceIntervalKm || 15000, technicalCheckIntervalDays: props.car?.technicalCheckIntervalDays || 365, - currentKm: props.car?.currentKm || "", + currentKm: props.car?.currentKm ?? "", }); async function submit() { @@ -55,7 +55,7 @@ async function submit() { serviceIntervalDays: Number(form.value.serviceIntervalDays) || 365, serviceIntervalKm: Number(form.value.serviceIntervalKm) || 15000, technicalCheckIntervalDays: Number(form.value.technicalCheckIntervalDays) || 365, - currentKm: form.value.currentKm ? Number(form.value.currentKm) : 0, + currentKm: form.value.currentKm === "" ? 0 : Number(form.value.currentKm), }; const saved = isEdit ? await api.updateCar(props.car.id, payload) diff --git a/Web App/web/src/components/ChargingFormModal.vue b/Web App/web/src/components/ChargingFormModal.vue index a549597..2cb1e61 100644 --- a/Web App/web/src/components/ChargingFormModal.vue +++ b/Web App/web/src/components/ChargingFormModal.vue @@ -92,7 +92,7 @@ function payload() {
- +
diff --git a/Web App/web/src/components/FuelFormModal.vue b/Web App/web/src/components/FuelFormModal.vue index d1b09b3..b0cd415 100644 --- a/Web App/web/src/components/FuelFormModal.vue +++ b/Web App/web/src/components/FuelFormModal.vue @@ -86,7 +86,7 @@ function payload() {
- +
diff --git a/Web App/web/src/components/ReminderFormModal.vue b/Web App/web/src/components/ReminderFormModal.vue index 33f4867..533777d 100644 --- a/Web App/web/src/components/ReminderFormModal.vue +++ b/Web App/web/src/components/ReminderFormModal.vue @@ -102,7 +102,7 @@ function payload() {

{{ t("forms.reminder.triggerHint") }} - {{ t("forms.reminder.currentKm", { km: formatKm(car.currentKm) }) }} + {{ t("forms.reminder.currentKm", { km: formatKm(car.currentKm) }) }}

diff --git a/Web App/web/src/components/ServiceFormModal.vue b/Web App/web/src/components/ServiceFormModal.vue index 955fdbc..02a2be3 100644 --- a/Web App/web/src/components/ServiceFormModal.vue +++ b/Web App/web/src/components/ServiceFormModal.vue @@ -41,7 +41,9 @@ async function submit() { const payload = { car: props.carId, date: new Date(form.value.date).toISOString(), - km: form.value.km ? Number(form.value.km) : 0, + // Blank-tested, not truthiness-tested: a service logged at 0 km on a car + // collected new is a real entry, and the field is required anyway. + km: form.value.km === "" ? 0 : Number(form.value.km), changedOil: form.value.changedOil, changedEngineAirFilter: form.value.changedEngineAirFilter, changedCabinAirFilter: form.value.changedCabinAirFilter, @@ -73,7 +75,7 @@ async function submit() {
- +
diff --git a/Web App/web/src/lib/format.js b/Web App/web/src/lib/format.js index 817bd43..5e6223b 100644 --- a/Web App/web/src/lib/format.js +++ b/Web App/web/src/lib/format.js @@ -48,8 +48,12 @@ function num(value) { return Number(value).toLocaleString(prefs.locale || undefined); } +// 0 prints as "0 km", not "—": a car picked up new has an odometer reading, and +// blanking it hides the very number the first service interval counts from. +// Nothing stores a placeholder 0 for the intervals — the server defaults those +// (applyCarDefaults) — so a zero reaching here is a real reading. export function formatKm(value) { - if (value == null || value === "" || value === 0) return "—"; + if (value == null || value === "") return "—"; return num(value) + " km"; } @@ -88,9 +92,11 @@ function dateSignal(nextServiceDate) { return { key: "ok", label: t("status.okDays", { days }) }; } -// kmSignal classifies the current odometer against the next-due km. +// kmSignal classifies the current odometer against the next-due km. Both values +// are tested for absence rather than truthiness: 0 km is where a new car starts, +// and reading that as "no data" strands the km signal until the first drive. function kmSignal(currentKm, nextServiceKm) { - if (!currentKm || !nextServiceKm) return { key: "unknown", label: t("status.noKm") }; + if (currentKm == null || nextServiceKm == null) return { key: "unknown", label: t("status.noKm") }; const remaining = nextServiceKm - currentKm; if (remaining < 0) return { key: "overdue", label: t("status.serviceOverdueKm", { km: num(Math.abs(remaining)) }) }; if (remaining <= KM_SOON) return { key: "soon", label: t("status.inKm", { km: num(remaining) }) }; diff --git a/Web App/web/src/views/Dashboard.vue b/Web App/web/src/views/Dashboard.vue index a711c0e..f5a5b66 100644 --- a/Web App/web/src/views/Dashboard.vue +++ b/Web App/web/src/views/Dashboard.vue @@ -129,7 +129,8 @@ function serviceLife(car) { const interval = Number(car.serviceIntervalKm); const nextKm = Number(car.latest?.nextServiceKm); const currentKm = Number(car.currentKm); - if (!interval || !nextKm || !currentKm) return null; + // A new car reads 0 and belongs at 0% of its interval, not hidden entirely. + if (!interval || !nextKm || !Number.isFinite(currentKm)) return null; const remaining = nextKm - currentKm; const pct = Math.max(0, Math.min(100, Math.round((1 - remaining / interval) * 100))); return { pct, tone: TONE_COLOR[serviceStatus(car.latest, car).key] || TONE_COLOR.unknown };