Skip to content

feat(dashboard): XP history chart + xpByDay tracking - #81

Open
CommunityPoke wants to merge 1 commit into
guirguispierre:mainfrom
CommunityPoke:devin/1789780683-xp-history-chart
Open

CommunityPoke wants to merge 1 commit into
guirguispierre:mainfrom
CommunityPoke:devin/1789780683-xp-history-chart

Conversation

@CommunityPoke

Copy link
Copy Markdown
Contributor

Summary

Adds an XP history chart to the dashboard. XP was previously stored only as cumulative totals on levels docs, so there was nothing to chart — awardXP now also maintains an xpByDay map (UTC YYYY-MM-DD → XP earned) on each member's level doc. One map key per active day keeps doc growth bounded; the existing guildId+xp composite index covers the new query.

  • src/services/xpHistory.js (new): pure helpers — dayKey, listDayKeys, addXpToDay, and buildXpHistory(docs, {days, topUsers}) which returns {days, totals, users[]} (guild-wide daily totals across all members + per-member daily series for the top N).
  • src/services/firestore.js: new gated getGuildLevels(guildId, limit); returns [] when Firebase is disabled, same contract as the other helpers.
  • GET /api/xp-history?days=&guildId= on the dashboard (behind the existing API-key auth): days clamped 7–90 (default 30), guild defaults to DISCORD_GUILD_ID.
  • index.html: new "XP History" panel — hand-rolled responsive SVG line/area chart (no new deps, matches the dashboard's vanilla JS + gold/glass styling), member selector for top-10 users, hover tooltips, loading/empty/error states, role="img" + aria-label, and a .sr-only data table for screen readers. Redraws on resize and refreshes with the existing 30s poll.

Validation

  • npm test: 551/551 pass (12 new cases in test/xpHistory.test.js covering the aggregation helpers and awardXP's xpByDay write via the in-memory Firestore fake).
  • Manually verified in-browser: loading → empty state (no XP recorded), and the rendered chart with axes, tooltips, and member switching using injected data.

Note: history accumulates from deploy forward — existing xp totals aren't backfillable into per-day buckets. README + CHANGELOG updated.

- awardXP records an xpByDay map (UTC day -> XP earned) on each levels doc
- New src/services/xpHistory.js aggregates level docs into daily series
- New GET /api/xp-history endpoint (auth'd, days/guildId params)
- Dashboard renders a responsive SVG chart with member selector,
  loading/empty/error states, and a screen-reader data table
- 12 new node:test cases covering the helpers and awardXP recording

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Code review — XP history chart

Reviewed the full diff, ran the suite on this branch locally: 551/551 tests pass (verified the claim). Index claim also checks out — firestore.indexes.json already has levels: guildId ASC + xp DESC, and the new route lands after app.use('/api', apiAuth), so auth coverage is correct. The gated getGuildLevels helper matches the module's existing pattern. Nice a11y work on the chart (role=img, aria-label, sr-only table) with zero new deps.

Findings, ordered by severity:

Medium — totals only aggregates the top-50 member docs, not the guild

src/dashboard/server.js:134 fetches getGuildLevels(guildId, 50) — top 50 by all-time xp — and buildXpHistory sums xpByDay across only those docs for totals. Once more than 50 members have XP, the "All members" series silently undercounts, and it's biased: a very active member ranked 51st contributes nothing, while inactive all-time-high members hold slots. Options: fetch all levels docs for the totals (read cost scales with member count), maintain a separate guild-aggregate xpByDay doc written alongside the member write, or relabel the series to make the top-50 scope explicit.

Low — xpByDay grows forever

src/commands/level.js:187 — one map key per active day, never pruned, and every awardXP read-modify-writes the whole map. The API clamps days to ≤90, so keys older than that are dead weight (~5–10 KB/yr for daily-active members — well under the 1 MiB doc limit, but pure waste and growing write payload). Consider pruning keys older than 90 days in addXpToDay.

Low — esc() doesn't escape quotes; label/userId go into HTML attributes

index.html:760 interpolates esc(label) into aria-label="...", and the member <option value="${esc(u.userId)}"> does the same. esc escapes &<> but not "/' — a quote in either field would break out of the attribute. Not exploitable today (Discord usernames are [a-z0-9._], user IDs are snowflakes), but a latent sink if the field source ever switches to display names (arbitrary Unicode). Cheap hardening: extend esc to cover quotes, or set attributes via DOM APIs.

Nit — quarter-tick labels can mislabel the axis

niceCeil(5) produces ticks at 1.25/2.5/3.75, and fmtNum's toFixed(1) renders them as "1.3"/"2.5"/"3.8" (index.html:723). Cosmetic — either pick tick steps that divide the nice number evenly or format to the precision the step needs.

None of these are merge-blockers for a small guild, but the totals scope is worth either fixing or documenting before it quietly misleads. Overall solid, well-tested PR.

Written by Devin

@WolfyBlair

Copy link
Copy Markdown

Please review @claude @codex

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants