refactor(settings): separate preview validation from internal failures

app/pages/settings.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import logging
+
 from fastapi.responses import RedirectResponse
 from nicegui import ui
 
@@ -9,6 +11,8 @@ from app.pages.common import dashboard_page, split_page_panes
 from app.services.alerts import AlertService, build_portfolio_alert_context
 from app.services.settings_status import save_status_text
 
+logger = logging.getLogger(__name__)
+
 
 def _alert_badge_classes(severity: str) -> str:
     return {
@@ -18,6 +22,23 @@ def _alert_badge_classes(severity: str) -> str:
     }.get(severity, "rounded-full bg-slate-100 px-3 py-1 text-xs font-semibold text-slate-700")
 
 
+def _save_card_status_text(
+    last_saved_config: PortfolioConfig,
+    *,
+    preview_config: PortfolioConfig | None = None,
+    invalid: bool = False,
+    save_failed: bool = False,
+) -> str:
+    base = save_status_text(last_saved_config).replace("Saved:", "Last saved:", 1)
+    if save_failed:
+        return f"Save failed — {base}"
+    if invalid:
+        return f"Unsaved invalid changes — {base}"
+    if preview_config is not None and preview_config.to_dict() != last_saved_config.to_dict():
+        return f"Unsaved changes — {base}"
+    return base
+
+
 def _render_workspace_recovery() -> None:
     with ui.column().classes("mx-auto mt-24 w-full max-w-2xl gap-6 px-6 text-center"):
         ui.icon("folder_off").classes("mx-auto text-6xl text-slate-400")
@@ -268,7 +289,7 @@ def settings_page(workspace_id: str) -> None:
                 "w-full rounded-2xl border border-slate-200 bg-white shadow-sm dark:border-slate-800 dark:bg-slate-900"
             ):
                 ui.label("Save Workspace Settings").classes("text-lg font-semibold text-slate-900 dark:text-slate-100")
-                status = ui.label(last_saved_status_text(last_saved_config)).classes(
+                status = ui.label(_save_card_status_text(last_saved_config)).classes(
                     "text-sm text-slate-500 dark:text-slate-400"
                 )
                 ui.button("Save settings", on_click=lambda: save_settings()).props("color=primary")
@@ -291,6 +312,17 @@ def settings_page(workspace_id: str) -> None:
         def render_alert_state() -> None:
             try:
                 preview_config = build_preview_config()
+            except (ValueError, TypeError) as exc:
+                alert_state_container.clear()
+                with alert_state_container:
+                    ui.label("INVALID").classes(_alert_badge_classes("critical"))
+                email_state_label.set_text("Fix validation errors to preview alert state")
+                alert_message.set_text(str(exc))
+                status.set_text(_save_card_status_text(last_saved_config, invalid=True))
+                alert_history_column.clear()
+                return
+
+            try:
                 alert_status = alert_service.evaluate(
                     preview_config,
                     build_portfolio_alert_context(
@@ -301,43 +333,45 @@ def settings_page(workspace_id: str) -> None:
                     ),
                     persist=False,
                 )
+            except Exception:
+                logger.exception("Settings alert preview failed for workspace %s", workspace_id)
                 alert_state_container.clear()
                 with alert_state_container:
-                    ui.label(alert_status.severity.upper()).classes(_alert_badge_classes(alert_status.severity))
+                    ui.label("UNAVAILABLE").classes(_alert_badge_classes("critical"))
-                email_state_label.set_text(
+                email_state_label.set_text("Preview unavailable due to an internal error")
-                    f"Email alerts {'enabled' if alert_status.email_alerts_enabled else 'disabled'} · Warning {alert_status.warning_threshold:.0%} · Critical {alert_status.critical_threshold:.0%}"
+                alert_message.set_text(
+                    "Preview unavailable due to an internal error. Last saved settings remain unchanged."
                 )
-                alert_message.set_text(alert_status.message)
+                status.set_text(_save_card_status_text(last_saved_config, preview_config=preview_config))
-                if preview_config.to_dict() == last_saved_config.to_dict():
-                    status.set_text(last_saved_status_text(last_saved_config))
-                else:
-                    status.set_text(f"Unsaved changes — {last_saved_status_text(last_saved_config)}")
                 alert_history_column.clear()
-                if alert_status.history:
+                return
-                    for event in alert_status.history[:5]:
+
-                        with alert_history_column:
+            alert_state_container.clear()
-                            with ui.row().classes(
+            with alert_state_container:
-                                "w-full items-start justify-between gap-3 rounded-lg bg-slate-50 p-3 dark:bg-slate-800"
+                ui.label(alert_status.severity.upper()).classes(_alert_badge_classes(alert_status.severity))
-                            ):
+            email_state_label.set_text(
-                                with ui.column().classes("gap-1"):
+                f"Email alerts {'enabled' if alert_status.email_alerts_enabled else 'disabled'} · Warning {alert_status.warning_threshold:.0%} · Critical {alert_status.critical_threshold:.0%}"
-                                    ui.label(event.message).classes(
+            )
-                                        "text-sm font-medium text-slate-900 dark:text-slate-100"
+            alert_message.set_text(alert_status.message)
-                                    )
+            status.set_text(_save_card_status_text(last_saved_config, preview_config=preview_config))
-                                    ui.label(f"Spot ${event.spot_price:,.2f} · LTV {event.ltv_ratio:.1%}").classes(
+            alert_history_column.clear()
-                                        "text-xs text-slate-500 dark:text-slate-400"
+            if alert_status.history:
-                                    )
+                for event in alert_status.history[:5]:
-                                ui.label(event.severity.upper()).classes(_alert_badge_classes(event.severity))
-                else:
                     with alert_history_column:
-                        ui.label("No alert history yet.").classes("text-sm text-slate-500 dark:text-slate-400")
+                        with ui.row().classes(
-            except (ValueError, TypeError) as exc:
+                            "w-full items-start justify-between gap-3 rounded-lg bg-slate-50 p-3 dark:bg-slate-800"
-                alert_state_container.clear()
+                        ):
-                with alert_state_container:
+                            with ui.column().classes("gap-1"):
-                    ui.label("INVALID").classes(_alert_badge_classes("critical"))
+                                ui.label(event.message).classes(
-                email_state_label.set_text("Fix validation errors to preview alert state")
+                                    "text-sm font-medium text-slate-900 dark:text-slate-100"
-                alert_message.set_text(str(exc))
+                                )
-                status.set_text(f"Unsaved invalid changes — {last_saved_status_text(last_saved_config)}")
+                                ui.label(f"Spot ${event.spot_price:,.2f} · LTV {event.ltv_ratio:.1%}").classes(
-                alert_history_column.clear()
+                                    "text-xs text-slate-500 dark:text-slate-400"
+                                )
+                            ui.label(event.severity.upper()).classes(_alert_badge_classes(event.severity))
+            else:
+                with alert_history_column:
+                    ui.label("No alert history yet.").classes("text-sm text-slate-500 dark:text-slate-400")
 
         def update_entry_basis(*_args: object) -> None:
             nonlocal syncing_entry_basis
@@ -427,11 +461,12 @@ def settings_page(workspace_id: str) -> None:
                 workspace_repo.save_portfolio_config(workspace_id, new_config)
                 last_saved_config = new_config
                 render_alert_state()
-                status.set_text(last_saved_status_text(last_saved_config))
+                status.set_text(_save_card_status_text(last_saved_config))
                 ui.notify("Settings saved successfully", color="positive")
             except ValueError as e:
-                status.set_text(f"Unsaved invalid changes — {last_saved_status_text(last_saved_config)}")
+                status.set_text(_save_card_status_text(last_saved_config, invalid=True))
                 ui.notify(f"Validation error: {e}", color="negative")
-            except Exception as e:
+            except Exception:
-                status.set_text(f"Save failed — {last_saved_status_text(last_saved_config)}")
+                logger.exception("Failed to save settings for workspace %s", workspace_id)
-                ui.notify(f"Failed to save: {e}", color="negative")
+                status.set_text(_save_card_status_text(last_saved_config, save_failed=True))
+                ui.notify("Failed to save settings. Check logs for details.", color="negative")

tests/test_settings_page.py

@@ -0,0 +1,40 @@
+from __future__ import annotations
+
+from app.models.portfolio import PortfolioConfig
+from app.pages.settings import _save_card_status_text
+
+
+def test_save_card_status_text_for_clean_state() -> None:
+    config = PortfolioConfig(gold_value=215_000.0, entry_price=215.0, loan_amount=145_000.0)
+
+    text = _save_card_status_text(config)
+
+    assert text.startswith("Last saved:")
+    assert "Unsaved changes" not in text
+    assert "Unsaved invalid changes" not in text
+    assert "Save failed" not in text
+
+
+def test_save_card_status_text_for_dirty_state() -> None:
+    last_saved = PortfolioConfig(gold_value=215_000.0, entry_price=215.0, loan_amount=145_000.0)
+    preview = PortfolioConfig(gold_value=215_000.0, entry_price=215.0, loan_amount=150_000.0)
+
+    text = _save_card_status_text(last_saved, preview_config=preview)
+
+    assert text.startswith("Unsaved changes — Last saved:")
+
+
+def test_save_card_status_text_for_invalid_state() -> None:
+    config = PortfolioConfig(gold_value=215_000.0, entry_price=215.0, loan_amount=145_000.0)
+
+    text = _save_card_status_text(config, invalid=True)
+
+    assert text.startswith("Unsaved invalid changes — Last saved:")
+
+
+def test_save_card_status_text_for_save_failure() -> None:
+    config = PortfolioConfig(gold_value=215_000.0, entry_price=215.0, loan_amount=145_000.0)
+
+    text = _save_card_status_text(config, save_failed=True)
+
+    assert text.startswith("Save failed — Last saved:")