n.siegfried/Project-SDE-WP-Suite
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

T7.6 - CR-014/D2/D9/D10: the Ready for QA gate, notification only, shipped off

Browse Source 
Marlena's ask: QA sees inbound work ahead of time, not after the fact.

The rung: 'Ready for QA' sits between In Progress and QC in BOTH ladders
(wp-creation-app.js STATUS_ORDER, server/app.py STATUS_ORDER) and in Field
View's list - inside the T7.3 transition model, not beside it: entering it
from an unreleased state crosses the release gates, and 'Issue' (hold) stays
a branch. The dashboard filter and the navigator grouping learned the state
from the ladder without their own edits.

Who hears (D2): the QA GROUP, a multi-pick of project members on the SOP
wizard's team step, stored as account ids at data.sop.project.qaGroupIds.
Entering Ready for QA emails that list and nobody else. A rejection emails
the owner AND the same list (amended answer), returns the package to In
Progress, and REQUIRES a fresh comment - server-enforced on both write paths
(the first version accepted any old comment already on the record, which made
every rejection after the first one free; the gate now demands a new entry).
Accept and reject are real buttons on the release banner; the comment modal
enforces its field; qa_ready / qa_rejected / status_changed all land in the
audit history.

The link (X1): wp_link() now opens THE package - wp-creation-index.html
?project&wp=<id>, which the creator boots directly and login.html?next=
round-trips for a signed-out recipient. It previously pointed at the suite
root, which is exactly the failure X1 names; assignment mail inherits the fix.

Email discipline (D10 + standing rules): ships OFF (the stored setting the
admin console already owns; PUT /api/settings is admin-only, 403 for anyone
else, and audited). With it off, transitions write outbox rows marked
'skipped' and the sink receives nothing. With it on, the probe runs a REAL
SMTP conversation against an in-process capture sink and asserts the count
and the exact recipient set. A dead SMTP host leaves a 'failed' outbox row
with the error recorded. The SMTP password exists only in the environment.

DEVIATION, stated: the task's Do-paragraph asks the email to include location
and a scope summary; the done-when list (and CLAUDE.md) says no customer IP in
a message body. The done-when wins: bodies carry the WP number, who moved it,
and the deep link. A location canary planted on the package is asserted absent
from every captured message. If the fuller body is wanted, that is a product
call - needs Nick.

Found while building, logged not fixed (BL-021): project_sop_team() reads
sop.data['project'], a path pushSOP never writes - the critical-reopen email
has never actually reached the PM/CM. One-line fix, owned by T9.9.

Field View (D9): 'Ready for QA' is carried by TEXT on the card at 390px.

Verification (each probe run alone): NEW tests/qa_gate_check.py 40/40.
Regressions: hold_check 50/50, pipeline_check 44/44, aggregates_check 16/16,
frame_check 39/39, validation_check 83/83.

Items: CR-014, D2, D9, D10 (X1, X3 honored)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Nick Siegfried
2026-08-19 10:54:07 -07:00
parent 5d27a1e086
commit 2486f87010
10 changed files with 700 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

155
server/app.py
View File

@@ -448,8 +448,12 @@ def require_assignable(db: Session, user_id: str, project_id: Optional[str]) ->
def wp_link(db: Session, wp: "models.WorkPackage") -> str:
"""A link that opens THIS work package. X1: never the app root - a recipient
who has to hunt for the package after signing in stops opening the emails.
The creator has been its own document since T7.1 and boots ?wp= deep links;
a signed-out recipient rides login.html?next= straight back to it."""
base = (notify.get_settings(db).get("app_base_url") or "").rstrip("/")
path = f"/work-package-suite.html?tab=wp&project={wp.project_id or ''}"
path = f"/wp-creation-index.html?project={wp.project_id or ''}&wp={wp.id}"
return (base + path) if base else path
@@ -524,6 +528,10 @@ class TestEmailIn(BaseModel):
class StatusIn(BaseModel):
status: str
# CR-014: a rejection (Ready for QA -> In Progress) must say why. The upsert
# route carries the comment inside data.qaRejections; this route has no data,
# so it carries the comment here and the server appends the record itself.
comment: Optional[str] = None
class ArchiveIn(BaseModel):
@@ -1411,7 +1419,8 @@ def delete_sop(sop_id: str, user: models.User = Depends(auth.get_current_user),
# _released() must never count it: counting it is what let every transition out of
# hold skip the release gates as "already released", which made /status a side
# door past an open constraint (CR-015 / T7.3).
STATUS_ORDER = ["Draft", "Scheduled", "Issued", "In Progress", "Issue", "QC", "Closed"]
STATUS_ORDER = ["Draft", "Scheduled", "Issued", "In Progress", "Ready for QA", "Issue", "QC", "Closed"]
QA_READY_STATUS = "Ready for QA"
ISSUED_IDX = STATUS_ORDER.index("Issued")
DONE_STATUS = "Closed"
HOLD_STATUS = "Issue"
@@ -1563,6 +1572,108 @@ def project_sop_team(db: Session, project_id: Optional[str]) -> list[str]:
return [i for i in (proj.get("pmId"), proj.get("cmId")) if i]
def project_qa_group(db: Session, project_id: Optional[str]) -> list["models.User"]:
"""D2: the QA group named on the project's latest complete SOP. pushSOP writes
the row as data={sop, state}, so the project block is data['sop']['project'] -
note that project_sop_team above reads data['project'], which that shape never
has (BL-021, logged, not fixed here)."""
if not project_id:
return []
sop = db.scalars(
select(models.Sop)
.where((models.Sop.project_id == project_id) & (models.Sop.complete.is_(True)))
.order_by(models.Sop.updated_at.desc())
.limit(1)
).first()
if not sop:
return []
data = sop.data or {}
proj = ((data.get("sop") or {}).get("project")
or data.get("project") or {})
ids = [i for i in (proj.get("qaGroupIds") or []) if isinstance(i, str) and i]
if not ids:
return []
return list(db.scalars(select(models.User).where(models.User.id.in_(ids))))
def qa_rejection_comment(data: Optional[dict]) -> str:
rejs = [r for r in ((data or {}).get("qaRejections") or []) if isinstance(r, dict)]
if not rejs:
return ""
return str(rejs[-1].get("comment") or "").strip()
def enforce_qa_rejection_comment(data: Optional[dict], new_status: str,
old_status: Optional[str],
old_data: Optional[dict] = None) -> None:
"""CR-014: a rejection with no reason is the after-the-fact surprise this gate
exists to end. Runs before anything is written, like the release gates.
The comment must be FRESH: a rejection entry already on the record satisfied
the first version of this check, which made every rejection after the first
one free. New entry, non-empty comment, or the transition is refused."""
if old_status == QA_READY_STATUS and new_status == "In Progress":
new_rejs = [r for r in ((data or {}).get("qaRejections") or []) if isinstance(r, dict)]
old_rejs = [r for r in ((old_data or {}).get("qaRejections") or []) if isinstance(r, dict)]
fresh = len(new_rejs) > len(old_rejs) and str(new_rejs[-1].get("comment") or "").strip()
if not fresh:
raise HTTPException(status_code=409, detail={
"message": "Returning a package from Ready for QA requires a comment",
})
def qa_ready_body(user: "models.User", wp: "models.WorkPackage",
actor: "models.User", link: str) -> str:
# A WP number and a deep link - NOT the package contents. The task text asked
# for location and a scope summary, but the done-when list (and the standing
# rule) says no customer IP in a message body; the link is the summary.
who = actor.full_name or actor.username
name = user.full_name or user.username
return (
f"Hi {name},\n\n"
f"{who} moved {wp.number or 'a work package'} to Ready for QA.\n"
f"It is in the QA queue waiting to be accepted or returned.\n\n"
f"Open it here:\n{link}\n\n"
f"— This is an automated message from the Work Package Suite."
)
def qa_reject_body(user: "models.User", wp: "models.WorkPackage",
actor: "models.User", link: str) -> str:
who = actor.full_name or actor.username
name = user.full_name or user.username
return (
f"Hi {name},\n\n"
f"{who} returned {wp.number or 'a work package'} from Ready for QA to In Progress.\n"
f"The reason is recorded on the package.\n\n"
f"Open it here:\n{link}\n\n"
f"— This is an automated message from the Work Package Suite."
)
def notify_qa_transition(db: Session, wp: "models.WorkPackage",
actor: "models.User", rejected: bool) -> list:
"""Entering Ready for QA emails the QA group and nobody else (D2). A rejection
emails the owner AND the same group (D9's sibling decision). Deduplicated;
every send is an outbox row, so a failure is recorded, never silent."""
recipients = {u.id: u for u in project_qa_group(db, wp.project_id)}
if rejected and wp.assignee_id:
owner = db.get(models.User, wp.assignee_id)
if owner:
recipients[owner.id] = owner
out = []
link = wp_link(db, wp)
for u in recipients.values():
out.append(notify.enqueue(
db, user=u, kind="qa_rejected" if rejected else "qa_ready",
subject=(f"Returned from QA: {wp.number or 'work package'}" if rejected
else f"Ready for QA: {wp.number or 'work package'}"),
body=(qa_reject_body(u, wp, actor, link) if rejected
else qa_ready_body(u, wp, actor, link)),
link=link, wp_id=wp.id, project_id=wp.project_id,
))
return out
def hold_body(user: "models.User", wp: "models.WorkPackage", names: list[str],
actor: "models.User", link: str) -> str:
# Constraint names and a WP number only — no package contents, same rule as the
@@ -1632,6 +1743,7 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User =
wp_id_for_checks = body.id or wp.id
check_predecessor_cycle(db, wp_id_for_checks, body.data)
enforce_release_gates(db, wp_id_for_checks, body.data, body.status, old_status)
enforce_qa_rejection_comment(body.data, body.status, old_status, old_data)
wp.project_id = body.project_id
wp.sop_id = body.sop_id
wp.parent_id = body.parent_id
@@ -1694,6 +1806,19 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User =
summary=(wp.number or wp.subject or wp.id),
detail={"critical": reopened, "status": wp.status})
notifs.extend(notify_critical_reopen(db, wp, reopened, user))
# CR-014: the QA gate. Entering Ready for QA tells the QA group their queue
# grew; a rejection tells the owner and the same group. Both write their own
# audit line - status_changed says from/to, these say what it MEANS.
if not is_new and old_status != wp.status:
if wp.status == QA_READY_STATUS:
log_event(db, user, "qa_ready", "wp", wp.id, project_id=wp.project_id,
summary=(wp.number or wp.subject or wp.id), detail={"from": old_status})
notifs.extend(notify_qa_transition(db, wp, user, rejected=False))
elif old_status == QA_READY_STATUS and wp.status == "In Progress":
log_event(db, user, "qa_rejected", "wp", wp.id, project_id=wp.project_id,
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))
# Notify a newly-assigned owner (skip self-assignment).
notif = None
if new_assignee and new_assignee != old_assignee and new_assignee != user.id:
@@ -2419,15 +2544,28 @@ def issue_wp(wp_id: str, user: models.User = Depends(auth.get_current_user), db:
@app.post("/api/wps/{wp_id}/status")
def set_wp_status(wp_id: str, body: StatusIn, user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)):
def set_wp_status(wp_id: str, body: StatusIn, background_tasks: BackgroundTasks, user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)):
wp = db.get(models.WorkPackage, wp_id)
if not wp:
raise HTTPException(status_code=404, detail="Work Package not found")
require_project_access(db, user, wp.project_id)
require_project_writable(db, user, wp.project_id, "Changing a work package's status")
old_status = wp.status
_qa_notifs = []
# Same gates as /issue — this route must not be a way around them.
enforce_release_gates(db, wp.id, wp.data, body.status, old_status)
if old_status == QA_READY_STATUS and body.status == "In Progress":
comment = str(body.comment or "").strip()
if not comment:
raise HTTPException(status_code=409, detail={
"message": "Returning a package from Ready for QA requires a comment",
})
data = dict(wp.data or {})
rejs = [r for r in (data.get("qaRejections") or []) if isinstance(r, dict)]
rejs.append({"ts": models.utcnow().isoformat(), "comment": comment,
"by": user.full_name or user.username, "from": QA_READY_STATUS})
data["qaRejections"] = rejs
wp.data = data
wp.status = body.status
if body.status == "Issued" and wp.issued_at is None:
wp.issued_at = models.utcnow()
@@ -2450,8 +2588,19 @@ def set_wp_status(wp_id: str, body: StatusIn, user: models.User = Depends(auth.g
detail={"to": body.status,
"constraint": str((_h or {}).get("constraint") or "")[:200],
"reason": str((_h or {}).get("details") or "")[:300]})
if body.status == QA_READY_STATUS:
log_event(db, user, "qa_ready", "wp", wp.id, project_id=wp.project_id,
summary=(wp.number or wp.subject or wp.id), detail={"from": old_status})
_qa_notifs = notify_qa_transition(db, wp, user, rejected=False)
elif old_status == QA_READY_STATUS and body.status == "In Progress":
log_event(db, user, "qa_rejected", "wp", wp.id, project_id=wp.project_id,
summary=(wp.number or wp.subject or wp.id),
detail={"comment": qa_rejection_comment(wp.data)[:300]})
_qa_notifs = notify_qa_transition(db, wp, user, rejected=True)
db.commit()
db.refresh(wp)
for n in _qa_notifs:
background_tasks.add_task(notify.deliver, n.id)
return wp.to_dict()