From 13b64a2d17e7c01e753bddece3cfacbcc1d75c21 Mon Sep 17 00:00:00 2001 From: Ian Roddis <31021769+iroddis@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:50:06 -0300 Subject: [PATCH] Sync with imported parts now properly updates augmented parts and creates any new attachment points --- app/api/augmented_parts.py | 4 +- app/schemas/augmented_part.py | 1 + app/services/augmented_part_sync.py | 100 ++++++++++++- app/web/admin.py | 4 +- app/web/pages.py | 4 +- tests/test_augmented_part_sync_refresh.py | 167 ++++++++++++++++++++++ 6 files changed, 272 insertions(+), 8 deletions(-) create mode 100644 tests/test_augmented_part_sync_refresh.py diff --git a/app/api/augmented_parts.py b/app/api/augmented_parts.py index 9d7e740..28ba812 100644 --- a/app/api/augmented_parts.py +++ b/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"{result.skipped} skipped (no type match)" + f"Sync: {result.created} created, {result.updated} updated, " + f"{result.archived} archived, {result.skipped} skipped (no type match)" ), ) return result diff --git a/app/schemas/augmented_part.py b/app/schemas/augmented_part.py index ab738bb..1a393ae 100644 --- a/app/schemas/augmented_part.py +++ b/app/schemas/augmented_part.py @@ -58,6 +58,7 @@ class AugmentedPartUpdate(BaseModel): class AugmentedPartSyncResult(BaseModel): created: int + updated: int archived: int skipped: int diff --git a/app/services/augmented_part_sync.py b/app/services/augmented_part_sync.py index 6979838..f8720ab 100644 --- a/app/services/augmented_part_sync.py +++ b/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 + ) diff --git a/app/web/admin.py b/app/web/admin.py index 97a7b91..6293aa5 100644 --- a/app/web/admin.py +++ b/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)" diff --git a/app/web/pages.py b/app/web/pages.py index a00d00a..0abd082 100644 --- a/app/web/pages.py +++ b/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)" diff --git a/tests/test_augmented_part_sync_refresh.py b/tests/test_augmented_part_sync_refresh.py new file mode 100644 index 0000000..209ab02 --- /dev/null +++ b/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