From 91b6e7441e33e2219844d7bfea71ad46b00fbc13 Mon Sep 17 00:00:00 2001 From: Abby Bigaouette Date: Thu, 11 Jun 2026 22:02:39 -0700 Subject: [PATCH 1/4] Part number variants: optional {variant} token, derived families MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an optional {variant} placeholder to the part numbering format (default 3-digit code, width configurable in the wizard). New parts mint variant 001; POST /api/parts/{id}/variants creates a draft sibling with the next code, copying attributes and BOM. Families are derived from the PN itself — one fact, one home — so no schema change; variant codes are never reused and the tier sequence counter never advances for variants. Part page gains a VARIANT action and a VAR ledger row, both absent when the format mints no variants. Co-Authored-By: Claude Opus 4.7 --- src/opal/api/routes/parts.py | 74 +++++++++++- src/opal/api/routes/project.py | 3 + src/opal/core/numbering.py | 41 +++++++ src/opal/project.py | 9 +- src/opal/web/routes.py | 33 +++++ src/opal/web/templates/parts/_ledger_is.html | 21 ++++ src/opal/web/templates/parts/detail.html | 19 +++ src/opal/web/templates/project/wizard.html | 18 ++- tests/unit/test_numbering.py | 121 ++++++++++++++++++- tests/unit/test_parts.py | 92 ++++++++++++++ tests/unit/test_project_config_db.py | 17 +++ 11 files changed, 439 insertions(+), 9 deletions(-) diff --git a/src/opal/api/routes/parts.py b/src/opal/api/routes/parts.py index d44863a..6337631 100644 --- a/src/opal/api/routes/parts.py +++ b/src/opal/api/routes/parts.py @@ -16,7 +16,9 @@ from opal.core.audit import get_model_dict, log_create, log_delete, log_update from opal.core.numbering import ( PartNumberError, + format_has_variant, next_part_number, + next_variant_part_number, peek_next_part_number, pn_exists, register_part_number, @@ -30,7 +32,7 @@ ensure_identity_mutable, reference_counts, ) -from opal.db.models import InventoryRecord, Part, Supplier, SupplierPart +from opal.db.models import BOMLine, InventoryRecord, Part, Supplier, SupplierPart router = APIRouter() @@ -310,6 +312,76 @@ def create_part( return get_part_with_quantity(db, part) +@router.post( + "/{part_id}/variants", response_model=PartResponse, status_code=status.HTTP_201_CREATED +) +def create_part_variant( + db: DbSession, + part_id: int, + user_id: CurrentUserId, +) -> PartResponse: + """Mint the next variant of a part: a draft sibling copying attributes and BOM. + + The variant shares the source's tier+sequence base with the next variant + code; the tier sequence counter does not advance. + """ + part = db.query(Part).filter(Part.id == part_id, Part.deleted_at.is_(None)).first() + if not part: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail=f"Part {part_id} not found" + ) + if not format_has_variant(get_active_project()): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Part numbering format has no {variant} placeholder", + ) + if not part.internal_pn: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="Part has no internal part number, so its variant family cannot be derived", + ) + + try: + new_pn = next_variant_part_number(db, part.tier, part.internal_pn) + except PartNumberError as e: + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) from e + + variant = Part( + name=part.name, + internal_pn=new_pn, + external_pn=part.external_pn, + description=part.description, + category=part.category, + unit_of_measure=part.unit_of_measure, + tracking_type=part.tracking_type, + tier=part.tier, + parent_id=part.parent_id, + reorder_point=part.reorder_point, + is_tooling=part.is_tooling, + calibration_interval_days=part.calibration_interval_days, + metadata_=dict(part.metadata_) if part.metadata_ else None, + ) + db.add(variant) + db.flush() + log_create(db, variant, user_id) + + for line in part.bom_lines: + bom_copy = BOMLine( + assembly_id=variant.id, + component_id=line.component_id, + quantity=line.quantity, + reference_designator=line.reference_designator, + notes=line.notes, + ) + db.add(bom_copy) + db.flush() + log_create(db, bom_copy, user_id) + + db.commit() + db.refresh(variant) + return get_part_with_quantity(db, variant) + + class NextPnResponse(BaseModel): """Preview of the next part number for a tier.""" diff --git a/src/opal/api/routes/project.py b/src/opal/api/routes/project.py index 3572fc5..95dcb8c 100644 --- a/src/opal/api/routes/project.py +++ b/src/opal/api/routes/project.py @@ -35,6 +35,7 @@ class PartNumberingInput(BaseModel): prefix: str = "" separator: str = "-" sequence_digits: int = 4 + variant_digits: int = 3 format: str = "{prefix}{sep}{tier_code}{sep}{sequence}" @@ -98,6 +99,7 @@ def from_config(cls, config: ProjectConfig) -> "ProjectConfigResponse": prefix=config.part_numbering.prefix, separator=config.part_numbering.separator, sequence_digits=config.part_numbering.sequence_digits, + variant_digits=config.part_numbering.variant_digits, format=config.part_numbering.format, ), categories=config.categories, @@ -125,6 +127,7 @@ def _numbering_from_input(pn: PartNumberingInput) -> PartNumberingConfig: prefix=pn.prefix, separator=pn.separator, sequence_digits=pn.sequence_digits, + variant_digits=pn.variant_digits, format=pn.format, ) diff --git a/src/opal/core/numbering.py b/src/opal/core/numbering.py index 47984a2..5f6d305 100644 --- a/src/opal/core/numbering.py +++ b/src/opal/core/numbering.py @@ -62,6 +62,8 @@ def part_number_regex(project: ProjectConfig | None, tier_level: int) -> re.Patt token = match.group(1) if token == "sequence": pattern += rf"(?P\d{{{project.part_numbering.sequence_digits},}})" + elif token == "variant": + pattern += rf"(?P\d{{{project.part_numbering.variant_digits},}})" elif token in values: pattern += re.escape(values[token]) else: @@ -127,6 +129,45 @@ def next_part_number(db: Session, tier_level: int) -> str: return pn +def format_has_variant(project: ProjectConfig | None) -> bool: + """True when the numbering format mints variant codes.""" + return project is not None and "{variant}" in project.part_numbering.format + + +def next_variant_part_number(db: Session, tier_level: int, source_pn: str) -> str: + """Mint the next variant of an existing part number. + + Variants share the source's tier+sequence base; the next code is + max(existing)+1 across ALL rows including soft-deleted ones — variant + codes are never reused. The tier sequence counter is not touched: + a variant is a sibling, not a new sequence. + """ + from opal.config import get_active_project + + project = get_active_project() + if project is None or not format_has_variant(project): + raise PartNumberError("Part numbering format has no {variant} placeholder") + + regex = part_number_regex(project, tier_level) + match = regex.match(source_pn) + if not match: + example = _format_part_number(project, tier_level, 1) + raise PartNumberError( + f"'{source_pn}' does not match the tier-{tier_level} part number format " + f"(e.g. {example}), so its variant family cannot be derived" + ) + sequence = int(match.group("sequence")) + + # Full-column scan + regex match: correct for arbitrary token order and + # fine at single-instance SQLite scale. + max_variant = 0 + for (pn,) in db.query(Part.internal_pn).filter(Part.internal_pn.isnot(None)).all(): + m = regex.match(pn) + if m and int(m.group("sequence")) == sequence: + max_variant = max(max_variant, int(m.group("variant"))) + return project.generate_part_number(tier_level, sequence, max_variant + 1) + + def peek_next_part_number(db: Session, tier_level: int) -> tuple[str, int]: """Preview the next part number for a tier without consuming it.""" from opal.config import get_active_project diff --git a/src/opal/project.py b/src/opal/project.py index abdd7ff..0b125db 100644 --- a/src/opal/project.py +++ b/src/opal/project.py @@ -22,6 +22,7 @@ class PartNumberingConfig(BaseModel): prefix: str = "" separator: str = "-" sequence_digits: int = 4 + variant_digits: int = 3 format: str = "{prefix}{sep}{tier_code}{sep}{sequence}" @@ -140,12 +141,13 @@ def get_requirement(self, req_id: str) -> RequirementConfig | None: return req return None - def generate_part_number(self, tier_level: int, sequence: int) -> str: + def generate_part_number(self, tier_level: int, sequence: int, variant: int = 1) -> str: """Generate a part number according to project config. Args: tier_level: The tier level (1, 2, 3, etc.) sequence: The sequence number for this part. + variant: The variant code, used only when the format includes {variant}. Returns: Formatted part number string. @@ -164,6 +166,7 @@ def generate_part_number(self, tier_level: int, sequence: int) -> str: tier_name=tier.name, tier_level=tier.level, sequence=str(sequence).zfill(self.part_numbering.sequence_digits), + variant=str(variant).zfill(self.part_numbering.variant_digits), ) @@ -280,6 +283,7 @@ def create_project_config( prefix: str = "", separator: str = "-", sequence_digits: int = 4, + variant_digits: int = 3, part_number_format: str = "{prefix}{sep}{tier_code}{sep}{sequence}", tiers: list[TierConfig] | None = None, requirements: list[RequirementConfig] | None = None, @@ -295,6 +299,7 @@ def create_project_config( prefix: Part number prefix. separator: Part number separator (default: "-"). sequence_digits: Number of digits in sequence (default: 4). + variant_digits: Number of digits in the variant code (default: 3). part_number_format: Format string for part numbers. tiers: List of inventory tiers (defaults to Flight/Ground/Loose). requirements: List of project requirements. @@ -315,6 +320,7 @@ def create_project_config( prefix=prefix, separator=separator, sequence_digits=sequence_digits, + variant_digits=variant_digits, format=part_number_format, ), requirements=requirements or [], @@ -358,6 +364,7 @@ def save_project_config(config: ProjectConfig) -> None: "prefix": config.part_numbering.prefix, "separator": config.part_numbering.separator, "sequence_digits": config.part_numbering.sequence_digits, + "variant_digits": config.part_numbering.variant_digits, "format": config.part_numbering.format, }, "requirements": [ diff --git a/src/opal/web/routes.py b/src/opal/web/routes.py index d129ef8..d380bf9 100644 --- a/src/opal/web/routes.py +++ b/src/opal/web/routes.py @@ -956,6 +956,39 @@ def _parts_detail_response( where_used = db.query(BOMLine).filter(BOMLine.component_id == part.id).all() context["where_used"] = where_used + # Variants: live siblings sharing this PN's tier+sequence base, derived + # from the PN itself — no stored relation + from opal.core.numbering import format_has_variant, part_number_regex + + can_variant = False + variant_rows: list[Part] = [] + if format_has_variant(project) and part.internal_pn and tier_config: + regex = part_number_regex(project, part.tier) + match = regex.match(part.internal_pn) + if match: + can_variant = True + sequence = int(match.group("sequence")) + siblings = ( + db.query(Part) + .filter( + Part.deleted_at.is_(None), + Part.id != part.id, + Part.tier == part.tier, + Part.internal_pn.isnot(None), + ) + .all() + ) + variant_rows = sorted( + ( + sib + for sib in siblings + if (m := regex.match(sib.internal_pn)) and int(m.group("sequence")) == sequence + ), + key=lambda p: p.internal_pn, + ) + context["can_variant"] = can_variant + context["variant_rows"] = variant_rows + # Test templates from opal.db.models.inventory import TestTemplate diff --git a/src/opal/web/templates/parts/_ledger_is.html b/src/opal/web/templates/parts/_ledger_is.html index d361b84..248af9d 100644 --- a/src/opal/web/templates/parts/_ledger_is.html +++ b/src/opal/web/templates/parts/_ledger_is.html @@ -26,6 +26,27 @@ {% endif %} +{# --- Variants: live siblings derived from the PN base; rendered only when + the numbering format mints variant codes --- #} +{% if can_variant %} +
+ VAR + {{ variant_rows | length if variant_rows else '—' }} + [+] +
+{% if variant_rows %} +
+ {% for v in variant_rows %} +
+ {{ v.internal_pn }} + {{ v.name }} + {{ v.lifecycle_state | upper }} +
+ {% endfor %} +
+{% endif %} +{% endif %} + {# --- Requirements: counts carry state inline, tersely --- #} {% set req_rows = part_requirements | selectattr('req') | list %} {% set tbd_count = req_rows | selectattr('req.tbd') | list | length %} diff --git a/src/opal/web/templates/parts/detail.html b/src/opal/web/templates/parts/detail.html index 896ef64..ae69039 100644 --- a/src/opal/web/templates/parts/detail.html +++ b/src/opal/web/templates/parts/detail.html @@ -33,6 +33,9 @@
{{ ok.btn("PRINT", attrs='onclick="window.open(\'/label?type=part&id=' ~ part.id ~ '\', \'_blank\', \'width=600,height=350\')"') }} {{ ok.btn("EDIT", attrs='onclick="openPartForm()"') }} + {% if can_variant %} + {{ ok.btn("VARIANT", attrs='onclick="createVariant()"') }} + {% endif %} {% if is_draft %} {{ ok.btn("ACTIVATE", variant="primary", attrs='onclick="openActivateConfirm()"') }} {% if not part_is_referenced %} @@ -303,6 +306,22 @@ } catch (e) { alert('Network error: ' + e.message); } } +// Variant — mints a draft sibling (next variant code) copying attributes + BOM +async function createVariant() { + try { + const r = await fetch(`/api/parts/${partId}/variants`, { + method: 'POST', headers: getHeaders(), + }); + if (r.ok) { + const part = await r.json(); + window.location = `/parts/${part.id}`; + } else { + const err = await r.json(); + alert(formatApiError(err.detail, 'Failed to create variant')); + } + } catch (e) { alert('Network error: ' + e.message); } +} + // Requirement allocation (assign here; the dossier is read + unlink only) async function assignRequirement() { const input = document.getElementById('assign-req-number'); diff --git a/src/opal/web/templates/project/wizard.html b/src/opal/web/templates/project/wizard.html index 46054ec..228c97b 100644 --- a/src/opal/web/templates/project/wizard.html +++ b/src/opal/web/templates/project/wizard.html @@ -74,7 +74,7 @@

3. PART Configure how part numbers are generated. The tier code is automatically included.

-
+
3. PART
+ +
+ + +
@@ -108,7 +117,7 @@

3. PART value="{{ existing_config.part_numbering.format if existing_config else '{prefix}{sep}{tier_code}{sep}{sequence}' }}" oninput="updatePnPreview()"> - Variables: {prefix}, {sep}, {tier_code}, {tier_name}, {tier_level}, {sequence} + Variables: {prefix}, {sep}, {tier_code}, {tier_name}, {tier_level}, {sequence}, {variant}

@@ -258,6 +267,7 @@

4. CATE const prefix = document.getElementById('pn-prefix').value || ''; const sep = document.getElementById('pn-separator').value; const digits = parseInt(document.getElementById('pn-digits').value); + const variantDigits = parseInt(document.getElementById('pn-variant-digits').value); const format = document.getElementById('pn-format').value; const tiers = getTiers(); @@ -274,7 +284,8 @@

4. CATE .replaceAll('{tier_code}', tier.code || '?') .replaceAll('{tier_name}', tier.name || '?') .replaceAll('{tier_level}', tier.level) - .replaceAll('{sequence}', sequence); + .replaceAll('{sequence}', sequence) + .replaceAll('{variant}', '1'.padStart(variantDigits, '0')); return `${pn} (${tier.name || 'Tier ' + tier.level})`; }); @@ -303,6 +314,7 @@

4. CATE prefix: document.getElementById('pn-prefix').value, separator: document.getElementById('pn-separator').value, sequence_digits: parseInt(document.getElementById('pn-digits').value), + variant_digits: parseInt(document.getElementById('pn-variant-digits').value), format: document.getElementById('pn-format').value, }, categories: getCategories(), diff --git a/tests/unit/test_numbering.py b/tests/unit/test_numbering.py index d4f47db..dc92d40 100644 --- a/tests/unit/test_numbering.py +++ b/tests/unit/test_numbering.py @@ -1,14 +1,28 @@ """Part number generation and identity lifecycle tests. -No project config is loaded under test, so numbers use the fallback -format PN-{tier}-{seq:04d}. Counters never roll back: soft-deleted and -abandoned numbers stay consumed forever. +No project config is loaded under test (the variant tests activate one +explicitly), so numbers use the fallback format PN-{tier}-{seq:04d}. +Counters never roll back: soft-deleted and abandoned numbers stay +consumed forever. """ from datetime import UTC, datetime -from opal.core.numbering import next_part_number, peek_next_part_number, pn_exists +import pytest + +import opal.config as config_mod +from opal.core.numbering import ( + PartNumberError, + format_has_variant, + next_part_number, + next_variant_part_number, + part_number_regex, + peek_next_part_number, + pn_exists, + validate_part_number, +) from opal.db.models import Part +from opal.project import PartNumberingConfig, ProjectConfig, TierConfig def _create(client, name: str, tier: int = 1, **extra) -> dict: @@ -117,6 +131,105 @@ def test_reserve_count_zero_rejected(client): assert response.status_code == 400 +# ============ Variants ============ + + +def _variant_project(**numbering_overrides) -> ProjectConfig: + numbering = { + "prefix": "RV", + "separator": "-", + "sequence_digits": 4, + "variant_digits": 3, + "format": "{prefix}{sep}{tier_code}{sep}{sequence}{sep}{variant}", + } | numbering_overrides + return ProjectConfig( + name="Variant Project", + tiers=[TierConfig(level=1, name="Flight", code="F")], + part_numbering=PartNumberingConfig(**numbering), + ) + + +@pytest.fixture +def variant_project(monkeypatch) -> ProjectConfig: + project = _variant_project() + monkeypatch.setattr(config_mod, "_active_project", project) + return project + + +def test_format_has_variant(): + assert not format_has_variant(None) + no_variant = _variant_project(format="{prefix}{sep}{tier_code}{sep}{sequence}") + assert not format_has_variant(no_variant) + assert format_has_variant(_variant_project()) + + +def test_variant_generation_and_regex_round_trip(variant_project): + assert variant_project.generate_part_number(1, 7) == "RV-F-0007-001" + assert variant_project.generate_part_number(1, 7, 12) == "RV-F-0007-012" + + match = part_number_regex(variant_project, 1).match("RV-F-0007-012") + assert match + assert match.group("sequence") == "0007" + assert match.group("variant") == "012" + assert validate_part_number(variant_project, 1, "RV-F-0007-012") == 7 + + +def test_new_parts_get_variant_001(db_session, variant_project): + assert next_part_number(db_session, 1) == "RV-F-0001-001" + assert next_part_number(db_session, 1) == "RV-F-0002-001" + + +def test_next_variant_increments_and_never_recycles(db_session, variant_project): + db_session.add(Part(name="Base", internal_pn="RV-F-0001-001", tier=1)) + db_session.flush() + assert next_variant_part_number(db_session, 1, "RV-F-0001-001") == "RV-F-0001-002" + + # Soft-deleted variants keep their codes consumed + db_session.add( + Part(name="Dead", internal_pn="RV-F-0001-002", tier=1, deleted_at=datetime.now(UTC)) + ) + db_session.flush() + assert next_variant_part_number(db_session, 1, "RV-F-0001-001") == "RV-F-0001-003" + + # Next code is max+1, and any family member is a valid source + db_session.add(Part(name="Five", internal_pn="RV-F-0001-005", tier=1)) + db_session.flush() + assert next_variant_part_number(db_session, 1, "RV-F-0001-005") == "RV-F-0001-006" + + +def test_variant_creation_never_advances_sequence_counter(db_session, variant_project): + base_pn = next_part_number(db_session, 1) + db_session.add(Part(name="Base", internal_pn=base_pn, tier=1)) + db_session.flush() + + next_variant_part_number(db_session, 1, base_pn) + assert peek_next_part_number(db_session, 1) == ("RV-F-0002-001", 2) + + +def test_next_variant_requires_variant_format(db_session, monkeypatch): + monkeypatch.setattr( + config_mod, + "_active_project", + _variant_project(format="{prefix}{sep}{tier_code}{sep}{sequence}"), + ) + with pytest.raises(PartNumberError): + next_variant_part_number(db_session, 1, "RV-F-0001") + + +def test_next_variant_rejects_pn_from_older_format(db_session, variant_project): + # Numbered before {variant} entered the format: family underivable + with pytest.raises(PartNumberError): + next_variant_part_number(db_session, 1, "RV-F-0001") + + +def test_variant_override_bumps_sequence_counter(client, variant_project): + override = _create(client, "Overridden", internal_pn="RV-F-0500-004") + assert override["internal_pn"] == "RV-F-0500-004" + + auto = _create(client, "After Override") + assert auto["internal_pn"] == "RV-F-0501-001" + + def test_draft_tier_change_regenerates_pn_and_abandons_old(client): draft = _create(client, "Mover") abandoned_pn = draft["internal_pn"] diff --git a/tests/unit/test_parts.py b/tests/unit/test_parts.py index 5dd0a20..82ed307 100644 --- a/tests/unit/test_parts.py +++ b/tests/unit/test_parts.py @@ -1,5 +1,11 @@ """Parts API tests.""" +import pytest + +import opal.config as config_mod +from opal.db.models import BOMLine, Part +from opal.project import PartNumberingConfig, ProjectConfig, TierConfig + def test_create_part(client): """Test creating a new part.""" @@ -154,3 +160,89 @@ def test_is_tooling_honored_on_every_tier(client): "/api/parts", json={"name": "Bench Meter", "tier": 3, "is_tooling": True} ).json() assert loose["is_tooling"] is True + + +# ============ Variants ============ + + +@pytest.fixture +def variant_project(monkeypatch) -> ProjectConfig: + project = ProjectConfig( + name="Variant Project", + tiers=[TierConfig(level=1, name="Flight", code="F")], + part_numbering=PartNumberingConfig( + prefix="RV", + format="{prefix}{sep}{tier_code}{sep}{sequence}{sep}{variant}", + ), + ) + monkeypatch.setattr(config_mod, "_active_project", project) + return project + + +def test_create_variant_copies_attributes_and_bom(client, db_session, variant_project): + component = client.post("/api/parts", json={"name": "Bolt", "tier": 1}).json() + source = client.post( + "/api/parts", + json={ + "name": "Bracket Assembly", + "tier": 1, + "category": "Structures", + "description": "Primary bracket", + "external_pn": "EXT-77", + }, + ).json() + assert source["internal_pn"] == "RV-F-0002-001" + + db_session.add( + BOMLine( + assembly_id=source["id"], + component_id=component["id"], + quantity=4, + reference_designator="B1-B4", + ) + ) + db_session.commit() + + response = client.post(f"/api/parts/{source['id']}/variants") + assert response.status_code == 201, response.text + variant = response.json() + + assert variant["internal_pn"] == "RV-F-0002-002" + assert variant["id"] != source["id"] + assert variant["name"] == "Bracket Assembly" + assert variant["category"] == "Structures" + assert variant["description"] == "Primary bracket" + assert variant["external_pn"] == "EXT-77" + assert variant["lifecycle_state"] == "draft" + + copied = db_session.query(BOMLine).filter(BOMLine.assembly_id == variant["id"]).all() + assert [(line.component_id, line.quantity, line.reference_designator) for line in copied] == [ + (component["id"], 4, "B1-B4") + ] + + # The tier sequence counter did not advance: the next new part takes 0003 + after = client.post("/api/parts", json={"name": "Next New", "tier": 1}).json() + assert after["internal_pn"] == "RV-F-0003-001" + + +def test_create_variant_rejected_without_variant_format(client): + # Fallback numbering (no project config) has no {variant} placeholder + source = client.post("/api/parts", json={"name": "Plain", "tier": 1}).json() + response = client.post(f"/api/parts/{source['id']}/variants") + assert response.status_code == 400 + assert "{variant}" in response.json()["detail"] + + +def test_create_variant_unknown_part_404(client, variant_project): + response = client.post("/api/parts/99999/variants") + assert response.status_code == 404 + + +def test_create_variant_rejects_pn_from_older_format(client, db_session, variant_project): + legacy = Part(name="Legacy", internal_pn="RV-F-0001", tier=1) + db_session.add(legacy) + db_session.commit() + + response = client.post(f"/api/parts/{legacy.id}/variants") + assert response.status_code == 422 + assert "RV-F-0001" in response.json()["detail"] diff --git a/tests/unit/test_project_config_db.py b/tests/unit/test_project_config_db.py index 0dfb84a..c3990f2 100644 --- a/tests/unit/test_project_config_db.py +++ b/tests/unit/test_project_config_db.py @@ -42,6 +42,23 @@ def test_save_and_load_round_trip(db_session): assert config_mod.get_active_project() is loaded +def test_legacy_blob_without_variant_digits_defaults_to_3(db_session): + """Blobs persisted before variant support load with the field defaulted.""" + from opal.config import set_app_setting + + legacy_blob = ( + '{"name": "Legacy", "tiers": [{"level": 1, "name": "FLIGHT", "code": "F"}], ' + '"part_numbering": {"prefix": "TST", "separator": "-", "sequence_digits": 4, ' + '"format": "{prefix}{sep}{tier_code}{sep}{sequence}"}}' + ) + set_app_setting(db_session, PROJECT_CONFIG_KEY, legacy_blob) + db_session.flush() + + loaded = load_project_from_db(db_session) + assert loaded is not None + assert loaded.part_numbering.variant_digits == 3 + + def test_load_returns_none_when_absent(db_session): assert load_project_from_db(db_session) is None From 415ed6c3005ff3cbb511c9579c1912fa4796e15f Mon Sep 17 00:00:00 2001 From: Abby Bigaouette Date: Thu, 11 Jun 2026 23:25:56 -0700 Subject: [PATCH 2/4] PN-canonical part URLs + variant-mode creation form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /parts/{pn} is now the canonical detail URL; id URLs 302-redirect to it (parts whose PN holds a '/' or shadows a literal sibling route stay id-addressed). VARIANT now opens the shared part form in a third mode — identity rendered as locked facts (next code, source tier), every other field prefilled from the source and editable — via the /parts/{ref}/variant deep link; the variants endpoint accepts an optional override body (omitted fields copy, explicit nulls clear, identity never overridable). BOM still copies server-side and is edited on the new draft's page. Co-Authored-By: Claude Opus 4.7 --- src/opal/api/routes/parts.py | 67 ++++++++++-- src/opal/web/routes.py | 109 +++++++++++++++---- src/opal/web/templates/parts/_ledger_is.html | 2 +- src/opal/web/templates/parts/_part_form.html | 88 ++++++++++----- src/opal/web/templates/parts/detail.html | 18 +-- tests/unit/test_part_urls.py | 96 ++++++++++++++++ tests/unit/test_parts.py | 48 ++++++++ 7 files changed, 352 insertions(+), 76 deletions(-) create mode 100644 tests/unit/test_part_urls.py diff --git a/src/opal/api/routes/parts.py b/src/opal/api/routes/parts.py index 6337631..3a51edb 100644 --- a/src/opal/api/routes/parts.py +++ b/src/opal/api/routes/parts.py @@ -75,6 +75,26 @@ class PartUpdate(BaseModel): metadata: dict[str, Any] | None = None +class PartVariantCreate(BaseModel): + """Overrides for a new variant; omitted fields copy from the source part. + + Identity (tier, internal_pn) is derived from the source and never + overridable — a variant is a sibling, not a new sequence. + """ + + name: str | None = None + description: str | None = None + category: str | None = None + unit_of_measure: str | None = None + tracking_type: str | None = None # "bulk" or "serialized" + external_pn: str | None = None + reorder_point: Decimal | None = None + is_tooling: bool | None = None + calibration_interval_days: int | None = None + parent_id: int | None = None + metadata: dict[str, Any] | None = None + + class PartResponse(BaseModel): """Schema for part response.""" @@ -319,11 +339,13 @@ def create_part_variant( db: DbSession, part_id: int, user_id: CurrentUserId, + variant_in: PartVariantCreate | None = None, ) -> PartResponse: """Mint the next variant of a part: a draft sibling copying attributes and BOM. The variant shares the source's tier+sequence base with the next variant - code; the tier sequence counter does not advance. + code; the tier sequence counter does not advance. Body fields override + the copied attributes; omitted fields copy, explicit nulls clear. """ part = db.query(Part).filter(Part.id == part_id, Part.deleted_at.is_(None)).first() if not part: @@ -341,25 +363,46 @@ def create_part_variant( detail="Part has no internal part number, so its variant family cannot be derived", ) + overrides = variant_in.model_dump(exclude_unset=True) if variant_in else {} + parent_id = overrides.get("parent_id", part.parent_id) + if parent_id is not None and parent_id != part.parent_id: + parent = db.query(Part).filter(Part.id == parent_id, Part.deleted_at.is_(None)).first() + if not parent: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Parent part {parent_id} not found", + ) + try: new_pn = next_variant_part_number(db, part.tier, part.internal_pn) except PartNumberError as e: raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(e)) from e + if "metadata" in overrides: + metadata = overrides["metadata"] + else: + metadata = dict(part.metadata_) if part.metadata_ else None + variant = Part( - name=part.name, + # name is NOT NULL: a null/empty override falls back to the source name + name=overrides.get("name") or part.name, internal_pn=new_pn, - external_pn=part.external_pn, - description=part.description, - category=part.category, - unit_of_measure=part.unit_of_measure, - tracking_type=part.tracking_type, + external_pn=overrides.get("external_pn", part.external_pn), + description=overrides.get("description", part.description), + category=overrides.get("category", part.category), + unit_of_measure=overrides.get("unit_of_measure") or part.unit_of_measure, + tracking_type=overrides.get("tracking_type") or part.tracking_type, tier=part.tier, - parent_id=part.parent_id, - reorder_point=part.reorder_point, - is_tooling=part.is_tooling, - calibration_interval_days=part.calibration_interval_days, - metadata_=dict(part.metadata_) if part.metadata_ else None, + parent_id=parent_id, + reorder_point=overrides.get("reorder_point", part.reorder_point), + # NOT NULL: a null override means "no opinion", not "clear" + is_tooling=( + part.is_tooling if overrides.get("is_tooling") is None else overrides["is_tooling"] + ), + calibration_interval_days=overrides.get( + "calibration_interval_days", part.calibration_interval_days + ), + metadata_=metadata, ) db.add(variant) db.flush() diff --git a/src/opal/web/routes.py b/src/opal/web/routes.py index d380bf9..f5e16a9 100644 --- a/src/opal/web/routes.py +++ b/src/opal/web/routes.py @@ -7,6 +7,7 @@ from datetime import UTC, date, datetime, timedelta from pathlib import Path from typing import Any +from urllib.parse import quote from fastapi import APIRouter, Form, Query, Request from fastapi.responses import HTMLResponse, RedirectResponse @@ -853,9 +854,7 @@ def parts_import(request: Request, db: DbSession) -> HTMLResponse: @router.get("/parts/new", response_class=HTMLResponse) -def parts_new( - request: Request, db: DbSession, parent_id: int | None = Query(None) -) -> HTMLResponse: +def parts_new(request: Request, db: DbSession, parent_id: int | None = Query(None)) -> HTMLResponse: """Deep link: the parts list with the create overlay open. The form never asks what the invoking context already knows — @@ -874,18 +873,14 @@ def parts_new( def _parts_detail_response( - request: Request, db: DbSession, part_id: int, edit_open: bool = False + request: Request, + db: DbSession, + part: Part, + edit_open: bool = False, + variant_pn: str | None = None, ) -> HTMLResponse: from opal.config import get_active_project - part = db.query(Part).filter(Part.id == part_id, Part.deleted_at.is_(None)).first() - if not part: - return templates.TemplateResponse( - "errors/404.html", - {"request": request, "message": f"Part {part_id} not found"}, - status_code=404, - ) - # The PN is the page's name; the DB row id appears nowhere context = get_base_context(request, db, f"{part.internal_pn or part.name} - OPAL") context["part"] = part @@ -899,7 +894,7 @@ def _parts_detail_response( ) # Get inventory records - inventory_records = db.query(InventoryRecord).filter(InventoryRecord.part_id == part_id).all() + inventory_records = db.query(InventoryRecord).filter(InventoryRecord.part_id == part.id).all() context["inventory_records"] = inventory_records # Calculate total quantity for display @@ -1050,19 +1045,93 @@ def _parts_detail_response( context["categories"] = sorted(category_set) context["form_open"] = edit_open + if variant_pn is not None: + # The shared form renders in variant mode: next code as a locked fact + context["form_variant"] = True + context["variant_pn"] = variant_pn + context["variant_pn_segments"] = _pn_segments( + variant_pn, tier_config.code if tier_config else str(part.tier) + ) + context["form_open"] = True + return templates.TemplateResponse("parts/detail.html", context) -@router.get("/parts/{part_id}", response_class=HTMLResponse) -def parts_detail(request: Request, db: DbSession, part_id: int) -> HTMLResponse: - """Part detail page; hosts the edit overlay.""" - return _parts_detail_response(request, db, part_id) +def _resolve_part_ref(db: DbSession, ref: str) -> Part | None: + """Resolve /parts/{ref}: exact PN match first, then numeric id. + + PN-first means a purely numeric part number wins over an id collision. + """ + part = db.query(Part).filter(Part.internal_pn == ref, Part.deleted_at.is_(None)).first() + if part is None and ref.isdigit(): + part = db.query(Part).filter(Part.id == int(ref), Part.deleted_at.is_(None)).first() + return part + + +# PNs the literal sibling routes would shadow; those parts stay id-addressed +_PN_SHADOWED_BY_ROUTES = {"new", "table", "search", "import"} -@router.get("/parts/{part_id}/edit", response_class=HTMLResponse) -def parts_edit(request: Request, db: DbSession, part_id: int) -> HTMLResponse: +def _canonical_part_redirect(part: Part, ref: str, suffix: str = "") -> RedirectResponse | None: + """302 an id-addressed request to the canonical PN URL. + + No redirect when the part has no PN, when the PN can't live in a single + path segment ('/'), or when a literal sibling route shadows it. + """ + pn = part.internal_pn + if not pn or pn == ref or "/" in pn or pn in _PN_SHADOWED_BY_ROUTES: + return None + return RedirectResponse(url=f"/parts/{quote(pn, safe='')}{suffix}", status_code=302) + + +def _parts_404(request: Request, ref: str) -> HTMLResponse: + return templates.TemplateResponse( + "errors/404.html", + {"request": request, "message": f"Part {ref} not found"}, + status_code=404, + ) + + +@router.get("/parts/{part_ref}", response_class=HTMLResponse, response_model=None) +def parts_detail(request: Request, db: DbSession, part_ref: str) -> HTMLResponse | RedirectResponse: + """Part detail page; hosts the edit overlay. The PN is the canonical URL.""" + part = _resolve_part_ref(db, part_ref) + if not part: + return _parts_404(request, part_ref) + if redirect := _canonical_part_redirect(part, part_ref): + return redirect + return _parts_detail_response(request, db, part) + + +@router.get("/parts/{part_ref}/edit", response_class=HTMLResponse, response_model=None) +def parts_edit(request: Request, db: DbSession, part_ref: str) -> HTMLResponse | RedirectResponse: """Deep link: the part page with the edit overlay open.""" - return _parts_detail_response(request, db, part_id, edit_open=True) + part = _resolve_part_ref(db, part_ref) + if not part: + return _parts_404(request, part_ref) + if redirect := _canonical_part_redirect(part, part_ref, "/edit"): + return redirect + return _parts_detail_response(request, db, part, edit_open=True) + + +@router.get("/parts/{part_ref}/variant", response_class=HTMLResponse, response_model=None) +def parts_new_variant( + request: Request, db: DbSession, part_ref: str +) -> HTMLResponse | RedirectResponse: + """Deep link: the part page with the variant form open (next code minted on save).""" + from opal.core.numbering import PartNumberError, next_variant_part_number + + part = _resolve_part_ref(db, part_ref) + if not part: + return _parts_404(request, part_ref) + if redirect := _canonical_part_redirect(part, part_ref, "/variant"): + return redirect + try: + variant_pn = next_variant_part_number(db, part.tier, part.internal_pn or "") + except PartNumberError: + # Format mints no variants (or the PN predates it): bounce to the page + return RedirectResponse(url=f"/parts/{quote(part_ref, safe='')}", status_code=302) + return _parts_detail_response(request, db, part, variant_pn=variant_pn) # ============ INVENTORY ============ diff --git a/src/opal/web/templates/parts/_ledger_is.html b/src/opal/web/templates/parts/_ledger_is.html index 248af9d..424985e 100644 --- a/src/opal/web/templates/parts/_ledger_is.html +++ b/src/opal/web/templates/parts/_ledger_is.html @@ -32,7 +32,7 @@
VAR {{ variant_rows | length if variant_rows else '—' }} - [+] + [+]
{% if variant_rows %}
diff --git a/src/opal/web/templates/parts/_part_form.html b/src/opal/web/templates/parts/_part_form.html index d6b372a..15c0c16 100644 --- a/src/opal/web/templates/parts/_part_form.html +++ b/src/opal/web/templates/parts/_part_form.html @@ -1,23 +1,36 @@ -{# The one part form — create and edit share it (title + submit differ). - Rendered as an overlay on the page the user is already on: no - intermediate menus. Deep links (/parts/new, /parts/{id}/edit) render - the underlying page with this overlay open. +{# The one part form — create, edit, and variant share it (title + submit + differ). Rendered as an overlay on the page the user is already on: no + intermediate menus. Deep links (/parts/new, /parts/{ref}/edit, + /parts/{ref}/variant) render the underlying page with this overlay open. Context: form_part (Part | undefined -> create mode), tiers, categories, form_prefill (dict, create mode), form_open (bool), - tier_name/pn_segments (edit mode, for the tag). #} + tier_name/pn_segments (edit mode, for the tag). Variant mode: + form_variant (bool) + variant_pn/variant_pn_segments — form_part is the + SOURCE part; identity (PN, tier) is a locked fact, the rest prefills. #} {% import 'opalkit/_macros.html' as ok %} {% from 'parts/_tag.html' import part_tag %} -{% set fm_edit = form_part is defined and form_part %} +{% set fm_variant = form_variant is defined and form_variant %} +{% set fm_edit = form_part is defined and form_part and not fm_variant %} {% set fm_draft = fm_edit and form_part.lifecycle_state == 'draft' %} +{% set fm_filled = fm_edit or fm_variant %} {% set form_prefill = form_prefill if form_prefill is defined else {} %}
-
{{ 'EDIT ' ~ form_part.internal_pn if fm_edit else 'NEW PART' }}
+
{{ 'VARIANT ' ~ variant_pn if fm_variant else ('EDIT ' ~ form_part.internal_pn if fm_edit else 'NEW PART') }}
- {% if fm_edit %} + {% if fm_variant %} + {{ part_tag('forming', + pn=variant_pn, + name=form_part.name, + tier_level=form_part.tier, + tier_name=tier_name, + uom=form_part.unit_of_measure, + tracking=form_part.tracking_type.value if form_part.tracking_type else '', + pn_segments=variant_pn_segments) }} + {% elif fm_edit %} {{ part_tag('header', pn=form_part.internal_pn, name=form_part.name, @@ -34,7 +47,22 @@ {% endif %}
- {% if not fm_edit or fm_draft %} + {% if fm_variant %} + {# Identity is derived from the source — PN and tier live in the tag only #} +
+
+ + +
+
+ + +
+
+ {% elif not fm_edit or fm_draft %} {# Identity rows — absent (not disabled) once active #}
@@ -89,7 +117,7 @@
+ value="{{ form_part.category or '' if fm_filled else (form_prefill.get('category') or '') }}"> {% for cat in categories %} @@ -97,32 +125,32 @@
+ value="{{ form_part.unit_of_measure or 'EA' if fm_filled else 'EA' }}">
+ value="{{ '%g' % form_part.reorder_point if fm_filled and form_part.reorder_point is not none else '' }}">
+ value="{{ form_part.calibration_interval_days or '' if fm_filled else '' }}">
@@ -131,12 +159,12 @@
+ value="{{ form_part.parent_id or '' if fm_filled else (form_prefill.get('parent_id') or '') }}">
- +
- +
{{ ok.btn("CANCEL", attrs='onclick="closePartForm()"') }} - {{ ok.btn("SAVE" if fm_edit else "CREATE DRAFT", variant="primary", type="submit") }} + {{ ok.btn("SAVE" if fm_edit else ("CREATE VARIANT" if fm_variant else "CREATE DRAFT"), variant="primary", type="submit") }}
@@ -166,15 +194,16 @@