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

T6.3/T6.4 - CR-004 and CR-018: picked not typed, and totals that add up

Browse Source 
CR-004 and CR-018 are the same change seen from two ends. CR-018 is why the
Acumatica cost code came out rather than being relabelled — the tracking
dimension the team wants is floor and area, not an accounting code — and CR-004
is what makes that dimension exist. Committed together because a rollup keyed on
free text is not a rollup, and structured location with nothing rolling up by it
is a form change nobody asked for.

CR-004 - three dependent dropdowns

  Building filters Floor filters Sector, off the project's own taxonomy from
  T5.4. Clearing a parent clears its children: not doing that is how a package
  ends up filed under a floor that is not in the building it claims.

  PATHS are stored, not names and not bare codes. A floor's own code is not
  unique across buildings; `B-ONE/L1` is. That is what lets the dashboard filter
  by a building and match everything beneath it with a prefix test, and it is
  what CR-018 groups on.

  The list is fetched with include_inactive=true, which is not a contradiction of
  CR-005's "deactivating hides it from new work packages" — they are two
  questions. What may be CHOSEN is active only. What may be SHOWN is everything,
  because a package already referencing a deactivated value still has to render
  its label, and blanking it on open would write the blank back on the next save.
  A deactivated value that IS on the package is offered, labelled "(no longer
  offered)"; on a fresh package it is not offered at all. Both checked.

  X5, checked the way aggregates_check checks its own: localStorage is poisoned
  with a fake building and the dropdown is required to ignore it.

  wp_location survives as a hidden field. A package written before this keeps
  what it said, and the form says so rather than dropping it.

CR-018 - the rollup

  LOCATION_DIMENSIONS is now ("building", "floor", "sector"). T4.1's note said
  "only this tuple and the keys inside each group change - the response shape
  does not", and that held exactly.

  Rolled up at EVERY level, server-side, not just at the leaf. "How many on
  floor 2" is the question CR-018 asks and it is a level above the leaf groups;
  summing them in the browser would be the same per-browser arithmetic B4
  removed. Actual Hours rolls up along the same dimensions - that is the field
  CR-017 retained, and this is why that decision mattered.

  Packages with no location are an explicit "(unassigned)" row, not a gap. The
  reason is arithmetic: a group set that silently omits them does not add up to
  the project total, and a rollup that does not reconcile is decoration. The
  probe checks every level sums to the project total, and to the estimated and
  actual hour totals, using distinct primes so a mis-sum cannot land on the
  right number by luck.

  A package with a building but no floor lands in the floor-level unassigned row
  alongside the one with no location at all - which is the honest answer, and is
  asserted by its hours rather than by its count.

  Free text captured before CR-004 groups under itself as a building rather than
  collapsing into unassigned, one level deep. Pretending free text is a
  hierarchy would file "FAB / LVL 1" under a building called "FAB / LVL 1".

  server/app.py                 dimensions, _location_levels, hours per group
  html/wp-creation-index.html   three selects where the text box was
  html/wp-creation-app.js       the pickers, the filters, the rollup panel
  html/wp-creation-styles.css   .loc-picker, .loc-rollup
  tests/rollup_check.py         new - 63 checks

Done when — CR-004
  [x] all three render as dropdowns populated from project configuration
  [x] dependent filtering works, and clearing a parent clears its children
  [x] values persist as codes; confirmed by reading what collectPackage stored
  [x] the dashboard filters by each of the three
  [x] a work package referencing a deactivated value still renders correctly
  [x] all option data comes from the server - proved by poisoning the cache

Done when — CR-018
  [x] the dashboard groups and totals by Building, Floor and Sector
  [x] totals reconcile against an unfiltered count, at every level
  [x] Actual Hours rolls up along the same dimensions
  [x] grouping is computed server-side - proved by putting nine fake packages in
      localStorage and requiring the panel to show none of them
  [x] work packages with no location appear in an explicit unassigned group

No migration: location lives in the work package's JSON data blob like every
other per-package field. No colour literal added.

Verified one at a time
  rollup_check      63/63  new
  generalinfo_check 49/49
  browser_check     71/71
  pipeline          43/43
  a11y              22/22
  aggregates        16/16
  f_items           F1-F5 FIXED, F6 REPRODUCES (T7.2)

Question for the PR, per CLAUDE.md: the dashboard's location filters and the
rollup both key on the path, so a package saved with free text and no codes is
unreachable by any location filter and sits in its own building-level row. That
is correct and it is also a migration question - whether the existing free-text
locations should be mapped onto the taxonomy once the B100 list arrives, or left
as history. Nothing here decides it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Nick Siegfried
2026-08-17 00:24:00 -05:00
parent ce2d008897
commit 4d186ea6a0
5 changed files with 871 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1131,6 +1131,10 @@ function collectPackage(){
number:gv('wp_number'), status:getRadio('status')||'Draft', subject:gv('wp_subject'),
type:gv('wp_type'), system:gv('wp_system'), location:gv('wp_location'),
p6Id:gv('wp_p6_id'), p6Desc:gv('wp_p6_desc'), // CR-001
// CR-004: codes (paths), not display strings. `location` keeps whatever
// free text the package already had — retained, never overwritten.
building:gv('wp_building'), floor:gv('wp_floor'), sector:gv('wp_sector'),
priority:wpPriorityOf({priority: gv('wp_priority')}), // CR-003
@@ -1235,7 +1239,7 @@ function renderPackage(pkg){
<tr><th>Type</th><td>${cell(pkg.type)}</td></tr>
${pkg.disciplines&&pkg.disciplines.length?`<tr><th>Discipline(s)</th><td>${esc(pkg.disciplines.join(', '))}${pkg.split?' <span style="color:var(--accent);font-size:10px">[MASTER — split into instances]</span>':''}${pkg.instanceOf?` <span style="color:var(--accent);font-size:10px">[instance of ${esc(pkg.parentNumber||'')}]</span>`:''}</td></tr>`:''}
<tr><th>System / Facility Code / UPN</th><td>${cell(pkg.system)}</td></tr>
${sectionOn('location')?`<tr><th>Location</th><td>${cell(pkg.location)}</td></tr>`:''}
${sectionOn('location')?`<tr><th>Location</th><td>${cell(wpLocationText(pkg))}</td></tr>`:''}
${fieldOn('costCode')?`<tr><th>Cost Code</th><td>${pkg.cost?esc(pkg.cost)+(costDesc?' — '+esc(costDesc):''):ns()}</td></tr>`:''}
${fieldOn('acumaticaTask')?`<tr><th>Acumatica Task</th><td>${cell(pkg.wbs)}</td></tr>`:''}
<tr><th>Assignees</th><td>${cell(pkg.assignees)}</td></tr>
@@ -1443,6 +1447,142 @@ function updateStickyStatus(){
else { el.className='sticky-status ss-notready'; el.textContent=`⚠ ${r.open} of ${r.total} constraint${r.open===1?'':'s'} open`+(r.blocking.length?` · ${r.blocking.length} predecessor${r.blocking.length===1?'':'s'}`:''); }
}
// ── LOCATION (CR-004 / T6.3) ─────────────────────────────────────────────────
// Three dependent dropdowns off the project's own taxonomy (CR-005 / T5.4). The
// values stored are PATHS — 'B-ONE/L1/S-A' — not display strings, because CR-018
// rolls cost up by them and a rollup keyed on a label breaks the day somebody
// fixes a typo in it. A floor's own code is not unique across buildings; its path
// is.
//
// Loaded with include_inactive=true, which is not a contradiction of CR-005's
// "deactivating hides it from new work packages". Two different questions:
//
// what may be CHOSEN active only — the option lists below filter on it
// what may be SHOWN everything, because a package already referencing a
// deactivated value still has to render its label
//
// Nothing here reads localStorage. X5 is explicit: the option lists cannot come
// from the browser's own copy.
let wpLocations = [];
let wpLocationsLoaded = false;
const LOC_LEVELS = ['building', 'floor', 'sector'];
const LOC_FIELD = {building: 'wp_building', floor: 'wp_floor', sector: 'wp_sector'};
function locNode(path){
return wpLocations.find(n => n.path === path) || null;
}
function locLabel(path){
const n = locNode(path);
return n ? n.name : (path || '');
}
// The label as it should READ on a package — a value that has since been
// deactivated says so, rather than looking like any other choice.
function locLabelFull(path){
const n = locNode(path);
if(!n) return path || '';
return n.active ? n.name : n.name + ' (no longer offered)';
}
async function loadLocations(){
if(!activeProjectId){ wpLocations = []; wpLocationsLoaded = true; buildLocationPickers(); return; }
try {
const r = await fetch('/api/projects/' + encodeURIComponent(activeProjectId)
+ '/locations?include_inactive=true',
{credentials:'same-origin', headers:{'Accept':'application/json'}});
if(!r.ok) throw new Error('HTTP ' + r.status);
const data = await r.json();
wpLocations = data.nodes || [];
} catch(e){
// No cache fallback, deliberately: an option list remembered from last time
// can offer a value this project no longer has, and B4's whole objection is
// to a per-browser answer that looks authoritative.
wpLocations = [];
}
wpLocationsLoaded = true;
buildLocationPickers();
}
/* Fill one level's <select>, filtered to the children of `parent`.
`keep` is the value already on the package. It is offered even when it is
inactive — otherwise opening an old package would silently blank its location
and the next save would write that blank back. */
function fillLocSelect(level, parent, keep){
const sel = document.getElementById(LOC_FIELD[level]);
if(!sel) return;
const wanted = LOC_LEVELS.indexOf(level);
const options = wpLocations.filter(n =>
n.level === level
&& (wanted === 0 ? !n.parent_id : (n.path.indexOf(parent + '/') === 0
&& n.path.split('/').length === wanted + 1))
&& (n.active || n.path === keep));
const placeholder = wanted === 0 ? 'Building…' : (level === 'floor' ? 'Floor…' : 'Sector…');
sel.innerHTML = `<option value="">${esc(placeholder)}</option>`
+ options.map(n => `<option value="${esc(n.path)}"${n.path === keep ? ' selected' : ''}>`
+ `${esc(locLabelFull(n.path))}</option>`).join('');
// A level with nothing under it is disabled rather than an empty dropdown that
// looks broken — and a level whose parent is unset is disabled for the same
// reason: there is nothing it could sensibly offer yet.
const noParent = wanted > 0 && !parent;
sel.disabled = noParent || (!options.length && !keep);
sel.title = noParent
? 'Choose a ' + LOC_LEVELS[wanted - 1] + ' first'
: (!options.length ? 'This project has no ' + level + ' values configured here' : '');
if(keep) sel.value = keep;
}
function buildLocationPickers(keepValues){
const cur = keepValues || {
building: (document.getElementById('wp_building') || {}).value || '',
floor: (document.getElementById('wp_floor') || {}).value || '',
sector: (document.getElementById('wp_sector') || {}).value || '',
};
fillLocSelect('building', '', cur.building);
fillLocSelect('floor', cur.building, cur.building ? cur.floor : '');
fillLocSelect('sector', cur.floor, cur.floor ? cur.sector : '');
renderLocationNote();
}
// Clearing a parent clears its children. Not doing this is how a package ends up
// filed under a floor that is not in the building it claims.
function onLocationChange(level){
const b = (document.getElementById('wp_building') || {}).value || '';
const f = (document.getElementById('wp_floor') || {}).value || '';
const s = (document.getElementById('wp_sector') || {}).value || '';
if(level === 'building') buildLocationPickers({building: b, floor: '', sector: ''});
else if(level === 'floor') buildLocationPickers({building: b, floor: f, sector: ''});
else buildLocationPickers({building: b, floor: f, sector: s});
}
function renderLocationNote(){
const note = document.getElementById('wp_location_note');
if(!note) return;
const legacy = (document.getElementById('wp_location') || {}).value || '';
if(!wpLocationsLoaded){ note.textContent = 'Loading this project\'s location list…'; return; }
if(!wpLocations.length){
note.textContent = 'This project has no location list yet — configure it on step 11 of the SOP.';
return;
}
// A package written before CR-004 has free text and no codes. It is SHOWN
// rather than dropped: the data is retained, which is the rule everywhere else
// in this plan too.
note.textContent = legacy && !(document.getElementById('wp_building') || {}).value
? 'Previously recorded as free text: “' + legacy + '”. Pick the structured values to replace it.'
: '';
}
function wpLocationOf(p){
return {building: p.building || '', floor: p.floor || '', sector: p.sector || ''};
}
// One readable line from whatever a package holds — structured if it has it,
// the old free text if that is all there is.
function wpLocationText(p){
const parts = LOC_LEVELS.map(l => p[l]).filter(Boolean).map(locLabelFull);
if(parts.length) return parts.join(' / ');
return p.location || '';
}
// ── SECTION TOGGLES (CR-006 / T5.5) ──────────────────────────────────────────
// Which sections this project uses. The list is wp-sections.js, shared with the
// SOP wizard; this file owns the mapping from a section id to the DOM it governs
@@ -1843,6 +1983,7 @@ function loadPackageIntoForm(p){
const set=(id,v)=>{const el=document.getElementById(id); if(el) el.value=v||'';};
set('wp_subject',p.subject); set('wp_system',p.system); set('wp_location',p.location);
set('wp_p6_id',p.p6Id); set('wp_p6_desc',p.p6Desc); // CR-001
buildLocationPickers(wpLocationOf(p)); // CR-004
set('wp_priority', wpPriorityOf(p)); // CR-003
set('wp_assignees',p.assignees); set('wp_distribution',p.distribution);
loadPeopleFromPkg(p);
@@ -1932,6 +2073,7 @@ function newPackage(){
['wp_subject','wp_system','wp_location','wp_p6_id','wp_p6_desc','wp_wbs','wp_assignee','wp_assignees','wp_distribution','wp_due','wp_spec','wp_desc','wp_hours','wp_kit_owner','wp_kit_date','wp_mimo_time','wp_mimo_loc','wp_actual_hrs','wp_installed_qty','wp_redlines','wp_lessons','wp_bimlink','wp_model_area','wp_scan_link'].forEach(id=>{const el=document.getElementById(id); if(el) el.value='';});
document.getElementById('wp_type').value=''; document.getElementById('wp_kit_status').value=''; document.getElementById('wp_cost').value='';
const prio=document.getElementById('wp_priority'); if(prio) prio.value=WP_PRIORITY_DEFAULT; // CR-003
buildLocationPickers({building:'', floor:'', sector:''}); // CR-004
['wp_iff','wp_clash'].forEach(id=>{const el=document.getElementById(id); if(el) el.value='';});
onClashChange();
pkgKind='iwp'; applyKind();
@@ -1973,7 +2115,7 @@ const WPData = {
if(typeof ProjectData!=='undefined' && ProjectData.pushWP) ProjectData.pushWP(p, activeProjectId); return true; },
};
let dashFilter={status:'',discipline:'',q:'',flag:'',priority:''};
let dashFilter={status:'',discipline:'',q:'',flag:'',priority:'',building:'',floor:'',sector:''};
// 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
@@ -1995,7 +2137,7 @@ function dashRefreshAfterWrite(){
const DASH_FLAGS = ['ready', 'onhold', 'overdue', 'mine'];
function dashToggleFlag(f, opts){
if(f==='all'){ dashFilter={status:'',discipline:'',q:'',flag:'',priority:''}; }
if(f==='all'){ dashFilter={status:'',discipline:'',q:'',flag:'',priority:'',building:'',floor:'',sector:''}; }
else { dashFilter.flag = dashFilter.flag===f ? '' : f; }
dashPage=0;
// S3: which slice of the board you are looking at is state, so it belongs in
@@ -2090,6 +2232,93 @@ function dashSortRows(rows){
});
}
// CR-004: one filter per level, dependent the same way the form's are — picking
// a building narrows the floors offered. Rendered only when the project has a
// taxonomy: three empty dropdowns on a project that has not configured one are
// three controls that cannot do anything.
function locFilterSelects(){
if(!wpLocations.length) return '';
const out = [];
LOC_LEVELS.forEach((level, i) => {
const parent = i === 0 ? '' : dashFilter[LOC_LEVELS[i - 1]];
if(i > 0 && !parent) return; // nothing to narrow to yet
const opts = wpLocations.filter(n =>
n.level === level
&& (i === 0 ? !n.parent_id : (n.path.indexOf(parent + '/') === 0
&& n.path.split('/').length === i + 1)));
if(!opts.length) return;
const label = level.charAt(0).toUpperCase() + level.slice(1);
out.push(`<select aria-label="Filter by ${esc(label.toLowerCase())}"`
+ ` onchange="dashSetLocationFilter('${level}', this.value)">`
+ `<option value="">All ${esc(level)}s</option>`
+ opts.map(n => `<option value="${esc(n.path)}"${dashFilter[level] === n.path ? ' selected' : ''}>`
+ `${esc(locLabelFull(n.path))}</option>`).join('')
+ `</select>`);
});
return out.join('');
}
// Clearing a parent clears its children here too, for the same reason as on the
// form: a floor filter under a different building filters to nothing and looks
// like an empty project.
function dashSetLocationFilter(level, value){
const i = LOC_LEVELS.indexOf(level);
dashFilter[level] = value;
for(let k = i + 1; k < LOC_LEVELS.length; k++) dashFilter[LOC_LEVELS[k]] = '';
dashPage = 0;
renderDashboard();
}
// ── CR-018 / T6.4: rollup by building, floor and sector ─────────────────────
// This is why the Acumatica cost code was removed rather than relabelled: the
// tracking dimension the team wants is floor and area, not an accounting code.
//
// Every number here is the SERVER's. The browser does not sum the leaf groups to
// get a floor total — that would be the same per-browser arithmetic B4 removed,
// and it would silently disagree with the header count the moment this browser's
// copy is behind. server/app.py::_location_levels does it once, for everyone.
function renderLocationRollup(m){
const loc = m && m.by_location;
if(!loc || !loc.levels) return '';
const unset = loc.unassigned_key || '(unassigned)';
const levels = (loc.dimensions || []).filter(d => (loc.levels[d] || []).length);
if(!levels.length) return '';
// Each row's `path` is a full location path — a floor is `B-ONE/L1` — so it
// resolves straight to a name. A group whose value at this level is unassigned
// is SHOWN, not hidden: it is the row that makes the totals add up, and hiding
// it is how a rollup ends up quietly missing work.
const label = path => path === unset ? 'Unassigned' : locLabelFull(path);
let h = `<div class="dash-panel"><div class="dash-panel-title">By location</div>`;
h += `<div class="field-hint">Totals come from the server and are computed at every level, `
+ `so each table adds up to the project on its own. Packages with no location assigned `
+ `appear as <strong>Unassigned</strong> rather than being left out.</div>`;
levels.forEach(dim => {
const rows = loc.levels[dim] || [];
const sum = k => rows.reduce((a, r) => a + (r[k] || 0), 0);
h += `<div class="loc-rollup"><div class="dash-bd-title">By ${esc(dim)}</div>`
+ `<table class="dash-table"><thead><tr>`
+ `<th>${esc(dim.charAt(0).toUpperCase() + dim.slice(1))}</th>`
+ `<th>WPs</th><th>Ready</th><th>On hold</th><th>Overdue</th>`
+ `<th>Est. hrs</th><th>Actual hrs</th></tr></thead><tbody>`;
rows.forEach(r => {
const isUnset = r.path === unset;
h += `<tr${isUnset ? ' class="loc-unassigned"' : ''}>`
+ `<td class="row-label">${esc(label(r.path))}</td>`
+ `<td>${r.total}</td><td>${r.release_ready}</td><td>${r.on_hold}</td>`
+ `<td>${r.overdue}</td><td>${r.est_hours}</td><td>${r.actual_hours}</td></tr>`;
});
// The total row is what makes "reconciles against an unfiltered count"
// checkable by looking rather than by trusting.
h += `<tr class="loc-total"><td class="row-label">All ${esc(dim)}s</td>`
+ `<td>${sum('total')}</td><td>${sum('release_ready')}</td><td>${sum('on_hold')}</td>`
+ `<td>${sum('overdue')}</td><td>${sum('est_hours')}</td><td>${sum('actual_hours')}</td></tr>`;
h += `</tbody></table></div>`;
});
return h + `</div>`;
}
function dashHeaderCells(){
return DASH_COLUMNS.map(c => {
if(c.sortable === false) return `<th>${esc(c.label)}</th>`;
@@ -2270,6 +2499,7 @@ function renderDashboard(){
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;
h+=renderLocationRollup(m);
// gating panel — what's blocking release, from the server
const gated=m.gating||[];
@@ -2290,6 +2520,7 @@ function renderDashboard(){
<select onchange="dashFilter.status=this.value;dashPage=0;renderDashboard()">${statusOpts}</select>
<select onchange="dashFilter.discipline=this.value;dashPage=0;renderDashboard()">${discOpts}</select>
<select aria-label="Filter by priority" onchange="dashFilter.priority=this.value;dashPage=0;renderDashboard()">${prioOpts}</select>
${locFilterSelects()}
<label class="dash-arch-toggle"><input type="checkbox" ${dashShowArchived?'checked':''} onchange="dashToggleArchived(this.checked)"> Show archived${dashShowArchived?' ('+dashArchived.length+')':''}</label>
</div>`;
@@ -2300,6 +2531,13 @@ function renderDashboard(){
if(dashFilter.status && p.status!==dashFilter.status) return false;
if(dashFilter.discipline && !((p.disciplines||[]).includes(dashFilter.discipline))) return false;
if(dashFilter.priority && wpPriorityOf(p)!==dashFilter.priority) return false;
// CR-004: filtering by a BUILDING matches every package under it, because the
// stored value is a path. Filtering by the floor code alone could not do that.
if(dashFilter.building && !((p.building||'')===dashFilter.building
|| (p.floor||'').indexOf(dashFilter.building+'/')===0)) return false;
if(dashFilter.floor && !((p.floor||'')===dashFilter.floor
|| (p.sector||'').indexOf(dashFilter.floor+'/')===0)) return false;
if(dashFilter.sector && (p.sector||'')!==dashFilter.sector) return false;
if(q && !((p.number||'')+' '+(p.subject||'')+' '+(p.type||'')).toLowerCase().includes(q)) return false;
if(dashFilter.flag==='mine' && p.assigneeId!==myUserId()) return false;
if(dashFilter.flag==='ready' && !(!p.split && !wpReleaseBlocked(p) && p.status!=='Closed' && p.status!=='Issue')) return false;
@@ -2606,11 +2844,19 @@ function defaultOwnerToMe(){
if(me && Array.from(sel.options).some(o=>o.value===me)) sel.value=me;
}
document.addEventListener('DOMContentLoaded', function(){
LOC_LEVELS.forEach(level => {
const el = document.getElementById(LOC_FIELD[level]);
if(el) el.addEventListener('change', () => onLocationChange(level));
});
});
function bootData(){
loadStore(); // reads the localStorage cache (hydrated from the server below)
bootSOP();
setRadio('status','Draft');
loadMembers();
loadLocations(); // CR-004: the option lists come from the server
initWpNavDrawer();
renderSavedList();
positionSectionNav();