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

T4.1 - B4: the counts come from the server, and disagreement is said out loud

Browse Source 
The defect B4 names is not that the numbers were wrong. On one browser with one
cache they were right. 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.

SERVER. /api/wps/metrics now returns everything the creator's dashboard shows -
total, mine, release-ready, on hold, overdue, est/actual hours, by_status,
by_discipline, progress (overall and per discipline) and the gating list. It
already existed for a subset; the rest was being summed in the browser.

Two things moved to the server rather than being duplicated there:

  - PROGRESS_WEIGHT, the status-to-percentage table. It was PROGRESS_W in
    wp-creation-app.js; the JS copy is deleted rather than left in place, because
    two copies of a weighting table is how the two drift apart.
  - "release-ready" now counts `waitingOn` predecessors as blocking, which the
    browser did and the old endpoint did not. Without that the phrase would have
    changed meaning the moment the dashboard stopped computing it locally.

New GET /api/projects/{id}/summary gives the launcher the SOP state it was
reading out of localStorage.

by_location is shaped for CR-018 in wave 6, per the task's instruction not to
build a shape that cannot group by building/floor/sector. It reports its own
dimensions alongside the groups:

  {"dimensions": ["location"], "groups": [{"key": {...}, "total": n,
    "release_ready": n, "on_hold": n, "overdue": n, "by_status": {...}}]}

Today a package carries one free-text `location`, so that is the one dimension.
_location_key() already prefers structured building/floor/sector when present, so
CR-004 changes the dimensions and the keys and leaves the response shape alone.

CLIENT. The launcher's SOP card and the dashboard's tiles, chips, progress bars
and gating panel all read the server. There is deliberately no cache fallback: a
silently-stale number that looks authoritative is the thing being removed, so a
failed request renders an explicit error and a retry.

Writes flush through the outbox before the counts are re-read (dashRefreshAfterWrite).
Without that the refresh races the push and shows pre-write totals - the same
stale number arriving by a different route.

THE ONE COUNT STILL COMPUTED LOCALLY, stated rather than skipped: the board table
is a LIST of the packages this browser holds, which is what keeps the field view
working offline, and its header counts rows. Rather than pretend otherwise, it is
reconciled against the server's total and shows "this browser has N of M" when
they differ - usually a save that has not finished syncing. Nobody now sees a
number that disagrees with the project without being told, which is what B4 asks
for.

VERIFICATION. tests/aggregates_check.py, 16 checks, all passing. It tests what was
broken rather than whether the totals are right - the latter passed before this
change:

  - root and pat get byte-identical aggregates for the same project
  - the dashboard shows the server's total after localStorage is POISONED with a
    different package list; it cannot do that if it is summing the cache
  - a simulated outage renders "Counts unavailable", names the failure, offers a
    retry, renders no tiles beside it, and announces via role=alert
  - the launcher reports the SOP complete against a cache that says otherwise,
    and says "Could not check" when the request fails
  - by_location's groups are keyed by its declared dimensions, carry their own
    rollups, and sum to the project total

browser_check 71/71, f_items 5 FIXED / F6 REPRODUCES.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Nick Siegfried
2026-08-15 19:24:12 -05:00
parent 12c0e5ca74
commit b670ae719d
4 changed files with 572 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

83
html/index.html
View File

@@ -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 = `<div class="proj-active">✓ Active project: <strong>${esc(active.name||'')}</strong>${active.number?' ('+esc(active.number)+')':''}
&nbsp;<button class="link-like" onclick="clearActiveProject()">change</button></div>`;
// 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();

131
html/wp-creation-app.js
View File

@@ -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 `<div class="dash-metric ${cls||''}${active}"${attr}><div class="dm-val">${val}</div><div class="dm-label">${esc(label)}</div></div>`;
};
// 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 = `<div class="dash-panel" role="alert"><div class="dash-panel-title">⚠ Counts unavailable</div>`;
eh += `<div class="field-hint">The dashboard totals are computed by the server so that everyone on this project sees the same numbers. That request failed (${esc(String(dashMetricsErr.message||dashMetricsErr))}), and this browser's own copy is not shown in its place because it can be out of date without looking it.</div>`;
eh += `<div class="material-actions"><button class="btn btn-ghost" onclick="loadDashMetrics()">Retry</button></div></div>`;
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 = `<div class="dash-panel" role="status"><div class="field-hint">Loading project totals…</div></div>`; }
return;
}
if(!m){ loadDashMetrics(); return; }
let h=`<div class="dash-metrics">
${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+=`<div class="dash-breakdown"><div><div class="dash-bd-title">By status</div>${statusChips||'—'}</div>
<div><div class="dash-bd-title">By discipline</div>${discChips}</div></div>`;
// 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=`<div class="dash-panel"><div class="dash-panel-title">Progress by phase</div>`;
prog+=`<div class="prog-row"><div class="prog-name"><strong>Overall</strong></div><div class="prog-bar"><div class="prog-fill" style="width:${overallPct}%"></div></div><div class="prog-pct">${overallPct}%</div></div>`;
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+=`<div class="prog-row"><div class="prog-name">${esc(d)}</div><div class="prog-bar"><div class="prog-fill" style="width:${pct}%"></div></div><div class="prog-pct">${pct}% <span class="prog-sub">${done}/${g.length}</span></div></div>`; });
m.progress.by_discipline.forEach(g=>{
prog+=`<div class="prog-row"><div class="prog-name">${esc(g.name)}</div><div class="prog-bar"><div class="prog-fill" style="width:${g.pct}%"></div></div><div class="prog-pct">${g.pct}% <span class="prog-sub">${g.done}/${g.total}</span></div></div>`; });
prog+=`<div class="field-hint" style="margin-top:8px">Weighted by status (Draft 0 · Scheduled 25 · Issued 50 · In&nbsp;Progress 75 · QC 90 · Closed 100%). Archived packages excluded.</div></div>`;
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+=`<div class="dash-panel"><div class="dash-panel-title">⛔ Gating constraints (${gated.length} package${gated.length===1?'':'s'} blocked)</div>`;
h+= gated.length ? `<table class="dash-table"><thead><tr><th>WP #</th><th>Subject</th><th>Blocked by</th></tr></thead><tbody>`+
gated.map(p=>`<tr><td class="row-label">${esc(p.number||'—')}</td><td>${esc(p.subject||'')}</td>
<td>${wpOpenConstraints(p).map(c=>esc(c.name)+(c.comment?` <span style="color:var(--text-dim)">(${esc(c.comment)})</span>`:'')).join('<br>')}</td></tr>`).join('')+
gated.map(g=>`<tr><td class="row-label">${esc(g.number||'—')}</td><td>${esc(g.subject||'')}</td>
<td>${(g.blocked_by||[]).map(c=>esc(c.name)+(c.comment?` <span style="color:var(--text-dim)">(${esc(c.comment)})</span>`:'')).join('<br>')}</td></tr>`).join('')+
`</tbody></table>` : `<div class="field-hint">No open constraints — every package is clear of gates.</div>`;
h+=`</div>`;
@@ -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+=`<div class="dash-panel"><div class="dash-panel-title">Work Packages (${totalRows})</div>
// 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)
? ` <span class="dash-chip chip-red" title="The server counts ${m.total} package(s) on this project; this browser is holding ${localCountable}. Usually a save that has not finished syncing.">this browser has ${localCountable} of ${m.total}</span>`
: '';
h+=`<div class="dash-panel"><div class="dash-panel-title">Work packages (${totalRows})${drift}</div>
<table class="dash-table"><thead><tr><th>WP #</th><th>Subject</th><th>Type</th><th>Discipline</th><th>Status</th><th>Gates</th><th>Due</th><th>Hrs</th><th></th></tr></thead><tbody>`;
if(!totalRows) h+=`<tr><td colspan="9" class="field-hint" style="padding:14px">No work packages match.</td></tr>`;
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); }

163
server/app.py
View File

@@ -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(),
}

257
tests/aggregates_check.py Normal file
View File

@@ -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())