From 9bcdaacd2c106fbccd9aee4258ecb54714ca6d0e Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 28 Jul 2026 12:16:19 -0500 Subject: [PATCH] Show every screen's schedule on one calendar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The week view could only answer "what plays on THIS screen". With one screen at a time an empty grid is ambiguous — nothing scheduled, or the schedule points at a different screen? That ambiguity is what a user actually hit. Adds an "All screens" scope alongside the per-screen one. Every block now names its target, with a stable per-target colour and a legend, so a full grid stays readable. The scope for all=1 comes from the request's resolved tenancy and is filtered on nothing else, so the tenant boundary rests entirely on that resolution. Tests pin both halves: an ordinary tenant gains nothing by naming another workspace in the query string, and the platform-admin act-as path still resolves the workspace it asks for — the two are easy to mistake for each other, so they are asserted separately. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- frontend/js/i18n/en.js | 1 + frontend/js/views/schedule.js | 68 +++++++- server/routes/schedules.js | 49 +++++- server/test/schedule-week-all-screens.test.js | 161 ++++++++++++++++++ 4 files changed, 265 insertions(+), 14 deletions(-) create mode 100644 server/test/schedule-week-all-screens.test.js diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 6dd3686..0067e81 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1298,6 +1298,7 @@ export default { 'schedule.tz_device': 'Times are in {zone} — the screen\u2019s timezone, not your {local}.', 'schedule.tz_same': 'Times are in {zone}.', 'schedule.tz_unknown': 'Times use the screen\u2019s own timezone once it reports one.', + 'schedule.all_screens': 'All screens', 'schedule.start_time': 'Start Time', 'schedule.end_time': 'End Time', 'schedule.repeat': 'Repeat', diff --git a/frontend/js/views/schedule.js b/frontend/js/views/schedule.js index 45abb06..89c62b2 100644 --- a/frontend/js/views/schedule.js +++ b/frontend/js/views/schedule.js @@ -34,7 +34,8 @@ export async function render(container) {

${t('schedule.title')} ?

${t('schedule.subtitle')}
- + ${devices.map(d => ``).join('')} @@ -42,6 +43,9 @@ export async function render(container) {
+ +
@@ -175,12 +179,31 @@ export async function render(container) { `${currentWeekStart.toLocaleDateString(undefined, { month: 'short', day: 'numeric' })} - ${end.toLocaleDateString(undefined, { month: 'short', day: 'numeric', year: 'numeric' })}`; } + // Stable colour per target, so the same screen is the same colour every week and + // across reloads. Hashing the id beats cycling a palette by index, which reshuffles + // whenever a device is added or removed. + const TARGET_COLORS = ['#3B82F6','#8B5CF6','#EC4899','#F59E0B','#10B981','#06B6D4','#EF4444','#84CC16','#A855F7','#14B8A6']; + function colorForTarget(key) { + let h = 0; + for (let i = 0; i < key.length; i++) h = (h * 31 + key.charCodeAt(i)) >>> 0; + return TARGET_COLORS[h % TARGET_COLORS.length]; + } + // What an event is aimed at. Group schedules name the group; device schedules name the + // device. In all-screens mode this is the thing the operator is actually scanning for. + function targetOf(ev) { + if (ev.group_id) return { key: 'g:' + ev.group_id, name: ev.group_name || t('schedule.target_group'), isGroup: true }; + return { key: 'd:' + (ev.device_id || '?'), name: ev.device_name || t('schedule.target_device'), isGroup: false }; + } + async function loadCalendar() { const deviceId = document.getElementById('schedDevice').value; if (!deviceId) return; + const allScreens = deviceId === '*'; updateWeekLabel(); - const events = await API(`/schedules/week?date=${currentWeekStart.toISOString()}&device_id=${deviceId}`); + // all=1 rather than a workspace id — the server scopes to the caller's own workspace. + const scope = allScreens ? 'all=1' : `device_id=${encodeURIComponent(deviceId)}`; + const events = await API(`/schedules/week?date=${currentWeekStart.toISOString()}&${scope}`); const cal = document.getElementById('calendar'); let html = '
'; @@ -204,6 +227,7 @@ export async function render(container) { cal.innerHTML = html; + const seenTargets = new Map(); events.forEach(ev => { const start = new Date(ev.instance_start || ev.start_time); const end = new Date(ev.instance_end || ev.end_time); @@ -216,19 +240,49 @@ export async function render(container) { if (!cell) return; const isGroupSchedule = !!ev.group_id; + const target = targetOf(ev); + seenTargets.set(target.key, target); const block = document.createElement('div'); const topOffset = (startHour - Math.floor(startHour)) * 28; + // In all-screens mode colour identifies WHO the block is for, so several targets share + // one grid and stay tellable apart. On a single screen the schedule's own colour is + // kept — there is only one target, so colour is free to mean something else. + const bg = allScreens ? colorForTarget(target.key) : (ev.color || '#3B82F6'); + const tall = duration * 28 >= 34; block.style.cssText = `position:absolute;top:${topOffset}px;left:2px;right:2px;height:${Math.max(20, duration * 28)}px; - background:${ev.color || '#3B82F6'};border-radius:3px;padding:2px 4px;font-size:10px;color:white;overflow:hidden;cursor:pointer;z-index:1;opacity:0.85; - ${isGroupSchedule ? 'border:1.5px dashed rgba(255,255,255,0.6);' : ''}`; + background:${bg};border-radius:3px;padding:2px 4px;font-size:10px;color:white;overflow:hidden;cursor:pointer;z-index:1;opacity:0.9; + line-height:1.25;${isGroupSchedule ? 'border:1.5px dashed rgba(255,255,255,0.65);' : ''}`; const label = ev.title || ev.playlist_name || ev.content_name || ev.widget_name || t('schedule.scheduled_label'); - const prefix = isGroupSchedule ? `[${esc(ev.group_name || t('schedule.target_group'))}] ` : ''; - block.textContent = prefix + label; - block.title = `${isGroupSchedule ? t('schedule.tooltip_group_prefix') + (ev.group_name || '') + '\n' : ''}${start.toLocaleTimeString()} - ${end.toLocaleTimeString()}\n${t('schedule.tooltip_priority', { n: ev.priority })}`; + if (allScreens && tall) { + // Two lines when there is room: who it is for, then what plays. The target reads + // first because that is what the eye is scanning the grid for. + block.innerHTML = `
${esc(target.name)}
` + + `
${esc(label)}
`; + } else { + block.textContent = (allScreens || isGroupSchedule) ? `${target.name} \u00b7 ${label}` : label; + } + + const kind = isGroupSchedule ? t('schedule.target_group') : t('schedule.target_device'); + block.title = `${kind}: ${target.name}\n${label}\n${start.toLocaleTimeString()} - ${end.toLocaleTimeString()}` + + `\n${t('schedule.tooltip_priority', { n: ev.priority })}` + + (ev.timezone ? `\n${t('schedule.tz_same').replace('{zone}', ev.timezone)}` : ''); block.onclick = () => editSchedule(ev); cell.appendChild(block); }); + + // Legend — only in all-screens mode, where the grid mixes targets. Sorted so the order + // is stable between reloads rather than following whatever the query happened to return. + const legend = document.getElementById('schedLegend'); + if (legend) { + const targets = [...seenTargets.values()].sort((a, b) => a.name.localeCompare(b.name)); + legend.style.display = (allScreens && targets.length) ? 'flex' : 'none'; + legend.innerHTML = targets.map(tg => ` + + ${esc(tg.name)} + `).join(''); + } } function editSchedule(ev) { diff --git a/server/routes/schedules.js b/server/routes/schedules.js index 2ab7ee0..99e96d5 100644 --- a/server/routes/schedules.js +++ b/server/routes/schedules.js @@ -34,6 +34,31 @@ function getDeviceSchedulesQuery() { `; } +// Every schedule in a workspace, each row carrying the NAME of what it targets. +// +// The per-device query answers "what plays on THIS screen". This answers "what is +// scheduled anywhere", which is what an operator actually needs to see: with a +// single-device calendar you cannot tell whether a gap is deliberate or whether you +// pointed the schedule at the wrong screen — the failure mode a real user hit. +function getWorkspaceSchedulesQuery() { + return ` + SELECT s.*, c.filename as content_name, w.name as widget_name, p.name as playlist_name, + dg.name as group_name, dg.color as group_color, + d.name as device_name + FROM schedules s + LEFT JOIN content c ON s.content_id = c.id + LEFT JOIN widgets w ON s.widget_id = w.id + LEFT JOIN playlists p ON s.playlist_id = p.id + LEFT JOIN device_groups dg ON s.group_id = dg.id + LEFT JOIN devices d ON s.device_id = d.id + WHERE s.enabled = 1 AND s.workspace_id = ? + ORDER BY + CASE WHEN s.device_id IS NOT NULL THEN 1 ELSE 0 END DESC, + s.priority DESC, + s.created_at ASC + `; +} + // Load a schedule + access context, sending 403/404 on failure. function loadScheduleAccess(req, res, requireWrite) { const schedule = db.prepare('SELECT * FROM schedules WHERE id = ?').get(req.params.id); @@ -117,13 +142,21 @@ router.get('/device/:deviceId', (req, res) => { // Expanded week view (resolves recurrences). Phase 2.2m: device access via workspace. router.get('/week', (req, res) => { - const { date, device_id } = req.query; - if (!device_id) return res.status(400).json({ error: 'device_id required' }); + const { date, device_id, all } = req.query; + if (!device_id && !all) return res.status(400).json({ error: 'device_id or all=1 required' }); - const device = db.prepare('SELECT workspace_id FROM devices WHERE id = ?').get(device_id); - if (!device) return res.status(404).json({ error: 'Device not found' }); - if (!device.workspace_id) return res.status(403).json({ error: 'Device not assigned to a workspace' }); - const ctx = workspaceAccess(req, device.workspace_id); + // all=1 -> every schedule on every screen, for the "all screens" calendar. The workspace + // comes from the caller's resolved tenancy, never from the query string: a client-supplied + // workspace_id here would be a cross-tenant read waiting to happen. + let scopeWorkspaceId = all ? req.workspaceId : null; + if (all && !scopeWorkspaceId) return res.json([]); + if (device_id) { + const device = db.prepare('SELECT workspace_id FROM devices WHERE id = ?').get(device_id); + if (!device) return res.status(404).json({ error: 'Device not found' }); + if (!device.workspace_id) return res.status(403).json({ error: 'Device not assigned to a workspace' }); + scopeWorkspaceId = device.workspace_id; + } + const ctx = workspaceAccess(req, scopeWorkspaceId); if (!ctx) return res.status(403).json({ error: 'Access denied' }); const weekStart = date ? new Date(date) : new Date(); @@ -132,7 +165,9 @@ router.get('/week', (req, res) => { const weekEnd = new Date(weekStart); weekEnd.setDate(weekEnd.getDate() + 7); - const schedules = db.prepare(getDeviceSchedulesQuery()).all(device_id, device_id); + const schedules = device_id + ? db.prepare(getDeviceSchedulesQuery()).all(device_id, device_id) + : db.prepare(getWorkspaceSchedulesQuery()).all(scopeWorkspaceId); const events = []; for (const s of schedules) { const expanded = expandSchedule(s, weekStart, weekEnd); diff --git a/server/test/schedule-week-all-screens.test.js b/server/test/schedule-week-all-screens.test.js new file mode 100644 index 0000000..b746372 --- /dev/null +++ b/server/test/schedule-week-all-screens.test.js @@ -0,0 +1,161 @@ +'use strict'; + +// The week calendar can answer two different questions, and it needs both. +// +// `?device_id=` answers "what plays on THIS screen" — the original behaviour, unchanged. +// `?all=1` answers "what is scheduled anywhere", which is what an operator actually needs +// to see. With a one-screen-at-a-time calendar you cannot tell whether an empty grid means +// nothing is scheduled or that you pointed the schedule at a different screen — and that is +// exactly the confusion a real user hit. +// +// The workspace for `all=1` comes from the caller's RESOLVED TENANCY, never from a raw +// client-supplied id. `all=1` filters on nothing but req.workspaceId, so the tenant boundary +// rests entirely on that resolution — asserted here rather than assumed. The platform-admin +// act-as path is pinned alongside it so the two are not confused for each other. + +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 { freePort } = require('./helpers/free-port'); +let PORT, BASE, proc, db; +const DATA_DIR = path.join(os.tmpdir(), 'st-schedall-' + crypto.randomBytes(4).toString('hex')); +const LOG = path.join(os.tmpdir(), 'st-schedall-' + crypto.randomBytes(4).toString('hex') + '.log'); +const A = {}, B = {}, OWNER = {}; + +const jfetch = async (p, opts = {}) => { + const res = await fetch(BASE + p, opts); + let body = null; try { body = await res.json(); } catch { /* */ } + return { status: res.status, body }; +}; +const auth = (t) => ({ Authorization: 'Bearer ' + t, 'Content-Type': 'application/json' }); + +async function tenant(label, ip) { + const email = label + crypto.randomBytes(4).toString('hex') + '@x.local'; + const reg = await jfetch('/api/auth/register', { + method: 'POST', headers: { 'Content-Type': 'application/json', 'X-Forwarded-For': ip }, + body: JSON.stringify({ email, password: 'Passw0rd123' }), + }); + const me = await jfetch('/api/auth/me', { headers: auth(reg.body.token) }); + return { token: reg.body.token, wsId: me.body.accessible_workspaces[0].id, + userId: reg.body.user.id, role: me.body.user ? me.body.user.role : reg.body.user.role }; +} +const mkDevice = (ws, name) => { + const id = crypto.randomUUID(); + db.prepare(`INSERT INTO devices (id,name,status,workspace_id,reported_timezone,created_at) + VALUES (?,?,'online',?, 'Asia/Tokyo', strftime('%s','now'))`).run(id, name, ws); + return id; +}; +const mkSchedule = (tok, body) => jfetch('/api/schedules', { + method: 'POST', headers: auth(tok), + body: JSON.stringify({ start_time: '2026-07-28T09:00:00', end_time: '2026-07-28T17:00:00', ...body }), +}); +const week = (tok, q) => jfetch(`/api/schedules/week?date=2026-07-27T00:00:00.000Z&${q}`, { headers: auth(tok) }); + +before(async () => { + PORT = await freePort(); + BASE = `http://127.0.0.1:${PORT}`; + 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 { /* */ } + await new Promise(r => setTimeout(r, 250)); + } + if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000)); + db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db')); + + // The FIRST account on a fresh self-hosted instance is made platform_admin — the instance + // owner — and platform staff can act-as into any workspace (lib/tenancy.js accessContext). + // That is deliberate, so burn a throwaway owner here: A and B must both be ordinary users + // or the cross-tenant assertion below would be testing the wrong thing. + Object.assign(OWNER, await tenant('owner', '198.51.20.9')); + Object.assign(A, await tenant('a', '198.51.20.1')); + Object.assign(B, await tenant('b', '198.51.20.2')); + assert.equal(A.role, 'user', 'A is an ordinary tenant, not the instance owner'); + assert.equal(B.role, 'user', 'B is an ordinary tenant, not the instance owner'); + + A.lobby = mkDevice(A.wsId, 'Lobby screen'); + A.cafe = mkDevice(A.wsId, 'Cafe screen'); + B.theirs = mkDevice(B.wsId, 'Their screen'); + + await mkSchedule(A.token, { device_id: A.lobby, title: 'Lobby morning' }); + await mkSchedule(A.token, { device_id: A.cafe, title: 'Cafe lunch' }); + await mkSchedule(B.token, { device_id: B.theirs, title: 'Other tenant' }); +}); +after(() => { try { db && db.close(); } catch { /* */ } try { proc.kill('SIGKILL'); } catch { /* */ } }); + +test('a single-screen calendar still shows only that screen', async () => { + const r = await week(A.token, `device_id=${A.lobby}`); + assert.equal(r.status, 200); + const titles = r.body.map(e => e.title); + assert.ok(titles.includes('Lobby morning'), 'its own schedule is there'); + assert.ok(!titles.includes('Cafe lunch'), 'another screen\'s schedule is not'); +}); + +test('all=1 shows every screen in the workspace at once', async () => { + const r = await week(A.token, 'all=1'); + assert.equal(r.status, 200); + const titles = r.body.map(e => e.title); + assert.ok(titles.includes('Lobby morning') && titles.includes('Cafe lunch'), + 'both screens appear on one grid'); +}); + +test('every event names the screen it targets, so blocks can be told apart', async () => { + const r = await week(A.token, 'all=1'); + const lobby = r.body.find(e => e.title === 'Lobby morning'); + const cafe = r.body.find(e => e.title === 'Cafe lunch'); + assert.equal(lobby.device_name, 'Lobby screen'); + assert.equal(cafe.device_name, 'Cafe screen'); +}); + +test('all=1 NEVER crosses tenants', async () => { + const r = await week(A.token, 'all=1'); + const titles = r.body.map(e => e.title); + assert.ok(!titles.includes('Other tenant'), 'the other workspace is not visible'); + // An ordinary tenant cannot steer the scope from the query string either. resolveTenancy + // does validate ?workspace_id= against access and falls through when there is none, but + // all=1 filters on nothing except req.workspaceId, so this is the assertion that keeps + // that true if the resolver's precedence is ever loosened. + const steered = await week(A.token, `all=1&workspace_id=${B.wsId}`); + assert.ok(Array.isArray(steered.body), 'still a normal response, not an error page'); + assert.ok(!steered.body.map(e => e.title).includes('Other tenant'), + 'a client-supplied workspace_id buys nothing without access to that workspace'); +}); + +test('a platform admin acting-as another workspace sees that workspace, by design', async () => { + // The counterpart to the test above: this is NOT a leak, it is the instance owner's + // documented act-as path. Pinned so the distinction stays legible. + const r = await week(OWNER.token, `all=1&workspace_id=${B.wsId}`); + assert.equal(OWNER.role, 'platform_admin'); + assert.ok(r.body.map(e => e.title).includes('Other tenant'), + 'act-as resolves the requested workspace for platform staff'); +}); + +test('a group schedule appears with its group name', async () => { + const gid = crypto.randomUUID(); + db.prepare('INSERT INTO device_groups (id,user_id,workspace_id,name) VALUES (?,?,?,?)') + .run(gid, A.userId, A.wsId, 'All lobby screens'); + db.prepare('INSERT INTO device_group_members (group_id, device_id) VALUES (?,?)').run(gid, A.lobby); + await mkSchedule(A.token, { group_id: gid, title: 'Group evening' }); + + const r = await week(A.token, 'all=1'); + const ev = r.body.find(e => e.title === 'Group evening'); + assert.ok(ev, 'the group schedule is on the all-screens grid'); + assert.equal(ev.group_name, 'All lobby screens'); + assert.ok(!ev.device_id, 'it targets a group, not a device'); +}); + +test('asking for neither scope is refused rather than silently guessing', async () => { + const r = await jfetch('/api/schedules/week?date=2026-07-27T00:00:00.000Z', { headers: auth(A.token) }); + assert.equal(r.status, 400); +});