Service history: 0 km is a reading, not a blank
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f521d2b220
commit
9abb03ee4f
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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"] = ""
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user