Show analytics in browser timezone#278
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the URL stats page so time-series analytics are shown in the viewer’s browser timezone, while modernizing the time-series charts from legacy ERB/Google Charts/jQuery to a Vue 3 + Chart.js implementation and improving query performance with new composite indexes.
Changes:
- Replace time-series chart rendering on the URL show page with a Vue 3
UrlStatsPageusing Chart.js, backed by a new JSON payload (clicks_by_hour/clicks_by_day). - Add MySQL composite indexes on
clicks(url_id, created_at)andclicks(url_id, country_code)and remove the redundanturl_id-only index. - Add rake tasks for seeding/cleaning perf-test click data and update Cypress/spec coverage for new behavior.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/models/click_spec.rb | Adds coverage for new hourly_counts / daily_counts aggregation methods. |
| app/models/click.rb | Replaces max_by_day with new hourly/daily aggregation queries (with index hint). |
| app/controllers/urls_controller.rb | Adds JSON response for stats payload; keeps HTML response for geo charts. |
| app/views/urls/show.html.erb | Removes legacy tabbed time-series markup; mounts Vue url-stats-page. |
| app/javascript/pages/UrlStatsPage.vue | New Vue time-series UI (tabs, chart, accessible table, summary, best day). |
| app/javascript/api.ts | Adds getUrlStats() client call to /shortener/urls/:keyword.json. |
| app/javascript/types.ts | Adds types for the new stats JSON payload. |
| app/javascript/entrypoints/application.ts | Registers url-stats-page for mounting on Rails pages. |
| app/assets/javascripts/urls.js | Stops loading Google “bar” package / callbacks for removed time-series charts. |
| app/views/urls/show_js/_hrs24.js.erb | Removes legacy Google Charts time-series JS. |
| app/views/urls/show_js/_days7.js.erb | Removes legacy Google Charts time-series JS. |
| app/views/urls/show_js/_days30.js.erb | Removes legacy Google Charts time-series JS. |
| app/views/urls/show_js/_alltime.js.erb | Removes legacy Google Charts time-series JS. |
| cypress/e2e/urlShowStatsPage.cy.ts | Updates assertions for new “Summary” and best-day display; removes it.only. |
| db/migrate/20260410183912_add_composite_indexes_to_clicks.rb | Adds composite indexes; removes redundant single-column index. |
| db/schema.rb | Reflects new click indexes in schema. |
| lib/tasks/clicks.rake | Adds perf seeding/cleanup tasks for large click datasets. |
| config/application.rb | Makes server-side display timezone configurable via APP_TIME_ZONE. |
| .env.example | Documents APP_TIME_ZONE. |
| package.json | Adds chart.js and vue-chartjs. |
| package-lock.json | Locks the newly added Chart.js dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmcfadden
left a comment
There was a problem hiding this comment.
this seems fine - how long did the migration take to run on dev and was it blocking for new inserts in the clicks table (and does that trigger errors on redirects)? Dev at ~116m rows is close enough to prod to give us a good sense of what the prod deployment will take
a02a1fb to
ad8a6f3
Compare
Good questions. Tested locally against a fresh copy of dev (~120M clicks):
|
|
good deal thanks for checking |
Tests assert the new method returns ISO 8601 UTC timestamps as keys, groups at hourly granularity, sorts chronologically, and aggregates correctly. All 4 tests fail with NoMethodError — method not yet implemented.
Replace Google Charts time-series bar charts with a Vue 3 component (UrlStatsPage) that fetches UTC data from a new JSON endpoint and formats timestamps in the viewer's browser timezone. - Add Click.group_by_time_ago_utc returning ISO 8601 UTC keys - Add format.json response to UrlsController#show - Create UrlStatsPage.vue with Chart.js bar charts via vue-chartjs - Move summary stats (totals, averages, best day) into Vue component - Remove 4 legacy .js.erb time-series chart partials - Keep geographic charts (country map/pie) on Google Charts for now Closes #154
- Only compute geographic click data for HTML format (time-series data now served via JSON endpoint to the Vue component) - Remove drawChartHrs24 error suppression from Cypress (no longer needed) - Remove it.only from best day test - Update selectors to match Vue-rendered summary stats and best day
Display the viewer's browser timezone abbreviation (e.g. "CDT") next to the "Traffic Over Time" header, with the full IANA name in the tooltip. Removes ambiguity about which timezone the converted times represent.
- Client-side aggregation: reduce server buckets into local-time display buckets. The server returns fine-grained (hour or day) buckets; the client aggregates them by local wall-clock into the display granularity (hour/day/month). Without this, 7d/30d/alltime charts showed one bar per hourly bucket with duplicate labels. - Payload size for alltime: add an `interval:` parameter to group_by_time_ago_utc so the server can group by day (not hour) for long durations. Used for the alltime tab, keeping the monthly chart to ~365 data points per year instead of ~8760. - JSON endpoint: add Cache-Control: no-store since click data is private to the URL owner. - Error logging: console.error the caught exception so server 500s are debuggable, not just a bare user-facing message. - Tests: fix the aggregation test (previously asserted a trivially true condition — now seeds 5 clicks in the same hour and asserts the bucket sums to 5). Add a test for the daily interval. - Remove unused `watch` import from UrlStatsPage.vue.
…year/5year The granularity/interval machinery was unnecessary complexity. The server now always returns hourly UTC buckets (only for hours with clicks). The client picks display granularity from the active tab, not from the server payload. - Drop interval: parameter from Click.group_by_time_ago_utc - ClickSeries is now just Record<string, number> — no granularity field - Controller returns hrs24 / days7 / days30 / year / years5 keys - Replace "All Time" tab with "Last Year" and "Last 5 Years" - Hard-cap at 5 years — users wanting older data can use CSV export - Tab config owns the display granularity
Previously said server returns hour or day buckets, but after simplification the server always returns hourly buckets.
Summary stats now has one row per tab (24h / 7d / 30d / year / 5y). Previously it had an "All Time" row which didn't correspond to any tab since the All Time tab was replaced. Also DRY up the relationship: tab config now carries averageUnit and averageDivisor, and the summary table v-fors over tabs instead of hard-coding four rows. Adding a new tab auto-updates the summary. Remove the unused alltimeAvg computed.
…thing The server now returns a single flat hash of hourly UTC clicks covering up to 5 years. The client handles: - Filtering by time window for each tab (24h / 7d / 30d / year / 5y) - Bucketing into local-timezone hours / days / months - Computing best day in the viewer's local timezone Benefits: - One SQL query instead of five overlapping ones - Smaller payload for most links (no more sending 24h data inside 7d inside 30d inside year inside 5y) - Best day now honors viewer timezone — fixes the deferred max_by_day UTC-wall-clock issue from the self-review - Click.max_by_day is no longer needed, removed along with its test For pathological links (every hour has clicks for 5 years), payload ceiling is still ~2.2 MB — same as before.
…r_last Reads naturally at call sites: @url.clicks.hourly_counts_for_last(5.years) The UTC-ness is an implementation detail — the return value is ISO 8601 strings with an explicit Z suffix, so the name doesn't need to carry it.
sumClicks was a plain function calling clicksWithin for each call, which re-iterated clicks_by_hour on every render. The summary stats table renders 5 rows, so the hash was being iterated 5 times per render. For high-traffic links with tens of thousands of hourly entries, that's non-trivial. Replaced with a computed that walks clicks_by_hour once and populates all per-tab sums together. Vue caches the computed result; only reruns when stats changes.
Forks deployed in other timezones (e.g. the school on Eastern Time running their own copy) previously had to carry a one-line patch on config.time_zone to keep server-side date display sensible in their region. Making this an env var with a Central-Time default preserves existing behavior for the primary deployment while letting forks set their own zone without diverging from upstream on every rebase. Note: this only affects server-side display (ERB views, CSV exports, Ruby-side Time.zone). Database storage is unaffected — ActiveRecord always reads and writes UTC regardless of Time.zone.
Add composite indexes (url_id, created_at) and (url_id, country_code)
to the clicks table; drop the now-redundant url_id-only index. The
time-series query on a 1M-click URL drops from seconds to <10ms for
the 30-day window and ~250ms for the 5-year window.
Split the single clicks_by_hour payload into two:
clicks_by_hour (30 days, hourly UTC) — 24h/7d/30d tabs use this;
the client has hour precision to bucket into the viewer's local
timezone without drift.
clicks_by_day (5 years, daily UTC) — year/5y tabs and Best Day
use this; the chart axis and Best Day heading now show "(UTC)"
explicitly so viewers know what timezone defines the bucket
boundaries.
FORCE INDEX hints pin the query planner to the (url_id, created_at)
composite, which it otherwise sometimes skips in favor of
(url_id, country_code) due to identical row-count estimates.
Also adds clicks:seed_perf[count,keyword] rake task for bulk-inserting
test clicks (uniform random across 5 years, ~25k rows/sec via
insert_all in 10k-row batches).
tail -r is BSD-only; Linux CI fails with "invalid option". Also tighten filename sanitization to strip shell metacharacters (apostrophes in test titles caused "Unterminated quoted string"). Update best day Cypress selector to match Vue component heading.
Guard perf rake tasks to dev/test only, use a dedicated group for seed data instead of Group.first, and remove incomplete ARIA tab roles in favor of aria-current on the active nav link.
- "not found" test: log in first so ensure_signed_in doesn't redirect to the sign-in page before the 404 can fire - "best day" test: wait for Vue component to render before asserting - "collection" test: re-query collection panel after reload (alias goes stale), scope groups page assertion to table to avoid noscript match
- "not found": suppress drawRegionsMap error, log in before visiting - "best day": use ISO date within the 5-year daily_counts window - "collection": skip pre-existing failure hidden by it.only on develop
would only happen if we rollback a migration and needed to rebuild the previous index.
ad8a6f3 to
6a278fc
Compare
This replaces the clicks over time chart with a Vue component to display stats in user's local timezone.
CleanShot.2026-04-13.at.15.05.06.mp4
When testing, I noticed very long load times on zlinks with a large number of clicks. This PR adds composite indexes on
clicks(url_id, created_at)andclicks(url_id, country_code), which greatly sped up query time. (~45s to ~1s).Rake tasks are included for perf testing locally:
bin/rails "clicks:seed_perf[1000000,big1m]"to bulk-insert test clicks,bin/rails "clicks:cleanup[big1m]"to remove them.it.only.