mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-11-01 18:52:21 +01:00
fix(settings): reconcile admin toggle on partial cleanup failure and report errors
- In imagePreferences.js, distinguish pre-persistence failures from post-save cleanup failures using err.globalSaved and err.phase. - In imagePreferences.js, notify listeners of global policy updates on partial success before propagating cleanup failure. - In imageModels.js, retain the persisted target state on admin toggle when global persistence succeeds but cleanup fails. - In imageModels.js, reconcile admin toggle with authoritative server state via readImagePreferences() after ambiguous or partial failures. - In imageModels.js, report partial success accurately without suppressing preference cleanup error messages. - Add regression coverage in test_settings_save_response_ok_js.py for cleanup HTTP 500, cleanup network rejection, successful cleanup, and failed global POST.
This commit is contained in:
@@ -96,10 +96,15 @@ export async function initImageSettings() {
|
|||||||
if (!res.ok) throw new Error(await res.text().catch(() => `HTTP ${res.status}`));
|
if (!res.ok) throw new Error(await res.text().catch(() => `HTTP ${res.status}`));
|
||||||
msg.textContent = 'Saved'; msg.style.color = 'var(--fg)'; setTimeout(() => { msg.textContent = ''; }, 2000);
|
msg.textContent = 'Saved'; msg.style.color = 'var(--fg)'; setTimeout(() => { msg.textContent = ''; }, 2000);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
if (e.globalSaved) {
|
||||||
|
msg.textContent = 'Policy saved, but failed to clear preferences: ' + (e.message || 'cleanup error');
|
||||||
|
msg.style.color = 'var(--red)';
|
||||||
|
} else {
|
||||||
msg.textContent = e.status === 403 ? 'Admin access is required to change these settings.' : 'Failed to save';
|
msg.textContent = e.status === 403 ? 'Admin access is required to change these settings.' : 'Failed to save';
|
||||||
msg.style.color = 'var(--red)';
|
msg.style.color = 'var(--red)';
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async function saveToggle() {
|
async function saveToggle() {
|
||||||
if (!checkIsAdmin()) {
|
if (!checkIsAdmin()) {
|
||||||
@@ -109,18 +114,39 @@ export async function initImageSettings() {
|
|||||||
msg.style.color = 'var(--red)';
|
msg.style.color = 'var(--red)';
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
const targetState = enabledToggle ? enabledToggle.checked : false;
|
||||||
try {
|
try {
|
||||||
const res = await _postSettings({ image_gen_enabled: enabledToggle ? enabledToggle.checked : false });
|
const res = await _postSettings({ image_gen_enabled: targetState });
|
||||||
if (!res.ok) throw new Error(await res.text().catch(() => `HTTP ${res.status}`));
|
if (!res.ok) throw new Error(await res.text().catch(() => `HTTP ${res.status}`));
|
||||||
if (enabledToggle) currentEnabled = enabledToggle.checked;
|
if (enabledToggle) currentEnabled = enabledToggle.checked;
|
||||||
msg.textContent = 'Saved'; msg.style.color = 'var(--fg)'; setTimeout(() => { msg.textContent = ''; }, 2000);
|
msg.textContent = 'Saved'; msg.style.color = 'var(--fg)'; setTimeout(() => { msg.textContent = ''; }, 2000);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
if (e.globalSaved) {
|
||||||
|
currentEnabled = targetState;
|
||||||
|
try {
|
||||||
|
const authSettings = await readImagePreferences();
|
||||||
|
if (typeof authSettings.image_gen_enabled === 'boolean') {
|
||||||
|
currentEnabled = authSettings.image_gen_enabled;
|
||||||
|
}
|
||||||
|
} catch (_) {}
|
||||||
|
if (enabledToggle) enabledToggle.checked = currentEnabled;
|
||||||
|
syncImgDisabled();
|
||||||
|
msg.textContent = 'Policy saved, but failed to clear preferences: ' + (e.message || 'cleanup error');
|
||||||
|
msg.style.color = 'var(--red)';
|
||||||
|
} else {
|
||||||
|
try {
|
||||||
|
const authSettings = await readImagePreferences();
|
||||||
|
if (typeof authSettings.image_gen_enabled === 'boolean') {
|
||||||
|
currentEnabled = authSettings.image_gen_enabled;
|
||||||
|
}
|
||||||
|
} catch (_) {}
|
||||||
if (enabledToggle) enabledToggle.checked = currentEnabled;
|
if (enabledToggle) enabledToggle.checked = currentEnabled;
|
||||||
syncImgDisabled();
|
syncImgDisabled();
|
||||||
msg.textContent = e.status === 403 ? 'Admin access is required to change these settings.' : 'Failed to save';
|
msg.textContent = e.status === 403 ? 'Admin access is required to change these settings.' : 'Failed to save';
|
||||||
msg.style.color = 'var(--red)';
|
msg.style.color = 'var(--red)';
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (modelSel) modelSel.addEventListener('change', saveModelPreferences);
|
if (modelSel) modelSel.addEventListener('change', saveModelPreferences);
|
||||||
if (editModelSel) editModelSel.addEventListener('change', saveModelPreferences);
|
if (editModelSel) editModelSel.addEventListener('change', saveModelPreferences);
|
||||||
|
|||||||
@@ -22,16 +22,24 @@ let saveQueue = Promise.resolve();
|
|||||||
export function waitForImagePreferences() { return saveQueue; }
|
export function waitForImagePreferences() { return saveQueue; }
|
||||||
export function saveImagePreferences(patch) {
|
export function saveImagePreferences(patch) {
|
||||||
const save = async () => {
|
const save = async () => {
|
||||||
let response = await fetch('/api/auth/settings', {
|
let response;
|
||||||
|
try {
|
||||||
|
response = await fetch('/api/auth/settings', {
|
||||||
method: 'POST', credentials: 'same-origin',
|
method: 'POST', credentials: 'same-origin',
|
||||||
headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(patch),
|
headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(patch),
|
||||||
});
|
});
|
||||||
|
} catch (netErr) {
|
||||||
|
invalidateSettings();
|
||||||
|
netErr.globalSaved = false;
|
||||||
|
throw netErr;
|
||||||
|
}
|
||||||
invalidateSettings();
|
invalidateSettings();
|
||||||
if (response.status === 403) {
|
if (response.status === 403) {
|
||||||
const userEntries = Object.entries(patch).filter(([k]) => USER_PREFERENCE_KEYS.includes(k));
|
const userEntries = Object.entries(patch).filter(([k]) => USER_PREFERENCE_KEYS.includes(k));
|
||||||
if (userEntries.length === 0) {
|
if (userEntries.length === 0) {
|
||||||
const err = new Error('Admin access is required to change these settings.');
|
const err = new Error('Admin access is required to change these settings.');
|
||||||
err.status = 403;
|
err.status = 403;
|
||||||
|
err.globalSaved = false;
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
for (const [key, value] of userEntries) {
|
for (const [key, value] of userEntries) {
|
||||||
@@ -43,6 +51,7 @@ export function saveImagePreferences(patch) {
|
|||||||
if (!prefRes.ok) {
|
if (!prefRes.ok) {
|
||||||
const err = new Error(await prefRes.text().catch(() => 'Could not save image preferences'));
|
const err = new Error(await prefRes.text().catch(() => 'Could not save image preferences'));
|
||||||
err.status = prefRes.status;
|
err.status = prefRes.status;
|
||||||
|
err.globalSaved = false;
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -70,16 +79,26 @@ export function saveImagePreferences(patch) {
|
|||||||
const failure = clearResults.find(r => !r.ok);
|
const failure = clearResults.find(r => !r.ok);
|
||||||
if (failure) {
|
if (failure) {
|
||||||
invalidateSettings();
|
invalidateSettings();
|
||||||
|
if (typeof window !== 'undefined' && typeof CustomEvent !== 'undefined') {
|
||||||
|
window.dispatchEvent(new CustomEvent('image-preferences-changed', { detail: patch }));
|
||||||
|
}
|
||||||
const err = new Error(failure.error?.message || 'Could not clear personal preferences');
|
const err = new Error(failure.error?.message || 'Could not clear personal preferences');
|
||||||
err.status = failure.error?.status || 500;
|
err.status = failure.error?.status || 500;
|
||||||
err.cause = failure.error;
|
err.cause = failure.error;
|
||||||
|
err.globalSaved = true;
|
||||||
|
err.phase = 'cleanup';
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
throw new Error('Could not save image settings');
|
const err = new Error('Could not save image settings');
|
||||||
|
err.status = response.status;
|
||||||
|
err.globalSaved = false;
|
||||||
|
throw err;
|
||||||
}
|
}
|
||||||
invalidateSettings();
|
invalidateSettings();
|
||||||
|
if (typeof window !== 'undefined' && typeof CustomEvent !== 'undefined') {
|
||||||
window.dispatchEvent(new CustomEvent('image-preferences-changed', { detail: patch }));
|
window.dispatchEvent(new CustomEvent('image-preferences-changed', { detail: patch }));
|
||||||
|
}
|
||||||
return response;
|
return response;
|
||||||
};
|
};
|
||||||
const result = saveQueue.then(save);
|
const result = saveQueue.then(save);
|
||||||
|
|||||||
@@ -265,6 +265,8 @@ def test_admin_cleanup_failure_and_rejection_surfaced():
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
results.http500Threw = true;
|
results.http500Threw = true;
|
||||||
results.http500Status = err.status;
|
results.http500Status = err.status;
|
||||||
|
results.http500GlobalSaved = err.globalSaved;
|
||||||
|
results.http500Phase = err.phase;
|
||||||
}
|
}
|
||||||
|
|
||||||
// 2. Network rejection during admin preference cleanup
|
// 2. Network rejection during admin preference cleanup
|
||||||
@@ -283,6 +285,8 @@ def test_admin_cleanup_failure_and_rejection_surfaced():
|
|||||||
} catch (err) {
|
} catch (err) {
|
||||||
results.networkRejectThrew = true;
|
results.networkRejectThrew = true;
|
||||||
results.networkRejectStatus = err.status;
|
results.networkRejectStatus = err.status;
|
||||||
|
results.networkRejectGlobalSaved = err.globalSaved;
|
||||||
|
results.networkRejectPhase = err.phase;
|
||||||
}
|
}
|
||||||
|
|
||||||
// 3. Successful cleanup
|
// 3. Successful cleanup
|
||||||
@@ -308,12 +312,214 @@ def test_admin_cleanup_failure_and_rejection_surfaced():
|
|||||||
data = json.loads(proc.stdout)
|
data = json.loads(proc.stdout)
|
||||||
assert data["http500Threw"] is True
|
assert data["http500Threw"] is True
|
||||||
assert data["http500Status"] == 500
|
assert data["http500Status"] == 500
|
||||||
|
assert data["http500GlobalSaved"] is True
|
||||||
|
assert data["http500Phase"] == "cleanup"
|
||||||
assert data["networkRejectThrew"] is True
|
assert data["networkRejectThrew"] is True
|
||||||
assert data["networkRejectStatus"] == 500
|
assert data["networkRejectStatus"] == 500
|
||||||
|
assert data["networkRejectGlobalSaved"] is True
|
||||||
|
assert data["networkRejectPhase"] == "cleanup"
|
||||||
assert data["okResStatus"] == 200
|
assert data["okResStatus"] == 200
|
||||||
assert data["clearedKeysCount"] == 3
|
assert data["clearedKeysCount"] == 3
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
|
||||||
|
def test_admin_toggle_reconciliation_and_partial_failure_reporting():
|
||||||
|
"""Verify admin toggle reconciles with authoritative server state and reports partial success accurately."""
|
||||||
|
js = """
|
||||||
|
const { initImageSettings } = await import('./static/js/settings/imageModels.js');
|
||||||
|
|
||||||
|
function createEnv() {
|
||||||
|
const elements = {};
|
||||||
|
function makeElement(id, tag = 'div') {
|
||||||
|
const listeners = {};
|
||||||
|
const el = {
|
||||||
|
id, tagName: tag.toUpperCase(),
|
||||||
|
value: '', checked: false, disabled: false, title: '',
|
||||||
|
style: {},
|
||||||
|
parentElement: { title: '', style: {} },
|
||||||
|
closest: () => ({ style: {} }),
|
||||||
|
addEventListener: (evt, cb) => { listeners[evt] = listeners[evt] || []; listeners[evt].push(cb); },
|
||||||
|
dispatchEvent: (evt) => Promise.all((listeners[evt] || []).map(cb => cb())),
|
||||||
|
length: 0, appendChild: () => {}, remove: () => {}
|
||||||
|
};
|
||||||
|
elements[id] = el;
|
||||||
|
return el;
|
||||||
|
}
|
||||||
|
|
||||||
|
const modelSel = makeElement('set-imgModelSelect', 'select');
|
||||||
|
const editModelSel = makeElement('set-imgEditModelSelect', 'select');
|
||||||
|
const qualSel = makeElement('set-imgQualitySelect', 'select');
|
||||||
|
const msg = makeElement('set-imgSettingsMsg', 'span');
|
||||||
|
const enabledToggle = makeElement('set-imgEnabledToggle', 'input');
|
||||||
|
|
||||||
|
globalThis.document = {
|
||||||
|
getElementById: (id) => elements[id] || null,
|
||||||
|
createElement: (tag) => makeElement('anon-' + Math.random(), tag),
|
||||||
|
};
|
||||||
|
globalThis.window = {
|
||||||
|
_isAdmin: true,
|
||||||
|
addEventListener: () => {},
|
||||||
|
removeEventListener: () => {},
|
||||||
|
dispatchEvent: () => {}
|
||||||
|
};
|
||||||
|
globalThis.CustomEvent = class CustomEvent {};
|
||||||
|
|
||||||
|
return { modelSel, editModelSel, qualSel, msg, enabledToggle };
|
||||||
|
}
|
||||||
|
|
||||||
|
const testResults = {};
|
||||||
|
|
||||||
|
// 1. Global POST 200 + cleanup HTTP 500 failure
|
||||||
|
{
|
||||||
|
const { enabledToggle, msg } = createEnv();
|
||||||
|
let serverGlobal = true;
|
||||||
|
globalThis.fetch = async (url, opts = {}) => {
|
||||||
|
if (url === '/api/model-endpoints') return { ok: true, json: async () => [] };
|
||||||
|
if (url === '/api/prefs') return { ok: true, json: async () => ({}) };
|
||||||
|
if (url === '/api/auth/settings') {
|
||||||
|
if (opts.method === 'POST') {
|
||||||
|
const body = JSON.parse(opts.body);
|
||||||
|
serverGlobal = body.image_gen_enabled;
|
||||||
|
return { status: 200, ok: true, json: async () => ({ ok: true }) };
|
||||||
|
}
|
||||||
|
return { ok: true, json: async () => ({ image_gen_enabled: serverGlobal }) };
|
||||||
|
}
|
||||||
|
if (url.startsWith('/api/prefs/')) {
|
||||||
|
return { status: 500, ok: false, text: async () => 'DB lock' };
|
||||||
|
}
|
||||||
|
throw new Error('Unexpected URL: ' + url);
|
||||||
|
};
|
||||||
|
await initImageSettings();
|
||||||
|
enabledToggle.checked = false;
|
||||||
|
await enabledToggle.dispatchEvent('change');
|
||||||
|
testResults.s1_checked = enabledToggle.checked;
|
||||||
|
testResults.s1_server = serverGlobal;
|
||||||
|
testResults.s1_msg = msg.textContent;
|
||||||
|
testResults.s1_color = msg.style.color;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 2. Global POST 200 + cleanup network rejection
|
||||||
|
{
|
||||||
|
const { enabledToggle, msg } = createEnv();
|
||||||
|
let serverGlobal = true;
|
||||||
|
globalThis.fetch = async (url, opts = {}) => {
|
||||||
|
if (url === '/api/model-endpoints') return { ok: true, json: async () => [] };
|
||||||
|
if (url === '/api/prefs') return { ok: true, json: async () => ({}) };
|
||||||
|
if (url === '/api/auth/settings') {
|
||||||
|
if (opts.method === 'POST') {
|
||||||
|
const body = JSON.parse(opts.body);
|
||||||
|
serverGlobal = body.image_gen_enabled;
|
||||||
|
return { status: 200, ok: true, json: async () => ({ ok: true }) };
|
||||||
|
}
|
||||||
|
return { ok: true, json: async () => ({ image_gen_enabled: serverGlobal }) };
|
||||||
|
}
|
||||||
|
if (url.startsWith('/api/prefs/')) {
|
||||||
|
throw new Error('Network offline');
|
||||||
|
}
|
||||||
|
throw new Error('Unexpected URL: ' + url);
|
||||||
|
};
|
||||||
|
await initImageSettings();
|
||||||
|
enabledToggle.checked = false;
|
||||||
|
await enabledToggle.dispatchEvent('change');
|
||||||
|
testResults.s2_checked = enabledToggle.checked;
|
||||||
|
testResults.s2_server = serverGlobal;
|
||||||
|
testResults.s2_msg = msg.textContent;
|
||||||
|
testResults.s2_color = msg.style.color;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 3. Successful cleanup
|
||||||
|
{
|
||||||
|
const { enabledToggle, msg } = createEnv();
|
||||||
|
let serverGlobal = true;
|
||||||
|
globalThis.fetch = async (url, opts = {}) => {
|
||||||
|
if (url === '/api/model-endpoints') return { ok: true, json: async () => [] };
|
||||||
|
if (url === '/api/prefs') return { ok: true, json: async () => ({}) };
|
||||||
|
if (url === '/api/auth/settings') {
|
||||||
|
if (opts.method === 'POST') {
|
||||||
|
const body = JSON.parse(opts.body);
|
||||||
|
serverGlobal = body.image_gen_enabled;
|
||||||
|
return { status: 200, ok: true, json: async () => ({ ok: true }) };
|
||||||
|
}
|
||||||
|
return { ok: true, json: async () => ({ image_gen_enabled: serverGlobal }) };
|
||||||
|
}
|
||||||
|
if (url.startsWith('/api/prefs/')) {
|
||||||
|
return { status: 200, ok: true, json: async () => ({ ok: true }) };
|
||||||
|
}
|
||||||
|
throw new Error('Unexpected URL: ' + url);
|
||||||
|
};
|
||||||
|
await initImageSettings();
|
||||||
|
enabledToggle.checked = false;
|
||||||
|
await enabledToggle.dispatchEvent('change');
|
||||||
|
testResults.s3_checked = enabledToggle.checked;
|
||||||
|
testResults.s3_server = serverGlobal;
|
||||||
|
testResults.s3_msg = msg.textContent;
|
||||||
|
testResults.s3_color = msg.style.color;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 4. Failed global POST (HTTP 500)
|
||||||
|
{
|
||||||
|
const { enabledToggle, msg } = createEnv();
|
||||||
|
let serverGlobal = true;
|
||||||
|
globalThis.fetch = async (url, opts = {}) => {
|
||||||
|
if (url === '/api/model-endpoints') return { ok: true, json: async () => [] };
|
||||||
|
if (url === '/api/prefs') return { ok: true, json: async () => ({}) };
|
||||||
|
if (url === '/api/auth/settings') {
|
||||||
|
if (opts.method === 'POST') {
|
||||||
|
return { status: 500, ok: false, text: async () => 'Internal Server Error' };
|
||||||
|
}
|
||||||
|
return { ok: true, json: async () => ({ image_gen_enabled: serverGlobal }) };
|
||||||
|
}
|
||||||
|
if (url.startsWith('/api/prefs/')) {
|
||||||
|
return { status: 200, ok: true, json: async () => ({ ok: true }) };
|
||||||
|
}
|
||||||
|
throw new Error('Unexpected URL: ' + url);
|
||||||
|
};
|
||||||
|
await initImageSettings();
|
||||||
|
enabledToggle.checked = false;
|
||||||
|
await enabledToggle.dispatchEvent('change');
|
||||||
|
testResults.s4_checked = enabledToggle.checked;
|
||||||
|
testResults.s4_server = serverGlobal;
|
||||||
|
testResults.s4_msg = msg.textContent;
|
||||||
|
testResults.s4_color = msg.style.color;
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(JSON.stringify(testResults));
|
||||||
|
"""
|
||||||
|
proc = subprocess.run(["node", "--input-type=module", "-e", js], capture_output=True, text=True, check=True)
|
||||||
|
import json
|
||||||
|
data = json.loads(proc.stdout)
|
||||||
|
|
||||||
|
# 1. Successful global POST followed by HTTP 500 cleanup failure:
|
||||||
|
# Toggle does not revert, server policy was persisted, and message reports partial success + cleanup error
|
||||||
|
assert data["s1_checked"] is False
|
||||||
|
assert data["s1_server"] is False
|
||||||
|
assert "Policy saved, but failed to clear preferences" in data["s1_msg"]
|
||||||
|
assert "DB lock" in data["s1_msg"]
|
||||||
|
assert data["s1_color"] == "var(--red)"
|
||||||
|
|
||||||
|
# 2. Successful global POST followed by network rejection during cleanup:
|
||||||
|
# Toggle does not revert, server policy was persisted, and message reports network failure
|
||||||
|
assert data["s2_checked"] is False
|
||||||
|
assert data["s2_server"] is False
|
||||||
|
assert "Policy saved, but failed to clear preferences" in data["s2_msg"]
|
||||||
|
assert "Network offline" in data["s2_msg"]
|
||||||
|
assert data["s2_color"] == "var(--red)"
|
||||||
|
|
||||||
|
# 3. Successful cleanup:
|
||||||
|
# Toggle saved, server updated, success message displayed
|
||||||
|
assert data["s3_checked"] is False
|
||||||
|
assert data["s3_server"] is False
|
||||||
|
assert data["s3_msg"] == "Saved"
|
||||||
|
assert data["s3_color"] == "var(--fg)"
|
||||||
|
|
||||||
|
# 4. Failed global POST:
|
||||||
|
# Toggle reverts to authoritative server state, error displayed
|
||||||
|
assert data["s4_checked"] is True
|
||||||
|
assert data["s4_server"] is True
|
||||||
|
assert data["s4_msg"] == "Failed to save"
|
||||||
|
assert data["s4_color"] == "var(--red)"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
|
@pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH")
|
||||||
def test_non_admin_toggle_rejection_and_ui_behavior():
|
def test_non_admin_toggle_rejection_and_ui_behavior():
|
||||||
"""Verify non-admin cannot change image_gen_enabled via API or UI, and receives unambiguous feedback."""
|
"""Verify non-admin cannot change image_gen_enabled via API or UI, and receives unambiguous feedback."""
|
||||||
|
|||||||
Reference in New Issue
Block a user