Skip to content

Commit feffc0b

Browse files
jpapiezCopilot
andcommitted
Address PR review: clarify article_number docs, repr the bad GTIN, inline the migration helper
Three points from the review, all fair. The article_number description still read 'Vendor article number, e.g. EAN, QR code, etc.' in both the request and response schemas. That was the wording this change exists to retire: with a dedicated gtin column, telling an API consumer to put an EAN in article_number sends the barcode to the wrong field. Both now point at gtin for the barcode. The client_v2 help text already said this; the OpenAPI schema was the half that got missed. The rejection message interpolated the raw value inside hand-written quotes, so a scanner that appends a newline or a control character produced a mangled, multi-line 400 body. !r keeps it on one line and escapes the value properly. The migration imported normalize_gtin from spoolman.gtin. A migration is a historical record -- replaying it on a fresh database has to produce what it produced when it was written -- so it must not change when the application helper is refactored, nor break if that module is moved. The normalization is now inlined, with a comment saying why the duplication is deliberate. Re-verified on a scratch SQLite database seeded with a UPC-12, EAN-13, GTIN-14, EAN-8, two separator forms, three vendor SKUs, a malformed scan, a bad check digit, a wrong length and a NULL: identical results to the imported version, article_number untouched, index present, downgrade clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 24cc6f7 commit feffc0b

3 files changed

Lines changed: 34 additions & 6 deletions

File tree

migrations/versions/2026_08_19_2100-c3a7e1f95d02_filament_gtin.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,42 @@
88
import sqlalchemy as sa
99
from alembic import op
1010

11-
from spoolman.gtin import normalize_gtin
12-
1311
# revision identifiers, used by Alembic.
1412
revision = "c3a7e1f95d02"
1513
down_revision = "9c1d5f2a7b31"
1614
branch_labels = None
1715
depends_on = None
1816

17+
# The GS1 lengths, and the padded width the column stores.
18+
#
19+
# This duplicates spoolman.gtin deliberately. A migration is a historical record: replaying it on
20+
# a fresh database has to produce what it produced the day it was written, so it must not change
21+
# underneath us when the application helper is refactored, and must not break if that module is
22+
# ever moved or renamed. Keep the two in step only if the *definition* of a valid GTIN changes,
23+
# which it will not -- it is a published GS1 standard.
24+
_GTIN_LENGTHS = (8, 12, 13, 14)
25+
_GTIN_STORED_LENGTH = 14
26+
_DIGITS = "0123456789"
27+
28+
29+
def _normalize_gtin(value: str | None) -> str | None:
30+
"""Return a barcode as its zero-padded 14 digit GTIN, or None if it is not a valid one."""
31+
if value is None:
32+
return None
33+
34+
digits = "".join(char for char in value if char in _DIGITS)
35+
if len(digits) not in _GTIN_LENGTHS:
36+
return None
37+
38+
# GS1 mod-10: weight the digits 3 and 1 alternating from the right, ignoring the check digit.
39+
total = 0
40+
for i, char in enumerate(reversed(digits[:-1])):
41+
total += int(char) * (3 if i % 2 == 0 else 1)
42+
if int(digits[-1]) != (10 - total % 10) % 10:
43+
return None
44+
45+
return digits.zfill(_GTIN_STORED_LENGTH)
46+
1947

2048
def upgrade() -> None:
2149
"""Add the filament GTIN column and backfill it from article_number.
@@ -50,7 +78,7 @@ def upgrade() -> None:
5078
rows = connection.execute(sa.select(filament.c.id, filament.c.article_number)).fetchall()
5179

5280
for row in rows:
53-
gtin = normalize_gtin(row.article_number)
81+
gtin = _normalize_gtin(row.article_number)
5482
if gtin is not None:
5583
connection.execute(sa.update(filament).where(filament.c.id == row.id).values(gtin=gtin))
5684

spoolman/api/v1/filament.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def _normalize_gtin_param(value: str | None) -> str | None:
4646
return None
4747
normalized = normalize_gtin(value)
4848
if normalized is None:
49-
raise ValueError(f"'{value}' is not a valid GTIN.")
49+
raise ValueError(f"{value!r} is not a valid GTIN.")
5050
return normalized
5151

5252

@@ -85,7 +85,7 @@ class FilamentParameters(BaseModel):
8585
article_number: str | None = Field(
8686
None,
8787
max_length=64,
88-
description="Vendor article number, e.g. EAN, QR code, etc.",
88+
description="Vendor article number or SKU. For the barcode printed on the packaging, use gtin instead.",
8989
examples=["PM70820"],
9090
)
9191
gtin: str | None = Field(

spoolman/api/v1/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ class Filament(BaseModel):
196196
article_number: str | None = Field(
197197
None,
198198
max_length=64,
199-
description="Vendor article number, e.g. EAN, QR code, etc.",
199+
description="Vendor article number or SKU. For the barcode printed on the packaging, use gtin instead.",
200200
examples=["PM70820"],
201201
)
202202
gtin: str | None = Field(

0 commit comments

Comments
 (0)