diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c3c3d6c..b5e6b30 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -87,7 +87,8 @@ KJ Controller is a web-based karaoke show management application. A Flask backen | `sleep_mode.py` | ~100 | `SleepManager` class: enter/exit low-power sleep mode, stop services, unmount SSD | | `push_dispatcher.py` | ~200 | `PushDispatcher` class: VAPID config, subscription scan, ladder decision (`now_singing`/`up_next`/`up_in_2`), dedup via `last_sent_state`, 500ms debounce, `ThreadPoolExecutor` send pool. Pure helpers at module level (`decide_ladder_step`, `next_entry_for_phone`, `render_payload`). | | `sing.py` | ~280 | Public `/sing/*` blueprint (landing, search, submit, status, rules, now, manifest, sw, push subscribe/unsubscribe) + token-gate decorator + per-IP rate limiter + host-based route guard + QR-overlay auto-sync helper | -| `sing_store.py` | ~260 | `SingStore` class: SQLite CRUD for `sing_requests` + `sing_push_subscriptions` + event-token helpers (regenerate / enable / auto-approve) on `rotation_meta` | +| `sing_store.py` | ~260 | `SingStore` class: SQLite CRUD for `sing_requests` (incl. `user_agent` device capture + `get_requests_for_entries` night-scoped session lookup) + `sing_push_subscriptions` + event-token helpers (regenerate / enable / auto-approve) on `rotation_meta` | +| `ua_parse.py` | ~130 | Pure best-effort User-Agent parser (`parse_user_agent`, `summarize`) → friendly browser/OS/device labels; no dependency | | `routes.py` | ~1000 | Flask Blueprint with all route handlers (includes `/rotation/requests/*` admin endpoints for the public request form). Hosts the playability gates: tier-1 inline `_playability_gate` (link/upload/download hard-block) + tier-2 async render verification (`_enqueue_tier2` → single-worker queue → `_run_tier2_check` against the active renderer, stamps `playability_warning`) | | `wait_estimate.py` | ~80 | Pure function `compute_estimate(entries, target_id, cfg)` producing `{position, expected_s, range_low_s, range_high_s, spread_source, close_to_front, now_singing}`. Uses tonight's sung-entry variance for the range; falls back to a configurable minimum spread. | | `sing_resolve.py` | ~120 | Pure decision logic for singer-submission download fallback: `classify_error` (unavailable → advance to next candidate vs transient → retry same) and `next_candidate_index` (bounded by `MAX_CANDIDATES`). No yt-dlp/network/Flask deps → exhaustively unit-tested. | @@ -409,6 +410,8 @@ Additional `static-sing/` assets added in sub-project #4: **Duet partners + multi-song done screen (2026-05-15):** Singers can attach up to 3 duet partners (name + optional phone) on the confirm screen via a new `additional_singers TEXT NULL` column on `sing_requests` (JSON array). `POST /sing/submit` validates the field (max 3, name required, phone format optional). `approve_sing_request` builds a `singers=[primary, …partner_names]` list and passes it to `rotation.add_entry(...)` — the existing `singers_json` plumbing on `rotation_entries` joins names with ` & ` for the legacy `singer` text column and persists the structured list. KJ admin approval card renders a duet block with `sms:` links for partner phones. The singer's done screen now lists all their submitted requests via a new `GET /sing/my-requests?ids=…` endpoint (max 20 ids per call, returns `{now_playing, requests:[{request, estimate?, performed}]}`) and includes a "+ Request another song" button that resets song-picking state while preserving identity. Request ids are tracked in `localStorage` (`sing_my_request_ids`) scoped per token so yesterday's ids don't leak into a new event. Partner phones are display-only (no push subscriptions for partners) — the KJ texts them manually from the admin card. +**Singer session provenance + smarter Merge (2026-08-13):** Motivated by the "two Chailas" incident (a self-registered singer vs a duet-partner label typed into someone else's request looked like two people). Every `/sing/submit` and `/sing/requests//change` now stores the submitting device's `User-Agent` on a new `sing_requests.user_agent` column. `routes._add_singer_session_info(singer_stats)` (called alongside `_add_last_sang_to_singer_stats` in `GET /rotation` and every singer action response) attaches a `session` block to each singer by matching their rotation-entry ids to linked requests via `SingStore.get_requests_for_entries(entry_ids, night_started)` (night-scoped, fails closed like the SMS phone lookup): `origin` is `singer_ui` (owns a linked request whose `singer_name` is theirs → `has_device`, carries `device`/`phone`/`request_count`/`sources`), `duet_partner` (only appears in another request's `additional_singers`), or `kj_added` (no linked request). The frontend shows a 📱 icon only on `singer_ui` singers (click → device-details popup) and replaces the inline Merge dropdown with a modal: searchable singer list (device-linked first), then a confirm step spelling out KEEP vs REMOVE, combined sung/queued totals, and a real-device warning + **Swap** that defaults the keeper to the phone-linked singer. `ua_parse.py` turns the raw UA into a friendly summary (Android UAs also yield the model, e.g. SM-S911B; iOS exposes only "iPhone"). + **Done-screen ordering + sung-song clearing (2026-07-17, v0.88.0):** The "Your songs tonight" list previously rendered in submission order (the client passes its `localStorage` id list to `/my-requests`, which echoes them back in that order) and never dropped sung songs (a `sing_request` stays status `approved` after its rotation entry is sung, and `get_rotation()` returns the ACTIVE queue only, so the sung entry got a `position=None` estimate and lingered as "Added to the queue" all night). Fix: `/my-requests` now sets a per-item `performed` boolean — for a linked entry not in the active queue it looks up `rotation.store.get_entry(...)` and marks `performed=True` when the entry is Done/Left (no estimate attached). The client (`sing.js`) sorts the active list into sung order (now singing → queue position → awaiting-KJ) and moves performed songs into a collapsed **"✓ Already sung tonight (N)"** section that's read-only (no cancel/change/reorder). `performed` songs are excluded from `_liveSongs`, so the persistent "🎤 My songs (N)" bar count and boot smart-restore ignore them (a singer who has sung everything lands on the request screen, not a stale list). ### Singer Web Push (sub-project #4) diff --git a/docs/archive/2026-08-13-singer-session-provenance-plan.md b/docs/archive/2026-08-13-singer-session-provenance-plan.md new file mode 100644 index 0000000..713b3e9 --- /dev/null +++ b/docs/archive/2026-08-13-singer-session-provenance-plan.md @@ -0,0 +1,87 @@ +# Singer session provenance + smarter Merge — plan + +**Date:** 2026-08-13 +**Branch:** `feat/singer-session-provenance` + +## Motivation + +Live-show investigation (the "two Chailas" case): the KJ Singers list can show the +same person twice when one name is a **self-registered singer** (own phone session) +and the other is a **duet-partner label** typed into someone else's submission, or a +**KJ-hand-added** entry. Today nothing in the UI distinguishes these, and Merge is a +bare name dropdown that doesn't say which singer is retained or what history moves. + +## Goals + +1. Show a **phone/device icon** beside singers who have a real device session + (they personally submitted ≥1 request from the singer UI). Duet-partner labels and + KJ-added singers get no icon. +2. Clicking the icon opens a **device-details popup** with whatever we captured + (parsed browser/OS/device + raw User-Agent, phone on file, first submitted, + request count, source types). +3. Replace the inline Merge dropdown with a proper **modal**: searchable/scrollable + singer list, clear "who is retained" preview, a real-device warning + Swap + direction, and an explicit confirm. + +## Data model + +- **New column** `sing_requests.user_agent TEXT` (additive migration in + `SingStore.init_schema`, mirrors `sing_push_subscriptions.user_agent`). +- `SingStore.create_request(..., user_agent=None)` stores it. +- `sing.py` `/submit` and `/requests//change` pass + `request.headers.get("User-Agent", "")[:500]`. +- Client Hints (Sec-CH-UA-Model) deferred — the singer PWA is usually a single + visit, so `Accept-CH` wouldn't be honoured on the first load. UA parsing + + raw string is enough for v1. + +## Backend — session enrichment + +- New `ua_parse.py`: pure `parse_user_agent(ua) -> {browser, os, device, is_mobile}` + (heuristic, best-effort, always keeps raw). Unit-tested. +- `SingStore.get_requests_for_entries(entry_ids, night_started)` → linked requests + (id, linked_entry_id, singer_name, phone, user_agent, created_at, source_type, + additional_singers), night-scoped (mirrors the SMS phone lookup guard). +- `routes._add_singer_session_info(singer_stats, app)` decorator, called alongside + `_add_last_sang_to_singer_stats` in `get_rotation` and `_singer_action_response`. + Per singer it attaches: + ``` + session: { + origin: 'singer_ui' | 'duet_partner' | 'kj_added', + has_device: bool, # true only for singer_ui + phone: str, # from newest own request (may be '') + device: {browser, os, device, is_mobile, raw}, # newest own request UA + request_count: int, + first_request_at: str|null, + sources: [str, ...], # distinct source_types + } + ``` + Classification: match the singer's rotation-entry ids to linked requests. + - own request whose `singer_name` == singer name → `singer_ui` (has_device). + - else appears in a linked request's `additional_singers` → `duet_partner`. + - else no linked request → `kj_added`. + +## Frontend + +- `buildSingerRow`: when `session.has_device`, insert a 📱 icon button after the name + → `openSingerDeviceModal(singer)`. +- `openSingerDeviceModal`: reuses the songs-modal backdrop pattern; shows parsed + device, phone, first-seen, request count, sources, and raw UA (monospace). +- Replace `showMergeDropdown` → `openMergeModal(singer)`: + - search box + scrollable list of all other singers (self excluded); each option + shows name + 📱 badge + sung/queued counts; done singers de-emphasised. + - selecting a target reveals a **confirmation panel**: keeper badge on the retained + name, combined-history sentence, real-device warning when merging a phone-linked + singer into a non-linked one, **Swap direction**, and Confirm. +- CSS for icon, device modal, merge modal (dark theme, existing tokens). + +## Tests + +- `ua_parse` unit tests (iOS Safari, Android Chrome w/ model, desktop, junk). +- Migration idempotency + UA persisted on submit (endpoint test). +- Session classification: singer_ui / duet_partner / kj_added on a seeded rotation. + +## Docs + +- `docs/ARCHITECTURE.md` (session provenance flow), `kj-controller/docs/CHANGELOG.md`, + version bump. +``` diff --git a/kj-controller/docs/CHANGELOG.md b/kj-controller/docs/CHANGELOG.md index a5622f3..733e161 100644 --- a/kj-controller/docs/CHANGELOG.md +++ b/kj-controller/docs/CHANGELOG.md @@ -4,6 +4,15 @@ Dated entries, newest first. Each entry notes any required deploy steps. --- +## 2026-08-13 - Singer session provenance + smarter Merge (v0.91.0) + +**Deploy:** backend (`sing_store.py`, `sing.py`, `routes.py`, new `ua_parse.py`) → **requires `systemctl restart kj-controller`** (interrupts playback — deploy between songs). Also frontend (`app.js`, `style.css`). **DB migration:** additive `sing_requests.user_agent` column (auto-applied on boot; safe to roll back — the column is simply ignored). + +- **Why:** the "two Chailas" incident — the KJ Singers list showed the same person twice because one entry was her own self-registered singer ("Chaila R", from her phone) and the other was a **duet-partner label** ("Chaila") that another singer typed into their own submission. Nothing in the UI distinguished a real device session from a duet label or a KJ-hand-added entry, and Merge didn't say which singer was kept. +- **Device capture:** every singer-UI `/submit` and `/change` now records the submitting device's `User-Agent` on `sing_requests`. A small heuristic parser (`ua_parse.py`) turns it into a friendly "iPhone · Safari · iOS 17.4" summary (Android UAs also expose the model, e.g. SM-S911B). +- **Provenance on the Singers list:** each singer now carries a `session` block classifying them as `singer_ui` (own device — gets a 📱 icon), `duet_partner` (credited in someone else's request), or `kj_added` (typed in by the KJ). Only real device sessions show the 📱 icon; clicking it opens a **device-details popup** (parsed device, phone on file, first submitted, song count, sources, raw User-Agent). +- **Merge redesign:** the inline dropdown is now a **modal** with a searchable/scrollable singer list (real-device sessions sorted first, each showing device/sung/queued badges). Selecting a partner opens a **confirm step** that spells out exactly which name is KEPT vs REMOVED, the combined sung/queued totals, and a note that a phone-linked keeper inherits the other's history. When you'd merge a phone-linked singer *away* into a non-linked one, it warns and offers **Swap** — defaulting the keeper to the device-linked singer (the KJ's stated preference). + ## 2026-07-17 - Auto Order — fair automatic rotation reordering (v0.90.0) **Deploy:** backend (`routes.py`, `rotation.py`, `rotation_store.py`, `sing_store.py`, `sing.py`, new `auto_order.py`) → **requires `systemctl restart kj-controller`** (interrupts playback — deploy between songs). Also frontend (`app.js`, `index.html`). No DB migration (reuses the `rotation_meta` key/value table). diff --git a/kj-controller/pyproject.toml b/kj-controller/pyproject.toml index 5f57b42..b0993c4 100644 --- a/kj-controller/pyproject.toml +++ b/kj-controller/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "kj-controller" -version = "0.90.0" +version = "0.91.0" description = "Web-based karaoke show management with mpv + VLC playback" requires-python = ">=3.11" diff --git a/kj-controller/routes.py b/kj-controller/routes.py index 169358f..1a249c2 100644 --- a/kj-controller/routes.py +++ b/kj-controller/routes.py @@ -2909,12 +2909,97 @@ def _add_last_sang_to_singer_stats(singer_stats, rotation): singer["last_sang_minutes"] = mins +def _add_singer_session_info(singer_stats, app=None): + """Attach ``session`` provenance to each singer stat dict. + + Answers "how did this singer come to exist?" so the KJ can tell a real + singer-UI device session apart from a duet-partner label or a KJ-hand-added + entry — and pick the right keeper when merging. Best-effort: any failure + leaves ``session`` absent rather than breaking the rotation poll. + + Classification per singer (matching their rotation-entry ids to linked + ``sing_requests``, night-scoped): + - ``singer_ui`` — owns >=1 linked request whose ``singer_name`` is theirs + -> ``has_device`` True; carries device/phone details. + - ``duet_partner`` — only appears in another request's ``additional_singers``. + - ``kj_added`` — has entries but no linked request (KJ typed them in). + """ + from ua_parse import parse_user_agent + + app = app or current_app._get_current_object() + sing_store = getattr(app, "sing_store", None) + if sing_store is None: + return + try: + night_started = sing_store.get_night_started_at() + all_ids = [] + for s in singer_stats: + for e in (s.get("entries") or []): + if e.get("id") is not None: + all_ids.append(e["id"]) + reqs = sing_store.get_requests_for_entries(all_ids, night_started) + except Exception: + return + + # entry_id -> linked requests (query already returns newest id first). + by_entry = {} + for r in reqs: + by_entry.setdefault(r["linked_entry_id"], []).append(r) + + for singer in singer_stats: + try: + name_lower = singer["name"].strip().lower() + own = [] + is_duet_partner = False + for e in (singer.get("entries") or []): + for r in by_entry.get(e.get("id"), []): + if (r.get("singer_name") or "").strip().lower() == name_lower: + own.append(r) + else: + for p in (r.get("additional_singers") or []): + if isinstance(p, dict) and (p.get("name") or "").strip().lower() == name_lower: + is_duet_partner = True + + if own: + own.sort(key=lambda r: r.get("id") or 0, reverse=True) + newest = own[0] + # Prefer the newest non-empty phone (singers sometimes drop the + # number on later submits after giving it the first time). + phone = "" + for r in own: + if (r.get("phone") or "").strip(): + phone = r["phone"].strip() + break + singer["session"] = { + "origin": "singer_ui", + "has_device": True, + "phone": phone, + "device": parse_user_agent(newest.get("user_agent")), + "request_count": len(own), + "first_request_at": min(r.get("created_at") or "" for r in own) or None, + "sources": sorted({r.get("source_type") for r in own if r.get("source_type")}), + } + else: + singer["session"] = { + "origin": "duet_partner" if is_duet_partner else "kj_added", + "has_device": False, + "phone": "", + "device": parse_user_agent(""), + "request_count": 0, + "first_request_at": None, + "sources": [], + } + except Exception: + continue + + def _singer_action_response(rotation): """Build standard response for singer action routes.""" entries = rotation.get_rotation() _decorate_rotation_entries(entries, rotation) singer_stats = rotation.get_singer_stats() _add_last_sang_to_singer_stats(singer_stats, rotation) + _add_singer_session_info(singer_stats) return jsonify({"success": True, "entries": entries, "singer_stats": singer_stats}) @@ -3148,6 +3233,7 @@ def get_rotation(): _decorate_rotation_entries(entries, rotation) singer_stats = rotation.get_singer_stats() _add_last_sang_to_singer_stats(singer_stats, rotation) + _add_singer_session_info(singer_stats) return jsonify({ "entries": entries, "singer_stats": singer_stats, diff --git a/kj-controller/sing.py b/kj-controller/sing.py index 57a02cf..34d6d7e 100644 --- a/kj-controller/sing.py +++ b/kj-controller/sing.py @@ -588,6 +588,7 @@ def submit(): source_meta=source_meta, notes=notes, additional_singers=additional, + user_agent=request.headers.get("User-Agent", "")[:500], ) auto_approved = False @@ -1020,6 +1021,7 @@ def change_request(req_id): source_type=source_type, source_ref=source_ref, source_meta=source_meta, token=req["token"], additional_singers=req.get("additional_singers"), supersedes_request_id=req_id, + user_agent=request.headers.get("User-Agent", "")[:500], ) return jsonify({"success": True, "request": { **_public_request_view(new_req), "edit_token": new_req.get("edit_token")}}) diff --git a/kj-controller/sing_store.py b/kj-controller/sing_store.py index 736032c..af1c722 100644 --- a/kj-controller/sing_store.py +++ b/kj-controller/sing_store.py @@ -182,6 +182,18 @@ def init_schema(self): except sqlite3.OperationalError as e: if "duplicate column name" not in str(e).lower(): raise + # Additive migration — `user_agent` (2026-08-13) records the submitting + # device's User-Agent so the KJ can tell a real singer-UI session apart + # from a duet-partner label or a KJ-hand-added entry, and eyeball the + # phone/browser. Mirrors sing_push_subscriptions.user_agent. Existing + # rows get NULL (unknown device). + try: + conn.execute( + "ALTER TABLE sing_requests ADD COLUMN user_agent TEXT DEFAULT NULL" + ) + except sqlite3.OperationalError as e: + if "duplicate column name" not in str(e).lower(): + raise conn.commit() # ------------------------------------------------------------------ @@ -407,6 +419,7 @@ def create_request( token=None, additional_singers=None, supersedes_request_id=None, + user_agent=None, ): """Insert a new pending request and return the created row as a dict.""" if not singer_name: @@ -430,8 +443,9 @@ def create_request( INSERT INTO sing_requests (token, singer_name, phone, song_artist, song_title, source_type, source_ref, source_meta, notes, - additional_singers, edit_token, supersedes_request_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + additional_singers, edit_token, supersedes_request_id, + user_agent) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, ( request_token, @@ -446,6 +460,7 @@ def create_request( partners_json, edit_token, supersedes_request_id, + (user_agent or None), ), ) conn.commit() @@ -478,6 +493,38 @@ def list_requests(self, status=None, token=None, limit=None): ).fetchall() return [self._row_to_dict(r) for r in rows] + def get_requests_for_entries(self, entry_ids, night_started=None): + """Return linked sing_requests for the given rotation-entry ids. + + Used to attribute rotation entries to the device session that submitted + them (singer session provenance). Night-scoped like the SMS phone lookup + (``_add_sms_status``): a New Rotation recycles rotation_entry ids, so a + recycled id could otherwise phantom-match a prior night's request. Fails + CLOSED (returns []) when ``night_started`` is None — callers pass the + current ``night_started_at`` which ``ensure_night_started`` guarantees. + + Returns a list of dicts (newest first) with the columns needed for + classification: id, linked_entry_id, singer_name, phone, user_agent, + created_at, source_type, additional_singers (deserialised). + """ + ids = [int(e) for e in (entry_ids or []) if e is not None] + if not ids or not night_started: + return [] + conn = self._get_conn() + placeholders = ",".join("?" * len(ids)) + rows = conn.execute( + f""" + SELECT id, linked_entry_id, singer_name, phone, user_agent, + created_at, source_type, additional_singers + FROM sing_requests + WHERE linked_entry_id IN ({placeholders}) + AND created_at >= ? + ORDER BY id DESC + """, + tuple(ids + [night_started]), + ).fetchall() + return [self._row_to_dict(r) for r in rows] + def count_by_status(self): """Return a dict {status: count} for all statuses present.""" conn = self._get_conn() diff --git a/kj-controller/static/app.js b/kj-controller/static/app.js index 733f7a6..1a6b0cc 100644 --- a/kj-controller/static/app.js +++ b/kj-controller/static/app.js @@ -6099,6 +6099,20 @@ function buildSingerRow(singer) { name.textContent = singer.name; info.appendChild(name); + // Device icon \u2014 only for singers with a real singer-UI device session, so + // the KJ can tell them apart at a glance from duet-partner labels / KJ-added + // entries. Click opens the captured device details. + if (singer.session && singer.session.has_device) { + const dev = document.createElement('button'); + dev.className = 'singer-device-icon'; + dev.textContent = '\uD83D\uDCF1'; // \uD83D\uDCF1 + const summary = singerDeviceSummary(singer.session); + dev.title = 'Linked device session' + + (summary ? ' \u2014 ' + summary : '') + ' (tap for details)'; + dev.onclick = (ev) => { ev.stopPropagation(); openSingerDeviceModal(singer); }; + info.appendChild(dev); + } + if (singer.has_tipped) { const tip = document.createElement('span'); tip.className = 'singer-stats-tip'; @@ -6199,7 +6213,7 @@ function buildSingerActions(actions, singer, row) { mergeBtn.className = 'singer-stats-btn'; mergeBtn.textContent = 'Merge'; mergeBtn.title = 'Merge this singer into another \u2014 use when the same person was added under two different names'; - mergeBtn.onclick = (ev) => showMergeDropdown(ev, singer); + mergeBtn.onclick = () => openMergeModal(singer); actions.appendChild(mergeBtn); const splitBtn = document.createElement('button'); @@ -6596,41 +6610,285 @@ function enterSingerEditMode(row, singer) { }); } -function showMergeDropdown(ev, singer) { - document.querySelectorAll('.singer-merge-dropdown').forEach(d => d.remove()); +// One-line human summary of a singer.session's device, e.g. +// "iPhone · Safari · iOS 17.4". '' when nothing was captured. +function singerDeviceSummary(session) { + const d = (session && session.device) || {}; + const parts = []; + for (const x of [d.device, d.browser, d.os]) { + if (x && parts.indexOf(x) === -1) parts.push(x); + } + return parts.join(' · '); +} + +// Human label + css class for a singer's provenance origin. +function singerOriginMeta(session) { + const origin = (session && session.origin) || null; + if (origin === 'singer_ui') return { label: 'Linked device session', cls: 'origin-device' }; + if (origin === 'duet_partner') return { label: 'Duet-partner label (no own device)', cls: 'origin-duet' }; + if (origin === 'kj_added') return { label: 'Added by KJ (no device)', cls: 'origin-kj' }; + return { label: 'Unknown origin', cls: 'origin-unknown' }; +} - const others = singerStatsData - .filter(s => s.name.toLowerCase() !== singer.name.toLowerCase() && s.status !== 'done') - .map(s => s.name); +let openSingerDeviceName = null; +function closeSingerDeviceModal() { + openSingerDeviceName = null; + document.querySelectorAll('.singer-device-modal-backdrop').forEach(d => d.remove()); + document.removeEventListener('keydown', singerDeviceEscHandler); +} +function singerDeviceEscHandler(e) { if (e.key === 'Escape') closeSingerDeviceModal(); } + +// Popup showing whatever we captured about the device that created this singer. +function openSingerDeviceModal(singer) { + if (openSingerDeviceName === singer.name + && document.querySelector('.singer-device-modal-backdrop')) { + closeSingerDeviceModal(); + return; + } + closeSingerDeviceModal(); + + const s = singer.session || {}; + const d = s.device || {}; + const backdrop = document.createElement('div'); + backdrop.className = 'singer-device-modal-backdrop'; + backdrop.onclick = (ev) => { if (ev.target === backdrop) closeSingerDeviceModal(); }; + + const origin = singerOriginMeta(s); + const rows = []; + const summary = singerDeviceSummary(s); + if (summary) rows.push(['Device', summary]); + if (d.device && !summary.startsWith(d.device)) rows.push(['Model', d.device]); + if (s.phone) rows.push(['Phone on file', s.phone]); + if (s.request_count) rows.push(['Songs submitted', String(s.request_count)]); + if (s.sources && s.sources.length) rows.push(['Sources', s.sources.join(', ')]); + if (s.first_request_at) { + const dt = new Date(s.first_request_at.replace(' ', 'T')); + rows.push(['First submitted', isNaN(dt) ? s.first_request_at : dt.toLocaleTimeString()]); + } + + const rowsHtml = rows.map(([k, v]) => + '
' + escHtml(k) + '' + + '' + escHtml(v) + '
').join(''); + + const rawHtml = d.raw + ? '
Raw User-Agent
' + + '
' + escHtml(d.raw) + '
' + : '
No User-Agent captured for this singer’s device ' + + '(submitted before device capture shipped, or KJ-added).
'; + + backdrop.innerHTML = + '
' + + '
' + + '

' + escHtml(singer.name) + ' — device

' + + '' + + '
' + + '
' + + '
' + escHtml(origin.label) + '
' + + (rowsHtml || '
No device details captured.
') + + rawHtml + + '
' + + '
'; + backdrop.querySelector('.singer-device-modal-close').onclick = closeSingerDeviceModal; - if (others.length === 0) return; - - const dropdown = document.createElement('div'); - dropdown.className = 'singer-merge-dropdown'; - others.forEach(targetName => { - const opt = document.createElement('button'); - opt.className = 'singer-merge-option'; - opt.textContent = targetName; - opt.onclick = () => { - dropdown.remove(); - singerAction('merge', { source_name: singer.name, target_name: targetName }); + document.body.appendChild(backdrop); + openSingerDeviceName = singer.name; + document.addEventListener('keydown', singerDeviceEscHandler); +} + +// --- Merge modal --------------------------------------------------------- +// Replaces the old inline dropdown: a real modal with a searchable/scrollable +// singer list and an explicit confirm step that spells out exactly which singer +// is kept and what history moves. +let mergeModalState = null; +function closeMergeModal() { + mergeModalState = null; + document.querySelectorAll('.merge-modal-backdrop').forEach(d => d.remove()); + document.removeEventListener('keydown', mergeModalEscHandler); +} +function mergeModalEscHandler(e) { if (e.key === 'Escape') closeMergeModal(); } + +function singerByName(name) { + const lower = (name || '').toLowerCase(); + return (singerStatsData || []).find(s => s.name.toLowerCase() === lower) || null; +} + +function openMergeModal(singer) { + closeMergeModal(); + mergeModalState = { origin: singer.name, partner: null, keep: null, query: '' }; + + const backdrop = document.createElement('div'); + backdrop.className = 'merge-modal-backdrop'; + backdrop.onclick = (ev) => { if (ev.target === backdrop) closeMergeModal(); }; + + const modal = document.createElement('div'); + modal.className = 'merge-modal'; + backdrop.appendChild(modal); + document.body.appendChild(backdrop); + + document.addEventListener('keydown', mergeModalEscHandler); + renderMergeModal(); +} + +// Small badge string for a singer option/summary (device + counts + status). +function singerBadgesHtml(s) { + if (!s) return ''; + const b = []; + if (s.session && s.session.has_device) b.push('📱 device'); + else if (s.session && s.session.origin === 'duet_partner') b.push('duet label'); + else b.push('KJ-added'); + b.push('' + (s.entries_sung || 0) + ' sung'); + b.push('' + (s.entries_waiting || 0) + ' queued'); + if (s.status && s.status !== 'active') b.push('' + escHtml(s.status) + ''); + return b.join(' '); +} + +// Fill the step-1 option list from the current search query, binding each +// option's click. Called on open and on every keystroke (list only — the +// search input keeps focus + caret position). +function renderMergeOptionList() { + const modal = document.querySelector('.merge-modal'); + if (!modal || !mergeModalState) return; + const st = mergeModalState; + const origin = singerByName(st.origin); + if (!origin) return; + const list = modal.querySelector('.merge-option-list'); + if (!list) return; + + const q = st.query.trim().toLowerCase(); + const others = (singerStatsData || []) + .filter(s => s.name.toLowerCase() !== st.origin.toLowerCase()) + .filter(s => !q || s.name.toLowerCase().includes(q)) + // Real-device sessions first (most likely merge intent), then by name. + .sort((a, b) => { + const ad = (a.session && a.session.has_device) ? 0 : 1; + const bd = (b.session && b.session.has_device) ? 0 : 1; + if (ad !== bd) return ad - bd; + return a.name.localeCompare(b.name); + }); + + list.innerHTML = others.map(s => + '').join('') || '
No matching singers.
'; + + list.querySelectorAll('.merge-option').forEach(btn => { + btn.onclick = () => { + st.partner = btn.getAttribute('data-name'); + // Default keeper: the phone-linked singer when exactly one has a + // device (the KJ's stated preference); else the picked partner. + const p = singerByName(st.partner); + const oDev = !!(origin.session && origin.session.has_device); + const pDev = !!(p && p.session && p.session.has_device); + if (oDev && !pDev) st.keep = origin.name; + else st.keep = st.partner; + renderMergeModal(); }; - dropdown.appendChild(opt); }); +} - const btn = ev.currentTarget; - const rect = btn.getBoundingClientRect(); - dropdown.style.top = (rect.bottom + 2) + 'px'; - dropdown.style.left = rect.left + 'px'; - document.body.appendChild(dropdown); +function renderMergeModal() { + const modal = document.querySelector('.merge-modal'); + if (!modal || !mergeModalState) return; + const st = mergeModalState; + const origin = singerByName(st.origin); + if (!origin) { closeMergeModal(); return; } + + if (!st.partner) { + // Step 1 — pick who to merge with. Static shell rendered once; only the + // option list re-renders on search input so the caret never jumps. + modal.innerHTML = + '
' + + '

Merge “' + escHtml(origin.name) + '”

' + + '' + + '
' + + '
' + escHtml(origin.name) + ' ' + singerBadgesHtml(origin) + '
' + + '' + + '
' + + '
Pick the singer this person should be combined with. ' + + 'You’ll confirm which name is kept next.
'; + + modal.querySelector('.merge-modal-close').onclick = closeMergeModal; + const search = modal.querySelector('.merge-search'); + search.oninput = () => { st.query = search.value; renderMergeOptionList(); }; + renderMergeOptionList(); + search.focus(); + return; + } - const close = (e) => { - if (!dropdown.contains(e.target)) { - dropdown.remove(); - document.removeEventListener('click', close); - } + // Step 2 — confirm direction. + const partner = singerByName(st.partner); + if (!partner) { st.partner = null; st.keep = null; renderMergeModal(); return; } + const keepName = st.keep; + const removeName = (keepName.toLowerCase() === origin.name.toLowerCase()) ? partner.name : origin.name; + const keep = singerByName(keepName); + const remove = singerByName(removeName); + + // Post-merge totals from the UNION of both singers' entries deduped by id — + // matches what the backend produces (merge reassigns entries; an entry both + // singers were credited on, e.g. a shared duet, collapses to one). A naive + // A+B sum would over-count that overlap case. + const mergedById = {}; + for (const e of (remove.entries || [])) mergedById[e.id] = e; + for (const e of (keep.entries || [])) mergedById[e.id] = e; + const mergedEntries = Object.values(mergedById); + const isDone = (e) => (e.status || '').toLowerCase().includes('done'); + const isLeft = (e) => (e.status || '').toLowerCase().includes('left'); + const combinedSung = mergedEntries.filter(isDone).length; + const combinedQueued = mergedEntries.filter(e => !isDone(e) && !isLeft(e)).length; + + const keepDev = !!(keep.session && keep.session.has_device); + const removeDev = !!(remove.session && remove.session.has_device); + let warnHtml = ''; + if (removeDev && !keepDev) { + warnHtml = '
⚠️ You’re removing ' + escHtml(remove.name) + + ', who has a real phone session, and keeping ' + escHtml(keep.name) + + ', who doesn’t. You usually want to keep the phone-linked singer — use Swap.
'; + } + let deviceNoteHtml = ''; + if (keepDev) { + deviceNoteHtml = '
' + escHtml(keep.name) + + ' has a linked phone session, so they’ll now see ' + escHtml(remove.name) + + '’s songs in their history (and any SMS/notifications stay on their number).
'; + } + + modal.innerHTML = + '
' + + '

Confirm merge

' + + '' + + '
' + + '
' + + '
' + + '
✓ KEEP
' + + '
' + escHtml(keep.name) + '
' + + '
' + singerBadgesHtml(keep) + '
' + + '
' + + '
← merges in
' + + '
' + + '
✗ REMOVE
' + + '
' + escHtml(remove.name) + '
' + + '
' + singerBadgesHtml(remove) + '
' + + '
' + + '
' + + warnHtml + + '
After merging, ' + escHtml(keep.name) + + ' will have ' + combinedSung + ' sung and ' + + combinedQueued + ' queued. The name “' + escHtml(remove.name) + + '” disappears from the list.
' + + deviceNoteHtml + + '
' + + '' + + '' + + '' + + '
'; + + modal.querySelector('.merge-modal-close').onclick = closeMergeModal; + modal.querySelector('.merge-btn-back').onclick = () => { st.partner = null; st.keep = null; renderMergeModal(); }; + modal.querySelector('.merge-btn-swap').onclick = () => { st.keep = remove.name; renderMergeModal(); }; + modal.querySelector('.merge-btn-confirm').onclick = () => { + closeMergeModal(); + singerAction('merge', { source_name: remove.name, target_name: keep.name }); }; - setTimeout(() => document.addEventListener('click', close), 0); } function enterRotationEditMode(row, entry, focusTarget) { diff --git a/kj-controller/static/style.css b/kj-controller/static/style.css index 25d2c20..961753d 100644 --- a/kj-controller/static/style.css +++ b/kj-controller/static/style.css @@ -1511,6 +1511,223 @@ body { color: #ffdf6b; } +/* --- Device icon on real-session singers --- */ +.singer-device-icon { + background: none; + border: none; + padding: 0 2px; + margin: 0; + cursor: pointer; + font-size: 0.85em; + line-height: 1; + filter: grayscale(0.15); + opacity: 0.9; +} +.singer-device-icon:hover { opacity: 1; filter: none; transform: scale(1.15); } + +/* --- Device details modal --- */ +.singer-device-modal-backdrop, +.merge-modal-backdrop { + position: fixed; + inset: 0; + background: rgba(0, 0, 0, 0.6); + display: flex; + align-items: center; + justify-content: center; + z-index: 300; +} +.singer-device-modal, +.merge-modal { + background: #1e1e1e; + border: 1px solid #444; + border-radius: 10px; + width: min(460px, 92vw); + max-height: 85vh; + overflow-y: auto; + box-shadow: 0 12px 40px rgba(0, 0, 0, 0.5); +} +.singer-device-modal-head, +.merge-modal-head { + display: flex; + align-items: center; + justify-content: space-between; + padding: 0.75rem 1rem; + border-bottom: 1px solid #333; +} +.singer-device-modal-head h3, +.merge-modal-head h3 { margin: 0; font-size: 1.05rem; color: #ffdf6b; } +.singer-device-modal-close, +.merge-modal-close { + background: none; + border: none; + color: #888; + font-size: 1.4rem; + line-height: 1; + cursor: pointer; +} +.singer-device-modal-close:hover, +.merge-modal-close:hover { color: #ff5bb8; } +.singer-device-modal-body { padding: 0.85rem 1rem 1.1rem; } +.device-origin { + display: inline-block; + font-size: 0.8em; + font-weight: 600; + padding: 3px 9px; + border-radius: 6px; + margin-bottom: 0.75rem; +} +.device-origin.origin-device { background: #14351f; color: #6ee7a0; border: 1px solid #2e7d4f; } +.device-origin.origin-duet { background: #3a2f14; color: #f5c869; border: 1px solid #7d6a2e; } +.device-origin.origin-kj { background: #2a2a2a; color: #aaa; border: 1px solid #555; } +.device-kv { + display: flex; + justify-content: space-between; + gap: 1rem; + padding: 4px 0; + border-bottom: 1px solid #2a2a2a; + font-size: 0.85em; +} +.device-k { color: #888; } +.device-v { color: #ddd; text-align: right; word-break: break-word; } +.device-empty { color: #888; font-size: 0.85em; padding: 6px 0; } +.device-ua-label { color: #777; font-size: 0.72em; margin: 0.85rem 0 0.3rem; } +.device-ua { + background: #121212; + border: 1px solid #333; + border-radius: 6px; + padding: 8px; + color: #9fd3ff; + font-size: 0.72em; + white-space: pre-wrap; + word-break: break-all; + margin: 0; +} + +/* --- Merge modal --- */ +.merge-modal-sub { + padding: 0.5rem 1rem 0; + font-size: 0.8em; + color: #ccc; + display: flex; + align-items: center; + flex-wrap: wrap; + gap: 4px; +} +.merge-search { + margin: 0.7rem 1rem 0.4rem; + width: calc(100% - 2rem); + box-sizing: border-box; + background: #121212; + border: 1px solid #444; + border-radius: 6px; + color: #eee; + padding: 7px 10px; + font-size: 0.9em; +} +.merge-option-list { + max-height: 42vh; + overflow-y: auto; + padding: 0 0.5rem; +} +.merge-option { + display: flex; + flex-direction: column; + align-items: flex-start; + gap: 3px; + width: 100%; + text-align: left; + background: #262626; + border: 1px solid #383838; + border-radius: 7px; + color: #eee; + padding: 8px 10px; + margin: 4px 0; + cursor: pointer; +} +.merge-option:hover { background: #33302a; border-color: #7d6a2e; } +.merge-option-name { font-weight: 600; color: #ffdf6b; font-size: 0.92em; } +.merge-option-badges { display: flex; flex-wrap: wrap; gap: 4px; } +.merge-badge { + font-size: 0.68em; + padding: 1px 6px; + border-radius: 5px; + background: #333; + color: #bbb; + white-space: nowrap; +} +.merge-badge-device { background: #14351f; color: #6ee7a0; } +.merge-badge-duet { background: #3a2f14; color: #f5c869; } +.merge-badge-kj { background: #2a2a2a; color: #999; } +.merge-badge-status { background: #2a2540; color: #b3a8e0; } +.merge-hint, .merge-empty { + padding: 0.6rem 1rem 1rem; + font-size: 0.78em; + color: #888; +} +.merge-confirm { + display: flex; + align-items: stretch; + gap: 0.5rem; + padding: 1rem; +} +.merge-side { + flex: 1; + border-radius: 8px; + padding: 0.7rem; + border: 1px solid #444; + background: #232323; +} +.merge-keep { border-color: #2e7d4f; background: #14290f; } +.merge-remove { border-color: #7d3a3a; background: #2a1414; opacity: 0.92; } +.merge-side-tag { font-size: 0.7em; font-weight: 700; letter-spacing: 0.03em; } +.merge-keep .merge-side-tag { color: #6ee7a0; } +.merge-remove .merge-side-tag { color: #f0857f; } +.merge-side-name { font-size: 1em; font-weight: 700; color: #ffdf6b; margin: 3px 0 5px; word-break: break-word; } +.merge-side-badges { display: flex; flex-wrap: wrap; gap: 3px; } +.merge-arrow { align-self: center; color: #888; font-size: 0.72em; white-space: nowrap; } +.merge-warn { + margin: 0 1rem 0.6rem; + padding: 0.6rem 0.75rem; + background: #3a2a14; + border: 1px solid #a06a2e; + border-radius: 7px; + color: #f5c869; + font-size: 0.8em; + line-height: 1.4; +} +.merge-summary { + margin: 0 1rem 0.5rem; + font-size: 0.82em; + color: #ddd; + line-height: 1.45; +} +.merge-note { + margin: 0 1rem 0.5rem; + font-size: 0.78em; + color: #9fd3ff; + line-height: 1.4; +} +.merge-actions { + display: flex; + flex-wrap: wrap; + gap: 0.5rem; + padding: 0.6rem 1rem 1rem; + justify-content: flex-end; +} +.merge-actions button { + border-radius: 6px; + padding: 8px 12px; + font-size: 0.82em; + cursor: pointer; + border: 1px solid #444; +} +.merge-btn-back { background: #2a2a2a; color: #aaa; } +.merge-btn-back:hover { background: #363636; color: #ddd; } +.merge-btn-swap { background: #2a2540; color: #b3a8e0; border-color: #4a4270; } +.merge-btn-swap:hover { background: #342d52; } +.merge-btn-confirm { background: #1f5c38; color: #eafff2; border-color: #2e7d4f; font-weight: 600; } +.merge-btn-confirm:hover { background: #267045; } + /* --- Responsive --- */ @media (max-width: 1024px) { diff --git a/kj-controller/tests/e2e/test_singer_stats_e2e.py b/kj-controller/tests/e2e/test_singer_stats_e2e.py index 2522bac..4a14cc1 100644 --- a/kj-controller/tests/e2e/test_singer_stats_e2e.py +++ b/kj-controller/tests/e2e/test_singer_stats_e2e.py @@ -178,3 +178,48 @@ def test_split_kai_into_kai_p(self, app_page): ) assert page.locator('.singer-stats-name', has_text='KaiSplit').count() > 0 assert page.locator('.singer-stats-name', has_text='KaiSplit P').count() > 0 + + +class TestMergeModalE2E: + def test_merge_modal_flow_and_no_device_icon_for_kj_added(self, app_page): + page = app_page + + # Two KJ-added singers so Merge has a target. + page.locator('.rotation-add-btn').click() + page.locator('#singer-input-container').wait_for(state='visible') + for singer, song in (('MergeA', 'Song A'), ('MergeB', 'Song B')): + page.locator('#rotation-singer').fill(singer) + page.locator('#rotation-singer').press('Enter') + page.locator('#rotation-song').fill(song) + page.locator('#rotation-add-btn-submit').click() + page.locator('.rotation-entry', has_text=singer).last.wait_for(state='visible') + + row_a = page.locator('.singer-stats-row', has_text='MergeA').first + row_a.wait_for(state='visible', timeout=15000) + + # KJ-added singers have no linked device session → no 📱 icon. + assert row_a.locator('.singer-device-icon').count() == 0 + + # Merge opens the new modal (not the old inline dropdown). + row_a.locator('.singer-stats-btn', has_text='Merge').click() + page.locator('.merge-modal').wait_for(state='visible', timeout=5000) + assert page.locator('.merge-search').count() == 1 + + # Pick MergeB as the partner → confirm step spells out keep/remove. + # Both are KJ-added (no device), so the default keeper is the picked + # partner, MergeB. + page.locator('.merge-option', has_text='MergeB').first.click() + page.locator('.merge-confirm').wait_for(state='visible', timeout=5000) + assert 'MergeB' in page.locator('.merge-keep .merge-side-name').inner_text() + + # Swap flips the keeper to MergeA (removing MergeB instead). + page.locator('.merge-btn-swap').click() + assert 'MergeA' in page.locator('.merge-keep .merge-side-name').inner_text() + + # Confirm performs the merge. Assert the real outcome — MergeB is absorbed + # into MergeA, so only MergeA's name remains in the singer list. + page.locator('.merge-btn-confirm').click() + page.locator('.merge-modal').wait_for(state='hidden', timeout=5000) + page.locator('.singer-stats-name', has_text='MergeB').first.wait_for( + state='detached', timeout=10000) + assert page.locator('.singer-stats-name', has_text='MergeA').count() > 0 diff --git a/kj-controller/tests/integration/test_sing_public_routes.py b/kj-controller/tests/integration/test_sing_public_routes.py index 7aa95d7..0654855 100644 --- a/kj-controller/tests/integration/test_sing_public_routes.py +++ b/kj-controller/tests/integration/test_sing_public_routes.py @@ -195,6 +195,17 @@ def test_happy_path_creates_pending_request(self, client, sing_app, token): # Stored assert sing_app.sing_store.count_pending() == 1 + def test_submit_captures_user_agent(self, client, sing_app, token): + ua = ("Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) " + "AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Mobile/15E148 Safari/604.1") + resp = client.post( + f"/sing/submit?t={token}", json=self._body(), + headers={"User-Agent": ua}, + ) + assert resp.status_code == 200 + req_id = resp.get_json()["request"]["id"] + assert sing_app.sing_store.get_request(req_id)["user_agent"] == ua + def test_missing_singer_name(self, client, token): resp = client.post(f"/sing/submit?t={token}", json=self._body(singer_name="")) assert resp.status_code == 400 diff --git a/kj-controller/tests/unit/test_sing_store.py b/kj-controller/tests/unit/test_sing_store.py index 03dfe9d..1ab9a31 100644 --- a/kj-controller/tests/unit/test_sing_store.py +++ b/kj-controller/tests/unit/test_sing_store.py @@ -44,6 +44,7 @@ def test_sing_requests_columns(self, store): "song_artist", "song_title", "source_type", "source_ref", "source_meta", "notes", "status", "rejected_reason", "reviewed_at", "linked_entry_id", "additional_singers", + "user_agent", } assert expected <= cols @@ -801,3 +802,41 @@ def test_toggle_persists_across_instances(self, tmp_path): s1.set_auto_reorder(False) assert SingStore(db).is_auto_reorder() is False s1.close() + + +class TestUserAgentAndSessionLookup: + def test_user_agent_persisted(self, store): + ua = "Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) Safari/604.1" + r = store.create_request( + singer_name="Chaila R", phone="", source_type="kn", + source_ref="https://youtu.be/x", user_agent=ua, + ) + assert store.get_request(r["id"])["user_agent"] == ua + + def test_user_agent_defaults_none(self, store): + r = store.create_request(singer_name="Al", phone="", source_type="local", source_ref="/a.mp4") + assert store.get_request(r["id"])["user_agent"] is None + + def test_get_requests_for_entries_night_scoped(self, store): + # Mark the night as started in the past so today's rows are in-scope. + store._set_meta("night_started_at", "2000-01-01 00:00:00") + r = store.create_request( + singer_name="Chaila R", phone="808", source_type="kn", + source_ref="https://youtu.be/x", user_agent="UA-1", + ) + store.mark_approved(r["id"], linked_entry_id=711) + + got = store.get_requests_for_entries([711], store.get_night_started_at()) + assert len(got) == 1 + assert got[0]["linked_entry_id"] == 711 + assert got[0]["singer_name"] == "Chaila R" + assert got[0]["user_agent"] == "UA-1" + + def test_get_requests_for_entries_fails_closed_without_night(self, store): + r = store.create_request(singer_name="Al", phone="", source_type="local", source_ref="/a.mp4") + store.mark_approved(r["id"], linked_entry_id=5) + # No night_started passed → fails closed (empty), never phantom-matches. + assert store.get_requests_for_entries([5], None) == [] + + def test_get_requests_for_entries_empty_ids(self, store): + assert store.get_requests_for_entries([], "2000-01-01 00:00:00") == [] diff --git a/kj-controller/tests/unit/test_singer_session_info.py b/kj-controller/tests/unit/test_singer_session_info.py new file mode 100644 index 0000000..5a17d1c --- /dev/null +++ b/kj-controller/tests/unit/test_singer_session_info.py @@ -0,0 +1,109 @@ +"""Unit tests for routes._add_singer_session_info — singer provenance. + +Reproduces the real "two Chailas" incident: a self-registered singer +("Chaila R", own device) vs a duet-partner label ("Chaila", typed into +Ashlee's submission) vs a KJ-hand-added singer (no request). +""" + +import types + +from sing_store import SingStore +import routes + + +def _seed_store(): + store = SingStore(":memory:") + # Night started in the past so today's rows are in-scope. + store._set_meta("night_started_at", "2000-01-01 00:00:00") + + # Ashlee A submits a duet crediting "Chaila" (no own device) → entry 706. + r_ashlee = store.create_request( + singer_name="Ashlee A", phone="8082177808", source_type="local", + source_ref="/grenade.zip", additional_singers=[{"name": "Chaila", "phone": ""}], + user_agent="UA-ashlee-iphone", + ) + store.mark_approved(r_ashlee["id"], linked_entry_id=706) + + # Chaila registers herself as "Chaila R" and submits a solo → entry 711. + r_chaila = store.create_request( + singer_name="Chaila R", phone="", source_type="kn", + source_ref="https://youtu.be/x", user_agent="UA-chaila-android", + ) + store.mark_approved(r_chaila["id"], linked_entry_id=711) + return store + + +def _fake_app(store): + return types.SimpleNamespace(sing_store=store) + + +def _singer_stats(): + # Mirrors get_singer_stats(): the duet entry 706 is credited to BOTH + # Ashlee A and Chaila; Chaila R owns 711; Walter is KJ-added (999). + return [ + {"name": "Ashlee A", "entries": [{"id": 706}]}, + {"name": "Chaila", "entries": [{"id": 706}]}, + {"name": "Chaila R", "entries": [{"id": 711}]}, + {"name": "Walter", "entries": [{"id": 999}]}, + ] + + +def _by_name(stats): + return {s["name"]: s for s in stats} + + +class TestSessionClassification: + def test_self_registered_is_singer_ui_with_device(self): + store = _seed_store() + stats = _singer_stats() + routes._add_singer_session_info(stats, app=_fake_app(store)) + chaila_r = _by_name(stats)["Chaila R"]["session"] + assert chaila_r["origin"] == "singer_ui" + assert chaila_r["has_device"] is True + assert chaila_r["device"]["device"] == "Android device" or chaila_r["device"]["raw"] == "UA-chaila-android" + assert chaila_r["request_count"] == 1 + store.close() + + def test_duet_partner_label_has_no_device(self): + store = _seed_store() + stats = _singer_stats() + routes._add_singer_session_info(stats, app=_fake_app(store)) + chaila = _by_name(stats)["Chaila"]["session"] + assert chaila["origin"] == "duet_partner" + assert chaila["has_device"] is False + assert chaila["phone"] == "" + store.close() + + def test_primary_submitter_is_singer_ui(self): + store = _seed_store() + stats = _singer_stats() + routes._add_singer_session_info(stats, app=_fake_app(store)) + ashlee = _by_name(stats)["Ashlee A"]["session"] + assert ashlee["origin"] == "singer_ui" + assert ashlee["has_device"] is True + assert ashlee["phone"] == "8082177808" + store.close() + + def test_kj_added_has_no_request(self): + store = _seed_store() + stats = _singer_stats() + routes._add_singer_session_info(stats, app=_fake_app(store)) + walter = _by_name(stats)["Walter"]["session"] + assert walter["origin"] == "kj_added" + assert walter["has_device"] is False + store.close() + + def test_no_sing_store_is_noop(self): + stats = _singer_stats() + routes._add_singer_session_info(stats, app=types.SimpleNamespace(sing_store=None)) + assert all("session" not in s for s in stats) + + def test_missing_night_marker_fails_closed(self): + store = SingStore(":memory:") # no night_started set + r = store.create_request(singer_name="Chaila R", phone="", source_type="kn", source_ref="x") + store.mark_approved(r["id"], linked_entry_id=711) + stats = [{"name": "Chaila R", "entries": [{"id": 711}]}] + routes._add_singer_session_info(stats, app=_fake_app(store)) + # Fails closed → treated as KJ-added rather than phantom-matching. + assert stats[0]["session"]["origin"] == "kj_added" + store.close() diff --git a/kj-controller/tests/unit/test_ua_parse.py b/kj-controller/tests/unit/test_ua_parse.py new file mode 100644 index 0000000..c1dda42 --- /dev/null +++ b/kj-controller/tests/unit/test_ua_parse.py @@ -0,0 +1,102 @@ +"""Unit tests for ua_parse — best-effort User-Agent parsing.""" + +from ua_parse import parse_user_agent, summarize + +IOS_SAFARI = ( + "Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) " + "AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Mobile/15E148 Safari/604.1" +) +ANDROID_CHROME = ( + "Mozilla/5.0 (Linux; Android 14; SM-S911B) AppleWebKit/537.36 " + "(KHTML, like Gecko) Chrome/124.0.0.0 Mobile Safari/537.36" +) +ANDROID_SAMSUNG = ( + "Mozilla/5.0 (Linux; Android 13; SM-G991B) AppleWebKit/537.36 (KHTML, like Gecko) " + "SamsungBrowser/23.0 Chrome/115.0.0.0 Mobile Safari/537.36" +) +WIN_EDGE = ( + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/124.0.0.0 Safari/537.36 Edg/124.0.0.0" +) +MAC_FIREFOX = ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:126.0) Gecko/20100101 Firefox/126.0" +) +IPAD = ( + "Mozilla/5.0 (iPad; CPU OS 16_5 like Mac OS X) AppleWebKit/605.1.15 " + "(KHTML, like Gecko) Version/16.5 Mobile/15E148 Safari/604.1" +) + + +class TestParse: + def test_ios_safari(self): + p = parse_user_agent(IOS_SAFARI) + assert p["device"] == "iPhone" + assert p["browser"] == "Safari" + assert p["os"] == "iOS 17.4" + assert p["is_mobile"] is True + + def test_android_model_extracted(self): + p = parse_user_agent(ANDROID_CHROME) + assert p["device"] == "SM-S911B" + assert p["browser"] == "Chrome" + assert p["os"] == "Android 14" + assert p["is_mobile"] is True + + def test_samsung_browser_before_chrome(self): + # UA contains both "SamsungBrowser" and "Chrome" — Samsung must win. + p = parse_user_agent(ANDROID_SAMSUNG) + assert p["browser"] == "Samsung Internet" + assert p["device"] == "SM-G991B" + + def test_edge_before_chrome(self): + p = parse_user_agent(WIN_EDGE) + assert p["browser"] == "Edge" + assert p["os"] == "Windows 10/11" + assert p["is_mobile"] is False + + def test_edge_for_android(self): + # EdgA/ (Edge on Android) also contains Chrome/ — Edge must win. + ua = ("Mozilla/5.0 (Linux; Android 13; Pixel 7) AppleWebKit/537.36 " + "(KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 EdgA/120.0.0.0") + p = parse_user_agent(ua) + assert p["browser"] == "Edge" + assert p["device"] == "Pixel 7" + assert p["is_mobile"] is True + + def test_mac_firefox_keeps_full_version(self): + p = parse_user_agent(MAC_FIREFOX) + assert p["browser"] == "Firefox" + assert p["os"] == "macOS 10.15" + assert p["device"] == "Mac" + + def test_ipad(self): + p = parse_user_agent(IPAD) + assert p["device"] == "iPad" + assert p["os"] == "iOS 16.5" + + def test_empty_and_none(self): + for ua in ("", None, " "): + p = parse_user_agent(ua) + assert p == {"browser": "", "os": "", "device": "", "is_mobile": False, "raw": ""} + + def test_raw_preserved(self): + assert parse_user_agent(IOS_SAFARI)["raw"] == IOS_SAFARI + + def test_junk_does_not_crash(self): + p = parse_user_agent("not a real user agent !!! 123") + assert p["raw"] == "not a real user agent !!! 123" + + def test_embedded_webkit_not_tagged_safari(self): + # An in-app WebView exposes a bare Safari/ token but no Version/ — must + # NOT be classified as Safari. + ua = ("Mozilla/5.0 (iPhone; CPU iPhone OS 17_4 like Mac OS X) " + "AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 Safari/604.1") + assert parse_user_agent(ua)["browser"] == "" + + +class TestSummarize: + def test_summary_dedupes_and_orders(self): + assert summarize(IOS_SAFARI) == "iPhone · Safari · iOS 17.4" + + def test_summary_empty(self): + assert summarize("") == "" diff --git a/kj-controller/ua_parse.py b/kj-controller/ua_parse.py new file mode 100644 index 0000000..3eddf42 --- /dev/null +++ b/kj-controller/ua_parse.py @@ -0,0 +1,122 @@ +"""Best-effort User-Agent parsing for singer session provenance. + +No third-party dependency — a compact heuristic parser that extracts the +browser, OS, and (where the UA exposes it) device model from a raw User-Agent +string. Used by the KJ Singers list to show a device-details popup so the KJ can +tell a real singer-UI session apart from a duet-partner label / KJ-added entry. + +Deliberately conservative: iOS UAs never expose the concrete model ("iPhone" +only), and modern Chromium freezes the version, so ``device`` is a friendly +label, not a guarantee. The raw UA is always kept for the KJ to eyeball. +""" + +import re + +_ANDROID_MODEL_RE = re.compile(r"Android [\d.]+;\s*(?:[a-z]{2}-[a-z]{2};\s*)?([^;)]+?)(?:\s+Build|;|\))", re.I) +_OS_VERSION_CLEAN_RE = re.compile(r"[_]") + + +def _os(ua): + """Return a friendly OS label, e.g. 'iOS 17.4', 'Android 14', 'Windows'.""" + m = re.search(r"iPhone OS ([\d_.]+)", ua) or re.search(r"CPU OS ([\d_.]+)", ua) + if m: + return "iOS " + _OS_VERSION_CLEAN_RE.sub(".", m.group(1)) + m = re.search(r"Android ([\d.]+)", ua) + if m: + return "Android " + m.group(1) + if "Windows NT 10.0" in ua: + return "Windows 10/11" + if "Windows" in ua: + return "Windows" + m = re.search(r"Mac OS X ([\d_.]+)", ua) + if m: + return "macOS " + _OS_VERSION_CLEAN_RE.sub(".", m.group(1)) + if "CrOS" in ua: + return "ChromeOS" + if "Linux" in ua: + return "Linux" + return "" + + +def _browser(ua): + """Return a friendly browser label. Order matters (Chrome UAs mention Safari).""" + # Edge, Samsung, Opera, Firefox must be checked before Chrome/Safari because + # their UAs also contain "Chrome"/"Safari" tokens. + for token, label in ( + ("Edg/", "Edge"), + ("EdgiOS/", "Edge"), + ("EdgA/", "Edge"), + ("SamsungBrowser/", "Samsung Internet"), + ("OPR/", "Opera"), + ("OPiOS/", "Opera"), + ("FxiOS/", "Firefox"), + ("Firefox/", "Firefox"), + ("CriOS/", "Chrome"), + ("Chrome/", "Chrome"), + ): + if token in ua: + return label + # Safari last: a real Safari emits the "Version/… Safari/" sequence. Requiring + # Version/ avoids tagging embedded WebKit clients (which expose a bare Safari + # compatibility token but no Version/) as Safari. + if "Safari/" in ua and "Version/" in ua: + return "Safari" + return "" + + +def _device(ua, os_label): + """Return a friendly device label, extracting an Android model when present.""" + if "iPhone" in ua: + return "iPhone" + if "iPad" in ua: + return "iPad" + if "iPod" in ua: + return "iPod" + if "Android" in ua: + m = _ANDROID_MODEL_RE.search(ua) + if m: + model = m.group(1).strip() + # Strip a trailing locale/UA-noise and obvious non-models. + if model and model.lower() not in ("wv", "mobile", "k"): + return model + return "Android device" + if os_label.startswith("Windows"): + return "Windows PC" + if os_label.startswith("macOS"): + return "Mac" + if os_label == "ChromeOS": + return "Chromebook" + if os_label == "Linux": + return "Linux PC" + return "" + + +def parse_user_agent(ua): + """Parse a raw UA string into a small dict. + + Returns ``{browser, os, device, is_mobile, raw}``. Empty/None input yields a + dict with empty fields (and ``raw=''``) so callers never see None. + """ + ua = (ua or "").strip() + if not ua: + return {"browser": "", "os": "", "device": "", "is_mobile": False, "raw": ""} + os_label = _os(ua) + return { + "browser": _browser(ua), + "os": os_label, + "device": _device(ua, os_label), + "is_mobile": ("Mobi" in ua) or ("Android" in ua) or ("iPhone" in ua) or ("iPad" in ua), + "raw": ua, + } + + +def summarize(ua): + """One-line human summary e.g. 'iPhone · Safari · iOS 17.4'. '' if unknown.""" + p = parse_user_agent(ua) + parts = [x for x in (p["device"], p["browser"], p["os"]) if x] + # Dedupe consecutive duplicates (e.g. device == os label) while keeping order. + seen = [] + for x in parts: + if x not in seen: + seen.append(x) + return " · ".join(seen)