Changed parts: the list is the car's, not the app's

The Changed parts section offered all three parts to every car. An EV changes no
oil, and a checkbox nobody will ever tick is one more thing to read past on every
service — so which parts a car records now belongs to the car, the same way its
tabs, its Information rows and its Service history columns already do.

It works the way those three do because a fourth mechanism for the same idea
would be a fourth to keep in step: hidden_service_parts on the car, validated by
the endpoint that already does this, stored as the hidden set so a part added in
a later release is on by default, and needing write access because the choice
belongs to the car and everyone it is shared with sees it.

There is no order beside it, which is the one place this departs from the other
three. Those arrange things whose position means something — a tab bar reads left
to right, a table's columns are read across. The parts are a checkbox list inside
a single column, and moving Cabin air filter above Oil says nothing. Adding one
later is the same shape as the others if that turns out to be wrong.

A part switched off leaves the form and the history together — the chips on the
phone's cards, the web column's summary and the panel it opens. "I don't record
this" means it stops taking up room, not that it takes up room saying nothing,
which is the rule a hidden column already follows. That is the judgment call
here: a car with five years of oil changes hides them all by switching the part
off. Nothing is written to the records, so switching it back on brings every one
of those chips back, which is what makes the call safe to reverse.

The part that would have been a silent data bug: the API rewrites all three
booleans from the body of a service update, so a form that simply stopped
sending a hidden part would set it false on the next edit of any old record.
Both forms therefore keep every part in their state and submit every one — only
the checkboxes are filtered. The mirror of that is a *new* record, where a hidden
part starts false rather than at its `initial`, since ticking a box nobody was
shown is not a default, it's a guess. Oil is the only part with initial: true, so
that case is live the moment anyone hides it.

Verified: go vet and go test ./... pass, with a new test covering that every part
is hideable (unlike the tabs and the columns — a service that changed nothing is
a real service), that the "parts" column key is refused as a part key and a part
key as a column key, and that no part is also a column. flutter analyze is clean
and flutter test passes 32 to 35, the new ones covering visibleParts, that a
hidden part's chips go while its stored boolean stays, and the picker's fourth
section. npm run build is clean.

Both apps were driven against throwaway stub APIs. Web: the picker saved
{"hiddenServiceParts":["oil"]}, the table's parts cell went from "Oil & Oil
filter +2" to "Engine air filter, Cabin air filter", the record whose only part
was oil went to an empty cell, the panel dropped to two rows, the add form
offered two unticked boxes where oil's initial: true would have ticked one, and
editing the three-part record sent changedOil:true back with a box that was never
on screen. Phone: the same car rendered chips "Engine air, Cabin air", "Changed
parts —" for the oil-only record, and an add sheet with exactly two unticked
boxes.

Not verified: no automated test guards the web behaviour — the web app still has
no test runner, so the above was read out of the live DOM and the outgoing
request bodies by hand. The phone's picker was checked by widget test and by
rendering, but its Save was not driven end to end. Neither app was run against
the real API Server: bootstrap appends the new field on the next start, and until
that start a client sending hiddenServiceParts takes a 400 — they deploy together
from this repo, but the server must go first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
tajniak81andClaude Opus 5 committed 2026-08-22 23:30:08 +02:00
1 parent 7718b32013
commit 35e6c511b7
24 files changed
+356 -50

No files matched your search

+9 -4
View File
@@ -27,9 +27,11 @@ navigation bar** — Garage, Charging, Settings, and Users for admins — in an
**Which tabs a car shows, and in what order, belongs to the car** — the same
arrangement the web app reads, so everyone it is shared with sees the same
page. Edit it under the **tune** icon: switch tabs, Information rows and
Service history columns on or off, and drag any of the three lists by its
handle to reorder it. (The web rearranges by dragging the tab bar and the
page. Edit it under the **tune** icon: switch tabs, Information rows, Service
history columns and the parts a service can change on or off, and drag any of
the first three lists by its handle to reorder it. (The parts have no order —
they are a checkbox list inside one column, and their position says nothing a
tab's or a column's does.) (The web rearranges by dragging the tab bar and the
column headings themselves; on a touch screen those gestures belong to the tab
bar and the scroll, so every arrangement is made in the picker instead.) Two
things cannot be switched off: Information — a page with no tabs left would be
@@ -57,7 +59,10 @@ navigation bar** — Garage, Charging, Settings, and Users for admins — in an
have changed shares the one **parts** column — they are a growing list, and a
column apiece would widen the web's table without end — and both that column
and the form's Changed parts section come from `lib/service_parts.dart`, the
twin of the web's `lib/serviceParts.js`.
twin of the web's `lib/serviceParts.js`. Which of them this car records is
the car's own choice as well: a part switched off leaves the form's
checkboxes and the cards' chips together, and nothing is written to the
records, so switching it back on brings the old chips back.
- **Technical check history** — the mandatory roadworthiness inspections
(przegląd techniczny, MOT, TÜV). Result, cost, station, and the certificate's
valid-until, which overrides the car's interval when present. A failed check
+2
View File
@@ -377,6 +377,8 @@
"fieldsOrderHint": "Træk felterne på fanen Oplysninger for at ændre deres rækkefølge.",
"serviceColumnsHeading": "Kolonner i servicehistorik",
"columnAlwaysOn": "{column} vises altid.",
"servicePartsHeading": "Udskiftede dele",
"servicePartsHint": "En fravalgt del forsvinder fra serviceformularen og fra historikken. Det, der allerede er registreret, bevares og kommer tilbage med den.",
"reorderHint": "Træk en række i håndtaget for at ændre rækkefølgen.",
"tabsReorder": "Faneblade",
"fieldsReorder": "Informationsfelter"
+2
View File
@@ -377,6 +377,8 @@
"fieldsOrderHint": "Drag the fields on the Information tab to change the order they appear in.",
"serviceColumnsHeading": "Service history columns",
"columnAlwaysOn": "{column} is always shown.",
"servicePartsHeading": "Changed parts",
"servicePartsHint": "A part switched off comes off the service form and out of the history. What is already recorded is kept, and comes back with it.",
"reorderHint": "Drag a row by its handle to change the order.",
"tabsReorder": "Tabs",
"fieldsReorder": "Information fields"
+2
View File
@@ -381,6 +381,8 @@
"fieldsOrderHint": "Przeciągnij pola na zakładce Informacje, aby zmienić ich kolejność.",
"serviceColumnsHeading": "Kolumny historii serwisowej",
"columnAlwaysOn": "Kolumna {column} jest zawsze widoczna.",
"servicePartsHeading": "Wymienione części",
"servicePartsHint": "Wyłączona część znika z formularza serwisu i z historii. Zapisane dane pozostają i wracają razem z nią.",
"reorderHint": "Przeciągnij wiersz za uchwyt, aby zmienić kolejność.",
"tabsReorder": "Zakładki",
"fieldsReorder": "Pola informacyjne"
+2
View File
@@ -146,6 +146,7 @@ class ApiClient {
List<String>? metricOrder,
List<String>? hiddenServiceColumns,
List<String>? serviceColumnOrder,
List<String>? hiddenServiceParts,
}) async {
final body = <String, dynamic>{
if (hiddenTabs != null) "hiddenTabs": hiddenTabs,
@@ -155,6 +156,7 @@ class ApiClient {
if (metricOrder != null) "metricOrder": metricOrder,
if (hiddenServiceColumns != null) "hiddenServiceColumns": hiddenServiceColumns,
if (serviceColumnOrder != null) "serviceColumnOrder": serviceColumnOrder,
if (hiddenServiceParts != null) "hiddenServiceParts": hiddenServiceParts,
};
final data = await _send("PUT", "/cars/$id/view", body: body);
return Car.fromJson(Map<String, dynamic>.from(data));
+8
View File
@@ -107,6 +107,12 @@ class Car {
final List<String> hiddenServiceColumns;
final List<String> serviceColumnOrder;
/// The parts this car's services never change, as part keys. They come off
/// the service form and out of the history's chips together — a part nobody
/// records is one nobody wants offered. No order beside it: the parts are a
/// checkbox list inside one column, and their position says nothing.
final List<String> hiddenServiceParts;
Car({
required this.id,
required this.name,
@@ -138,6 +144,7 @@ class Car {
this.metricOrder = const [],
this.hiddenServiceColumns = const [],
this.serviceColumnOrder = const [],
this.hiddenServiceParts = const [],
});
factory Car.fromJson(Map<String, dynamic> j) => Car(
@@ -171,6 +178,7 @@ class Car {
metricOrder: _asStrList(j["metricOrder"]),
hiddenServiceColumns: _asStrList(j["hiddenServiceColumns"]),
serviceColumnOrder: _asStrList(j["serviceColumnOrder"]),
hiddenServiceParts: _asStrList(j["hiddenServiceParts"]),
);
bool get isOwner => access == "owner";
+21 -4
View File
@@ -470,6 +470,7 @@ class _CarDetailScreenState extends State<CarDetailScreen> {
final columns = arrangeKeys(kServiceColumnKeys, car.serviceColumnOrder)
.where((key) => !car.hiddenServiceColumns.contains(key))
.toList();
final parts = visibleParts(car);
return _TabList(
empty: data.services.isEmpty ? t("car.services.empty") : null,
onAdd: car.canWrite ? () => _addService(car) : null,
@@ -478,6 +479,7 @@ class _CarDetailScreenState extends State<CarDetailScreen> {
.map((s) => _ServiceTile(
record: s,
columns: columns,
parts: parts,
onEdit: car.canWrite ? () => _editService(car, s) : null,
onDelete: car.canWrite ? () => _deleteService(s) : null,
))
@@ -825,11 +827,16 @@ class _ServiceTile extends StatelessWidget {
/// The visible column keys, already arranged. Never empty: date cannot be
/// hidden.
final List<String> columns;
/// The parts this car still records. Passed in rather than read off the car
/// for the same reason the columns are: every row shows the same ones.
final List<ServicePart> parts;
final VoidCallback? onEdit;
final VoidCallback? onDelete;
const _ServiceTile({
required this.record,
required this.columns,
required this.parts,
this.onEdit,
this.onDelete,
});
@@ -912,7 +919,7 @@ class _ServiceTile extends StatelessWidget {
/// them in one table cell and so names the first and counts the rest behind a
/// panel; a card has the width to simply show them all.
Widget _parts(BuildContext context) {
final changed = changedParts(record);
final changed = changedParts(record, parts);
if (changed.isEmpty) return _empty(context, "parts");
return Wrap(
spacing: 6,
@@ -1976,7 +1983,14 @@ class _ServiceSheetState extends State<_ServiceSheet> {
/// Which parts this record says were changed, keyed the way [kServiceParts]
/// keys them, so a part added to that list turns up in this sheet without a
/// second edit here.
///
/// Every part, not only the shown ones: an edit has to send back what a part
/// this car has switched off already said, because the API rewrites all of the
/// booleans from the body and an omitted one would come back false.
late final Map<String, bool> _changed;
/// The parts this car still records, and so the only ones with a checkbox.
late final List<ServicePart> _parts;
final _pending = PendingAttachment();
bool _saving = false;
String? _error;
@@ -1990,11 +2004,14 @@ class _ServiceSheetState extends State<_ServiceSheet> {
_date = r?.date ?? DateTime.now();
_km = TextEditingController(text: r == null ? "" : "${r.km}");
_notes = TextEditingController(text: r?.notes ?? "");
_parts = visibleParts(widget.car);
// An existing record is read through the part's own accessor; a new one
// starts from the part's default, which is why an oil change comes ticked.
// starts from the part's default, which is why an oil change comes ticked —
// unless this car has switched that part off, since ticking a box nobody was
// shown is not a default, it's a guess.
_changed = {
for (final part in kServiceParts)
part.key: r == null ? part.initial : part.changed(r),
part.key: r == null ? part.initial && _parts.contains(part) : part.changed(r),
};
}
@@ -2092,7 +2109,7 @@ class _ServiceSheetState extends State<_ServiceSheet> {
],
),
const SizedBox(height: 8),
for (final part in kServiceParts)
for (final part in _parts)
CheckboxListTile(
value: _changed[part.key] ?? false,
onChanged: (v) => setState(() => _changed[part.key] = v ?? false),
+36
View File
@@ -3,6 +3,7 @@ import "package:flutter/material.dart";
import "../i18n.dart";
import "../main.dart";
import "../models.dart";
import "../service_parts.dart";
import "../theme.dart";
/// Every tab a car's page can show, in the order they appear when the car has no
@@ -179,6 +180,7 @@ class _CarViewSheetState extends State<CarViewSheet> {
late Set<String> _visibleTabs;
late Set<String> _visibleFields;
late Set<String> _visibleColumns;
late Set<String> _visibleServiceParts;
bool _saving = false;
String? _error;
@@ -194,6 +196,7 @@ class _CarViewSheetState extends State<CarViewSheet> {
_visibleColumns = kHideableServiceColumnKeys
.where((k) => !car.hiddenServiceColumns.contains(k))
.toSet();
_visibleServiceParts = visibleParts(car).map((p) => p.key).toSet();
}
/// Whether [key] is one of the rows on offer below: Information never is
@@ -253,6 +256,10 @@ class _CarViewSheetState extends State<CarViewSheet> {
tabOrder: _tabOrder,
fieldOrder: _fieldOrder,
serviceColumnOrder: _columnOrder,
hiddenServiceParts: kServiceParts
.where((p) => !_visibleServiceParts.contains(p.key))
.map((p) => p.key)
.toList(),
);
if (mounted) Navigator.pop(context, car);
} catch (e) {
@@ -381,6 +388,35 @@ class _CarViewSheetState extends State<CarViewSheet> {
style: TextStyle(fontSize: 11, color: DriverVault.muted(context)),
),
),
const SizedBox(height: 20),
_heading(context, t("car.viewPicker.servicePartsHeading")),
// No drag handle here, and no stored order: the parts are a
// checkbox list inside one column, so their position carries
// nothing a tab's or a column's does.
for (final part in kServiceParts)
Padding(
padding: const EdgeInsets.symmetric(vertical: 2),
child: Row(
children: [
Checkbox(
value: _visibleServiceParts.contains(part.key),
onChanged: (v) => setState(() => (v ?? false)
? _visibleServiceParts.add(part.key)
: _visibleServiceParts.remove(part.key)),
),
Expanded(
child: Text(t(part.label), style: const TextStyle(fontSize: 14)),
),
],
),
),
Padding(
padding: const EdgeInsets.only(top: 6),
child: Text(
t("car.viewPicker.servicePartsHint"),
style: TextStyle(fontSize: 11, color: DriverVault.muted(context)),
),
),
if (_error != null)
Padding(
padding: const EdgeInsets.only(top: 12),
+19 -6
View File
@@ -76,9 +76,22 @@ final List<ServicePart> kServiceParts = [
),
];
/// The parts this record says were changed. A record written before a part
/// existed simply doesn't carry its field, which the model reads as false —
/// "not changed" rather than "unknown", because that service genuinely didn't
/// change it.
List<ServicePart> changedParts(ServiceRecord service) =>
kServiceParts.where((part) => part.changed(service)).toList();
/// The parts a car actually records, which is every one it hasn't switched off.
/// An EV changes no oil, and a checkbox nobody will ever tick is one more thing
/// to read past on every service. The hidden set rather than the visible one, so
/// a part added in a later release is on by default.
List<ServicePart> visibleParts(Car car) =>
kServiceParts.where((part) => !car.hiddenServiceParts.contains(part.key)).toList();
/// The parts this record says were changed, among the ones [parts] still shows.
/// A record written before a part existed simply doesn't carry its field, which
/// the model reads as false — "not changed" rather than "unknown", because that
/// service genuinely didn't change it.
///
/// A part switched off disappears from the history as well as from the form, the
/// same way a hidden column does: what "I don't record this" means is that it
/// stops taking up room, not that it takes up room saying nothing. Its stored
/// boolean is left alone, so switching it back on brings the old records' chips
/// back with it.
List<ServicePart> changedParts(ServiceRecord service, List<ServicePart> parts) =>
parts.where((part) => part.changed(service)).toList();
+51 -9
View File
@@ -12,6 +12,7 @@ import "package:drivervault_phone/i18n.dart";
import "package:drivervault_phone/main.dart";
import "package:drivervault_phone/models.dart";
import "package:drivervault_phone/screens/car_view_sheet.dart";
import "package:drivervault_phone/service_parts.dart";
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
@@ -83,14 +84,37 @@ void main() {
await tester.pumpAndSettle();
}
Car car({List<String> hiddenColumns = const [], List<String> columnOrder = const []}) =>
Car car({
List<String> hiddenColumns = const [],
List<String> columnOrder = const [],
List<String> hiddenParts = const [],
}) =>
Car.fromJson({
"id": "car1",
"name": "bZ4X",
"hiddenServiceColumns": hiddenColumns,
"serviceColumnOrder": columnOrder,
"hiddenServiceParts": hiddenParts,
});
// The sheet stacks its sections in one list: tabs, Information rows, the
// Service history columns, then the parts. Counted from the end rather than
// the start so the tab rows — whose number depends on showProvider — don't
// have to be worked out here.
List<bool> ticks(WidgetTester tester) => tester
.widgetList<Checkbox>(find.byType(Checkbox))
.map((b) => b.value ?? false)
.toList();
List<bool> columnTicks(WidgetTester tester) {
final all = ticks(tester);
final end = all.length - kServiceParts.length;
return all.sublist(end - kServiceColumnKeys.length, end);
}
List<bool> partTicks(WidgetTester tester) {
final all = ticks(tester);
return all.sublist(all.length - kServiceParts.length);
}
testWidgets("offers every column, with the date locked on", (tester) async {
await pump(tester, car());
@@ -114,15 +138,33 @@ void main() {
testWidgets("a switched-off column comes back unticked", (tester) async {
await pump(tester, car(hiddenColumns: ["parts", "file"]));
// Read the ticks in the order the rows are laid out — the sheet lists the
// tabs and the Information rows above the columns, so the columns are the
// last seven.
final ticks = tester
expect(columnTicks(tester), [true, true, true, true, false, true, false]);
});
testWidgets("offers every part, none of them locked", (tester) async {
await pump(tester, car());
expect(find.text(t("car.viewPicker.servicePartsHeading").toUpperCase()),
findsOneWidget);
for (final part in kServiceParts) {
expect(find.text(t(part.label)), findsWidgets,
reason: "the ${part.key} part is not on offer");
}
expect(partTicks(tester), List.filled(kServiceParts.length, true));
// Only the date is locked. Every part can be switched off: a service that
// changed nothing is a real service, so the form needs none of them.
final locked = tester
.widgetList<Checkbox>(find.byType(Checkbox))
.map((b) => b.value ?? false)
.toList();
final columns = ticks.sublist(ticks.length - kServiceColumnKeys.length);
expect(columns, [true, true, true, true, false, true, false]);
.where((b) => b.onChanged == null);
expect(locked, hasLength(1));
});
testWidgets("a switched-off part comes back unticked", (tester) async {
await pump(tester, car(hiddenParts: ["oil", "cabinFilter"]));
expect(partTicks(tester), [false, true, false]);
// And it changes nothing about the columns, which are their own set.
expect(columnTicks(tester), List.filled(kServiceColumnKeys.length, true));
});
testWidgets("the rows follow the car's arrangement", (tester) async {
+26 -4
View File
@@ -299,18 +299,40 @@ void main() {
"changedCabinAirFilter": cabin,
});
expect(changedParts(record()).isEmpty, isTrue);
expect(changedParts(record(oil: true)).map((p) => p.key), ["oil"]);
final all = kServiceParts;
expect(changedParts(record(), all).isEmpty, isTrue);
expect(changedParts(record(oil: true), all).map((p) => p.key), ["oil"]);
// Catalogue order, not the order the fields happen to be read in.
expect(
changedParts(record(cabin: true, oil: true)).map((p) => p.key),
changedParts(record(cabin: true, oil: true), all).map((p) => p.key),
["oil", "cabinFilter"],
);
// A record written before a part existed carries no field for it, which
// reads as "not changed" rather than as a missing value.
final old = ServiceRecord.fromJson({"id": "s", "car": "c", "km": 0});
expect(changedParts(old).isEmpty, isTrue);
expect(changedParts(old, all).isEmpty, isTrue);
});
test("a part a car has switched off leaves its records alone", () {
final car = Car.fromJson({"id": "c", "name": "bZ4X", "hiddenServiceParts": ["oil"]});
expect(car.hiddenServiceParts, ["oil"]);
expect(visibleParts(car).map((p) => p.key), ["engineFilter", "cabinFilter"]);
// A car nobody has configured offers every part — the hidden set, so one
// added in a later release is on by default.
expect(visibleParts(Car.fromJson({"id": "d", "name": "Yaris"})).length, kServiceParts.length);
// The chips follow the car, so an oil change recorded before the part was
// switched off stops being shown...
final oiled = ServiceRecord.fromJson({
"id": "s", "car": "c", "km": 90000,
"changedOil": true, "changedCabinAirFilter": true,
});
expect(changedParts(oiled, visibleParts(car)).map((p) => p.key), ["cabinFilter"]);
// ...but the record still says so, which is what brings the chip back if the
// part is switched on again. Nothing here writes to the record.
expect(oiled.changedOil, isTrue);
});
test("arrangeKeys: partial orders keep every key, unknown ones are dropped", () {