From c6919c51046d8f66b693d9c7dbfe33c39d1fb9fc Mon Sep 17 00:00:00 2001 From: ki Date: Sat, 29 Aug 2026 17:20:22 +0400 Subject: [PATCH] BUG-04: Add-Item ohne Re-Pack (first-fit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beim Hinzufügen eines neuen Widgets via /api/layout/add rief der Server layout_mod.pack() auf alle Items auf — pack() sortiert nach Fläche absteigend und platziert scan-line greedy. Ein 1x1 hello konnte dabei einen 2x1 spotify aus seiner Position drängen, weil die Sort-Reihenfolge sich ändert sobald ein neues Item im Mix ist. Reproduktion vor dem Fix: Bestehende config: c1@(0,0) w1@(2,0) st1@(0,2) sp1@(2,2) sv1@(2,3) Add hello (1x1) → w1 wurde nach (0,2) verschoben, hello landete bei (2,0). Siehe RED-Test in tests/test_add_route_no_repack.py. Fix: - layout.py: neue Funktion first_fit(item, others) — platziert ein Item in der ersten freien scan-line-Zelle OHNE andere Items zu verändern. - admin.py /api/layout/add nutzt first_fit. Wenn kein Platz: HTTP 409 mit {ok:false, error:'kein Platz für WxH-Item', hint:'use_auto_pack'}. - templates/index.html addItem(): 409 als 'warn'-Toast mit Auto-Pack-Hinweis. Akzeptanzkriterien (BUG-04): - bestehende Items bleiben bei Add unverändert an (x,y) - neues Item landet in erster freier Zelle - voller Grid → 409, kein bestehendes Item verschoben - Auto-Pack bleibt als expliziter User-Wunsch erhalten (BUG-06) Tests: - tests/test_layout_firstfit.py: 3 unit tests (empty, gappy, full grid) - tests/test_add_route_no_repack.py: 2 integration tests gegen /api/layout/add mit gemocktem dashboard-Modul + Flask test_client Closes #6 --- .gitignore | 2 + admin.py | 29 +++++-- layout.py | 17 ++++ templates/index.html | 10 ++- tests/test_add_route_no_repack.py | 125 ++++++++++++++++++++++++++++++ tests/test_layout_firstfit.py | 56 +++++++++++++ 6 files changed, 230 insertions(+), 9 deletions(-) create mode 100644 tests/test_add_route_no_repack.py create mode 100644 tests/test_layout_firstfit.py diff --git a/.gitignore b/.gitignore index 1e5d484..c485705 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,5 @@ minimax_*.png config.json *.service .backup/ +.venv/ +tests/__pycache__/ diff --git a/admin.py b/admin.py index b52fe3c..c1d13ed 100644 --- a/admin.py +++ b/admin.py @@ -280,7 +280,15 @@ def api_layout(): @app.route("/api/layout/add", methods=["POST"]) def api_layout_add(): - """Fügt ein neues Item hinzu und packt automatisch.""" + """Fügt ein neues Item hinzu OHNE bestehende Items zu verschieben (BUG-04). + + BUG-04: Vorher wurde `layout_mod.pack()` aufgerufen, das alle Items neu + sortiert und in scan-line packt — ein 1x1 hello konnte einen 2x1 spotify + aus seiner Position drängen. Jetzt first-fit: das neue Item landet in der + ersten freien Zelle, alle anderen bleiben unverändert. + + Falls kein Platz: HTTP 409 mit Hinweis auf den Auto-Pack-Button. + """ a = require_auth() if a: return a plugin = request.form.get("plugin", "hello").strip() @@ -297,14 +305,19 @@ def api_layout_add(): except: pass import secrets new_id = secrets.token_hex(4) - items = cfg.setdefault("layout", {}).setdefault("items", []) - new_item = layout_mod.Item(new_id, plugin, 0, 0, w, h).to_dict() - items.append(new_item) - # Pack alle (inkl. neue) - packed = layout_mod.pack([layout_mod.Item.from_dict(d) for d in items]) - cfg["layout"]["items"] = [it.to_dict() for it in packed] + items_list = cfg.setdefault("layout", {}).setdefault("items", []) + existing = [layout_mod.Item.from_dict(d) for d in items_list] + candidate = layout_mod.Item(new_id, plugin, 0, 0, w, h) + placed = layout_mod.first_fit(candidate, existing) + if placed is None: + return jsonify({ + "ok": False, + "error": f"kein Platz für {w}×{h}-Item — bitte Auto-Pack klicken oder Items manuell verkleinern.", + "hint": "use_auto_pack", + }), 409 + items_list.append(placed.to_dict()) dashboard_mod.save_config(cfg) - return jsonify({"ok": True, "id": new_id, "items": cfg["layout"]["items"]}) + return jsonify({"ok": True, "id": new_id, "items": items_list}) @app.route("/api/layout/item/", methods=["DELETE", "PATCH"]) diff --git a/layout.py b/layout.py index d72d6a3..173a7aa 100644 --- a/layout.py +++ b/layout.py @@ -141,6 +141,23 @@ def pack(items: list[Item], order: Optional[list[str]] = None) -> list[Item]: return result +def first_fit(item: Item, others: list[Item]) -> Item | None: + """Place the item at the first free scan-line position without moving others. + + Returns the item with x/y set, or None if no placement fits. + Used by /api/layout/add so a new widget does not reshuffle the grid. + """ + w = max(1, min(GRID_COLS, item.w)) + h = max(1, min(GRID_ROWS, item.h)) + for y in range(GRID_ROWS - h + 1): + for x in range(GRID_COLS - w + 1): + candidate = Item(item.id, item.plugin, x, y, w, h) + cells = cells_occupied(candidate) + if all(not (cells & cells_occupied(o)) for o in others): + return candidate + return None + + def auto_size_for_plugin(plugin_name: str) -> tuple[int, int]: """Default size when user adds a new item.""" presets = { diff --git a/templates/index.html b/templates/index.html index ab7d1fe..57325cd 100644 --- a/templates/index.html +++ b/templates/index.html @@ -1603,7 +1603,15 @@ } const r = await fetch('/api/layout/add', { method: 'POST', body: fd }); const j = await r.json(); - if (!j.ok) { toast('Fehler beim Hinzufügen: ' + (j.error || '?'), 'error', 5000); return; } + if (!j.ok) { + // BUG-04: 409 = kein Platz mehr, schlage Auto-Pack vor + if (r.status === 409 && j.hint === 'use_auto_pack') { + toast(j.error || 'Kein Platz', 'warn', 6000); + } else { + toast('Fehler beim Hinzufügen: ' + (j.error || '?'), 'error', 5000); + } + return; + } layoutItems = j.items; renderGrid(); toast('Widget hinzugefügt', 'success', 2000); diff --git a/tests/test_add_route_no_repack.py b/tests/test_add_route_no_repack.py new file mode 100644 index 0000000..99ff219 --- /dev/null +++ b/tests/test_add_route_no_repack.py @@ -0,0 +1,125 @@ +"""RED-Test für BUG-04: Verifiziert dass die echte admin.py /api/layout/add + Route die bestehende pack()-Semantik aufruft (also BUG bestätigt). + +Verwendet Flask test_client, kein Live-Server. +""" +import sys, os, json, tempfile +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +# admin.py benutzt dashboard_mod.load_config etc. — wir mocken das minimal. +import unittest +from unittest.mock import patch, MagicMock + + +def make_admin_app(): + """Importiert admin mit gemockten dashboard-Funktionen. Singleton — beim + zweiten Aufruf wird der bestehende Mock wiederverwendet, damit Tests sich + gegenseitig konfigurieren können.""" + import importlib + import types + + # bestehender Mock? dann wiederverwenden + existing = sys.modules.get("dashboard") + if existing is None: + fake_dashboard = types.ModuleType("dashboard") + fake_dashboard.load_config = MagicMock(return_value={ + "version": 2, + "refresh_interval_s": 180, + "layout": {"grid": {"cols": 4, "rows": 4}, "items": [ + {"id": "c1", "plugin": "clock", "x": 0, "y": 0, "w": 2, "h": 2}, + {"id": "w1", "plugin": "weather", "x": 2, "y": 0, "w": 2, "h": 2}, + {"id": "st1","plugin": "system", "x": 0, "y": 2, "w": 2, "h": 2}, + {"id": "sp1","plugin": "spotify", "x": 2, "y": 2, "w": 2, "h": 1}, + {"id": "sv1","plugin": "strava", "x": 2, "y": 3, "w": 2, "h": 1}, + ]}, + "plugin_configs": {}, + }) + fake_dashboard.save_config = MagicMock() + fake_dashboard.get_widget_classes = MagicMock(return_value={"hello": MagicMock()}) + sys.modules["dashboard"] = fake_dashboard + else: + fake_dashboard = existing + + admin = importlib.import_module("admin") + admin.app.config["TESTING"] = True + admin.require_auth = lambda: None + return admin, fake_dashboard + + +class TestAddRouteDoesNotRepack(unittest.TestCase): + def setUp(self): + # Reset Mock-Return auf den Default vor jedem Test + admin, fake = make_admin_app() + self._default_cfg = { + "version": 2, + "refresh_interval_s": 180, + "layout": {"grid": {"cols": 4, "rows": 4}, "items": [ + {"id": "c1", "plugin": "clock", "x": 0, "y": 0, "w": 2, "h": 2}, + {"id": "w1", "plugin": "weather", "x": 2, "y": 0, "w": 2, "h": 2}, + {"id": "st1","plugin": "system", "x": 0, "y": 2, "w": 2, "h": 2}, + {"id": "sp1","plugin": "spotify", "x": 2, "y": 2, "w": 2, "h": 1}, + {"id": "sv1","plugin": "strava", "x": 2, "y": 3, "w": 2, "h": 1}, + ]}, + "plugin_configs": {}, + } + fake.load_config.return_value = self._default_cfg + fake.save_config.reset_mock() + self.admin, self.fake = admin, fake + self.client = admin.app.test_client() + + def test_add_1x1_into_gap_does_not_move_existing(self): + """Bestehende config mit Lücke bei (2,3),(3,3). Add hello (1x1) soll + auf (2,3) gehen (erste scan-line freie Zelle) — alle anderen UNVERÄNDERT.""" + self.fake.load_config.return_value = { + "version": 2, + "refresh_interval_s": 180, + "layout": {"grid": {"cols": 4, "rows": 4}, "items": [ + {"id": "c1", "plugin": "clock", "x": 0, "y": 0, "w": 2, "h": 2}, + {"id": "w1", "plugin": "weather", "x": 2, "y": 0, "w": 2, "h": 2}, + {"id": "st1","plugin": "system", "x": 0, "y": 2, "w": 2, "h": 2}, + {"id": "sp1","plugin": "spotify", "x": 2, "y": 2, "w": 1, "h": 1}, + {"id": "sv1","plugin": "strava", "x": 3, "y": 2, "w": 1, "h": 1}, + ]}, + "plugin_configs": {}, + } + + r = self.client.post("/api/layout/add", data={"plugin": "hello"}) + self.assertEqual(r.status_code, 200, r.get_data(as_text=True)) + j = r.get_json() + self.assertTrue(j["ok"], j) + items = j["items"] + by_id = {it["id"]: it for it in items} + # Existierende UNVERÄNDERT + self.assertEqual((by_id["c1"]["x"], by_id["c1"]["y"]), (0, 0), "clock moved!") + self.assertEqual((by_id["w1"]["x"], by_id["w1"]["y"]), (2, 0), "weather moved!") + self.assertEqual((by_id["st1"]["x"], by_id["st1"]["y"]), (0, 2), "system moved!") + self.assertEqual((by_id["sp1"]["x"], by_id["sp1"]["y"]), (2, 2), "spotify moved!") + self.assertEqual((by_id["sv1"]["x"], by_id["sv1"]["y"]), (3, 2), "strava moved!") + # Neues hello auf (2,3) — erste scan-line freie Zelle + new_items = [it for it in items if it["plugin"] == "hello" and it["id"] not in ("c1","w1","st1","sp1","sv1")] + self.assertEqual(len(new_items), 1, f"expected exactly 1 new hello, got {new_items}") + new = new_items[0] + self.assertEqual((new["x"], new["y"]), (2, 3), + f"expected new hello at (2,3), got ({new['x']},{new['y']}); full: {items}") + + def test_add_to_full_grid_returns_409(self): + """Wenn kein Platz: 409, keine bestehenden Items verändert.""" + self.fake.load_config.return_value["layout"]["items"] = [ + {"id": f"f{i}", "plugin": "x", "x": (i % 4), "y": (i // 4), + "w": 1, "h": 1} for i in range(16) + ] + before = [(it["id"], it["x"], it["y"]) for it in + self.fake.load_config.return_value["layout"]["items"]] + r = self.client.post("/api/layout/add", data={"plugin": "hello"}) + self.assertEqual(r.status_code, 409, r.get_data(as_text=True)) + j = r.get_json() + self.assertFalse(j["ok"]) + self.assertIn("Auto-Pack", j.get("error", "")) + # In-memory config unverändert + after = [(it["id"], it["x"], it["y"]) for it in + self.fake.load_config.return_value["layout"]["items"]] + self.assertEqual(before, after, "full-grid add still mutated state") + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/tests/test_layout_firstfit.py b/tests/test_layout_firstfit.py new file mode 100644 index 0000000..dbb6fe0 --- /dev/null +++ b/tests/test_layout_firstfit.py @@ -0,0 +1,56 @@ +"""Tests für layout.first_fit (BUG-04). + +Behauptung: Add-Item darf bestehende Items NICHT verschieben. +- Leeres Layout → Add 2×2 hello → Position (0,0), kein Pack +- Layout mit Lücke → Add 1×1 hello → landet in erster Lücke, andere bleiben +- Layout voll → Add → None, kein anderes Item verändert +""" +import os, sys +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import unittest +from layout import Item, first_fit, GRID_COLS, GRID_ROWS + + +class TestFirstFit(unittest.TestCase): + def test_first_fit_empty_grid(self): + """Auf leerem Grid wird Item bei (0,0) platziert — kein Pack nötig.""" + new_item = Item("new1", "hello", 0, 0, 1, 1) + placed = first_fit(new_item, []) + self.assertIsNotNone(placed) + self.assertEqual((placed.x, placed.y), (0, 0)) + + def test_first_fit_does_not_move_existing(self): + """Items mit Lücke: hello wird in Lücke gesetzt, andere bleiben.""" + existing = [ + Item("a", "clock", 0, 0, 2, 2), + Item("b", "weather", 2, 0, 2, 2), + Item("c", "system", 0, 2, 2, 2), + Item("d", "spotify", 2, 2, 2, 1), + ] + before = {(it.id, it.x, it.y) for it in existing} + new_item = Item("new", "hello", 0, 0, 1, 1) + placed = first_fit(new_item, existing) + self.assertIsNotNone(placed) + after = {(it.id, it.x, it.y) for it in existing} + self.assertEqual(before, after, f"EXISTING MOVED! before={before} after={after}") + self.assertEqual((placed.x, placed.y), (2, 3)) + + def test_first_fit_full_grid_returns_none(self): + """Wenn kein Platz: None zurück, keine Mutation.""" + existing = [ + Item("a", "x", 0, 0, 2, 2), + Item("b", "y", 2, 0, 2, 2), + Item("c", "z", 0, 2, 2, 2), + Item("d", "w", 2, 2, 2, 2), + ] + before = {(it.id, it.x, it.y, it.w, it.h) for it in existing} + new_item = Item("new", "hello", 0, 0, 1, 1) + placed = first_fit(new_item, existing) + self.assertIsNone(placed) + after = {(it.id, it.x, it.y, it.w, it.h) for it in existing} + self.assertEqual(before, after) + + +if __name__ == "__main__": + unittest.main(verbosity=2)