Repository navigation
feat(history): add calendar month selection - #1866
bferanmi806-sketch wants to merge 5 commits into
Conversation
Add this-month, last-month, and a month picker to the shared history tiles. Aggregate each month using local calendar boundaries and keep the current month partial. Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
ffcdb6e to
20f9229
Compare
Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>
miravoss26
left a comment
There was a problem hiding this comment.
Adds calendar-month history to the four history tiles and the pie: month=YYYY-MM on /api/energy/daily, This month / Last month / month picker in the UI, with a timezone-safe month helper and tests.
- Question
go/internal/api/api.go(handleEnergyDaily): insideif month != "",loc, err := time.LoadLocation(...)is a newlocscoped to that block. The outerloc(fromnow.Location()) is what the rest of the handler sees. If code further down builds day boundaries withtime.Date(..., loc)rather than fromtodayMidnight, month requests would be bucketed in the server zone, not the box zone. I could not see the rest of the handler in this diff, so worth confirming, ideally with a test where box tz differs from process tz. - Low
daysis silently ignored whenmonthis set. Fine, but a 400 or a comment would make it explicit. - Low The PR text says it is still pending the author's visual review. Treat the UI part as unreviewed by a human.
- Security screen: no secrets, no new dependencies, no new hosts.
monthis strictly parsed (2006-01round-trip), future months rejected.
Needs a human decision on the loc shadowing question before merge; the rest looks sound from my read.
|
Hi @miravoss26, thanks for the detailed review! You're right about the "loc" shadowing. I checked the remainder of "handleEnergyDaily", and the outer "loc" isn't used after the month block. The handler rebuilds "todayMidnight" using the box timezone, and the daily loop derives its boundaries using "todayMidnight.AddDate()", which preserves that location. "calendarMonthBounds" also receives "now" after conversion to the box timezone. So the month calculations should be correct despite the shadowing. I'll make the variable scoping clearer and add a regression test with different server and box timezones to verify this explicitly. For "days", the month parameter intentionally takes precedence. I'll document that behaviour. Good catch on the PR description too. I'll update the outdated draft wording and clarify the visual review status. Thanks again for taking the time to review this! |
|
@miravoss26 Follow-up pushed in The full Linux API package run passed; its helper-only |
miravoss26
left a comment
There was a problem hiding this comment.
Adds a calendar-month selector to the history tiles; /api/energy/daily?month=YYYY-MM takes precedence over days.
- I read the Go side (handler,
calendarMonthBounds, tests). Month parsing is strict, future months are rejected, boundaries use the box timezone and the current month stays partial. DST and leap-year cases are covered, and so is the process-TZ vs box-TZ split. No security concerns: input is a parsedYYYY-MM, no new deps or hosts. - I did not review the frontend half of the diff (picker, chart scrolling) line by line.
From my read the API side is fine. A human should look at the UI behaviour, and this is an external contributor PR, so a maintainer call on scope.
|
Manual visual review completed on PR head 16a8133 using the running app with isolated synthetic history. At 1440px desktop and 390px mobile, the month picker and Last month/custom month states worked: November 2026 (current month), October 2026 (31 days), and February 2024 (29 days). All four bar cards updated, and the pie view matched the month totals. At 390px, the page stayed within the viewport; horizontal scrolling remained inside each chart tile (285px viewport, 1334px chart content). ArrowRight advanced the chart and updated the directional cue. Runtime timezone was Europe/London. No confirmed UI defects; no UI code changed. This is browser/viewport evidence, not physical touch-device verification. Desktop bars — November 2026 month selector and all four daily charts: |



Summary
Implements #1318: the four shared history tiles and pie view can show this month, last month, or a month selected with the native picker. The API uses calendar days in the box's timezone and keeps the current month partial. EV-cost endpoints are unchanged.
Month selection now waits for the box's runtime timezone and recalculates the current month at interaction time, including month boundaries. A custom selection no longer highlights This month. Month charts scroll horizontally at a readable day width, with a subtle directional cue only while days extend beyond the visible area; Week returns to the compact layout, and the chart region supports keyboard scrolling.
Validation
Final Linux web suite: 903 tests passed; focused history regressions: 11 passed. JavaScript syntax and diff whitespace checks passed.
Real local Go API with a fresh isolated data directory, seeded through the built-in backfill utility (1100 days, five-minute samples, seed 1866): verified populated import, production, consumption and export bars plus pie totals. Checked current/last month, February 2024 (29 days), partial current month, and missing-data behavior.
Desktop and 390px mobile browser review: readable chart labels, horizontal chart scrolling without page overflow, correct custom-month highlight, and populated pie. Before/after screenshots retained for the author's human review.
A pass-through proxy held September's actual API response for 2.5 seconds. Logs confirmed October arrived first; both bars and pie retained October after the stale September response arrived.
Timezone helper tests cover differing box/browser calendar months at a UTC boundary, year rollover and invalid metadata. Browser timezone emulation and physical touch scrolling were unavailable. ArrowRight scrolling worked. Five status-poll warnings coincided with restarting the test proxy; no other browser warnings/errors were captured.
The prior full Go API and
make verifypassed on the signed pre-follow-up head. The follow-up Go checks are listed below. The earlier verify run skipped its Avahi container run and release Docker-context checks because the test container had no Docker daemon.Follow-up verification: the full Linux API package test run passed; the helper-only test
TestPlannerFailureCrashHelperwas skipped. The timezone/month regression passed, and the API comment documents that month takes precedence over days. Thego vet ./internal/apicheck passed, and the full Go tree built forlinux/amd64. The Linux test binary was cross-compiled on Windows and executed in WSL.Integration and review
This PR is no longer a draft. Human visual review is complete; see the Nov 10 follow-up comment and screenshots. Actual hunk comparisons found no shared changed lines with #1859, #1862, #1863 or #1751. The closest edits are #1859's live-history rendering beside the selector, #1751's daily response fields after the month range setup, and #1546's EV card insertion after the history cake. Preserve both sets of changes when integrating and rerun the relevant checks after those PRs land.
Fork Actions runs have required upstream approval (
action_required, zero jobs/logs). The repository policy endpoint returned 403, so the exact approval rule could not be confirmed. A maintainer needs to inspect and approve the runs if required. No reviewers have been requested.Visual comparison
Populated energy history from the actual local API. Month charts now scroll horizontally to keep each day's labels readable; the mobile view no longer highlights This month when a custom month is selected.
Mobile captures use a browser viewport, not a physical phone. These images are retained for human visual review; additional viewport screenshots and results are in the Nov 10 follow-up comment.