From 4d186ea6a0e828d8b6a0cf2ef92049fbb3278c9f Mon Sep 17 00:00:00 2001 From: "n.siegfried" Date: Mon, 17 Aug 2026 00:24:00 -0500 Subject: [PATCH] T6.3/T6.4 - CR-004 and CR-018: picked not typed, and totals that add up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- html/wp-creation-app.js | 252 +++++++++++++++++- html/wp-creation-index.html | 16 +- html/wp-creation-styles.css | 15 ++ server/app.py | 93 ++++++- tests/rollup_check.py | 508 ++++++++++++++++++++++++++++++++++++ 5 files changed, 871 insertions(+), 13 deletions(-) create mode 100644 tests/rollup_check.py diff --git a/html/wp-creation-app.js b/html/wp-creation-app.js index 55e1aa1..83d6b94 100644 --- a/html/wp-creation-app.js +++ b/html/wp-creation-app.js @@ -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){ Type${cell(pkg.type)} ${pkg.disciplines&&pkg.disciplines.length?`Discipline(s)${esc(pkg.disciplines.join(', '))}${pkg.split?' [MASTER — split into instances]':''}${pkg.instanceOf?` [instance of ${esc(pkg.parentNumber||'')}]`:''}`:''} System / Facility Code / UPN${cell(pkg.system)} - ${sectionOn('location')?`Location${cell(pkg.location)}`:''} + ${sectionOn('location')?`Location${cell(wpLocationText(pkg))}`:''} ${fieldOn('costCode')?`Cost Code${pkg.cost?esc(pkg.cost)+(costDesc?' — '+esc(costDesc):''):ns()}`:''} ${fieldOn('acumaticaTask')?`Acumatica Task${cell(pkg.wbs)}`:''} Assignees${cell(pkg.assignees)} @@ -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 ` + + `` + + opts.map(n => ``).join('') + + ``); + }); + 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 = `
By location
`; + h += `
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 Unassigned rather than being left out.
`; + levels.forEach(dim => { + const rows = loc.levels[dim] || []; + const sum = k => rows.reduce((a, r) => a + (r[k] || 0), 0); + h += `
By ${esc(dim)}
` + + `` + + `` + + `` + + ``; + rows.forEach(r => { + const isUnset = r.path === unset; + h += `` + + `` + + `` + + ``; + }); + // The total row is what makes "reconciles against an unfiltered count" + // checkable by looking rather than by trusting. + h += `` + + `` + + ``; + h += `
${esc(dim.charAt(0).toUpperCase() + dim.slice(1))}WPsReadyOn holdOverdueEst. hrsActual hrs
${esc(label(r.path))}${r.total}${r.release_ready}${r.on_hold}${r.overdue}${r.est_hours}${r.actual_hours}
All ${esc(dim)}s${sum('total')}${sum('release_ready')}${sum('on_hold')}${sum('overdue')}${sum('est_hours')}${sum('actual_hours')}
`; + }); + return h + `
`; +} + function dashHeaderCells(){ return DASH_COLUMNS.map(c => { if(c.sortable === false) return `${esc(c.label)}`; @@ -2270,6 +2499,7 @@ function renderDashboard(){ 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; + h+=renderLocationRollup(m); // gating panel — what's blocking release, from the server const gated=m.gating||[]; @@ -2290,6 +2520,7 @@ function renderDashboard(){ + ${locFilterSelects()} `; @@ -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(); diff --git a/html/wp-creation-index.html b/html/wp-creation-index.html index 2e6dee3..ab1f3a9 100644 --- a/html/wp-creation-index.html +++ b/html/wp-creation-index.html @@ -164,7 +164,21 @@ -
+ +
+ +
+ + + +
+
+ +