mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 08:48:05 +00:00
fix(web): treat only 2xx as a successful display save
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aUvzTXpEYhNEWYQvFqQU9
This commit is contained in:
@@ -225,6 +225,27 @@ class TestConfigAPI:
|
|||||||
assert 'chain length' in response.get_json()['message']
|
assert 'chain length' in response.get_json()['message']
|
||||||
mock_config_manager.save_config_atomic.assert_not_called()
|
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):
|
def test_save_double_sided_disabled_ignores_bad_values(self, client, mock_config_manager):
|
||||||
"""While disabled, unusable copies/axis are dropped rather than rejected."""
|
"""While disabled, unusable copies/axis are dropped rather than rejected."""
|
||||||
mock_config_manager.load_config.return_value['display']['double_sided'] = {
|
mock_config_manager.load_config.return_value['display']['double_sided'] = {
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ import uuid
|
|||||||
import logging
|
import logging
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Dict, Any
|
from typing import Dict, Any, Optional
|
||||||
from urllib.parse import urlparse, urlunparse
|
from urllib.parse import urlparse, urlunparse
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
@@ -871,7 +871,7 @@ def save_main_config():
|
|||||||
enabled = _coerce_to_bool(data.get('double_sided_enabled'))
|
enabled = _coerce_to_bool(data.get('double_sided_enabled'))
|
||||||
ds_config['enabled'] = 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."""
|
"""Error message if copies doesn't divide the panel evenly, else None."""
|
||||||
# Use axis from this request if provided, else from stored config.
|
# Use axis from this request if provided, else from stored config.
|
||||||
hw = current_config.get('display', {}).get('hardware', {})
|
hw = current_config.get('display', {}).get('hardware', {})
|
||||||
|
|||||||
@@ -615,12 +615,18 @@ if (typeof window.fixInvalidNumberInputs !== 'function') {
|
|||||||
// `responseJSON` (that's a jQuery property) — read `responseText` and the real
|
// `responseJSON` (that's a jQuery property) — read `responseText` and the real
|
||||||
// status code, otherwise a failed save reports success.
|
// status code, otherwise a failed save reports success.
|
||||||
window.showDisplaySaveResult = function(xhr) {
|
window.showDisplaySaveResult = function(xhr) {
|
||||||
let message = 'Display settings saved';
|
// Only 2xx counts as saved. A network failure reports status 0, which any
|
||||||
let status = xhr.status >= 400 ? 'error' : 'success';
|
// `>= 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 {
|
try {
|
||||||
const data = JSON.parse(xhr.responseText);
|
const data = JSON.parse(xhr.responseText);
|
||||||
if (data.message) message = data.message;
|
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 {
|
} catch {
|
||||||
// Non-JSON body — fall back to the status-code verdict above.
|
// Non-JSON body — fall back to the status-code verdict above.
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user