fix(web): verify the onboarding timezone step, don't compare it to the default

The Getting Started card's timezone step ticked when the saved timezone differed from the value config.template.json ships (America/New_York), OR-ed with the saved city differing from Tampa. Both halves were wrong. "Differs from the default" answers "did somebody edit this?", but what the checklist needs to know is whether the value is right. A user genuinely in America/New_York could never satisfy it, so the card nagged forever with four of five steps done -- the case that prompted this, on a panel whose timezone was correct all along. The city half was worse than useless: the saved city says nothing about whether the timezone is set, and because the two were OR-ed, saving a city ticked the step off with the timezone still wrong. That is the direction that actually breaks displays, since event times then render in the wrong zone. The browser already knows its own zone, so compare against that. No new persisted state, no network, and it catches the reverse case the old test got backwards: a panel still set to the old zone after a move now stays unticked, where before it ticked the moment the value stopped being the default. Zones are compared by the wall-clock time they produce for one instant rather than by identifier, so aliases (Asia/Calcutta vs Asia/Kolkata, Europe/Kiev vs Europe/Kyiv) don't read as a mismatch. When they genuinely differ the step names the browser's zone, so an unticked box says why. Configs with no timezone, an unparseable zone, or a browser without Intl leave the step open for the existing manual tick. The step still deep-links to the General tab, and the location value stays visible in its label -- it just no longer votes on whether the timezone is configured. Tests render the partial across configured zones and both cities: the step never pre-ticks server-side, carries the configured zone for the client to check, is unmoved by the city, and the panel-size step still resolves server-side. Reverting the template fails 9 of the 11. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
2026-08-17 16:48:06 +00:00 · 2026-08-17 11:36:08 -04:00
parent 08265c1135
commit 2f64dbc48c
2 changed files with 217 additions and 6 deletions
@@ -0,0 +1,152 @@
+"""
+Getting Started checklist: what the server decides, and what it must not.
+
+The timezone step used to tick server-side when the saved timezone differed
+from the shipped default, OR-ed with the saved city. That made the step
+unsatisfiable for anyone genuinely in the default zone (the card nagged
+forever), and let a saved city tick it off while the timezone was still wrong.
+The step is now verified in the browser against its own zone, so the server's
+only job is to hand over the configured value and stay out of the decision.
+
+These tests pin that contract: the panel-size step still reflects config, the
+timezone step never pre-ticks, it carries the configured zone, and the city
+has no influence on it.
+"""
+
+import copy
+import re
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+from flask import Flask
+
+PROJECT_ROOT = Path(__file__).parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+BASE_CONFIG = {
+    "timezone": "America/New_York",
+    "location": {"city": "Tampa", "state": "Florida", "country": "US"},
+    "display": {
+        "hardware": {"rows": 32, "cols": 64, "chain_length": 2, "parallel": 1},
+        "runtime": {},
+        "double_sided": {"enabled": False},
+        "vegas_scroll": {"plugin_order": [], "excluded_plugins": []},
+        "plugin_rotation_order": [],
+    },
+    "plugin_system": {},
+    "schedule": {},
+    "dim_schedule": {},
+    "sync": {},
+}
+
+
+def render(config):
+    """Render the overview partial against one config, as app.py would."""
+    base = PROJECT_ROOT / "web_interface"
+    app = Flask(
+        __name__,
+        template_folder=str(base / "templates"),
+        static_folder=str(base / "static"),
+    )
+    app.config["TESTING"] = True
+
+    from web_interface.blueprints import pages_v3 as pv
+
+    # pages_v3 is a module-level singleton shared across the test process;
+    # restore whatever the previous test left on it.
+    original_cm = getattr(pv.pages_v3, "config_manager", None)
+    original_pm = getattr(pv.pages_v3, "plugin_manager", None)
+
+    mock_cm = MagicMock()
+    mock_cm.load_config.return_value = config
+    mock_cm.get_raw_file_content.return_value = config
+    pv.pages_v3.config_manager = mock_cm
+
+    mock_pm = MagicMock()
+    mock_pm.plugins = {}
+    mock_pm.get_all_plugin_info.return_value = []
+    mock_pm.get_plugin_display_modes.side_effect = lambda pid: []
+    pv.pages_v3.plugin_manager = mock_pm
+
+    app.register_blueprint(pv.pages_v3, url_prefix="")
+    try:
+        resp = app.test_client().get("/partials/overview")
+        assert resp.status_code == 200, resp.status_code
+        return resp.get_data(as_text=True)
+    finally:
+        pv.pages_v3.config_manager = original_cm
+        pv.pages_v3.plugin_manager = original_pm
+
+
+def timezone_step(body):
+    """The checklist <button> for the timezone step."""
+    match = re.search(r"<button[^>]*data-check=\"timezone\"[^>]*>", body)
+    assert match, "timezone step not found in the rendered checklist"
+    return match.group(0)
+
+
+def config_with(**overrides):
+    config = copy.deepcopy(BASE_CONFIG)
+    for key, value in overrides.items():
+        config[key] = value
+    return config
+
+
+@pytest.mark.parametrize(
+    "timezone",
+    ["America/New_York", "America/Los_Angeles", "Europe/Madrid", "Asia/Kolkata"],
+)
+def test_timezone_step_never_pre_ticks_server_side(timezone):
+    """The browser owns this decision; the server must not pre-empt it.
+
+    The default zone is in the list deliberately: that is the case the old
+    default-comparison could never tick.
+    """
+    step = timezone_step(render(config_with(timezone=timezone)))
+    assert 'data-done="0"' in step, step
+
+
+@pytest.mark.parametrize(
+    "timezone",
+    ["America/New_York", "Europe/Madrid", "Pacific/Auckland"],
+)
+def test_timezone_step_carries_the_configured_zone(timezone):
+    """JS compares data-tz against the browser, so it has to be the real value."""
+    assert f'data-tz="{timezone}"' in timezone_step(render(config_with(timezone=timezone)))
+
+
+def test_city_does_not_influence_the_timezone_step():
+    """The coupling this change removes: city said nothing about the timezone,
+    and OR-ing it let a saved city tick the step off with the zone still wrong."""
+    tampa = timezone_step(render(config_with(
+        location={"city": "Tampa", "state": "Florida", "country": "US"})))
+    seattle = timezone_step(render(config_with(
+        location={"city": "Seattle", "state": "Washington", "country": "US"})))
+    assert tampa == seattle
+
+
+def test_missing_timezone_leaves_the_step_open():
+    """Nothing saved means nothing to verify: the step stays unticked and the
+    JS bails on the empty value rather than comparing against ''."""
+    step = timezone_step(render(config_with(timezone="")))
+    assert 'data-tz=""' in step
+    assert 'data-done="0"' in step
+
+
+@pytest.mark.parametrize(
+    "hardware,expected",
+    [
+        ({"rows": 32, "cols": 64, "chain_length": 2, "parallel": 1}, "1"),
+        ({"rows": 0, "cols": 0, "chain_length": 0, "parallel": 1}, "0"),
+    ],
+)
+def test_panel_size_step_still_reflects_config(hardware, expected):
+    """Regression guard: the hardware step is still decided server-side."""
+    config = config_with()
+    config["display"]["hardware"] = hardware
+    body = render(config)
+    match = re.search(r"<button[^>]*data-tab=\"display\"[^>]*>", body)
+    assert match, "panel-size step not found"
+    assert f'data-done="{expected}"' in match.group(0), match.group(0)
@@ -63,13 +63,13 @@

 <!-- Getting Started checklist: non-gating, dismissible (localStorage), items
     auto-check from existing config/endpoints — no new persisted state.
-     Known heuristic limits (acceptable, disclosed): values left at legitimate
-     defaults (e.g. a user actually in Tampa) read as "not done". -->
+     The timezone step is verified against the browser's own zone rather than
+     compared to the shipped default; see the data-check="timezone" block below
+     for why. -->
 {% set _hw = main_config.display.hardware if main_config and main_config.display else {} %}
 {% set _hw_done = (_hw.rows or 0) > 0 and (_hw.cols or 0) > 0 and (_hw.chain_length or 0) > 0 %}
 {% set _loc = main_config.location if main_config and main_config.location else {} %}
-{% set _loc_done = (main_config.timezone and main_config.timezone != 'America/New_York')
-                   or (_loc.city and _loc.city != 'Tampa') %}
+{% set _tz = (main_config.timezone if main_config else '') or '' %}
 <div id="getting-started-card" class="bg-blue-50 border border-blue-200 rounded-lg p-4 mb-4" style="display:none" role="region" aria-label="Getting started checklist">
    <div class="flex items-start justify-between">
        <div class="flex-1">
@@ -78,8 +78,8 @@
            <ul class="space-y-1 text-sm" id="getting-started-items">
                <li><button type="button" class="gs-item text-left w-full" data-done="{{ '1' if _hw_done else '0' }}" data-tab="display">
                    <i class="far fa-square mr-2"></i>Set your panel size (Display tab)</button></li>
-                <li><button type="button" class="gs-item text-left w-full" data-done="{{ '1' if _loc_done else '0' }}" data-tab="general">
-                    <i class="far fa-square mr-2"></i>Set your timezone and location (General tab)</button></li>
+                <li><button type="button" class="gs-item text-left w-full" data-done="0" data-check="timezone" data-tz="{{ _tz }}" data-tab="general">
+                    <i class="far fa-square mr-2"></i>Set your timezone{% if _tz %} — currently {{ _tz }}{% if _loc.city %}, {{ _loc.city }}{% endif %}{% endif %} (General tab)<span data-gs-tz-note class="text-xs"></span></button></li>
                <li><button type="button" class="gs-item text-left w-full" data-done="0" data-check="installed" data-tab="plugins">
                    <i class="far fa-square mr-2"></i>Install a plugin from the Plugin Store</button></li>
                <li><button type="button" class="gs-item text-left w-full" data-done="0" data-check="enabled" data-tab="plugins">
@@ -165,6 +165,65 @@
    });
    maybeAutoHide();

+    // Timezone: verified against the browser's own zone.
+    //
+    // This step used to tick when the saved timezone differed from the value
+    // config.template.json ships (America/New_York), with the saved city
+    // OR-ed in. Two things were wrong with that. "Differs from the default"
+    // answers "did somebody edit this?", but what the checklist needs to know
+    // is whether the value is RIGHT — so anyone who genuinely lives in the
+    // default zone could never satisfy it and the card nagged forever. And
+    // the city has no bearing on whether the timezone is set: because the two
+    // were OR-ed, saving a city ticked the step off with the timezone still
+    // wrong, which is the direction that actually breaks displays (event
+    // times render in the wrong zone).
+    //
+    // The browser already knows its zone, so compare against that: no new
+    // persisted state, no network, and it catches the reverse case too — a
+    // panel still set to the old zone after a move now stays unticked, where
+    // the old test ticked it the moment the value stopped being the default.
+    function sameZone(a, b) {
+        if (a === b) return true;
+        // Compare the wall-clock time each zone yields for one instant, not
+        // the identifiers: aliases (Asia/Calcutta vs Asia/Kolkata,
+        // Europe/Kiev vs Europe/Kyiv) name one zone and must not read as a
+        // mismatch.
+        try {
+            var now = new Date();
+            var stamp = function (tz) {
+                return new Intl.DateTimeFormat('en-US', {
+                    timeZone: tz, dateStyle: 'short', timeStyle: 'short'
+                }).format(now);
+            };
+            return stamp(a) === stamp(b);
+        } catch (e) {
+            // An unparseable zone in the config is worth surfacing, not hiding.
+            return false;
+        }
+    }
+
+    (function () {
+        var tzBtn = card.querySelector('[data-check="timezone"]');
+        if (!tzBtn) return;
+        var configured = tzBtn.dataset.tz || '';
+        if (!configured) return;          // nothing saved yet: leave it open
+        var local = '';
+        try {
+            local = (Intl.DateTimeFormat().resolvedOptions().timeZone) || '';
+        } catch (e) {
+            return;                        // no Intl: leave it to the manual tick
+        }
+        if (!local) return;
+        if (sameZone(configured, local)) {
+            markDone(tzBtn);
+            return;
+        }
+        // Unticked on its own says "wrong" without saying why; name the zone
+        // the browser is in so the step is actionable.
+        var note = tzBtn.querySelector('[data-gs-tz-note]');
+        if (note) note.textContent = ' — this browser is in ' + local;
+    }());
+
    // Plugin-derived states from the existing installed-plugins endpoint.
    fetch('/api/v3/plugins/installed')
        .then(function (r) { return r.json(); })