Files
tajniak81andClaude Opus 5 9abb03ee4f 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>
2026-08-21 13:26:56 +02:00

251 lines
7.2 KiB
Go

package api
import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"sort"
"drivervault/apiserver/internal/models"
)
// Fuel tracking: a log of refills per car, plus the efficiency derived from it.
//
// Nothing about consumption is stored — it is recomputed from the whole history
// on every read (models.ComputeFuelDerived). Editing a fill three months back
// therefore corrects every window it touches, with no rows to migrate.
// fetchFuelEntries loads a car's refills oldest-first and fills in the derived
// efficiency fields. Ordering is by odometer rather than date because the
// full-tank windows are spans of distance, and a fill logged with the wrong date
// would otherwise scramble the chain.
func (s *Server) fetchFuelEntries(r *http.Request, carID string) ([]models.FuelEntry, error) {
res, err := s.pb.List(r.Context(), colFuel, url.Values{
"filter": {fmt.Sprintf("car='%s'", carID)},
"sort": {"km"},
"perPage": {"1000"},
})
if err != nil {
return nil, err
}
var recs []fuelRecord
if err := json.Unmarshal(res.Items, &recs); err != nil {
return nil, err
}
out := make([]models.FuelEntry, 0, len(recs))
for _, rec := range recs {
out = append(out, rec.toModel())
}
// PocketBase sorts numerically here, but re-sorting locally keeps the
// invariant ComputeFuelDerived depends on explicit and cheap.
sort.SliceStable(out, func(i, j int) bool { return out[i].Km < out[j].Km })
models.ComputeFuelDerived(out)
return out, nil
}
// listCarFuelEntries serves GET /api/cars/{id}/fuel-entries, newest first.
func (s *Server) listCarFuelEntries(w http.ResponseWriter, r *http.Request) {
carID := r.PathValue("id")
if !s.requireCarAccess(w, r, carID, accessRead) {
return
}
entries, err := s.fetchFuelEntries(r, carID)
if err != nil {
writePBError(w, err)
return
}
writeJSON(w, http.StatusOK, reverseFuel(entries))
}
// listFuelEntries serves GET /api/fuel-entries?car={id}.
func (s *Server) listFuelEntries(w http.ResponseWriter, r *http.Request) {
carID := r.URL.Query().Get("car")
if !s.requireCarAccess(w, r, carID, accessRead) {
return
}
entries, err := s.fetchFuelEntries(r, carID)
if err != nil {
writePBError(w, err)
return
}
writeJSON(w, http.StatusOK, reverseFuel(entries))
}
// listCarFuelStats serves GET /api/cars/{id}/fuel-stats.
func (s *Server) listCarFuelStats(w http.ResponseWriter, r *http.Request) {
carID := r.PathValue("id")
if !s.requireCarAccess(w, r, carID, accessRead) {
return
}
entries, err := s.fetchFuelEntries(r, carID)
if err != nil {
writePBError(w, err)
return
}
writeJSON(w, http.StatusOK, models.ComputeFuelStats(entries))
}
// reverseFuel flips the oldest-first working order into the newest-first order
// clients display.
func reverseFuel(in []models.FuelEntry) []models.FuelEntry {
out := make([]models.FuelEntry, len(in))
for i, e := range in {
out[len(in)-1-i] = e
}
return out
}
// withFuelDerived recomputes the whole history and returns the one entry the
// caller just wrote, so a create/update response carries the same derived
// figures the list would show.
func (s *Server) withFuelDerived(r *http.Request, carID, id string) (models.FuelEntry, error) {
entries, err := s.fetchFuelEntries(r, carID)
if err != nil {
return models.FuelEntry{}, err
}
for _, e := range entries {
if e.ID == id {
return e, nil
}
}
return models.FuelEntry{}, fmt.Errorf("fuel entry %s not found after write", id)
}
func (s *Server) getFuelEntry(w http.ResponseWriter, r *http.Request) {
var rec fuelRecord
if err := s.pb.GetOne(r.Context(), colFuel, r.PathValue("id"), &rec); err != nil {
writePBError(w, err)
return
}
if !s.requireCarAccess(w, r, rec.Car, accessRead) {
return
}
entry, err := s.withFuelDerived(r, rec.Car, rec.ID)
if err != nil {
writePBError(w, err)
return
}
writeJSON(w, http.StatusOK, entry)
}
func (s *Server) createFuelEntry(w http.ResponseWriter, r *http.Request) {
var in models.FuelEntry
if err := decodeJSON(r, &in); err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
if err := validateFuelEntry(in); err != "" {
writeError(w, http.StatusBadRequest, err)
return
}
if !s.requireCarAccess(w, r, in.Car, accessWrite) {
return
}
var rec fuelRecord
if err := s.pb.Create(r.Context(), colFuel, fuelPayload(in), &rec); err != nil {
writePBError(w, err)
return
}
// A refill is also the freshest odometer reading there is; keeping the car in
// step means the km-based service and reminder status stay honest without the
// user retyping the number on the car itself.
s.advanceOdometer(r, in.Car, in.Km)
entry, err := s.withFuelDerived(r, rec.Car, rec.ID)
if err != nil {
writePBError(w, err)
return
}
writeJSON(w, http.StatusCreated, entry)
}
func (s *Server) updateFuelEntry(w http.ResponseWriter, r *http.Request) {
var in models.FuelEntry
if err := decodeJSON(r, &in); err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
var existing fuelRecord
if err := s.pb.GetOne(r.Context(), colFuel, r.PathValue("id"), &existing); err != nil {
writePBError(w, err)
return
}
if !s.requireCarAccess(w, r, existing.Car, accessWrite) {
return
}
// The car is fixed by the record being edited; a body claiming another car
// must not move the entry across the access boundary just checked.
in.Car = existing.Car
if err := validateFuelEntry(in); err != "" {
writeError(w, http.StatusBadRequest, err)
return
}
var rec fuelRecord
if err := s.pb.Update(r.Context(), colFuel, r.PathValue("id"), fuelPayload(in), &rec); err != nil {
writePBError(w, err)
return
}
s.advanceOdometer(r, rec.Car, in.Km)
entry, err := s.withFuelDerived(r, rec.Car, rec.ID)
if err != nil {
writePBError(w, err)
return
}
writeJSON(w, http.StatusOK, entry)
}
func (s *Server) deleteFuelEntry(w http.ResponseWriter, r *http.Request) {
var existing fuelRecord
if err := s.pb.GetOne(r.Context(), colFuel, r.PathValue("id"), &existing); err != nil {
writePBError(w, err)
return
}
if !s.requireCarAccess(w, r, existing.Car, accessWrite) {
return
}
if err := s.pb.Delete(r.Context(), colFuel, r.PathValue("id")); err != nil {
writePBError(w, err)
return
}
w.WriteHeader(http.StatusNoContent)
}
// validateFuelEntry returns a human-readable reason the entry is unusable, or ""
// when it is fine.
func validateFuelEntry(f models.FuelEntry) string {
switch {
case f.Car == "":
return "car is required"
case f.Date.IsZero():
return "date 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:
return "cost cannot be negative"
}
return ""
}
// advanceOdometer moves the car's current_km forward to km. It only ever
// increases the reading and never fails the caller's request: a stale odometer
// is a cosmetic problem, whereas rejecting a valid refill over it is not. A
// lower km means the user is backfilling older history, which must not rewind
// the car.
func (s *Server) advanceOdometer(r *http.Request, carID string, km int) {
if km <= 0 {
return
}
var car carRecord
if err := s.pb.GetOne(r.Context(), colCars, carID, &car); err != nil {
return
}
if km <= car.CurrentKm {
return
}
_ = s.pb.Update(r.Context(), colCars, carID, map[string]any{"current_km": km}, nil)
}