feat: comprehensive code review fixes + receivers-only architecture
Security:
- Refuse startup with default secret_key in production (was just logging)
- Settings endpoint now requires admin role
- Password validation on initial setup
- DOM-based HTML sanitizer replaces regex in template previews
- Add *.log to .gitignore
Performance & reliability:
- Token refresh deduplication prevents race condition on concurrent 401s
- Theme media query listener registered once (no leak)
- IconPicker uses $derived instead of function call per render
- Snackbar uses single-batch state update instead of while loop
- Replace 11 inline hover handlers with CSS :hover in layout
Architecture - receivers-only:
- Delivery endpoints (chat_id, email, url, room_id, topic) now stored
exclusively in TargetReceiver rows, never in target.config
- Migration extracts existing delivery fields to receiver rows
- Notifier and dispatcher remove all config fallbacks
- Frontend targets page shows receivers list per target with
add/remove/toggle/test per receiver
- Single-receiver test endpoint: POST /targets/{id}/receivers/{id}/test
Code quality:
- Extract AuthLayout.svelte from login/setup (150 lines CSS dedup)
- Split telegram-bots page (754→51 lines + 3 tab components)
- Split notification-trackers page (547→432 lines + 4 components)
- Deduplicate _send_reply into shared handler.send_reply()
- Add locale column to template models, replace name-based detection
- Fix delete_notification_tracker dead protection check
- Fix check_telegram_bot query (filter by type, remove bogus OR)
- Add graceful scheduler shutdown in lifespan
- Consistent /bots?tab=telegram URLs across all nav links
i18n:
- Error page, chat actions, target types, provider types internationalized
- All new receiver UI strings in EN + RU
This commit is contained in:
@@ -6,6 +6,7 @@ and the Phase 1 entity refactor (tracker → notification_tracker, etc.).
|
||||
|
||||
import json
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.ext.asyncio import AsyncEngine
|
||||
@@ -713,3 +714,139 @@ async def migrate_target_receivers(engine: AsyncEngine) -> None:
|
||||
|
||||
if migrated:
|
||||
logger.info("Migrated %d target receivers from legacy config", migrated)
|
||||
|
||||
|
||||
async def migrate_receivers_from_config(engine: AsyncEngine) -> None:
|
||||
"""Extract delivery endpoint fields from target.config into TargetReceiver rows.
|
||||
|
||||
For each NotificationTarget that still has a delivery field (chat_id, url,
|
||||
webhook_url, email, topic, room_id) in its config JSON:
|
||||
1. Create a TargetReceiver row (if one with the same key doesn't exist)
|
||||
2. Remove the delivery field(s) from the config JSON
|
||||
|
||||
Idempotent: checks for existing receiver before creating; only strips fields
|
||||
that are still present in config.
|
||||
"""
|
||||
# Mapping: target_type -> (delivery field in config, receiver config builder)
|
||||
_DELIVERY_FIELDS: dict[str, dict[str, str]] = {
|
||||
"telegram": {"chat_id": "chat_id"},
|
||||
"webhook": {"url": "url"},
|
||||
"email": {"email": "email"},
|
||||
"discord": {"webhook_url": "webhook_url"},
|
||||
"slack": {"webhook_url": "webhook_url"},
|
||||
"ntfy": {"topic": "topic"},
|
||||
"matrix": {"room_id": "room_id"},
|
||||
}
|
||||
|
||||
async with engine.begin() as conn:
|
||||
if not await _has_table(conn, "notification_target"):
|
||||
return
|
||||
if not await _has_table(conn, "target_receiver"):
|
||||
return
|
||||
|
||||
targets = (await conn.execute(
|
||||
text("SELECT id, type, config FROM notification_target")
|
||||
)).fetchall()
|
||||
|
||||
created = 0
|
||||
cleaned = 0
|
||||
for row in targets:
|
||||
target_id, target_type, raw_config = row[0], row[1], row[2]
|
||||
try:
|
||||
cfg = json.loads(raw_config) if isinstance(raw_config, str) else (raw_config or {})
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
cfg = {}
|
||||
|
||||
field_map = _DELIVERY_FIELDS.get(target_type, {})
|
||||
if not field_map:
|
||||
continue
|
||||
|
||||
# Check if any delivery field is present in config
|
||||
delivery_field = list(field_map.keys())[0] # e.g. "chat_id", "url"
|
||||
delivery_value = cfg.get(delivery_field)
|
||||
if not delivery_value:
|
||||
continue
|
||||
|
||||
# Build receiver config
|
||||
receiver_config: dict[str, Any] = {delivery_field: delivery_value}
|
||||
# For webhook, also move headers to receiver config
|
||||
if target_type == "webhook" and "headers" in cfg:
|
||||
receiver_config["headers"] = cfg["headers"]
|
||||
|
||||
receiver_key = str(delivery_value)
|
||||
|
||||
# Check if receiver already exists
|
||||
existing = (await conn.execute(
|
||||
text(
|
||||
"SELECT id FROM target_receiver "
|
||||
"WHERE target_id = :tid AND receiver_key = :rk"
|
||||
),
|
||||
{"tid": target_id, "rk": receiver_key},
|
||||
)).fetchone()
|
||||
|
||||
if not existing:
|
||||
# Derive a name for the receiver
|
||||
if target_type == "telegram":
|
||||
name = f"Chat {delivery_value}"
|
||||
elif target_type == "webhook":
|
||||
name = str(delivery_value)[:50]
|
||||
elif target_type == "email":
|
||||
name = str(delivery_value)
|
||||
else:
|
||||
name = str(delivery_value)[:50]
|
||||
|
||||
await conn.execute(
|
||||
text(
|
||||
"INSERT INTO target_receiver "
|
||||
"(target_id, name, config, receiver_key, enabled, created_at) "
|
||||
"VALUES (:tid, :name, :cfg, :rk, 1, CURRENT_TIMESTAMP)"
|
||||
),
|
||||
{
|
||||
"tid": target_id,
|
||||
"name": name,
|
||||
"cfg": json.dumps(receiver_config),
|
||||
"rk": receiver_key,
|
||||
},
|
||||
)
|
||||
created += 1
|
||||
|
||||
# Remove delivery fields from config
|
||||
new_cfg = dict(cfg)
|
||||
new_cfg.pop(delivery_field, None)
|
||||
# For webhook, also remove headers (moved to receiver)
|
||||
if target_type == "webhook":
|
||||
new_cfg.pop("headers", None)
|
||||
|
||||
if new_cfg != cfg:
|
||||
await conn.execute(
|
||||
text(
|
||||
"UPDATE notification_target SET config = :cfg WHERE id = :tid"
|
||||
),
|
||||
{"cfg": json.dumps(new_cfg), "tid": target_id},
|
||||
)
|
||||
cleaned += 1
|
||||
|
||||
if created:
|
||||
logger.info("Created %d receiver rows from target config delivery fields", created)
|
||||
if cleaned:
|
||||
logger.info("Cleaned delivery fields from %d target configs", cleaned)
|
||||
|
||||
|
||||
async def migrate_template_locale(engine: AsyncEngine) -> None:
|
||||
"""Add locale column to template_config and command_template_config.
|
||||
|
||||
Backfill locale from name: "(RU)" -> "ru", else "en" for system-owned rows.
|
||||
"""
|
||||
async with engine.begin() as conn:
|
||||
for table in ("template_config", "command_template_config"):
|
||||
if await _has_column(conn, table, "locale"):
|
||||
continue
|
||||
logger.info("Adding locale column to %s", table)
|
||||
await conn.execute(text(f"ALTER TABLE {table} ADD COLUMN locale TEXT DEFAULT ''"))
|
||||
# Backfill system-owned rows
|
||||
await conn.execute(text(
|
||||
f"UPDATE {table} SET locale = 'ru' WHERE user_id = 0 AND name LIKE '%(RU)%'"
|
||||
))
|
||||
await conn.execute(text(
|
||||
f"UPDATE {table} SET locale = 'en' WHERE user_id = 0 AND locale = ''"
|
||||
))
|
||||
|
||||
@@ -170,6 +170,7 @@ class TemplateConfig(SQLModel, table=True):
|
||||
name: str
|
||||
description: str = Field(default="")
|
||||
icon: str = Field(default="")
|
||||
locale: str = Field(default="") # e.g. "en", "ru"; empty = unspecified
|
||||
|
||||
date_format: str = Field(default="%d.%m.%Y, %H:%M UTC")
|
||||
date_only_format: str = Field(default="%d.%m.%Y")
|
||||
@@ -330,6 +331,7 @@ class CommandTemplateConfig(SQLModel, table=True):
|
||||
name: str
|
||||
description: str = Field(default="")
|
||||
icon: str = Field(default="")
|
||||
locale: str = Field(default="") # e.g. "en", "ru"; empty = unspecified
|
||||
created_at: datetime = Field(default_factory=_utcnow)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user