From 07a71f043217d8d227012abd72079a4bc5329872 Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Thu, 7 May 2026 18:14:56 -0700 Subject: [PATCH] Discover section controller foundation + migrate Recent Releases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every section on the discover page (Recent Releases, Your Artists, Your Albums, Seasonal Albums, Seasonal Mix, Fresh Tape, The Archives, Build Playlist, Time Machine, Browse by Genre, ListenBrainz Playlists, Because You Listen To, plus ~13 hidden sections) currently re-implements the same lifecycle by hand: 1. show a loading spinner in the carousel container 2. fetch the section's endpoint 3. parse the response, decide if the data is empty 4. either render the items, show an empty-state, or show an error 5. wire post-render handlers (download buttons, hover behavior, etc) 6. maybe expose refresh() ~30 sections worth of duplicated boilerplate, all subtly drifting. Different empty-state messages. Different error handling (some `console.debug`, some silently swallowed, some leave the spinner spinning forever). Different sync-status icons (✓/⏳/✗ vs ♪/✓/✗). No consistent error toast. Lifted the lifecycle into a shared `createDiscoverSectionController` in `webui/static/discover-section-controller.js`. Renderers stay per-section because section data shapes legitimately differ — album cards vs artist circles vs playlist tiles vs track rows. The controller is the wrapper, not a forced visual abstraction. Foundation contract: createDiscoverSectionController({ id: 'recent-releases', // for diagnostic logging contentEl: '#carousel', // selector or Element fetchUrl: '/api/discover/...', extractItems: (data) => [...], // pull list from response renderItems: (items, data, ctx) => '', onRendered: (ctx) => { ... }, // optional post-render hook loadingMessage / emptyMessage / errorMessage: copy sectionEl + hideWhenEmpty: optional whole-section visibility isSuccess / isEmpty: optional gate overrides }) Returns `{ load, refresh, destroy, getState }`. Validates config up front so misuse fails at register-time, not silently on load. Coalesces concurrent loads (same in-flight promise returned) so a double-click or repeated trigger doesn't double-fetch. `refresh()` bypasses the coalesce so the refresh button always re-fires. Errors are logged (console.debug by default, console.error when verboseErrors=true). Renderer hook errors are caught + logged so a buggy render callback can't tear down the controller — keeps the page resilient. Migrated `Recent Releases` as the proof — simplest album-card shape, no source-gating, no refresh button. Verified the contract covers it end-to-end. The legacy `loadDiscoverRecentReleases` entry-point stays public so existing callers don't change; internally it lazy-builds the controller and triggers `load()`. NOT in this commit: - Other section migrations (one section per follow-up commit, keeps reviews small + lets us sequence the work) - Registry-driven section list (so the dead-section audit becomes registry deletions instead of section-by-section removal) - Global error toast wrapper - Per-section "requires X primary source" gate - Sync-status icon renderer unification Once every section is on the controller, the discover-page cleanup work (kill the 13 dead sections, standardize sync-status icons, add error toasts) becomes single-line registry-level edits instead of 30 separate section-by-section rewrites. 2204/2204 full suite green. JS parses clean (`node --check`). Manual smoke deferred until follow-up commits — Recent Releases unchanged on the wire (same endpoint, same payload shape, same render output). --- webui/index.html | 1 + webui/static/discover-section-controller.js | 324 ++++++++++++++++++++ webui/static/discover.js | 88 +++--- webui/static/helper.js | 5 + 4 files changed, 373 insertions(+), 45 deletions(-) create mode 100644 webui/static/discover-section-controller.js diff --git a/webui/index.html b/webui/index.html index 3de06f8e..7b0d993a 100644 --- a/webui/index.html +++ b/webui/index.html @@ -7908,6 +7908,7 @@ + diff --git a/webui/static/discover-section-controller.js b/webui/static/discover-section-controller.js new file mode 100644 index 00000000..fdfef9af --- /dev/null +++ b/webui/static/discover-section-controller.js @@ -0,0 +1,324 @@ +/** + * Discover Section Controller + * --------------------------- + * + * Owns the lifecycle every discover-page section already does by hand: + * + * 1. show a loading spinner in the carousel container + * 2. fetch the section's endpoint + * 3. parse the response, decide whether the data is empty + * 4. either show the empty state, render the items, or show an error + * 5. wire any post-render handlers (download buttons, hover, etc) + * 6. expose a refresh() method so the same lifecycle can re-fire + * + * Each section currently re-implements this 30 times in `discover.js` + * with subtle drift — different empty-state messages, inconsistent + * error handling (some console.debug, some silently swallowed, some + * leave the spinner spinning forever), inconsistent refresh-button + * feedback. This controller is the "lift what's truly shared" + * extraction: register a section once, the controller handles the + * lifecycle, the section provides only its renderer. + * + * Renderers stay per-section because section data shapes legitimately + * differ (album cards vs artist circles vs playlist tiles vs track + * rows). The controller is the lifecycle wrapper around those + * renderers, not a forced visual abstraction. + * + * MIGRATION STATUS: this is the foundation commit. Only `Recent + * Releases` has been migrated as a proof. The other sections + * (Your Artists, Your Albums, Seasonal, Fresh Tape, The Archives, + * etc) still use their hand-rolled load functions in discover.js + * and will migrate one section per commit. + * + * USAGE: + * + * const ctrl = createDiscoverSectionController({ + * id: 'recent-releases', + * contentEl: '#recent-releases-carousel', + * fetchUrl: '/api/discover/recent-releases', + * extractItems: (data) => data.albums || [], + * renderItems: (items, data, ctx) => buildCardsHtml(items), + * onRendered: (ctx) => attachClickHandlers(ctx.contentEl), + * loadingMessage: 'Loading recent releases...', + * emptyMessage: 'No recent releases found', + * errorMessage: 'Failed to load recent releases', + * }); + * ctrl.load(); + * + * Future enhancements (not in this foundation commit): + * - global error toast wrapper (so users see something when an + * endpoint fails instead of the silent-empty-state default) + * - registry-driven section list (so the dead-section audit + * becomes registry edits, not section-by-section deletions) + * - per-section "requires X primary source" gate + */ + +(function () { + 'use strict'; + + // Validate the config object up front — bad configs fail fast at + // section-register time instead of silently breaking on load. + function _validateConfig(cfg) { + if (!cfg || typeof cfg !== 'object') { + throw new Error('createDiscoverSectionController: config required'); + } + if (typeof cfg.id !== 'string' || !cfg.id) { + throw new Error('createDiscoverSectionController: config.id required (string)'); + } + if (typeof cfg.contentEl !== 'string' && !(cfg.contentEl instanceof Element)) { + throw new Error(`[discover:${cfg.id}] config.contentEl required (selector or Element)`); + } + if (typeof cfg.fetchUrl !== 'string' || !cfg.fetchUrl) { + throw new Error(`[discover:${cfg.id}] config.fetchUrl required`); + } + if (typeof cfg.renderItems !== 'function') { + throw new Error(`[discover:${cfg.id}] config.renderItems required (function)`); + } + } + + function _resolveEl(el) { + if (el instanceof Element) return el; + if (typeof el === 'string') return document.querySelector(el); + return null; + } + + /** + * @param {Object} cfg - Section config (see file header for shape) + * @returns {Object} Public API: { load, refresh, destroy, getState } + */ + function createDiscoverSectionController(cfg) { + _validateConfig(cfg); + + const config = Object.assign({ + // Wrapper element to show/hide when section becomes empty. + // Null = always visible, only the contents change. + sectionEl: null, + // Hide the whole section if the response is empty + // (vs showing an empty-state message inside the carousel). + hideWhenEmpty: false, + // For sections that need to show data even on empty (e.g. + // "Recent Releases" with "no recent releases" copy). + // When true and items.length === 0, render the empty state. + renderEmptyState: true, + // HTTP method + options for the fetch call. Default GET, no + // body. fetchOptions is a function so callers can compute it + // at load time (e.g. read filter selects). + fetchMethod: 'GET', + fetchOptions: null, + // Pull the items array from the response. Default looks + // for `data.items` then `data.albums` then `data.artists`. + extractItems: null, + // Override the success check. Default: require data.success + // when present, otherwise treat any 2xx as success. + isSuccess: null, + // Override empty-detection. Default: items.length === 0. + isEmpty: null, + // Post-render hook — attach event handlers, etc. + onRendered: null, + // Lifecycle copy. Empty string = no message. + loadingMessage: 'Loading...', + emptyMessage: 'Nothing to show', + errorMessage: 'Failed to load', + // CSS class names — let surfaces override for styling. + loadingClass: 'discover-loading', + emptyClass: 'discover-empty', + errorClass: 'discover-empty', + // When true, log full error stacks to console.error. Default + // logs to console.debug — keeps the console quiet for users + // while staying inspectable when devtools is open. + verboseErrors: false, + }, cfg); + + const state = { + phase: 'idle', // idle | loading | rendered | empty | error + lastData: null, + lastError: null, + inFlight: null, + }; + + function _setHtml(el, html) { + if (el) el.innerHTML = html; + } + + function _showLoading() { + const contentEl = _resolveEl(config.contentEl); + if (!contentEl) return; + const msg = config.loadingMessage + ? `

${config.loadingMessage}

` + : ''; + _setHtml(contentEl, ` +
+ + ${msg} +
+ `); + state.phase = 'loading'; + } + + function _showEmpty() { + const contentEl = _resolveEl(config.contentEl); + if (!contentEl) return; + if (config.hideWhenEmpty) { + const sectionEl = _resolveEl(config.sectionEl); + if (sectionEl) sectionEl.style.display = 'none'; + state.phase = 'empty'; + return; + } + if (config.renderEmptyState) { + _setHtml(contentEl, ` +
+

${config.emptyMessage}

+
+ `); + } else { + _setHtml(contentEl, ''); + } + state.phase = 'empty'; + } + + function _showError(error) { + const contentEl = _resolveEl(config.contentEl); + if (!contentEl) return; + _setHtml(contentEl, ` +
+

${config.errorMessage}

+
+ `); + state.phase = 'error'; + state.lastError = error; + const log = config.verboseErrors ? console.error : console.debug; + log(`[discover:${config.id}]`, error); + } + + function _showSection() { + const sectionEl = _resolveEl(config.sectionEl); + if (sectionEl) sectionEl.style.display = ''; + } + + function _extractItems(data) { + if (config.extractItems) return config.extractItems(data) || []; + // Sensible defaults for the most common response shapes. + if (Array.isArray(data?.items)) return data.items; + if (Array.isArray(data?.albums)) return data.albums; + if (Array.isArray(data?.artists)) return data.artists; + if (Array.isArray(data?.tracks)) return data.tracks; + if (Array.isArray(data?.results)) return data.results; + return []; + } + + function _isSuccess(data) { + if (config.isSuccess) return config.isSuccess(data); + // If `success` is present, require it to be truthy. Otherwise + // a 2xx response with parseable JSON counts as success. + if (data && Object.prototype.hasOwnProperty.call(data, 'success')) { + return Boolean(data.success); + } + return true; + } + + function _isEmpty(items, data) { + if (config.isEmpty) return config.isEmpty(items, data); + return !Array.isArray(items) || items.length === 0; + } + + async function load() { + // Coalesce concurrent loads — if a fetch is already in flight, + // return the same promise rather than firing a second call. + if (state.inFlight) return state.inFlight; + + const contentEl = _resolveEl(config.contentEl); + if (!contentEl) { + console.debug(`[discover:${config.id}] contentEl not found, skipping load`); + return Promise.resolve(); + } + + _showLoading(); + + const fetchOpts = (typeof config.fetchOptions === 'function') + ? (config.fetchOptions() || {}) + : {}; + const init = Object.assign( + { method: config.fetchMethod }, + fetchOpts, + ); + + const promise = (async () => { + try { + const resp = await fetch(config.fetchUrl, init); + if (!resp.ok) { + throw new Error(`HTTP ${resp.status}`); + } + const data = await resp.json(); + state.lastData = data; + + if (!_isSuccess(data)) { + // Treat success=false as empty rather than error so + // the user sees the "nothing here" copy. Endpoints + // returning success=false with a network/auth reason + // can opt into error treatment via isSuccess. + _showEmpty(); + return; + } + + const items = _extractItems(data); + if (_isEmpty(items, data)) { + _showEmpty(); + return; + } + + _showSection(); + const html = config.renderItems(items, data, { contentEl, config }); + _setHtml(contentEl, html || ''); + state.phase = 'rendered'; + + if (typeof config.onRendered === 'function') { + try { + config.onRendered({ contentEl, items, data, config }); + } catch (hookErr) { + // Don't let a renderer hook error rip down the + // controller — log + continue. + console.debug(`[discover:${config.id}] onRendered hook threw:`, hookErr); + } + } + } catch (err) { + _showError(err); + } finally { + state.inFlight = null; + } + })(); + + state.inFlight = promise; + return promise; + } + + async function refresh() { + // Clear in-flight first so refresh() always re-fires the + // network call (load() coalesces, refresh() bypasses). + state.inFlight = null; + return load(); + } + + function destroy() { + state.inFlight = null; + state.lastData = null; + state.lastError = null; + state.phase = 'idle'; + } + + function getState() { + // Expose a copy so callers can inspect without holding onto + // mutable internal state. + return { + phase: state.phase, + hasData: state.lastData !== null, + error: state.lastError, + }; + } + + return { load, refresh, destroy, getState }; + } + + // Expose globally — the discover page is one big shared script + // surface, no module system in play. + window.createDiscoverSectionController = createDiscoverSectionController; +})(); diff --git a/webui/static/discover.js b/webui/static/discover.js index 7671d015..77e1e453 100644 --- a/webui/static/discover.js +++ b/webui/static/discover.js @@ -842,54 +842,52 @@ function showDiscoverHeroEmpty() { if (subtitleEl) subtitleEl.textContent = 'Run a watchlist scan to generate personalized recommendations'; } +// Recent Releases — first section migrated to the shared +// `createDiscoverSectionController`. The controller owns the +// loading / empty / error / refresh lifecycle that every other +// discover section currently re-implements by hand. This function +// stays as the public entry-point so existing callers don't change; +// internally it builds (or reuses) the controller and triggers a +// load. See `discover-section-controller.js` for the contract. +let _recentReleasesCtrl = null; + +function _renderRecentReleaseCard(album, index) { + const coverUrl = album.album_cover_url || '/static/placeholder-album.png'; + return ` +
+
+ ${album.album_name} +
+
+

${album.album_name}

+

${album.artist_name}

+

${album.release_date}

+
+
+ `; +} + async function loadDiscoverRecentReleases() { - try { - const carousel = document.getElementById('recent-releases-carousel'); - if (!carousel) return; - - carousel.innerHTML = '

Loading recent releases...

'; - - const response = await fetch('/api/discover/recent-releases'); - if (!response.ok) { - throw new Error('Failed to fetch recent releases'); - } - - const data = await response.json(); - if (!data.success || !data.albums || data.albums.length === 0) { - carousel.innerHTML = '

No recent releases found

'; - return; - } - - // Store albums for download functionality - discoverRecentAlbums = data.albums; - - // Build carousel HTML - let html = ''; - data.albums.forEach((album, index) => { - const coverUrl = album.album_cover_url || '/static/placeholder-album.png'; - html += ` -
-
- ${album.album_name} -
-
-

${album.album_name}

-

${album.artist_name}

-

${album.release_date}

-
-
- `; + if (!_recentReleasesCtrl) { + _recentReleasesCtrl = createDiscoverSectionController({ + id: 'recent-releases', + contentEl: '#recent-releases-carousel', + fetchUrl: '/api/discover/recent-releases', + extractItems: (data) => data.albums || [], + renderItems: (items) => { + // Module-level `discoverRecentAlbums` is what the click + // handler reads to look up the album by index. Keep it + // in sync so `openDownloadModalForRecentAlbum(index)` + // still resolves correctly after re-renders. + discoverRecentAlbums = items; + return items.map((album, i) => _renderRecentReleaseCard(album, i)).join(''); + }, + loadingMessage: 'Loading recent releases...', + emptyMessage: 'No recent releases found', + errorMessage: 'Failed to load recent releases', }); - - carousel.innerHTML = html; - - } catch (error) { - console.error('Error loading recent releases:', error); - const carousel = document.getElementById('recent-releases-carousel'); - if (carousel) { - carousel.innerHTML = '

Failed to load recent releases

'; - } } + return _recentReleasesCtrl.load(); } // =============================== diff --git a/webui/static/helper.js b/webui/static/helper.js index 255d5ed0..d59543b9 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3429,6 +3429,11 @@ function closeHelperSearch() { // projects that span multiple commits before shipping. Strip the flag at // release time and add a real `date:` line at the top of the version block. const WHATS_NEW = { + '2.4.3': [ + // --- post-2.4.2 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps --- + { date: 'Unreleased — 2.4.3 dev cycle' }, + { title: 'Internal: Discover Section Controller Foundation', desc: 'every section on the discover page (recent releases, your artists, your albums, seasonal, fresh tape, the archives, etc) re-implements the same lifecycle by hand: show spinner → fetch endpoint → parse → either render or show empty state or show error → maybe wire post-render handlers → maybe expose refresh. ~30 sections, all subtly drifting — different empty messages, different error handling (some console.debug, some silently swallowed, some leave the spinner spinning forever), different sync-status icons, no consistent error toast. lifted that lifecycle into a shared `createDiscoverSectionController` (renderers stay per-section because section data shapes legitimately differ — album cards vs artist circles vs playlist tiles vs track rows; the controller is the wrapper, not a forced visual abstraction). this commit is the foundation: built the controller + migrated `recent releases` as proof. each remaining section will migrate in its own follow-up commit (keeps reviews small + lets us sequence the work). once everything is on the controller, the discover-page cleanup work (kill 13 dead sections, standardize sync-status icons, add error toasts) becomes single-line registry edits instead of section-by-section rewrites.', page: 'discover' }, + ], '2.4.2': [ // --- May 7, 2026 — patch release --- { date: 'May 7, 2026 — 2.4.2 release' },