From 1f5234a3e97ea27e05e56e904cc26569579aafa5 Mon Sep 17 00:00:00 2001 From: Darek Date: Sun, 12 Apr 2026 12:57:10 -0500 Subject: [PATCH] Simplify battery management: bulk add, device-level auto-install, mass operations - Replace single-battery add form with bulk add (brand + count, auto-generated labels) - Add device-level install form: specify brand+qty pairs, system autoselects available batteries - Add bulk actions on dashboard: retire, delete, unassign, change brand (checkbox multi-select) - Keep per-battery assign route for special cases (e.g. known low-capacity battery) - Remove unique constraint on Battery.label (labels are now auto-generated) - Add *.snapshot to .gitignore for DB snapshot files - Rewrite tests: 35 passing (11 new tests for bulk-add, device-install, bulk-actions) Co-Authored-By: Claude Sonnet 4.6 --- .gitignore | 1 + app.py | 153 ++++++++++++++++++++++++++---- models.py | 2 +- templates/battery_add.html | 23 ++--- templates/dashboard.html | 143 ++++++++++++++++++---------- templates/device_detail.html | 18 ++++ tests/conftest.py | 15 ++- tests/test_acceptance.py | 179 +++++++++++++++++++++++++++-------- 8 files changed, 403 insertions(+), 131 deletions(-) diff --git a/.gitignore b/.gitignore index 67afd6f..5f5ce8c 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ __pycache__/ *.db *.db-shm *.db-wal +*.snapshot instance/ .pytest_cache/ *.egg-info/ diff --git a/app.py b/app.py index 4c720fb..a00c909 100644 --- a/app.py +++ b/app.py @@ -1,5 +1,5 @@ from flask import Flask, render_template, redirect, url_for, request, flash, abort -from sqlalchemy import create_engine +from sqlalchemy import create_engine, func from sqlalchemy.orm import scoped_session, sessionmaker from models import Base, Battery, Device @@ -37,31 +37,27 @@ def create_app(config_object="config"): @app.route("/battery/add", methods=["GET", "POST"]) def battery_add(): if request.method == "POST": - label = request.form.get("label", "").strip() brand = request.form.get("brand", "").strip() - status = request.form.get("status", "available").strip() notes = request.form.get("notes", "").strip() or None + try: + count = max(1, min(50, int(request.form.get("count", 1) or 1))) + except (ValueError, TypeError): + count = 1 - if not label or not brand: - flash("Label and brand are required.", "error") - return render_template("battery_add.html"), 400 - - if status not in ("available", "installed", "retired"): - status = "available" - - if db.query(Battery).filter_by(label=label).first(): - flash(f"A battery with label '{label}' already exists.", "error") + if not brand: + flash("Brand is required.", "error") return render_template("battery_add.html", - form_label=label, form_brand=brand, - form_status=status, form_notes=notes or ""), 400 + form_brand="", form_count=1, form_notes=notes or ""), 400 - battery = Battery(label=label, brand=brand, status=status, notes=notes) - db.add(battery) + existing = db.query(func.count(Battery.id)).filter_by(brand=brand).scalar() + for i in range(count): + label = f"{brand} {existing + i + 1:03d}" + db.add(Battery(label=label, brand=brand, status="available", notes=notes)) db.commit() - flash(f"Battery {label} added.", "success") + flash(f"Added {count} {brand} batter{'y' if count == 1 else 'ies'}.", "success") return redirect(url_for("dashboard")) - return render_template("battery_add.html") + return render_template("battery_add.html", form_count=1) # ------------------------------------------------------------------ # # Battery — detail @@ -190,6 +186,54 @@ def create_app(config_object="config"): return redirect(url_for("dashboard")) return render_template("battery_delete.html", battery=battery) + # ------------------------------------------------------------------ # + # Battery — bulk action + # ------------------------------------------------------------------ # + + @app.route("/battery/bulk-action", methods=["POST"]) + def battery_bulk_action(): + ids = request.form.getlist("battery_ids", type=int) + if not ids: + flash("No batteries selected.", "error") + return redirect(url_for("dashboard")) + + batteries = db.query(Battery).filter(Battery.id.in_(ids)).all() + action = request.form.get("action") + n = len(batteries) + + if action == "retire": + for b in batteries: + b.status = "retired" + b.device_id = None + db.commit() + flash(f"Retired {n} batter{'y' if n == 1 else 'ies'}.", "success") + elif action == "delete": + for b in batteries: + db.delete(b) + db.commit() + flash(f"Deleted {n} batter{'y' if n == 1 else 'ies'}.", "success") + elif action == "unassign": + count = sum(1 for b in batteries if b.is_installed()) + for b in batteries: + if b.is_installed(): + b.status = "available" + b.device_id = None + db.commit() + flash(f"Unassigned {count} batter{'y' if count == 1 else 'ies'}.", "success") + elif action == "set_brand": + new_brand = request.form.get("new_brand", "").strip() + if not new_brand: + flash("Brand name is required.", "error") + return redirect(url_for("dashboard")) + for b in batteries: + b.brand = new_brand + db.commit() + flash(f"Updated brand to '{new_brand}' for {n} batter{'y' if n == 1 else 'ies'}.", "success") + else: + flash("Unknown action.", "error") + + return redirect(url_for("dashboard")) + # ------------------------------------------------------------------ # # Devices — list # ------------------------------------------------------------------ # @@ -248,6 +292,79 @@ def create_app(config_object="config"): abort(404) return render_template("device_detail.html", device=device) + # ------------------------------------------------------------------ # + # Devices — install batteries + # ------------------------------------------------------------------ # + + @app.route("/device//install", methods=["POST"]) + def device_install(device_id): + device = db.get(Device, device_id) + if device is None: + abort(404) + + brands = request.form.getlist("brand[]") + qtys_raw = request.form.getlist("qty[]") + + pairs = [] + for brand, qty_raw in zip(brands, qtys_raw): + brand = brand.strip() + try: + qty = int(qty_raw) + except (ValueError, TypeError): + qty = 0 + if brand and qty > 0: + pairs.append((brand, qty)) + + if not pairs: + flash("No batteries specified.", "error") + return redirect(url_for("device_detail", device_id=device_id)) + + free_slots = device.battery_slots - device.installed_count() + total_requested = sum(qty for _, qty in pairs) + if total_requested > free_slots: + flash( + f"Only {free_slots} slot(s) free, but {total_requested} requested.", + "error", + ) + return redirect(url_for("device_detail", device_id=device_id)) + + # Validate availability before writing anything + for brand, qty in pairs: + available_count = ( + db.query(func.count(Battery.id)) + .filter_by(brand=brand, status="available") + .scalar() + ) + if available_count < qty: + flash( + f"Need {qty} {brand}, but only {available_count} available.", + "error", + ) + return redirect(url_for("device_detail", device_id=device_id)) + + # All checks passed — perform installs + total_installed = 0 + for brand, qty in pairs: + batch = ( + db.query(Battery) + .filter_by(brand=brand, status="available") + .order_by(Battery.id) + .limit(qty) + .all() + ) + for b in batch: + b.status = "installed" + b.device_id = device.id + total_installed += 1 + + db.commit() + flash( + f"Installed {total_installed} batter{'y' if total_installed == 1 else 'ies'}" + f" into {device.name}.", + "success", + ) + return redirect(url_for("device_detail", device_id=device_id)) + # ------------------------------------------------------------------ # # Devices — delete # ------------------------------------------------------------------ # diff --git a/models.py b/models.py index b846a0b..9b6e334 100644 --- a/models.py +++ b/models.py @@ -31,7 +31,7 @@ class Battery(Base): __tablename__ = "battery" id = Column(Integer, primary_key=True, autoincrement=True) - label = Column(String(50), nullable=False, unique=True) + label = Column(String(50), nullable=False) brand = Column(String(100), nullable=False) status = Column(String(20), nullable=False, default="available") device_id = Column(Integer, ForeignKey("device.id", ondelete="SET NULL"), nullable=True) diff --git a/templates/battery_add.html b/templates/battery_add.html index bbd9e58..9e4e23f 100644 --- a/templates/battery_add.html +++ b/templates/battery_add.html @@ -1,17 +1,11 @@ {% extends "base.html" %} -{% block title %}Add Battery — Battery Tracker{% endblock %} +{% block title %}Add Batteries — Battery Tracker{% endblock %} {% block content %} -

Add Battery

+

Add Batteries

-
- - -
-
- - + + + Labels are auto-generated (e.g. Eneloop 001, Eneloop 002)
- +
- + Cancel
diff --git a/templates/dashboard.html b/templates/dashboard.html index 74f80d3..5953047 100644 --- a/templates/dashboard.html +++ b/templates/dashboard.html @@ -29,60 +29,101 @@
-
- - - - - - - - - - - - {% for b in batteries %} - - - - - + + {% else %} + + {% endfor %} + +
LabelBrandStatusAssigned ToActions
{{ b.label }}{{ b.brand }} - {{ b.status|capitalize }} - - {% if b.device %} - {{ b.device.name }} - {% if b.device.has_mixed_brands() %} - ⚠ mixed +
+ + + +
+ + + + + + + + + + + + + {% for b in batteries %} + + + + + + - + - - {% else %} - - {% endfor %} - -
LabelBrandStatusAssigned ToActions
{{ b.label }}{{ b.brand }} + {{ b.status|capitalize }} + + {% if b.device %} + {{ b.device.name }} + {% if b.device.has_mixed_brands() %} + ⚠ mixed + {% endif %} + {% else %} + — {% endif %} - {% else %} - — - {% endif %} - - View + + View - {% if b.is_available() %} - Assign - {% endif %} + {% if b.is_available() %} + Assign + {% endif %} - {% if b.is_installed() %} - - - - {% endif %} + {% if b.is_installed() %} + + {% endif %} - {% if not b.is_retired() %} -
- -
- {% endif %} -
No batteries found. Add one.
-
+ {% if not b.is_retired() %} + + {% endif %} +
No batteries found. Add some.
+
+ +
+ + {% endblock %} diff --git a/templates/device_detail.html b/templates/device_detail.html index c263478..4e6bd12 100644 --- a/templates/device_detail.html +++ b/templates/device_detail.html @@ -25,6 +25,24 @@
+
+

Install Batteries

+ {% set free_slots = device.battery_slots - device.installed_count() %} +

{{ free_slots }} slot(s) free

+
+
+ Brand + Qty + {% for _ in range(4) %} + + + {% endfor %} +
+ +
+
+

Installed Batteries

{% set installed = device.batteries | selectattr('status', 'eq', 'installed') | list %} diff --git a/tests/conftest.py b/tests/conftest.py index 76fd09f..1695b02 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -31,11 +31,18 @@ def client(app): @pytest.fixture() def seeded_client(app): - """Test client pre-loaded with 2 devices and 3 batteries (2 available, 1 retired).""" + """Test client pre-loaded with 2 devices and 3 batteries (2 available, 1 retired). + + Batteries: + id=1 BrandX 001 (BrandX, available) + id=2 BrandY 001 (BrandY, available) + id=3 BrandX 002 (BrandX, retired) + """ with app.test_client() as c: c.post("/device/add", data={"name": "Device A", "battery_slots": "2"}) c.post("/device/add", data={"name": "Device B", "battery_slots": "1"}) - c.post("/battery/add", data={"label": "TST-01", "brand": "BrandX", "status": "available"}) - c.post("/battery/add", data={"label": "TST-02", "brand": "BrandY", "status": "available"}) - c.post("/battery/add", data={"label": "TST-03", "brand": "BrandX", "status": "retired"}) + c.post("/battery/add", data={"brand": "BrandX", "count": "1"}) # id=1 + c.post("/battery/add", data={"brand": "BrandY", "count": "1"}) # id=2 + c.post("/battery/add", data={"brand": "BrandX", "count": "1"}) # id=3 + c.post("/battery/3/retire") yield c diff --git a/tests/test_acceptance.py b/tests/test_acceptance.py index 4e10b6a..cecaeb6 100644 --- a/tests/test_acceptance.py +++ b/tests/test_acceptance.py @@ -4,7 +4,8 @@ Acceptance tests for Battery Tracker. Each test gets a fresh in-memory SQLite database via the fixtures in conftest.py. The `seeded_client` fixture pre-populates: Devices: Device A (2 slots), Device B (1 slot) - Batteries: TST-01 (BrandX, available), TST-02 (BrandY, available), TST-03 (BrandX, retired) + Batteries: BrandX 001 (id=1, available), BrandY 001 (id=2, available), + BrandX 002 (id=3, retired) """ import pytest @@ -17,7 +18,6 @@ import pytest def get_location(resp): """Return the redirect Location header (without host).""" loc = resp.headers.get("Location", "") - # Strip scheme+host if present if loc.startswith("http"): from urllib.parse import urlparse loc = urlparse(loc).path @@ -36,34 +36,37 @@ def follow(client, resp): def test_dashboard_loads(seeded_client): resp = seeded_client.get("/") assert resp.status_code == 200 - assert b"TST-01" in resp.data - assert b"TST-02" in resp.data - assert b"TST-03" in resp.data + assert b"BrandX 001" in resp.data + assert b"BrandY 001" in resp.data + assert b"BrandX 002" in resp.data # ------------------------------------------------------------------ # -# Battery — add +# Battery — bulk add # ------------------------------------------------------------------ # -def test_add_battery(client): +def test_bulk_add_batteries(client): resp = client.post("/battery/add", - data={"label": "NEW-01", "brand": "TestBrand", "status": "available"}, + data={"brand": "Eneloop", "count": "3"}, follow_redirects=True) assert resp.status_code == 200 - assert b"NEW-01" in resp.data + assert b"Eneloop 001" in resp.data + assert b"Eneloop 002" in resp.data + assert b"Eneloop 003" in resp.data -def test_add_battery_duplicate_label(seeded_client): - resp = seeded_client.post("/battery/add", - data={"label": "TST-01", "brand": "OtherBrand", "status": "available"}) +def test_bulk_add_autogenerates_sequential_labels(client): + client.post("/battery/add", data={"brand": "Eneloop", "count": "2"}) + resp = client.post("/battery/add", data={"brand": "Eneloop", "count": "2"}, + follow_redirects=True) + assert b"Eneloop 003" in resp.data + assert b"Eneloop 004" in resp.data + + +def test_bulk_add_missing_brand(client): + resp = client.post("/battery/add", data={"brand": "", "count": "1"}) assert resp.status_code == 400 - assert b"already exists" in resp.data - - -def test_add_battery_missing_fields(client): - resp = client.post("/battery/add", data={"label": "", "brand": ""}) - assert resp.status_code == 400 - assert b"required" in resp.data + assert b"required" in resp.data.lower() # ------------------------------------------------------------------ # @@ -73,7 +76,7 @@ def test_add_battery_missing_fields(client): def test_battery_detail(seeded_client): resp = seeded_client.get("/battery/1") assert resp.status_code == 200 - assert b"TST-01" in resp.data + assert b"BrandX 001" in resp.data assert b"BrandX" in resp.data @@ -93,11 +96,10 @@ def test_edit_notes(seeded_client): # ------------------------------------------------------------------ # -# Battery — assign +# Battery — assign (per-battery, kept for special cases) # ------------------------------------------------------------------ # def test_assign_battery(seeded_client): - # TST-01 is battery id=1, Device A is device id=1 resp = seeded_client.post("/battery/1/assign", data={"device_id": "1"}, follow_redirects=True) @@ -106,7 +108,7 @@ def test_assign_battery(seeded_client): def test_assign_retired_battery_blocked(seeded_client): - # TST-03 is retired (id=3) + # id=3 is retired resp = seeded_client.post("/battery/3/assign", data={"device_id": "1"}, follow_redirects=True) @@ -115,9 +117,9 @@ def test_assign_retired_battery_blocked(seeded_client): def test_assign_over_capacity_blocked(seeded_client): - # Fill Device B (1 slot) with TST-01 + # Fill Device B (1 slot) with id=1 seeded_client.post("/battery/1/assign", data={"device_id": "2"}) - # Try to assign TST-02 to the same full device + # Try to assign id=2 to the same full device resp = seeded_client.post("/battery/2/assign", data={"device_id": "2"}, follow_redirects=True) @@ -126,14 +128,13 @@ def test_assign_over_capacity_blocked(seeded_client): def test_brand_mix_warning(seeded_client): - # Assign TST-01 (BrandX) to Device A + # Assign BrandX 001 (id=1) to Device A seeded_client.post("/battery/1/assign", data={"device_id": "1"}) - # Assign TST-02 (BrandY) to same Device A → brand mix warning, but succeeds + # Assign BrandY 001 (id=2) to same Device A → brand mix warning, but succeeds resp = seeded_client.post("/battery/2/assign", data={"device_id": "1"}, follow_redirects=True) assert resp.status_code == 200 - # Warning flash should mention mixing assert b"mixing" in resp.data.lower() or b"mix" in resp.data.lower() or b"brand" in resp.data.lower() @@ -145,7 +146,6 @@ def test_unassign_battery(seeded_client): seeded_client.post("/battery/1/assign", data={"device_id": "1"}) resp = seeded_client.post("/battery/1/unassign", follow_redirects=True) assert resp.status_code == 200 - # Battery should show as available on dashboard resp2 = seeded_client.get("/battery/1") assert b"available" in resp2.data.lower() @@ -169,7 +169,7 @@ def test_retire_clears_device(seeded_client): def test_retire_already_retired(seeded_client): - # TST-03 is already retired (id=3) + # id=3 is already retired by fixture resp = seeded_client.post("/battery/3/retire", follow_redirects=True) assert resp.status_code == 200 assert b"already retired" in resp.data.lower() @@ -183,7 +183,7 @@ def test_delete_battery_confirmation_page(seeded_client): resp = seeded_client.get("/battery/1/delete") assert resp.status_code == 200 assert b"Confirm Delete" in resp.data - assert b"TST-01" in resp.data + assert b"BrandX 001" in resp.data def test_delete_battery(seeded_client): @@ -192,6 +192,108 @@ def test_delete_battery(seeded_client): assert resp.status_code == 404 +# ------------------------------------------------------------------ # +# Battery — bulk actions +# ------------------------------------------------------------------ # + +def test_bulk_retire(client): + client.post("/battery/add", data={"brand": "TestBrand", "count": "2"}) + resp = client.post("/battery/bulk-action", + data={"battery_ids": ["1", "2"], "action": "retire"}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"retired" in client.get("/battery/1").data.lower() + assert b"retired" in client.get("/battery/2").data.lower() + + +def test_bulk_delete(client): + client.post("/battery/add", data={"brand": "TestBrand", "count": "2"}) + resp = client.post("/battery/bulk-action", + data={"battery_ids": ["1", "2"], "action": "delete"}, + follow_redirects=True) + assert resp.status_code == 200 + assert client.get("/battery/1").status_code == 404 + assert client.get("/battery/2").status_code == 404 + + +def test_bulk_unassign(client): + client.post("/device/add", data={"name": "Gadget", "battery_slots": "2"}) + client.post("/battery/add", data={"brand": "TestBrand", "count": "2"}) + client.post("/device/1/install", data={"brand[]": "TestBrand", "qty[]": "2"}) + resp = client.post("/battery/bulk-action", + data={"battery_ids": ["1", "2"], "action": "unassign"}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"available" in client.get("/battery/1").data.lower() + + +def test_bulk_set_brand(client): + client.post("/battery/add", data={"brand": "OldBrand", "count": "2"}) + resp = client.post("/battery/bulk-action", + data={"battery_ids": ["1", "2"], "action": "set_brand", + "new_brand": "NewBrand"}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"NewBrand" in client.get("/battery/1").data + + +def test_bulk_action_no_selection(client): + client.post("/battery/add", data={"brand": "TestBrand", "count": "1"}) + resp = client.post("/battery/bulk-action", + data={"action": "retire"}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"No batteries selected" in resp.data + + +# ------------------------------------------------------------------ # +# Device — install batteries +# ------------------------------------------------------------------ # + +def test_device_install_autoselects(client): + client.post("/device/add", data={"name": "Gadget", "battery_slots": "2"}) + client.post("/battery/add", data={"brand": "Eneloop", "count": "3"}) + resp = client.post("/device/1/install", + data={"brand[]": "Eneloop", "qty[]": "2"}, + follow_redirects=True) + assert resp.status_code == 200 + # Device detail should show 2 installed batteries + assert b"Eneloop" in resp.data + assert b"2 / 2" in resp.data + + +def test_device_install_mixed_brands(client): + client.post("/device/add", data={"name": "Remote", "battery_slots": "4"}) + client.post("/battery/add", data={"brand": "Eneloop", "count": "2"}) + client.post("/battery/add", data={"brand": "Energizer", "count": "2"}) + resp = client.post("/device/1/install", + data={"brand[]": ["Eneloop", "Energizer"], "qty[]": ["2", "2"]}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"Eneloop" in resp.data + assert b"Energizer" in resp.data + + +def test_device_install_insufficient_batteries(client): + client.post("/device/add", data={"name": "Gadget", "battery_slots": "4"}) + client.post("/battery/add", data={"brand": "Eneloop", "count": "1"}) + resp = client.post("/device/1/install", + data={"brand[]": "Eneloop", "qty[]": "2"}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"available" in resp.data.lower() + + +def test_device_install_over_capacity(client): + client.post("/device/add", data={"name": "Gadget", "battery_slots": "1"}) + client.post("/battery/add", data={"brand": "Eneloop", "count": "3"}) + resp = client.post("/device/1/install", + data={"brand[]": "Eneloop", "qty[]": "3"}, + follow_redirects=True) + assert resp.status_code == 200 + assert b"slot" in resp.data.lower() + + # ------------------------------------------------------------------ # # Device — list # ------------------------------------------------------------------ # @@ -248,12 +350,9 @@ def test_device_detail_not_found(client): # ------------------------------------------------------------------ # def test_delete_device_frees_batteries(seeded_client): - # Assign TST-01 to Device A seeded_client.post("/battery/1/assign", data={"device_id": "1"}) - # Delete Device A resp = seeded_client.post("/device/1/delete", follow_redirects=True) assert resp.status_code == 200 - # TST-01 should now be available resp2 = seeded_client.get("/battery/1") assert b"available" in resp2.data.lower() @@ -268,14 +367,12 @@ def test_delete_device_removed(seeded_client): # Full round-trip # ------------------------------------------------------------------ # -def test_add_assign_delete_battery(client): - # Add a device +def test_add_install_delete_battery(client): client.post("/device/add", data={"name": "Gadget", "battery_slots": "1"}) - # Add a battery - client.post("/battery/add", data={"label": "RT-01", "brand": "AcmeBrand", "status": "available"}) - # Assign it - resp = client.post("/battery/1/assign", data={"device_id": "1"}, follow_redirects=True) + client.post("/battery/add", data={"brand": "AcmeBrand", "count": "1"}) + resp = client.post("/device/1/install", + data={"brand[]": "AcmeBrand", "qty[]": "1"}, + follow_redirects=True) assert b"Gadget" in resp.data - # Delete it client.post("/battery/1/delete") assert client.get("/battery/1").status_code == 404