drawings: VISUAL's rev is the source of truth; attach it when M-Files holds it Effective or Retired

When M-Files' Effective rev differs from the rev VISUAL calls, the drawing
job looks that rev up (MFilesKit DrawingLookup.revision). If M-Files has
it as Effective or Retired, the job copies it into drawing_revisions and
attaches it to the draft with no flag and no Clerical approval, so the
draft can auto-release. Past draft, already attached or with a hand
drawing: unchanged, no flag and no change order.

Any other status, or no document at that rev, still flags rev_mismatch
with a note saying why. Packet Station only ever attaches Effective or
Retired drawings. rev_approve stays for that case only.

This replaces the "Use rev V as VISUAL calls it" choice from #46: the
use=called path in rev-approve and its button are removed. A packet that
already kept the rev that way keeps it, since that rev is now used anyway.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

tests/server/feedhelpers.py

@@ -8,6 +8,7 @@ once, the way the worker would, over a connection as the worker role, against th
 from __future__ import annotations
 
 import importlib
+import json
 import sys
 from datetime import date, timedelta
 from pathlib import Path
@@ -72,6 +73,17 @@ def fixture_dir(tmp: Path, today: date | None = None) -> Path:
     return out
 
 
+def set_revisions(directory: Path, dwg: str, revisions: dict[str, Any] | None, state: int = 1) -> None:
+    """Replace the `revisions` of one drawing in drawings.<state>.json (None removes them)."""
+    path = Path(directory) / f"drawings.{state}.json"
+    data = json.loads(path.read_text(encoding="utf-8"))
+    if revisions is None:
+        data[dwg].pop("revisions", None)
+    else:
+        data[dwg]["revisions"] = revisions
+    path.write_text(json.dumps(data, indent=1), encoding="utf-8")
+
+
 def switch_state(directory: Path, n: int | str) -> None:
     (Path(directory) / "state").write_text(str(n), encoding="utf-8")
 

tests/server/test_drawing_decide.py

@@ -15,11 +15,12 @@ from feedhelpers import (
     packet_id,
     run_drawings,
     run_feed,
+    set_revisions,
     system_audit,
 )
 from pshelpers import app_conn, needs_pg
 
-from packetstation.drawings.decide import Decision, Target, decide, rev_confirmed
+from packetstation.drawings.decide import Called, Decision, Target, decide, rev_confirmed
 from packetstation.drawings.fake import FixtureLookup
 from packetstation.drawings.lookup import Found
 from packetstation.settings import Settings
@@ -129,24 +130,44 @@ def test_approved_other_rev_still_mismatches() -> None:
     assert d(target(visual_rev="2", rev_ok="4"), found(rev=3)).kind == "rev_mismatch"
 
 
-def test_kept_called_rev_stays_while_the_effective_rev_is_the_same() -> None:
-    t = target(visual_rev="2", drawing_state="attached", attached_rev="2", kept_latest="3")
-    r = d(t, found(rev=3))
+RETIRED_2 = Called(2, "Retired", same_file=False)
+
+
+def test_visual_rev_retired_in_m_files_is_attached_without_a_flag() -> None:
+    t = target(visual_rev="2", drawing_state="rev_mismatch")
+    r = decide(t, found(rev=3), same_file=False, called=RETIRED_2)
+    assert (r.kind, r.flag, r.drawing_state) == ("attach_called", None, "attached")
+    assert r.note == "Rev 2 is Retired in M-Files; VISUAL calls it (M-Files has rev 3)"
+    assert rev_confirmed(t, found(rev=3), RETIRED_2)
+    assert not rev_confirmed(t, found(rev=3))
+
+
+def test_visual_rev_retired_already_attached_stays() -> None:
+    t = target(visual_rev="2", drawing_state="attached", attached_rev="2")
+    r = decide(t, found(rev=3), same_file=False, called=Called(2, "Retired", same_file=True))
     assert (r.kind, r.flag, r.drawing_state) == ("unchanged", None, "attached")
-    assert r.note == "Rev 2 kept by Clerical; M-Files has rev 3"
-    assert rev_confirmed(t, found(rev=3))
 
 
-def test_kept_called_rev_opens_no_change_order_past_draft() -> None:
-    t = target(visual_rev="2", status="ready", drawing_state="attached", attached_rev="2", kept_latest="3")
-    assert d(t, found(rev=3)).kind == "unchanged"
+def test_visual_rev_retired_past_draft_and_hand_drawing_stay() -> None:
+    shop = target(visual_rev="2", status="ready", drawing_state="attached", attached_rev="2")
+    assert decide(shop, found(rev=4), same_file=False, called=RETIRED_2).kind == "unchanged"  # no CO, no flag
+    hand = target(visual_rev="2", drawing_state="hand", has_hand_drawing=True)
+    r = decide(hand, found(rev=3), same_file=False, called=RETIRED_2)
+    assert (r.kind, r.flag, r.drawing_state) == ("unchanged", None, "hand")
 
 
-def test_kept_called_rev_flags_again_on_a_newer_effective_rev() -> None:
-    t = target(visual_rev="2", drawing_state="attached", attached_rev="2", kept_latest="3")
-    r = d(t, found(rev=4))
-    assert (r.kind, r.flag, r.drawing_state) == ("rev_mismatch", "rev_mismatch", "attached")
-    assert not rev_confirmed(t, found(rev=4))
+def test_visual_rev_retired_with_no_effective_rev() -> None:
+    r = decide(target(visual_rev="2"), found(rev=None, name=None), same_file=False, called=RETIRED_2)
+    assert r.kind == "attach_called" and r.note.endswith("(no Effective rev in M-Files)")
+
+
+def test_an_approved_effective_rev_wins_over_the_called_rev() -> None:
+    r = decide(target(visual_rev="2", rev_ok="3"), found(rev=3), same_file=False, called=RETIRED_2)
+    assert (r.kind, r.flag) == ("attach", None)
+
+
+def test_no_called_copy_still_flags_the_mismatch() -> None:
+    assert decide(target(visual_rev="2"), found(rev=3), same_file=False, called=None).kind == "rev_mismatch"
 
 
 def test_match_on_a_draft_attaches() -> None:
@@ -266,24 +287,23 @@ def test_job_attaches_and_flags(fx: Settings) -> None:
         lay = packet_by_key(conn, gen.KEY_LAYOUT_A)
         assert lay is not None and lay["drawing_state"] == "attached"
 
-        mm = packet_by_key(conn, gen.KEY_TWO_OPS)  # VISUAL rev 2, M-Files rev 3
-        assert mm is not None and "rev_mismatch" in mm["flags"] and mm["drawing_state"] == "rev_mismatch"
-        assert mm["visual"]["appr"]["rev"] == 3
-        staged = conn.execute(
-            "SELECT asset_id IS NOT NULL, rev FROM drawing_lookups WHERE dwg = 'L900-0201'"
-        ).fetchone()
-        assert staged == (True, 3)  # staged for Clerical Supervisor approval
-        assert _drawing_pages(conn, mm["id"]) == []
-        called = mm["visual"]["called"]  # VISUAL's rev 2, Retired in M-Files, copied for Clerical's choice
+        # VISUAL rev 2, M-Files' Effective rev 3, rev 2 Retired in M-Files: VISUAL wins, rev 2 is attached, no flag
+        mm = packet_by_key(conn, gen.KEY_TWO_OPS)
+        assert mm is not None and not set(mm["flags"]) & DRAWING_FLAGS and mm["drawing_state"] == "attached"
+        assert mm["visual"]["appr"]["rev"] == 3 and mm["visual"]["rev_confirmed"] is True
+        assert mm["visual"]["vault"]["rev"] == 2 and mm["visual"]["vault"]["controlled"] == "Retired"
+        pages = _drawing_pages(conn, mm["id"])
+        assert pages and all(p[2] == "L900-0201" and p[3] == "2" for p in pages)
+        called = mm["visual"]["called"]
         assert (called["rev"], called["controlled"], called["state"]) == ("2", "Retired", "Superseded")
-        assert called["asset_id"] and called["pages"] >= 1
         rv = conn.execute(
             "SELECT controlled, asset_id IS NOT NULL FROM drawing_revisions WHERE dwg = 'L900-0201' AND rev = 2"
         ).fetchone()
         assert rv == ("Retired", True)
         assert conn.execute(
-            "SELECT count(*) FROM packet_log WHERE packet_id = %s AND note LIKE %s",
-            (mm["id"], "%rev 2 is Retired in M-Files%"),
+            "SELECT count(*) FROM packet_log WHERE packet_id = %s AND action = 'Drawing attached from M-Files'"
+            " AND note LIKE %s",
+            (mm["id"], "%Rev 2 is Retired in M-Files; VISUAL calls it (M-Files has rev 3)%"),
         ).fetchone()[0]
 
         nr = packet_by_key(conn, gen.KEY_DRAFTING)
@@ -305,6 +325,33 @@ def test_job_attaches_and_flags(fx: Settings) -> None:
         assert system_audit(conn, "drawing.fetched")
 
 
+@needs_pg
+@pytest.mark.parametrize(
+    ("revisions", "note"),
+    [
+        (None, "rev 2 is not in M-Files"),
+        (
+            {"2": {"file": "L900-0201 r2.pdf", "status": "Obsolete", "obj_id": 32, "file_id": 132}},
+            "rev 2 is Obsolete in M-Files - not attached",
+        ),
+    ],
+)
+def test_job_flags_when_the_visual_rev_cannot_be_attached(
+    fx: Settings, revisions: dict[str, Any] | None, note: str
+) -> None:
+    set_revisions(fx.fixture_dir, "L900-0201", revisions)
+    run_feed(fx, 1)
+    run_drawings(fx)
+    with app_conn(fx) as conn:
+        mm = packet_by_key(conn, gen.KEY_TWO_OPS)
+        assert mm is not None and "rev_mismatch" in mm["flags"] and mm["drawing_state"] == "rev_mismatch"
+        assert _drawing_pages(conn, mm["id"]) == []
+        assert conn.execute("SELECT count(*) FROM drawing_revisions").fetchone()[0] == 0  # nothing copied
+        assert conn.execute(
+            "SELECT count(*) FROM packet_log WHERE packet_id = %s AND note LIKE %s", (mm["id"], f"%{note}%")
+        ).fetchone()[0]
+
+
 @needs_pg
 def test_job_second_run_changes_nothing(fx: Settings) -> None:
     run_feed(fx, 1)

tests/server/test_feed_api.py

@@ -22,6 +22,7 @@ from feedhelpers import (
     run_drawings,
     run_feed,
     set_feed,
+    set_revisions,
 )
 from pkthelpers import SIGN, seed_users, tiny_pdf, upload
 from pshelpers import ORIGIN_HEADERS, app_conn, needs_pg, sign_in
@@ -176,6 +177,7 @@ def test_batches_and_changes(client: TestClient, fx: Settings) -> None:
 
 
 def test_rev_approve(client: TestClient, fx: Settings) -> None:
+    set_revisions(fx.fixture_dir, "L900-0201", None)  # M-Files has no rev 2 at all: the mismatch stays flagged
     run_feed(fx, 1)
     run_drawings(fx)
     with app_conn(fx) as conn:
@@ -221,49 +223,6 @@ def test_rev_approve(client: TestClient, fx: Settings) -> None:
         assert p["drawing_state"] == "attached" and "rev_mismatch" not in p["flags"]
 
 
-def test_rev_approve_keeps_the_rev_visual_calls(client: TestClient, fx: Settings) -> None:
-    run_feed(fx, 1)
-    run_drawings(fx)
-    with app_conn(fx) as conn:
-        pid = packet_id(conn, gen.KEY_TWO_OPS)  # VISUAL rev 2 (Retired in M-Files), M-Files rev 3
-        nf = packet_id(conn, gen.KEY_NOT_FOUND)
-    body = {"reason": "Engineering says build lot 1 to rev 2", "sign": SIGN, "use": "called"}
-    as_user(client, "clerk.leo")
-    r = client.post(f"/api/packets/{nf}/rev-approve", json=body)
-    assert r.status_code == 409 and r.json()["code"] == "nothing_staged", r.text
-    r = client.post(f"/api/packets/{pid}/rev-approve", json={**body, "use": "oldest"})
-    assert r.status_code == 422, r.text
-    r = client.post(f"/api/packets/{pid}/rev-approve", json=body)
-    assert r.status_code == 200, r.text
-    with app_conn(fx) as conn:
-        p = packet_row(conn, pid)
-        assert p["drawing_state"] == "attached" and "rev_mismatch" not in p["flags"]
-        ok = p["visual"]["rev_ok"]
-        assert (ok["rev"], ok["vis_rev"], ok["use"], ok["latest"], ok["controlled"]) == (
-            "2",
-            "2",
-            "called",
-            "3",
-            "Retired",
-        )
-        assert p["visual"]["rev_confirmed"] is True
-        pages = drawing_pages(conn, pid)
-        assert pages and all(pg[1] == "2" for pg in pages)
-        log = conn.execute(
-            "SELECT note FROM packet_log WHERE packet_id = %s AND action = 'Rev VISUAL calls kept by hand'", (pid,)
-        ).fetchall()
-        assert len(log) == 1 and "Retired in M-Files" in log[0][0] and "M-Files has rev 3" in log[0][0]
-    # the drawing job leaves the kept rev alone while M-Files' Effective rev is still 3
-    with app_conn(fx) as conn:
-        conn.execute("UPDATE drawing_lookups SET checked_at = now() - interval '2 days', next_check_at = now()")
-    run_drawings(fx)
-    with app_conn(fx) as conn:
-        p = packet_row(conn, pid)
-        assert p["drawing_state"] == "attached" and "rev_mismatch" not in p["flags"]
-        assert p["visual"]["rev_confirmed"] is True and p["visual"].get("called") is None
-        assert all(pg[1] == "2" for pg in drawing_pages(conn, pid))
-
-
 # ---------------------------------------------------------------- hand uploads
 
 
@@ -325,6 +284,7 @@ def _hand_bom(client: TestClient, pid: int, data: bytes, name: str, **form: str)
 
 @pytest.mark.parametrize("kind", ["xlsx", "csv"])
 def test_hand_bom(client: TestClient, fx: Settings, kind: str) -> None:
+    set_revisions(fx.fixture_dir, "L900-0201", None)  # M-Files has no rev 2: the mismatch stays flagged
     run_feed(fx, 1)
     run_drawings(fx)
     with app_conn(fx) as conn:

tests/server/test_feed_apply.py

@@ -484,8 +484,11 @@ def test_auto_release(fx: Settings) -> None:
         assert wh["stage"] == "incoming" and wh["by"] is None
         tr = row(conn, "SELECT * FROM packet_transitions WHERE packet_id = %s", (wp["id"],))
         assert tr["action"] == "vis_auto_release" and tr["by"] is None
+        # VISUAL calls rev 2, which M-Files holds as Retired: attached without a flag, so it is released too
+        two = packet_by_key(conn, gen.KEY_TWO_OPS)
+        assert two is not None and two["status"] == "warehouse" and two["visual"]["vault"]["controlled"] == "Retired"
         # flagged drafts stay with Clerical
-        for key in (gen.KEY_TWO_OPS, gen.KEY_NOT_FOUND, gen.KEY_DRAFTING, gen.KEY_NO_DWG):
+        for key in (gen.KEY_NOT_FOUND, gen.KEY_DRAFTING, gen.KEY_NO_DWG):
             p = packet_by_key(conn, key)
             assert p is not None and p["status"] == "draft", key
         assert all(r[0] == "system" for r in system_audit(conn, "packet.vis_auto_release"))