diff --git a/html/index.html b/html/index.html index 9490c8e..e0d6fe7 100644 --- a/html/index.html +++ b/html/index.html @@ -133,6 +133,9 @@ color: var(--cds-support-success); margin-bottom: 0.5rem; } + /* B4: an unreachable server is a third state, and it has to look like neither + of the other two. Not green, not silence. */ + .card-status.card-status-error { color: var(--wp-status-warning-text); } .card.disabled { opacity: 0.6; pointer-events: none; @@ -567,24 +570,28 @@ if(info) info.innerHTML = `
✓ Active project: ${esc(active.name||'')}${active.number?' ('+esc(active.number)+')':''}  
`; - // Pull the project's shared SOP/WPs from the server into the local cache - // first, so the SOP "Complete / Review" status reflects what other users did. - if(ProjectData.pullProject){ ProjectData.pullProject(active.id).then(()=>reflectSOPStatus(active)).catch(()=>reflectSOPStatus(active)); } - else reflectSOPStatus(active); + // Pull the project's shared SOP/WPs into the local cache so the tools boot + // with data. The card status below does NOT come from that cache — see + // reflectSOPStatus. + if(ProjectData.pullProject){ ProjectData.pullProject(active.id).catch(()=>{}); } + reflectSOPStatus(active); } function clearActiveProject(){ ProjectData.setActive(null); renderProjectPicker(); applyActiveProject(); } - // Reflect SOP completion on the tool cards (scoped to the active project). + // Reflect SOP completion on the tool cards (B4). + // + // This used to read `wp_suite_sop_complete` out of localStorage. That key is a + // per-browser mirror of the server, so the card answered "has THIS browser seen + // the SOP completed", not "is the SOP complete" — and the two diverge the moment + // a colleague finishes the SOP on their own machine. The card said "Complete SOP + // first" and nothing indicated the answer was stale, which is the failure mode + // B4 describes: a per-browser number that looks authoritative. + // + // There is deliberately no cache fallback. If the server cannot be reached the + // card says so; it does not guess, and it does not show a remembered answer as + // though it were current. function reflectSOPStatus(active){ - let complete = false, projName = ''; - try { - // Storage is namespaced per project, so these already scope to `active`. - complete = localStorage.getItem(ProjectData.key('wp_suite_sop_complete')) === '1'; - const sop = JSON.parse(localStorage.getItem(ProjectData.key('wp_suite_sop')) || 'null'); - projName = sop && sop.project && sop.project.name || ''; - } catch(e){} - const sopCard = document.getElementById('card-sop'); const sopBtn = document.getElementById('card-sop-btn'); const wpCard = document.getElementById('card-wp'); @@ -596,18 +603,44 @@ wpCard && wpCard.classList.remove('disabled'); const oldStatus = sopCard.querySelector('.card-status'); if(oldStatus) oldStatus.remove(); - if(complete){ - sopCard.classList.add('complete'); - sopBtn.textContent = 'Review'; - const status = document.createElement('div'); - status.className = 'card-status'; - status.textContent = '✓ SOP Complete' + (projName ? ' — ' + projName : ''); - sopCard.insertBefore(status, sopCard.firstChild); - if(wpBtn) wpBtn.textContent = 'Open Creator'; - } else { - if(wpCard) wpCard.classList.add('disabled'); - if(wpBtn) wpBtn.textContent = 'Complete SOP first'; - } + const setStatus = (text, cls) => { + const el = document.createElement('div'); + el.className = 'card-status' + (cls ? ' ' + cls : ''); + el.textContent = text; + sopCard.insertBefore(el, sopCard.firstChild); + return el; + }; + + // Neutral while in flight: neither "complete" nor "not complete" is known yet, + // and asserting either would be the same lie in a different direction. + sopBtn.textContent = 'Open tool'; + if(wpBtn) wpBtn.textContent = 'Checking…'; + + fetch('/api/projects/' + encodeURIComponent(active.id) + '/summary', + { headers: { 'Accept': 'application/json' } }) + .then(r => { if(!r.ok) throw new Error('HTTP ' + r.status); return r.json(); }) + .then(sum => { + if(ProjectData.getActiveId() !== active.id) return; // switched while in flight + const old = sopCard.querySelector('.card-status'); if(old) old.remove(); + if(sum.sop_complete){ + sopCard.classList.add('complete'); + sopBtn.textContent = 'Review'; + setStatus('✓ SOP complete' + (sum.sop_name ? ' — ' + sum.sop_name : '')); + if(wpBtn) wpBtn.textContent = 'Open creator'; + } else { + if(wpCard) wpCard.classList.add('disabled'); + if(wpBtn) wpBtn.textContent = 'Complete SOP first'; + } + }) + .catch(err => { + if(ProjectData.getActiveId() !== active.id) return; + const old = sopCard.querySelector('.card-status'); if(old) old.remove(); + // Explicitly unknown. The Creator is left reachable rather than disabled: + // locking someone out of their work because a status request failed is a + // worse outcome than letting the tool tell them itself. + setStatus('⚠ Could not check SOP status — ' + (err && err.message || 'offline'), 'card-status-error'); + if(wpBtn) wpBtn.textContent = 'Open creator'; + }); } initProjects(); diff --git a/html/wp-creation-app.js b/html/wp-creation-app.js index 5d69ce8..a75665b 100644 --- a/html/wp-creation-app.js +++ b/html/wp-creation-app.js @@ -1779,6 +1779,21 @@ const WPData = { }; let dashFilter={status:'',discipline:'',q:'',flag:''}; +// A write has to reach the server before the server can count it. The outbox is +// the only path writes take, so flush it and then re-read - otherwise the refresh +// races the push and shows the pre-write totals, which is the same stale number +// B4 removed, just arriving a different way. +function dashRefreshAfterWrite(){ + const done = () => loadDashMetrics(); + try { + if(typeof ProjectData!=='undefined' && ProjectData.flushSync){ + Promise.resolve(ProjectData.flushSync()).then(done, done); + return; + } + } catch(e){} + done(); +} + function dashToggleFlag(f){ if(f==='all'){ dashFilter={status:'',discipline:'',q:'',flag:''}; } else { dashFilter.flag = dashFilter.flag===f ? '' : f; } @@ -1790,8 +1805,10 @@ function dashSetStatus(s){ dashFilter.status = dashFilter.status===s ? '' : s; d let dashPage=0, dashShowArchived=false, dashArchived=[]; const DASH_PAGE_SIZE=25; // Weighted completion by status (0..1) so progress is smoother than done/not-done. -const PROGRESS_W={'Draft':0,'Scheduled':0.25,'Issue':0.4,'Issued':0.5,'In Progress':0.75,'QC':0.9,'Closed':1}; -function wpProgress(p){ const w=PROGRESS_W[p.status]; return w==null?0:w; } +// The progress weights moved to server/app.py PROGRESS_WEIGHT at T4.1 (B4), so the +// bar is computed once for everyone instead of once per browser. Deleted here +// rather than left in place: a second copy of a weighting table is how the two +// drift apart, and nothing in this file reads it any more. function dashGo(pg){ dashPage=pg; renderDashboard(); } function dashToggleArchived(on){ dashShowArchived=!!on; dashPage=0; @@ -1804,13 +1821,13 @@ function dashArchive(id){ if(!confirm('Archive "'+(p.number||p.subject||'this package')+'"? It will be hidden from the active board but kept for the record.')) return; if(typeof ProjectData!=='undefined' && ProjectData.archiveWP) ProjectData.archiveWP(id,true); const ix=savedPackages.findIndex(x=>x.id===id); if(ix>=0){ p.archived=true; dashArchived.unshift(p); savedPackages.splice(ix,1); } - saveStore(); renderSavedList(); renderDashboard(); toast('Archived '+(p.number||'')); + saveStore(); renderSavedList(); toast('Archived '+(p.number||'')); dashRefreshAfterWrite(); } function dashUnarchive(id){ const ix=dashArchived.findIndex(x=>x.id===id); const p=ix>=0?dashArchived[ix]:null; if(!p) return; if(typeof ProjectData!=='undefined' && ProjectData.archiveWP) ProjectData.archiveWP(id,false); p.archived=false; dashArchived.splice(ix,1); if(!savedPackages.some(x=>x.id===id)) savedPackages.push(p); - saveStore(); renderSavedList(); renderDashboard(); toast('Restored '+(p.number||'')); + saveStore(); renderSavedList(); toast('Restored '+(p.number||'')); dashRefreshAfterWrite(); } // Consistent colored status pill, reused by the dashboard board and the saved list. function statusPill(s){ @@ -1832,30 +1849,74 @@ function showDashboard(){ document.querySelectorAll('.main > .card, .main > .nav-row').forEach(e=>e.style.display='none'); document.getElementById('pkg-output').style.display='none'; const dv=document.getElementById('dashboard-view'); if(dv) dv.style.display=''; - currentView='Dashboard'; cmtUpdateCurStep(); renderDashboard(); + currentView='Dashboard'; cmtUpdateCurStep(); + // Ask the server every time the dashboard is opened. Counts that were correct + // when you last looked are not evidence that they are correct now. + dashMetrics=null; loadDashMetrics(); window.scrollTo({top:0,behavior:'smooth'}); track('dashboard_open'); } +// ── B4: dashboard counts come from the server ──────────────────────────────── +// Every number on this dashboard used to be summed in the browser from +// savedPackages, which is this browser's localStorage. Two people on the same +// project saw different totals and neither was told. /api/wps/metrics computes +// them from the database instead, so the answer is the project's answer. +// +// There is no localStorage fallback. A silently-stale number that looks +// authoritative is the failure being fixed, so a failed fetch renders an error +// panel and a retry - not a zero, and not the last good answer. +let dashMetrics = null; // last successful server response +let dashMetricsErr = null; // Error, if the last fetch failed +let dashMetricsLoading = false; + +function dashMetricsUrl(){ + const q = []; + if(activeProjectId) q.push('project_id=' + encodeURIComponent(activeProjectId)); + return '/api/wps/metrics' + (q.length ? '?' + q.join('&') : ''); +} + +function loadDashMetrics(){ + dashMetricsLoading = true; dashMetricsErr = null; + return fetch(dashMetricsUrl(), { headers: { 'Accept': 'application/json' } }) + .then(r => { if(!r.ok) throw new Error('HTTP ' + r.status); return r.json(); }) + .then(m => { dashMetrics = m; dashMetricsErr = null; }) + .catch(e => { dashMetricsErr = e; dashMetrics = null; }) + .then(() => { dashMetricsLoading = false; renderDashboard(); }); +} + function renderDashboard(){ const all=countableWPs(); - const byStatus={}; STATUS_ORDER.concat(['Issue']).forEach(s=>byStatus[s]=0); - let estH=0, actH=0, ready=0, hold=0, overdue=0, mine=0; const byDisc={}; const meId=myUserId(); - all.forEach(p=>{ - byStatus[p.status]=(byStatus[p.status]||0)+1; - estH+=parseFloat(p.hours)||0; actH+=parseFloat(p.actualHrs)||0; - if(p.status==='Issue') hold++; - if(meId && p.assigneeId===meId) mine++; - if(!wpReleaseBlocked(p) && p.status!=='Closed' && p.status!=='Issue') ready++; - if(isOverdue(p)) overdue++; - (p.disciplines&&p.disciplines.length?p.disciplines:['(none)']).forEach(d=>byDisc[d]=(byDisc[d]||0)+1); - }); + const m = dashMetrics; + const byStatus = m ? Object.assign({}, m.by_status) : {}; + const byDisc = m ? Object.assign({}, m.by_discipline) : {}; + if(m) STATUS_ORDER.concat(['Issue']).forEach(s=>{ if(byStatus[s]==null) byStatus[s]=0; }); + const estH=m?m.est_hours:0, actH=m?m.actual_hours:0, ready=m?m.release_ready:0, + hold=m?m.on_hold:0, overdue=m?m.overdue:0, mine=m?m.mine:0; + const meId=myUserId(); // Clickable metric cards filter the board (flag-based); a card with no flag is static. const card=(label,val,cls,flag)=>{ const active = flag && dashFilter.flag===flag ? ' dm-active' : ''; const attr = flag ? ` onclick="dashToggleFlag('${flag}')" title="Click to filter the board"` : ''; return `
${val}
${esc(label)}
`; }; + // The counts panel is server-derived; if that request failed, say so instead of + // rendering a row of zeros that reads as "this project is empty". + if(dashMetricsErr){ + let eh = ``; + const host0=document.getElementById('dashboard-view'); + if(host0){ host0.innerHTML = eh; } + return; + } + if(!m && dashMetricsLoading){ + const host1=document.getElementById('dashboard-view'); + if(host1){ host1.innerHTML = `
Loading project totals…
`; } + return; + } + if(!m){ loadDashMetrics(); return; } + let h=`
- ${card('Total WPs', all.length, '', 'all')} + ${card('Total WPs', m.total, '', 'all')} ${meId ? card('My WPs', mine, mine?'dm-blue':'', 'mine') : ''} ${card('Release-ready', ready, ready?'dm-green':'', 'ready')} ${card('On hold', hold, hold?'dm-red':'', 'onhold')} @@ -1872,23 +1933,23 @@ function renderDashboard(){ h+=`
By status
${statusChips||'—'}
By discipline
${discChips}
`; - // progress by phase (discipline), weighted by status; archived excluded - const overallPct = all.length ? Math.round(all.reduce((s,p)=>s+wpProgress(p),0)/all.length*100) : 0; - const phaseGroups={}; - all.forEach(p=>{ (p.disciplines&&p.disciplines.length?p.disciplines:['(none)']).forEach(d=>{ (phaseGroups[d]=phaseGroups[d]||[]).push(p); }); }); + // progress by phase (discipline), weighted by status; archived excluded. + // The weights live in server/app.py PROGRESS_WEIGHT now, so one definition + // produces the bar for everyone rather than one per browser. + const overallPct = m.progress.overall_pct; let prog=`
Progress by phase
`; prog+=`
Overall
${overallPct}%
`; - Object.keys(phaseGroups).sort().forEach(d=>{ const g=phaseGroups[d]; const pct=g.length?Math.round(g.reduce((s,p)=>s+wpProgress(p),0)/g.length*100):0; const done=g.filter(p=>p.status==='Closed').length; - prog+=`
${esc(d)}
${pct}% ${done}/${g.length}
`; }); + m.progress.by_discipline.forEach(g=>{ + prog+=`
${esc(g.name)}
${g.pct}% ${g.done}/${g.total}
`; }); prog+=`
Weighted by status (Draft 0 · Scheduled 25 · Issued 50 · In Progress 75 · QC 90 · Closed 100%). Archived packages excluded.
`; h+=prog; - // gating panel — what's blocking release - const gated=all.filter(p=>wpOpenConstraints(p).length>0); + // gating panel — what's blocking release, from the server + const gated=m.gating||[]; h+=`
⛔ Gating constraints (${gated.length} package${gated.length===1?'':'s'} blocked)
`; h+= gated.length ? ``+ - gated.map(p=>` - `).join('')+ + gated.map(g=>` + `).join('')+ `
WP #SubjectBlocked by
${esc(p.number||'—')}${esc(p.subject||'')}${wpOpenConstraints(p).map(c=>esc(c.name)+(c.comment?` (${esc(c.comment)})`:'')).join('
')}
${esc(g.number||'—')}${esc(g.subject||'')}${(g.blocked_by||[]).map(c=>esc(c.name)+(c.comment?` (${esc(c.comment)})`:'')).join('
')}
` : `
No open constraints — every package is clear of gates.
`; h+=`
`; @@ -1921,7 +1982,18 @@ function renderDashboard(){ if(dashPage>=pages) dashPage=pages-1; if(dashPage<0) dashPage=0; const pageRows=rows.slice(dashPage*DASH_PAGE_SIZE, dashPage*DASH_PAGE_SIZE+DASH_PAGE_SIZE); - h+=`
Work Packages (${totalRows})
+ // The board is a LIST, not a rollup: it renders the packages this browser holds, + // which is what keeps the field view usable offline. Its header is therefore the + // only count on this page not computed by the server — so it is reconciled against + // the server's instead of being left to disagree in silence, which is the whole of + // what B4 objects to. A divergence means this browser's copy is behind (a pending + // outbox write, a stale tab), and saying so is more useful than hiding it. + const localCountable = (dashShowArchived ? WPData.list() : WPData.list().filter(p=>!p.archived)) + .filter(p=>!p.split).length; + const drift = (!dashShowArchived && m && localCountable !== m.total) + ? ` this browser has ${localCountable} of ${m.total}` + : ''; + h+=`
Work packages (${totalRows})${drift}
`; if(!totalRows) h+=``; pageRows.forEach(p=>{ @@ -1970,7 +2042,8 @@ function dashIssue(id){ return; } if(!confirm('Issue work package "'+(p.number||p.subject)+'"? This marks it released to the field.')) return; - WPData.issue(id); renderDashboard(); renderSavedList(); toast('Issued '+(p.number||'')); track('dashboard_issue'); + WPData.issue(id); renderSavedList(); toast('Issued '+(p.number||'')); track('dashboard_issue'); + dashRefreshAfterWrite(); } function dashView(i){ if(savedPackages[i]) renderPackage(savedPackages[i]); } function dashEdit(i){ const p=savedPackages[i]; if(!p) return; editingId=p.id; loadPackageIntoForm(p); } diff --git a/server/app.py b/server/app.py index 3b56662..945acbc 100644 --- a/server/app.py +++ b/server/app.py @@ -1717,11 +1717,45 @@ def list_wps( return [(w.to_dict() if full else w.summary()) for w in rows] +# Weighted completion by status, 0..1, so progress reads as a slope rather than +# done/not-done. Mirrors PROGRESS_W in wp-creation-app.js — the dashboard used to +# compute this in the browser from localStorage, which is exactly what B4 removes. +PROGRESS_WEIGHT = { + "Draft": 0.0, "Scheduled": 0.25, "Issue": 0.4, "Issued": 0.5, + "In Progress": 0.75, "QC": 0.9, "Closed": 1.0, +} + +# The dimensions a location rollup is grouped by. Today a work package carries one +# free-text `location` ("building / level / sector / room"), so that is the single +# dimension. CR-004 replaces it with structured building / floor / sector in wave 6; +# when it does, only this tuple and the keys inside each group change — the response +# shape does not, which is what T4.1 means by "can be grouped by location without a +# schema change". CR-018's rollup consumes `by_location.groups` either way. +LOCATION_DIMENSIONS = ("location",) + + +def _location_key(data: dict) -> dict: + """The location dimensions of one package, as a dict keyed by dimension name. + + Structured fields win when they exist, so this keeps working unchanged the day + CR-004 lands; until then it falls back to the free-text field.""" + structured = {d: (data.get(d) or "").strip() for d in ("building", "floor", "sector")} + if any(structured.values()): + return {k: v or "(unset)" for k, v in structured.items()} + return {"location": (data.get("location") or "").strip() or "(unset)"} + + @app.get("/api/wps/metrics") def wp_metrics(project_id: Optional[str] = Query(None), sop_id: Optional[str] = Query(None), user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)): - """Aggregates for the dashboard. Masters (data.split == true) are excluded - from counts so a split package's hours aren't double-counted with its - instances.""" + """Every count and rollup the creator's dashboard shows, computed from the + database rather than from the caller's own browser (B4). + + Masters (data.split == true) are excluded so a split package's hours are not + double-counted with its instances. Archived packages are excluded throughout. + + Two people on the same project get the same numbers from this endpoint. They + did not when each browser derived them from its own localStorage, and neither + was told — which is the failure B4 exists to remove.""" stmt = select(models.WorkPackage).where(models.WorkPackage.archived_at.is_(None)) if project_id: stmt = stmt.where(models.WorkPackage.project_id == project_id) @@ -1730,9 +1764,17 @@ def wp_metrics(project_id: Optional[str] = Query(None), sop_id: Optional[str] = stmt = scope_to_access(stmt, models.WorkPackage.project_id, db, user) rows = db.scalars(stmt).all() + today = models.utcnow().date().isoformat() + by_status: dict[str, int] = {} by_discipline: dict[str, int] = {} - total = ready = on_hold = est_hours = actual_hours = 0 + loc_groups: dict[tuple, dict] = {} + prog_by_disc: dict[str, dict] = {} + gating: list[dict] = [] + total = ready = on_hold = overdue = mine = 0 + est_hours = actual_hours = 0.0 + progress_sum = 0.0 + for w in rows: data = w.data or {} if data.get("split"): @@ -1741,22 +1783,127 @@ def wp_metrics(project_id: Optional[str] = Query(None), sop_id: Optional[str] = by_status[w.status] = by_status.get(w.status, 0) + 1 if w.status == "Issue": on_hold += 1 + if w.assignee_id and w.assignee_id == user.id: + mine += 1 + + due = (data.get("due") or "").strip() + is_overdue = bool(due and w.status != "Closed" and due < today) + if is_overdue: + overdue += 1 + constraints = data.get("constraints") or [] - open_count = sum(1 for c in constraints if c.get("status") == "open") - if open_count == 0 and w.status not in ("Closed", "Issue"): + open_constraints = [c for c in constraints if c.get("status") == "open"] + # `waitingOn` are predecessor packages not yet closed; the browser counted + # them as blocking too, so the server has to, or "release-ready" changes + # meaning the moment the dashboard stops computing it locally. + waiting_on = [x for x in (data.get("waitingOn") or []) if x] + blocked = bool(open_constraints or waiting_on) + if not blocked and w.status not in ("Closed", "Issue"): ready += 1 + if open_constraints: + gating.append({ + "id": w.id, "number": w.number, "subject": w.subject, + "blocked_by": [ + {"name": c.get("name") or "", "comment": c.get("comment") or ""} + for c in open_constraints + ], + }) + try: est_hours += float(data.get("hours") or 0) + except (TypeError, ValueError): + pass + try: actual_hours += float(data.get("actualHrs") or 0) except (TypeError, ValueError): pass - for d in (data.get("disciplines") or ["(none)"]): + + weight = PROGRESS_WEIGHT.get(w.status, 0.0) + progress_sum += weight + + disciplines = data.get("disciplines") or ["(none)"] + for d in disciplines: by_discipline[d] = by_discipline.get(d, 0) + 1 + slot = prog_by_disc.setdefault(d, {"total": 0, "done": 0, "weight": 0.0}) + slot["total"] += 1 + slot["weight"] += weight + if w.status == "Closed": + slot["done"] += 1 + + key = _location_key(data) + kt = tuple(key.get(d, "(unset)") for d in LOCATION_DIMENSIONS) if len(key) == len(LOCATION_DIMENSIONS) else tuple(sorted(key.items())) + slot = loc_groups.setdefault(kt, {"key": key, "total": 0, "release_ready": 0, + "on_hold": 0, "overdue": 0, "by_status": {}}) + slot["total"] += 1 + slot["by_status"][w.status] = slot["by_status"].get(w.status, 0) + 1 + if not blocked and w.status not in ("Closed", "Issue"): + slot["release_ready"] += 1 + if w.status == "Issue": + slot["on_hold"] += 1 + if is_overdue: + slot["overdue"] += 1 + + dimensions = list(LOCATION_DIMENSIONS) + if loc_groups: + first = next(iter(loc_groups.values()))["key"] + dimensions = list(first.keys()) return { - "total": total, "release_ready": ready, "on_hold": on_hold, + "total": total, "mine": mine, "release_ready": ready, "on_hold": on_hold, + "overdue": overdue, "est_hours": round(est_hours), "actual_hours": round(actual_hours), "by_status": by_status, "by_discipline": by_discipline, + "progress": { + "overall_pct": round(progress_sum / total * 100) if total else 0, + "by_discipline": [ + {"name": d, "pct": round(v["weight"] / v["total"] * 100) if v["total"] else 0, + "done": v["done"], "total": v["total"]} + for d, v in sorted(prog_by_disc.items()) + ], + }, + "gating": sorted(gating, key=lambda g: (g["number"] or "", g["id"])), + "by_location": {"dimensions": dimensions, "groups": sorted( + loc_groups.values(), key=lambda g: tuple(str(v) for v in g["key"].values()))}, + "generated_at": models.utcnow().isoformat(), + } + + +@app.get("/api/projects/{project_id}/summary") +def project_summary(project_id: str, user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)): + """What the launcher needs to describe a project without asking the browser + what it remembers (B4). + + The launcher used to read `wp_suite_sop_complete` out of localStorage, which is + a per-browser mirror: a colleague completing the SOP on their machine left your + card saying "Complete SOP first" with nothing to indicate the answer was stale.""" + proj = db.get(models.Project, project_id) + if not proj: + raise HTTPException(status_code=404, detail="Project not found") + require_project_access(db, user, project_id) + + 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() + + wp_total = db.scalar( + select(func.count()).select_from(models.WorkPackage).where( + models.WorkPackage.project_id == project_id, + models.WorkPackage.archived_at.is_(None), + ) + ) or 0 + + return { + "project_id": project_id, + "project_name": proj.name, + "sop_complete": sop is not None, + "sop_id": sop.id if sop else None, + "sop_name": (sop.name if sop else "") or "", + "sop_updated_at": models._iso(sop.updated_at) if sop else None, + "wp_total": wp_total, + "generated_at": models.utcnow().isoformat(), } diff --git a/tests/aggregates_check.py b/tests/aggregates_check.py new file mode 100644 index 0000000..d3ad614 --- /dev/null +++ b/tests/aggregates_check.py @@ -0,0 +1,257 @@ +#!/usr/bin/env python3 +"""Do the counts come from the server? — B4 / T4.1. + +The defect B4 names is not "the numbers are wrong". It is that they were derived +from the caller's own localStorage, so two people on the same project saw +different numbers and neither was told. A test that only checks the totals are +correct would have passed before this change, because on one browser with one +cache they were correct. + +So this checks the thing that was actually broken: + + 1. the same project reports the same aggregates to two different users + 2. the dashboard shows the SERVER's total even when the browser's own cache has + been poisoned with a different one — which it cannot do if it is summing + localStorage + 3. a failed aggregate request renders an explicit error and a retry, not a zero + and not the last good answer + 4. the launcher's SOP status survives a poisoned cache the same way + 5. the aggregate response can be grouped by location without a schema change + +Self-contained: throwaway SQLite, its own uvicorn, headless Edge or Chrome over +CDP, all torn down. Exit 0 all passed, 1 a failure, 2 could not run. +""" +import json +import os +import subprocess +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, _c # noqa: E402 + + +def api(base, path, token, method="GET", body=None): + req = urllib.request.Request(base + path, method=method) + req.add_header("Cookie", "wp_session=" + token) + req.add_header("Accept", "application/json") + data = None + if body is not None: + data = json.dumps(body).encode() + req.add_header("Content-Type", "application/json") + with urllib.request.urlopen(req, data, timeout=15) as r: + return json.loads(r.read().decode() or "null") + + +def main(): + exe = cdp.find_browser() + if not exe: + print("no headless-capable browser found; set WP_BROWSER.") + return 2 + + tmpdir = tempfile.mkdtemp(prefix="wpsuite-aggregates-") + db_path = os.path.join(tmpdir, "check.db") + server = None + try: + tok = seed(db_path) + port = cdp.free_port() + base = "http://127.0.0.1:%d" % port + server = start_server(port, db_path) + if server is None: + print("the test server would not start.") + return 2 + print("\nAggregate counts — B4 / T4.1\nTarget: %s" % base) + + # Extra packages so the counts are not all the same number: one on hold, + # one blocked by an open constraint, one closed, each with a location. + for i, (num, status, constraint, loc, hours) in enumerate([ + ("WP03-HOLD", "Issue", "cleared", "B100 / L2 / P", "12"), + ("WP04-GATE", "Draft", "open", "B100 / L2 / P", "8"), + ("WP05-DONE", "Closed", "cleared", "B100 / L3 / Q", "20"), + ]): + api(base, "/api/wps", tok["root"], "POST", { + "id": "wpX%d" % i, "project_id": "projA", "sop_id": "sopA", + "number": num, "subject": num, "type": "Conduit Install", + "status": status, + "data": {"disciplines": ["Electrical"], "hours": hours, + "location": loc, + "constraints": [{"name": "Materials", "status": constraint, + "comment": "waiting on delivery"}]}, + }) + + print("\n1. the same project reports the same aggregates to two users") + as_root = api(base, "/api/wps/metrics?project_id=projA", tok["root"]) + as_pat = api(base, "/api/wps/metrics?project_id=projA", tok["pat"]) + comparable = ["total", "release_ready", "on_hold", "overdue", + "est_hours", "actual_hours", "by_status", "by_discipline"] + same = {k: as_root[k] for k in comparable} == {k: as_pat[k] for k in comparable} + chk("root and pat get identical aggregates for projA", same, + "root=%s pat=%s" % ({k: as_root[k] for k in comparable}, + {k: as_pat[k] for k in comparable})) + chk("the fixture is not degenerate (>=5 packages, a hold, a gate)", + as_root["total"] >= 5 and as_root["on_hold"] >= 1 and len(as_root["gating"]) >= 1, + "total=%s on_hold=%s gating=%s" % (as_root["total"], as_root["on_hold"], + len(as_root["gating"]))) + chk("'mine' is per-user, so it is allowed to differ", + "mine" in as_root and "mine" in as_pat) + + print("\n5. the aggregate can be grouped by location without a schema change") + loc = as_root.get("by_location") or {} + chk("by_location carries its dimensions", isinstance(loc.get("dimensions"), list) + and len(loc["dimensions"]) >= 1, loc.get("dimensions")) + groups = loc.get("groups") or [] + chk("by_location groups are keyed by those dimensions", + bool(groups) and all(set(g["key"]) == set(loc["dimensions"]) for g in groups), + [g.get("key") for g in groups[:3]]) + chk("each group carries its own rollup, not just a count", + bool(groups) and all({"total", "release_ready", "on_hold", "by_status"} <= set(g) + for g in groups), + list(groups[0]) if groups else None) + chk("the groups sum to the project total", + sum(g["total"] for g in groups) == as_root["total"], + "%s vs %s" % (sum(g["total"] for g in groups), as_root["total"])) + + browser = cdp.Browser(exe) + page = browser.page() + try: + print("\n2. the dashboard shows the server's total, not the browser's") + page.clear_cookies() + page.set_cookie("wp_session", tok["root"]) + page.goto(base + "/wp-creation-index.html?project=projA") + time.sleep(1.5) + # Poison this browser's cache with a different number of packages than + # the server has. If any displayed count still tracks localStorage, it + # will report 2 and the server's total will not match. + page.eval("""(() => { + const fake = [ + {id:'fake1', number:'FAKE-1', subject:'not on the server', status:'Draft', + disciplines:['Electrical'], hours:'999', constraints:[]}, + {id:'fake2', number:'FAKE-2', subject:'also not', status:'Draft', + disciplines:['Electrical'], hours:'999', constraints:[]} + ]; + localStorage.setItem('wp_iwp_v1::projA', JSON.stringify(fake)); + localStorage.setItem('wp_iwp_v1', JSON.stringify(fake)); + return true; + })()""") + page.goto(base + "/wp-creation-index.html?project=projA&view=dashboard") + time.sleep(1.2) + page.eval("typeof showDashboard==='function' && showDashboard()") + for _ in range(30): + ready = page.eval("!!document.querySelector('.dash-metric .dm-val')") + if ready: + break + time.sleep(0.3) + shown = page.eval( + "(()=>{const e=[...document.querySelectorAll('.dash-metric')]" + ".find(x=>/Total WPs/i.test(x.textContent));" + "return e?e.querySelector('.dm-val').textContent.trim():null})()") + chk("dashboard 'Total WPs' equals the server total", + str(shown) == str(as_root["total"]), + "shown=%r server=%r (poisoned cache said 2)" % (shown, as_root["total"])) + chk("...and is therefore not the poisoned cache's 2", str(shown) != "2", shown) + + print("\n3. a failed aggregate request is an error, not a zero") + page.eval("""(() => { + const real = window.fetch; + window.fetch = function(u, o){ + if (String(u).indexOf('/api/wps/metrics') !== -1) + return Promise.reject(new Error('simulated outage')); + return real.apply(this, arguments); + }; + dashMetrics = null; dashMetricsErr = null; + loadDashMetrics(); + return true; + })()""") + time.sleep(1.2) + txt = page.eval("(document.getElementById('dashboard-view')||{}).textContent||''") + chk("an explicit error panel is shown", "Counts unavailable" in txt, txt[:120]) + chk("...naming the failure", "simulated outage" in txt, txt[:160]) + chk("...offering a retry", + page.eval("!!document.querySelector('#dashboard-view button')")) + chk("no metric tiles are rendered alongside the error", + page.eval("document.querySelectorAll('#dashboard-view .dash-metric').length") == 0) + chk("the error region announces itself", + page.eval("!!document.querySelector('#dashboard-view [role=alert]')")) + + print("\n4. the launcher's SOP status comes from the server too") + page.goto(base + "/index.html") + time.sleep(0.6) + page.eval("""(() => { + try { + localStorage.setItem('wp_active_project_id', 'projA'); + localStorage.setItem('wp_suite_sop_complete::projA', '0'); + localStorage.removeItem('wp_suite_sop::projA'); + } catch(e){} + return true; + })()""") + page.goto(base + "/index.html") + for _ in range(30): + s = page.eval("(document.querySelector('#card-sop .card-status')||{}).textContent||''") + if s: + break + time.sleep(0.3) + status = page.eval( + "(document.querySelector('#card-sop .card-status')||{}).textContent||''") + chk("launcher reports the SOP complete despite a cache that says otherwise", + "SOP complete" in status, "card said %r" % status) + + page.goto(base + "/index.html") + time.sleep(0.4) + page.eval("""(() => { + const real = window.fetch; + window.fetch = function(u, o){ + if (String(u).indexOf('/summary') !== -1) + return Promise.reject(new Error('simulated outage')); + return real.apply(this, arguments); + }; + return true; + })()""") + page.eval("typeof applyActiveProject==='function' && applyActiveProject()") + time.sleep(1.0) + status = page.eval( + "(document.querySelector('#card-sop .card-status')||{}).textContent||''") + chk("a failed status request says so on the card", + "Could not check" in status, "card said %r" % status) + finally: + page.close() + browser.close() + except urllib.error.HTTPError as e: + print("API error: %s %s" % (e.code, e.read()[:300])) + return 2 + finally: + if server: + server.kill() + try: + server.wait(timeout=10) + except subprocess.TimeoutExpired: + pass + try: + from server.db import engine + engine.dispose() + except Exception: + pass + import shutil + for _ in range(10): + shutil.rmtree(tmpdir, ignore_errors=True) + if not os.path.exists(tmpdir): + break + time.sleep(0.3) + + total = len(_PASS) + len(_FAIL) + print("\n%s\n%d/%d checks passed." % ("-" * 54, len(_PASS), total)) + if _FAIL: + for f in _FAIL: + print(" - " + f) + return 1 + print("\nResult: " + _c("ALL PASS — counts come from the server.", "32") + "\n") + return 0 + + +if __name__ == "__main__": + sys.exit(main())
WP #SubjectTypeDisciplineStatusGatesDueHrs
No work packages match.