From bb76ca5375980a586e09e840b1224bdd5cc7159b Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 5 Sep 2026 22:33:25 +0200 Subject: [PATCH] fix(usage): keep the settings tab usable on a bad collector URL, and report layouts and CSS accurately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three items, one of which explains an error seen in the app. "The operation could not be completed" could come from a config typo. status() called collectorUrl() bare, and that throws on a bare hostname, a path, a query, or http in production. The settings page renders one generic failure when its status query errors, so a misconfigured USAGE_COLLECTOR_URL replaced the whole tab with that sentence — no cause, and no way to read the status or withdraw, because every control there sits behind that call. The URL is now reported as collector_error: 'INVALID_COLLECTOR_URL' beside the real state, the tab says what is wrong and how to fix it, and the links are only rendered when there is somewhere to point them. gallery_layouts reported grid for every preset-themed install. color_theme holds either a theme object or the NAME of a preset — the theme picker stores names, and eventTypeService seeds them (`theme_preset: 'corporateTimeline'`). Only reading value.galleryLayout made masonry, timeline, mosaic and the two gallery presets invisible. Names now resolve, and an event with no theme of its own resolves through the global one instead of being counted as grid. Only the name -> layout mapping is duplicated, not the presets; frontend/src/types/theme.types.ts stays the source of truth, and an unknown name reports `other` so a preset added later degrades to "something else" rather than quietly inflating the grid count. custom_css missed CSS applied through a template. An enabled css_templates row applied via events.css_template_id is gallery styling by the same definition as the settings fields — the Custom CSS tab is where both are authored — but neither the snapshot nor the middleware saw it, so those installs reported custom_css entirely false. Existence only; template contents are never read. Eleven tests. Reverting each fix in turn fails 3, 1 and 3 of them. Refs #1110 --- .../services/usageSnapshotSignals.test.js | 152 ++++++++++++++++++ backend/src/usage/UsageService.js | 78 ++++++++- .../settings/tabs/ProductUsageTab.tsx | 28 ++-- frontend/src/i18n/locales/de.json | 1 + frontend/src/i18n/locales/en.json | 1 + frontend/src/services/productUsage.service.ts | 3 +- 6 files changed, 246 insertions(+), 17 deletions(-) create mode 100644 backend/__tests__/services/usageSnapshotSignals.test.js diff --git a/backend/__tests__/services/usageSnapshotSignals.test.js b/backend/__tests__/services/usageSnapshotSignals.test.js new file mode 100644 index 00000000..b31efcf4 --- /dev/null +++ b/backend/__tests__/services/usageSnapshotSignals.test.js @@ -0,0 +1,152 @@ +/** + * Report accuracy (#1110). + * + * Two signals were wrong in ways that only show up in the aggregate, where + * nobody can tell the number is wrong: preset-themed installs all reported + * `grid`, and CSS applied through a template reported no custom CSS at all. + * + * Also covers status() surviving a misconfigured collector URL — it used to + * throw, which took down the settings tab that is the only way to withdraw. + */ +const knex = require('knex'); +const { UsageService } = require('../../src/usage/UsageService'); + +async function bootDb() { + const db = knex({ + client: 'sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await db.schema.createTable('product_usage_state', (t) => { + t.integer('id').primary(); + t.string('status', 30).notNullable().defaultTo('disabled'); + t.boolean('notice_dismissed').notNullable().defaultTo(false); + t.string('installation_id', 64); + t.string('public_key', 59); + t.text('private_key_encrypted'); + t.string('instance_binding', 64); + t.bigInteger('sequence').notNullable().defaultTo(0); + t.text('pending_packet'); t.text('last_packet'); t.text('last_receipt'); + t.string('last_report_date', 10); t.string('last_error', 80); + t.text('feedback_preferences'); t.string('lease_token', 36); + t.bigInteger('lease_until').notNullable().defaultTo(0); + t.bigInteger('cancel_seq').notNullable().defaultTo(0); + }); + await db('product_usage_state').insert({ id: 1 }); + await db.schema.createTable('product_usage_markers', (t) => t.string('feature', 60).primary()); + await db.schema.createTable('app_settings', (t) => { + t.string('setting_key').primary(); t.text('setting_value'); t.string('setting_type'); + }); + await db.schema.createTable('feature_flags', (t) => { + t.string('key').primary(); t.boolean('value'); + }); + await db.schema.createTable('events', (t) => { + t.increments('id'); t.text('color_theme'); t.string('external_path'); + t.integer('css_template_id'); + }); + await db.schema.createTable('css_templates', (t) => { + t.increments('id'); t.boolean('is_enabled'); t.text('css_content'); + }); + for (const table of ['email_configs', 'mail_accounts']) { + await db.schema.createTable(table, (t) => { t.increments('id'); t.string('smtp_host'); }); + } + await db.schema.createTable('whatsapp_configs', (t) => { + t.increments('id'); t.boolean('enabled'); t.string('phone_number_id'); t.string('access_token'); + }); + return db; +} + +const service = (db, over = {}) => + new UsageService(db, { secret: 'q'.repeat(48), ...over }); + +describe('gallery_layouts resolves what the gallery actually renders', () => { + let db; + afterEach(async () => { if (db) await db.destroy(); db = null; }); + + it('maps preset NAMES to their layouts instead of calling them all grid', async () => { + db = await bootDb(); + await db('events').insert([ + { color_theme: 'modernMasonry' }, + { color_theme: 'corporateTimeline' }, + { color_theme: 'galleryStory' }, + ]); + const report = await service(db).snapshot(); + expect(report.gallery_layouts.sort()).toEqual( + ['gallery-story', 'masonry', 'timeline'].sort() + ); + }); + + it('still reads a theme object', async () => { + db = await bootDb(); + await db('events').insert([{ color_theme: JSON.stringify({ galleryLayout: 'mosaic' }) }]); + expect((await service(db).snapshot()).gallery_layouts).toEqual(['mosaic']); + }); + + it('reports an unknown preset as other, not as grid', async () => { + // A preset added on the frontend must not silently inflate the grid count. + db = await bootDb(); + await db('events').insert([{ color_theme: 'somePresetAddedLater' }]); + expect((await service(db).snapshot()).gallery_layouts).toEqual(['other']); + }); + + it('uses the global theme for an event that has none of its own', async () => { + db = await bootDb(); + await db('app_settings').insert({ + setting_key: 'theme_config', + setting_value: JSON.stringify({ galleryLayout: 'carousel' }), + }); + await db('events').insert([{ color_theme: null }]); + expect((await service(db).snapshot()).gallery_layouts).toEqual(['carousel']); + }); +}); + +describe('custom_css counts CSS applied through a template', () => { + let db; + afterEach(async () => { if (db) await db.destroy(); db = null; }); + + it('is configured when an enabled template is applied to an event', async () => { + db = await bootDb(); + const [id] = await db('css_templates').insert({ is_enabled: true, css_content: '.a{}' }); + await db('events').insert([{ color_theme: null, css_template_id: id }]); + expect((await service(db).snapshot()).features.custom_css.configured).toBe(true); + }); + + it('is not configured when the applied template is disabled', async () => { + db = await bootDb(); + const [id] = await db('css_templates').insert({ is_enabled: false, css_content: '.a{}' }); + await db('events').insert([{ color_theme: null, css_template_id: id }]); + expect((await service(db).snapshot()).features.custom_css.configured).toBe(false); + }); + + it('is not configured when an enabled template is applied to nothing', async () => { + db = await bootDb(); + await db('css_templates').insert({ is_enabled: true, css_content: '.a{}' }); + await db('events').insert([{ color_theme: null }]); + expect((await service(db).snapshot()).features.custom_css.configured).toBe(false); + }); +}); + +describe('status survives a misconfigured collector URL', () => { + let db; + afterEach(async () => { if (db) await db.destroy(); db = null; }); + + it.each([ + ['a bare hostname', 'usage.picpeak.app'], + ['a URL with a path', 'https://usage.picpeak.app/collect'], + ['a URL with a query', 'https://usage.picpeak.app/?x=1'], + ])('reports %s as a configuration error rather than failing the request', async (_l, endpoint) => { + db = await bootDb(); + const status = await service(db, { endpoint }).status(); + expect(status.collector_error).toBe('INVALID_COLLECTOR_URL'); + expect(status.collector_url).toBeNull(); + // The operator can still read their state — and therefore still withdraw. + expect(status.status).toBe('disabled'); + }); + + it('reports no error for a valid collector', async () => { + db = await bootDb(); + const status = await service(db, { endpoint: 'https://usage.picpeak.app' }).status(); + expect(status.collector_error).toBeNull(); + expect(status.collector_url).toBe('https://usage.picpeak.app'); + }); +}); diff --git a/backend/src/usage/UsageService.js b/backend/src/usage/UsageService.js index 95491f18..6c7b693a 100644 --- a/backend/src/usage/UsageService.js +++ b/backend/src/usage/UsageService.js @@ -56,6 +56,41 @@ const SETTING_KEYS = [ 'general_public_site_custom_css' ]; const truth = (value) => value === true || value === 1 || value === '1'; + +// `events.color_theme` holds either a theme object or the NAME of a preset — +// the admin theme picker stores names, and eventTypeService seeds them too +// (`theme_preset: 'corporateTimeline'`). Reading only `value.galleryLayout` +// therefore reported `grid` for every preset-themed install. +// +// Only the layout each name maps to is duplicated here, not the presets +// themselves; frontend/src/types/theme.types.ts stays the source of truth. A +// name this map does not know reports `other` rather than a confident `grid`, +// so a preset added on the frontend degrades to "something else" instead of +// quietly inflating the grid count. +const PRESET_LAYOUTS = { + default: 'grid', + elegantWedding: 'grid', + modernMasonry: 'masonry', + birthdayFun: 'carousel', + corporateTimeline: 'timeline', + artisticMosaic: 'mosaic', + darkClassic: 'grid', + darkElegant: 'grid', + darkModern: 'masonry', + galleryPremium: 'gallery-premium', + galleryStory: 'gallery-story' +}; + +function resolveLayout(value) { + const named = + typeof value === 'string' + ? PRESET_LAYOUTS[value] + : value && typeof value === 'object' + ? value.galleryLayout + : null; + if (!named) return typeof value === 'string' ? 'other' : 'grid'; + return LAYOUTS.includes(named) ? named : 'other'; +} const parse = (value) => { try { return JSON.parse(value); @@ -170,11 +205,26 @@ class UsageService { } async status() { const state = await this.state(); + // Reported, not thrown. status() used to call collectorUrl() bare, so a + // misconfigured USAGE_COLLECTOR_URL — a bare hostname, a path, a query, or + // http in production — failed this request outright. The settings page + // renders one generic failure when its status query errors, so the + // operator saw "the operation could not be completed" with no cause AND + // no way to reach their own state: they could not read the status or even + // withdraw, because every control on that tab is behind this call. + let collectorUrl = null; + let collectorError = null; + try { + collectorUrl = this.collectorUrl(); + } catch { + collectorError = 'INVALID_COLLECTOR_URL'; + } return { status: state.status, notice_dismissed: Boolean(state.notice_dismissed), installation_id: state.installation_id, - collector_url: this.collectorUrl(), + collector_url: collectorUrl, + collector_error: collectorError, schema_version: 'usage.v1', last_report_date: state.last_report_date, last_error: state.last_error, @@ -613,21 +663,35 @@ class UsageService { .first() ); const theme = settings.theme_config || {}; + // An enabled template applied to an event is gallery styling by the same + // definition as the settings fields — the Custom CSS tab is where both are + // authored. Existence only; template contents are never read. + const appliedTemplate = await this.db('css_templates') + .where('is_enabled', formatBoolean(true)) + .whereIn( + 'id', + this.db('events').whereNotNull('css_template_id').select('css_template_id') + ) + .select('id') + .first(); features.custom_css.configured = Boolean( settings.general_custom_css || settings.general_public_site_custom_css || - theme.customCss + theme.customCss || + appliedTemplate ); // Read only the theme field, never event names, IDs, sizes, counts, or photos. const themes = await this.db('events').distinct('color_theme'); const layouts = new Set(); + // An event with no theme of its own renders with the global one. + const inheritedLayout = resolveLayout(theme); for (const row of themes) { const value = parse(row.color_theme); - const layout = - value && typeof value === 'object' - ? value.galleryLayout || 'grid' - : 'grid'; - layouts.add(LAYOUTS.includes(layout) ? layout : 'other'); + if (row.color_theme === null || row.color_theme === '') { + layouts.add(inheritedLayout); + } else { + layouts.add(resolveLayout(value)); + } if (value && typeof value === 'object' && value.customCss) features.custom_css.configured = true; } diff --git a/frontend/src/features/settings/tabs/ProductUsageTab.tsx b/frontend/src/features/settings/tabs/ProductUsageTab.tsx index c512d86f..1aade5ab 100644 --- a/frontend/src/features/settings/tabs/ProductUsageTab.tsx +++ b/frontend/src/features/settings/tabs/ProductUsageTab.tsx @@ -155,6 +155,14 @@ export default function ProductUsageTab() { {data.last_report_date && (

{t('productUsage.lastReport', { date: data.last_report_date })}

)} + {data.collector_error === 'INVALID_COLLECTOR_URL' && ( +

+ {/* Shown alongside the real controls, not instead of them: with a + bad URL the operator still needs to read their status and + still needs to be able to withdraw. */} + {t('productUsage.invalidCollectorUrl')} +

+ )} {data.last_error && (

{/* Retrying cannot fix an unreadable signing key, and neither can @@ -210,14 +218,16 @@ export default function ProductUsageTab() { )} - - {t('productUsage.transparency')} - + {data.collector_url && ( + + {t('productUsage.transparency')} + + )} {active && ( @@ -449,7 +459,7 @@ export default function ProductUsageTab() { {message &&

{message}

} {consent && ( setConsent(false)} enable={() => diff --git a/frontend/src/i18n/locales/de.json b/frontend/src/i18n/locales/de.json index 5e282121..2ffcb6f7 100644 --- a/frontend/src/i18n/locales/de.json +++ b/frontend/src/i18n/locales/de.json @@ -26,6 +26,7 @@ "hash": "Dein vertraulicher Abfrage-Hash", "lastReport": "Zuletzt angenommener Bericht: {{date}} (UTC)", "deliveryProblem": "Die Übertragung benötigt Aufmerksamkeit. Bei Löschung oder Identitätskonflikt ist die Erfassung gestoppt. Versuche es erneut oder deaktiviere die Teilnahme, um die Daten zu löschen.", + "invalidCollectorUrl": "Die konfigurierte Collector-URL ist ungültig, daher kann die Teilnahme weder gestartet noch übermittelt werden. Setzen Sie USAGE_COLLECTOR_URL auf einen https-Origin ohne Pfad, Query oder Zugangsdaten (oder lassen Sie sie leer, um den Standard zu verwenden).", "signingKeyUnreadable": "Der Signaturschlüssel für die Nutzungsdaten kann nicht gelesen werden. Meist wurde USAGE_ENCRYPTION_KEY — oder das als Rückfallwert genutzte JWT_SECRET — geändert. Berichte können nicht gesendet und auch die Löschanfrage kann nicht signiert werden. Stellen Sie das ursprüngliche Schlüsselmaterial wieder her, um die Löschung abzuschließen; erneutes Senden oder Deaktivieren allein behebt dies nicht.", "inspect": "Genau sehen, was geteilt wird", "preview": "Nächsten Bericht ansehen", diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 46ee590f..2853dd0b 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -26,6 +26,7 @@ "hash": "Your private lookup hash", "lastReport": "Last accepted report: {{date}} (UTC)", "deliveryProblem": "Delivery needs attention. Collection stops during deletion or an identity conflict. Use retry, or disable participation to delete its data.", + "invalidCollectorUrl": "The configured usage collector URL is not valid, so participation cannot be started or delivered. Set USAGE_COLLECTOR_URL to an https origin with no path, query or credentials (or leave it unset to use the default).", "signingKeyUnreadable": "The usage signing key cannot be read, which usually means USAGE_ENCRYPTION_KEY — or JWT_SECRET, which it falls back to — was changed. Reports cannot be sent and the deletion request cannot be signed either. Restore the original encryption material to finish deletion; retrying or disabling will not resolve it on its own.", "inspect": "See exactly what is shared", "preview": "Preview next report", diff --git a/frontend/src/services/productUsage.service.ts b/frontend/src/services/productUsage.service.ts index 8bbc4739..8bcadc55 100644 --- a/frontend/src/services/productUsage.service.ts +++ b/frontend/src/services/productUsage.service.ts @@ -9,7 +9,8 @@ export interface UsageStatus { | 'identity_conflict'; notice_dismissed: boolean; installation_id: string | null; - collector_url: string; + collector_url: string | null; + collector_error?: 'INVALID_COLLECTOR_URL' | null; schema_version: string; last_report_date: string | null; last_error: string | null;