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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
133
tests/kitting_notify_check.py
Normal file
133
tests/kitting_notify_check.py
Normal file
@@ -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())
|
||||
Reference in New Issue
Block a user