Sync with imported parts now properly updates augmented parts and creates any new attachment points

app/api/augmented_parts.py

@@ -86,8 +86,8 @@ async def sync_augmented_parts(
         resource_type="AugmentedPart",
         resource_id=None,
         description=(
-            f"Sync: {result.created} created, {result.archived} archived, "
+            f"Sync: {result.created} created, {result.updated} updated, "
-            f"{result.skipped} skipped (no type match)"
+            f"{result.archived} archived, {result.skipped} skipped (no type match)"
         ),
     )
     return result

app/schemas/augmented_part.py

@@ -58,6 +58,7 @@ class AugmentedPartUpdate(BaseModel):
 
 class AugmentedPartSyncResult(BaseModel):
     created: int
+    updated: int
     archived: int
     skipped: int
 

app/services/augmented_part_sync.py

@@ -9,7 +9,24 @@ Phase 1 – archive stale entries
     remaining associations (i.e. it was created purely from now-deleted
     ImportedParts), preserving its fit/attachment-point history.
 
-Phase 2 – create entries for new ImportedParts
+Phase 2 – refresh entries for already-linked ImportedParts
+  • For each ImportedPart already linked to an AugmentedPart (from a prior
+    sync run), refresh the fields sync itself owns — description, colour,
+    and uac_diameter_range_mm (see field mapping below) — to match the
+    ImportedPart's current values.
+  • Never touches "extra"/admin-owned AugmentedPart fields: part_type_id,
+    sub_part_type_id, scope_model_id, manufacturer_id, sku, archived. Those
+    are only ever set once, at creation (phase 3) or via the admin API/UI.
+  • Adds an "Ocular" and/or "Objective" attachment point if the part doesn't
+    already have one of that name and the current ImportedPart data now
+    supports deriving one. Never deletes or modifies an existing attachment
+    point, even if the source data that produced it has since changed.
+  • If an AugmentedPart has more than one backing ImportedPart, the one with
+    the lexicographically smallest InventoryID is used as the source (this
+    doesn't happen via any current creation path, but the data model and
+    phase 1's archiving logic both allow for it).
+
+Phase 3 – create entries for new ImportedParts
   • Any ImportedPart not yet linked to an AugmentedPart gets one created.
   • Description   ← ImportedPart.Description (fallback: InventoryID)
   • Colour        ← ImportedPart.TBXColour   (fallback: "Unknown"; created if needed)
@@ -40,6 +57,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.crud import attachment_point as attachment_point_crud
 from app.crud import attachment_point_name as attachment_point_name_crud
+from app.crud import augmented_part as augmented_part_crud
 from app.crud import clamp_connection as clamp_connection_crud
 from app.crud import iso_thread as iso_thread_crud
 from app.crud import manufacturer as manufacturer_crud
@@ -408,6 +426,52 @@ async def delete_imported_part_and_archive_orphans(
     return archived
 
 
+async def _refresh_augmented_part(
+    db: AsyncSession, ap: AugmentedPart, ip: ImportedPart
+) -> bool:
+    """Refresh the AugmentedPart fields sync owns (description, colour, UAC
+    diameter range) from the current state of `ip`, and add an "Ocular"
+    and/or "Objective" attachment point if the part doesn't already have one
+    and the current data now supports deriving one.
+
+    Never touches "extra" AugmentedPart fields (part_type_id, sub_part_type_id,
+    scope_model_id, manufacturer_id, sku, archived) and never deletes or
+    replaces an attachment point that already exists. Returns True if
+    anything changed.
+    """
+    description = (ip.Description or "").strip() or ip.InventoryID
+
+    colour_name = (ip.TBXColour or "").strip() or "Unknown"
+    colour = await part_colour_crud.get_by_name(db, colour_name)
+    if colour is None:
+        colour = await part_colour_crud.create(db, name=colour_name)
+
+    uac_range = _parse_uac_diameter_range(ip.UACDiameterRangemm)
+    uac_value = uac_range[0] if uac_range else None
+
+    changes = {}
+    if ap.description != description:
+        changes["description"] = description
+    if ap.colour_id != colour.id:
+        changes["colour_id"] = colour.id
+    if ap.uac_diameter_range_mm != uac_value:
+        changes["uac_diameter_range_mm"] = uac_value
+    if changes:
+        await augmented_part_crud.update(db, ap, changes)
+
+    existing_points = await attachment_point_crud.list_for_augmented_part(db, ap.id)
+    existing_names = {p.attachment_point_name.name for p in existing_points}
+    points_before = len(existing_points)
+
+    if ATTACHMENT_POINT_NAME not in existing_names:
+        await _add_attachment_point(db, ap, ip)
+    if UAC_ATTACHMENT_POINT_NAME not in existing_names:
+        await _add_uac_clamp_attachment_point(db, ap, ip)
+
+    points_after = await attachment_point_crud.list_for_augmented_part(db, ap.id)
+    return bool(changes) or len(points_after) > points_before
+
+
 async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
     # ------------------------------------------------------------------ phase 1
 
@@ -430,19 +494,45 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
 
     # ------------------------------------------------------------------ phase 2
 
-    # Re-query linked IDs after phase 1 cleanup
+    # Re-query linked IDs after phase 1 cleanup — these are the ImportedParts
+    # already linked to an AugmentedPart from a prior run (not created below).
     result = await db.execute(
         select(augmented_part_imported_parts.c.imported_part_id).distinct()
     )
     now_linked_ids: set[str] = {row[0] for row in result.all()}
     unlinked_ids = current_ids - now_linked_ids
 
+    updated = 0
+
+    if now_linked_ids:
+        result = await db.execute(
+            select(
+                augmented_part_imported_parts.c.augmented_part_id,
+                augmented_part_imported_parts.c.imported_part_id,
+            ).where(
+                augmented_part_imported_parts.c.imported_part_id.in_(now_linked_ids)
+            )
+        )
+        ip_ids_by_ap_id: dict[int, list[str]] = {}
+        for ap_id, ip_id in result.all():
+            ip_ids_by_ap_id.setdefault(ap_id, []).append(ip_id)
+
+        for ap_id, ip_ids in ip_ids_by_ap_id.items():
+            ap = await db.get(AugmentedPart, ap_id)
+            ip = await db.get(ImportedPart, min(ip_ids))
+            if ap is None or ip is None:
+                continue
+            if await _refresh_augmented_part(db, ap, ip):
+                updated += 1
+
+    # ------------------------------------------------------------------ phase 3
+
     created = 0
     skipped = 0
 
     if not unlinked_ids:
         return AugmentedPartSyncResult(
-            created=created, archived=archived, skipped=skipped
+            created=created, updated=updated, archived=archived, skipped=skipped
         )
 
     result = await db.execute(
@@ -497,4 +587,6 @@ async def sync(db: AsyncSession) -> AugmentedPartSyncResult:
 
     await db.commit()
 
-    return AugmentedPartSyncResult(created=created, archived=archived, skipped=skipped)
+    return AugmentedPartSyncResult(
+        created=created, updated=updated, archived=archived, skipped=skipped
+    )

app/web/admin.py

@@ -1789,7 +1789,8 @@ async def sync_augmented_parts(
                 resource_type="AugmentedPart",
                 resource_id=None,
                 description=(
-                    f"Sync: {result.created} created, {result.archived} archived, "
+                    f"Sync: {result.created} created, {result.updated} updated, "
+                    f"{result.archived} archived, "
                     f"{result.skipped} skipped (no type match)"
                 ),
             )
@@ -1797,6 +1798,7 @@ async def sync_augmented_parts(
                 job_id,
                 message=(
                     f"Sync complete: {result.created} created, "
+                    f"{result.updated} updated, "
                     f"{result.archived} archived"
                     + (
                         f", {result.skipped} skipped (type not recognised)"

app/web/pages.py

@@ -234,7 +234,8 @@ async def dashboard_sync_augmented_parts(
                 resource_type="AugmentedPart",
                 resource_id=None,
                 description=(
-                    f"Sync: {result.created} created, {result.archived} archived, "
+                    f"Sync: {result.created} created, {result.updated} updated, "
+                    f"{result.archived} archived, "
                     f"{result.skipped} skipped (no type match)"
                 ),
             )
@@ -242,6 +243,7 @@ async def dashboard_sync_augmented_parts(
                 job_id,
                 message=(
                     f"Sync complete: {result.created} created, "
+                    f"{result.updated} updated, "
                     f"{result.archived} archived"
                     + (
                         f", {result.skipped} skipped (type not recognised)"

tests/test_augmented_part_sync_refresh.py

@@ -0,0 +1,167 @@
+"""Sync should refresh already-linked AugmentedParts' sync-owned fields when
+their backing ImportedPart's data changes, add missing attachment points,
+and never touch "extra" admin-owned fields or existing attachment points."""
+
+import pytest
+from sqlalchemy import select
+
+from app.crud import attachment_point as attachment_point_crud
+from app.crud import part_colour as part_colour_crud
+from app.models.augmented_part import AugmentedPart
+from app.models.imported_part import ImportedPart
+from app.models.manufacturer import Manufacturer
+from app.models.part_colour import PartColour
+from app.models.part_type import PartType
+from app.services import augmented_part_sync
+
+
+async def _seed_reference_data(db):
+    manufacturer = Manufacturer(name="Acme", url="https://acme.test")
+    colour = PartColour(name="Black")
+    part_type = PartType(name="Scope")
+    db.add_all([manufacturer, colour, part_type])
+    await db.flush()
+    return manufacturer, colour, part_type
+
+
+async def _get_ap_by_sku(db, sku):
+    return (
+        await db.execute(select(AugmentedPart).where(AugmentedPart.sku == sku))
+    ).scalar_one()
+
+
+@pytest.mark.asyncio
+async def test_sync_refreshes_scalar_fields_on_already_linked_part(db_sessionmaker):
+    async with db_sessionmaker() as db:
+        await _seed_reference_data(db)
+        db.add(
+            ImportedPart(
+                InventoryID="INV-REFRESH-1",
+                Description="Old Scope description",
+                TBXColour="Black",
+            )
+        )
+        await db.commit()
+
+        result = await augmented_part_sync.sync(db)
+        assert result.created == 1
+        assert result.updated == 0
+
+        ap = await _get_ap_by_sku(db, "INV-REFRESH-1")
+        assert ap.description == "Old Scope description"
+        assert ap.uac_diameter_range_mm is None
+
+        # Simulate the source feed changing.
+        ip = await db.get(ImportedPart, "INV-REFRESH-1")
+        ip.Description = "New Scope description"
+        ip.TBXColour = "Tan"
+        ip.UACDiameterRangemm = "03505500"
+        await db.commit()
+
+        result2 = await augmented_part_sync.sync(db)
+        assert result2.created == 0
+        assert result2.updated == 1
+
+        await db.refresh(ap)
+        assert ap.description == "New Scope description"
+        assert ap.uac_diameter_range_mm == "03.50-55.00"
+        tan = await part_colour_crud.get_by_name(db, "Tan")
+        assert ap.colour_id == tan.id
+
+        # Re-running sync with no further source changes is a no-op.
+        result3 = await augmented_part_sync.sync(db)
+        assert result3.updated == 0
+
+
+@pytest.mark.asyncio
+async def test_sync_never_touches_extra_admin_owned_fields(db_sessionmaker):
+    async with db_sessionmaker() as db:
+        manufacturer, colour, part_type = await _seed_reference_data(db)
+        other_manufacturer = Manufacturer(name="OtherCo", url="https://other.test")
+        other_part_type = PartType(name="Flip Cover")
+        db.add_all([other_manufacturer, other_part_type])
+        await db.flush()
+
+        db.add(ImportedPart(InventoryID="INV-EXTRA-1", Description="A Scope widget"))
+        await db.commit()
+
+        result = await augmented_part_sync.sync(db)
+        assert result.created == 1
+
+        ap = await _get_ap_by_sku(db, "INV-EXTRA-1")
+
+        # Simulate an admin manually re-classifying the part after creation.
+        ap.part_type_id = other_part_type.id
+        ap.sub_part_type_id = None
+        ap.manufacturer_id = other_manufacturer.id
+        ap.archived = False
+        await db.commit()
+        admin_part_type_id = ap.part_type_id
+        admin_manufacturer_id = ap.manufacturer_id
+
+        # Source feed also changes, to prove refresh runs at all.
+        ip = await db.get(ImportedPart, "INV-EXTRA-1")
+        ip.Description = "A Scope widget, renamed"
+        await db.commit()
+
+        result2 = await augmented_part_sync.sync(db)
+        assert result2.updated == 1
+
+        await db.refresh(ap)
+        assert ap.description == "A Scope widget, renamed"  # sync-owned: refreshed
+        assert ap.part_type_id == admin_part_type_id  # extra field: untouched
+        assert ap.manufacturer_id == admin_manufacturer_id  # extra field: untouched
+        assert ap.sku == "INV-EXTRA-1"  # extra field: untouched
+
+
+@pytest.mark.asyncio
+async def test_sync_adds_missing_attachment_point_without_touching_existing_one(
+    db_sessionmaker,
+):
+    async with db_sessionmaker() as db:
+        await _seed_reference_data(db)
+        db.add(
+            ImportedPart(
+                InventoryID="INV-AP-1",
+                Description="A Scope widget",
+                # No thread/OD/UAC data yet — no attachment points created.
+            )
+        )
+        await db.commit()
+
+        result = await augmented_part_sync.sync(db)
+        assert result.created == 1
+
+        ap = await _get_ap_by_sku(db, "INV-AP-1")
+        points = await attachment_point_crud.list_for_augmented_part(db, ap.id)
+        assert points == []
+
+        # Source feed gains an OD value -> an "Ocular" point should appear.
+        ip = await db.get(ImportedPart, "INV-AP-1")
+        ip.ProductODmm = "40"
+        ip.ProductODin = "1.575"
+        await db.commit()
+
+        result2 = await augmented_part_sync.sync(db)
+        assert result2.updated == 1
+
+        points = await attachment_point_crud.list_for_augmented_part(db, ap.id)
+        assert len(points) == 1
+        ocular = points[0]
+        assert ocular.attachment_point_name.name == "Ocular"
+        original_static_connection_id = ocular.static_connection_id
+
+        # Source OD changes again — the existing point must NOT be replaced.
+        ip.ProductODmm = "56"
+        ip.ProductODin = "2.205"
+        await db.commit()
+
+        result3 = await augmented_part_sync.sync(db)
+        # description/colour/uac unchanged, and the Ocular point already
+        # exists, so nothing should be reported as updated.
+        assert result3.updated == 0
+
+        points = await attachment_point_crud.list_for_augmented_part(db, ap.id)
+        assert len(points) == 1
+        assert points[0].id == ocular.id
+        assert points[0].static_connection_id == original_static_connection_id