diff --git a/backend/routers/garments.py b/backend/routers/garments.py index 03587b3f..1c1598f4 100644 --- a/backend/routers/garments.py +++ b/backend/routers/garments.py @@ -123,9 +123,9 @@ async def upload_garment( is_co_ord_set: bool = Form(False), segment: str = Form("new_dev"), # new_dev | shopping — drives auto style-number series category: str | None = Form(None), # mens | womens | kids - fabric_quality: str | None = Form(None), # QR-mandatory (buyer-visible quality) - pattern_number: str | None = Form(None), # QR-mandatory - counters: int | None = Form(None), # QR-mandatory (physical pieces made) + fabric_quality: str | None = Form(None), # mandatory (buyer-visible quality) + pattern_number: str | None = Form(None), # OPTIONAL — recorded when the pattern master exists + counters: int | None = Form(None), # mandatory (physical pieces made) remove_bg: bool = Form(False), # when True, run background removal on each saved image capture_mode: str = Form("gallery"), # "gallery" | "camera" — how the shot was taken db: Session = Depends(get_db), @@ -325,7 +325,9 @@ async def upload_garment( ) # Freeze + store the QR right away if the style is QR-ready (style no. auto-assigned + # the mandatory fields present) — so every uploaded style carries a scannable QR in the DB. - if style_number and fabric_quality and pattern_number and counters is not None: + # Keyed on the MANDATORY list (style number, fabric quality, counters): pattern + # number is optional since 2026-09-28, so it must not gate the frozen QR. + if style_number and fabric_quality and counters is not None: garment.qr_payload = plm.build_qr_payload(style_number) garment.qr_generated_at = plm.now_utc() db.add(garment) @@ -833,8 +835,8 @@ def confirm_draft_garment( the confirmation itself is unchanged, and so is every status code. Issue #893: confirming a draft is the same act as approving it in the review - queue, so it is gated on the same QR-mandatory trio (fabric quality, pattern - number, counters) and refuses with the same 422 payload. Without this, the + queue, so it is gated on the same mandatory fields (style number, fabric + quality, counters) and refuses with the same 422 payload. Without this, the rule was optional in practice for anything calling the API directly. """ g = db.query(Garment).filter(Garment.id == garment_id, @@ -999,25 +1001,36 @@ def update_garment(garment_id: str, body: dict, db: Session = Depends(get_db), allowed = { "style_number", "fabric_composition", "season", "design_team_city", "collection_name", "description", "status", "is_co_ord_set", "linked_piece_ids", - # PLM / metadata fields (segment drives auto-numbering series; quality/ - # pattern/counters are QR-mandatory) — editable post-upload so a wrong - # value doesn't force a re-upload. + # PLM / metadata fields (segment drives auto-numbering series; quality and + # counters are mandatory, pattern number is optional) — editable post-upload + # so a wrong value doesn't force a re-upload. "segment", "fabric_quality", "pattern_number", "counters", } - # Style-number guard (#641) — must run BEFORE the assignment loop: the session - # autoflushes a pending UPDATE ahead of the next query, so assigning a clashing - # value first would surface the DB's UniqueViolation as an unhandled 500. - # Same 409 shape as the upload guard above. The DB unique index covers every - # row (soft-deleted included), so the lookup must not filter is_deleted. - # An explicit JSON null is a legitimate "clear the number" call (62 rows in - # the dev DB carry NULL) — it skips the guard and keeps the old behaviour. - if body.get("style_number") is not None: - submitted = str(body["style_number"]).strip() + # Style-number guard (#641, tightened 2026-09-28) — must run BEFORE the + # assignment loop: the session autoflushes a pending UPDATE ahead of the next + # query, so assigning a clashing value first would surface the DB's + # UniqueViolation as an unhandled 500. Same 409 shape as the upload guard + # above. The DB unique index covers every row (soft-deleted included), so the + # lookup must not filter is_deleted. + # + # The style number is MANDATORY. This guard used to treat an explicit JSON + # null as a legitimate "clear the number", and the garment edit form sends + # exactly that when the field is emptied — which is how a style lost its + # identity (and with it its tag, its scan and every TNA join). A null, a + # blank or a whitespace-only value is now a 422 like any other empty string. + # Rows that ALREADY carry NULL are left alone: no backfill here, they simply + # read as "Style number still empty" in the readiness gate until someone + # fills the number in the garment form. + if "style_number" in body: + submitted = str(body["style_number"] or "").strip() if not submitted: raise HTTPException( status_code=422, - detail={"error": "Style number cannot be empty", "code": "INVALID_STYLE_NUMBER"}, + detail={ + "error": "Style number is required — it is the garment's identity (tag, scan, TNA).", + "code": "STYLE_NUMBER_REQUIRED", + }, ) body["style_number"] = submitted # the assignment loop stores the trimmed form clash = ( diff --git a/backend/routers/plm.py b/backend/routers/plm.py index 3aea3653..8ab4bbca 100644 --- a/backend/routers/plm.py +++ b/backend/routers/plm.py @@ -74,17 +74,16 @@ def next_style_number( def _require(g: Garment) -> list[str]: - """Mandatory fields before a QR can be issued (style number is auto). + """Mandatory fields before a QR can be issued. - The QR-mandatory trio itself comes from ``plm_service.QR_MANDATORY_FIELDS`` — - the SAME list the Review-Queue approval gate reads, so "QR-ready" means one - thing everywhere (a garment the gate lets through is one this endpoint will - happily tag, and vice versa). + The list itself comes from ``plm_service.MANDATORY_FIELDS`` — the SAME list + the Review-Queue approval gate reads, so "QR-ready" means one thing everywhere + (a garment the gate lets through is one this endpoint will happily tag, and + vice versa). Style number is IN that list (2026-09-28): it is the garment's + identity, not an auto detail, so it is no longer added here as a special case + — doing that would have listed it twice. """ - missing = plm.missing_qr_fields(g) - if not g.style_number: - missing.insert(0, "style_number") - return missing + return plm.missing_qr_fields(g) @router.post("/{garment_id}/qr", responses={404: {"description": "Not found"}, 422: {"description": "Unprocessable Entity"}}) diff --git a/backend/services/plm_service.py b/backend/services/plm_service.py index 77e04779..cb3e98e0 100644 --- a/backend/services/plm_service.py +++ b/backend/services/plm_service.py @@ -3,7 +3,7 @@ Numbering and QR-tag rules come from the design-team meeting (Notes/Features.md): - Auto style number per category (mens/womens/kids), separate series for Shopping vs new dev. - Colourways of one style = same base code + sub-letter A/B/C/D. -- QR is generated after mandatory fields (quality, pattern number, counters) and is a FROZEN +- QR is generated after the mandatory fields (style number, quality, counters) and is a FROZEN unique identifier — re-issuing makes a NEW QR, never edits the old one. - The physical hangtag shows only Style Number + Fabric Quality; the rest lives behind the scan. @@ -25,8 +25,11 @@ from models.garment import Garment, StyleCounter # Re-exported so QR callers keep reading the contract through this module; the # definitions themselves are dependency-free (services/qr_fields.py). from services.qr_fields import ( # noqa: F401 + MANDATORY_FIELD_LABELS, + MANDATORY_FIELDS, QR_FIELD_LABELS, QR_MANDATORY_FIELDS, + missing_mandatory_fields, missing_qr_fields, qr_field_label, ) @@ -37,7 +40,7 @@ SHOPPING_PREFIX = "S" # prepended for shopping samples → e.g. SM0001 PAD = 4 # zero-padding width → 0001 COLOURWAY_CODES = ["A", "B", "C", "D"] -# ── QR-mandatory garment fields ──────────────────────────────────────────────── +# ── Mandatory garment fields ─────────────────────────────────────────────────── # The list + "what counts as filled" live in `services/qr_fields.py` — a module # with no third-party imports, so the Review-Queue approval gate and the offline # CI tests can read the SAME contract without pulling qrcode/PIL in. Imported diff --git a/backend/services/qr_fields.py b/backend/services/qr_fields.py index f5a190b9..14a667e9 100644 --- a/backend/services/qr_fields.py +++ b/backend/services/qr_fields.py @@ -1,47 +1,62 @@ -"""The QR-mandatory garment fields — the ONE list both gates read. +"""The mandatory garment fields — the ONE list both gates read. Deliberately dependency-free (pure Python, no qrcode/PIL/SQLAlchemy): this is the contract shared by the QR-issuing endpoint, the Review-Queue approval gate and the offline CI tests, so it must be importable in the leanest environment. The rendering/generation side stays in `services/plm_service.py`. -Why the fields matter (design-team rule, Notes/Features.md): the QR is the -garment's identity — fabric quality, pattern number and counters are what the -printed hangtag shows and what a phone scan resolves to. A garment that reaches -the library without them can never be tagged, counted, or joined upstream. +Why these fields (design-team rule, Notes/Features.md): the QR is the garment's +identity. The **style number** IS that identity — the scan URL, the hangtag and +every TNA/order join key off it — and **fabric quality + counters** are what the +printed tag shows and what a phone scan resolves to. A garment that reaches the +library without them can never be tagged, counted, or joined upstream. -`routers/plm.py:_require()` and `services/review_service._approve_draft()` both -call `missing_qr_fields()`, so "QR-ready" cannot mean two different things: a -garment the approval gate lets through is one the tag endpoint will happily tag. +**Pattern number is NOT mandatory (2026-09-28, design team).** It is recorded +when it exists and stays editable everywhere, but the physical sample routinely +reaches the studio before the pattern master is keyed, and blocking the approval +on it stalled garments that were otherwise complete. It is no longer part of +``QR_MANDATORY_FIELDS``. + +`routers/plm.py:_require()`, `services/review_service._approve_draft()` and +`routers/garments.confirm_draft_garment()` all call `missing_qr_fields()`, so +"QR-ready" cannot mean two different things: a garment the approval gate lets +through is one the tag endpoint will happily tag. """ -QR_MANDATORY_FIELDS = ("fabric_quality", "pattern_number", "counters") +MANDATORY_FIELDS = ("style_number", "fabric_quality", "counters") -QR_FIELD_LABELS = { +# Deprecated alias — every caller that predates the rename keeps working. +QR_MANDATORY_FIELDS = MANDATORY_FIELDS + +MANDATORY_FIELD_LABELS = { + "style_number": "Style number", "fabric_quality": "Fabric quality", - "pattern_number": "Pattern no.", "counters": "Counters (pieces)", } +# Deprecated alias. +QR_FIELD_LABELS = MANDATORY_FIELD_LABELS + def qr_field_label(field: str) -> str: - """Human label for a QR field ('pattern_number' → 'Pattern no.').""" - return QR_FIELD_LABELS.get(field, field.replace("_", " ")) + """Human label for a mandatory field ('style_number' → 'Style number').""" + return MANDATORY_FIELD_LABELS.get(field, field.replace("_", " ")) def missing_qr_fields(garment) -> list[str]: - """QR-mandatory fields still empty on ``garment`` (empty list ⇒ QR-ready). + """Mandatory fields still empty on ``garment`` (empty list ⇒ QR-ready). ``counters`` is an Integer column: 0 is a real answer ("none made yet"), so - only NULL counts as missing. On the two text columns a blank/whitespace-only - string is missing too — the same test the QR endpoint has always applied, so - a garment that was refused a QR cannot slip through the approval gate. + only NULL counts as missing. On the text columns (style number, fabric + quality) a blank/whitespace-only string is missing too — the same test the + QR endpoint has always applied, so a garment that was refused a QR cannot + slip through the approval gate. An attribute that cannot be read counts as MISSING (never as ready): neither gate may approve on a guess. """ missing: list[str] = [] - for field in QR_MANDATORY_FIELDS: + for field in MANDATORY_FIELDS: value = getattr(garment, field, None) if value is None: missing.append(field) @@ -50,6 +65,11 @@ def missing_qr_fields(garment) -> list[str]: return missing +# Same function under the name that matches the constant (new call sites should +# read this one; the historical name is kept because 4 modules import it). +missing_mandatory_fields = missing_qr_fields + + # --------------------------------------------------------------------------- # The refusal contract (#893) # @@ -59,15 +79,16 @@ def missing_qr_fields(garment) -> list[str]: # minimal environment may not carry. # --------------------------------------------------------------------------- CODE_QR_FIELDS_MISSING = "QR_FIELDS_MISSING" -ERR_QR_FIELDS_MISSING = "This garment can't be approved yet — QR fields are missing" +ERR_QR_FIELDS_MISSING = "This garment can't be approved yet — mandatory fields are missing" QR_REFUSAL_MESSAGE = ( - "Fill the QR fields — fabric quality, pattern number and counters — then " - "{verb}. They are what the printed tag and the phone scan resolve to." + "Fill the mandatory fields — style number, fabric quality and counters — then " + "{verb}. They are the garment's identity: what the printed tag, the phone scan " + "and every TNA join resolve to." ) def qr_refusal_detail(garment, verb: str = "approve") -> dict: - """The 422 payload for a garment whose QR-mandatory trio is incomplete. + """The 422 payload for a garment whose mandatory fields are incomplete. Shape-compatible with the review approve refusal on purpose: the same UI handling (naming and highlighting the fields) then works whichever door the diff --git a/backend/services/review_service.py b/backend/services/review_service.py index ad63f86f..afff70a6 100644 --- a/backend/services/review_service.py +++ b/backend/services/review_service.py @@ -23,7 +23,7 @@ logger = logging.getLogger(__name__) ERR_REVIEW_NOT_FOUND = "Review item not found" # A capture draft cannot be APPROVED (confirmed into the library) while any -# QR-mandatory garment field is still empty. The same trio the QR endpoint +# mandatory garment field is still empty. The same list the QR endpoint # checks before it will issue a tag (services/plm_service.QR_MANDATORY_FIELDS): # approving past it is how a garment reaches the library that can never be # tagged/scanned — the identity the TNA + production counts hang off. The @@ -303,8 +303,8 @@ class ReviewService: Idempotent: an already-confirmed garment returns the same 200 shape as ``confirm-draft`` (``already_confirmed: True``) rather than 404. - Gated on the QR-mandatory garment fields: a draft whose fabric quality / - pattern number / counters are still empty is REFUSED with + Gated on the mandatory garment fields: a draft whose style number / + fabric quality / counters are still empty is REFUSED with ``code=QR_FIELDS_MISSING`` + ``missing[]`` (the router turns it into a 422) instead of being confirmed into the library, where it could never be issued a QR tag. The idempotent branch above is deliberately NOT gated — diff --git a/backend/tests/test_garment_style_number_conflict.py b/backend/tests/test_garment_style_number_conflict.py index 84a1b8d7..59fb2d6c 100644 --- a/backend/tests/test_garment_style_number_conflict.py +++ b/backend/tests/test_garment_style_number_conflict.py @@ -12,8 +12,10 @@ What is asserted here: (the guard must run before any assignment — the session autoflushes a pending UPDATE ahead of the next query, which is what produced the 500); 2. re-submitting the garment's OWN style number still saves; - 3. an empty style number is a 422 INVALID_STYLE_NUMBER, while an explicit - JSON null clears the number (200, no clash check); + 3. an empty style number — blank, whitespace-only or an explicit JSON null — + is a 422 STYLE_NUMBER_REQUIRED (2026-09-28: the number is mandatory, so the + old "null clears the number" path is gone; the row is left unchanged and no + clash check runs); 4. the lookup covers soft-deleted rows too (the DB unique index does); 5. an IntegrityError at commit (the two-reviewers race) is a 409, not a 500, and the session stays usable; @@ -203,10 +205,13 @@ def test_style_number_is_stored_trimmed(make_client, db): # =========================================================================== -# 3. empty style number → 422, row untouched +# 3. empty style number → 422, row untouched (blank / whitespace / JSON null) # =========================================================================== -@pytest.mark.parametrize("value", ["", " ", "\t"]) +@pytest.mark.parametrize("value", ["", " ", "\t", None]) def test_empty_style_number_returns_422(make_client, db, value): + """The style number is MANDATORY (2026-09-28). Blank, whitespace-only and an + explicit JSON null are the same refusal — the null case used to be a silent + 'clear the number' 200, which is how a garment lost its identity.""" _garment(db, "g-a", "SA A0001") r = make_client(_admin()).put("/api/garments/g-a", @@ -214,8 +219,8 @@ def test_empty_style_number_returns_422(make_client, db, value): assert r.status_code == 422, r.text detail = r.json()["detail"] - assert detail["code"] == "INVALID_STYLE_NUMBER" - assert detail["error"] == "Style number cannot be empty" + assert detail["code"] == "STYLE_NUMBER_REQUIRED" + assert "Style number is required" in detail["error"] row = _reload(db, "g-a") assert row.style_number == "SA A0001" assert row.description is None @@ -329,23 +334,28 @@ def test_reservation_leaves_a_free_first_number_alone(db): assert plm_service.peek_next_style_number(db, "shopping", "mens") == "SM0001" -def test_explicit_null_clears_the_style_number(make_client, db): - """JSON null is a legitimate "clear the number" call (62 rows in the dev DB - carry NULL, 0 carry an empty string): 200, column NULL, no clash check.""" +def test_explicit_null_no_longer_clears_the_style_number(make_client, db): + """INVERTED 2026-09-28 (was test_explicit_null_clears_the_style_number). + + The garment edit form cleared the number by sending JSON null — a real path a + user could hit by emptying the field. The number is mandatory now, so the same + call is a 422 and the row keeps its identity; the number is NOT freed.""" _garment(db, "g-a", "SA A0001") _garment(db, "g-b", "SA B0002") r = make_client(_admin()).put("/api/garments/g-a", json={"style_number": None, "description": "number cleared"}) - assert r.status_code == 200, r.text + assert r.status_code == 422, r.text + assert r.json()["detail"]["code"] == "STYLE_NUMBER_REQUIRED" row = _reload(db, "g-a") - assert row.style_number is None - assert row.description == "number cleared" - # The other garment is untouched and the number is free to be reused. + assert row.style_number == "SA A0001" # identity kept + assert row.description is None # nothing else was written either + # The number was never freed, so it still clashes like any other row's. assert _reload(db, "g-b").style_number == "SA B0002" r2 = make_client(_admin()).put("/api/garments/g-b", json={"style_number": "SA A0001"}) - assert r2.status_code == 200, r2.text + assert r2.status_code == 409, r2.text + assert r2.json()["detail"]["code"] == "STYLE_NUMBER_EXISTS" def test_patch_without_style_number_is_not_a_conflict(make_client, db): diff --git a/backend/tests/test_qr_mandatory_fields.py b/backend/tests/test_qr_mandatory_fields.py index 4ae1f32d..a6153862 100644 --- a/backend/tests/test_qr_mandatory_fields.py +++ b/backend/tests/test_qr_mandatory_fields.py @@ -1,14 +1,22 @@ -"""The QR-mandatory field contract — dependency-light on purpose. +"""The mandatory-field contract — dependency-light on purpose. This is the file CI can actually RUN (the curated offline job installs sqlalchemy + pillow and, in the router step, qrcode — and `services/__init__.py` is empty, so importing `services.plm_service` pulls none of the heavy runtime). -What it locks down is the thing that must never drift: the SAME three fields -gate BOTH a QR being issued AND a capture draft being approved out of the Review -Queue. `routers/plm.py:_require` and `services/review_service._approve_draft` -both read `missing_qr_fields()`, so a change to the list, to what counts as -"filled", or to a caller switching to its own copy shows up here. +What it locks down is the thing that must never drift: the SAME fields gate BOTH +a QR being issued AND a capture draft being approved out of the Review Queue. +`routers/plm.py:_require`, `services/review_service._approve_draft` and +`routers/garments.confirm_draft_garment` all read `missing_qr_fields()`, so a +change to the list, to what counts as "filled", or to a caller switching to its +own copy shows up here. + +2026-09-28 rule change (design team): **style number is mandatory, pattern number +is NOT.** The style number is the garment's identity (scan URL, hangtag, every TNA +join); the pattern master is routinely keyed after the physical sample arrives, so +blocking an approval on it stalled otherwise-complete garments. The assertions +below are the regression guard for exactly that: pattern number must never come +back as a reason to refuse. The end-to-end gate behaviour (the 422, the queue payload, the per-field rows) lives in tests/test_review_qr_approval_gate.py, which needs fastapi + chromadb. @@ -26,66 +34,95 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1])) import pytest from services.qr_fields import ( - QR_FIELD_LABELS, + MANDATORY_FIELDS, + MANDATORY_FIELD_LABELS, QR_MANDATORY_FIELDS, + QR_FIELD_LABELS, missing_qr_fields, qr_field_label, ) class _G: - """A stand-in garment: `missing_qr_fields` only reads these three attrs.""" + """A stand-in garment: `missing_qr_fields` only reads these attrs.""" - def __init__(self, fabric_quality=None, pattern_number=None, counters=None): + def __init__(self, style_number: str | None = "SA W0001", fabric_quality=None, + pattern_number=None, counters=None): + self.style_number = style_number self.fabric_quality = fabric_quality self.pattern_number = pattern_number self.counters = counters -def test_the_trio_is_exactly_fabric_quality_pattern_number_counters(): - assert QR_MANDATORY_FIELDS == ("fabric_quality", "pattern_number", "counters") +def test_the_list_is_exactly_style_number_fabric_quality_counters(): + assert MANDATORY_FIELDS == ("style_number", "fabric_quality", "counters") + + +def test_pattern_number_is_not_mandatory(): + """The 2026-09-28 rule: an empty pattern number is NOT a reason to refuse a + garment a QR or an approval — the pattern master is keyed later than the + sample arrives.""" + assert "pattern_number" not in MANDATORY_FIELDS + assert missing_qr_fields(_G(style_number="W1", fabric_quality="Poplin", counters=4)) == [] + # …and it is never named in a refusal, even when every field around it is filled. + assert "pattern_number" not in MANDATORY_FIELD_LABELS + + +def test_style_number_replaced_pattern_number_in_the_alias(): + """`QR_MANDATORY_FIELDS` predates the rename and must stay identical.""" + assert QR_MANDATORY_FIELDS == MANDATORY_FIELDS + assert QR_FIELD_LABELS is MANDATORY_FIELD_LABELS def test_every_field_has_a_human_label(): - for field in QR_MANDATORY_FIELDS: - assert QR_FIELD_LABELS[field].strip() - assert qr_field_label(field) == QR_FIELD_LABELS[field] + for field in MANDATORY_FIELDS: + assert MANDATORY_FIELD_LABELS[field].strip() + assert qr_field_label(field) == MANDATORY_FIELD_LABELS[field] + assert qr_field_label("style_number") == "Style number" # An unknown field degrades to a readable label rather than a KeyError. assert qr_field_label("some_new_field") == "some new field" def test_all_empty_reports_all_three(): - assert missing_qr_fields(_G()) == list(QR_MANDATORY_FIELDS) + assert missing_qr_fields(_G(style_number=None)) == list(MANDATORY_FIELDS) -@pytest.mark.parametrize("field", list(QR_MANDATORY_FIELDS)) +@pytest.mark.parametrize("field", list(MANDATORY_FIELDS)) def test_any_single_empty_field_is_reported(field): - """Two of three is not QR-ready — the gate is all-or-nothing.""" - filled = {"fabric_quality": "Cotton Poplin 120gsm", "pattern_number": "PT-4471", + """Two of three is not ready — the gate is all-or-nothing.""" + filled = {"style_number": "SA W0301", "fabric_quality": "Cotton Poplin 120gsm", "counters": 6} filled[field] = None assert missing_qr_fields(_G(**filled)) == [field] def test_complete_garment_is_qr_ready(): - assert missing_qr_fields(_G("Cotton Poplin 120gsm", "PT-4471", 6)) == [] + assert missing_qr_fields(_G("SA W0301", "Cotton Poplin 120gsm", "PT-4471", 6)) == [] def test_counters_zero_is_a_real_answer(): """0 pieces is filled ("none made yet"); only NULL is missing.""" - assert missing_qr_fields(_G("Denim 12oz", "PT-1", 0)) == [] + assert missing_qr_fields(_G("SA W0301", "Denim 12oz", "PT-1", 0)) == [] def test_blank_and_whitespace_text_is_missing(): """The QR endpoint has always refused a blank quality; the approval gate must - agree, or a garment it lets through is one the tag endpoint still rejects.""" - assert missing_qr_fields(_G(" ", "PT-1", 6)) == ["fabric_quality"] - assert missing_qr_fields(_G("Denim 12oz", "", 6)) == ["pattern_number"] + agree, or a garment it lets through is one the tag endpoint still rejects. + The style number is held to the same test.""" + assert missing_qr_fields(_G(" ", "Cotton Poplin", "PT-1", 6)) == ["style_number"] + assert missing_qr_fields(_G("SA W0301", " ", "PT-1", 6)) == ["fabric_quality"] + + +def test_a_style_less_garment_is_never_qr_ready(): + """A NULL style number reads as missing — this is what stops the garment form + from claiming "QR-ready" for a style it could never print a tag for.""" + assert missing_qr_fields(_G(None, "Cotton Poplin", "PT-1", 6)) == ["style_number"] def test_partial_object_with_the_fields_reads_correctly(): - """A light object that carries the trio (a projection) is judged on its values.""" + """A light object that carries the list (a projection) is judged on its values.""" class _Partial: + style_number = "SA W0301" fabric_quality = "Denim 12oz" pattern_number = "PT-1" counters = 3 @@ -94,13 +131,14 @@ def test_partial_object_with_the_fields_reads_correctly(): def test_object_without_the_attributes_is_treated_as_missing(): - """Deliberate conservative default: a garment whose QR fields cannot be read is - reported as MISSING, never as ready — the gate must not approve on a guess. - (Every caller passes a real Garment row, so this is a guard rail, not a path.)""" + """Deliberate conservative default: a garment whose mandatory fields cannot be + read is reported as MISSING, never as ready — the gate must not approve on a + guess. (Every caller passes a real Garment row, so this is a guard rail, not a + path.)""" class _Bare: - style_number = "SA W0001" + pass - assert missing_qr_fields(_Bare()) == list(QR_MANDATORY_FIELDS) + assert missing_qr_fields(_Bare()) == list(MANDATORY_FIELDS) # --------------------------------------------------------------------------- @@ -114,9 +152,9 @@ def test_qr_refusal_detail_names_every_missing_field(): qr_refusal_detail, ) - class _G: # one of the trio filled, one blank, one null + class _G: # style number blank, quality filled, counters null, pattern empty id = "g-1" - style_number = "SA W0302" + style_number = " " fabric_quality = "Cotton Poplin 120gsm" pattern_number = "" counters = None @@ -124,30 +162,43 @@ def test_qr_refusal_detail_names_every_missing_field(): detail = qr_refusal_detail(_G(), verb="confirm") assert detail["code"] == CODE_QR_FIELDS_MISSING assert detail["error"] == ERR_QR_FIELDS_MISSING - assert detail["missing"] == ["pattern_number", "counters"] - assert detail["missing_labels"] == ["Pattern no.", "Counters (pieces)"] - assert detail["garment_id"] == "g-1" and detail["style_number"] == "SA W0302" + assert detail["missing"] == ["style_number", "counters"] + assert detail["missing_labels"] == ["Style number", "Counters (pieces)"] + assert detail["garment_id"] == "g-1" and detail["style_number"] == " " assert "confirm" in detail["message"] +def test_a_blank_pattern_number_is_never_a_refusal_reason(): + from services.qr_fields import qr_refusal_detail + + class _G: + id, style_number = "g-3", "SA W0304" + fabric_quality = "Cotton Poplin 120gsm" + pattern_number = None + counters = 2 + + assert qr_refusal_detail(_G())["missing"] == [] + + def test_qr_refusal_detail_defaults_to_the_approve_wording(): """The verb only changes for the door that is not the review queue.""" from services.qr_fields import qr_refusal_detail class _G: - id, style_number = "g-2", "SA W0303" + id, style_number = "g-2", None fabric_quality = pattern_number = "" counters = None assert "then approve." in qr_refusal_detail(_G())["message"] -def test_the_shipped_approve_wording_is_unchanged(): - """Users have already seen this sentence on the review queue; the shared - template must keep producing it byte for byte.""" +def test_the_shipped_refusal_wording_names_the_new_fields(): + """Users have seen this sentence on the review queue; it must name the fields + the gate actually reads.""" from services.qr_fields import QR_REFUSAL_MESSAGE assert QR_REFUSAL_MESSAGE.format(verb="approve") == ( - "Fill the QR fields — fabric quality, pattern number and counters — then " - "approve. They are what the printed tag and the phone scan resolve to." + "Fill the mandatory fields — style number, fabric quality and counters — then " + "approve. They are the garment's identity: what the printed tag, the phone scan " + "and every TNA join resolve to." ) diff --git a/backend/tests/test_review_qr_approval_gate.py b/backend/tests/test_review_qr_approval_gate.py index cec24d32..dca2b2c1 100644 --- a/backend/tests/test_review_qr_approval_gate.py +++ b/backend/tests/test_review_qr_approval_gate.py @@ -1,21 +1,26 @@ -"""Review-Queue approval gate on the QR-mandatory garment fields. +"""Review-Queue approval gate on the mandatory garment fields. -Shridhar's rule (2026-09-23): a garment must NOT be approvable out of the Review -Queue while any QR-mandatory field is still empty — fabric quality, pattern -number, counters are what the printed hangtag and the phone scan resolve to, so -a garment approved without them can never be tagged upstream of TNA/production. +Shridhar's rule (2026-09-23, updated 2026-09-28): a garment must NOT be approvable +out of the Review Queue while a mandatory field is still empty. The mandatory set +is **style number, fabric quality, counters** — the garment's identity (scan URL, +hangtag, every TNA join) plus what the printed tag shows. **Pattern number is +optional** since 2026-09-28: it is recorded when the pattern master exists, and an +approval must never wait on it (the sample routinely arrives first). That is the +single most important assertion in this file — see +`test_a_pattern_less_draft_now_approves`. Asserted at the layer that enforces it: - * a capture draft missing ANY of the three is refused with 422 + * a capture draft missing ANY mandatory field is refused with 422 ``code=QR_FIELDS_MISSING`` + ``missing[]``, and stays ``is_draft=True``; * ``counters=0`` is a real answer (not missing); a blank/whitespace string is; - * a QR-complete draft still approves, and re-approving stays idempotent (a - garment confirmed before this rule existed must not start 422-ing); + * a draft with NO pattern number approves — and a QR-complete draft still + approves, with re-approving idempotent (a garment confirmed before this rule + existed must not start 422-ing); * per-field AI-tag approvals are UNAFFECTED — they leave the garment where it is, so they are not the "garment approval" the gate is about; * the queue payload carries ``qr_missing`` so the row can warn before the click; * the gate and the QR endpoint report the SAME missing fields (one shared list - in services/plm_service.py) — a garment the gate lets through is one + in services/qr_fields.py) — a garment the gate lets through is one ``POST /api/plm/{id}/qr`` will happily tag. Harness mirrors tests/test_capture_jobs_autosave.py (scratch file-backed SQLite + @@ -40,8 +45,8 @@ import pytest # deliberately excludes (see requirements-test.txt). Skip the whole module where # they are absent instead of ERRORing the run (the convention # tests/test_soft_delete_restore.py uses for its router cases). The -# dependency-light half of the contract — the shared QR field list and what -# counts as filled — lives in tests/test_qr_mandatory_fields.py, which CI DOES run. +# dependency-light half of the contract — the shared field list and what counts +# as filled — lives in tests/test_qr_mandatory_fields.py, which CI DOES run. pytest.importorskip("fastapi", reason="the gate tests drive the app over TestClient") pytest.importorskip("chromadb", reason="review_service imports EmbeddingService (chromadb)") @@ -64,11 +69,13 @@ import models.moodboard # noqa: F401 import models.feed_interaction # noqa: F401 import models.feed_post # noqa: F401 from services.garment_capture_preview_store import GarmentCapturePreview # noqa: F401 -from models.garment import Garment, GarmentTag -from models.rbac import Role +from models.garment import Garment, GarmentTag # noqa: F401 +from models.rbac import Role # noqa: F401 from models.review_queue import ReviewQueueItem from models.user import User -from services.plm_service import QR_MANDATORY_FIELDS, missing_qr_fields +from services.plm_service import MANDATORY_FIELDS, QR_MANDATORY_FIELDS, missing_qr_fields + +assert MANDATORY_FIELDS == QR_MANDATORY_FIELDS # the deprecated alias is the same list # --------------------------------------------------------------------------- @@ -126,14 +133,15 @@ def _user(role: str = "Admin", email: str | None = None, uid: str = "usr-1", role=role, department_id=department_id, is_active=True, password_hash="x") +# Deliberately WITHOUT a pattern number: the fixtures prove the new rule. QR_COMPLETE = {"fabric_quality": "Cotton Poplin 120gsm", - "pattern_number": "PT-4471", + "pattern_number": None, "counters": 12} -def _draft(db, gid: str = "g-qr-draft", style: str = "SA W0900", is_draft: bool = True, - **overrides) -> Garment: - """A capture draft whose QR fields default to EMPTY (the blocked case).""" +def _draft(db, gid: str = "g-qr-draft", style: str | None = "SA W0900", + is_draft: bool = True, **overrides) -> Garment: + """A capture draft whose mandatory fields default to EMPTY (the blocked case).""" cols = {"fabric_quality": None, "pattern_number": None, "counters": None} cols.update(overrides) g = Garment(id=gid, style_number=style, category="mens", season="AW26", @@ -153,19 +161,19 @@ def _approve(client, gid: str): # --------------------------------------------------------------------------- # The gate # --------------------------------------------------------------------------- -def test_qr_incomplete_draft_is_refused_and_stays_draft(make_client, db): - _draft(db) +def test_incomplete_draft_is_refused_and_stays_draft(make_client, db): + _draft(db, style=None) # no style number, no quality, no counters r = _approve(make_client(_user()), "g-qr-draft") assert r.status_code == 422, r.text detail = r.json()["detail"] assert detail["code"] == "QR_FIELDS_MISSING" - assert detail["missing"] == list(QR_MANDATORY_FIELDS) - assert detail["missing_labels"] == ["Fabric quality", "Pattern no.", "Counters (pieces)"] + assert detail["missing"] == list(MANDATORY_FIELDS) + assert detail["missing_labels"] == ["Style number", "Fabric quality", "Counters (pieces)"] assert detail["garment_id"] == "g-qr-draft" - assert detail["style_number"] == "SA W0900" + assert detail["style_number"] is None # The reviewer is given something to show, not a bare status code. - assert "QR fields" in detail["message"] + assert "mandatory fields" in detail["message"] # Nothing was confirmed — the draft is still a draft and still in the queue. g = db.query(Garment).filter(Garment.id == "g-qr-draft").first() @@ -177,12 +185,34 @@ def test_qr_incomplete_draft_is_refused_and_stays_draft(make_client, db): assert [i["id"] for i in queue["data"]] == ["draft:g-qr-draft"] -@pytest.mark.parametrize("field", list(QR_MANDATORY_FIELDS)) -def test_every_qr_field_individually_blocks_approval(make_client, db, field): +def test_a_pattern_less_draft_now_approves(make_client, db): + """THE 2026-09-28 promise: an empty pattern number does not hold an approval. + + Before the change this draft was refused with `missing=["pattern_number"]`; + the pattern master is keyed after the sample arrives, so the garment sat in + the queue unable to move.""" + _draft(db, gid="g-no-pattern", style="SA W1100", fabric_quality="Cotton Poplin 120gsm", + pattern_number=None, counters=4) + + r = _approve(make_client(_user()), "g-no-pattern") + + assert r.status_code == 200, r.text + assert r.json()["already_confirmed"] is False + g = db.query(Garment).filter(Garment.id == "g-no-pattern").first() + db.refresh(g) + assert g.is_draft is False + # …and the pattern number is still simply absent — never invented, never required. + assert g.pattern_number is None + + +@pytest.mark.parametrize("field", list(MANDATORY_FIELDS)) +def test_every_mandatory_field_individually_blocks_approval(make_client, db, field): """Filling two of three is not enough — each field alone refuses the approval.""" - cols = dict(QR_COMPLETE) - cols[field] = None - _draft(db, gid=f"g-{field}", style=f"SA W1{field[:3]}", **cols) + cols = dict(QR_COMPLETE) # fabric_quality + counters + cols["style_number"] = f"SA W1{field[:3]}" + cols[field] = None # …the one field under test is empty + style = cols.pop("style_number") + _draft(db, gid=f"g-{field}", style=style, **cols) r = _approve(make_client(_user()), f"g-{field}") @@ -195,14 +225,26 @@ def test_every_qr_field_individually_blocks_approval(make_client, db, field): def test_blank_and_whitespace_text_fields_count_as_missing(make_client, db): """A whitespace-only value is not a fabric quality — the QR endpoint always - refused it, so the gate must too (same helper).""" + refused it, so the gate must too (same helper). A blank style number is held + to the same test.""" _draft(db, gid="g-blank", style="SA W1001", fabric_quality=" ", pattern_number="", counters=4) - r = _approve(make_client(_user()), "g-blank") - assert r.status_code == 422, r.text - assert r.json()["detail"]["missing"] == ["fabric_quality", "pattern_number"] + assert r.json()["detail"]["missing"] == ["fabric_quality"] + + _draft(db, gid="g-blank-style", style=" ", fabric_quality="Denim 12oz", counters=4) + r2 = _approve(make_client(_user()), "g-blank-style") + assert r2.status_code == 422, r2.text + assert r2.json()["detail"]["missing"] == ["style_number"] + + +def test_blank_pattern_number_is_not_missing_at_all(make_client, db): + _draft(db, gid="g-pattern-blank", style="SA W1101", fabric_quality="Denim 12oz", + pattern_number=" ", counters=4) + r = _approve(make_client(_user()), "g-pattern-blank" + ) + assert r.status_code == 200, r.text def test_counters_zero_is_a_real_answer(make_client, db): @@ -230,7 +272,7 @@ def test_qr_complete_draft_approves_and_repeat_approve_is_idempotent(make_client assert g.review_status == "human_verified" # Re-approving an ALREADY confirmed garment stays a 200 even though this one - # has no QR fields: the gate is on the confirmation, not on the re-read. + # has no mandatory fields: the gate is on the confirmation, not on the re-read. _draft(db, gid="g-old", style="SA W1004", is_draft=False) ok = _approve(make_client(_user()), "g-old") assert ok.status_code == 200, ok.text @@ -247,12 +289,21 @@ def test_queue_payload_exposes_qr_readiness(make_client, db): by_id = {i["id"]: i for i in data} partial = by_id["draft:g-partial"] - assert partial["qr_missing"] == ["pattern_number", "counters"] - assert partial["qr_missing_labels"] == ["Pattern no.", "Counters (pieces)"] - assert partial["qr_required"] == list(QR_MANDATORY_FIELDS) + assert partial["qr_missing"] == ["counters"] + assert partial["qr_missing_labels"] == ["Counters (pieces)"] + assert partial["qr_required"] == list(MANDATORY_FIELDS) assert by_id["draft:g-ready"]["qr_missing"] == [] +def test_queue_payload_names_a_missing_style_number(make_client, db): + _draft(db, gid="g-no-style", style=None, fabric_quality="Cotton Poplin 120gsm", + counters=4) + data = make_client(_user()).get("/api/review/queue").json()["data"] + mine = [i for i in data if i["id"] == "draft:g-no-style"][0] + assert mine["qr_missing"] == ["style_number"] + assert mine["qr_missing_labels"] == ["Style number"] + + def test_per_field_approval_is_not_gated(make_client, db): """Per-field AI-tag rows leave the garment where it is, so they keep approving normally — only the garment confirmation is gated (Shridhar's call).""" @@ -281,6 +332,9 @@ def test_draft_correct_shares_the_gate(make_client, db): assert r.status_code == 422, r.text assert r.json()["detail"]["code"] == "QR_FIELDS_MISSING" + # The style number is present on this fixture, so the refusal names the two + # fields that are actually empty. + assert r.json()["detail"]["missing"] == ["fabric_quality", "counters"] g = db.query(Garment).filter(Garment.id == "g-corr").first() db.refresh(g) assert g.is_draft is True @@ -288,8 +342,7 @@ def test_draft_correct_shares_the_gate(make_client, db): def test_gate_and_qr_endpoint_agree_on_what_is_missing(make_client, db): """One shared list: the fields the queue gate names are the fields the QR - endpoint refuses to issue a tag without (style number is QR-only, so it may - not appear in the gate's list).""" + endpoint refuses to issue a tag without.""" _draft(db, gid="g-agree", style="SA W1009", fabric_quality="Cotton Poplin 120gsm") client = make_client(_user()) @@ -298,15 +351,30 @@ def test_gate_and_qr_endpoint_agree_on_what_is_missing(make_client, db): assert qr.status_code == 422, qr.text qr_missing = qr.json()["detail"]["missing"] - assert approve_missing == ["pattern_number", "counters"] - assert qr_missing == approve_missing # style number IS present → nothing dropped + assert approve_missing == ["counters"] # pattern number is not a reason + assert qr_missing == approve_missing # style number IS present → nothing dropped assert set(approve_missing) <= set(qr_missing) +def test_qr_endpoint_refuses_a_style_less_garment_once(make_client, db): + """The style number is what the tag prints, so the endpoint names it exactly + once (regression guard: it used to be inserted as a special case AND read from + the shared list, which double-listed it).""" + _draft(db, gid="g-no-style-qr", style=None, fabric_quality="Cotton Poplin 120gsm", + counters=4) + + r = make_client(_user()).post("/api/plm/g-no-style-qr/qr") + + assert r.status_code == 422, r.text + assert r.json()["detail"]["missing"] == ["style_number"] + + def test_missing_qr_fields_helper_is_the_single_source(make_client, db): """The helper is what both callers read, so lock its contract down here.""" g = _draft(db, gid="g-helper", style="SA W1010", counters=3) - assert missing_qr_fields(g) == ["fabric_quality", "pattern_number"] + assert missing_qr_fields(g) == ["fabric_quality"] g.fabric_quality = "Cotton Poplin 120gsm" g.pattern_number = "PT-9" assert missing_qr_fields(g) == [] + g.style_number = None + assert missing_qr_fields(g) == ["style_number"] diff --git a/docs/README.md b/docs/README.md index 98ff0ba9..782532bb 100644 --- a/docs/README.md +++ b/docs/README.md @@ -20,7 +20,7 @@ An AI platform for a garment export house, built around **two pillars**: taxonomy with a confidence score per field. ≥85% auto-saves, 65–85% goes to review, <65% is flagged. - **Auto style-numbering** — on upload, a gap-safe style number is reserved per **segment** (new-dev / shopping) × **category** (mens / womens / kids), e.g. `M0001`, `W0002`, `SM0001`; colourways use a shared base + `-A/-B`. -- **QR hangtag** — once a style has fabric quality + pattern no. + counters, a **frozen** QR is generated and +- **QR hangtag** — once a style has style number + fabric quality + counters, a **frozen** QR is generated and stored; a printable hangtag shows **Style No. + Fabric Quality** with everything else behind the scan. - **QR scan landing** (`/g/:style`, public) — scanning resolves to the full garment detail view. - **Semantic + intent search**, **faceted filters**, **Boards/Collections** (personal + public "playlists"), diff --git a/docs/plans/plm-portal-features.md b/docs/plans/plm-portal-features.md index 14b8b31a..ae4b5181 100644 --- a/docs/plans/plm-portal-features.md +++ b/docs/plans/plm-portal-features.md @@ -33,7 +33,7 @@ sample-dev date, selected-by, location…). The **physical hangtag shows only St Quality** (what a buyer checks); everything else lives behind the scan. QR/counters drive **sample tracking** (where each piece is: Bangalore / returned / in-house). -- Mandatory before QR: **fabric quality, pattern number, counters** (style number is auto). +- Mandatory before QR: **style number, fabric quality, counters** — **pattern number is optional** (recorded when the pattern master exists; it no longer blocks the tag or an approval). - **Counters** = how many physical pieces of the style exist. - ⚠️ **Open:** exact numbering convention (prefix/format) — implemented configurable, pending the team's confirmed scheme. diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 9df059c1..6eb1eaec 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -476,14 +476,14 @@ export interface ReviewItem { * answer rather than a low-confidence model reading. Optional — absent means * normal classification. */ capture_degraded?: boolean; - /** Capture drafts only: QR-mandatory garment fields still EMPTY, so the + /** Capture drafts only: mandatory garment fields still EMPTY, so the * garment cannot be approved yet ([] ⇒ approvable + taggable). Names come * from the backend's shared QR field list, so the row can warn before the * reviewer clicks Approve. Absent on a backend that predates the gate. */ qr_missing?: string[]; /** Human labels for `qr_missing` ("Pattern no.", "Counters (pieces)"). */ qr_missing_labels?: string[]; - /** The full QR-mandatory field list, so the row can say what IS required. */ + /** The full mandatory field list, so the row can say what IS required. */ qr_required?: string[]; } @@ -667,7 +667,8 @@ export async function nextStyleNumber(segment: string, category: string, reserve return res.data; } -// Freeze a QR for a garment (needs fabric quality + pattern no. + counters). 422 lists what's missing. +// Freeze a QR for a garment (needs style number + fabric quality + counters — pattern +// number is OPTIONAL since 2026-09-28). 422 lists what's missing. export async function generateQr(garmentId: string, reissue = false): Promise<{ style_number: string; qr_payload: string; qr_generated_at: string | null; already_existed: boolean }> { const res = await api.post(`/plm/${garmentId}/qr`, null, { params: { reissue } }); return res.data; @@ -1316,7 +1317,7 @@ export async function getReviewQueue(): Promise { * it. Optional: absent on a backend that predates the field. * * Throws the server's 422 `QR_FIELDS_MISSING` when a capture draft's - * QR-mandatory fields are still empty (nothing is committed) — read it with + * mandatory fields are still empty (nothing is committed) — read it with * `qrApprovalBlock(e)` in lib/qrFields to name the fields to the reviewer. */ export async function approveReviewItem(itemId: string): Promise<{ possible_duplicates?: BulkDuplicateMatch[] }> { const res = await api.put(`/review/${itemId}/approve`); diff --git a/frontend/src/components/BulkGarmentUpload.tsx b/frontend/src/components/BulkGarmentUpload.tsx index 2b8a14c4..1db10cd2 100644 --- a/frontend/src/components/BulkGarmentUpload.tsx +++ b/frontend/src/components/BulkGarmentUpload.tsx @@ -77,32 +77,36 @@ interface GroupFields { isCoord: boolean; } -/** QR readiness of one capture group's metadata — the same trio the Review Queue +/** QR readiness of one capture group's metadata — the same fields the Review Queue * refuses to approve a garment without (lib/qrFields mirrors the backend). The * capture itself is never blocked; this is what tells the uploader, while the - * sample's hangtag is still in hand, why a later approval would stop. */ + * sample's hangtag is still in hand, why a later approval would stop. + * + * `styleNumberAuto`: this flow reserves the group's style number server-side at + * SAVE time, so it is never the uploader's job to type one — passing the flag + * keeps the note from accusing the form of a field it does not own. */ function qrMissingFor(fields: GroupFields): string[] { return missingQrFields({ fabric_quality: fields.fabricQuality, pattern_number: fields.patternNumber, counters: fields.counters.trim() === '' ? null : Number(fields.counters), - }); + }, { styleNumberAuto: true }); } -/** '· QR' marker on a QR field's label, amber + 'required' while it is empty. */ +/** '· QR' marker on a mandatory field's label, amber + 'required' while it is empty. */ function QrMark({ missing, field }: { readonly missing: string[]; readonly field: string }) { return missing.includes(field) ? · QR · required : · QR; } -/** The readiness line under the QR trio of one group. */ +/** The readiness line under the mandatory fields of one group. */ function QrReadinessNote({ gid, fields }: { readonly gid: number; readonly fields: GroupFields }) { const missing = qrMissingFor(fields); if (missing.length === 0) { return (

- QR-ready — quality · pattern · counters filled + QR-ready — quality · counters filled

); } @@ -1741,7 +1745,7 @@ export default function BulkGarmentUpload() {
- + toggleGroupField(g.id, 'patternNumber', e.target.value)} data-testid={`bk-pattern-${g.id}`} />
diff --git a/frontend/src/components/ReviewAiDetailsModal.tsx b/frontend/src/components/ReviewAiDetailsModal.tsx index e33da10c..53852b7f 100644 --- a/frontend/src/components/ReviewAiDetailsModal.tsx +++ b/frontend/src/components/ReviewAiDetailsModal.tsx @@ -5,7 +5,7 @@ import Dropdown from './shared/Dropdown'; import CloseButton from './shared/CloseButton'; import ColourPaletteInput from './shared/ColourPaletteInput'; import { useToast } from '../context/ToastContext'; -import { QR_FIELDS, missingQrFields, qrMissingSentence } from '../lib/qrFields'; +import { QR_FIELDS, QR_OPTIONAL_FIELDS, missingQrFields, qrMissingSentence } from '../lib/qrFields'; import { AI_FIELD_ORDER, prettify, aiInputCls, aiLabelCls, parseColourValue, serializeColour, type ColourEntry, @@ -35,8 +35,9 @@ interface UploadTags { rejected?: Record; } -/** Per-field input config for the QR trio (the row order comes from QR_FIELDS). */ +/** Per-field input config for the mandatory fields (the row order comes from QR_FIELDS). */ const QR_INPUT: Record = { + style_number: { placeholder: 'W1234' }, fabric_quality: { placeholder: 'Cotton Poplin 120gsm' }, pattern_number: { placeholder: 'PTN-4471' }, counters: { type: 'number', placeholder: '3' }, @@ -105,6 +106,7 @@ export default function ReviewAiDetailsModal({ result, previews, onClose }: Revi .then((g) => { if (!alive) return; const next = { + style_number: g.style_number ?? '', fabric_quality: g.fabric_quality ?? '', pattern_number: g.pattern_number ?? '', counters: g.counters != null ? String(g.counters) : '', @@ -121,7 +123,7 @@ export default function ReviewAiDetailsModal({ result, previews, onClose }: Revi counters: (qr.counters ?? '').trim() === '' ? null : Number(qr.counters), }) : []; const qrDirty = qr !== null && qrInitial !== null - && QR_FIELDS.some((f) => (qr[f.key] ?? '') !== (qrInitial[f.key] ?? '')); + && [...QR_FIELDS, ...QR_OPTIONAL_FIELDS].some((f) => (qr[f.key] ?? '') !== (qrInitial[f.key] ?? '')); // Ordered field list: canonical taxonomy first, then any extra keys returned. // `colour` is excluded — it renders as the palette editor below, not a text input. @@ -153,7 +155,12 @@ export default function ReviewAiDetailsModal({ result, previews, onClose }: Revi counters = parsed; } const cleared = (v: string) => (v === '' ? null : v); + const styleNumber = (qr.style_number ?? '').trim(); await updateGarment(result.garment_id, { + // The style number is mandatory: only send it when it actually carries a + // value, so an untouched field never trips the server's 422 + // STYLE_NUMBER_REQUIRED — the readiness note below states the gap instead. + ...(styleNumber ? { style_number: styleNumber } : {}), fabric_quality: cleared(qr.fabric_quality ?? ''), pattern_number: cleared(qr.pattern_number ?? ''), counters, @@ -231,7 +238,7 @@ export default function ReviewAiDetailsModal({ result, previews, onClose }: Revi without them can never be approved out of the queue. */} {qr !== null && (
- QR fields · required for approval + Mandatory fields · required for approval
{QR_FIELDS.map((f) => (
@@ -253,6 +260,24 @@ export default function ReviewAiDetailsModal({ result, previews, onClose }: Revi
))}
+ {/* Pattern number is recorded when it exists but never blocks an + approval (2026-09-28) — still editable here, while the sample's + hangtag is in hand, just no longer part of the gate. */} + {QR_OPTIONAL_FIELDS.map((f) => ( +
+ + setQr((prev) => (prev ? { ...prev, [f.key]: e.target.value } : prev))} + data-testid={`qr-field-${f.key}`} + /> +
+ ))} {qrMissing.length > 0 ? (

Review approval blocked · {qrMissingSentence(qrMissing)} still empty. The garment saves and reaches the queue either way — it just can’t be approved until these are filled. diff --git a/frontend/src/components/library/GarmentDetailModal.tsx b/frontend/src/components/library/GarmentDetailModal.tsx index 543ad145..cc72518b 100644 --- a/frontend/src/components/library/GarmentDetailModal.tsx +++ b/frontend/src/components/library/GarmentDetailModal.tsx @@ -273,9 +273,19 @@ export default function GarmentDetailModal({ g, onClose, onSave, onOpenGarment, async function save() { if (!form) return; + // The style number is MANDATORY (2026-09-28). It is the garment's identity — + // the QR scan URL, the hangtag and every TNA join key off it — so the form + // refuses to save without one instead of sending the null that used to clear + // it (the server rejects an empty/null number with 422 STYLE_NUMBER_REQUIRED + // too; this is the same rule stated before the request leaves the browser). + const styleNumber = (form.style_number ?? '').trim(); + if (!styleNumber) { + setErr('Style number is required — it is the garment’s identity (tag, scan, TNA).'); + return; + } // Validate before any request goes out: Number('12 pcs') is NaN, and // JSON.stringify turns NaN into null, which would silently CLEAR a - // QR-mandatory field while the form reports a successful save (HG-105). + // mandatory field while the form reports a successful save (HG-105). const countersRaw = (form.counters ?? '').trim(); let counters: number | null = null; if (countersRaw !== '') { @@ -293,7 +303,7 @@ export default function GarmentDetailModal({ g, onClose, onSave, onOpenGarment, // successful (HG-106). Selects can't be emptied, so they keep "unchanged". const cleared = (v: string) => (v === '' ? null : v); await updateGarment(g.id, { - style_number: cleared(form.style_number), + style_number: styleNumber, // never null/blank — validated above season: cleared(form.season), design_team_city: cleared(form.design_team_city), collection_name: cleared(form.collection_name), @@ -524,7 +534,7 @@ export default function GarmentDetailModal({ g, onClose, onSave, onOpenGarment,

Edit details

- + @@ -557,7 +567,7 @@ export default function GarmentDetailModal({ g, onClose, onSave, onOpenGarment, /> - + {/* Why an approval (and the QR tag itself) will be refused — said diff --git a/frontend/src/lib/qrFields.ts b/frontend/src/lib/qrFields.ts index bb02413c..1d22ed35 100644 --- a/frontend/src/lib/qrFields.ts +++ b/frontend/src/lib/qrFields.ts @@ -1,19 +1,23 @@ /** - * QR-mandatory garment fields — the three fields a garment needs before it can - * be issued a QR tag AND before a capture draft can be approved out of the - * Review Queue. + * The mandatory garment fields — what a garment needs before it can be issued a + * QR tag AND before a capture draft can be approved out of the Review Queue. * - * ⚠️ These are the SAME three fields the backend enforces - * (`backend/services/plm_service.py`: `QR_MANDATORY_FIELDS` / - * `missing_qr_fields()`), and the queue gate reads them from there. The two - * lists must stay in step: the backend is the authority (it refuses the write), - * this copy is what lets the UI name the fields BEFORE the reviewer clicks and - * highlight them in the forms. + * ⚠️ These are the SAME fields the backend enforces + * (`backend/services/qr_fields.py`: `MANDATORY_FIELDS` / `missing_qr_fields()`), + * and the queue gate reads them from there. The two lists must stay in step: the + * backend is the authority (it refuses the write), this copy is what lets the UI + * name the fields BEFORE the reviewer clicks and highlight them in the forms. * * Why the rule exists (design-team rule, Notes/Features.md): the QR is the - * garment's identity — quality + pattern number + counters are what the printed - * hangtag shows and what the phone scan resolves to, so a garment that reaches - * the library without them can never be tagged or counted upstream. + * garment's identity. The STYLE NUMBER is that identity (scan URL, hangtag, every + * TNA join) — it is mandatory and can no longer be cleared — and fabric quality + + * counters are what the printed tag shows and what the phone scan resolves to. + * + * PATTERN NO. IS NOT MANDATORY (2026-09-28, design team): it is recorded when it + * exists and stays editable everywhere, but the sample routinely reaches the + * studio before the pattern master is keyed, and blocking on it stalled garments + * that were otherwise complete. It lives in `QR_OPTIONAL_FIELDS` — still rendered, + * never required, never named in a refusal. */ export interface QrFieldDef { @@ -26,11 +30,19 @@ export interface QrFieldDef { } export const QR_FIELDS: readonly QrFieldDef[] = [ + { key: 'style_number', label: 'Style number', short: 'Style number' }, { key: 'fabric_quality', label: 'Fabric quality', short: 'Fabric quality' }, - { key: 'pattern_number', label: 'Pattern no.', short: 'Pattern no.' }, { key: 'counters', label: 'Counters (pieces)', short: 'Counters' }, ]; +/** + * Editable in the same forms, but NEVER required — an empty one is not a reason + * to refuse an approval and must not be reported as "missing". + */ +export const QR_OPTIONAL_FIELDS: readonly QrFieldDef[] = [ + { key: 'pattern_number', label: 'Pattern no.', short: 'Pattern no.' }, +]; + /** The label for one field key, tolerating anything the server sends back. */ export function qrFieldLabel(key: string): string { return QR_FIELDS.find((f) => f.key === key)?.label ?? key.replace(/_/g, ' '); @@ -43,6 +55,7 @@ export function qrFieldShort(key: string): string { /** The garment fields the readiness check reads (all optional: a partial * garment object from a list endpoint is honest about what it has). */ export interface QrFieldsInput { + readonly style_number?: string | null; readonly fabric_quality?: string | null; readonly pattern_number?: string | null; readonly counters?: number | string | null; @@ -52,12 +65,19 @@ export interface QrFieldsInput { * Mirror of the backend's `missing_qr_fields()`. * * `counters` is an Integer column: 0 is a real answer ("none made yet"), only a - * null/blank one is missing. The two text columns count as missing when blank — - * the same test the QR endpoint has always applied. + * null/blank one is missing. The text columns (style number, fabric quality) + * count as missing when blank — the same test the QR endpoint has always applied. + * + * `styleNumberAuto`: the capture/bulk flows reserve the style number at SAVE time, + * so the number cannot be empty in a form that is still being filled in. Those + * callers pass this flag instead of being told their style number is missing. */ -export function missingQrFields(g: QrFieldsInput): string[] { +export function missingQrFields(g: QrFieldsInput, opts?: { + readonly styleNumberAuto?: boolean; +}): string[] { const missing: string[] = []; for (const { key } of QR_FIELDS) { + if (key === 'style_number' && opts?.styleNumberAuto) continue; const v = g[key as keyof QrFieldsInput]; if (v == null) missing.push(key); else if (typeof v === 'string' && !v.trim()) missing.push(key); @@ -65,7 +85,7 @@ export function missingQrFields(g: QrFieldsInput): string[] { return missing; } -/** "Pattern no. and Counters" / "Fabric quality, Pattern no. and Counters". */ +/** "Style number and Counters" / "Style number, Fabric quality and Counters". */ export function qrMissingSentence(missing: readonly string[]): string { const names = missing.map(qrFieldShort); if (names.length === 0) return '';