fix(overview): wire activity heatmap to real activity_events aggregate#68
Merged
Conversation
The Overview dashboard's activityHeatmap slice was a static
MOCK_HEATMAP constant carried over from phase 0 — phase 3 shipped
activity_events but never wired the heatmap to it. Replace the mock
with a real query.
- New GetOverviewDashboardQuery::activityHeatmap() aggregates
activity_events.occurred_at over the last 90 days into a 7×6 grid
([day_of_week][four_hour_bucket]).
- Bucketing happens in PHP, not SQL — DAYOFWEEK() (MySQL) and
strftime('%w', ...) (SQLite) disagree on indexing AND on timezone
handling, and the test suite runs on SQLite while prod uses MySQL.
At phase-1 row counts (≤90d × low webhook traffic) loading the
timestamps and bucketing in PHP is sub-millisecond.
- 90-day window is long enough to surface a recurring rhythm but
recent enough to reflect current habits.
- MOCK_HEATMAP constant removed; class docblock graduates the slice
from "still mock" to "real today" with the timezone caveat called
out explicitly (hour buckets are sharper exposure to app.timezone
than the existing day-bucket case).
Tests: 7 new tests covering empty grid, day-and-hour placement,
multi-event accumulation, 90-day cutoff (just inside / just outside),
fixed 7×6 shape, and a bucket-boundary contract pinning 00:00 / 03:59
/ 04:00 / 11:59 / 12:00 / 23:59 to their canonical buckets.
Self-review pass via superpowers:code-reviewer; addressed both
recommendations (boundary test added, timezone caveat documented).
Sparse-account UX (a fresh account renders mostly muted) is intentional
phase-1 behavior — fix is a component-level empty state, not a query
change, if it ever becomes a friction point.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Overview dashboard's
activityHeatmapslice was a staticMOCK_HEATMAPconstant carried over from phase 0. Phase 3 shippedactivity_eventsbut never wired the heatmap to it. Last carryover from phases 0–4. Non-spec follow-up branch.Summary
GetOverviewDashboardQuery::activityHeatmap()aggregatesactivity_events.occurred_atover the last 90 days into a 7×6 grid ([day_of_week][four_hour_bucket]). Day index 0=Sun matches both Carbon'sdayOfWeekand the JSDate#getDay()axis the heatmap component already uses.DAYOFWEEK()(MySQL) andstrftime('%w', …)(SQLite) disagree on indexing AND on timezone handling — and the test suite runs on SQLite while prod uses MySQL. At phase-1 row counts (≤90d × low webhook traffic) loading the timestamps + iterating in PHP is sub-millisecond. Re-evaluate around ~100k events in the window.MOCK_HEATMAPconstant removed; class docblock graduates the slice to "real today" with the timezone caveat documented (hour buckets are sharper TZ exposure than day buckets).Test plan
vendor/bin/pint --testpasses.php artisan test— 7 new tests across the heatmap; full suite 291 passed (was 282). The 41 failures are env-only POST CSRF (419) baseline; CI passes them.npm run buildclean.maxCount; on a fresh account it should render mostly muted (honest, not a bug).Test coverage
Empty grid · day-and-hour placement · multi-event accumulation in same bucket · 90-day cutoff (just inside / just outside) · fixed 7×6 shape · bucket-boundary contract (00:00 / 03:59 / 04:00 / 11:59 / 12:00 / 23:59 → their canonical buckets).
Self-review notes
Self-review pass via
superpowers:code-reviewer; addressed both recommendations:dailyCounts()because a 6h TZ shift moves events into different buckets, not just adjacent days).Sparse-account UX (intentional phase-1 behavior): a fresh account with 7 days of events renders a mostly muted heatmap. That's an honest signal, not a bug. If it ever becomes a friction point, the fix is a component-level empty state ("collecting rhythm — check back in a week") rather than a query change.
Index status: at phase-1 row counts the cost is negligible. No new index. Watch item for
EXPLAINonce a real account crosses ~100k events in the 90-day window.What's left in the carryover bucket
None. After this lands, all phase-0 mock placeholders that have a real data source today are graduated. The remaining
MOCK_KPIS(services / alerts / uptime) ride with their own future phases (5/6/7/8).