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.
143 lines
5.1 KiB
JavaScript
143 lines
5.1 KiB
JavaScript
/**
|
|
* Unit tests for the Umami v2 metrics-API client (#661 Bug C).
|
|
*
|
|
* The client is only consumed by `/admin/dashboard/analytics` today to fetch
|
|
* the device-breakdown chart, so these tests pin:
|
|
* - The exact URL shape sent to Umami (`/api/websites/<id>/metrics?type=device&startAt=…&endAt=…`)
|
|
* - The `x-umami-api-key` auth header
|
|
* - The `{ x, y }` → `{ desktop, mobile, tablet }` percentage normalisation
|
|
* - `laptop` mapping into `desktop` for our 3-bucket UI
|
|
* - Defensive returns: missing config / non-2xx / non-JSON / empty array
|
|
* all return `null` so the route layer can fall back to access_logs.
|
|
*/
|
|
|
|
const { fetchUmamiDeviceBreakdown } = require('../../src/services/umamiClient');
|
|
|
|
const ORIGINAL_FETCH = global.fetch;
|
|
|
|
afterEach(() => {
|
|
global.fetch = ORIGINAL_FETCH;
|
|
});
|
|
|
|
function mockJson(body, { status = 200 } = {}) {
|
|
global.fetch = jest.fn(async () => ({
|
|
ok: status >= 200 && status < 300,
|
|
status,
|
|
json: async () => body,
|
|
}));
|
|
}
|
|
|
|
describe('fetchUmamiDeviceBreakdown', () => {
|
|
test('returns null when config is incomplete (back-compat for installs without API key)', async () => {
|
|
expect(await fetchUmamiDeviceBreakdown({})).toBeNull();
|
|
expect(await fetchUmamiDeviceBreakdown({ baseUrl: 'https://u.example' })).toBeNull();
|
|
expect(await fetchUmamiDeviceBreakdown({ baseUrl: 'https://u.example', websiteId: 'w' })).toBeNull();
|
|
// No fetch should be issued in any of those cases.
|
|
expect(global.fetch).toBe(ORIGINAL_FETCH);
|
|
});
|
|
|
|
test('builds the expected URL + sends the x-umami-api-key header', async () => {
|
|
mockJson([{ x: 'desktop', y: 10 }]);
|
|
await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u.example.com/',
|
|
websiteId: 'site-123',
|
|
apiKey: 'secret',
|
|
startMs: 1700000000000,
|
|
endMs: 1700003600000,
|
|
});
|
|
expect(global.fetch).toHaveBeenCalledTimes(1);
|
|
const [calledUrl, init] = global.fetch.mock.calls[0];
|
|
expect(calledUrl).toBe(
|
|
'https://u.example.com/api/websites/site-123/metrics?type=device&startAt=1700000000000&endAt=1700003600000',
|
|
);
|
|
expect(init.headers['x-umami-api-key']).toBe('secret');
|
|
expect(init.method).toBe('GET');
|
|
});
|
|
|
|
test('encodes the websiteId so a path segment with reserved chars is safe', async () => {
|
|
mockJson([{ x: 'desktop', y: 1 }]);
|
|
await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u.example.com',
|
|
websiteId: 'a/b?c',
|
|
apiKey: 'k',
|
|
startMs: 0, endMs: 0,
|
|
});
|
|
const [calledUrl] = global.fetch.mock.calls[0];
|
|
expect(calledUrl).toContain('/api/websites/a%2Fb%3Fc/metrics');
|
|
});
|
|
|
|
test('normalises a typical { x, y } payload into integer percentages', async () => {
|
|
mockJson([
|
|
{ x: 'desktop', y: 60 },
|
|
{ x: 'mobile', y: 30 },
|
|
{ x: 'tablet', y: 10 },
|
|
]);
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
expect(out).toEqual({ desktop: 60, mobile: 30, tablet: 10 });
|
|
});
|
|
|
|
test('maps `laptop` into `desktop` for the 3-bucket UI', async () => {
|
|
mockJson([
|
|
{ x: 'desktop', y: 50 },
|
|
{ x: 'laptop', y: 20 },
|
|
{ x: 'mobile', y: 30 },
|
|
]);
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
// desktop = (50 + 20) / 100 = 70%
|
|
expect(out).toEqual({ desktop: 70, mobile: 30, tablet: 0 });
|
|
});
|
|
|
|
test('drops unknown buckets entirely (avoids silent miscategorisation)', async () => {
|
|
mockJson([
|
|
{ x: 'desktop', y: 80 },
|
|
{ x: 'mobile', y: 20 },
|
|
{ x: 'unknown-future-bucket', y: 100 },
|
|
]);
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
// 100 isn't counted into total, so 80/(80+20) = 80%, 20/(80+20) = 20%.
|
|
expect(out).toEqual({ desktop: 80, mobile: 20, tablet: 0 });
|
|
});
|
|
|
|
test('returns null on empty payload (caller falls back to access_logs)', async () => {
|
|
mockJson([]);
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
expect(out).toBeNull();
|
|
});
|
|
|
|
test('returns null on non-2xx upstream response', async () => {
|
|
mockJson({ error: 'unauthorized' }, { status: 401 });
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
expect(out).toBeNull();
|
|
});
|
|
|
|
test('returns null on invalid JSON body', async () => {
|
|
global.fetch = jest.fn(async () => ({
|
|
ok: true,
|
|
status: 200,
|
|
json: async () => { throw new SyntaxError('not json'); },
|
|
}));
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
expect(out).toBeNull();
|
|
});
|
|
|
|
test('returns null on network error (fetch throws)', async () => {
|
|
global.fetch = jest.fn(async () => { throw new Error('ECONNREFUSED'); });
|
|
const out = await fetchUmamiDeviceBreakdown({
|
|
baseUrl: 'https://u', websiteId: 'w', apiKey: 'k', startMs: 0, endMs: 0,
|
|
});
|
|
expect(out).toBeNull();
|
|
});
|
|
});
|