diff --git a/containers/libreportal/frontend/core/setup/css/setup-wizard.css b/containers/libreportal/frontend/core/setup/css/setup-wizard.css index b6c7704..91f0fe8 100755 --- a/containers/libreportal/frontend/core/setup/css/setup-wizard.css +++ b/containers/libreportal/frontend/core/setup/css/setup-wizard.css @@ -1519,3 +1519,49 @@ body.setup-wizard-open .custom-select-popup { z-index: 10001; } @media (max-width: 640px) { .lp-relocate-banner { flex-direction: column; align-items: stretch; } } + +/* The fields one backend needs, boxed under its own heading. Loose rows + appearing and disappearing beneath a type dropdown give no signal that they + belong to the choice above them. */ +.setup-subgroup { + margin: 4px 0 12px; + padding: 12px 14px 14px; + border-radius: 10px; + border: 1px solid rgba(var(--text-rgb), 0.13); + background: rgba(var(--text-rgb), 0.04); + display: flex; + flex-direction: column; + gap: 12px; +} +.setup-subgroup-title { + font-size: 0.72rem; + font-weight: 600; + letter-spacing: 0.09em; + text-transform: uppercase; + color: var(--accent-color, #4fc3f7); + opacity: 0.95; +} +.setup-subgroup-note { + margin: -6px 0 0; + font-size: 0.8rem; + line-height: 1.45; + color: rgba(var(--text-rgb), 0.62); +} +.setup-subgroup-title + .setup-subgroup-note { margin-top: -6px; } + +/* .setup-step gives its DIRECT children a 16px gap, which is where every other + step's fields get their spacing. The restore step's fields sit one level + deeper, inside a .setup-section, so they inherited none of it and each + input ran straight into the next field's label. */ +#sw-rs-fields { + display: flex; + flex-direction: column; + gap: 16px; +} +#sw-rs-fields + .setup-field, +#sw-rs-status:not(:empty) { + margin-top: 16px; +} +.setup-step .setup-section > .setup-field + .setup-field { + margin-top: 16px; +} diff --git a/containers/libreportal/frontend/core/setup/js/setup-wizard.js b/containers/libreportal/frontend/core/setup/js/setup-wizard.js index 3d0b755..1c28ae4 100755 --- a/containers/libreportal/frontend/core/setup/js/setup-wizard.js +++ b/containers/libreportal/frontend/core/setup/js/setup-wizard.js @@ -31,7 +31,7 @@ class SetupWizard { this.stepNames = ['Start', 'Experience', 'Identity', 'Domains', 'Storage', 'Backups', 'Import', 'Recommended', 'Metrics', 'Backup', 'Contents', 'Rebuild']; this.stepIcons = ['\u{1F9ED}', '\u{1F331}', '\u{1FA90}', '\u{1F6F0}\u{FE0F}', '\u{1F4BE}', '\u{1F6DF}', '\u{1F4E6}', '\u{1F6E1}\u{FE0F}', '\u{1F4CA}', - '\u{1F5C4}\u{FE0F}', '\u{1F50D}', '\u{267B}\u{FE0F}']; + '\u{1F4E6}', '\u{1F50D}', '\u{267B}\u{FE0F}']; // 'new' | 'restore'. Chosen on Start; everything downstream reads it. this.installMode = 'new'; // What `restore read` found: host, hosts, apps, domains, location_idx. @@ -115,6 +115,25 @@ class SetupWizard { } } + _applyModeFromQuery() { + try { + const q = new URLSearchParams(window.location.search); + if (q.get('mode') === 'restore') { + this.installMode = 'restore'; + const radio = this.container.querySelector('input[name="sw-mode"][value="restore"]'); + if (radio) radio.checked = true; + this.totalSteps = this._effectiveTotalSteps(); + this.renderRestoreSource(); + const type = q.get('type'); + const sel = this.container.querySelector('#sw-rs-type'); + if (type && sel && Array.from(sel.options).some(o => o.value === type)) { + sel.value = type; + sel.dispatchEvent(new Event('change', { bubbles: true })); + } + } + } catch { /* a malformed query must never stop the wizard opening */ } + } + _visibleSteps() { return this.stepNames.map((_, i) => i).filter((i) => this._stepVisible(i)); } @@ -140,6 +159,11 @@ class SetupWizard { // the order, and it makes a step reachable without clicking through the // ones before it — which is what lets a screenshot or a headless test look // at, say, Backups directly. + // ?mode=restore selects the restore branch, and ?type=sftp the backend + // within it, so any state of this wizard is reachable by URL. Reaching the + // restore steps otherwise takes a click and a change event, which means a + // screenshot or a test has to drive the page before it can look at it. + this._applyModeFromQuery(); this.showStep(this._stepFromQuery()); } @@ -428,7 +452,7 @@ class SetupWizard { ?
- +
@@ -775,7 +799,16 @@ class SetupWizard { // Not an unmounted one: choosing it would put app data on a bare // mountpoint, which is the failure this whole feature exists to avoid. .filter(c => c.verdict !== 'refuse' && c.state !== 'unmounted') - .forEach(c => opts.push({ value: c.path, label: c.name ? `${c.name} (${c.path})` : c.path })); + .forEach(c => opts.push({ + value: c.path, + label: c.name ? `${c.name} (${c.path})` : c.path, + // The drive, as opposed to the directory app data lives in. A + // registered location's path is usually a subdirectory of its mount, + // so anything derived from the path rather than the mount ends up + // nested inside the app data. + mount: c.mount || c.path, + registered: !!c.registered + })); // Somewhere the scan did not find: a NAS mount, an LVM volume, a directory // on a drive already in use. The scan lists whole filesystems, so anything // that is a path rather than a disk had no way in before this. @@ -880,12 +913,30 @@ class SetupWizard { } // Where a chosen drive puts things when nobody says otherwise. + // + // Both defaults hang off the drive's MOUNT POINT, never off the selected + // location's path. A location's path is where app data goes and is usually a + // subdirectory — /mnt/disk/apps — so deriving the system tree from it gave + // /mnt/disk/apps/libreportal-system: LibrePortal's own files buried inside + // the app data, on a path that reads as a mistake because it is one. + // + // The names follow the rest of the product's layout — libreportal-system, + // libreportal-containers, libreportal-backups — rather than a bare "apps", + // so a drive shared with anything else stays legible. _defaultPathFor(which, drive) { if (!drive || drive === 'primary') { return which === 'system' ? '' : (this.storagePrimary || ''); } - const base = drive.replace(/\/$/, ''); - return which === 'system' ? `${base}/libreportal-system` : base; + const opt = (this._storageChoices() || []).find(o => o.value === drive); + const mount = String((opt && opt.mount) || drive).replace(/\/$/, ''); + + if (which === 'system') return `${mount}/libreportal-system`; + + // An already-registered location keeps its own path: it exists, it may + // already hold data, and quietly proposing a different directory on the + // same drive would strand it. + if (opt && opt.registered) return String(drive).replace(/\/$/, ''); + return `${mount}/libreportal-apps`; } // Exact paths — advanced only. @@ -1018,8 +1069,8 @@ class SetupWizard { if (!box || box.dataset.rendered === '1') return; box.dataset.rendered = '1'; - const field = (id, label, tip, icon, ph, type) => ` -
+ const field = (id, label, tip, icon, ph) => ` +
`; + // The fields a backend needs, boxed together under its own heading. + // Loose rows appearing and disappearing under the type dropdown gave no + // signal that they belong to the choice above them; a titled container + // makes "these three are the SFTP part" visible at a glance. + const group = (type, title, note, fields) => ` +
+
${this.escapeHtml(title)}
+ ${note ? `

${this.escapeHtml(note)}

` : ''} + ${fields} +
`; + box.innerHTML = `
` + - field('sw-rs-path', 'Folder', - "The repository folder itself \u2014 the one containing config, data/ and snapshots/, not the folder above it.", - '\u{1F4C1}', '/mnt/usb/libreportal-backups', 'local') + - field('sw-rs-ssh-user', 'SSH user', - 'The account used to reach the server over SSH.', - '\u{1F464}', 'backups', 'sftp') + - field('sw-rs-ssh-host', 'SSH host', - 'Hostname or IP of the server holding the repository.', - '\u{1F5A5}\u{FE0F}', 'nas.example.com', 'sftp') + - field('sw-rs-ssh-path', 'Remote folder', - 'Path to the repository on that server.', - '\u{1F4C1}', '/srv/libreportal-backups', 'sftp') + - field('sw-rs-uri', 'Server URL', - 'The full repository URL, as restic writes it.', - '\u{1F517}', 'rest:https://backup.example.com/', 'rest') + - field('sw-rs-s3uri', 'Bucket URL', - 'The bucket holding the repository, as restic writes it.', - '\u{1FAA3}', 's3:s3.amazonaws.com/my-bucket', 's3') + - field('sw-rs-s3id', 'Access key ID', - 'The key ID for that bucket. Its secret goes in the password field below.', - '\u{1F511}', 'AKIA\u2026', 's3') + - field('sw-rs-b2uri', 'Bucket', - 'The B2 bucket holding the repository.', - '\u{1FAA3}', 'b2:my-bucket', 'b2') + - field('sw-rs-b2id', 'Account ID', - 'Your B2 account or application key ID.', - '\u{1F194}', '', 'b2'); + group('local', 'On this machine', + 'A folder on a disk plugged into this server, or mounted on it.', + field('sw-rs-path', 'Folder', + "The repository folder itself \u2014 the one containing config, data/ and snapshots/, not the folder above it.", + '\u{1F4C1}', '/mnt/usb/libreportal-backups')) + + group('sftp', 'SFTP server', + 'Reached over SSH, with the key or password this server already uses.', + field('sw-rs-ssh-user', 'SSH user', + 'The account used to reach the server over SSH.', + '\u{1F464}', 'backups') + + field('sw-rs-ssh-host', 'SSH host', + 'Hostname or IP of the server holding the repository.', + '\u{1F310}', 'nas.example.com') + + field('sw-rs-ssh-path', 'Remote folder', + 'Path to the repository on that server.', + '\u{1F4C1}', '/srv/libreportal-backups')) + + group('rest', 'REST server', '', + field('sw-rs-uri', 'Server URL', + 'The full repository URL, as restic writes it.', + '\u{1F517}', 'rest:https://backup.example.com/')) + + group('s3', 'S3 bucket', + 'The secret key goes in the password field below, not here.', + field('sw-rs-s3uri', 'Bucket URL', + 'The bucket holding the repository, as restic writes it.', + '\u{1FAA3}', 's3:s3.amazonaws.com/my-bucket') + + field('sw-rs-s3id', 'Access key ID', + 'The key ID for that bucket.', + '\u{1F511}', 'AKIA\u2026')) + + group('b2', 'Backblaze B2', + 'The application key goes in the password field below.', + field('sw-rs-b2uri', 'Bucket', + 'The B2 bucket holding the repository.', + '\u{1FAA3}', 'b2:my-bucket') + + field('sw-rs-b2id', 'Account ID', + 'Your B2 account or application key ID.', + '\u{1F194}', '')); const sync = () => { const type = (this.container.querySelector('#sw-rs-type') || {}).value || 'local'; @@ -1987,13 +2058,18 @@ class SetupWizard { )); } - // Drives to register: whatever either dropdown points at. Selecting a drive - // IS the request to register it — there is no separate tick to forget. + // Drives to register as storage LOCATIONS. Selecting a drive for app data IS + // the request to register it — there is no separate tick to forget. + // + // The LibrePortal row is deliberately not included. A storage location is + // somewhere app data lives; LibrePortal's own tree is not app data, and + // libreportal-relocate creates that directory itself as root. Registering + // the drive because it was picked there produced a location the user never + // asked for, pointing at a mount they had chosen for something else. collectStorage() { const out = []; - [this.storageSystemChoice, this.storageDefault].forEach((v) => { - if (v && v !== 'primary' && !out.includes(v)) out.push(v); - }); + const v = this.storageDefault; + if (v && v !== 'primary') out.push(v); return out; } diff --git a/docs/roadmap/storage-locations.md b/docs/roadmap/storage-locations.md index 8315801..bb905cc 100644 --- a/docs/roadmap/storage-locations.md +++ b/docs/roadmap/storage-locations.md @@ -631,6 +631,39 @@ checked nothing at all. They now probe the WebUI with `lp-shot --url` and curl: if it answers HTTP then the browser is the only thing that can have broken, and that is a failure, not a skip. +## 12.65 — Default paths hung off the wrong thing + +The advanced Storage step offered `/mnt/disk/apps/libreportal-system` as the +default place for LibrePortal's own tree, which is wrong twice over. + +A registered location's **path is where app data goes**, and is usually a +subdirectory of the drive — `/mnt/disk/apps`. Deriving anything else from it +nests that thing inside the app data. LibrePortal's own files are not app data +and have no business living under it. + +Both defaults now hang off the location's **mount point**, which meant adding +`mount` to each entry in the storage feed (only the `system` block carried one +before): + +| | before | after | +|---|---|---| +| LibrePortal | `/mnt/disk/apps/libreportal-system` | `/mnt/disk/libreportal-system` | +| New apps, unregistered drive | `/mnt/disk` | `/mnt/disk/libreportal-apps` | +| New apps, registered location | its own path | unchanged | + +The names follow the layout the rest of the product already uses — +`libreportal-system`, `libreportal-containers`, `libreportal-backups` — rather +than a bare `apps`, so a drive shared with anything else stays legible. A +location that is already registered keeps its own path: it exists, it may +already hold data, and proposing a different directory on the same drive would +strand it. + +`collectStorage()` no longer registers the drive picked for LibrePortal. A +storage location is somewhere *app data* lives; the system tree is not app +data, and `libreportal-relocate` creates that directory itself as root. Picking +a drive there was producing a location the user never asked for, pointing at a +mount they had chosen for something else entirely. + ## 12.7 — The relocate hand-off, and why it stays a command Asked directly: why is moving LibrePortal itself not just a button that runs at diff --git a/scripts/dev/lp-restore-wizard-test b/scripts/dev/lp-restore-wizard-test index a2703e5..2ec0bde 100755 --- a/scripts/dev/lp-restore-wizard-test +++ b/scripts/dev/lp-restore-wizard-test @@ -67,6 +67,35 @@ read -r -d '' DRIVE <<'JS' out.passwordHasIcon = !!document.querySelector('#sw-rs-pass') ?.closest('.setup-input-row')?.querySelector('.setup-field-icon'); + // Fields for one backend are boxed under their own heading. Loose rows + // appearing beneath the type dropdown gave no signal that they belonged to + // the choice above them. + const shownGroup = () => { + const g = Array.from(document.querySelectorAll('.setup-subgroup')) + .filter(el => el.style.display !== 'none'); + return g.length === 1 ? g[0].querySelector('.setup-subgroup-title').textContent.trim() : null; + }; + const selType = (v) => { const s = $('#sw-rs-type'); s.value = v; fire(s, 'change'); }; + selType('sftp'); out.groupForSftp = shownGroup(); + selType('b2'); out.groupForB2 = shownGroup(); + selType('local'); out.groupForLocal = shownGroup(); + + // Every visible field must clear the one above it. The step's 16px gap only + // reaches .setup-step's DIRECT children, and these sit a level deeper inside + // a .setup-section — so each input ran straight into the next field's label. + selType('sftp'); + w.showStep(1); + await new Promise(r => setTimeout(r, 200)); + const onScreen = Array.from(document.querySelectorAll('.setup-step[data-step="9"] .setup-field')) + .filter(el => el.offsetParent !== null); + let minGap = Infinity; + for (let i = 1; i < onScreen.length; i++) { + const a = onScreen[i - 1].getBoundingClientRect(), b = onScreen[i].getBoundingClientRect(); + minGap = Math.min(minGap, Math.round(b.top - a.bottom)); + } + out.visibleFieldCount = onScreen.length; + out.smallestFieldGap = onScreen.length > 1 ? minGap : null; + out.kinds = Array.from(document.querySelectorAll('#sw-rs-type option')).map(o => o.value); const groupsFor = (t) => { const sel = $('#sw-rs-type'); sel.value = t; fire(sel, 'change'); @@ -96,11 +125,23 @@ read -r -d '' DRIVE <<'JS' let stashedValue = null, sentBody = null; w.stashSecret = async (v) => { stashedValue = v; return 'secret:' + '0'.repeat(32); }; const realFetch = window.fetch; + // Both halves are stubbed, not just the POST. readBackup polls the published + // document for a full minute before giving up, and leaving that loop running + // kept the page from ever going network-idle — the whole eval then died on + // the harness's 90s cap, which reads as "the browser failed" rather than as + // a test that never finished. window.fetch = async (url, opts) => { - if (String(url).includes('/api/setup/restore/read')) { + const u = String(url); + if (u.includes('/api/setup/restore/read')) { sentBody = JSON.parse(opts.body); return { ok: true, json: async () => ({ ok: true, taskId: 't', nonce: 'n' }) }; } + if (u.includes('/data/system/restore_read.json')) { + return { ok: true, json: async () => ({ + nonce: 'n', host: 'oldbox', hosts: ['oldbox'], + system: { present: true, date: '2026-08-28T13:10:02+01:00', domains: [] }, + apps: [] }) }; + } return realFetch(url, opts); }; $('#sw-rs-pass').value = 'hunter2-not-a-real-password'; @@ -112,12 +153,9 @@ read -r -d '' DRIVE <<'JS' out.payloadCarriesRef = !!(sentBody && sentBody.location && sentBody.location.password_ref); out.payloadCarriesNoPassword = !!(sentBody && sentBody.location && !JSON.stringify(sentBody.location).includes('hunter2')); + // Now that the poll is stubbed too, the read completes rather than hanging. + await readPromise.catch(() => {}); window.fetch = realFetch; - // Deliberately NOT awaited: the stubbed response has no matching document to - // find, so readBackup polls for a full minute before giving up. What is - // under test already happened — what left the browser — and waiting for the - // timeout only makes the test take a minute longer than it needs to. - readPromise.catch(() => {}); // The Contents step must present the two snapshot KINDS as two things. A // repository holds one system=config snapshot and one per app, restored by @@ -184,6 +222,10 @@ chk "fields carry icons" "$(g .fieldsHaveIcons)" true chk "fields carry tooltips" "$(g .fieldsHaveTooltips)" true chk "so does the password field" "$(g .passwordHasIcon)" true chk "local shows only its own" "$(g .localShowsOnlyLocal)" true +chk "sftp fields are boxed together" "$(g .groupForSftp)" "SFTP server" +chk "b2 fields are boxed together" "$(g .groupForB2)" "Backblaze B2" +chk "local fields are boxed too" "$(g .groupForLocal)" "On this machine" +chk "no field overlaps the next" "$(g '.smallestFieldGap >= 8')" true chk "sftp shows only its own" "$(g .sftpShowsOnlySftp)" true chk "empty path refused" "$(g .emptyPathRefused)" true chk "relative path refused" "$(g .relativePathRefused)" true diff --git a/scripts/dev/lp-storage-step-test b/scripts/dev/lp-storage-step-test index 0d92293..ee39fb1 100755 --- a/scripts/dev/lp-storage-step-test +++ b/scripts/dev/lp-storage-step-test @@ -115,7 +115,19 @@ read -r -d '' DRIVE <<'JS' sysSel.value = disk; fire(sysSel, 'change'); out.systemRowAppears = !!$('#sw-path-system'); out.systemPathDefault = $('#sw-path-system') ? $('#sw-path-system').value : ''; - out.systemPathWant = disk + '/libreportal-system'; + // Off the drive's MOUNT, never the location's path. A location's path is + // where app data goes and is usually a subdirectory, so deriving from it + // produced /mnt/disk/apps/libreportal-system — LibrePortal's own tree buried + // inside the app data. + const chosen = (w._storageChoices() || []).find(o => o.value === disk); + const mount = String((chosen && chosen.mount) || disk).replace(/\/$/, ''); + out.systemPathWant = mount + '/libreportal-system'; + out.systemPathIsOutsideAppData = !out.systemPathDefault.startsWith(disk.replace(/\/$/, '') + '/'); + // A storage location is somewhere APP DATA lives. LibrePortal's own tree is + // not app data, and relocate creates that directory itself — registering the + // drive because it was picked here produced a location nobody asked for. + out.systemChoiceRegistersNothingExtra = + (w.collectStorage() || []).every(v => v !== mount + '/libreportal-system'); // The system directory is not an app-data location and must never be // registered as one — collectStorage takes the drive, not the subdirectory. out.registersDriveNotSystemDir = (w.collectStorage() || []).includes(out.systemPathDefault); @@ -227,6 +239,8 @@ else chk "path follows the chosen drive" "$(g .pathFollowsDrive)" "$(g .pathFollowsDriveWant)" chk "LibrePortal row appears" "$(g .systemRowAppears)" true chk "and defaults under that drive" "$(g .systemPathDefault)" "$(g .systemPathWant)" + chk "not nested in the app data" "$(g .systemPathIsOutsideAppData)" true + chk "and registers no extra location" "$(g .systemChoiceRegistersNothingExtra)" true chk "system dir is not registered" "$(g .registersDriveNotSystemDir)" false echo "editing a path" diff --git a/scripts/webui/data/generators/system/webui_storage_candidates.sh b/scripts/webui/data/generators/system/webui_storage_candidates.sh index 2ebbde1..33dcd4c 100644 --- a/scripts/webui/data/generators/system/webui_storage_candidates.sh +++ b/scripts/webui/data/generators/system/webui_storage_candidates.sh @@ -53,6 +53,14 @@ webuiGenerateStorageCandidates() loc_pct=$(df -Pk "$path" 2>/dev/null | awk 'NR==2 {gsub("%","",$5); print $5}') locations+="{\"id\":\"$(_lpJsonEsc "$id")\",\"name\":\"$(_lpJsonEsc "$name")\",\"path\":\"$(_lpJsonEsc "$path")\",\"state\":\"$(_lpJsonEsc "$state")\",\"apps\":\"$(_lpJsonEsc "$apps")\"" locations+=",\"size\":\"$(_lpJsonEsc "$loc_size")\",\"free\":\"$(_lpJsonEsc "$loc_free")\"" + # The MOUNT POINT, not just the path. A location's path is where app + # data goes — often a subdirectory like /mnt/disk/apps — and deriving + # anything else from it nests that thing inside the app data. The + # wizard needs the drive itself to offer a sane default for + # LibrePortal's own tree. + local loc_mount + loc_mount=$(findmnt -no TARGET --target "$path" 2>/dev/null | tail -1) + locations+=",\"mount\":\"$(_lpJsonEsc "${loc_mount:-$path}")\"" locations+=",\"fstype\":\"$(_lpJsonEsc "$loc_fs")\",\"used_pct\":${loc_pct:-0},\"removable\":false}" done < <(runStorage verify 2>/dev/null) locations+="]"