diff --git a/app/pages/settings.py b/app/pages/settings.py index 48c7f95..4cbec11 100644 --- a/app/pages/settings.py +++ b/app/pages/settings.py @@ -53,20 +53,24 @@ def settings_page(workspace_id: str) -> None: return None return parsed if parsed > 0 else None - def as_non_negative_float(value: object) -> float: + def as_non_negative_float(value: object) -> float | None: try: parsed = float(value) except (TypeError, ValueError): - return 0.0 - return max(parsed, 0.0) + return None + return parsed if parsed >= 0 else None def build_preview_config() -> PortfolioConfig: + parsed_loan_amount = as_non_negative_float(loan_amount.value) + if parsed_loan_amount is None: + raise ValueError("Loan amount must be zero or greater") + return PortfolioConfig( gold_value=as_positive_float(gold_value.value), entry_price=as_positive_float(entry_price.value), gold_ounces=as_positive_float(gold_ounces.value), entry_basis_mode=str(entry_basis_mode.value), - loan_amount=as_non_negative_float(loan_amount.value), + loan_amount=parsed_loan_amount, margin_threshold=float(margin_threshold.value), monthly_budget=float(monthly_budget.value), ltv_warning=float(ltv_warning.value), @@ -125,12 +129,14 @@ def settings_page(workspace_id: str) -> None: step=0.01, ).classes("w-full") - loan_amount = ui.number( - "Loan amount ($)", - value=config.loan_amount, - min=0, - step=1000, - ).classes("w-full") + loan_amount = ( + ui.input( + "Loan amount ($)", + value=str(config.loan_amount), + ) + .props("type=number min=0 step=1000") + .classes("w-full") + ) margin_threshold = ui.number( "Margin call LTV threshold", @@ -337,7 +343,7 @@ def settings_page(workspace_id: str) -> None: loan = as_non_negative_float(loan_amount.value) margin = as_positive_float(margin_threshold.value) - if collateral_value is not None and collateral_value > 0: + if collateral_value is not None and collateral_value > 0 and loan is not None: ltv = (loan / collateral_value) * 100 buffer = ((margin or 0.0) - loan / collateral_value) * 100 if margin is not None else 0.0 ltv_display.set_text(f"{ltv:.1f}%") @@ -346,9 +352,15 @@ def settings_page(workspace_id: str) -> None: ltv_display.set_text("—") buffer_display.set_text("—") - if margin is not None and ounces is not None and ounces > 0: + if loan is not None and margin is not None and ounces is not None and ounces > 0: margin_price_display.set_text(f"${loan / (margin * ounces):,.2f}/oz") - elif margin is not None and price is not None and collateral_value is not None and collateral_value > 0: + elif ( + loan is not None + and margin is not None + and price is not None + and collateral_value is not None + and collateral_value > 0 + ): implied_ounces = collateral_value / price margin_price_display.set_text(f"${loan / (margin * implied_ounces):,.2f}/oz") else: diff --git a/docs/roadmap/done/CORE-001D3A-alerts-settings-entrypoint-normalization.yaml b/docs/roadmap/done/CORE-001D3A-alerts-settings-entrypoint-normalization.yaml index 837f160..08346ed 100644 --- a/docs/roadmap/done/CORE-001D3A-alerts-settings-entrypoint-normalization.yaml +++ b/docs/roadmap/done/CORE-001D3A-alerts-settings-entrypoint-normalization.yaml @@ -17,4 +17,5 @@ completed_notes: - Alert evaluation now compares thresholds through normalized Decimal-backed boundary values instead of ad-hoc `float(...)` extraction. - Settings save-status formatting now snapshots numeric boundary values through a named adapter before rendering. - Added focused regression tests for numeric-string coercion and bool fail-closed behavior in `tests/test_alerts.py` and `tests/test_settings.py`. + - Hardened `/{workspace_id}/settings` so blank loan input no longer silently saves as a zero-loan portfolio; the loan field now preserves blank state until validation and has a focused Playwright regression in `tests/test_settings_validation_playwright.py`. - Validated with focused pytest coverage, browser-visible Playwright coverage, and `make build` on local Docker. diff --git a/tests/test_settings_validation_playwright.py b/tests/test_settings_validation_playwright.py new file mode 100644 index 0000000..5f9450a --- /dev/null +++ b/tests/test_settings_validation_playwright.py @@ -0,0 +1,39 @@ +from __future__ import annotations + +from playwright.sync_api import expect, sync_playwright + +BASE_URL = "http://127.0.0.1:8000" + + +def test_settings_reject_invalid_loan_amount_without_silent_zero_fallback() -> None: + with sync_playwright() as p: + browser = p.chromium.launch(headless=True) + page = browser.new_page(viewport={"width": 1440, "height": 1000}) + + page.goto(BASE_URL, wait_until="domcontentloaded", timeout=30000) + expect(page.locator("text=Create a private workspace URL").first).to_be_visible(timeout=10000) + page.get_by_role("button", name="Get started").click() + page.wait_for_url(f"{BASE_URL}/*", timeout=15000) + workspace_url = page.url + + page.goto(f"{workspace_url}/settings", wait_until="domcontentloaded", timeout=30000) + expect(page.locator("text=Settings").first).to_be_visible(timeout=15000) + loan_input = page.get_by_label("Loan amount ($)") + loan_input.fill("") + loan_input.press("Tab") + + expect(page.locator("text=INVALID").first).to_be_visible(timeout=15000) + expect(page.locator("text=Loan amount must be zero or greater").first).to_be_visible(timeout=15000) + + page.get_by_role("button", name="Save settings").click() + expect(page.locator("text=Validation error: Loan amount must be zero or greater").first).to_be_visible( + timeout=15000 + ) + + settings_text = page.locator("body").inner_text(timeout=15000) + assert "LTV=0.0%" not in settings_text + assert "RuntimeError" not in settings_text + assert "Server error" not in settings_text + assert "Traceback" not in settings_text + + browser.close()