fix(journal): wire weather refresh on config save; drop orphaned cache rows
Two related gaps in the journal weather panel:
1. Saving locations via PUT /journal/config didn't trigger a weather
fetch, so newly-entered sites had no cache row (or a stale one) until
the user manually clicked the panel's refresh button. The panel
rendered "two sites with empty values" against pre-existing cache
rows that no longer matched what the user had configured.
2. get_cached_weather_rows returned every WeatherCache row for the user
regardless of whether the location was still in journal_config.
Briefing-era rows survived migration 0040 (which only deleted the
briefing_config setting, not the cache table) and showed up as
ghost tabs in the UI.
Changes:
- get_cached_weather_rows accepts an optional valid_keys filter; rows
whose location_key is not in the set are excluded.
- routes/journal.py:
- put_config kicks off a background refresh_location_cache for any
saved location with valid lat/lon.
- GET /weather and POST /weather/refresh both pass valid_keys derived
from the current config so orphaned rows don't surface.
- services/journal_prep.py filters the weather section to currently-
configured locations as well; uses a lazy import of get_journal_config
to avoid a cycle (journal_scheduler imports journal_prep).
153 tests pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -66,6 +66,17 @@ async def _resolve_config(user_id: int) -> dict:
|
||||
return {**DEFAULT_JOURNAL_CONFIG, **config}
|
||||
|
||||
|
||||
def _valid_location_keys(cfg: dict) -> set[str]:
|
||||
"""Keys in ``cfg.locations`` that have a usable lat/lon. Anything else
|
||||
(orphaned cache rows, locations the user typed but didn't geocode) is
|
||||
excluded so it can't render as a fake site in the UI."""
|
||||
locations = cfg.get("locations") or {}
|
||||
return {
|
||||
key for key, loc in locations.items()
|
||||
if isinstance(loc, dict) and loc.get("lat") is not None and loc.get("lon") is not None
|
||||
}
|
||||
|
||||
|
||||
@journal_bp.get("/config")
|
||||
@login_required
|
||||
async def get_config():
|
||||
@@ -82,9 +93,41 @@ async def put_config():
|
||||
return jsonify({"error": "config must be an object"}), 400
|
||||
await set_setting(user_id, "journal_config", json.dumps(body))
|
||||
await update_user_schedule(user_id)
|
||||
|
||||
# Trigger a background weather refresh for any newly-saved location with
|
||||
# valid lat/lon. Without this, the cache row for the location doesn't
|
||||
# exist (or stays stale) until the user clicks the manual refresh button,
|
||||
# so the journal weather panel renders empty for newly-entered sites.
|
||||
valid_locs = [
|
||||
(key, loc)
|
||||
for key, loc in (body.get("locations") or {}).items()
|
||||
if isinstance(loc, dict) and loc.get("lat") is not None and loc.get("lon") is not None
|
||||
]
|
||||
if valid_locs:
|
||||
asyncio.create_task(_refresh_locations_in_background(user_id, valid_locs))
|
||||
|
||||
return jsonify({"ok": True})
|
||||
|
||||
|
||||
async def _refresh_locations_in_background(
|
||||
user_id: int, locations: list[tuple[str, dict]]
|
||||
) -> None:
|
||||
for key, loc in locations:
|
||||
try:
|
||||
await weather_svc.refresh_location_cache(
|
||||
user_id=user_id,
|
||||
location_key=key,
|
||||
location_label=loc.get("label", key),
|
||||
lat=loc["lat"],
|
||||
lon=loc["lon"],
|
||||
)
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Post-save weather refresh failed for user %d / %s",
|
||||
user_id, key, exc_info=True,
|
||||
)
|
||||
|
||||
|
||||
@journal_bp.get("/today")
|
||||
@login_required
|
||||
async def get_today():
|
||||
@@ -258,7 +301,9 @@ async def _refresh_stale_in_background(user_id: int, stale_keys: set[str]) -> No
|
||||
@login_required
|
||||
async def get_weather():
|
||||
user_id = get_current_user_id()
|
||||
rows = await weather_svc.get_cached_weather_rows(user_id)
|
||||
cfg = await _resolve_config(user_id)
|
||||
valid_keys = _valid_location_keys(cfg)
|
||||
rows = await weather_svc.get_cached_weather_rows(user_id, valid_keys)
|
||||
temp_unit = await _journal_temp_unit(user_id)
|
||||
|
||||
# Kick off a best-effort background refresh for stale rows so the next page
|
||||
@@ -318,7 +363,8 @@ async def refresh_weather():
|
||||
)
|
||||
except Exception:
|
||||
logger.warning("Failed to refresh weather for %s", key, exc_info=True)
|
||||
rows = await weather_svc.get_cached_weather_rows(user_id)
|
||||
valid_keys = _valid_location_keys(cfg)
|
||||
rows = await weather_svc.get_cached_weather_rows(user_id, valid_keys)
|
||||
cards = [
|
||||
card for row in rows
|
||||
if (card := weather_svc.parse_weather_card_data(row, temp_unit)) is not None
|
||||
|
||||
@@ -178,7 +178,15 @@ async def gather_daily_sections(
|
||||
sections["events"] = []
|
||||
|
||||
try:
|
||||
weather_rows = await get_cached_weather_rows(user_id)
|
||||
# Lazy import: journal_scheduler imports this module for prep generation,
|
||||
# so a top-level import would cycle.
|
||||
from fabledassistant.services.journal_scheduler import get_journal_config
|
||||
cfg = await get_journal_config(user_id)
|
||||
valid_weather_keys = {
|
||||
key for key, loc in (cfg.get("locations") or {}).items()
|
||||
if isinstance(loc, dict) and loc.get("lat") is not None and loc.get("lon") is not None
|
||||
}
|
||||
weather_rows = await get_cached_weather_rows(user_id, valid_weather_keys)
|
||||
sections["weather"] = [w.to_dict() for w in weather_rows]
|
||||
except Exception:
|
||||
logger.exception("daily_prep weather section failed for user %d", user_id)
|
||||
|
||||
@@ -232,12 +232,23 @@ def parse_weather_card_data(
|
||||
}
|
||||
|
||||
|
||||
async def get_cached_weather_rows(user_id: int) -> list:
|
||||
"""Return raw WeatherCache ORM rows for a user (for card parsing)."""
|
||||
async def get_cached_weather_rows(
|
||||
user_id: int,
|
||||
valid_keys: set[str] | None = None,
|
||||
) -> list:
|
||||
"""Return raw WeatherCache ORM rows for a user (for card parsing).
|
||||
|
||||
If ``valid_keys`` is provided, only rows whose ``location_key`` is in the
|
||||
set are returned. This is how callers drop orphaned cache rows whose
|
||||
location is no longer in the user's ``journal_config.locations`` (e.g.
|
||||
leftovers from the briefing era, or a location that's been removed).
|
||||
Passing an empty set returns no rows.
|
||||
"""
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(WeatherCache).where(WeatherCache.user_id == user_id)
|
||||
)
|
||||
stmt = select(WeatherCache).where(WeatherCache.user_id == user_id)
|
||||
if valid_keys is not None:
|
||||
stmt = stmt.where(WeatherCache.location_key.in_(list(valid_keys)))
|
||||
result = await session.execute(stmt)
|
||||
return list(result.scalars().all())
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user