From b67f9e4a2a6a1ac86243949e66fecce2d28192f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 20:23:35 +0000 Subject: [PATCH] fix(web): treat only 2xx as a successful display save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #422. - showDisplaySaveResult tested `xhr.status >= 400` for failure, so a network error — which reports status 0 — was waved through as "Display settings saved". That's the same class of false-success bug this branch set out to fix. Test the 2xx range instead, and let a response body refine a successful verdict without overturning a failed one. - Annotate the _copies_fits_hardware helper, matching the annotated helpers already in api_v3.py. - Cover the vertical divisibility branch: chain_length 2 would divide evenly, so only parallel 3 can produce the rejection, which pins the branch to the right hardware dimension. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016aUvzTXpEYhNEWYQvFqQU9 --- test/test_web_api.py | 21 +++++++++++++++++++ web_interface/blueprints/api_v3.py | 4 ++-- .../templates/v3/partials/display.html | 12 ++++++++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/test/test_web_api.py b/test/test_web_api.py index 2bfe46bb..d65b11ff 100644 --- a/test/test_web_api.py +++ b/test/test_web_api.py @@ -225,6 +225,27 @@ class TestConfigAPI: assert 'chain length' in response.get_json()['message'] mock_config_manager.save_config_atomic.assert_not_called() + def test_save_double_sided_vertical_checks_parallel(self, client, mock_config_manager): + """The vertical axis is checked against parallel, not chain_length.""" + mock_config_manager.load_config.return_value['display']['hardware'] = { + 'chain_length': 2, 'parallel': 3, + } + + response = client.post( + '/api/v3/config/main', + data={ + 'double_sided_enabled': 'true', + 'double_sided_copies': '2', + 'double_sided_axis': 'vertical', + }, + content_type='application/x-www-form-urlencoded', + ) + + # chain_length 2 would divide evenly — only parallel 3 rejects this. + assert response.status_code == 400 + assert 'parallel' in response.get_json()['message'] + mock_config_manager.save_config_atomic.assert_not_called() + def test_save_double_sided_disabled_ignores_bad_values(self, client, mock_config_manager): """While disabled, unusable copies/axis are dropped rather than rejected.""" mock_config_manager.load_config.return_value['display']['double_sided'] = { diff --git a/web_interface/blueprints/api_v3.py b/web_interface/blueprints/api_v3.py index ae1a8c38..f92c7852 100644 --- a/web_interface/blueprints/api_v3.py +++ b/web_interface/blueprints/api_v3.py @@ -13,7 +13,7 @@ import uuid import logging from datetime import datetime from pathlib import Path -from typing import Dict, Any +from typing import Dict, Any, Optional from urllib.parse import urlparse, urlunparse logger = logging.getLogger(__name__) @@ -871,7 +871,7 @@ def save_main_config(): enabled = _coerce_to_bool(data.get('double_sided_enabled')) ds_config['enabled'] = enabled - def _copies_fits_hardware(copies): + def _copies_fits_hardware(copies: int) -> Optional[str]: """Error message if copies doesn't divide the panel evenly, else None.""" # Use axis from this request if provided, else from stored config. hw = current_config.get('display', {}).get('hardware', {}) diff --git a/web_interface/templates/v3/partials/display.html b/web_interface/templates/v3/partials/display.html index 0add70c7..80cc0709 100644 --- a/web_interface/templates/v3/partials/display.html +++ b/web_interface/templates/v3/partials/display.html @@ -615,12 +615,18 @@ if (typeof window.fixInvalidNumberInputs !== 'function') { // `responseJSON` (that's a jQuery property) — read `responseText` and the real // status code, otherwise a failed save reports success. window.showDisplaySaveResult = function(xhr) { - let message = 'Display settings saved'; - let status = xhr.status >= 400 ? 'error' : 'success'; + // Only 2xx counts as saved. A network failure reports status 0, which any + // `>= 400` test would wave through as success. + const httpSuccess = xhr.status >= 200 && xhr.status < 300; + let message = httpSuccess + ? 'Display settings saved' + : 'Display settings were not saved. Check your connection and try again.'; + let status = httpSuccess ? 'success' : 'error'; try { const data = JSON.parse(xhr.responseText); if (data.message) message = data.message; - if (data.status) status = data.status; + // A body can refine a successful verdict but never overturn a failed one. + if (httpSuccess && data.status) status = data.status; } catch { // Non-JSON body — fall back to the status-code verdict above. }