Sync development into main - #2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR syncs the development branch into main, bringing the Puzzel Dashboard Edge App prototype to a more complete implementation: rendering is moved to lit-html templates, queue cards include Chart.js visuals, and data/credentials fetching is refactored with cache-based failover and updated mock-server capabilities.
Changes:
- Introduces
lit-htmltemplate-based rendering for queues/agents, including per-queue bar charts via Chart.js. - Refactors runtime refresh flow into
dashboard.ts+credentials.ts, adding cache-backed failover behavior and corresponding unit tests. - Expands mock server to support real-token proxying (Puzzel + Screenly broker endpoints) and updates manifests/docs/tests accordingly.
Reviewed changes
Copilot reviewed 40 out of 53 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/templates/queue-chart.ts | Adds Chart.js bar chart mounting for queue metrics. |
| src/templates/queue-card.ts | Adds lit-html queue cards and mounts charts after render. |
| src/templates/queue-card.lib.ts | Extracts formatDuration helper for queue wait time display. |
| src/templates/queue-card.lib.test.ts | Unit tests for formatDuration. |
| src/templates/index.ts | Provides renderDashboard orchestration for templates. |
| src/templates/agent-tile.ts | Adds lit-html agent tile rendering. |
| src/templates/agent-tile.lib.ts | Extracts status-to-label/color mapping helpers. |
| src/templates/agent-tile.lib.test.ts | Unit tests for agent status label/color helpers. |
| src/style.css | Adds component-layer Tailwind styles for new template markup. |
| src/render.ts | Removes old DOM-manipulation render implementation. |
| src/main.ts | Switches boot flow to locale-aware refresh() loop. |
| src/dashboard.ts | Implements refresh + data fetching with cache failover. |
| src/dashboard.test.ts | Unit tests for dashboard refresh, caching, and error behavior. |
| src/credentials.ts | Adds access-token retrieval via Screenly broker with caching. |
| src/credentials.test.ts | Unit tests for credential fetching + cache fallbacks. |
| src/constants.ts | Updates API constants and adds cache namespace constant. |
| src/auth.ts | Removes old token-refresh helper logic. |
| src/app.ts | Removes old screen/error helpers and inlined formatting/mapping. |
| src/api.ts | Refactors API functions to read settings at point-of-use; expands queue stats fields. |
| src/api.test.ts | Unit tests for API URL construction and error handling. |
| screenly.yml | Updates settings schema (adds user_id, locale/timezone overrides; removes api_base_url). |
| screenly_qc.yml | Adds QC/staging manifest mirroring the Screenly settings. |
| README.md | Updates configuration documentation to match new settings. |
| package.json | Adds chart.js and lit-html; adjusts test runner flags. |
| mock-server/src/views/index.ejs | Revamps mock server UI for connect/disconnect and token display. |
| mock-server/src/routes/screenly.ts | Adds /screenly/access_token/ broker endpoint for the app. |
| mock-server/src/routes/puzzel.ts | Adds /puzzel/... routes with optional proxy-to-real behavior. |
| mock-server/src/refresh.ts | Adds scheduled token refresh loop for connected mode. |
| mock-server/src/oauth.ts | Adds client-credentials flow to fetch/store Puzzel tokens. |
| mock-server/src/index.ts | Simplifies startup using createApp() and starts refresh loop. |
| mock-server/src/db.ts | Adds SQLite persistence for tokens (auth.db). |
| mock-server/src/data.ts | Extends mock queue stats and jitter logic for CIQ fields. |
| mock-server/src/constants.ts | Adds Puzzel token/API base URL constants for proxying. |
| mock-server/src/app.ts | Extracts Express app wiring and adds connect/disconnect endpoints. |
| mock-server/README.md | Documents new proxy/broker behavior and endpoints. |
| mock-server/.gitignore | Ignores .env and auth.db to prevent committing secrets/tokens. |
| mock-server/.env.example | Provides example env vars for real Puzzel connection. |
| index.html | Removes old error screen markup and reformats layout container. |
| e2e/screenshots.spec.ts | Refactors screenshot tests to use shared helpers and adds empty-queues screenshots. |
| e2e/screenshots.lib.ts | Adds screenshot test harness + mocked routes/data. |
| bun.lock | Locks new dependencies (chart.js, lit-html) and transitive packages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 53 changed files in this pull request and generated 2 comments.
Suppressed comments (8)
src/credentials.ts:12
- When a dev
access_tokenis provided (intended for testing), the function still calls the Screenly OAuth broker first and then reports an error if that call fails. This creates unnecessary network traffic and noisy error reporting during local development; short-circuit to the dev token early when it’s set.
src/credentials.ts:49 - On error, the function can return an empty string if there is no cached token and no dev
access_token. Returning an empty token will reliably lead to downstream 401/403s and makes the root failure harder to diagnose; throw when no fallback token exists.
src/main.ts:26 setInterval(() => refresh(locale), ...)schedules an async function without handling rejections, which can produce unhandled promise rejections during refresh failures (and preventssignalReady()if the initial refresh throws). Wrap refresh in an async tick that catches/logs errors and pass that tosetInterval(and optionally ensuresignalReady()still runs).
src/credentials.ts:19- The access token URL is built via string concatenation, which is brittle if
screenly_oauth_tokens_urlis missing a trailing slash (or already contains a path). Usingnew URL()avoids subtle double-slash / missing-slash issues.
screenly.yml:35 - Locale examples use underscore separators (e.g.
en_US), but JS Intl BCP-47 language tags typically use hyphens (e.g.en-US). Using the canonical format avoids confusion and potentialRangeErrorin Intl APIs when users copy/paste examples.
help_text: |
Override the default locale with a supported language code (e.g., en_US, fr_FR, de_DE).
screenly_qc.yml:35
- Locale examples use underscore separators (e.g.
en_US), but JS Intl BCP-47 language tags typically use hyphens (e.g.en-US). Using the canonical format avoids confusion and potentialRangeErrorin Intl APIs when users copy/paste examples.
help_text: |
Override the default locale with a supported language code (e.g., en_US, fr_FR, de_DE).
README.md:46
- Locale examples use underscore separators (e.g.
en_US), but JS Intl BCP-47 language tags typically use hyphens (e.g.en-US). Updating the examples avoids confusion and potential IntlRangeErrorwhen users copy/paste these values into settings.
| `override_locale` | Override the default locale with a supported language code (e.g., en_US, fr_FR, de_DE). | No | `en` |
src/dashboard.ts:41
- If fetching dashboard data fails and
display_errorsis off, the original error is swallowed and the subsequent "No cached dashboard data found." error loses the root cause. Preserve the original error as acausewhen there’s no cache so logs/debugging can identify why fetch failed.
|
Review comments addressed in #7. |
- Bump queue-card, agent-tile, and chart tick font sizes to meet Android TV's minimum body/heading text guidelines - Zoom and re-color the app header to match the queue cards - Add TV-safe-area padding around the whole dashboard - Only render queue cards and agent tiles that fully fit on screen, instead of showing a truncated row - Regenerate screenshots
| connect() | ||
| .then((result) => { | ||
| if ('error' in result) { | ||
| console.error(`Puzzel token refresh failed: ${result.error}`) | ||
| return | ||
| } | ||
| console.log(`Puzzel token refreshed at ${new Date().toISOString()}`) | ||
| scheduleNextRefresh() | ||
| }) | ||
| .catch((err) => console.error('Puzzel token refresh error:', err)) |
| import { Router } from 'express' | ||
| import { getConnection } from '../oauth' | ||
|
|
||
| export const screenlyRouter = Router() | ||
|
|
||
| screenlyRouter.get('/access_token/', (_req, res) => { | ||
| const connection = getConnection() | ||
| if (!connection) { | ||
| res.status(404).json({ error: 'Not connected. Visit / to connect.' }) | ||
| return | ||
| } | ||
|
|
||
| res.json({ token: connection.token }) | ||
| }) |
| renderCards(container, queues, locale) | ||
|
|
||
| const fittingCount = countFittingCards(container) | ||
| if (fittingCount < queues.length) { | ||
| renderCards(container, queues.slice(0, fittingCount), locale) | ||
| } |
PRs included