Skip to content

Commit 07e27fb

Browse files
fix(web): stop double-marking manually overridden plan rate cells
Editable Import/Export rate cells were marked as overridden by two independent signals that both render the same turned-F glyph: the server-computed row.{import,export}_rate_adjust_type === 'manual' (baked into the cell text before it reaches renderRateCell), and renderRateCell()'s own isOverride check against the overrides array. Both fire for the same single override, stacking two identical markers - and since the Clear link is gated only on the second signal, the two can disagree, leaving a marker with no working Clear behind it until a restart. The backend already replaces rather than stacks overrides correctly (manual_select() in userinterface.py explicitly removes any existing entry for the same time slot before adding a new one, confirmed via a reporter's log showing the "Removed existing rate override..." line firing correctly) - this was purely a rendering bug, not duplicate stored state. Fix: skip the adjust-type marker specifically for 'manual' when editable, since renderRateCell already owns the marker + Clear UI for that case. Other adjust types (offset/future/user/increment/ saving) are unrelated to the clickable-override mechanism and keep their marker as before. Also hardened toggleForceDropdown() with a null guard - a stale/ missing dropdown id would otherwise throw silently, indistinguishable from a cell just not responding to clicks at all (a separate symptom reported alongside the double marker, not yet root-caused). New regression tests in test_plan_why_reason.py covering both. Fixes #4474. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 003e4d3 commit 07e27fb

2 files changed

Lines changed: 59 additions & 6 deletions

File tree

apps/predbat/tests/test_plan_why_reason.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,39 @@ def render():
504504
print("ERROR: SyntaxWarning in web_helper.py line {}: {}".format(item.lineno, item.message))
505505
failed = True
506506

507+
# --- Test 15: editable rate cells don't double-mark a manual override (batpred#4474) ---
508+
# row.{import,export}_rate_adjust_type === 'manual' and renderRateCell()'s own isOverride
509+
# check both independently render the same turned-F glyph for the same single override -
510+
# visually stacking two identical markers, and since only isOverride gates the Clear link,
511+
# the two signals can disagree and leave a marker with no working Clear behind it. The fix
512+
# skips the adjust-type marker specifically for 'manual' when editable, leaving isOverride as
513+
# the sole source of truth for both the marker and the Clear control in that case.
514+
print("Test editable rate cells don't double-render the manual-override marker")
515+
for label, adjust_var in (("import", "importAdjustType"), ("export", "exportAdjustType")):
516+
rate_field = "{}_rate_adjust_type".format(label)
517+
expected = "const {} = (editable && row.{} === 'manual') ? null : row.{};".format(adjust_var, rate_field, rate_field)
518+
if expected not in renderer_js:
519+
print("ERROR: expected {} to null out a 'manual' adjust type in editable mode (found: {})".format(adjust_var, expected in renderer_js))
520+
failed = True
521+
# The old unconditional form must be gone, not just superseded - if it's still present
522+
# anywhere the double-render bug is still reachable.
523+
old_unconditional = "const {} = row.{} ? ` ${{getAdjustSymbol(row.{})}}` : '';".format({"import": "importAdjust", "export": "exportAdjust"}[label], rate_field, rate_field)
524+
if old_unconditional in renderer_js:
525+
print("ERROR: old unconditional {} form is still present alongside the fix".format({"import": "importAdjust", "export": "exportAdjust"}[label]))
526+
failed = True
527+
528+
# --- Test 16: toggleForceDropdown doesn't throw on a stale/missing dropdown id (#4474 follow-up) ---
529+
# Lives in get_plan_css() alongside the rest of the dropdown open/close JS, not
530+
# get_plan_renderer_js() (which only covers the table-building functions).
531+
print("Test toggleForceDropdown guards against a missing dropdown element")
532+
plan_css_full = web_helper.get_plan_css()
533+
toggle_start = plan_css_full.index("function toggleForceDropdown(")
534+
toggle_end = plan_css_full.index("\n }", toggle_start)
535+
toggle_src = plan_css_full[toggle_start:toggle_end]
536+
if "if (!dropdown)" not in toggle_src:
537+
print("ERROR: expected toggleForceDropdown to null-guard document.getElementById(id) before using it")
538+
failed = True
539+
507540
if not failed:
508541
print("All plan why-reason tests passed")
509542
return failed

apps/predbat/web_helper.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5997,6 +5997,16 @@ def get_plan_css():
59975997
function toggleForceDropdown(id) {
59985998
closeDropdowns();
59995999
var dropdown = document.getElementById(id);
6000+
if (!dropdown) {
6001+
// dropdownId is assigned by a counter that increments across the whole table render
6002+
// and gets baked into the cell's onclick string; if that string is now stale relative
6003+
// to the current DOM (e.g. after a plan refresh reassigned different ids), this would
6004+
// otherwise throw here and silently abort the click with no visible effect at all -
6005+
// indistinguishable from the cell just not responding (batpred#4474 follow-up). Log
6006+
// instead of throwing so a real cause leaves a trace even without DevTools handy.
6007+
console.warn("toggleForceDropdown: no element found for id", id);
6008+
return;
6009+
}
60006010
if (dropdown.style.display === "block") {
60016011
dropdown.style.display = "none";
60026012
} else {
@@ -6493,14 +6503,22 @@ def get_plan_renderer_js():
64936503
html += `<td id=time bgcolor=#FFFFFF>${timeDisplay}</td>`;
64946504
}
64956505
6496-
// Import rate - formatted bold if in charge window, italic with symbol if estimated
6506+
// Import rate - formatted bold if in charge window, italic with symbol if estimated.
6507+
// 'manual' is excluded here when editable: renderRateCell() below already shows its
6508+
// own override marker (and the only functioning Clear control) for that case, driven
6509+
// by a separately-computed isOverride check. Baking this marker in too stacks a
6510+
// second, visually identical glyph from a source the Clear button doesn't know about
6511+
// - if the two ever disagree, you get a marker with no working Clear behind it
6512+
// (batpred#4474). Non-'manual' adjust types (offset/future/user/increment/saving)
6513+
// aren't part of that clickable-override mechanism, so they keep their marker as-is.
64976514
const importBold = row.state && (row.state === 'Chrg' || row.state === 'HoldChrg' || row.state === 'FrzChrg');
64986515
let importText = row.import_rate.toFixed(2);
64996516
if (showDebug && row.import_rate_adjusted !== undefined) {
65006517
importText += ` (${row.import_rate_adjusted.toFixed(2)})`;
65016518
}
6502-
const importAdjust = row.import_rate_adjust_type ? ` ${getAdjustSymbol(row.import_rate_adjust_type)}` : '';
6503-
if (row.import_rate_adjust_type) {
6519+
const importAdjustType = (editable && row.import_rate_adjust_type === 'manual') ? null : row.import_rate_adjust_type;
6520+
const importAdjust = importAdjustType ? ` ${getAdjustSymbol(importAdjustType)}` : '';
6521+
if (importAdjustType) {
65046522
importText = `<i>${importText}${importAdjust}</i>`;
65056523
}
65066524
if (importBold) {
@@ -6512,13 +6530,15 @@ def get_plan_renderer_js():
65126530
html += `<td id=import ${cellStyle} bgcolor=${row.rate_color_import || '#FFFFFF'}>${importText}</td>`;
65136531
}
65146532
6515-
// Export rate - italic with symbol if estimated
6533+
// Export rate - italic with symbol if estimated (see import rate comment above for
6534+
// why 'manual' is excluded in editable mode)
65166535
let exportText = row.export_rate.toFixed(2);
65176536
if (showDebug && row.export_rate_adjusted !== undefined) {
65186537
exportText += ` (${row.export_rate_adjusted.toFixed(2)})`;
65196538
}
6520-
const exportAdjust = row.export_rate_adjust_type ? ` ${getAdjustSymbol(row.export_rate_adjust_type)}` : '';
6521-
if (row.export_rate_adjust_type) {
6539+
const exportAdjustType = (editable && row.export_rate_adjust_type === 'manual') ? null : row.export_rate_adjust_type;
6540+
const exportAdjust = exportAdjustType ? ` ${getAdjustSymbol(exportAdjustType)}` : '';
6541+
if (exportAdjustType) {
65226542
exportText = `<i>${exportText}${exportAdjust}</i>`;
65236543
}
65246544
if (editable) {

0 commit comments

Comments
 (0)