From 829fa2223664fad5d3398fd35b79a33570827604 Mon Sep 17 00:00:00 2001 From: "n.siegfried" Date: Wed, 19 Aug 2026 12:05:24 -0700 Subject: [PATCH] T8.3 - CR-011: material moves, the field hears about it once A kitting status change (detected on the upsert, which is how the browser and the offline outbox both save) emails the package's distribution list (distributionIds) plus its warehouse owner (kitOwnerId - CR-010's default recipient), minus the actor, deduplicated. The mail matches the house convention - greeting, one line of what happened, the deep link, the automated-message footer - and says old status, new status, who, and the delivery location (deliveryLoc when CR-012 lands at T8.4; mimoLoc today). The link opens THAT package (X1), same wp_link as every other mail. No burst: an unsent notification for the same package and recipient is REWRITTEN to the newest transition instead of joined by a sibling - three rapid changes leave one row per recipient saying where kitting ended up, while the audit history keeps all three, uncoalesced. Found by the probe and fixed: a row held while email was OFF stayed 'skipped' forever; the change that finds email ON now promotes it to pending and schedules it - otherwise turning the gate on silently orphaned everything coalesced before it. The gate is T7.6's gate, reused - the probe greps that no second email flag exists anywhere. Off by default; admin-only (403 for anyone else); every send terminates at the in-process SMTP sink with count and recipients asserted; no real mail leaves this branch. Send failures ride the shared notify.deliver path whose failure handling qa_gate_check pins. Verification (each probe run alone): NEW tests/kitting_notify_check.py 17/17 (the sink is imported from qa_gate_check - one sink implementation, not two). Regression: qa_gate_check 40/40. Items: CR-011, D10 (X1 honored) Co-Authored-By: Claude Fable 5 --- docs/reference/file-map.md | 1 + server/app.py | 82 +++++++++++++++++++++ tests/kitting_notify_check.py | 133 ++++++++++++++++++++++++++++++++++ 3 files changed, 216 insertions(+) create mode 100644 tests/kitting_notify_check.py diff --git a/docs/reference/file-map.md b/docs/reference/file-map.md index d4d6854..1ec523b 100644 --- a/docs/reference/file-map.md +++ b/docs/reference/file-map.md @@ -291,6 +291,7 @@ Wave 8 adds these: ```bash python tests/kitting_check.py # CR-009/CR-010 - statuses, owner, filter 21 checks +python tests/kitting_notify_check.py # CR-011 - kitting mail, coalesced, gated 17 checks ``` **Three probes were re-pointed at `T7.1`.** `sections_check.py` 5b drove the live diff --git a/server/app.py b/server/app.py index 899a722..916d4b5 100644 --- a/server/app.py +++ b/server/app.py @@ -1711,6 +1711,76 @@ def hold_body(user: "models.User", wp: "models.WorkPackage", names: list[str], ) +def kitting_body(user: "models.User", wp: "models.WorkPackage", actor: "models.User", + old_status: str, new_status: str, link: str) -> str: + # Same convention as the assignment and hold mails: a greeting, one line of + # what happened, the deep link, the footer. The delivery location is MIMO + # logistics (where material stages), not package contents - the wave 8 spec + # names it in the done-when. deliveryLoc arrives with CR-012 (T8.4); + # mimoLoc is what exists today, so both are read. + who = actor.full_name or actor.username + name = user.full_name or user.username + delivery = str((wp.data or {}).get("deliveryLoc") + or (wp.data or {}).get("mimoLoc") or "").strip() or "not set" + return ( + f"Hi {name},\n\n" + f"{who} moved kitting on {wp.number or 'a work package'} from " + f"{old_status or 'Not Started'} to {new_status or 'Not Started'}.\n" + f"Delivery location: {delivery}.\n\n" + f"Open it here:\n{link}\n\n" + f"— This is an automated message from the Work Package Suite." + ) + + +def notify_kitting_change(db: Session, wp: "models.WorkPackage", actor: "models.User", + old_status: str, new_status: str) -> list: + """CR-011: the package's distribution list plus its warehouse owner (CR-010's + default recipient), minus the actor, deduplicated. COALESCED: if an unsent + kitting notification already exists for this package and recipient, it is + rewritten to the newest transition instead of joined by a sibling - rapid + consecutive changes produce one email saying where kitting ended up, not a + burst of near-identical ones.""" + ids = [i for i in ((wp.data or {}).get("distributionIds") or []) if isinstance(i, str)] + ko = (wp.data or {}).get("kitOwnerId") + if isinstance(ko, str) and ko: + ids.append(ko) + link = wp_link(db, wp) + subject = f"Kitting {new_status or 'updated'}: {wp.number or 'work package'}" + existing = {n.user_id: n for n in db.scalars( + select(models.Notification).where( + (models.Notification.wp_id == wp.id) + & (models.Notification.kind == "kitting_status") + & (models.Notification.status.in_(("pending", "skipped"))))).all()} + s_cfg = notify.get_settings(db) + deliverable = bool(s_cfg.get("email_enabled")) and notify.smtp_ready(s_cfg) + seen, out = set(), [] + for uid in ids: + if uid in seen or uid == actor.id: + continue + seen.add(uid) + u = db.get(models.User, uid) + if not u or not u.is_active: + continue + held = existing.get(uid) + if held is not None: + held.subject = subject[:300] + held.body = kitting_body(u, wp, actor, old_status, new_status, link) + held.link = link[:500] + # A row held while email was OFF stays 'skipped' forever unless the + # change that finds email ON promotes it - otherwise turning the + # gate on silently orphans everything coalesced before it. + if deliverable and u.email and held.status == "skipped": + held.status = "pending" + out.append(held) + continue + out.append(notify.enqueue( + db, user=u, kind="kitting_status", subject=subject, + body=kitting_body(u, wp, actor, old_status, new_status, link), + link=link, wp_id=wp.id, project_id=wp.project_id, + )) + return out + + def notify_critical_reopen(db: Session, wp: "models.WorkPackage", names: list[str], actor: "models.User") -> list["models.Notification"]: """Owner + PM + CM + everyone on the package's distribution list, minus whoever @@ -1847,6 +1917,18 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User = summary=(wp.number or wp.subject or wp.id), detail={"comment": qa_rejection_comment(wp.data)[:300]}) notifs.extend(notify_qa_transition(db, wp, user, rejected=True)) + # CR-011: a kitting status change tells the distribution list and the + # warehouse owner where the material stands. Detected here because the + # browser saves kitting through this upsert (the outbox replays it too). + if not is_new: + _kit_old = str((old_data or {}).get("kitStatus") or "") + _kit_new = str((wp.data or {}).get("kitStatus") or "") + if _kit_old != _kit_new: + log_event(db, user, "kitting_status_changed", "wp", wp.id, + project_id=wp.project_id, + summary=(wp.number or wp.subject or wp.id), + detail={"from": _kit_old, "to": _kit_new}) + notifs.extend(notify_kitting_change(db, wp, user, _kit_old, _kit_new)) # Notify a newly-assigned owner (skip self-assignment). notif = None if new_assignee and new_assignee != old_assignee and new_assignee != user.id: diff --git a/tests/kitting_notify_check.py b/tests/kitting_notify_check.py new file mode 100644 index 0000000..7fd33b3 --- /dev/null +++ b/tests/kitting_notify_check.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +"""Does a kitting change tell the right people, once? — CR-011 / D10, T8.3. + +The field learns when material moves without chasing it in Teams. A kitting +status change emails the package's distribution list plus its warehouse owner +(CR-010's default recipient), through the SAME gate T7.6 built - off by +default, admin-only, every send an outbox row, no real mail anywhere. + +Coalescing: rapid consecutive changes rewrite the held (unsent) notification +to the newest transition instead of stacking near-identical siblings. + +Reuses qa_gate_check's SMTP sink - one sink implementation, not two. +Exit 0 all passed, 1 a failure, 2 could not run. +""" +import json +import os +import re +import sys +import tempfile +import time +import urllib.error +import urllib.request + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +import cdp # noqa: E402 +from browser_check import seed, start_server, chk, _PASS, _FAIL # noqa: E402 +from qa_gate_check import SmtpSink, api, wait_for # noqa: E402 + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def ascii_(v, n=300): + return re.sub(r"\s+", " ", str(v)).encode("ascii", "replace").decode()[:n] + + +def save_wp(base, tok, kit_status, extra=None): + data = {"constraints": [{"name": "Materials", "status": "cleared", "comment": ""}], + "kitStatus": kit_status, "kitOwner": "Sue", "kitOwnerId": "user_sue", + "distributionIds": ["user_pat"], "mimoLoc": "Staging 04, dock B"} + data.update(extra or {}) + return api(base, "/api/wps", tok, "POST", { + "id": "wpKN1", "project_id": "projA", "number": "KN-1", + "subject": "kit notify host", "status": "In Progress", "data": data}) + + +def main(): + exe = cdp.find_browser() # not used, but keeps environment parity checks + tmpdir = tempfile.mkdtemp(prefix="wpsuite-kitnotify-") + db_path = os.path.join(tmpdir, "check.db") + server = None + sink = SmtpSink() + sink.start() + try: + tok = seed(db_path) + port = cdp.free_port() + base = "http://127.0.0.1:%d" % port + server = start_server(port, db_path) + root, pat = tok["root"], tok["pat"] + + # ── 1. shipped OFF, and the burst coalesces while held ─────────────── + print("\n1. off by default, and no burst") + save_wp(base, root, "Not Started") + for st in ("Picking", "Staged", "In Transit"): # three rapid changes + code, _ = save_wp(base, root, st) + chk("kitting change to %s saves" % st, code == 200, code) + _, rows = api(base, "/api/notifications?all=true", root) + kit_rows = [r for r in (rows or []) if r.get("kind") == "kitting_status"] + chk("with email OFF the rows are skipped, not silent, and COALESCED - " + "one per recipient, not one per change", + len(kit_rows) == 2 and all(r.get("status") == "skipped" for r in kit_rows), + ascii_([(r.get("email"), r.get("status")) for r in kit_rows])) + chk("...rewritten to the NEWEST transition", + all("In Transit" in (r.get("subject") or "") for r in kit_rows), + ascii_([r.get("subject") for r in kit_rows])) + chk("...and the sink received nothing", len(sink.messages) == 0) + _, ev = api(base, "/api/audit?entity_type=wp&entity_id=wpKN1" + "&action=kitting_status_changed", root) + chk("every change is in the audit history, uncoalesced", + len(ev or []) == 3, len(ev or [])) + + # ── 2. the T7.6 gate, reused ───────────────────────────────────────── + print("\n2. the same gate") + src = open(os.path.join(ROOT, "server", "notify.py"), encoding="utf-8").read() + chk("no second email flag was invented - the T7.6 gate is THE gate", + src.count("email_enabled") >= 1 + and "kitting_email" not in src + and "kitting_email" not in open(os.path.join(ROOT, "server", "app.py"), + encoding="utf-8").read()) + code, _ = api(base, "/api/settings", pat, "PUT", {"email_enabled": True}) + chk("a non-administrator still cannot turn it on (D10)", code == 403, code) + code, _ = api(base, "/api/settings", root, "PUT", { + "email_enabled": True, "smtp_host": "127.0.0.1", "smtp_port": sink.port, + "smtp_use_tls": False, "from_addr": "suite@sink.local", "app_base_url": base}) + chk("an administrator can", code == 200, code) + + # ── 3. the send ────────────────────────────────────────────────────── + print("\n3. the send, against the sink") + code, _ = save_wp(base, root, "Delivered") + chk("the change lands", code == 200, code) + chk("the sink captures the distribution list + the warehouse owner: two", + wait_for(lambda: len(sink.messages) == 2, 12), len(sink.messages)) + rcpts = sorted(m["to"][0] for m in sink.messages) + chk("...exactly them, actor excluded", + rcpts == ["pat@example.test", "sue@example.test"], ascii_(rcpts)) + body = sink.messages[0]["data"] if sink.messages else "" + chk("the mail says old status, new status and who", + "In Transit" in body and "Delivered" in body and "Root" in body, + ascii_(body, 260)) + chk("...and the delivery location", "Staging 04, dock B" in body) + chk("...and a deep link to THAT package, not the app root", + "/wp-creation-index.html?project=projA&wp=wpKN1" in body) + chk("...in the house convention (greeting + automated-message footer)", + body.count("Hi ") >= 1 and "automated message from the Work Package Suite" in body) + + finally: + sink.stop() + if server is not None: + try: + server.terminate() + except Exception: + pass + + print("\n" + "-" * 54) + print("%d/%d checks passed." % (len(_PASS), len(_PASS) + len(_FAIL))) + for f in _FAIL: + print(" - " + f) + return 1 if _FAIL else 0 + + +if __name__ == "__main__": + sys.exit(main())