mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-24 14:47:23 +00:00
fix(web): surface Sync's destructive-confirm gate in the admin UI
startSync() used to POST once and, on any 2xx, render a hardcoded "Presets: OK / Recents: OK / Sources: OK" regardless of what the response actually said -- exactly why a silent partial data loss (see the previous commit) would have looked like success to the user. Now: on a 409 (destructive) response, build a specific confirm message from the diff (e.g. "presets: 6 -> 5: Ici Roussillon") and gate via window.confirm(), matching the existing QuickFix confirm UX; on confirm, retry with ?confirmed=true. On success, render the actual per-resource counts from the response body instead of a canned string. Adds an HTTP-level regression test (TestHandleInitialSync_DestructiveSyncReturns409ThenAppliesWhenConfirmed) covering the same refuse-then-confirm flow through the real handler and router, complementing the lower-level setup package test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
2bac4fb208
commit
7fa70d725a
@@ -0,0 +1,153 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
"github.com/gesellix/bose-soundtouch/pkg/models"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
|
||||
"github.com/gesellix/bose-soundtouch/pkg/service/setup"
|
||||
"github.com/go-chi/chi/v5"
|
||||
)
|
||||
|
||||
// TestHandleInitialSync_DestructiveSyncReturns409ThenAppliesWhenConfirmed is
|
||||
// an HTTP-level regression test for #614's Sync-button data-loss bug (see
|
||||
// setup.TestSyncDeviceData_DestructiveSyncRequiresConfirmation for the
|
||||
// lower-level coverage of the same fix): a device already has more presets
|
||||
// stored than the mock speaker's live /presets now reports. The first,
|
||||
// unconfirmed sync request must come back 409 with the diff and must not
|
||||
// write anything; a retry with ?confirmed=true must apply it.
|
||||
func TestHandleInitialSync_DestructiveSyncReturns409ThenAppliesWhenConfirmed(t *testing.T) {
|
||||
const (
|
||||
accountID = "1234567"
|
||||
deviceID = "AABBCCDDEEFF"
|
||||
)
|
||||
|
||||
// A real local server, not a black-hole IP: notifySpeakerSourcesUpdated
|
||||
// (part of the confirmed-apply path) uses its own HTTP client rather
|
||||
// than the injectable sm.HTTPGet, so it needs somewhere real to fail
|
||||
// fast against (404) instead of timing out.
|
||||
mockDevice := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.URL.Path {
|
||||
case "/info":
|
||||
w.Header().Set("Content-Type", "application/xml")
|
||||
fmt.Fprintf(w, `<?xml version="1.0" encoding="UTF-8"?><info deviceID="%s"><name>Test Device</name><type>SoundTouch 20</type><margeAccountUUID>%s</margeAccountUUID></info>`, deviceID, accountID)
|
||||
case "/presets":
|
||||
w.Header().Set("Content-Type", "application/xml")
|
||||
fmt.Fprint(w, `<?xml version="1.0" encoding="UTF-8"?><presets><preset id="1"><ContentItem source="LOCAL_INTERNET_RADIO" type="stationurl" location="/x" isPresetable="true"><itemName>Station 1</itemName></ContentItem></preset></presets>`)
|
||||
case "/recents":
|
||||
w.Header().Set("Content-Type", "application/xml")
|
||||
fmt.Fprint(w, `<?xml version="1.0" encoding="UTF-8"?><recents></recents>`)
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
defer mockDevice.Close()
|
||||
|
||||
deviceIP := mockDevice.Listener.Addr().String()
|
||||
|
||||
tempDir, err := os.MkdirTemp("", "handlers-sync-destructive-guard-*")
|
||||
if err != nil {
|
||||
t.Fatalf("tempdir: %v", err)
|
||||
}
|
||||
defer func() { _ = os.RemoveAll(tempDir) }()
|
||||
|
||||
ds := datastore.NewDataStore(tempDir)
|
||||
|
||||
seeded := []models.ServicePreset{
|
||||
{ID: "1", ButtonNumber: "1", ServiceContentItem: models.ServiceContentItem{Name: "Station 1"}},
|
||||
{ID: "2", ButtonNumber: "2", ServiceContentItem: models.ServiceContentItem{Name: "Station 2"}},
|
||||
}
|
||||
if err := ds.SavePresets(accountID, deviceID, seeded); err != nil {
|
||||
t.Fatalf("seed SavePresets: %v", err)
|
||||
}
|
||||
|
||||
if err := ds.SaveDeviceInfo(accountID, deviceID, &models.ServiceDeviceInfo{
|
||||
DeviceID: deviceID,
|
||||
AccountID: accountID,
|
||||
IPAddress: deviceIP,
|
||||
}); err != nil {
|
||||
t.Fatalf("SaveDeviceInfo: %v", err)
|
||||
}
|
||||
|
||||
sm := setup.NewManager("http://localhost:8000", ds, nil)
|
||||
|
||||
server := NewServer(ds, sm, "http://localhost:8000", false, false, false)
|
||||
|
||||
r := chi.NewRouter()
|
||||
r.Post("/api/setup/sync/{deviceId}", server.HandleInitialSync)
|
||||
|
||||
ts := httptest.NewServer(r)
|
||||
defer ts.Close()
|
||||
|
||||
// First, unconfirmed request: must be refused with 409.
|
||||
resp, err := http.Post(ts.URL+"/api/setup/sync/"+deviceID, "application/json", nil)
|
||||
if err != nil {
|
||||
t.Fatalf("POST sync (unconfirmed): %v", err)
|
||||
}
|
||||
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
if resp.StatusCode != http.StatusConflict {
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
t.Fatalf("expected 409 for a destructive unconfirmed sync, got %d: %s", resp.StatusCode, body)
|
||||
}
|
||||
|
||||
var result setup.SyncResult
|
||||
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
|
||||
t.Fatalf("decode 409 body: %v", err)
|
||||
}
|
||||
|
||||
if result.Applied {
|
||||
t.Fatal("expected Applied=false in the 409 response")
|
||||
}
|
||||
|
||||
if !result.Destructive {
|
||||
t.Fatal("expected Destructive=true in the 409 response")
|
||||
}
|
||||
|
||||
presetsAfterRefusal, err := ds.GetPresets(accountID, deviceID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetPresets after refused sync: %v", err)
|
||||
}
|
||||
|
||||
if len(presetsAfterRefusal) != 2 {
|
||||
t.Fatalf("expected the original 2 presets to survive the refused sync, got %d", len(presetsAfterRefusal))
|
||||
}
|
||||
|
||||
// Retry, confirmed: must apply.
|
||||
resp2, err := http.Post(ts.URL+"/api/setup/sync/"+deviceID+"?confirmed=true", "application/json", nil)
|
||||
if err != nil {
|
||||
t.Fatalf("POST sync (confirmed): %v", err)
|
||||
}
|
||||
|
||||
defer func() { _ = resp2.Body.Close() }()
|
||||
|
||||
if resp2.StatusCode != http.StatusOK {
|
||||
body, _ := io.ReadAll(resp2.Body)
|
||||
t.Fatalf("expected 200 for a confirmed sync, got %d: %s", resp2.StatusCode, body)
|
||||
}
|
||||
|
||||
var confirmedResult setup.SyncResult
|
||||
if err := json.NewDecoder(resp2.Body).Decode(&confirmedResult); err != nil {
|
||||
t.Fatalf("decode 200 body: %v", err)
|
||||
}
|
||||
|
||||
if !confirmedResult.Applied {
|
||||
t.Fatal("expected Applied=true after confirming")
|
||||
}
|
||||
|
||||
presetsAfterConfirm, err := ds.GetPresets(accountID, deviceID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetPresets after confirmed sync: %v", err)
|
||||
}
|
||||
|
||||
if len(presetsAfterConfirm) != 1 {
|
||||
t.Fatalf("expected confirmed sync to shrink to 1 preset, got %d", len(presetsAfterConfirm))
|
||||
}
|
||||
}
|
||||
@@ -876,6 +876,37 @@ function getDeviceDisplayName(deviceId) {
|
||||
return deviceId;
|
||||
}
|
||||
|
||||
// buildSyncConfirmMessage renders a human-readable summary of a destructive
|
||||
// SyncResult (see setup.SyncResult/SyncResourceDiff) for window.confirm() —
|
||||
// e.g. "Sync would remove 1 preset: Ici Roussillon. Continue?".
|
||||
function buildSyncConfirmMessage(result) {
|
||||
const lines = ["This Data Sync would remove data that's currently stored:"];
|
||||
for (const diff of result.diffs || []) {
|
||||
if (!diff.destructive) {
|
||||
continue;
|
||||
}
|
||||
const removedNote = diff.removed && diff.removed.length ? ": " + diff.removed.join(", ") : "";
|
||||
lines.push("- " + diff.resource + ": " + diff.currentCount + " → " + diff.incomingCount + removedNote);
|
||||
}
|
||||
lines.push("This usually means the speaker's own live data was incomplete at this moment. Continue anyway?");
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
async function requestSync(deviceId, confirmed) {
|
||||
let url = "/api/setup/sync/" + encodeURIComponent(deviceId);
|
||||
if (confirmed) {
|
||||
url += "?confirmed=true";
|
||||
}
|
||||
const response = await fetch(url, {method: "POST"});
|
||||
let result = null;
|
||||
try {
|
||||
result = await response.clone().json();
|
||||
} catch (e) {
|
||||
// Non-JSON error body (e.g. a plain-text 500) — handled below via response.text().
|
||||
}
|
||||
return {response, result};
|
||||
}
|
||||
|
||||
async function startSync() {
|
||||
const deviceId = document.getElementById("sync-device-list").value;
|
||||
if (!deviceId) {
|
||||
@@ -895,14 +926,29 @@ async function startSync() {
|
||||
log.innerHTML = "";
|
||||
|
||||
try {
|
||||
const response = await fetch("/api/setup/sync/" + encodeURIComponent(deviceId), {method: "POST"},);
|
||||
if (response.ok) {
|
||||
let {response, result} = await requestSync(deviceId, false);
|
||||
|
||||
if (response.status === 409 && result) {
|
||||
if (!confirm(buildSyncConfirmMessage(result))) {
|
||||
status.style.backgroundColor = "#eef";
|
||||
status.textContent = "Sync cancelled for " + display + " — nothing was changed.";
|
||||
return;
|
||||
}
|
||||
|
||||
({response, result} = await requestSync(deviceId, true));
|
||||
}
|
||||
|
||||
if (response.ok && result) {
|
||||
status.style.backgroundColor = "#dfd";
|
||||
status.textContent = "✅ Sync completed successfully for " + display + "!";
|
||||
results.style.display = "block";
|
||||
log.textContent = "Data fetched and saved to local datastore for " + display + ".\nPresets: OK\nRecents: OK\nSources: OK";
|
||||
const lines = (result.diffs || []).map(
|
||||
(diff) => diff.resource + ": " + diff.currentCount + " → " + diff.incomingCount,
|
||||
);
|
||||
lines.push("sources: synced");
|
||||
log.textContent = "Data fetched and saved to local datastore for " + display + ".\n" + lines.join("\n");
|
||||
} else {
|
||||
const err = await response.text();
|
||||
const err = result ? JSON.stringify(result) : await response.text();
|
||||
throw new Error(err);
|
||||
}
|
||||
} catch (error) {
|
||||
|
||||
Reference in New Issue
Block a user