diff --git a/docs/superpowers/plans/2026-06-11-pcb-supplier-attachment.md b/docs/superpowers/plans/2026-06-11-pcb-supplier-attachment.md new file mode 100644 index 0000000..27fc9d5 --- /dev/null +++ b/docs/superpowers/plans/2026-06-11-pcb-supplier-attachment.md @@ -0,0 +1,562 @@ +# PCB & SMT Stencil Supplier Attachment — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `bom_export.py` attach a `SupplierPart` (default JLCPCB) to every newly-created PCB and SMT Stencil Part — so re-order workflows in InvenTree just work. + +**Architecture:** Three localized changes in `scripts/bom_export.py`: (1) a new `_ensure_fab_supplier_part` helper that idempotently links a Part to a Supplier with a derived SKU, (2) `create_pcb_part` and `create_stencil_part` gain a keyword-only `fab_supplier=None` argument and call the helper after Part create/reuse, (3) `main()` adds `--pcb-supplier` / `--no-pcb-supplier` CLI flags, resolves the supplier via `get_or_create_supplier`, and threads it through. `create_assembly_part` stays unchanged. + +**Tech Stack:** Python 3.13, `inventree==0.23.1`, pytest. + +**Spec:** `docs/superpowers/specs/2026-06-11-pcb-supplier-attachment-design.md` + +**Working directory for all commands:** `/home/pbuchegger/OE5XRX/HW-Module-CI` + +--- + +## Conventions + +- Tests use `sys.path.insert(0, str(Path(__file__).resolve().parents[1]))`. +- Activate venv before pytest: `source .venv/bin/activate && pytest `. +- Commit messages: `feat(bom-export): ` or `test(bom-export): `. +- TDD strict: failing test → confirm fail → implement → confirm pass → commit. + +--- + +## File Structure + +| File | Change | +|---|---| +| `scripts/bom_export.py` | New `_ensure_fab_supplier_part` helper; `create_pcb_part` / `create_stencil_part` accept `fab_supplier`; new constant `DEFAULT_PCB_SUPPLIER_NAME`; CLI flags `--pcb-supplier` / `--no-pcb-supplier`; `main()` resolves supplier and threads it through | +| `scripts/tests/test_bom_export_fab_supplier.py` | New file with 10 unit tests covering helper + create-function behaviour | + +--- + +## Task 1: `_ensure_fab_supplier_part` helper + +**Files:** +- Modify: `scripts/bom_export.py` (add helper near other Part-creation helpers, around line 240) +- Test: `scripts/tests/test_bom_export_fab_supplier.py` (new file with 5 helper tests) + +- [ ] **Step 1: Write 5 failing tests** + +Create `scripts/tests/test_bom_export_fab_supplier.py`: + +```python +"""Unit tests for bom_export.py's fab-supplier-attachment helpers.""" +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import MagicMock, patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from bom_export import _ensure_fab_supplier_part # noqa: E402 + + +def _part(pk=100, name="v0.1 BusBoard PCB"): + p = MagicMock() + p.pk = pk + p.name = name + return p + + +def _supplier(pk=381, name="JLCPCB"): + s = MagicMock() + s.pk = pk + s.name = name + return s + + +def _supplier_part_mock(pk, sku): + sp = MagicMock() + sp.pk = pk + sp.SKU = sku + return sp + + +def test_ensure_fab_supplier_part_creates_when_missing(): + """No existing SupplierPart for (part, supplier) → create new.""" + api = MagicMock() + part = _part(pk=1291) + supplier = _supplier(pk=381) + sku = "v0.1 BusBoard PCB rev 0.1" + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [] + _ensure_fab_supplier_part(api, part, supplier, sku) + + SP.list.assert_called_once_with(api, part=1291, supplier=381) + SP.create.assert_called_once() + payload = SP.create.call_args[0][1] + assert payload["part"] == 1291 + assert payload["supplier"] == 381 + assert payload["SKU"] == sku + + +def test_ensure_fab_supplier_part_skips_when_existing(): + """Matching SKU already linked → no new SupplierPart created.""" + api = MagicMock() + part = _part(pk=1291) + supplier = _supplier(pk=381) + sku = "v0.1 BusBoard PCB rev 0.1" + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [_supplier_part_mock(pk=500, sku=sku)] + _ensure_fab_supplier_part(api, part, supplier, sku) + + SP.create.assert_not_called() + + +def test_ensure_fab_supplier_part_creates_when_existing_different_sku(): + """SP exists but for a different SKU (= different design rev) → create.""" + api = MagicMock() + part = _part(pk=1291) + supplier = _supplier(pk=381) + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [ + _supplier_part_mock(pk=500, sku="OLD-DESIGN-PCB rev 0.0") + ] + _ensure_fab_supplier_part(api, part, supplier, + "v0.1 BusBoard PCB rev 0.1") + + SP.create.assert_called_once() + + +def test_ensure_fab_supplier_part_list_failure_skips_gracefully(): + """API failure during list → no exception escapes, no create attempted.""" + api = MagicMock() + part = _part() + supplier = _supplier() + + with patch("bom_export.SupplierPart") as SP: + SP.list.side_effect = ConnectionError("server down") + _ensure_fab_supplier_part(api, part, supplier, "X") + + SP.create.assert_not_called() + + +def test_ensure_fab_supplier_part_create_failure_logged_not_raised(): + """API failure during create → logged, no exception escapes.""" + api = MagicMock() + part = _part() + supplier = _supplier() + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [] + SP.create.side_effect = RuntimeError("conflict") + # Must not raise: + _ensure_fab_supplier_part(api, part, supplier, "X") +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `source .venv/bin/activate && pytest scripts/tests/test_bom_export_fab_supplier.py -v` +Expected: 5 fails with `ImportError: cannot import name '_ensure_fab_supplier_part'`. + +- [ ] **Step 3: Implement the helper** + +Open `scripts/bom_export.py`. Find the existing Part-creation helpers (around line 240, just before `def create_pcb_part`). Add the helper: + +```python +def _ensure_fab_supplier_part( + api: InvenTreeAPI, + part: Part, + supplier: Company, + sku: str, +) -> None: + """Idempotently link a Part to a fab Supplier with a derived SKU. + + Same defensive pattern as ``ensure_supplier_parts`` in + ``inventree_sync/client.py``: list existing SupplierParts for + ``(part, supplier)``, post-filter on SKU (the server-side filter + has been observed unreliable on this InvenTree version), skip + creation when a match already exists. + + Errors during list / create are logged and swallowed — fab-supplier + linkage is best-effort metadata. The release artefacts (PCB Part, + Assembly, BOM) are the primary outputs and must not fail because of + a SupplierPart hiccup. + """ + try: + existing = SupplierPart.list(api, part=part.pk, supplier=supplier.pk) + except Exception as exc: + log.warning( + "SupplierPart lookup for part=%s supplier=%s failed: %s; " + "skipping fab linkage.", part.pk, supplier.pk, exc) + return + for sp in existing: + if str(getattr(sp, "SKU", "") or "") == sku: + log.info( + "SupplierPart for part=%s supplier=%s SKU=%r already " + "exists (pk=%s); skipping.", + part.pk, supplier.pk, sku, sp.pk) + return + try: + SupplierPart.create(api, { + "part": part.pk, + "supplier": supplier.pk, + "SKU": sku, + }) + log.info( + "Linked SupplierPart for part=%s (%s) → %s SKU=%r", + part.pk, part.name, supplier.name, sku) + except Exception as exc: + log.warning( + "SupplierPart create failed for part=%s supplier=%s SKU=%r: " + "%s; fab linkage skipped (add manually in the UI if needed).", + part.pk, supplier.pk, sku, exc) +``` + +Also ensure `Company` is imported at the top of `bom_export.py`. Check the existing imports: +- `from inventree.company import SupplierPart` is already there (around line 22). +- Add `Company` to that import: `from inventree.company import Company, SupplierPart`. + +- [ ] **Step 4: Run tests to verify pass** + +Run: `source .venv/bin/activate && pytest scripts/tests/test_bom_export_fab_supplier.py -v -k "ensure_fab_supplier_part"` +Expected: 5 passed. + +- [ ] **Step 5: Commit** + +```bash +git add scripts/bom_export.py scripts/tests/test_bom_export_fab_supplier.py +git commit -m "feat(bom-export): _ensure_fab_supplier_part helper + +Idempotently link a Part to a fab Supplier (JLCPCB by default) with a +derived SKU. Same defensive list-then-post-filter pattern as +ensure_supplier_parts in client.py. Failures during list / create are +logged and swallowed — fab linkage is best-effort metadata, never +blocks the release CI run. + +Co-Authored-By: Claude Opus 4.7 " +``` + +--- + +## Task 2: `create_pcb_part` / `create_stencil_part` attach the fab supplier + +**Files:** +- Modify: `scripts/bom_export.py` (`create_pcb_part` ~line 246, `create_stencil_part` ~line 288) +- Test: `scripts/tests/test_bom_export_fab_supplier.py` (append 4 tests) + +- [ ] **Step 1: Write 4 failing tests** + +Append to `scripts/tests/test_bom_export_fab_supplier.py`: + +```python +from bom_export import create_pcb_part, create_stencil_part, create_assembly_part # noqa: E402 + + +def _category(pk=10): + c = MagicMock() + c.pk = pk + return c + + +def test_create_pcb_part_attaches_fab_supplier_on_new_part(): + """New PCB Part → helper called with SKU derived from name+revision.""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1291, name="v0.1 BusBoard PCB") + supplier = _supplier(pk=381, name="JLCPCB") + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + + result = create_pcb_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=supplier) + + assert result is new_part + helper.assert_called_once_with( + api, new_part, supplier, "v0.1 BusBoard PCB rev 0.1", + ) + + +def test_create_pcb_part_attaches_fab_supplier_on_reuse(): + """Existing PCB Part → helper STILL called (retroactive linkage).""" + api = MagicMock() + cat = _category() + existing = _part(pk=999, name="v0.1 BusBoard PCB") + supplier = _supplier() + + with patch("bom_export.find_part_by_name_and_revision", + return_value=existing), \ + patch("bom_export._ensure_fab_supplier_part") as helper: + result = create_pcb_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=supplier) + + assert result is existing + helper.assert_called_once_with( + api, existing, supplier, "v0.1 BusBoard PCB rev 0.1", + ) + + +def test_create_pcb_part_skips_fab_supplier_when_none(): + """fab_supplier=None → helper NOT called.""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1291) + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + create_pcb_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=None) + + helper.assert_not_called() + + +def test_create_stencil_part_attaches_fab_supplier_on_new_part(): + """Stencil branch: SKU uses 'SMT Stencil' subtype.""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1293, name="v0.1 BusBoard SMT Stencil") + supplier = _supplier() + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + create_stencil_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=supplier) + + helper.assert_called_once_with( + api, new_part, supplier, "v0.1 BusBoard SMT Stencil rev 0.1", + ) + + +def test_create_assembly_part_does_not_attach_fab_supplier(): + """Assembly Module never gets supplier (built in-house).""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1292, name="v0.1 BusBoard Module") + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + create_assembly_part(api, cat, "v0.1 BusBoard", "0.1", "/tmp/img.png") + + helper.assert_not_called() +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `source .venv/bin/activate && pytest scripts/tests/test_bom_export_fab_supplier.py -v -k "create_"` +Expected: 5 fails — `create_pcb_part` doesn't accept `fab_supplier` kwarg yet. + +- [ ] **Step 3: Modify the three create functions** + +In `scripts/bom_export.py`, find `create_pcb_part` (around line 246). Add `fab_supplier` keyword-only parameter and the helper call: + +```python +def create_pcb_part( + api: InvenTreeAPI, + category: PartCategory, + name: str, + version: str, + image: str | None, + *, + fab_supplier: Optional[Company] = None, +) -> Part: + full_name = f"{name} PCB" + existing = find_part_by_name_and_revision(api, full_name, version) + if existing is not None: + log.info("Reusing existing PCB part '%s' rev %s (pk=%s)", + full_name, version, existing.pk) + if fab_supplier is not None: + _ensure_fab_supplier_part( + api, existing, fab_supplier, f"{full_name} rev {version}", + ) + return existing + + part = Part.create(api, { + "category": category.pk, + "name": full_name, + "revision": version, + "component": True, + }) + if image is not None: + assert part.uploadImage(image) is not None, f"Image upload failed: {image}" + log.info("Created PCB part '%s' rev %s (pk=%s)", full_name, version, part.pk) + if fab_supplier is not None: + _ensure_fab_supplier_part( + api, part, fab_supplier, f"{full_name} rev {version}", + ) + return part +``` + +Do the analogous change in `create_stencil_part` (around line 288). Keep `create_assembly_part` unchanged. + +Make sure `Optional` is imported at the top of `bom_export.py` (it likely is — verify). + +- [ ] **Step 4: Run tests to verify pass** + +Run: `source .venv/bin/activate && pytest scripts/tests/test_bom_export_fab_supplier.py -v` +Expected: all 10 pass (5 helper + 5 create-function tests). + +- [ ] **Step 5: Run the whole suite for regression check** + +Run: `source .venv/bin/activate && pytest scripts/tests/` +Expected: previous 211 passed + 10 new = 221 passed. + +- [ ] **Step 6: Commit** + +```bash +git add scripts/bom_export.py scripts/tests/test_bom_export_fab_supplier.py +git commit -m "feat(bom-export): create_pcb_part / create_stencil_part attach fab supplier + +Both create functions gain a keyword-only fab_supplier=None argument. +When non-None, _ensure_fab_supplier_part is called after Part create OR +reuse — the reuse path enables retroactive linkage by re-running +bom_export.py on releases that predate this change. + +SKU is derived as f'{full_name} rev {version}', stable per design +revision, idempotent across re-runs. + +create_assembly_part is intentionally NOT modified: the assembly is +built in-house, no external supplier applies. + +Co-Authored-By: Claude Opus 4.7 " +``` + +--- + +## Task 3: CLI flags + `main()` wiring + +**Files:** +- Modify: `scripts/bom_export.py` (`parse_args` around line 418, `main()` around line 469) + +- [ ] **Step 1: Add the constant and CLI flags** + +In `scripts/bom_export.py`, near the top after the existing category-name constants (around line 38): + +```python +# Default fab supplier name. CLI --pcb-supplier overrides per-run; the +# Company is auto-created via get_or_create_supplier if missing. +DEFAULT_PCB_SUPPLIER_NAME = "JLCPCB" +``` + +In `parse_args` (around line 418), add the two CLI flags inside the parser block. Place them between the `--categories` and `--planned-builds` flags: + +```python + parser.add_argument( + "--pcb-supplier", + default=DEFAULT_PCB_SUPPLIER_NAME, + help=( + "Company name of the fab that produces the PCB + SMT stencil " + f"(default: {DEFAULT_PCB_SUPPLIER_NAME!r}). The Company is " + "auto-created if missing. Use --no-pcb-supplier to skip " + "SupplierPart linkage entirely." + ), + ) + parser.add_argument( + "--no-pcb-supplier", + dest="pcb_supplier", action="store_const", const=None, + help="Skip SupplierPart linkage on PCB + SMT stencil parts.", + ) +``` + +- [ ] **Step 2: Resolve the supplier and thread through `main()`** + +In `main()` (around line 469), right after `api = InvenTreeAPI()` and the existing `reporter = ...` line, add: + +```python + fab_supplier: Optional[Company] = None + if args.pcb_supplier is not None: + fab_supplier = get_or_create_supplier(api, name=args.pcb_supplier) + if fab_supplier is None: + log.error( + "Could not get or create fab supplier %r — proceeding " + "without SupplierPart linkage.", args.pcb_supplier) +``` + +Then find the calls to `create_pcb_part`, `create_assembly_part`, `create_stencil_part` (around line 556-558) and pass `fab_supplier=fab_supplier` to the PCB and stencil ones (assembly stays as-is): + +```python + pcb = create_pcb_part( + api, pcb_cat, args.name, args.version, args.pcb_image, + fab_supplier=fab_supplier, + ) + assembly = create_assembly_part(api, assembly_cat, args.name, args.version, args.assembly_image) + stencil = create_stencil_part( + api, stencil_cat, args.name, args.version, args.stencil_image, + fab_supplier=fab_supplier, + ) +``` + +The dry-run branch (the long `if reporter is not None:` block around line 482-540) doesn't currently model SupplierPart decisions. Leave it as-is for this PR; dry-run accurately reports PCB/Assembly/Stencil decisions but is silent on the SupplierPart side. That's an acceptable known limitation — the helper is best-effort anyway. Note this in a code comment near the dry-run block: + +```python + if reporter is not None: + # Dry-run path: record decisions, skip side-effecting operations + # ... + # Note: SupplierPart linkage (--pcb-supplier) is NOT modelled in + # the dry-run report. The helper itself is best-effort + # (failures are logged, never raised), so dry-run silence is + # acceptable. Real-run output covers it via the helper's + # info/warning log lines. +``` + +- [ ] **Step 3: Verify `--help` still works and shows the new flags** + +Run: `source .venv/bin/activate && python3 scripts/bom_export.py --help` +Expected: usage text includes `--pcb-supplier PCB_SUPPLIER` and `--no-pcb-supplier`, no import errors. + +- [ ] **Step 4: Run the whole suite** + +Run: `source .venv/bin/activate && pytest scripts/tests/` +Expected: 221 passed. + +- [ ] **Step 5: Commit** + +```bash +git add scripts/bom_export.py +git commit -m "feat(bom-export): --pcb-supplier / --no-pcb-supplier CLI flags + +main() resolves the fab supplier Company via get_or_create_supplier +when --pcb-supplier is set (default: JLCPCB). The resolved Company is +threaded into create_pcb_part and create_stencil_part as a keyword +argument. create_assembly_part is intentionally not touched (assembly +is built in-house). + +--no-pcb-supplier opts out: PCB+stencil Parts get no SupplierPart +attached, matching pre-PR behaviour for users who don't want this. + +Dry-run path doesn't model SupplierPart decisions — the linkage is +best-effort metadata, real-run logs cover the outcome. + +Co-Authored-By: Claude Opus 4.7 " +``` + +--- + +## Self-Review + +**Spec coverage:** +- ✅ Goal 1 (attach SupplierPart to PCB+Stencil) → Task 2. +- ✅ Goal 2 (CLI flag selects supplier) → Task 3. +- ✅ Goal 3 (derived SKU = `{full_name} rev {version}`) → Task 2 implementation. +- ✅ Goal 4 (idempotency) → Task 1 helper. +- ✅ Goal 5 (`--no-pcb-supplier` opt-out) → Task 3. +- ✅ Goal 6 (Assembly unchanged) → Task 2 explicit test + commit message. +- ✅ Goal 7 (no unrelated changes) → File list scope. + +**Placeholder scan:** no TBD/TODO. All code blocks contain concrete code. + +**Type consistency:** +- `_ensure_fab_supplier_part(api, part, supplier, sku) -> None` — used consistently in tests and create-function calls. +- `fab_supplier: Optional[Company] = None` keyword-only — same signature in `create_pcb_part`, `create_stencil_part`. `create_assembly_part` does NOT add this param (verified in Goal 6 test). +- `DEFAULT_PCB_SUPPLIER_NAME: str` constant referenced in argparse default + help string. + +No gaps found. diff --git a/docs/superpowers/specs/2026-06-11-pcb-supplier-attachment-design.md b/docs/superpowers/specs/2026-06-11-pcb-supplier-attachment-design.md new file mode 100644 index 0000000..7e0f211 --- /dev/null +++ b/docs/superpowers/specs/2026-06-11-pcb-supplier-attachment-design.md @@ -0,0 +1,324 @@ +# Attach a Supplier to PCB & SMT Stencil Parts in `bom_export.py` + +**Status:** Spec ready. +**Scope:** Bugfix in `scripts/bom_export.py`. `create_pcb_part` and +`create_stencil_part` currently create the InvenTree Part only, with no +`SupplierPart` linkage. When the operator later wants to reorder the PCB +or stencil from the fab (JLCPCB in our case), there's no record of which +supplier produced it — the only metadata is the design name + revision. +**Predecessor:** PR #32 `use supplier_reference + auto-reference` +(main @ 0a0eb92). +**Erstellt:** 2026-06-11. + +--- + +## Motivation + +After the recent v0.1 bulk-import we ended up with 4 PCBs (pks 1291, +1294, 1297, 1301) and 4 SMT Stencils (pks 1293, 1296, 1299, 1303) in +InvenTree — none with a SupplierPart. JLCPCB exists in InvenTree as +Company pk=381 (`is_supplier=true`, `is_manufacturer=false`), it just +isn't linked. + +Consequences: +- Re-order path requires the operator to remember which fab made each + PCB design — fine for 4 designs, not fine for 20+. +- InvenTree's "Purchasing" view shows no supplier for these parts, so + they can't be added to a JLCPCB PurchaseOrder via the normal flow. +- Stock-low alerts can't link to a supplier for one-click reorder. + +The fix is mechanically small: extend the create functions to also POST +a `SupplierPart` linking the Part to a configured fab supplier. Same +idempotent pattern as `ensure_supplier_parts` in `client.py`. + +--- + +## Goals + +- `create_pcb_part` and `create_stencil_part` attach a `SupplierPart` + to a configured fab supplier (default JLCPCB) for every newly-created + PCB and stencil Part. +- The supplier is selectable per-run via CLI flag (different fabs per + project must work without editing code). +- The SKU is derived from the Part name + revision so it's stable + across re-runs — same design rev always points to the same SKU. +- Idempotent: re-running `bom_export.py` for the same release doesn't + produce duplicate SupplierParts. +- Opt-out: an explicit flag suppresses SupplierPart creation entirely + (for users without a fab or who manage supplier linkage manually). +- The `Assembly` part stays without supplier (we assemble in-house). +- The implementation does NOT modify `inventree_sync/order_import.py`, + `inventree_sync/dry_run.py`, or any unrelated code path. + +## Non-Goals + +- **No** `link` / `pricing` / `pack_quantity` field handling on the + SupplierPart (deferred — re-order not planned in the near term). +- **No** support for multiple fabs per assembly (PCB from JLCPCB, + stencil from PCBWay etc.). One supplier per run, both parts attach + to it. +- **No** ManufacturerPart creation. Bare-PCB substrate has no MPN / + manufacturer-side identifier worth tracking. +- **No** auto-derivation of the SupplierPart link from GitHub release + URLs (consciously deferred — `--pcb-supplier-link` flag could be a + future addition). +- **No** retroactive backfill of the 8 existing v0.1 PCB/Stencil + records. That happens as a separate ad-hoc API run outside this PR. +- **No** TME or other 3rd-distributor handling (separate brainstorming + thread, may or may not happen). + +--- + +## Designentscheidungen + +### Why CLI flag instead of hardcoded JLCPCB + +We could hardcode `PCB_SUPPLIER_NAME = "JLCPCB"`. But the `bom_export.py` +script is consumer-repo-agnostic — `HW-Module-CI`'s reusable workflows +call it from `HW-Module-BusBoard`, `HW-Module-PowerBoard`, etc., and a +future project might want PCBWay or AISLER. Hardcoding would tie the +shared script to one consumer's choice. + +CLI flag `--pcb-supplier NAME` (default `"JLCPCB"`) keeps the default +matching reality (everyone uses JLCPCB today) while leaving the door +open. The Auto-Release workflow at `create-release-docs.yaml` can be +updated to pass a project-specific value later if needed; today the +default suffices. + +### Why SKU = `{full_name} rev {version}` (stable, deterministic) + +A SupplierPart needs a SKU. JLCPCB itself assigns ephemeral order IDs +per production batch (no permanent design-side SKU). Options considered: + +1. **`{full_name} rev {version}`** — e.g. `"v0.1 BusBoard PCB rev 0.1"`. + Stable per design, idempotent on re-runs. +2. **Git SHA** — stable per commit. Burns through SupplierPart slots on + every release. Wrong granularity. +3. **Random UUID** — never stable across runs. Defeats the point. +4. **No SKU** — InvenTree's SupplierPart model requires a non-empty + SKU. Rejected at the API level. + +Chosen: **Option 1**. Idempotent re-run finds the existing SupplierPart +by (part, supplier, SKU) and skips creation. Same pattern as +`ensure_supplier_parts` in `client.py` line ~720. + +### Why opt-out flag instead of "supplier=None means skip" + +A `--pcb-supplier ""` could mean "skip supplier linkage". But empty- +string-as-sentinel is confusing in CLI parsing (and quoting is messy in +YAML CI configs). Explicit `--no-pcb-supplier` (action="store_true") is +clearer and survives argparse normalization unambiguously. + +### Why Assembly stays without supplier + +The Assembly Module is built by OE5XRX itself (or whoever runs the +import). There's no external supplier. If a project ever outsources SMT +assembly to JLCPCB-Assembly or PCBWay-Assembly, that's a future +extension — add a separate `--assembly-supplier NAME` flag then, +following the same pattern. YAGNI for now. + +--- + +## Komponenten + +### `scripts/bom_export.py` — Änderungen + +**New constants:** + +```python +# Default fab supplier name. CLI --pcb-supplier overrides per-run; the +# JLCPCB Company is auto-created via get_or_create_supplier if missing. +DEFAULT_PCB_SUPPLIER_NAME = "JLCPCB" +``` + +**New CLI flags:** + +```python +parser.add_argument( + "--pcb-supplier", default=DEFAULT_PCB_SUPPLIER_NAME, + help=( + "Company name of the fab that produces the PCB + SMT stencil " + f"(default: {DEFAULT_PCB_SUPPLIER_NAME!r}). The Company is " + "auto-created if missing. Use --no-pcb-supplier to skip " + "SupplierPart linkage entirely." + ), +) +parser.add_argument( + "--no-pcb-supplier", dest="pcb_supplier", action="store_const", + const=None, + help="Skip SupplierPart linkage on PCB + SMT stencil parts.", +) +``` + +After argparse: `args.pcb_supplier` is either a non-empty string (a +name) or `None` (opt-out). Both flags target the same dest so the order +in the command line is "last wins" — predictable behaviour. + +**New helper** in `scripts/bom_export.py` (NOT in `inventree_sync` +package — this is bom_export-specific so far): + +```python +def _ensure_fab_supplier_part( + api: InvenTreeAPI, + part: Part, + supplier: Company, + sku: str, +) -> None: + """Idempotently link a Part to a fab Supplier with a derived SKU. + + Same defensive pattern as `ensure_supplier_parts` in client.py: + list existing SupplierParts for (part, supplier), post-filter on SKU + (server-side filter is unreliable on this InvenTree version), skip + when a match already exists. + + Errors during create are logged but never raised — caller treats + fab-supplier linkage as best-effort, not blocking. A missing + SupplierPart can always be added later via the InvenTree UI. + """ + try: + existing = SupplierPart.list(api, part=part.pk, supplier=supplier.pk) + except Exception as exc: + log.warning( + "SupplierPart lookup for part=%s supplier=%s failed: %s; " + "skipping fab linkage.", part.pk, supplier.pk, exc) + return + for sp in existing: + if str(getattr(sp, "SKU", "") or "") == sku: + log.info( + "SupplierPart for part=%s supplier=%s SKU=%r already " + "exists (pk=%s); skipping.", + part.pk, supplier.pk, sku, sp.pk) + return + try: + SupplierPart.create(api, { + "part": part.pk, + "supplier": supplier.pk, + "SKU": sku, + }) + log.info( + "Linked SupplierPart for part=%s (%s) → %s SKU=%r", + part.pk, part.name, supplier.name, sku) + except Exception as exc: + log.warning( + "SupplierPart create failed for part=%s supplier=%s SKU=%r: %s; " + "fab linkage skipped (add manually in the UI if needed).", + part.pk, supplier.pk, sku, exc) +``` + +**Modified create functions:** + +`create_pcb_part` and `create_stencil_part` gain an optional +`fab_supplier: Optional[Company] = None` parameter. When non-None, the +function calls `_ensure_fab_supplier_part` after Part create / reuse +with SKU = `f"{full_name} rev {version}"`. + +Existing-Part path (reuse branch) ALSO calls `_ensure_fab_supplier_part` +— this lets the operator add fab linkage retroactively by re-running +`bom_export.py` on a release where Parts already exist but were created +before this PR. Idempotent thanks to the SKU-existence check. + +`create_assembly_part` stays unchanged (no fab supplier — built in-house). + +**Modified `main()`:** + +```python +fab_supplier: Optional[Company] = None +if args.pcb_supplier is not None: + fab_supplier = get_or_create_supplier(api, name=args.pcb_supplier) + if fab_supplier is None: + log.error( + "Could not get or create fab supplier %r — proceeding without " + "SupplierPart linkage.", args.pcb_supplier) + +# ... existing pcb / assembly / stencil creation ... +pcb = create_pcb_part( + api, pcb_cat, args.name, args.version, args.pcb_image, + fab_supplier=fab_supplier, +) +# assembly: unchanged +stencil = create_stencil_part( + api, stencil_cat, args.name, args.version, args.stencil_image, + fab_supplier=fab_supplier, +) +``` + +`get_or_create_supplier` already exists in `inventree_sync/client.py` +(imported at the top of `bom_export.py`). Re-use, no new import. + +### Tests + +Test file: `scripts/tests/test_bom_export_fab_supplier.py` (new). + +| Test | Verifies | +|---|---| +| `test_ensure_fab_supplier_part_creates_when_missing` | Mock `SupplierPart.list` returns `[]`. Call helper. Assert `SupplierPart.create` called with `{part, supplier, SKU}`. | +| `test_ensure_fab_supplier_part_skips_when_existing` | Mock `SupplierPart.list` returns one SP with matching SKU. Assert `SupplierPart.create` NOT called. | +| `test_ensure_fab_supplier_part_creates_when_existing_different_sku` | Mock returns SP with different SKU. Assert `create` IS called (different design rev). | +| `test_ensure_fab_supplier_part_list_failure_skips_gracefully` | Mock `list` raises. Assert no exception escapes and no create attempted. | +| `test_ensure_fab_supplier_part_create_failure_logged_not_raised` | Mock `create` raises. Assert warning logged, exception swallowed. | +| `test_create_pcb_part_attaches_fab_supplier_on_new_part` | Full path: new Part. Assert `_ensure_fab_supplier_part` called with `SKU=" PCB rev "`. | +| `test_create_pcb_part_attaches_fab_supplier_on_reuse` | Existing Part returned. Helper STILL called (retroactive linkage). | +| `test_create_pcb_part_skips_fab_supplier_when_none` | `fab_supplier=None`. Helper NOT called. | +| `test_create_stencil_part_attaches_fab_supplier_on_new_part` | Same as PCB but with `" SMT Stencil rev "` SKU. | +| `test_create_assembly_part_does_not_attach_fab_supplier` | Assembly never gets supplier (verify no helper call). | + +CLI-integration test added to existing `scripts/tests/` if appropriate +(or skipped — the existing `bom_export.py` doesn't have a CLI test +file). Decision: skip the CLI integration test; the unit tests cover +the logic, and the CLI flag itself is mechanical argparse. + +--- + +## Idempotenz-Garantien + +| Re-Run-Szenario | Verhalten | +|---|---| +| First-ever run of v1.0 | PCB+Stencil Parts created. SupplierPart created for each linking to JLCPCB. | +| Re-run of same v1.0 release | Existing Parts found via `find_part_by_name_and_revision`. SupplierPart found via SKU match. Both skipped. | +| Re-run of v1.0 with `--pcb-supplier PCBWay` | Existing PCB Part reused. New SupplierPart created for PCBWay (different supplier). Old JLCPCB SupplierPart unchanged. | +| Re-run with `--no-pcb-supplier` | Existing Parts reused. No SupplierPart action. Existing SupplierParts (from previous runs) remain. | +| Backfill: Part exists, was created before this PR (no SupplierPart) | First call to the modified `create_pcb_part` adds the SupplierPart. | + +--- + +## Error Handling + +| Failure | Verhalten | +|---|---| +| `get_or_create_supplier` returns None (Company API down or 403) | `main()` logs error, sets `fab_supplier=None`, proceeds. PCB+Stencil created without SupplierPart. | +| `SupplierPart.list` raises in helper | Logged as warning, helper returns without create. | +| `SupplierPart.create` raises in helper | Logged as warning, helper returns. PCB/Stencil Part already exists at this point. | +| User passes invalid name to `--pcb-supplier` (e.g. with `is_customer=true`) | `get_or_create_supplier` always creates with `is_supplier=true`, `is_manufacturer=false`. If a Company with that name + wrong flag already exists, it gets returned — operator can re-classify in UI. | + +The contract: **fab-supplier linkage is best-effort**. The script must +never fail the release CI run due to a fab-supplier issue. The release +docs / assembly creation are the primary outputs; SupplierPart is +secondary metadata. + +--- + +## Backwards-Compatibility + +- `create_pcb_part` / `create_stencil_part` gain a keyword-only + `fab_supplier=None` argument. Existing call sites in `main()` are + updated; any external caller (none today) would default to no + supplier. +- Default CLI behaviour: SupplierPart linkage IS created (`--pcb-supplier` + defaults to `"JLCPCB"`). This is a behaviour change vs. main, but the + current behaviour is the bug — no opposition expected. +- `--no-pcb-supplier` provides the explicit opt-out for users who don't + want this. +- Existing unit tests for `create_pcb_part` etc. (if any) pass + `fab_supplier=None` implicitly via the default. + +--- + +## Out-of-Scope + +- ManufacturerPart linkage (PCB substrate maker, e.g. FR4 vendor). +- `link` field on SupplierPart pointing to GitHub release asset. +- `pack_quantity` / `pricing` fields on SupplierPart. +- Multi-fab support (PCB from one fab, stencil from another). +- Assembly-side supplier (JLCPCB SMT-Assembly). +- Retroactive backfill of existing v0.1 PCBs/Stencils — done outside + this PR as a one-shot API run. diff --git a/scripts/bom_export.py b/scripts/bom_export.py index b8e5f7e..91c0dae 100644 --- a/scripts/bom_export.py +++ b/scripts/bom_export.py @@ -19,13 +19,13 @@ from typing import Optional from inventree.api import InvenTreeAPI -from inventree.company import SupplierPart +from inventree.company import Company, SupplierPart from inventree.part import BomItem, Part, PartCategory from inventree_sync import BomEntry, ensure_parts_exist from inventree_sync.attachments import attach_kibot_outputs from inventree_sync.categories import load_category_map -from inventree_sync.client import ensure_related_parts, find_part_by_name_and_revision +from inventree_sync.client import ensure_related_parts, find_part_by_name_and_revision, get_or_create_supplier from inventree_sync.cost_report import generate_cost_report from inventree_sync.dry_run import DryRunReporter @@ -37,6 +37,10 @@ ASSEMBLY_CATEGORY_NAME = "PCBA" STENCIL_CATEGORY_NAME = "SMT Stencil" +# Default fab supplier name. CLI --pcb-supplier overrides per-run; the +# Company is auto-created via get_or_create_supplier if missing. +DEFAULT_PCB_SUPPLIER_NAME = "JLCPCB" + # --------------------------------------------------------------------------- # Error collector @@ -243,12 +247,76 @@ def match_supplier_parts( # PCB + assembly + stencil creation # --------------------------------------------------------------------------- -def create_pcb_part(api: InvenTreeAPI, category: PartCategory, name: str, version: str, image: str | None) -> Part: +def _ensure_fab_supplier_part( + api: InvenTreeAPI, + part: Part, + supplier: Company, + sku: str, +) -> None: + """Idempotently link a Part to a fab Supplier with a derived SKU. + + Same defensive pattern as ``ensure_supplier_parts`` in + ``inventree_sync/client.py``: list existing SupplierParts for + ``(part, supplier)``, post-filter on SKU (the server-side filter + has been observed unreliable on this InvenTree version), skip + creation when a match already exists. + + Errors during list / create are logged and swallowed — fab-supplier + linkage is best-effort metadata. The release artefacts (PCB Part, + Assembly, BOM) are the primary outputs and must not fail because of + a SupplierPart hiccup. + """ + try: + existing = SupplierPart.list(api, part=part.pk, supplier=supplier.pk) + # Broad except is intentional — fab linkage is best-effort metadata + # per docstring; release artefacts must not fail on this. + except Exception as exc: + log.warning( + "SupplierPart lookup for part=%s supplier=%s failed: %s; " + "skipping fab linkage.", part.pk, supplier.pk, exc) + return + for sp in existing: + if str(getattr(sp, "SKU", "") or "") == sku: + log.info( + "SupplierPart for part=%s supplier=%s SKU=%r already " + "exists (pk=%s); skipping.", + part.pk, supplier.pk, sku, sp.pk) + return + try: + SupplierPart.create(api, { + "part": part.pk, + "supplier": supplier.pk, + "SKU": sku, + }) + log.info( + "Linked SupplierPart for part=%s (%s) -> %s SKU=%r", + part.pk, part.name, supplier.name, sku) + # Broad except is intentional — fab linkage is best-effort metadata + # per docstring; release artefacts must not fail on this. + except Exception as exc: + log.warning( + "SupplierPart create failed for part=%s supplier=%s SKU=%r: " + "%s; fab linkage skipped (add manually in the UI if needed).", + part.pk, supplier.pk, sku, exc) + + +def create_pcb_part( + api: InvenTreeAPI, + category: PartCategory, + name: str, + version: str, + image: str | None, + *, + fab_supplier: Optional[Company] = None, +) -> Part: full_name = f"{name} PCB" existing = find_part_by_name_and_revision(api, full_name, version) if existing is not None: log.info("Reusing existing PCB part '%s' rev %s (pk=%s)", full_name, version, existing.pk) + if fab_supplier is not None: + _ensure_fab_supplier_part(api, existing, fab_supplier, + f"{full_name} rev {version}") return existing part = Part.create(api, { @@ -260,6 +328,9 @@ def create_pcb_part(api: InvenTreeAPI, category: PartCategory, name: str, versio if image is not None: assert part.uploadImage(image) is not None, f"Image upload failed: {image}" log.info("Created PCB part '%s' rev %s (pk=%s)", full_name, version, part.pk) + if fab_supplier is not None: + _ensure_fab_supplier_part(api, part, fab_supplier, + f"{full_name} rev {version}") return part @@ -291,12 +362,17 @@ def create_stencil_part( name: str, version: str, image: str | None = None, + *, + fab_supplier: Optional[Company] = None, ) -> Part: full_name = f"{name} SMT Stencil" existing = find_part_by_name_and_revision(api, full_name, version) if existing is not None: log.info("Reusing existing stencil part '%s' rev %s (pk=%s)", full_name, version, existing.pk) + if fab_supplier is not None: + _ensure_fab_supplier_part(api, existing, fab_supplier, + f"{full_name} rev {version}") return existing part = Part.create(api, { @@ -308,6 +384,9 @@ def create_stencil_part( if image is not None: assert part.uploadImage(image) is not None, f"Image upload failed: {image}" log.info("Created stencil part '%s' rev %s (pk=%s)", full_name, version, part.pk) + if fab_supplier is not None: + _ensure_fab_supplier_part(api, part, fab_supplier, + f"{full_name} rev {version}") return part @@ -458,6 +537,21 @@ def parse_args() -> argparse.Namespace: "decreased). Default: 10." ), ) + parser.add_argument( + "--pcb-supplier", + default=DEFAULT_PCB_SUPPLIER_NAME, + help=( + "Company name of the fab that produces the PCB + SMT stencil " + f"(default: {DEFAULT_PCB_SUPPLIER_NAME!r}). The Company is " + "auto-created if missing. Use --no-pcb-supplier to skip " + "SupplierPart linkage entirely." + ), + ) + parser.add_argument( + "--no-pcb-supplier", + dest="pcb_supplier", action="store_const", const=None, + help="Skip SupplierPart linkage on PCB + SMT stencil parts.", + ) parser.add_argument( "--dry-run", dest="dry_run", action="store_true", help="Simulate the sync flow without InvenTree side-effects. " @@ -486,6 +580,12 @@ def main() -> None: # fetch). Read-only InvenTree lookups (find_part_by_name_and_revision, # BomItem.list, SupplierPart.list in match_supplier_parts) still run — # they're how we know whether something WOULD be CREATE vs REUSE. + # Note: SupplierPart linkage (--pcb-supplier) is intentionally + # not modelled in the dry-run report. The fab-supplier lookup + # itself only runs in the non-dry-run branch below (would + # auto-create the Company otherwise), so dry-run is fully + # side-effect-free with respect to fab linkage. Real-run output + # surfaces decisions via the helper's info/warning log lines. ensure_parts_exist(api, entries, category_map, reporter=reporter) match_supplier_parts(api, entries, reporter=reporter) @@ -543,6 +643,21 @@ def main() -> None: # Non-dry-run path: original flow continues below. collector = ErrorCollector() + # Resolve fab supplier now (not before the dry-run gate): get_or_create_supplier + # calls Company.create when the named Company is missing, which would violate + # the dry-run contract — same bug class as PR #31. + fab_supplier: Optional[Company] = None + # Normalise the --pcb-supplier value: strip whitespace and treat empty + # as opt-out (equivalent to --no-pcb-supplier). Otherwise a stray + # `--pcb-supplier " "` would have us POST a Company with a blank name. + pcb_supplier_name = (args.pcb_supplier or "").strip() or None + if pcb_supplier_name is not None: + fab_supplier = get_or_create_supplier(api, name=pcb_supplier_name) + if fab_supplier is None: + log.error( + "Could not get or create fab supplier %r — proceeding " + "without SupplierPart linkage.", pcb_supplier_name) + # Create any parts that don't exist in InvenTree yet ensure_parts_exist(api, entries, category_map) @@ -553,9 +668,15 @@ def main() -> None: assembly_cat = get_category_by_name(api, ASSEMBLY_CATEGORY_NAME) stencil_cat = get_category_by_name(api, STENCIL_CATEGORY_NAME) - pcb = create_pcb_part(api, pcb_cat, args.name, args.version, args.pcb_image) + pcb = create_pcb_part( + api, pcb_cat, args.name, args.version, args.pcb_image, + fab_supplier=fab_supplier, + ) assembly = create_assembly_part(api, assembly_cat, args.name, args.version, args.assembly_image) - stencil = create_stencil_part(api, stencil_cat, args.name, args.version, args.stencil_image) + stencil = create_stencil_part( + api, stencil_cat, args.name, args.version, args.stencil_image, + fab_supplier=fab_supplier, + ) # Link stencil ↔ PCB as related parts (not BOM – the stencil is a # production tool, not a consumed component of the assembly). diff --git a/scripts/tests/test_bom_export_fab_supplier.py b/scripts/tests/test_bom_export_fab_supplier.py new file mode 100644 index 0000000..d423076 --- /dev/null +++ b/scripts/tests/test_bom_export_fab_supplier.py @@ -0,0 +1,362 @@ +"""Unit tests for bom_export.py's fab-supplier-attachment helpers.""" +from __future__ import annotations + +import logging +import sys +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from bom_export import main # noqa: E402 (used by dry-run test below) + +from bom_export import _ensure_fab_supplier_part # noqa: E402 + + +def _part(pk=100, name="v0.1 BusBoard PCB"): + p = MagicMock() + p.pk = pk + p.name = name + return p + + +def _supplier(pk=381, name="JLCPCB"): + s = MagicMock() + s.pk = pk + s.name = name + return s + + +def _supplier_part_mock(pk, sku): + sp = MagicMock() + sp.pk = pk + sp.SKU = sku + return sp + + +def test_ensure_fab_supplier_part_creates_when_missing(): + """No existing SupplierPart for (part, supplier) → create new.""" + api = MagicMock() + part = _part(pk=1291) + supplier = _supplier(pk=381) + sku = "v0.1 BusBoard PCB rev 0.1" + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [] + _ensure_fab_supplier_part(api, part, supplier, sku) + + SP.list.assert_called_once_with(api, part=1291, supplier=381) + SP.create.assert_called_once() + payload = SP.create.call_args[0][1] + assert payload["part"] == 1291 + assert payload["supplier"] == 381 + assert payload["SKU"] == sku + + +def test_ensure_fab_supplier_part_skips_when_existing(): + """Matching SKU already linked → no new SupplierPart created.""" + api = MagicMock() + part = _part(pk=1291) + supplier = _supplier(pk=381) + sku = "v0.1 BusBoard PCB rev 0.1" + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [_supplier_part_mock(pk=500, sku=sku)] + _ensure_fab_supplier_part(api, part, supplier, sku) + + SP.create.assert_not_called() + + +def test_ensure_fab_supplier_part_creates_when_existing_different_sku(): + """SP exists but for a different SKU (= different design rev) → create.""" + api = MagicMock() + part = _part(pk=1291) + supplier = _supplier(pk=381) + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [ + _supplier_part_mock(pk=500, sku="OLD-DESIGN-PCB rev 0.0") + ] + _ensure_fab_supplier_part(api, part, supplier, + "v0.1 BusBoard PCB rev 0.1") + + SP.create.assert_called_once() + + +def test_ensure_fab_supplier_part_list_failure_skips_gracefully(caplog): + """API failure during list → no exception escapes, no create attempted.""" + caplog.set_level(logging.WARNING, logger="bom_export") + api = MagicMock() + part = _part() + supplier = _supplier() + + with patch("bom_export.SupplierPart") as SP: + SP.list.side_effect = ConnectionError("server down") + _ensure_fab_supplier_part(api, part, supplier, "X") + + SP.create.assert_not_called() + # Operator's only signal that fab linkage failed must survive future refactors + assert any( + r.levelno == logging.WARNING and "SupplierPart lookup" in r.message + for r in caplog.records + ) + + +def test_ensure_fab_supplier_part_create_failure_logged_not_raised(caplog): + """API failure during create → logged, no exception escapes.""" + caplog.set_level(logging.WARNING, logger="bom_export") + api = MagicMock() + part = _part() + supplier = _supplier() + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [] + SP.create.side_effect = RuntimeError("conflict") + # Must not raise: + _ensure_fab_supplier_part(api, part, supplier, "X") + + assert any( + r.levelno == logging.WARNING and "SupplierPart create failed" in r.message + for r in caplog.records + ) + + +def test_ensure_fab_supplier_part_skips_create_when_any_existing_sku_matches(): + """A list with a non-matching SP followed by a matching one → no create. + + The loop must scan past the non-match and recognise the second entry as + a hit (renaming clarifies — "first match" was misleading because the + matching SKU is the second item in the list).""" + api = MagicMock() + part = _part() + supplier = _supplier() + sku = "v0.1 BusBoard PCB rev 0.1" + + with patch("bom_export.SupplierPart") as SP: + SP.list.return_value = [ + _supplier_part_mock(pk=500, sku="OLD-DESIGN-PCB rev 0.0"), + _supplier_part_mock(pk=501, sku=sku), # match + ] + _ensure_fab_supplier_part(api, part, supplier, sku) + + SP.create.assert_not_called() + + +def test_ensure_fab_supplier_part_tolerates_none_sku_on_existing(): + """An existing SP with SKU=None → coerced to '' by the `or` chain, no match → create.""" + api = MagicMock() + part = _part() + supplier = _supplier() + target_sku = "v0.1 BusBoard PCB rev 0.1" + + with patch("bom_export.SupplierPart") as SP: + sp_with_none = MagicMock() + sp_with_none.pk = 999 + sp_with_none.SKU = None + SP.list.return_value = [sp_with_none] + _ensure_fab_supplier_part(api, part, supplier, target_sku) + + # SKU=None coerced to "", doesn't match target → create proceeds + SP.create.assert_called_once() + + +from bom_export import create_pcb_part, create_stencil_part, create_assembly_part # noqa: E402 + + +def _category(pk=10): + c = MagicMock() + c.pk = pk + return c + + +def test_create_pcb_part_attaches_fab_supplier_on_new_part(): + """New PCB Part → helper called with SKU derived from name+revision.""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1291, name="v0.1 BusBoard PCB") + supplier = _supplier(pk=381, name="JLCPCB") + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + + result = create_pcb_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=supplier) + + assert result is new_part + helper.assert_called_once_with( + api, new_part, supplier, "v0.1 BusBoard PCB rev 0.1", + ) + + +def test_create_pcb_part_attaches_fab_supplier_on_reuse(): + """Existing PCB Part → helper STILL called (retroactive linkage).""" + api = MagicMock() + cat = _category() + existing = _part(pk=999, name="v0.1 BusBoard PCB") + supplier = _supplier() + + with patch("bom_export.find_part_by_name_and_revision", + return_value=existing), \ + patch("bom_export._ensure_fab_supplier_part") as helper: + result = create_pcb_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=supplier) + + assert result is existing + helper.assert_called_once_with( + api, existing, supplier, "v0.1 BusBoard PCB rev 0.1", + ) + + +def test_create_pcb_part_skips_fab_supplier_when_none(): + """fab_supplier=None → helper NOT called.""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1291) + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + create_pcb_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=None) + + helper.assert_not_called() + + +def test_create_stencil_part_attaches_fab_supplier_on_new_part(): + """Stencil branch: SKU uses 'SMT Stencil' subtype.""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1293, name="v0.1 BusBoard SMT Stencil") + supplier = _supplier() + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + create_stencil_part(api, cat, "v0.1 BusBoard", "0.1", + "/tmp/img.png", fab_supplier=supplier) + + helper.assert_called_once_with( + api, new_part, supplier, "v0.1 BusBoard SMT Stencil rev 0.1", + ) + + +def test_create_assembly_part_does_not_attach_fab_supplier(): + """Assembly Module never gets supplier (built in-house).""" + api = MagicMock() + cat = _category() + new_part = _part(pk=1292, name="v0.1 BusBoard Module") + + with patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.Part") as PART, \ + patch("bom_export._ensure_fab_supplier_part") as helper: + PART.create.return_value = new_part + new_part.uploadImage = MagicMock(return_value=True) + create_assembly_part(api, cat, "v0.1 BusBoard", "0.1", "/tmp/img.png") + + helper.assert_not_called() + + +def test_dry_run_does_not_resolve_fab_supplier(tmp_path, monkeypatch): + """--dry-run must NOT call get_or_create_supplier (would auto-create Company). + + Same bug class as PR #31 for the order-importer: any write-on-missing + helper called before the dry-run gate violates the contract. + """ + csv_file = tmp_path / "test-bom.csv" + csv_file.write_text( + "Row,Description,Part,References,Value,Footprint,Quantity Per PCB,Status,Datasheet,LCSC,MOUSER\n" + "1,Resistor,R,R1,10k,R_0805_2012Metric,1, ,~,C17414,\n" + ) + pcb_img = tmp_path / "pcb.png" + pcb_img.write_text("dummy") + asm_img = tmp_path / "asm.png" + asm_img.write_text("dummy") + monkeypatch.setenv("INVENTREE_API_HOST", "http://localhost") + monkeypatch.setenv("INVENTREE_API_TOKEN", "deadbeef") + + monkeypatch.setattr(sys, "argv", [ + "bom_export.py", + "--csv_file", str(csv_file), + "--name", "TestBoard", + "--version", "1.0", + "--pcb_image", str(pcb_img), + "--assembly_image", str(asm_img), + "--pcb-supplier", "NewFab", + "--dry-run", + ]) + + with patch("bom_export.InvenTreeAPI"), \ + patch("bom_export.ensure_parts_exist"), \ + patch("bom_export.match_supplier_parts"), \ + patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.get_or_create_supplier") as gos, \ + patch("bom_export.load_category_map", return_value={}): + try: + main() + except SystemExit: + pass # normal exit from main() + + # The dry-run path must NOT fetch the fab supplier (would create Company) + gos.assert_not_called() + + +@pytest.mark.parametrize("supplier_value", ["", " ", "\t", "\n"]) +def test_blank_pcb_supplier_treated_as_opt_out( + supplier_value, tmp_path, monkeypatch, +): + """--pcb-supplier with empty/whitespace value normalises to opt-out. + + Without this guard a stray `--pcb-supplier " "` would POST a Company + with a blank name. Behaviour must match --no-pcb-supplier. + """ + csv_file = tmp_path / "test-bom.csv" + csv_file.write_text( + "Row,Description,Part,References,Value,Footprint,Quantity Per PCB,Status,Datasheet,LCSC,MOUSER\n" + "1,Resistor,R,R1,10k,R_0805_2012Metric,1, ,~,C17414,\n" + ) + pcb_img = tmp_path / "pcb.png" + pcb_img.write_text("dummy") + asm_img = tmp_path / "asm.png" + asm_img.write_text("dummy") + monkeypatch.setenv("INVENTREE_API_HOST", "http://localhost") + monkeypatch.setenv("INVENTREE_API_TOKEN", "deadbeef") + + monkeypatch.setattr(sys, "argv", [ + "bom_export.py", + "--csv_file", str(csv_file), + "--name", "TestBoard", + "--version", "1.0", + "--pcb_image", str(pcb_img), + "--assembly_image", str(asm_img), + "--pcb-supplier", supplier_value, + ]) + + with patch("bom_export.InvenTreeAPI"), \ + patch("bom_export.ensure_parts_exist"), \ + patch("bom_export.match_supplier_parts"), \ + patch("bom_export.find_part_by_name_and_revision", return_value=None), \ + patch("bom_export.get_category_by_name"), \ + patch("bom_export.create_pcb_part"), \ + patch("bom_export.create_assembly_part"), \ + patch("bom_export.create_stencil_part"), \ + patch("bom_export.ensure_related_parts"), \ + patch("bom_export.populate_bom"), \ + patch("bom_export.generate_cost_report"), \ + patch("bom_export.get_or_create_supplier") as gos, \ + patch("bom_export.load_category_map", return_value={}): + try: + main() + except SystemExit: + pass + + # Empty/whitespace name must not trigger Company auto-creation + gos.assert_not_called()