Paul Nothaft
7534447b6c
Reporter @alexvaltchev hit three independent bugs on the Analytics
Dashboard. All three fixed in one PR; pluggable-tracker support
(Rybbit, Plausible, etc.) left for a separate discussion.
## Bug A — Summary cards showed 0
Two layers, both fixed.
**Frontend** (`AnalyticsPage.tsx:142-149`): the cards summed
`chartData[].views/uniqueVisitors/downloads`. The backend now (and
already) emits a dedicated `totals` object computed via separate
COUNT queries, which is what the cards should read. Postgres returns
counts as strings, so coerce via `Number()`.
**Backend** (`adminDashboard.js:268-282`): the chartData merge used
`dateObj.date === row.date`. On Postgres, pg's driver auto-converts
`DATE(timestamp)` to a JS Date object — the string-equality match
failed silently and `chartData` stayed all-zero on every Postgres
install with traffic. Added a `normaliseDateKey()` helper that
returns YYYY-MM-DD regardless of driver shape, plus `Number()`
coercion on the counts. SQLite path unchanged.
## Bug B — "Umami Not Configured" banner despite valid config
`AnalyticsPage.tsx:90` did `settings.reduce(...)` on the
`/admin/settings` response. That endpoint returns a
key/value **object** (verified at `adminSettings.js:108-149`), not
an array, so `.reduce` threw `data.reduce is not a function` and
the catch silently rendered the "Not Configured" banner even on
perfectly-configured installs. Read the umami keys directly off the
response object.
## Bug C — Device breakdown 0/0/0
Two-pronged fix.
**Primary path — Umami device API** (`services/umamiClient.js`,
wired into `adminDashboard.js`). When the admin provides an Umami
v2 API key (new setting `analytics_umami_api_key`), the backend
fetches the per-period device breakdown from Umami's
`/api/websites/:id/metrics?type=device` endpoint. Umami tracks
devices natively — far more accurate than our coarse user-agent
heuristic. The new `devicesSource` field in the response lets the
UI hint at where the numbers came from.
**Fallback hardening — local heuristic** (`adminDashboard.js:296-320`).
The existing access_logs `LIKE '%Mobile%' / '%Tablet%'` query stays
in place as a fallback for installs without Umami. Hardened with:
`whereNotNull('user_agent')` skips rows we never captured a UA on,
`Number()` coercion on COUNT results (pg returns strings), and a
guard against divide-by-zero when access_logs is empty.
## API key handling
Mirrors the existing recaptcha-secret pattern: stored plaintext in
`app_settings`, masked as `••••••••` on every GET via the existing
`adminSettings.js` GET handlers, and the frontend save mutation
silently drops the masked sentinel so re-saving without typing a
new key preserves the stored value.
## End-to-end smoke (dockerised backend with my fixes applied)
```
chartData total views: 27 ← previously 0 (date merge broken on PG)
totals: {'views': '27', 'downloads': '3', 'uniqueVisitors': '1'}
devices: {'desktop': 100, 'mobile': 0, 'tablet': 0} ← was 0/0/0
devicesSource: access_logs ← falls back correctly
analytics_umami_api_key (GET /settings/analytics): ••••••••
```
## Tests
**Backend** (15 new cases):
- `umamiClient.test.js` (10): missing-config → null, URL shape +
`x-umami-api-key` header, websiteId URL-encoding, `{x,y}` →
percentages, `laptop` → `desktop` mapping, unknown buckets
dropped, empty payload → null, non-2xx → null, invalid JSON →
null, network error → null.
- `analyticsDateMerge.test.js` (5): YYYY-MM-DD string pass-through,
ISO timestamp slice, JS Date (pg shape) → YYYY-MM-DD, null/empty
→ null, coercion for unexpected types.
**Frontend**: full 84-case vitest suite still green (no analytics
unit tests existed before; not adding any here — the changes are
narrow and the unit-level confidence comes from the type system +
the backend smoke above).
Closes #661 (bugs A + B + C). Rybbit / pluggable tracker support is
the next conversation per the issue author's follow-up.
59 lines
2.3 KiB
JavaScript
59 lines
2.3 KiB
JavaScript
/**
|
|
* Pins the date-merge fix in `adminDashboard.js` /analytics route
|
|
* (#661 Bug A). The merge previously failed on Postgres because pg's
|
|
* driver returns `DATE(timestamp)` as a JS Date object, while SQLite
|
|
* returns a string — the old `dateObj.date === row.date` comparison
|
|
* was false on Postgres so chartData stayed all-zero even with traffic.
|
|
*
|
|
* We test the normalisation helper here in isolation. The route-level
|
|
* integration is covered by the existing dashboard route test.
|
|
*/
|
|
|
|
// The helper is internal to the route file; reimport via a small wrapper
|
|
// so we don't need to export everything publicly.
|
|
const path = require('path');
|
|
const fs = require('fs');
|
|
|
|
const ROUTE_SRC = fs.readFileSync(
|
|
path.join(__dirname, '../../src/routes/adminDashboard.js'),
|
|
'utf8',
|
|
);
|
|
// Tiny evaluator that grabs the normaliseDateKey function definition from
|
|
// the route source so the test pins the actual shipping implementation,
|
|
// not a copy.
|
|
function extractNormaliseDateKey() {
|
|
const match = ROUTE_SRC.match(/function normaliseDateKey\([\s\S]*?\n\}/);
|
|
if (!match) throw new Error('normaliseDateKey not found in adminDashboard.js');
|
|
// eslint-disable-next-line no-new-func
|
|
return new Function(`${match[0]}; return normaliseDateKey;`)();
|
|
}
|
|
|
|
const normaliseDateKey = extractNormaliseDateKey();
|
|
|
|
describe('analytics route — normaliseDateKey (#661 Bug A)', () => {
|
|
test('passes through a YYYY-MM-DD string unchanged', () => {
|
|
expect(normaliseDateKey('2026-06-22')).toBe('2026-06-22');
|
|
});
|
|
|
|
test('slices off a time component on a longer ISO string', () => {
|
|
expect(normaliseDateKey('2026-06-22T00:00:00.000Z')).toBe('2026-06-22');
|
|
});
|
|
|
|
test('normalises a JS Date object (Postgres pg-driver shape) to YYYY-MM-DD', () => {
|
|
const d = new Date('2026-06-22T12:34:56Z');
|
|
expect(normaliseDateKey(d)).toBe('2026-06-22');
|
|
});
|
|
|
|
test('returns null for null / undefined / empty', () => {
|
|
expect(normaliseDateKey(null)).toBeNull();
|
|
expect(normaliseDateKey(undefined)).toBeNull();
|
|
expect(normaliseDateKey('')).toBeNull();
|
|
});
|
|
|
|
test('coerces unexpected types via String() to avoid throwing', () => {
|
|
// We don't expect to receive a number from either driver, but the
|
|
// helper should not crash if it does — date merge will simply miss.
|
|
expect(normaliseDateKey(20260622)).toBe('20260622');
|
|
});
|
|
});
|