BUG-04: Add-Item ohne Re-Pack (first-fit)
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
This commit is contained in:
@@ -13,3 +13,5 @@ minimax_*.png
|
|||||||
config.json
|
config.json
|
||||||
*.service
|
*.service
|
||||||
.backup/
|
.backup/
|
||||||
|
.venv/
|
||||||
|
tests/__pycache__/
|
||||||
|
|||||||
@@ -280,7 +280,15 @@ def api_layout():
|
|||||||
|
|
||||||
@app.route("/api/layout/add", methods=["POST"])
|
@app.route("/api/layout/add", methods=["POST"])
|
||||||
def api_layout_add():
|
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()
|
a = require_auth()
|
||||||
if a: return a
|
if a: return a
|
||||||
plugin = request.form.get("plugin", "hello").strip()
|
plugin = request.form.get("plugin", "hello").strip()
|
||||||
@@ -297,14 +305,19 @@ def api_layout_add():
|
|||||||
except: pass
|
except: pass
|
||||||
import secrets
|
import secrets
|
||||||
new_id = secrets.token_hex(4)
|
new_id = secrets.token_hex(4)
|
||||||
items = cfg.setdefault("layout", {}).setdefault("items", [])
|
items_list = cfg.setdefault("layout", {}).setdefault("items", [])
|
||||||
new_item = layout_mod.Item(new_id, plugin, 0, 0, w, h).to_dict()
|
existing = [layout_mod.Item.from_dict(d) for d in items_list]
|
||||||
items.append(new_item)
|
candidate = layout_mod.Item(new_id, plugin, 0, 0, w, h)
|
||||||
# Pack alle (inkl. neue)
|
placed = layout_mod.first_fit(candidate, existing)
|
||||||
packed = layout_mod.pack([layout_mod.Item.from_dict(d) for d in items])
|
if placed is None:
|
||||||
cfg["layout"]["items"] = [it.to_dict() for it in packed]
|
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)
|
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/<iid>", methods=["DELETE", "PATCH"])
|
@app.route("/api/layout/item/<iid>", methods=["DELETE", "PATCH"])
|
||||||
|
|||||||
@@ -141,6 +141,23 @@ def pack(items: list[Item], order: Optional[list[str]] = None) -> list[Item]:
|
|||||||
return result
|
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]:
|
def auto_size_for_plugin(plugin_name: str) -> tuple[int, int]:
|
||||||
"""Default size when user adds a new item."""
|
"""Default size when user adds a new item."""
|
||||||
presets = {
|
presets = {
|
||||||
|
|||||||
@@ -1603,7 +1603,15 @@
|
|||||||
}
|
}
|
||||||
const r = await fetch('/api/layout/add', { method: 'POST', body: fd });
|
const r = await fetch('/api/layout/add', { method: 'POST', body: fd });
|
||||||
const j = await r.json();
|
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;
|
layoutItems = j.items;
|
||||||
renderGrid();
|
renderGrid();
|
||||||
toast('Widget hinzugefügt', 'success', 2000);
|
toast('Widget hinzugefügt', 'success', 2000);
|
||||||
|
|||||||
@@ -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)
|
||||||
@@ -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)
|
||||||
Reference in New Issue
Block a user