fix(settings): clarify last-saved status state
This commit is contained in:
@@ -42,6 +42,7 @@ def settings_page(workspace_id: str) -> None:
|
|||||||
return RedirectResponse(url="/", status_code=307)
|
return RedirectResponse(url="/", status_code=307)
|
||||||
|
|
||||||
config = workspace_repo.load_portfolio_config(workspace_id)
|
config = workspace_repo.load_portfolio_config(workspace_id)
|
||||||
|
last_saved_config = config
|
||||||
alert_service = AlertService()
|
alert_service = AlertService()
|
||||||
|
|
||||||
syncing_entry_basis = False
|
syncing_entry_basis = False
|
||||||
@@ -69,6 +70,9 @@ def settings_page(workspace_id: str) -> None:
|
|||||||
return str(int(parsed))
|
return str(int(parsed))
|
||||||
return str(parsed)
|
return str(parsed)
|
||||||
|
|
||||||
|
def last_saved_status_text(config: PortfolioConfig) -> str:
|
||||||
|
return save_status_text(config).replace("Saved:", "Last saved:", 1)
|
||||||
|
|
||||||
def build_preview_config() -> PortfolioConfig:
|
def build_preview_config() -> PortfolioConfig:
|
||||||
parsed_loan_amount = as_non_negative_float(loan_amount.value)
|
parsed_loan_amount = as_non_negative_float(loan_amount.value)
|
||||||
if parsed_loan_amount is None:
|
if parsed_loan_amount is None:
|
||||||
@@ -251,10 +255,9 @@ 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"
|
"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")
|
ui.label("Save Workspace Settings").classes("text-lg font-semibold text-slate-900 dark:text-slate-100")
|
||||||
status = ui.label(
|
status = ui.label(last_saved_status_text(last_saved_config)).classes(
|
||||||
f"Current: start=${config.gold_value:,.0f}, entry=${config.entry_price:,.2f}/oz, "
|
"text-sm text-slate-500 dark:text-slate-400"
|
||||||
f"weight={config.gold_ounces:,.2f} oz, current LTV={config.current_ltv:.1%}"
|
)
|
||||||
).classes("text-sm text-slate-500 dark:text-slate-400")
|
|
||||||
ui.button("Save settings", on_click=lambda: save_settings()).props("color=primary")
|
ui.button("Save settings", on_click=lambda: save_settings()).props("color=primary")
|
||||||
|
|
||||||
def apply_entry_basis_mode() -> None:
|
def apply_entry_basis_mode() -> None:
|
||||||
@@ -292,6 +295,10 @@ def settings_page(workspace_id: str) -> None:
|
|||||||
f"Email alerts {'enabled' if alert_status.email_alerts_enabled else 'disabled'} · Warning {alert_status.warning_threshold:.0%} · Critical {alert_status.critical_threshold:.0%}"
|
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(alert_status.message)
|
alert_message.set_text(alert_status.message)
|
||||||
|
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()
|
alert_history_column.clear()
|
||||||
if alert_status.history:
|
if alert_status.history:
|
||||||
for event in alert_status.history[:5]:
|
for event in alert_status.history[:5]:
|
||||||
@@ -316,6 +323,7 @@ def settings_page(workspace_id: str) -> None:
|
|||||||
ui.label("INVALID").classes(_alert_badge_classes("critical"))
|
ui.label("INVALID").classes(_alert_badge_classes("critical"))
|
||||||
email_state_label.set_text("Fix validation errors to preview alert state")
|
email_state_label.set_text("Fix validation errors to preview alert state")
|
||||||
alert_message.set_text(str(exc))
|
alert_message.set_text(str(exc))
|
||||||
|
status.set_text(f"Unsaved invalid changes — {last_saved_status_text(last_saved_config)}")
|
||||||
alert_history_column.clear()
|
alert_history_column.clear()
|
||||||
|
|
||||||
def update_entry_basis(*_args: object) -> None:
|
def update_entry_basis(*_args: object) -> None:
|
||||||
@@ -389,14 +397,17 @@ def settings_page(workspace_id: str) -> None:
|
|||||||
update_entry_basis()
|
update_entry_basis()
|
||||||
|
|
||||||
def save_settings() -> None:
|
def save_settings() -> None:
|
||||||
|
nonlocal last_saved_config
|
||||||
try:
|
try:
|
||||||
new_config = build_preview_config()
|
new_config = build_preview_config()
|
||||||
workspace_repo.save_portfolio_config(workspace_id, new_config)
|
workspace_repo.save_portfolio_config(workspace_id, new_config)
|
||||||
|
last_saved_config = new_config
|
||||||
render_alert_state()
|
render_alert_state()
|
||||||
|
status.set_text(last_saved_status_text(last_saved_config))
|
||||||
status.set_text(save_status_text(new_config))
|
|
||||||
ui.notify("Settings saved successfully", color="positive")
|
ui.notify("Settings saved successfully", color="positive")
|
||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
|
status.set_text(f"Unsaved invalid changes — {last_saved_status_text(last_saved_config)}")
|
||||||
ui.notify(f"Validation error: {e}", color="negative")
|
ui.notify(f"Validation error: {e}", color="negative")
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
status.set_text(f"Save failed — {last_saved_status_text(last_saved_config)}")
|
||||||
ui.notify(f"Failed to save: {e}", color="negative")
|
ui.notify(f"Failed to save: {e}", color="negative")
|
||||||
|
|||||||
@@ -17,5 +17,5 @@ completed_notes:
|
|||||||
- Alert evaluation now compares thresholds through normalized Decimal-backed boundary values instead of ad-hoc `float(...)` extraction.
|
- 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.
|
- 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`.
|
- 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`.
|
- 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, keeps whole-dollar formatting stable, and marks the save card as `Last saved` / `Unsaved invalid changes` instead of presenting stale saved state as current.
|
||||||
- Validated with focused pytest coverage, browser-visible Playwright coverage, and `make build` on local Docker.
|
- Validated with focused pytest coverage, browser-visible Playwright coverage, and `make build` on local Docker.
|
||||||
|
|||||||
@@ -18,12 +18,14 @@ def test_settings_reject_invalid_loan_amount_without_silent_zero_fallback() -> N
|
|||||||
|
|
||||||
page.goto(f"{workspace_url}/settings", wait_until="domcontentloaded", timeout=30000)
|
page.goto(f"{workspace_url}/settings", wait_until="domcontentloaded", timeout=30000)
|
||||||
expect(page.locator("text=Settings").first).to_be_visible(timeout=15000)
|
expect(page.locator("text=Settings").first).to_be_visible(timeout=15000)
|
||||||
|
expect(page.locator("text=Last saved:").first).to_be_visible(timeout=15000)
|
||||||
loan_input = page.get_by_label("Loan amount ($)")
|
loan_input = page.get_by_label("Loan amount ($)")
|
||||||
loan_input.fill("")
|
loan_input.fill("")
|
||||||
loan_input.press("Tab")
|
loan_input.press("Tab")
|
||||||
|
|
||||||
expect(page.locator("text=INVALID").first).to_be_visible(timeout=15000)
|
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)
|
expect(page.locator("text=Loan amount must be zero or greater").first).to_be_visible(timeout=15000)
|
||||||
|
expect(page.locator("text=Unsaved invalid changes — Last saved:").first).to_be_visible(timeout=15000)
|
||||||
|
|
||||||
page.get_by_role("button", name="Save settings").click()
|
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(
|
expect(page.locator("text=Validation error: Loan amount must be zero or greater").first).to_be_visible(
|
||||||
@@ -38,5 +40,6 @@ def test_settings_reject_invalid_loan_amount_without_silent_zero_fallback() -> N
|
|||||||
|
|
||||||
page.reload(wait_until="domcontentloaded", timeout=30000)
|
page.reload(wait_until="domcontentloaded", timeout=30000)
|
||||||
expect(page.get_by_label("Loan amount ($)")).to_have_value("145000")
|
expect(page.get_by_label("Loan amount ($)")).to_have_value("145000")
|
||||||
|
expect(page.locator("text=Last saved:").first).to_be_visible(timeout=15000)
|
||||||
|
|
||||||
browser.close()
|
browser.close()
|
||||||
|
|||||||
Reference in New Issue
Block a user