mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-29 09:23:16 -06:00
fix(server): persist + ship + real-time per-item mute (#129)
The dashboard mute toggle was a no-op end to end. The active model is playlist_items
(the device payload is its published_snapshot); the legacy `assignments` table the bug
report cited is unused for devices. Three breaks:
- PUT /api/assignments/:id silently dropped `muted` (only read sort_order/duration_sec/
zone_id). It now accepts muted (coerced 0/1) and ITEM_SELECT returns it, so the toggle
persists and its on/off state sticks.
- playlist_items had no `muted` column — added (schema + idempotent migration).
- buildSnapshotItems didn't select muted, so it never reached the published_snapshot /
device payload — now included.
Real-time: on a mute change, emit device:mute-changed { content_id, widget_id, muted } to
every device on that playlist so the player toggles the matching item's volume live,
decoupled from publish (the value is also in the next snapshot, so it persists). Adds a
[mute] log line (the report noted zero mute log entries).
Test: test/mute.test.js — PUT persists + returns muted, it reaches the published
snapshot, and a non-mute update doesn't reset it. Server suite 164/164.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
965920cd17
commit
332f4c5b5c
|
|
@ -148,6 +148,10 @@ const migrations = [
|
||||||
// playlist_items conversion (migrateAssignmentsToPlaylists) dropped this
|
// playlist_items conversion (migrateAssignmentsToPlaylists) dropped this
|
||||||
// column. Column ADD is idempotent via the surrounding try/catch loop.
|
// column. Column ADD is idempotent via the surrounding try/catch loop.
|
||||||
"ALTER TABLE playlist_items ADD COLUMN zone_id TEXT REFERENCES layout_zones(id) ON DELETE SET NULL",
|
"ALTER TABLE playlist_items ADD COLUMN zone_id TEXT REFERENCES layout_zones(id) ON DELETE SET NULL",
|
||||||
|
// #129: per-item mute. The legacy `assignments` table had a muted column, but the
|
||||||
|
// active device payload is built from playlist_items -> published_snapshot, which never
|
||||||
|
// carried it, so the dashboard mute toggle was a no-op end to end.
|
||||||
|
"ALTER TABLE playlist_items ADD COLUMN muted INTEGER NOT NULL DEFAULT 0",
|
||||||
// Slice 1: idempotency guard for the one-time signup welcome/admin emails.
|
// Slice 1: idempotency guard for the one-time signup welcome/admin emails.
|
||||||
// Non-null = this user has already been handled, so we never double-send.
|
// Non-null = this user has already been handled, so we never double-send.
|
||||||
// New signups are stamped with the real unix-seconds time the send block ran
|
// New signups are stamped with the real unix-seconds time the send block ran
|
||||||
|
|
|
||||||
|
|
@ -367,6 +367,7 @@ CREATE TABLE IF NOT EXISTS playlist_items (
|
||||||
zone_id TEXT REFERENCES layout_zones(id) ON DELETE SET NULL,
|
zone_id TEXT REFERENCES layout_zones(id) ON DELETE SET NULL,
|
||||||
sort_order INTEGER NOT NULL DEFAULT 0,
|
sort_order INTEGER NOT NULL DEFAULT 0,
|
||||||
duration_sec INTEGER NOT NULL DEFAULT 10,
|
duration_sec INTEGER NOT NULL DEFAULT 10,
|
||||||
|
muted INTEGER NOT NULL DEFAULT 0,
|
||||||
created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')),
|
created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')),
|
||||||
updated_at INTEGER NOT NULL DEFAULT (strftime('%s','now'))
|
updated_at INTEGER NOT NULL DEFAULT (strftime('%s','now'))
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -44,7 +44,7 @@ function ensureDevicePlaylist(deviceId, userId) {
|
||||||
|
|
||||||
// Standard item query with joined content/widget info
|
// Standard item query with joined content/widget info
|
||||||
const ITEM_SELECT = `
|
const ITEM_SELECT = `
|
||||||
SELECT pi.id, pi.playlist_id, pi.content_id, pi.widget_id, pi.zone_id, pi.sort_order, pi.duration_sec,
|
SELECT pi.id, pi.playlist_id, pi.content_id, pi.widget_id, pi.zone_id, pi.sort_order, pi.duration_sec, pi.muted,
|
||||||
pi.created_at, pi.updated_at,
|
pi.created_at, pi.updated_at,
|
||||||
COALESCE(c.filename, w.name) as filename,
|
COALESCE(c.filename, w.name) as filename,
|
||||||
c.mime_type, c.filepath, c.thumbnail_path,
|
c.mime_type, c.filepath, c.thumbnail_path,
|
||||||
|
|
@ -139,12 +139,28 @@ function checkItemWrite(req, res) {
|
||||||
return item;
|
return item;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #129: real-time mute. Tell every device on this playlist to toggle the volume of the
|
||||||
|
// matching currently-playing item NOW (decoupled from publish — the device matches by
|
||||||
|
// content_id/widget_id and applies it live). The new value is also written to the row, so
|
||||||
|
// it lands in the next published snapshot and persists across playlist reloads.
|
||||||
|
function emitMuteChanged(req, item, muted) {
|
||||||
|
try {
|
||||||
|
const io = req.app.get('io');
|
||||||
|
if (!io) return;
|
||||||
|
const deviceNs = io.of('/device');
|
||||||
|
const devices = db.prepare('SELECT id FROM devices WHERE playlist_id = ?').all(item.playlist_id);
|
||||||
|
const payload = { content_id: item.content_id || null, widget_id: item.widget_id || null, muted: !!muted };
|
||||||
|
for (const d of devices) deviceNs.to(d.id).emit('device:mute-changed', payload);
|
||||||
|
console.log(`[mute] item ${item.id} (content ${item.content_id || item.widget_id}) -> ${muted ? 'MUTED' : 'unmuted'}; notified ${devices.length} device(s)`);
|
||||||
|
} catch (e) { /* best-effort live toggle; the published snapshot is the source of truth */ }
|
||||||
|
}
|
||||||
|
|
||||||
// Update playlist item
|
// Update playlist item
|
||||||
router.put('/:id', (req, res) => {
|
router.put('/:id', (req, res) => {
|
||||||
const item = checkItemWrite(req, res);
|
const item = checkItemWrite(req, res);
|
||||||
if (!item) return;
|
if (!item) return;
|
||||||
|
|
||||||
const { sort_order, duration_sec, zone_id } = req.body;
|
const { sort_order, duration_sec, zone_id, muted } = req.body;
|
||||||
const updates = [];
|
const updates = [];
|
||||||
const values = [];
|
const values = [];
|
||||||
|
|
||||||
|
|
@ -153,12 +169,17 @@ router.put('/:id', (req, res) => {
|
||||||
// zone_id can be null (clear the zone) - treat undefined as "no change",
|
// zone_id can be null (clear the zone) - treat undefined as "no change",
|
||||||
// any other value (including null) as "write this".
|
// any other value (including null) as "write this".
|
||||||
if (zone_id !== undefined) { updates.push('zone_id = ?'); values.push(zone_id || null); }
|
if (zone_id !== undefined) { updates.push('zone_id = ?'); values.push(zone_id || null); }
|
||||||
|
// #129: per-item mute (coerced to 0/1). Was silently dropped here before, so the
|
||||||
|
// dashboard toggle did nothing.
|
||||||
|
const mutedChanged = muted !== undefined && (item.muted ? 1 : 0) !== (muted ? 1 : 0);
|
||||||
|
if (muted !== undefined) { updates.push('muted = ?'); values.push(muted ? 1 : 0); }
|
||||||
|
|
||||||
if (updates.length > 0) {
|
if (updates.length > 0) {
|
||||||
updates.push("updated_at = strftime('%s','now')");
|
updates.push("updated_at = strftime('%s','now')");
|
||||||
values.push(req.params.id);
|
values.push(req.params.id);
|
||||||
db.prepare(`UPDATE playlist_items SET ${updates.join(', ')} WHERE id = ?`).run(...values);
|
db.prepare(`UPDATE playlist_items SET ${updates.join(', ')} WHERE id = ?`).run(...values);
|
||||||
markDraft(item.playlist_id);
|
markDraft(item.playlist_id);
|
||||||
|
if (mutedChanged) emitMuteChanged(req, item, muted ? 1 : 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
const updated = db.prepare(`${ITEM_SELECT} WHERE pi.id = ?`).get(req.params.id);
|
const updated = db.prepare(`${ITEM_SELECT} WHERE pi.id = ?`).get(req.params.id);
|
||||||
|
|
|
||||||
|
|
@ -66,7 +66,7 @@ function requirePlaylistWrite(req, res, next) {
|
||||||
// Build the snapshot item list for a playlist (denormalized for device payload)
|
// Build the snapshot item list for a playlist (denormalized for device payload)
|
||||||
function buildSnapshotItems(playlistId) {
|
function buildSnapshotItems(playlistId) {
|
||||||
const items = db.prepare(`
|
const items = db.prepare(`
|
||||||
SELECT pi.id AS _iid, pi.content_id, pi.widget_id, pi.zone_id, pi.sort_order, pi.duration_sec,
|
SELECT pi.id AS _iid, pi.content_id, pi.widget_id, pi.zone_id, pi.sort_order, pi.duration_sec, pi.muted,
|
||||||
COALESCE(c.filename, w.name) as filename, c.mime_type, c.filepath, c.file_size,
|
COALESCE(c.filename, w.name) as filename, c.mime_type, c.filepath, c.file_size,
|
||||||
c.duration_sec as content_duration, c.remote_url,
|
c.duration_sec as content_duration, c.remote_url,
|
||||||
w.name as widget_name, w.widget_type, w.config as widget_config
|
w.name as widget_name, w.widget_type, w.config as widget_config
|
||||||
|
|
|
||||||
100
server/test/mute.test.js
Normal file
100
server/test/mute.test.js
Normal file
|
|
@ -0,0 +1,100 @@
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
// #129 mute: the per-item `muted` flag must persist on PUT /api/assignments/:id, come
|
||||||
|
// back in the item read (ITEM_SELECT), and reach the device by being included in the
|
||||||
|
// playlist's published_snapshot (buildSnapshotItems). Before the fix the PUT silently
|
||||||
|
// dropped `muted`, playlist_items had no such column, and the snapshot never carried it —
|
||||||
|
// so the dashboard mute toggle was a no-op end to end.
|
||||||
|
|
||||||
|
const { test, before, after } = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const { spawn } = require('node:child_process');
|
||||||
|
const path = require('node:path');
|
||||||
|
const os = require('node:os');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const crypto = require('node:crypto');
|
||||||
|
const Database = require('better-sqlite3');
|
||||||
|
|
||||||
|
const PORT = 3994;
|
||||||
|
const BASE = `http://127.0.0.1:${PORT}`;
|
||||||
|
const DATA_DIR = path.join(os.tmpdir(), 'st-mute-test-' + crypto.randomBytes(4).toString('hex'));
|
||||||
|
const LOG = path.join(os.tmpdir(), 'st-mute-' + crypto.randomBytes(4).toString('hex') + '.log');
|
||||||
|
const PW = 'Passw0rd123';
|
||||||
|
let proc, db;
|
||||||
|
const S = {};
|
||||||
|
|
||||||
|
async function jfetch(p, opts = {}) {
|
||||||
|
const res = await fetch(BASE + p, opts);
|
||||||
|
let body = null; try { body = await res.json(); } catch { /* non-JSON */ }
|
||||||
|
return { status: res.status, body };
|
||||||
|
}
|
||||||
|
const auth = (tok) => ({ headers: { Authorization: 'Bearer ' + tok, 'Content-Type': 'application/json' } });
|
||||||
|
const post = (tok, obj) => ({ method: 'POST', ...auth(tok), body: JSON.stringify(obj || {}) });
|
||||||
|
const put = (tok, obj) => ({ method: 'PUT', ...auth(tok), body: JSON.stringify(obj || {}) });
|
||||||
|
|
||||||
|
before(async () => {
|
||||||
|
const logFd = fs.openSync(LOG, 'w');
|
||||||
|
proc = spawn('node', ['server.js'], {
|
||||||
|
cwd: path.join(__dirname, '..'),
|
||||||
|
env: { ...process.env, DATA_DIR, SELF_HOSTED: 'true', PORT: String(PORT), NODE_ENV: 'test' },
|
||||||
|
stdio: ['ignore', logFd, logFd],
|
||||||
|
});
|
||||||
|
let up = false;
|
||||||
|
for (let i = 0; i < 80; i++) {
|
||||||
|
try { const r = await fetch(BASE + '/api/status'); if (r.ok) { up = true; break; } } catch { /* not yet */ }
|
||||||
|
await new Promise((r) => setTimeout(r, 250));
|
||||||
|
}
|
||||||
|
if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
|
||||||
|
|
||||||
|
const reg = await jfetch('/api/auth/register', post(null, { email: 'm' + crypto.randomBytes(4).toString('hex') + '@x.local', password: PW }));
|
||||||
|
S.jwt = reg.body.token;
|
||||||
|
const pl = await jfetch('/api/playlists', post(S.jwt, { name: 'mute-pl' }));
|
||||||
|
S.playlistId = pl.body.id;
|
||||||
|
|
||||||
|
// Seed a content row + a playlist_item on a single connection (avoids WAL visibility
|
||||||
|
// races; FK off so a NULL-workspace content row is fine for the test).
|
||||||
|
db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db'), { timeout: 5000 });
|
||||||
|
db.pragma('foreign_keys = OFF');
|
||||||
|
S.contentId = crypto.randomUUID();
|
||||||
|
db.prepare("INSERT INTO content (id, filename, filepath, mime_type, file_size, remote_url) VALUES (?,?,?,?,0,?)")
|
||||||
|
.run(S.contentId, 'clip', '', 'video/mp4', 'https://example.com/clip.mp4');
|
||||||
|
const info = db.prepare('INSERT INTO playlist_items (playlist_id, content_id, sort_order, duration_sec) VALUES (?,?,0,10)')
|
||||||
|
.run(S.playlistId, S.contentId);
|
||||||
|
S.itemId = info.lastInsertRowid;
|
||||||
|
});
|
||||||
|
|
||||||
|
after(() => {
|
||||||
|
try { db.close(); } catch { /* */ }
|
||||||
|
try { proc.kill('SIGKILL'); } catch { /* */ }
|
||||||
|
for (const f of [DATA_DIR, LOG]) { try { fs.rmSync(f, { recursive: true, force: true }); } catch { /* */ } }
|
||||||
|
});
|
||||||
|
|
||||||
|
test('PUT /assignments/:id persists muted and returns it (ITEM_SELECT)', async () => {
|
||||||
|
const on = await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: true }));
|
||||||
|
assert.equal(on.status, 200);
|
||||||
|
assert.equal(on.body.muted, 1, 'muted persisted + returned as 1');
|
||||||
|
|
||||||
|
const off = await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: false }));
|
||||||
|
assert.equal(off.status, 200);
|
||||||
|
assert.equal(off.body.muted, 0, 'unmute persisted + returned as 0');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('muted reaches the device via the published snapshot (buildSnapshotItems)', async () => {
|
||||||
|
await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: true }));
|
||||||
|
const pub = await jfetch(`/api/playlists/${S.playlistId}/publish`, post(S.jwt, {}));
|
||||||
|
assert.equal(pub.status, 200);
|
||||||
|
|
||||||
|
const snapRow = db.prepare('SELECT published_snapshot FROM playlists WHERE id = ?').get(S.playlistId);
|
||||||
|
const snap = JSON.parse(snapRow.published_snapshot);
|
||||||
|
const item = snap.find((i) => i.content_id === S.contentId);
|
||||||
|
assert.ok(item, 'the item is in the published snapshot');
|
||||||
|
assert.equal(item.muted, 1, 'snapshot (device payload) carries muted=1');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('PUT ignoring muted (other field) leaves muted untouched', async () => {
|
||||||
|
await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { muted: true }));
|
||||||
|
const r = await jfetch(`/api/assignments/${S.itemId}`, put(S.jwt, { duration_sec: 15 }));
|
||||||
|
assert.equal(r.status, 200);
|
||||||
|
assert.equal(r.body.muted, 1, 'a non-mute update does not reset muted');
|
||||||
|
assert.equal(r.body.duration_sec, 15);
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue