Paddy
d418f156fa
- Date pickers (entry form + edit dialog) now display TT.MM.JJJJ instead of
ISO (DATE_DISPLAY_FORMAT). Storage stays YYYY-MM-DD; parse_date() already
accepted both formats, so existing data and the self-updater are unaffected.
- New optional "Straße" field (street + house number) in the entry form and
edit dialog, backed by a new `street` CSV column. geocode_city() and
GeocoderWorker.enqueue() gained a street parameter: when set, a full-address
query is tried first for a much more precise map point, falling back
automatically to the existing city/PLZ search if it doesn't resolve.
- Tab 2 and the entry queue show a Straße column; Tab 2 search now also
matches on street.
- Fix: pandas turns a blank CSV cell into NaN even for a dtype=str column, so
every existing (blank-street) row would have shown literal "nan" in Tab 2.
DataStore._load now does street.fillna("") after every read.
Verified with a non-GUI test suite (date parsing, query construction, CSV
round-trip incl. the NaN case) and a full GUI build/drive test on Python
3.12/Tk 9. Docs updated (changelog, overview, architecture, data-model,
dev-notes, improvements).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
240 lines
11 KiB
Markdown
240 lines
11 KiB
Markdown
# Improvement roadmap
|
||
|
||
Review of the current code, prioritized for the actual deployment: **one
|
||
non-technical user, Linux Mint laptop, offline some of the time, no one else to
|
||
fix things when they break.** That ordering puts *data safety* and *robustness*
|
||
above features.
|
||
|
||
Priorities: 🔴 do first · 🟡 worth doing · 🟢 nice to have
|
||
|
||
**Status (2026-09-07):** items 1, 2 and 3 are implemented. Items 6 and 10 are
|
||
partly done (logging added; shared autocomplete helpers extracted; map now
|
||
`fit_bounds`; region list is now a constant). See
|
||
[changelog.md](changelog.md).
|
||
|
||
---
|
||
|
||
## 1. Data safety 🔴 — ✅ done
|
||
|
||
Implemented: atomic writes (`tmp` + `fsync` + `os.replace`), rotating backups in
|
||
`data/backups/` (startup + before every change, keep 20), `EmptyDataError` /
|
||
corrupt-file fallback to an empty frame or the newest backup, and all writes
|
||
(including append) routed through `_save_csv`. A "restore from backup" button in
|
||
the UI is still open; for now it is a manual file copy.
|
||
|
||
<details><summary>Original finding</summary>
|
||
|
||
`data/orte.csv` is the only copy of the data and there is no safety net.
|
||
|
||
- **No backups.** A bad edit, a botched delete, or a disk hiccup loses history
|
||
permanently. Delete is explicitly irreversible and only guarded by a yes/no
|
||
dialog.
|
||
- **Non-atomic full rewrites.** `update_row` / `delete_rows` do
|
||
`df.to_csv(CSV_PATH)` directly. A crash or power loss mid-write can leave a
|
||
truncated file.
|
||
- **External-edit clobber.** The CSV is read once at startup. If it is opened in
|
||
a spreadsheet and changed while the app runs, the next save silently
|
||
overwrites those changes.
|
||
- **Empty-file crash.** If `orte.csv` exists but is 0 bytes (e.g. an interrupted
|
||
write), `pd.read_csv` raises `EmptyDataError` and the app won't start.
|
||
|
||
**Suggested changes**
|
||
|
||
- Write every full rewrite to a temp file in the same directory, then
|
||
`os.replace()` it into place (atomic on POSIX).
|
||
- On startup, and before each destructive write, copy the current file to
|
||
`data/backups/orte-YYYYMMDD-HHMMSS.csv`; keep the last ~20 and prune the rest.
|
||
- Add a "Backup jetzt erstellen" / "Aus Backup wiederherstellen" pair in Tab 2,
|
||
or at least open the backups folder from a menu.
|
||
- Handle `EmptyDataError` / missing columns by falling back to an empty frame
|
||
and logging a warning.
|
||
|
||
</details>
|
||
|
||
## 2. Geocoder thread robustness 🔴 — ✅ done
|
||
|
||
Implemented: every queue item is wrapped in `try/except Exception` (both the
|
||
geocode call and the callback), failures are logged and the loop continues.
|
||
`geocode_city` now raises `GeocodingUnavailable` when every attempt hit a
|
||
service/network error, and after 3 consecutive such failures the app shows a
|
||
one-time "Standortdienst nicht erreichbar" dialog. `RateLimiter` was not
|
||
adopted; the manual `sleep(1)` stays.
|
||
|
||
<details><summary>Original finding</summary>
|
||
|
||
`GeocoderWorker.run` has no error boundary around the per-item work. If
|
||
`geocode_city` or a callback raises anything other than
|
||
`GeocoderTimedOut` / `GeocoderServiceError` (a network stack error, a bug in a
|
||
callback, `GeocoderQuotaExceeded`), the **worker thread dies** and every
|
||
subsequent entry stays stuck on `⏳ wird gesucht…` with no error shown. The user
|
||
has no way to know geocoding stopped working.
|
||
|
||
**Suggested changes**
|
||
|
||
- Wrap each item's processing in `try/except Exception`, log the traceback, mark
|
||
that row as failed, and keep the loop alive.
|
||
- Consider `geopy.extra.rate_limiter.RateLimiter` (with `swallow_exceptions`)
|
||
instead of the manual `sleep(1)`.
|
||
- Surface a clear one-time status message when geocoding fails repeatedly
|
||
("Standortdienst nicht erreichbar – bitte Internetverbindung prüfen").
|
||
|
||
</details>
|
||
|
||
## 3. Easier, safer input 🔴 — ✅ done
|
||
|
||
Implemented: the date field is now `ttkbootstrap.DateEntry` (calendar picker) in
|
||
both the entry form and the edit dialog, with `parse_date()` validation on save
|
||
(a bad date is rejected with a dialog). The edit dialog gained manual
|
||
**Breitengrad / Längengrad** fields. Tab 2 shows rows without coordinates in red,
|
||
has a "Nur ohne Koordinaten" filter, a hint line with the count, and a
|
||
right-click **Koordinaten suchen** action.
|
||
|
||
<details><summary>Original finding</summary>
|
||
|
||
For the target user the free-text date field is the biggest usability risk.
|
||
|
||
- **Date field is unvalidated free text** in both the entry form and
|
||
`EditDialog`. A typo like `2026-13-05` or `05.02.2026` is stored verbatim,
|
||
breaks date sorting, and never appears correctly on the timeline.
|
||
- **No recourse when geocoding fails.** A row that comes back
|
||
`⚠ nicht gefunden` saves with blank coordinates and then just silently never
|
||
shows on the map. There is no way to type coordinates by hand or retry from
|
||
Tab 2.
|
||
|
||
**Suggested changes**
|
||
|
||
- Replace the date `Entry` with `ttkbootstrap.DateEntry` (calendar picker) in
|
||
both places. If keeping free text, validate on add/save and refuse invalid
|
||
dates with a clear message.
|
||
- In `EditDialog`, add optional `lat` / `lon` fields so a location can be fixed
|
||
manually.
|
||
- In Tab 2, highlight rows with missing coordinates (e.g. red text) and add a
|
||
filter / right-click "Koordinaten suchen" so failed geocodes are visible and
|
||
fixable after the fact.
|
||
|
||
</details>
|
||
|
||
## 4. Put it under version control 🟡 — partly done
|
||
|
||
`git init` done and `.gitignore` added (covers `.venv/`, `__pycache__/`,
|
||
`data/karte.html`, `data/backups/`, `data/app.log`). Still open: the initial
|
||
commit (left to Patrick), and the decision on committing `data/orte.csv` vs. an
|
||
example file.
|
||
- Decide on `data/orte.csv`: for a personal tool it's reasonable to commit it,
|
||
or commit a small `data/orte.example.csv` and ignore the real one.
|
||
- This `docs/` folder.
|
||
|
||
Gives you history, a rollback path, and a clean way to push updates to the
|
||
laptop (`git pull`).
|
||
|
||
## 5. Packaging & distribution 🟡 — mostly done
|
||
|
||
- ✅ `requirements.txt` fully pinned (direct + transitive), tested on 3.12.
|
||
- ✅ `run.sh` + `install.sh` (generates the `.desktop` entry) shipped in the repo.
|
||
- ✅ Bundled `icon.png`.
|
||
- ⬜ Optional: a [PyInstaller](https://pyinstaller.org/) one-file bundle would
|
||
drop the apt/venv step entirely, at the cost of building per release. Not
|
||
needed while `install.sh` works.
|
||
|
||
## 6. Diagnostics / logging 🟡 — ✅ done
|
||
|
||
`_setup_logging()` writes a `RotatingFileHandler` to `data/app.log`
|
||
(512 KB × 3). Saves, map generation, geocode failures and uncaught exceptions
|
||
are logged. `_on_open_map` now logs and narrows its handling. Open: no in-app
|
||
"open log folder" shortcut yet.
|
||
|
||
## 7. Stable row identity 🟡
|
||
|
||
Rows are keyed by the pandas index, which is renumbered after every delete. An
|
||
in-flight geocode callback for one row can land on another row if the user
|
||
deletes something in between. `_apply_edit_geocode` guards against a *deleted*
|
||
index but not a *reused* one.
|
||
|
||
**Suggested change:** add an immutable `id` column (uuid or incrementing int)
|
||
written to the CSV, and match callbacks on that instead of the positional index.
|
||
|
||
## 8. Duplicate handling 🟡
|
||
|
||
`_on_save_all` detects likely duplicates but saves them anyway with only a
|
||
transient status-bar line the user will probably miss. Consider a modal
|
||
"Diese Einträge existieren schon – trotzdem speichern?" with a per-row choice,
|
||
or at least skip exact duplicates by default.
|
||
|
||
## 9. Offline map 🟢
|
||
|
||
`karte.html` loads Leaflet/jQuery/Bootstrap from CDNs and tiles from
|
||
`server.arcgisonline.com` (Esri; `MAP_TILES` — Carto needs a key, OSM 403s
|
||
`file://`). The map is blank without internet even though all the data is local.
|
||
If offline use matters, vendor the Leaflet JS/CSS into `data/` and post-process
|
||
the Folium output to point at local files (tiles would still need caching or an
|
||
offline tile pack — larger effort).
|
||
|
||
## 10. Small code-quality items 🟢 — partly done
|
||
|
||
- ✅ Shared autocomplete helpers (`filter_locations`, `split_city_plz`)
|
||
extracted; `EditDialog` and `App` both use them.
|
||
- ✅ Region list is now the `GEOCODE_REGIONS` constant; map centre/zoom are
|
||
`MAP_DEFAULT_CENTER` / `MAP_DEFAULT_ZOOM`.
|
||
- ✅ `generate_map` now `fit_bounds` to the markers.
|
||
- ⬜ Add a `tests/` folder with pytest coverage for `DataStore` (append / update
|
||
/ delete / `find_duplicates`) and the input helpers. A non-GUI smoke script
|
||
exists (used during development) but is not in the repo — worth formalizing.
|
||
- ⬜ Add `ruff` / `mypy` config.
|
||
|
||
---
|
||
|
||
## UI/UX review (2026-09-07)
|
||
|
||
Second review, focused on the interface. Items 1–12 implemented; 13 deferred.
|
||
|
||
| # | Change | Status |
|
||
|---|--------|--------|
|
||
| 1 | Larger base font + taller Treeview rows (`UI_FONT_SIZE`) | ✅ |
|
||
| 2 | Colour-coded status bar + 8 s auto-clear to a neutral state | ✅ |
|
||
| 3 | "Warteschlange leeren" asks for confirmation; more form spacing | ✅ |
|
||
| 4 | ▲/▼ arrow on the active sort column in Tab 2 | ✅ |
|
||
| 5 | Dropped the now-redundant `(JJJJ-MM-TT)` hint | ✅ |
|
||
| 6 | Tab 2: single **"Karte"** column (`✓` / red `fehlt`) instead of raw lat/lon | ✅ |
|
||
| 7 | Explicit "Auswahl entfernen" button + <kbd>Delete</kbd> key for queue rows | ✅ |
|
||
| 8 | Duplicate save shows a modal with the list, not a transient hint | ✅ |
|
||
| 9 | Persistent entry count (window title + Tab 2 hint line) | ✅ |
|
||
| 10 | "Karte öffnen" with no coordinates shows a dialog | ✅ |
|
||
| 11 | Remember window size & position (`data/window.json`) | ✅ |
|
||
| 12 | DRK-red header bar + runtime-drawn red-cross icon (`data/icon.png`) | ✅ |
|
||
| 13 | One-click "Hinzufügen & sofort speichern" | ⬜ deferred |
|
||
|
||
Full custom DRK-red *theme* (recolouring `primary` etc.) was **not** done — the
|
||
red header bar gives the branding without fighting ttkbootstrap's theme system.
|
||
|
||
## Self-update (2026-09-07) — ✅ done
|
||
|
||
**Hilfe ▸ Nach Update suchen** / quiet check on start / restart via `os.execv`.
|
||
`git merge --ff-only` only, `pip install` if `requirements.txt` changed, `data/`
|
||
untouched. Needs a git clone + `git` installed (`Updater.available`).
|
||
Open follow-ups: no rollback if a pushed update is broken (mitigated by
|
||
`--ff-only` from a branch Patrick controls + pip errors caught pre-restart);
|
||
`run.sh` / `.desktop` still not committed (item 5).
|
||
|
||
## German date + street address (2026-09-12) — ✅ done, one idea deferred
|
||
|
||
Date pickers display `TT.MM.JJJJ`; storage untouched. New optional `street`
|
||
CSV column feeds a full-address geocode attempt before falling back to
|
||
city/PLZ. See [changelog.md](changelog.md).
|
||
|
||
**Deferred idea, not requested:** since the same person is often logged at the
|
||
same city repeatedly, the address autocomplete could remember and suggest the
|
||
last-used street for a selected city (the way selecting a city already
|
||
auto-fills its PLZ). Would remove re-typing the same street each time, at the
|
||
cost of a bit more state to reason about. Worth doing if re-typing the address
|
||
turns out to be annoying in practice — not implemented now to keep the change
|
||
minimal.
|
||
|
||
## Suggested order of remaining work
|
||
|
||
1. Deploy to the laptop: `git clone` + `./install.sh` (see [setup.md](setup.md)).
|
||
2. Stable `id` column (item 7 of the first review).
|
||
3. Formalize tests (item 10) — a non-GUI `tests/` suite already exists in
|
||
spirit (dev smoke scripts); move it into the repo.
|
||
4. Offline map (item 9) only if offline use becomes real.
|
||
5. Optional PyInstaller bundle (item 5).
|