diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ed2474..3ebdd5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,12 @@ All notable changes to this project are recorded here. The format follows [Keep - Board columns are frames about three cards tall (700 px, never taller than the screen) that scroll on their own, with the column heading staying put, so a long column no longer pushes the next one far down the page. Narrow screens keep the plain one-column flow. - Sign-in refused as cross-origin in Firefox: the app sent `Referrer-Policy: no-referrer`, under which Firefox sends `Origin: null` on same-origin POSTs. The policy is now `same-origin`, which still sends nothing to other sites. +### Changed + +- VISUAL is the source of truth for the drawing rev. When M-Files' Effective rev differs from the rev VISUAL calls but M-Files still holds that rev as Retired (or Effective), the drawing job attaches it to the draft with no flag and no Clerical approval, and the packet page says the drawing is retired in M-Files. Any other M-Files status, or no document at that rev, still flags `rev_mismatch`; Packet Station only ever attaches Effective or Retired drawings. This replaces the "Use rev V as VISUAL calls it" choice from issue 41. Needs MFilesKit 0.1.3 (`DrawingLookup.revision`) and migration 0009. + ### Added -- Rev mismatch with a retired rev (Gitea issue 41): when VISUAL calls a rev that M-Files still holds but has retired, the packet is still flagged, and a Clerical Supervisor can choose either rev from the packet: "Use latest rev N" (M-Files' Effective rev, as before) or "Use rev V as VISUAL calls it". Both PDFs open from the packet first. Keeping VISUAL's rev needs a reason and a signature, is logged, and holds until M-Files releases a newer rev or VISUAL's rev changes. Needs MFilesKit 0.1.3 (`DrawingLookup.revision`) and migration 0009. - Phase 8: Project Management, the find bar, presence and motion. The Project Management screen (`/pm`, view only) with the tabs Overview (the headline numbers, a plain-words summary, the pipeline by step with the slowest step in pink, the units late or at risk), Projects & units (every project with its units, due date, days left, packets by step, % done and health: On track, At risk, Late, Complete, No packets yet; a project is as healthy as its worst unit; tap a project for its units and a unit for its jobs), Bottlenecks & aging (time in the current step, every open packet longest waiting first with over 3 days flagged, average time per step for completed moves), Hours vs target (station timer hours and VISUAL ticket hours side by side against the work-order target by department, worker and project, plus every packet over target), Quality & issues (Q.C. rejects, open shorts, open change orders, warehouse holds, sent back) and Find jobs. The period picker (30 days, 90 days, 12 months, all) sets the window for history figures. Every table has Excel (CSV); "Excel (CSV) — all" saves every report; "PDF report" builds the PESCO production status report (cover with the headline numbers, summary and pipeline, projects, flow, labor, issues) on the server with the controlled-copy stamp. Every export is logged as "report exported" in the audit and the print log. - The find bar on every home screen: search over project number or name, lot, operation, drawing and rev, WPS, customer, PO, old project numbers, work orders, heat numbers, part IDs, welder name or stamp, change order and ECN numbers, with several words narrowing and Esc clearing; the project then unit picker; Filters with Department, Status (including At a station and Everything not filed), Station, Worker and, for Clerical and Project Management, the flag kind; the quick flags Change order open, Shorts open, Project overdue, Due within 7 days and Mine (what that screen is responsible for; hidden on the station and Project Management screens); Board or List with the list sortable by last updated, project / lot, due date, status or department. Filters and the view are saved per browser (`ps.flt`); the search text is not. Tapping a number tile shows only those jobs; tapping again clears. The numbers follow the project pick but not the other filters. diff --git a/docs/FEED.md b/docs/FEED.md index 45ad808..173f294 100644 --- a/docs/FEED.md +++ b/docs/FEED.md @@ -291,8 +291,8 @@ The file is downloaded only when the object id and version or the file id and ve | No released rev, but a drafting rev or a drafting assignment | `not_released` | `not_released` | Note "Not released in M-Files yet (sheet of ) - drafting status: rev N ". | | No released rev and nothing in drafting | `not_found` | `not_found` | Note "Drawing not in M-Files - check the drawing number in VISUAL". | | VISUAL's rev is not a whole number | `bad_rev` | `bad_rev` | Not attached. | -| Revs differ, and no approved rev matches M-Files | `rev_mismatch` | `rev_mismatch` | Note "Rev mismatch: VISUAL calls rev V, M-Files has rev N", plus the drafting status. The M-Files copy is fetched and staged for Clerical Supervisor approval. On a draft the rev VISUAL calls is looked up too: when M-Files still holds it (Retired, say), that copy is fetched into `drawing_revisions` and kept on the packet as `visual.called`, and the log note says "rev V is Retired in M-Files". When M-Files has no document at that rev, the note says "rev V is not in M-Files". No change order: VISUAL did not change. | -| Revs differ, but Clerical kept the rev VISUAL calls while M-Files' Effective rev is still the one they chose against | `unchanged` | none | The kept rev stays attached and confirmed. No flag and no change order, also past draft. A newer Effective rev in M-Files flags the packet again. | +| Revs differ, and no approved rev matches M-Files, and M-Files holds the rev VISUAL calls as Effective or Retired | `attach_called` | none | VISUAL is the source of truth. On a draft the rev VISUAL calls is fetched into `drawing_revisions` and attached, with no flag and no Clerical approval: `drawing_state` `attached`, rev confirmed, `visual.vault.controlled` set (for example Retired), `visual.called` recorded. Log "Drawing attached from M-Files" with the note "Rev V is Retired in M-Files; VISUAL calls it (M-Files has rev N)". Already attached, a hand drawing, or past draft: unchanged, no flag and no change order. The same holds when M-Files has no Effective rev at all. | +| Revs differ, and no approved rev matches M-Files, and M-Files has no document at the rev VISUAL calls, or has it at another status (Released, Obsolete) | `rev_mismatch` | `rev_mismatch` | Note "Rev mismatch: VISUAL calls rev V, M-Files has rev N", plus the drafting status, plus "rev V is not in M-Files" or "rev V is in M-Files - not attached". Only an Effective or Retired drawing is ever attached. The M-Files copy is fetched and staged for Clerical Supervisor approval. No change order: VISUAL did not change. | | Revs match (or the approved rev matches), draft | `attach` | none | Drawing pages attached, `drawing_state` `attached`, drawing flags cleared, rev confirmed. Log "Drawing attached from M-Files". | | Revs match, draft with a hand drawing at the same rev | `unchanged` | none | The hand drawing stays. | | Revs match, past draft, same file (same sha256) | `unchanged` | none | Nothing. | @@ -323,7 +323,7 @@ Flags live in `packets.flags`. A draft with any flag shows in Clerical's Flagged | Flag | Text shown | What Clerical does | |---|---|---| | `drawing_needed` | Made from VISUAL - drawing not attached yet | Usually nothing: the drawing job attaches it. Run now to hurry it. Upload by hand only if it stays stuck. | -| `rev_mismatch` | The M-Files note, for example "Rev mismatch: VISUAL calls rev 2, M-Files has rev 3" | Get the rev corrected in VISUAL. Or a Clerical Supervisor chooses, with a reason and a signature: the latest (Effective) rev in M-Files, or the rev VISUAL calls when M-Files still holds it (Retired). Both PDFs can be opened from the packet first. | +| `rev_mismatch` | The M-Files note, for example "Rev mismatch: VISUAL calls rev 2, M-Files has rev 3; rev 2 is not in M-Files" | M-Files has no usable copy of the rev VISUAL calls. Get the rev corrected in VISUAL, or a Clerical Supervisor approves the staged M-Files rev with a reason and a signature. (When M-Files holds the rev VISUAL calls as Effective or Retired, it is attached automatically and the packet is not flagged.) | | `not_released` | The note: "Not released in M-Files yet ... drafting status: rev N " | Wait for drafting to release it, or upload the drawing by hand with a reason and a signature. | | `not_found` | Drawing not in M-Files - check the drawing number in VISUAL | Check the drawing number in VISUAL and have it fixed there. Or upload by hand. | | `lookup_failed` | M-Files lookup failed: (the reason) | Nothing at first: the job retries with backoff. If it persists, tell IT. | diff --git a/docs/POLICY.md b/docs/POLICY.md index 0de7301..48a1f3b 100644 --- a/docs/POLICY.md +++ b/docs/POLICY.md @@ -867,7 +867,7 @@ What the feed may never write: When M-Files has a different released rev from VISUAL, the drawing job stages the M-Files copy. A Clerical Supervisor (PS-Clerical-Supervisor) can approve it on a draft with rev_approve: a reason of at least 8 characters and a signature ("Approve the M-Files revision"). The pages are attached and the approval is kept on the packet (`visual.rev_ok`). With nothing staged the answer is 409 `nothing_staged`. -The same action, with `use: "called"`, keeps the rev VISUAL calls instead, when M-Files still holds that rev at another status (Retired). The drawing job copies it on a draft flagged `rev_mismatch`. The same role, reason and signature apply, and the signed content carries `use`. `visual.rev_ok` records `use: "called"`, the M-Files status of the kept rev and the Effective rev at the time (`latest`). The log says "Rev VISUAL calls kept by hand". The choice holds while M-Files' Effective rev stays `latest`: no flag and no change order. A newer Effective rev flags the packet again, and a new VISUAL rev clears the choice. With no copy of that rev the answer is 409 `nothing_staged`. +VISUAL is the source of truth for the rev. When M-Files holds the rev VISUAL calls as Effective or Retired, the drawing job attaches it to the draft with no flag, so there is nothing for Clerical to approve (see [FEED.md](FEED.md), the decision table). rev_approve is only for a mismatch where M-Files has no usable copy of that rev. Packet Station never attaches a drawing at any other M-Files status. A packet whose `visual.rev_ok` has `use: "called"` (Clerical kept the rev VISUAL calls by hand, in the build before this rule) keeps it: that rev is now used anyway. ### Hand uploads on flagged drafts diff --git a/docs/SCHEMA.md b/docs/SCHEMA.md index a1e16e7..5cae3d8 100644 --- a/docs/SCHEMA.md +++ b/docs/SCHEMA.md @@ -377,7 +377,7 @@ Migration 0004. Every ID part is text (`wo_base`, `lot`, `split`, `sub`, part ID - `visual_ignored_wos`: work orders kept out by the go-live date, unique on (`wo_base`, `lot`, `split`, `sub`). `rel`, `project`, `unit_number`, `res`, `dwg`, `first_seen_batch`, `last_seen_at`, and `pulled_in_at`, `pulled_in_by`, `pulled_in_reason` when Clerical pulls one in. - `visual_resource_map`: `resource` (PK), `target` (`fold`, `ignore` or a two-digit department code, by CHECK), `confirmed`, `updated_at`, `updated_by`. Seeded from the old build's `visual_map.json`, every entry unconfirmed. "Electical" is spelled that way in VISUAL. - `drawing_lookups`: one row per drawing number as VISUAL writes it (`dwg` PK, trimmed). The last answer: `checked_at`, `ok`, `rev` (released), `name`, `obj_id`, `obj_version`, `file_id`, `file_version`, `file_size`, `base` (H sheet), `draft_rev`, `draft_where`, `in_drafting`, `note`. Retry state: `fail_count`, `failing_since`, `next_check_at`. The fetched file: `asset_id`, `sha256`. `visual_rev_seen` is the VISUAL rev at the last lookup, so a rev change moves the drawing to the front of the queue. A rev mismatch keeps the staged M-Files copy here for approval. -- `drawing_revisions` (0009): other revs of a drawing that M-Files still holds, one row per (`dwg`, `rev`). The drawing job writes it when a draft's VISUAL rev differs from the Effective rev and M-Files has a document at the VISUAL rev. `controlled` (M-Files' Controlled Status, for example Retired), `state` (workflow state, for example Superseded), `checked_at`, `name`, the M-Files identity (`obj_id`, `obj_version`, `file_id`, `file_version`, `file_size`), `base` (H sheet), and the fetched copy (`asset_id`, `sha256`). rev_approve with `use: "called"` attaches from here. +- `drawing_revisions` (0009): other revs of a drawing that M-Files still holds, one row per (`dwg`, `rev`). The drawing job writes it when a packet's VISUAL rev differs from the Effective rev and M-Files has that rev as Effective or Retired, and attaches drafts from it. `controlled` (M-Files' Controlled Status, for example Retired), `state` (workflow state, for example Superseded), `checked_at`, `name`, the M-Files identity (`obj_id`, `obj_version`, `file_id`, `file_version`, `file_size`), `base` (H sheet), and the fetched copy (`asset_id`, `sha256`). Column additions in 0004: diff --git a/frontend/src/api/packets.ts b/frontend/src/api/packets.ts index 1c29f39..a42ba62 100644 --- a/frontend/src/api/packets.ts +++ b/frontend/src/api/packets.ts @@ -68,8 +68,7 @@ export const packetsApi = { databook: (id: number) => api.post(`${base(id)}/databook`, {}), /* Phase 3: VISUAL packets. */ - revApprove: (id: number, body: { reason: string; use?: "latest" | "called" } & Guarded) => - api.post(`${base(id)}/rev-approve`, body), + revApprove: (id: number, body: { reason: string } & Guarded) => api.post(`${base(id)}/rev-approve`, body), handDrawing: (id: number, body: { asset_id: string; reason: string } & Guarded) => api.post(`${base(id)}/hand/drawing`, body), /** Multipart cannot carry JSON, so the signature rides as sign_factor and sign_secret. */ diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 46b37f5..cb13b8a 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -905,11 +905,13 @@ export interface FoldedOp { tgt?: number | string; } -/** The rev VISUAL calls, as M-Files holds it at another status (Retired, say). Set by the drawing job on a draft flagged - * rev_mismatch. `missing` when M-Files has no document at that rev. */ +/** The rev VISUAL calls, as M-Files holds it when that is not the Effective rev. Set by the drawing job. A copy (Effective + * or Retired) is attached automatically; `missing` when M-Files has no document at that rev, `refused` when its status + * may not be attached. */ export interface CalledRev { rev: string; missing?: boolean; + refused?: boolean; controlled?: string; state?: string | null; name?: string | null; @@ -923,6 +925,9 @@ export interface VisualInfo { /** Set by the read model from drawing_lookups when M-Files has a staged copy. */ staged_rev?: { rev: number | string; name: string | null; asset_id?: string }; called?: CalledRev | null; + /** The M-Files copy attached by the drawing job. `controlled` is set when it is the rev VISUAL calls at a status other + * than Effective's newest (Retired, say). */ + vault?: { asset_id: string; name?: string | null; rev: number | string; note?: string | null; at?: string; controlled?: string } | null; partspecs?: string[]; folded?: FoldedOp[]; manual_dwg?: HandUpload; diff --git a/frontend/src/packet/visual.test.tsx b/frontend/src/packet/visual.test.tsx index 24f4e8a..dcfba55 100644 --- a/frontend/src/packet/visual.test.tsx +++ b/frontend/src/packet/visual.test.tsx @@ -116,29 +116,31 @@ describe("Packet page, VISUAL box", () => { expect(await screen.findByText("Rev 3 approved and attached.")).toBeInTheDocument(); }); - it("offers the retired rev VISUAL calls next to the latest, and keeps it with use=called", async () => { - const called = { rev: "2", controlled: "Retired", state: "Superseded", name: "M00543 r2.pdf", asset_id: "as-r2", pages: 1 }; - const draft = visualDraft({ visual: { ...visualDraft().visual, staged_rev: { rev: 3, name: "M00543 r3.pdf", asset_id: "as-r3" }, called } }); - const kept = visualDraft({ - version: 4, + it("says when the rev VISUAL calls is attached although M-Files has retired it", async () => { + const page = { n: 1, type: "drawing", asset_id: "as-r2", pn: 1, dwg: "M00543", rev: "2", void: null, url: "/_blob/as-r2" }; + const attached = visualDraft({ flags: [], flag: null, drawing_state: "attached", - visual: { rev_ok: { rev: "2", vis_rev: "2", use: "called", latest: "3", controlled: "Retired", reason: "Lot 1 builds to rev 2" } }, + pages: [page] as unknown as PacketRead["pages"], + visual: { + appr: { ok: true, rev: 3, vrev: "2", name: "M00543 r3.pdf", note: "" }, + vault: { asset_id: "as-r2", name: "M00543 r2.pdf", rev: 2, controlled: "Retired" }, + }, }); - const fetch = mockFetch({ ...base(draft, LEO), "POST /api/packets/1/rev-approve": { body: kept } }); + mockFetch({ ...base(attached, LEO) }); renderApp("/packets/1"); - expect(await screen.findByRole("button", { name: "Use latest rev 3" })).toBeInTheDocument(); - expect(screen.getByText(/retired \(Superseded\) in M-Files/)).toBeInTheDocument(); - fireEvent.click(screen.getByRole("button", { name: "Use rev 2 as VISUAL calls it" })); - const ask = await screen.findByRole("dialog", { name: "Use rev 2 for M00543" }); - fireEvent.change(within(ask).getByLabelText("Reason"), { target: { value: "Lot 1 builds to rev 2" } }); - fireEvent.click(within(ask).getByRole("button", { name: "Use rev 2" })); - await sign(); - expect(callsTo(fetch, "POST", "/api/packets/1/rev-approve")).toEqual([ - { reason: "Lot 1 builds to rev 2", use: "called", sign: { factor: "password", secret: "pw-1" } }, - ]); - expect(await screen.findByText("Rev 2 attached as VISUAL calls it.")).toBeInTheDocument(); + expect(await screen.findByText("Rev 2 attached as VISUAL calls it")).toBeInTheDocument(); + expect(screen.getByText(/retired in M-Files, which has rev 3 as Effective/)).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /Approve rev/ })).not.toBeInTheDocument(); + }); + + it("explains why the rev VISUAL calls is not attached when its status is not Effective or Retired", async () => { + const draft = visualDraft({ visual: { ...visualDraft().visual, called: { rev: "2", controlled: "Obsolete", refused: true } } }); + mockFetch({ ...base(draft, LEO) }); + renderApp("/packets/1"); + expect(await screen.findByText(/VISUAL calls rev 2, which is obsolete in M-Files/)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Approve rev 3 for this packet" })).toBeInTheDocument(); }); it("uploads a hand BOM as multipart with the signature fields", async () => { diff --git a/frontend/src/packet/visual.tsx b/frontend/src/packet/visual.tsx index f3d42e6..eeb5885 100644 --- a/frontend/src/packet/visual.tsx +++ b/frontend/src/packet/visual.tsx @@ -163,7 +163,9 @@ export function VisualBox({ const drawingFlag = flags.some((f) => (DRAWING_FLAGS as readonly string[]).includes(f)); const appr = v.appr; const staged = v.staged_rev; - const called = v.called && !v.called.missing && v.called.asset_id ? v.called : null; + const called = v.called ?? null; + const vault = v.vault; + const retiredAttached = hasDwg && !drawingFlag && !!vault?.controlled && vault.controlled.toLowerCase() !== "effective"; const mismatch = flags.includes("rev_mismatch"); const md = v.manual_dwg; const mb = v.manual_bom; @@ -188,25 +190,6 @@ export function VisualBox({ await mutate((g) => packetsApi.revApprove(p.id, { reason, ...g }), { sign: SIGN_REV_APPROVE, ok: `Rev ${staged.rev} approved and attached.` }); }; - const keepCalled = async () => { - if (!called) return; - const status = (called.controlled || "not Effective").toLowerCase(); - const reason = await askReason({ - title: `Use rev ${called.rev} for ${p.drawing ?? "this drawing"}`, - sub, - note: `VISUAL calls rev ${called.rev}, which is ${status} in M-Files${staged ? `. The Effective rev is ${staged.rev}` : ""}. Rev ${called.rev}${called.name ? ` (${called.name})` : ""} is attached to this packet and the shop builds to it. It is recorded with your name, signature and the reason. If M-Files releases a newer rev or VISUAL's rev changes, the packet is flagged again.`, - placeholder: `e.g. Engineering says build this lot to rev ${called.rev}`, - min: 3, - max: 400, - submitLabel: `Use rev ${called.rev}`, - }); - if (reason === null) return; - await mutate((g) => packetsApi.revApprove(p.id, { reason, use: "called", ...g }), { - sign: SIGN_REV_APPROVE, - ok: `Rev ${called.rev} attached as VISUAL calls it.`, - }); - }; - const upload = async (kind: "drawing" | "bom", file: File, reason: string) => { setHand(null); if (kind === "bom") { @@ -259,6 +242,15 @@ export function VisualBox({ ) : null} + {retiredAttached && vault ? ( +
+ + Rev {String(vault.rev)} attached as VISUAL calls it + {" "} + - {(vault.controlled || "").toLowerCase()} in M-Files{appr?.rev !== undefined && appr.rev !== null ? `, which has rev ${appr.rev} as Effective` : ""}. +
+ ) : null} + {!hasDwg || drawingFlag ? (
{hasDwg ? "Made from VISUAL - drawing needs a look." : "Made from VISUAL - drawing not attached yet."} {p.drawing || "No drawing number"} rev {p.rev || "-"}. The drawing @@ -280,38 +272,23 @@ export function VisualBox({ .
) : null} - {called ? ( -
- VISUAL calls rev {called.rev}: {called.name ?? `rev ${called.rev}`}, {(called.controlled || "not Effective").toLowerCase()} - {called.state ? ` (${called.state})` : ""} in M-Files{" "} - - View - - . + {called?.missing ? ( +
VISUAL calls rev {called.rev}, which is not in M-Files.
+ ) : called?.refused ? ( +
+ VISUAL calls rev {called.rev}, which is {(called.controlled || "").toLowerCase()} in M-Files. Only an Effective or Retired drawing is + attached.
- ) : v.called?.missing ? ( -
VISUAL calls rev {v.called.rev}, which is not in M-Files.
) : null} - {staged || called ? ( + {staged ? ( canApproveRev ? (
- {staged ? ( - - ) : null} - {called ? ( - - ) : null} +
) : draft ? ( - - {called && staged - ? `A Clerical supervisor can choose rev ${staged.rev} or rev ${called.rev} for this packet.` - : `A Clerical supervisor can approve rev ${String((staged ?? called)?.rev)} for this packet.`} - + A Clerical supervisor can approve rev {String(staged.rev)} for this packet. ) : null ) : ( The M-Files drawing is being copied. Check back after the next drawing run. diff --git a/server/packetstation/api/packet_visual.py b/server/packetstation/api/packet_visual.py index 8be1245..84e56d1 100644 --- a/server/packetstation/api/packet_visual.py +++ b/server/packetstation/api/packet_visual.py @@ -12,7 +12,7 @@ import hashlib import uuid from collections.abc import Mapping from datetime import UTC, datetime -from typing import Any, Literal +from typing import Any from fastapi import APIRouter, Depends, Request from psycopg.types.json import Jsonb @@ -49,9 +49,6 @@ MSG_DRAFT_ONLY = "Only a draft takes a hand upload. Once it leaves Clerical, cha class RevApproveBody(BaseModel): reason: str | None = Field(default=None, max_length=2000) - # "latest": M-Files' Effective rev in place of the rev VISUAL calls. "called": the rev VISUAL calls, which M-Files - # holds at another status (Retired, say). - use: Literal["latest", "called"] = "latest" sign: SignBody | None = None it_reason: str | None = Field(default=None, max_length=1000) @@ -189,22 +186,17 @@ def rev_approve( p: Principal = Depends(current_principal), # noqa: B008 d: Db = Depends(get_db), # noqa: B008 ) -> dict[str, Any]: - """A Clerical Supervisor settles a rev mismatch on a draft: M-Files' Effective rev in place of the rev VISUAL calls - (`use = latest`), or the rev VISUAL calls although M-Files has retired it (`use = called`). For this draft only.""" + """A Clerical Supervisor approves the rev M-Files has in place of the rev VISUAL calls, for this draft only.""" conn = d.conn reason = reason_ok(body.reason) pp = _pp(d, p) row = packets.load_row(conn, packet_id) - if body.use == "called": - staged = packets.called_lookup(conn, row["drawing"], row["rev"]) if row else None - else: - staged = packets.staged_lookup(conn, row["drawing"]) if row else None + staged = packets.staged_lookup(conn, row["drawing"]) if row else None content = { "action": str(A.REV_APPROVE), "packet_id": packet_id, "drawing": row["drawing"] if row else None, "visual_rev": row["rev"] if row else None, - "use": body.use, "rev": staged["rev"] if staged else None, "sha256": bytes(staged["sha256"]).hex() if staged and staged["sha256"] else None, "reason": reason, @@ -223,17 +215,7 @@ def rev_approve( ) as w: if w.packet["source_kind"] != "visual": raise ApiError(409, "not_visual", MSG_NOT_VISUAL) - latest = packets.staged_lookup(conn, w.packet["drawing"]) - if body.use == "called": - lk = packets.called_lookup(conn, w.packet["drawing"], w.packet["rev"]) - if lk is None: - raise ApiError( - 409, - "nothing_staged", - "M-Files has no copy of the rev VISUAL calls ready. Try after the next run.", - ) - else: - lk = latest + lk = packets.staged_lookup(conn, w.packet["drawing"]) if lk is None: raise ApiError( 409, "nothing_staged", "No M-Files copy of this drawing is ready yet. Try after the next run." @@ -247,21 +229,13 @@ def rev_approve( vis_rev = w.packet["rev"] or "" replace_drawing_pages(conn, packet_id, asset, dwg=w.packet["drawing"], rev=rev) visual = dict(w.packet["visual"] or {}) - ok = {"rev": rev, "vis_rev": vis_rev, **who_of(w), "at": now_iso(), "reason": reason} - if body.use == "called": - # The decision holds while M-Files' Effective rev stays the one Clerical chose against. - ok |= { - "use": "called", - "latest": None if latest is None else str(latest["rev"]), - "controlled": lk["controlled"], - } - visual["rev_ok"] = ok + visual["rev_ok"] = {"rev": rev, "vis_rev": vis_rev, **who_of(w), "at": now_iso(), "reason": reason} visual["rev_confirmed"] = True visual["vault"] = { "asset_id": str(asset["id"]), "name": lk["name"], "rev": rev, - "note": f"{lk['controlled']} in M-Files" if body.use == "called" else lk["note"], + "note": lk["note"], "at": now_iso(), } visual.pop("manual_dwg", None) @@ -271,12 +245,8 @@ def rev_approve( (flags_without(w.packet["flags"], (*packets.DRAWING_FLAGS, "hand_drawing")), packet_id), ) w.changed("pages", "drawing_state", "flags", "visual") - w.detail.update(asset_id=str(asset["id"]), rev=rev, vis_rev=vis_rev, use=body.use) - if body.use == "called": - effective = f"M-Files has rev {ok['latest']}" if ok["latest"] else "no Effective rev in M-Files" - w.log("Rev VISUAL calls kept by hand", f"rev {rev} ({lk['controlled']} in M-Files), {effective} · {reason}") - else: - w.log("Different rev approved by hand", f"rev {rev} in place of VISUAL rev {vis_rev or '-'} · {reason}") + w.detail.update(asset_id=str(asset["id"]), rev=rev, vis_rev=vis_rev) + w.log("Different rev approved by hand", f"rev {rev} in place of VISUAL rev {vis_rev or '-'} · {reason}") return _read(d, pp, packet_id) diff --git a/server/packetstation/domain/packets.py b/server/packetstation/domain/packets.py index 1b16425..1b8c584 100644 --- a/server/packetstation/domain/packets.py +++ b/server/packetstation/domain/packets.py @@ -646,19 +646,6 @@ def staged_rev(conn: Any, drawing: Any) -> dict[str, Any] | None: return {"rev": lk["rev"], "name": lk["name"], "asset_id": str(lk["asset_id"])} -def called_lookup(conn: Any, drawing: Any, rev: Any) -> dict[str, Any] | None: - """The drawing_revisions row for the rev VISUAL calls, when the drawing job copied it (asset set).""" - dwg = str(drawing or "").strip() - r = str(rev or "").strip() - if not dwg or not r.isdigit(): - return None - return one( - conn, - "SELECT * FROM drawing_revisions WHERE dwg = %s AND rev = %s AND asset_id IS NOT NULL", - (dwg, int(r)), - ) - - def staged_lookup(conn: Any, drawing: Any) -> dict[str, Any] | None: """The drawing_lookups row for a drawing number when it holds a fetched copy (asset and rev set).""" dwg = str(drawing or "").strip() diff --git a/server/packetstation/drawings/decide.py b/server/packetstation/drawings/decide.py index ebe9a73..552b9e7 100644 --- a/server/packetstation/drawings/decide.py +++ b/server/packetstation/drawings/decide.py @@ -1,6 +1,9 @@ """The drawing decision table. Pure: no database, no M-Files. -`decide(target, found, same_file=...)` says what the drawing job does to one packet for one lookup. Note texts are +`decide(target, found, same_file=..., called=...)` says what the drawing job does to one packet for one lookup. +VISUAL is the source of truth for the rev: when M-Files' Effective rev differs from the rev VISUAL calls but M-Files +still holds that rev as Effective or Retired, the rev VISUAL calls is used, with no flag and no Clerical approval. +The job passes `called` only for those two statuses; no drawing at any other status is ever attached. Note texts are the app's own (from the old revMismatch and draftLine in visual.js, "-" in place of the em-dash), never M-Files' note passed through, except inside the lookup_failed text. @@ -9,14 +12,15 @@ Rows, in the order they are tried: 2. past draft, a pending feed change order waits for this drawing at the released rev -> stage_for_co. Past draft, the pending change order already stages this drawing at the released rev -> unchanged, or stage_for_co again when M-Files has a different file for that rev. + 2a. M-Files' Effective rev is not the rev VISUAL calls (or there is none), no approved rev equals it, and M-Files + holds the rev VISUAL calls as Effective or Retired (`called`) -> on a draft, attach_called (unchanged when that very + file is attached, or a hand drawing is); past draft, unchanged. No flag. Note "Rev V is Retired in M-Files; + VISUAL calls it (M-Files has rev N)". 3. no released rev, a draft rev or a drafting assignment -> flag not_released with the drafting status. 4. no released rev otherwise -> flag not_found. 5. VISUAL rev not a number (and no approved rev equal to M-Files') -> flag bad_rev, not attached. - 6. revs differ (and no approved rev equal to M-Files') -> flag rev_mismatch. The job stages the M-Files copy - for Clerical Supervisor approval on drafts only, and on a draft also copies the rev VISUAL calls when M-Files - still holds it (Retired, say), so Clerical can choose either. - Revs differ, but Clerical chose to keep the rev VISUAL calls while M-Files' Effective rev was this one -> - unchanged (no flag, no change order). A newer Effective rev flags the packet again. + 6. revs differ (and no approved rev equal to M-Files', and M-Files has no document at the rev VISUAL calls) -> + flag rev_mismatch. The job stages the M-Files copy for Clerical Supervisor approval on drafts only. 7. revs match, draft with a hand drawing -> unchanged (the feed already took the hand drawing off on a rev move). 8. revs match, draft with this very file attached at this rev -> unchanged. 9. revs match, draft -> attach ("sheet of drawing " for an H sheet). @@ -41,11 +45,23 @@ NOT_FOUND = "not_found" BAD_REV = "bad_rev" REV_MISMATCH = "rev_mismatch" ATTACH = "attach" +ATTACH_CALLED = "attach_called" UNCHANGED = "unchanged" OPEN_CO = "open_co" STAGE_FOR_CO = "stage_for_co" -KINDS = (ATTACH, UNCHANGED, REV_MISMATCH, NOT_RELEASED, NOT_FOUND, LOOKUP_FAILED, BAD_REV, OPEN_CO, STAGE_FOR_CO) +KINDS = ( + ATTACH, + ATTACH_CALLED, + UNCHANGED, + REV_MISMATCH, + NOT_RELEASED, + NOT_FOUND, + LOOKUP_FAILED, + BAD_REV, + OPEN_CO, + STAGE_FOR_CO, +) FLAG_KINDS = (LOOKUP_FAILED, NOT_RELEASED, NOT_FOUND, BAD_REV, REV_MISMATCH) # drawing_state enum values (0003). bad_rev has no state of its own: it is a rev mismatch Clerical can approve. @@ -72,8 +88,15 @@ class Target: co_pending: bool = False co_staged_sha: bytes | None = None # sha256 of the file the pending CO already stages rejected_sha: bytes | None = None # sha256 staged on the last drawing-origin CO Clerical rejected - # Clerical chose the rev VISUAL calls (a Retired rev) over M-Files' Effective rev; this is that Effective rev. - kept_latest: str | None = None + + +@dataclass(frozen=True) +class Called: + """The rev VISUAL calls, as M-Files holds it (Effective or Retired), with Packet Station's copy.""" + + rev: int + controlled: str # M-Files' Controlled Status, e.g. "Retired" + same_file: bool # this copy is the drawing attached to the packet @dataclass(frozen=True) @@ -111,25 +134,33 @@ def failed_note(f: Found) -> str: return f"{_FAILED_PREFIX}{detail or 'no detail'}" -def kept_called(t: Target, f: Found) -> bool: - """Clerical kept the rev VISUAL calls, and M-Files' Effective rev is still the one they chose against.""" - return f.ok and f.rev is not None and t.kept_latest is not None and str(t.kept_latest).strip() == str(f.rev) +def uses_called(t: Target, f: Found, called: Called | None) -> bool: + """The rev VISUAL calls is used from M-Files at another status: it differs from M-Files' Effective rev (or there + is none), Clerical approved no other rev, and M-Files holds it.""" + if not f.ok or called is None: + return False + v = _int(t.visual_rev) + if v is None or v != called.rev or v == f.rev: + return False + return not (f.rev is not None and t.rev_ok is not None and str(t.rev_ok).strip() == str(f.rev)) -def rev_confirmed(t: Target, f: Found) -> bool: - """The old revConfirmed: M-Files' released rev is the rev VISUAL calls, or Clerical approved that rev, or Clerical - kept the rev VISUAL calls over that rev.""" +def rev_confirmed(t: Target, f: Found, called: Called | None = None) -> bool: + """The old revConfirmed: M-Files' released rev is the rev VISUAL calls, or Clerical approved that rev, or M-Files + holds the rev VISUAL calls at another status.""" + if uses_called(t, f, called): + return True if not f.ok or f.rev is None: return False v = _int(t.visual_rev) if v is not None and v == f.rev: return True - if kept_called(t, f): - return True return t.rev_ok is not None and str(t.rev_ok).strip() == str(f.rev) -def decide(t: Target, f: Found, *, same_file: bool, fetched_sha: bytes | None = None) -> Decision: +def decide( + t: Target, f: Found, *, same_file: bool, fetched_sha: bytes | None = None, called: Called | None = None +) -> Decision: """What to do to one packet. `same_file`: the M-Files file has the sha256 of the attached drawing asset. `fetched_sha`: the sha256 of the @@ -163,6 +194,15 @@ def decide(t: Target, f: Found, *, same_file: bool, fetched_sha: bytes | None = if fetched_sha is not None and t.co_staged_sha is not None and fetched_sha != t.co_staged_sha: return keep(STAGE_FOR_CO, f"New file for rev {f.rev} in M-Files; staged again for the change order") return keep(UNCHANGED, f"Rev {f.rev} is staged on the pending change order") + # 2a + if called is not None and uses_called(t, f, called): + has = f"M-Files has rev {f.rev}" if f.rev is not None else "no Effective rev in M-Files" + note = f"Rev {called.rev} is {called.controlled} in M-Files; VISUAL calls it ({has})" + if not draft or t.has_hand_drawing: + return keep(UNCHANGED, note) + if t.drawing_state == "attached" and called.same_file and t.attached_rev == str(called.rev): + return keep(UNCHANGED, note) + return Decision(ATTACH_CALLED, None, "attached", note) # 3, 4 if f.rev is None: d = draft_line(f) @@ -173,8 +213,6 @@ def decide(t: Target, f: Found, *, same_file: bool, fetched_sha: bytes | None = approved = t.rev_ok is not None and str(t.rev_ok).strip() == str(f.rev) vrev = (t.visual_rev or "").strip() v = _int(vrev) - if not approved and kept_called(t, f): - return keep(UNCHANGED, f"Rev {vrev} kept by Clerical; M-Files has rev {f.rev}") if not approved: # 5 if v is None: diff --git a/server/packetstation/worker/jobs/drawing_lookup.py b/server/packetstation/worker/jobs/drawing_lookup.py index e4362be..731e1fa 100644 --- a/server/packetstation/worker/jobs/drawing_lookup.py +++ b/server/packetstation/worker/jobs/drawing_lookup.py @@ -4,9 +4,11 @@ flag, stage or open a change order on the packets that use them. Per run: the queue (drawings/queue.py) cut to `drawing_budget`; per drawing one `Lookup.status`, a fetch when a released file exists (drawings/fetch.py downloads only when its M-Files identity changed), the `drawing_lookups` row, then `decide` for every live packet that uses the drawing (its own drawing, or the target of its pending feed -change order), applied through `feed.writes.system_write`. A busy packet is left for the next run. On a draft -flagged rev_mismatch, the rev VISUAL calls is looked up too (`Lookup.revision`) and copied into assets when M-Files -still holds it (Retired, say), so Clerical can choose it or the Effective rev. +change order), applied through `feed.writes.system_write`. A busy packet is left for the next run. + +VISUAL is the source of truth for the rev. When M-Files' Effective rev is not the rev VISUAL calls, that rev is +looked up too (`Lookup.revision`). If M-Files has it as Effective or Retired, it is copied into assets and attached +to the draft with no flag. Any other status (or no document at that rev) leaves the packet flagged rev_mismatch. Never writes to M-Files. """ @@ -127,7 +129,7 @@ def _drawing(ctx: JobContext, lk: Lookup, item: queue.QueueItem, counts: Counts, vrev_seen = item.visual_rev if item.co_id is None else own_rev(conn, item.dwg) save_lookup(conn, item, found, fetched, now, fetch_failed=fetch_failed, visual_rev_seen=vrev_seen) scope = [str(d) for d in (getattr(ctx.feed_settings, "departments", None) or [])] - called: dict[int, dict[str, Any] | None] = {} # VISUAL rev -> what M-Files holds at that rev, this drawing + called: dict[int, CalledCopy | None] = {} # VISUAL rev -> what M-Files holds at that rev, this drawing for pid, relations in sharing(conn, item.dwg, scope): _packet(ctx, lk, item.dwg, found, fetched, pid, relations, counts, called) @@ -239,12 +241,27 @@ def save_revision(conn: Any, dwg: str, rf: Found, fetched: FetchResult, now: dat ) +# The M-Files statuses a rev VISUAL calls may be attached at. Anything else is never attached. +ATTACHABLE = frozenset({"effective", "retired"}) + + +@dataclass(frozen=True) +class CalledCopy: + """The rev VISUAL calls as M-Files holds it. `record` goes on the packet as `visual.called`: {rev, controlled, + state, name, asset_id, sha256, pages} for a copy, {rev, missing: true} when M-Files has no document at that rev, + {rev, controlled, state, refused: true} when its status may not be attached. `found` and `fetched` are set only + for a copy.""" + + record: dict[str, Any] + found: Found | None = None + fetched: FetchResult | None = None + + def called_rev( - ctx: JobContext, lk: Lookup, dwg: str, visual_rev: str, memo: dict[int, dict[str, Any] | None], counts: Counts -) -> dict[str, Any] | None: - """The rev VISUAL calls, as M-Files holds it: {rev, controlled, state, name, asset_id, sha256, pages}, or - {rev, missing: true} when M-Files has no document at that rev. None when it could not be read this run (the - packet keeps what it had) or the VISUAL rev is not a number.""" + ctx: JobContext, lk: Lookup, dwg: str, visual_rev: str, memo: dict[int, CalledCopy | None], counts: Counts +) -> CalledCopy | None: + """The rev VISUAL calls, as M-Files holds it, copied into assets when its status is Effective or Retired. None when + it could not be read this run (the packet keeps what it had) or the VISUAL rev is not a number.""" v = D.as_int(visual_rev) if v is None: return None @@ -258,7 +275,10 @@ def called_rev( ctx.error(f"drawing {dwg}: rev {v}: {e}") return None if rf is None: - memo[v] = {"rev": str(v), "missing": True} + memo[v] = CalledCopy({"rev": str(v), "missing": True}) + return memo[v] + if rf.controlled.casefold() not in ATTACHABLE: + memo[v] = CalledCopy({"rev": str(v), "controlled": rf.controlled, "state": rf.state or None, "refused": True}) return memo[v] try: fetched = fetch(conn, ctx.settings, lk, rf, dwg=dwg) @@ -284,7 +304,7 @@ def called_rev( "reused": fetched.reused, }, ) - memo[v] = { + record = { "rev": str(v), "controlled": rf.controlled, "state": rf.state or None, @@ -293,6 +313,7 @@ def called_rev( "sha256": fetched.sha256.hex(), "pages": fetched.pages, } + memo[v] = CalledCopy(record, rf, fetched) return memo[v] @@ -397,13 +418,17 @@ def target_for(dwg: str, pf: PacketFacts, *, own: bool) -> D.Target: co_waiting = _dwg_rev(waiting) co_staged = _dwg_rev(staged) packet_rev = str(row["rev"] or "").strip() - rev_ok = kept_latest = None + rev_ok = None ro = visual.get("rev_ok") - if isinstance(ro, dict) and ro.get("rev") is not None and str(ro.get("vis_rev") or "").strip() == packet_rev: - if ro.get("use") == "called": - kept_latest = None if ro.get("latest") is None else str(ro["latest"]).strip() - else: - rev_ok = str(ro["rev"]).strip() + # A rev_ok with use = "called" (Clerical kept the rev VISUAL calls, before that became automatic) approves no other + # rev: the rev VISUAL calls is used anyway. + if ( + isinstance(ro, dict) + and ro.get("rev") is not None + and ro.get("use") != "called" + and str(ro.get("vis_rev") or "").strip() == packet_rev + ): + rev_ok = str(ro["rev"]).strip() visual_rev = packet_rev if own or co_waiting is None else co_waiting[1] return D.Target( dwg=dwg, @@ -419,7 +444,6 @@ def target_for(dwg: str, pf: PacketFacts, *, own: bool) -> D.Target: co_pending=co is not None, co_staged_sha=_sha(staged.get("sha256")) if isinstance(staged, dict) else None, rejected_sha=pf.rejected_sha, - kept_latest=kept_latest, ) @@ -461,7 +485,7 @@ def _packet( pid: int, relations: set[str], counts: Counts, - called_memo: dict[int, dict[str, Any] | None], + called_memo: dict[int, CalledCopy | None], ) -> None: conn = ctx.conn pf = facts(conn, pid) @@ -470,7 +494,14 @@ def _packet( own = "own" in relations t = target_for(dwg, pf, own=own) same_file = fetched is not None and fetched.sha256 in pf.attached_shas - d = D.decide(t, found, same_file=same_file, fetched_sha=fetched.sha256 if fetched else None) + cc: CalledCopy | None = None + dc: D.Called | None = None + v = D.as_int(t.visual_rev) + if own and found.ok and v is not None and v != found.rev: + cc = called_rev(ctx, lk, dwg, t.visual_rev, called_memo, counts) + if cc is not None and cc.found is not None and cc.fetched is not None: + dc = D.Called(v, cc.found.controlled, cc.fetched.sha256 in pf.attached_shas) + d = D.decide(t, found, same_file=same_file, fetched_sha=fetched.sha256 if fetched else None, called=dc) if not own and d.kind != D.STAGE_FOR_CO: return # only the CO's target drawing: nothing to do until it can be staged if d.kind in (D.ATTACH, D.OPEN_CO, D.STAGE_FOR_CO) and fetched is None: @@ -486,18 +517,17 @@ def _packet( if not _same_appr(visual.get("appr"), appr): patch["appr"] = appr if found.ok: - confirmed = D.rev_confirmed(t, found) + confirmed = D.rev_confirmed(t, found, dc) if bool(visual.get("rev_confirmed")) != confirmed: patch["rev_confirmed"] = confirmed - if d.kind == D.REV_MISMATCH and row["status"] == "draft": - called = called_rev(ctx, lk, dwg, t.visual_rev, called_memo, counts) - if called is not None and visual.get("called") != called: - patch["called"] = called - elif visual.get("called") is not None: + if cc is not None: + if visual.get("called") != cc.record: + patch["called"] = cc.record + elif visual.get("called") is not None and (v is None or v == found.rev): patch["called"] = None state_after = d.drawing_state if own else str(row["drawing_state"]) material = ( - d.kind in (D.ATTACH, D.OPEN_CO, D.STAGE_FOR_CO) + d.kind in (D.ATTACH, D.ATTACH_CALLED, D.OPEN_CO, D.STAGE_FOR_CO) or flags_after != flags or state_after != row["drawing_state"] or bool(patch) @@ -508,20 +538,28 @@ def _packet( action = A.VIS_UPDATE_DRAFT if row["status"] == "draft" else A.VIS_OPEN_CO label, note = "Drawing status from M-Files", d.note or None called_now = patch.get("called") or {} - if note and called_now.get("controlled"): - note = f"{note}; rev {called_now['rev']} is {called_now['controlled']} in M-Files" - elif note and called_now.get("missing"): + if d.kind == D.REV_MISMATCH and note and called_now.get("refused"): + note = f"{note}; rev {called_now['rev']} is {called_now['controlled']} in M-Files - not attached" + elif d.kind == D.REV_MISMATCH and note and called_now.get("missing"): note = f"{note}; rev {called_now['rev']} is not in M-Files" - if d.kind == D.ATTACH and fetched is not None: + use_found, use_fetched = found, fetched + if d.kind == D.ATTACH_CALLED and cc is not None and cc.found is not None and cc.fetched is not None: + use_found, use_fetched = cc.found, cc.fetched + if d.kind in (D.ATTACH, D.ATTACH_CALLED) and use_fetched is not None: label = "Drawing attached from M-Files" - note = "; ".join(x for x in (fetched.name or found.name or "", d.note) if x) or None - detail = {"dwg": dwg, "rev": found.rev, "kind": d.kind, "asset_id": str(fetched.asset_id) if fetched else None} + note = "; ".join(x for x in (use_fetched.name or use_found.name or "", d.note) if x) or None + detail = { + "dwg": dwg, + "rev": use_found.rev, + "kind": d.kind, + "asset_id": str(use_fetched.asset_id) if use_fetched else None, + } try: with writes.system_write(conn, pid, action, label=label, note=note, detail=detail) as sw: if int(sw.packet["version"]) != int(row["version"]): raise _Stale - if d.kind == D.ATTACH and fetched is not None: - _attach(sw, pid, dwg, found, fetched, d, flags_after, patch, now) + if d.kind in (D.ATTACH, D.ATTACH_CALLED) and use_fetched is not None: + _attach(sw, pid, dwg, use_found, use_fetched, d, flags_after, patch, now) counts["attached"] += 1 else: _status(sw, pid, state_after, flags_after, patch) @@ -581,6 +619,7 @@ def _attach( "rev": found.rev, "note": d.note, "at": now.isoformat(), + **({"controlled": found.controlled} if found.controlled else {}), } merged = {**patch, "rev_confirmed": True, "vault": vault} conn.execute( diff --git a/tests/server/feedhelpers.py b/tests/server/feedhelpers.py index e85863e..54b8804 100644 --- a/tests/server/feedhelpers.py +++ b/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..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") diff --git a/tests/server/test_drawing_decide.py b/tests/server/test_drawing_decide.py index 29c9abc..d2913f9 100644 --- a/tests/server/test_drawing_decide.py +++ b/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) diff --git a/tests/server/test_feed_api.py b/tests/server/test_feed_api.py index 36bdbe0..2f03cfd 100644 --- a/tests/server/test_feed_api.py +++ b/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: diff --git a/tests/server/test_feed_apply.py b/tests/server/test_feed_apply.py index d81af31..8b7081f 100644 --- a/tests/server/test_feed_apply.py +++ b/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"))