mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-19 07:33:18 +02:00
Show the Print Log's per-run cost and energy, and let users pick columns (#2636)
The list and update endpoints serialised field by field and never named
cost / energy_kwh / energy_cost, so values Bambuddy had been recording
all along went out as nulls. Both now validate from the ORM row, which
removes the chance to omit a field rather than patching the three that
were missing.
Adds a Filament Used column plus a Columns picker for Cost, Energy,
Energy Cost and Finished, persisted per browser.
Also fixes the log view being unreachable with zero archives: the empty
state ran before the view check, hiding a log that outlives the archives
it refers to.
---
Sort the Print Log by any column (#2636)
Adds sort_by / sort_dir to the print-log endpoint, driven by clickable
column headers. Server-side because paging is: ordering the rows the
client holds would sort one page rather than the log.
Empty values are held last in both directions — Postgres sorts NULLs
high and SQLite low, so the same click would otherwise open on blanks
on one backend and values on the other. id DESC breaks ties so paging
through a low-cardinality sort can't repeat or skip a row.
This commit is contained in:
maziggy
@@ -3,7 +3,7 @@ from datetime import datetime
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query
|
||||
from fastapi.responses import FileResponse
|
||||
from sqlalchemy import delete, func, select
|
||||
from sqlalchemy import delete, func, nullslast, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from backend.app.core.auth import (
|
||||
@@ -22,6 +22,30 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
router = APIRouter(prefix="/print-log", tags=["print-log"])
|
||||
|
||||
# Sortable columns, keyed by the id the Print Log table uses for its columns
|
||||
# (#2636). An explicit map rather than getattr on a caller-supplied string:
|
||||
# the client picks the key, so anything else would let a request order by any
|
||||
# attribute it can name.
|
||||
#
|
||||
# ``date`` coalesces because the column renders ``started_at or created_at`` —
|
||||
# sorting on started_at alone would scatter the rows that have no start time
|
||||
# (queue-skipped entries) instead of interleaving them where the user sees
|
||||
# them.
|
||||
_SORTABLE_COLUMNS = {
|
||||
"date": func.coalesce(PrintLogEntry.started_at, PrintLogEntry.created_at),
|
||||
"print_name": PrintLogEntry.print_name,
|
||||
"printer": PrintLogEntry.printer_name,
|
||||
"user": PrintLogEntry.created_by_username,
|
||||
"status": PrintLogEntry.status,
|
||||
"duration": PrintLogEntry.duration_seconds,
|
||||
"completed_at": PrintLogEntry.completed_at,
|
||||
"filament": PrintLogEntry.filament_type,
|
||||
"filament_used": PrintLogEntry.filament_used_grams,
|
||||
"cost": PrintLogEntry.cost,
|
||||
"energy": PrintLogEntry.energy_kwh,
|
||||
"energy_cost": PrintLogEntry.energy_cost,
|
||||
}
|
||||
|
||||
|
||||
@router.get("/", response_model=PrintLogResponse)
|
||||
async def get_print_log(
|
||||
@@ -33,6 +57,8 @@ async def get_print_log(
|
||||
date_to: datetime | None = None,
|
||||
limit: int = Query(default=50, ge=1, le=500),
|
||||
offset: int = Query(default=0, ge=0),
|
||||
sort_by: str = Query(default="date"),
|
||||
sort_dir: str = Query(default="desc", pattern="^(asc|desc)$"),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
auth_result: tuple[User | None, bool] = Depends(
|
||||
require_ownership_permission(
|
||||
@@ -72,37 +98,36 @@ async def get_print_log(
|
||||
total_result = await db.execute(count_query)
|
||||
total = total_result.scalar() or 0
|
||||
|
||||
# Get paginated results
|
||||
query = query.order_by(PrintLogEntry.created_at.desc()).offset(offset).limit(limit)
|
||||
# Sorting happens here rather than in the browser because the table is
|
||||
# paginated server-side: ordering the 25 rows the client happens to hold
|
||||
# would answer "the most expensive print on this page", which is not what
|
||||
# clicking a column header means.
|
||||
sort_column = _SORTABLE_COLUMNS.get(sort_by)
|
||||
if sort_column is None:
|
||||
raise HTTPException(400, f"Cannot sort by {sort_by!r}")
|
||||
ordering = sort_column.asc() if sort_dir == "asc" else sort_column.desc()
|
||||
# NULLs last in both directions, so a column that is empty for half the
|
||||
# rows (cost before a spool is priced, energy without a smart plug) never
|
||||
# buries the rows that do have values. Left to the database this differs
|
||||
# per backend — Postgres sorts NULLs high, SQLite sorts them low — so the
|
||||
# same click would give two different first pages depending on deployment.
|
||||
query = query.order_by(nullslast(ordering), PrintLogEntry.id.desc())
|
||||
# id.desc() above is the tiebreaker: without it, rows sharing a value
|
||||
# (every "completed" when sorting by status) come back in whatever order
|
||||
# the planner picks, which can differ between pages and duplicate or drop
|
||||
# a row as the user pages through.
|
||||
query = query.offset(offset).limit(limit)
|
||||
result = await db.execute(query)
|
||||
entries = result.scalars().all()
|
||||
|
||||
# Validate straight off the ORM rows rather than naming each field: the
|
||||
# hand-written version dropped whatever it forgot to mention, and a
|
||||
# forgotten field is indistinguishable from a NULL column on the wire.
|
||||
# It lost failure_reason that way (#1687 part 4), then cost / energy_kwh /
|
||||
# energy_cost, which were written to the table but never sent — so the
|
||||
# Print Log's cost and energy columns read empty for every run (#2636).
|
||||
return PrintLogResponse(
|
||||
items=[
|
||||
PrintLogEntrySchema(
|
||||
id=e.id,
|
||||
archive_id=e.archive_id,
|
||||
print_name=e.print_name,
|
||||
printer_name=e.printer_name,
|
||||
printer_id=e.printer_id,
|
||||
status=e.status,
|
||||
started_at=e.started_at,
|
||||
completed_at=e.completed_at,
|
||||
duration_seconds=e.duration_seconds,
|
||||
filament_type=e.filament_type,
|
||||
filament_color=e.filament_color,
|
||||
filament_used_grams=e.filament_used_grams,
|
||||
# failure_reason was silently dropped by the GET serialiser
|
||||
# before #1687 part 4 — without it the Print Log table couldn't
|
||||
# surface what the Failure Analysis widget already groups by.
|
||||
failure_reason=e.failure_reason,
|
||||
thumbnail_path=e.thumbnail_path,
|
||||
created_by_id=e.created_by_id,
|
||||
created_by_username=e.created_by_username,
|
||||
created_at=e.created_at,
|
||||
)
|
||||
for e in entries
|
||||
],
|
||||
items=[PrintLogEntrySchema.model_validate(e) for e in entries],
|
||||
total=total,
|
||||
)
|
||||
|
||||
@@ -285,22 +310,7 @@ async def update_print_log_entry(
|
||||
entry.status,
|
||||
)
|
||||
|
||||
return PrintLogEntrySchema(
|
||||
id=entry.id,
|
||||
archive_id=entry.archive_id,
|
||||
print_name=entry.print_name,
|
||||
printer_name=entry.printer_name,
|
||||
printer_id=entry.printer_id,
|
||||
status=entry.status,
|
||||
started_at=entry.started_at,
|
||||
completed_at=entry.completed_at,
|
||||
duration_seconds=entry.duration_seconds,
|
||||
filament_type=entry.filament_type,
|
||||
filament_color=entry.filament_color,
|
||||
filament_used_grams=entry.filament_used_grams,
|
||||
failure_reason=entry.failure_reason,
|
||||
thumbnail_path=entry.thumbnail_path,
|
||||
created_by_id=entry.created_by_id,
|
||||
created_by_username=entry.created_by_username,
|
||||
created_at=entry.created_at,
|
||||
)
|
||||
# Same field-by-field trap as the list route: this one also omitted cost
|
||||
# and the energy pair, so the row the client merged back after an edit
|
||||
# blanked whichever columns it was showing for them.
|
||||
return PrintLogEntrySchema.model_validate(entry)
|
||||
|
||||
@@ -1,9 +1,17 @@
|
||||
from datetime import datetime
|
||||
|
||||
from pydantic import BaseModel
|
||||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
|
||||
class PrintLogEntrySchema(BaseModel):
|
||||
# from_attributes lets the routes build this straight off the ORM row.
|
||||
# The GET serialiser used to name every field by hand, and each field it
|
||||
# forgot came back as its default — a silent null rather than an error.
|
||||
# That cost the log its failure_reason (#1687 part 4) and then its cost /
|
||||
# energy_kwh / energy_cost (#2636). Validating from the row removes the
|
||||
# chance to forget one.
|
||||
model_config = ConfigDict(from_attributes=True)
|
||||
|
||||
id: int
|
||||
archive_id: int | None = None
|
||||
print_name: str | None = None
|
||||
|
||||
@@ -1952,3 +1952,128 @@ class TestSoftDeletedArchivesAreExcluded:
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.json()["failed_prints"] == 1
|
||||
|
||||
|
||||
class TestPrintLogSorting:
|
||||
"""#2636: the Print Log's column headers sort the whole log.
|
||||
|
||||
Sorting is server-side because paging is: ordering the rows the browser
|
||||
happens to hold would answer "the most expensive print on this page",
|
||||
not "the most expensive print". These pin the ordering contract the
|
||||
headers depend on, including the two cases that differ per database
|
||||
backend or per query plan if left implicit.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
async def _seed(db_session, printer_id: int, rows: list[dict]):
|
||||
from datetime import datetime
|
||||
|
||||
from backend.app.models.print_log import PrintLogEntry
|
||||
|
||||
created = []
|
||||
for i, row in enumerate(rows):
|
||||
entry = PrintLogEntry(
|
||||
printer_id=printer_id,
|
||||
status=row.get("status", "completed"),
|
||||
print_name=row.get("print_name", f"job-{i}"),
|
||||
started_at=datetime(2026, 1, 1 + i, 12, 0, 0),
|
||||
created_at=datetime(2026, 1, 1 + i, 12, 0, 0),
|
||||
filament_used_grams=row.get("grams"),
|
||||
cost=row.get("cost"),
|
||||
energy_kwh=row.get("kwh"),
|
||||
)
|
||||
db_session.add(entry)
|
||||
created.append(entry)
|
||||
await db_session.commit()
|
||||
return created
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_sorts_by_filament_used_in_both_directions(
|
||||
self, async_client: AsyncClient, printer_factory, db_session
|
||||
):
|
||||
printer = await printer_factory()
|
||||
await self._seed(
|
||||
db_session,
|
||||
printer.id,
|
||||
[{"grams": 5.0}, {"grams": 120.0}, {"grams": 30.0}],
|
||||
)
|
||||
|
||||
asc = await async_client.get("/api/v1/print-log/?sort_by=filament_used&sort_dir=asc")
|
||||
assert asc.status_code == 200
|
||||
assert [e["filament_used_grams"] for e in asc.json()["items"]] == [5.0, 30.0, 120.0]
|
||||
|
||||
desc = await async_client.get("/api/v1/print-log/?sort_by=filament_used&sort_dir=desc")
|
||||
assert [e["filament_used_grams"] for e in desc.json()["items"]] == [120.0, 30.0, 5.0]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_empty_values_sort_last_whichever_direction(
|
||||
self, async_client: AsyncClient, printer_factory, db_session
|
||||
):
|
||||
"""Cost is NULL until a spool is priced and energy until a smart plug
|
||||
reports, so these columns are half-empty for most people. Postgres
|
||||
sorts NULLs high and SQLite sorts them low, so without an explicit
|
||||
NULLS LAST the same click gives a different first page depending on
|
||||
which database the user deployed — and on one of them, a screenful
|
||||
of blanks."""
|
||||
printer = await printer_factory()
|
||||
await self._seed(
|
||||
db_session,
|
||||
printer.id,
|
||||
[{"cost": None}, {"cost": 2.5}, {"cost": None}, {"cost": 0.75}],
|
||||
)
|
||||
|
||||
for direction, expected in (("asc", [0.75, 2.5]), ("desc", [2.5, 0.75])):
|
||||
resp = await async_client.get(f"/api/v1/print-log/?sort_by=cost&sort_dir={direction}")
|
||||
costs = [e["cost"] for e in resp.json()["items"]]
|
||||
assert costs[:2] == expected, (direction, costs)
|
||||
assert costs[2:] == [None, None], (direction, costs)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_ties_are_broken_stably_across_pages(self, async_client: AsyncClient, printer_factory, db_session):
|
||||
"""Sorting by a column where every row shares a value (status) leaves
|
||||
the order to the planner unless a tiebreaker is added — and an
|
||||
unstable order can show the same row on two pages while another never
|
||||
appears at all."""
|
||||
printer = await printer_factory()
|
||||
await self._seed(db_session, printer.id, [{"status": "completed"} for _ in range(6)])
|
||||
|
||||
first = await async_client.get("/api/v1/print-log/?sort_by=status&sort_dir=asc&limit=3&offset=0")
|
||||
second = await async_client.get("/api/v1/print-log/?sort_by=status&sort_dir=asc&limit=3&offset=3")
|
||||
page_1 = [e["id"] for e in first.json()["items"]]
|
||||
page_2 = [e["id"] for e in second.json()["items"]]
|
||||
|
||||
assert len(set(page_1) & set(page_2)) == 0, "a row appeared on both pages"
|
||||
assert len(set(page_1) | set(page_2)) == 6, "a row was never returned"
|
||||
# Repeating the same request must give the same page back.
|
||||
again = await async_client.get("/api/v1/print-log/?sort_by=status&sort_dir=asc&limit=3&offset=0")
|
||||
assert [e["id"] for e in again.json()["items"]] == page_1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_unknown_sort_column_is_rejected(self, async_client: AsyncClient):
|
||||
"""The client picks the sort key, so the column list is a whitelist —
|
||||
anything else would let a request order by any attribute it can name."""
|
||||
resp = await async_client.get("/api/v1/print-log/?sort_by=created_by_id")
|
||||
assert resp.status_code == 400
|
||||
resp = await async_client.get("/api/v1/print-log/?sort_by=1;DROP")
|
||||
assert resp.status_code == 400
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_invalid_direction_is_rejected(self, async_client: AsyncClient):
|
||||
resp = await async_client.get("/api/v1/print-log/?sort_by=date&sort_dir=sideways")
|
||||
assert resp.status_code == 422
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_defaults_to_newest_first(self, async_client: AsyncClient, printer_factory, db_session):
|
||||
"""No sort params — the pre-#2636 behaviour, which existing clients
|
||||
and the first page load both rely on."""
|
||||
printer = await printer_factory()
|
||||
await self._seed(db_session, printer.id, [{"print_name": "oldest"}, {"print_name": "newest"}])
|
||||
|
||||
resp = await async_client.get("/api/v1/print-log/")
|
||||
assert [e["print_name"] for e in resp.json()["items"]] == ["newest", "oldest"]
|
||||
|
||||
@@ -145,3 +145,68 @@ class TestWriteLogEntryReconciledDuration:
|
||||
async def test_non_reconciled_missing_times_is_none(self):
|
||||
entry = await self._write(status="completed", started_at=datetime(2026, 7, 15, 10, 0, 0))
|
||||
assert entry.duration_seconds is None
|
||||
|
||||
|
||||
class TestSchemaValidatesFromOrmRow:
|
||||
"""#2636: the Print Log's cost and energy columns read empty for every
|
||||
run because both routes built the response field-by-field and simply
|
||||
never mentioned ``cost`` / ``energy_kwh`` / ``energy_cost``. Pydantic
|
||||
filled the gap with each field's default, so a dropped field looked
|
||||
exactly like a NULL column on the wire — no error, no log line. The
|
||||
same trap had already eaten ``failure_reason`` once (#1687 part 4).
|
||||
|
||||
Validating from the ORM row is what removes the chance to forget one, so
|
||||
these tests pin the mechanism rather than any particular field list.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _row(**overrides):
|
||||
row = MagicMock()
|
||||
row.id = 7
|
||||
row.archive_id = 3
|
||||
row.print_name = "Benchy"
|
||||
row.printer_name = "X1C-01"
|
||||
row.printer_id = 1
|
||||
row.status = "completed"
|
||||
row.started_at = datetime(2026, 7, 24, 18, 35, 0)
|
||||
row.completed_at = datetime(2026, 7, 24, 19, 24, 0)
|
||||
row.duration_seconds = 2940
|
||||
row.filament_type = "PLA"
|
||||
row.filament_color = "#000000"
|
||||
row.filament_used_grams = 15.5
|
||||
row.cost = 0.42
|
||||
row.energy_kwh = 0.31
|
||||
row.energy_cost = 0.09
|
||||
# Non-null so `test_every_declared_field_is_carried` can assert that
|
||||
# nothing falls back to its default.
|
||||
row.failure_reason = "warping"
|
||||
row.thumbnail_path = "archives/1/x/thumbnail.png"
|
||||
row.created_by_id = 2
|
||||
row.created_by_username = "martin"
|
||||
row.created_at = datetime(2026, 7, 24, 18, 35, 0)
|
||||
for k, v in overrides.items():
|
||||
setattr(row, k, v)
|
||||
return row
|
||||
|
||||
def test_money_and_energy_survive_the_round_trip(self):
|
||||
entry = PrintLogEntrySchema.model_validate(self._row())
|
||||
assert entry.cost == 0.42
|
||||
assert entry.energy_kwh == 0.31
|
||||
assert entry.energy_cost == 0.09
|
||||
assert entry.filament_used_grams == 15.5
|
||||
|
||||
def test_every_declared_field_is_carried(self):
|
||||
"""Nothing on the schema may come back as its default when the row
|
||||
has a value — that is the whole failure mode, generalised."""
|
||||
entry = PrintLogEntrySchema.model_validate(self._row())
|
||||
for name in PrintLogEntrySchema.model_fields:
|
||||
assert getattr(entry, name) is not None, f"{name} was dropped in serialisation"
|
||||
|
||||
def test_a_genuinely_null_column_stays_null(self):
|
||||
"""The counterpart: energy is written by a background task after the
|
||||
row, so a just-finished print really has none. That must read as
|
||||
None, not as a fabricated zero."""
|
||||
entry = PrintLogEntrySchema.model_validate(self._row(energy_kwh=None, energy_cost=None))
|
||||
assert entry.energy_kwh is None
|
||||
assert entry.energy_cost is None
|
||||
assert entry.cost == 0.42
|
||||
|
||||
Reference in New Issue
Block a user