refactor: comprehensive consistency review — UI/UX, code quality, functional parity
Fix 19 issues across 3 priority tiers found during full-codebase review: CRITICAL: - Fix undefined --color-secondary CSS variable causing invisible UI elements - Fix Google Photos command templates using nonexistent asset.originalFileName - Fix scheduler template variable docs (tracker_name → schedule_name) - Add missing admin guard on notification template update endpoint HIGH: - Fix 5 hardcoded English strings missing i18n (MultiEntitySelect, actions, settings, TelegramBotTab, users) - Replace 17 raw <button> elements with shared <Button> component - Replace 5 raw error divs with shared <ErrorBanner> component - Refactor webhook handler duplication into shared _dispatch_webhook_event() - Add 30+ provider-specific fields to TrackingConfig TypeScript type - Add default TrackingConfig seeds for immich and google_photos - Add provider-specific command variable docs (Gitea, Planka, NUT, GP, Webhook) MEDIUM: - Replace hardcoded hex colors and Tailwind classes with CSS variable tokens - Remove dead code (unused imports, orphaned check_notification_tracker) - Fix Svelte 5 patterns ($state for _prevProviderId, remove unnecessary as any) - Fix inconsistent POST response shape (targets now returns full response) - Fix Google Photos template dead asset.year branches, clarify album_url docs
This commit is contained in:
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import hmac
|
||||
import json
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
@@ -71,6 +72,98 @@ def _passes_filters(
|
||||
return True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Shared dispatch helper
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def _dispatch_webhook_event(
|
||||
engine: Any,
|
||||
provider_id: int,
|
||||
provider_name: str,
|
||||
provider_config: dict[str, Any],
|
||||
event: ServiceEvent,
|
||||
detail_keys: tuple[str, ...],
|
||||
) -> int:
|
||||
"""Load trackers, filter, create EventLogs, dispatch notifications, and commit.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
engine:
|
||||
SQLAlchemy async engine.
|
||||
provider_id:
|
||||
ID of the ServiceProvider that received the webhook.
|
||||
provider_name:
|
||||
Human-readable name of the provider (for logging).
|
||||
provider_config:
|
||||
The provider's ``config`` dict (passed through to target config builder).
|
||||
event:
|
||||
Parsed :class:`ServiceEvent` to dispatch.
|
||||
detail_keys:
|
||||
Keys from ``event.extra`` to include in the EventLog ``details`` dict.
|
||||
|
||||
Returns
|
||||
-------
|
||||
int
|
||||
Number of successfully dispatched notifications.
|
||||
"""
|
||||
dispatched = 0
|
||||
async with AsyncSession(engine) as session:
|
||||
tracker_result = await session.exec(
|
||||
select(NotificationTracker).where(
|
||||
NotificationTracker.provider_id == provider_id,
|
||||
NotificationTracker.enabled == True, # noqa: E712
|
||||
)
|
||||
)
|
||||
trackers = tracker_result.all()
|
||||
|
||||
for tracker in trackers:
|
||||
filters = tracker.filters or {}
|
||||
if not _passes_filters(event, filters):
|
||||
_LOGGER.debug(
|
||||
"Event filtered out for tracker %d (%s)", tracker.id, tracker.name
|
||||
)
|
||||
continue
|
||||
|
||||
link_data = await load_link_data(session, tracker.id)
|
||||
if not link_data:
|
||||
continue
|
||||
|
||||
# Log event
|
||||
extra_details = {k: v for k, v in event.extra.items() if k in detail_keys}
|
||||
session.add(EventLog(
|
||||
tracker_id=tracker.id,
|
||||
tracker_name=tracker.name,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider_name,
|
||||
event_type=event.event_type.value,
|
||||
collection_id=event.collection_id,
|
||||
collection_name=event.collection_name,
|
||||
assets_count=0,
|
||||
details={
|
||||
"provider_type": event.provider_type.value,
|
||||
**extra_details,
|
||||
},
|
||||
))
|
||||
|
||||
# Dispatch to targets
|
||||
dispatcher = NotificationDispatcher()
|
||||
target_configs = _build_target_configs(event, link_data, provider_config)
|
||||
if target_configs:
|
||||
results = await dispatcher.dispatch(event, target_configs)
|
||||
for r in results:
|
||||
if r.get("success"):
|
||||
dispatched += 1
|
||||
else:
|
||||
_LOGGER.error(
|
||||
"Notification failed for tracker %d: %s",
|
||||
tracker.id, r.get("error", "unknown"),
|
||||
)
|
||||
|
||||
await session.commit()
|
||||
|
||||
return dispatched
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Gitea webhook endpoint
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -108,74 +201,27 @@ async def gitea_webhook(provider_id: int, request: Request):
|
||||
|
||||
try:
|
||||
payload = await request.json()
|
||||
except Exception:
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
raise HTTPException(status_code=400, detail="Invalid JSON")
|
||||
|
||||
event = parse_gitea_webhook(event_header, payload, provider.name)
|
||||
if event is None:
|
||||
return {"ok": True, "skipped": "unmapped event"}
|
||||
|
||||
# --- Find trackers for this provider and dispatch ---
|
||||
dispatched = 0
|
||||
async with AsyncSession(engine) as session:
|
||||
tracker_result = await session.exec(
|
||||
select(NotificationTracker).where(
|
||||
NotificationTracker.provider_id == provider_id,
|
||||
NotificationTracker.enabled == True,
|
||||
)
|
||||
)
|
||||
trackers = tracker_result.all()
|
||||
|
||||
for tracker in trackers:
|
||||
# Apply filters
|
||||
filters = tracker.filters or {}
|
||||
if not _passes_filters(event, filters):
|
||||
_LOGGER.debug(
|
||||
"Event filtered out for tracker %d (%s)", tracker.id, tracker.name
|
||||
)
|
||||
continue
|
||||
|
||||
# Load tracker-target links
|
||||
link_data = await load_link_data(session, tracker.id)
|
||||
if not link_data:
|
||||
continue
|
||||
|
||||
# Log event
|
||||
session.add(EventLog(
|
||||
tracker_id=tracker.id,
|
||||
tracker_name=tracker.name,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider.name,
|
||||
event_type=event.event_type.value,
|
||||
collection_id=event.collection_id,
|
||||
collection_name=event.collection_name,
|
||||
assets_count=0,
|
||||
details={
|
||||
"provider_type": event.provider_type.value,
|
||||
**{k: v for k, v in event.extra.items() if k in (
|
||||
"sender", "branch", "commit_count",
|
||||
"issue_number", "issue_title",
|
||||
"pr_number", "pr_title",
|
||||
"release_tag", "release_name",
|
||||
)},
|
||||
},
|
||||
))
|
||||
|
||||
# Dispatch to targets
|
||||
dispatcher = NotificationDispatcher()
|
||||
target_configs = _build_target_configs(event, link_data, provider.config or {})
|
||||
if target_configs:
|
||||
results = await dispatcher.dispatch(event, target_configs)
|
||||
for r in results:
|
||||
if r.get("success"):
|
||||
dispatched += 1
|
||||
else:
|
||||
_LOGGER.error(
|
||||
"Notification failed for tracker %d: %s",
|
||||
tracker.id, r.get("error", "unknown"),
|
||||
)
|
||||
|
||||
await session.commit()
|
||||
# --- Dispatch ---
|
||||
dispatched = await _dispatch_webhook_event(
|
||||
engine=engine,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider.name,
|
||||
provider_config=provider.config or {},
|
||||
event=event,
|
||||
detail_keys=(
|
||||
"sender", "branch", "commit_count",
|
||||
"issue_number", "issue_title",
|
||||
"pr_number", "pr_title",
|
||||
"release_tag", "release_name",
|
||||
),
|
||||
)
|
||||
|
||||
return {"ok": True, "dispatched": dispatched}
|
||||
|
||||
@@ -218,7 +264,7 @@ async def planka_webhook(provider_id: int, request: Request):
|
||||
# Parse payload
|
||||
try:
|
||||
payload = await request.json()
|
||||
except Exception:
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
raise HTTPException(status_code=400, detail="Invalid JSON")
|
||||
|
||||
event_type = payload.get("type", "")
|
||||
@@ -230,65 +276,20 @@ async def planka_webhook(provider_id: int, request: Request):
|
||||
if event is None:
|
||||
return {"ok": True, "skipped": "unmapped event"}
|
||||
|
||||
# --- Find trackers for this provider and dispatch ---
|
||||
dispatched = 0
|
||||
async with AsyncSession(engine) as session:
|
||||
tracker_result = await session.exec(
|
||||
select(NotificationTracker).where(
|
||||
NotificationTracker.provider_id == provider_id,
|
||||
NotificationTracker.enabled == True,
|
||||
)
|
||||
)
|
||||
trackers = tracker_result.all()
|
||||
|
||||
for tracker in trackers:
|
||||
filters = tracker.filters or {}
|
||||
if not _passes_filters(event, filters):
|
||||
_LOGGER.debug(
|
||||
"Event filtered out for tracker %d (%s)", tracker.id, tracker.name
|
||||
)
|
||||
continue
|
||||
|
||||
link_data = await load_link_data(session, tracker.id)
|
||||
if not link_data:
|
||||
continue
|
||||
|
||||
# Log event
|
||||
session.add(EventLog(
|
||||
tracker_id=tracker.id,
|
||||
tracker_name=tracker.name,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider.name,
|
||||
event_type=event.event_type.value,
|
||||
collection_id=event.collection_id,
|
||||
collection_name=event.collection_name,
|
||||
assets_count=0,
|
||||
details={
|
||||
"provider_type": event.provider_type.value,
|
||||
**{k: v for k, v in event.extra.items() if k in (
|
||||
"sender", "card_name", "board_name",
|
||||
"list_name", "old_list_name", "new_list_name",
|
||||
"comment_text", "task_name", "attachment_name",
|
||||
"label_name",
|
||||
)},
|
||||
},
|
||||
))
|
||||
|
||||
# Dispatch to targets
|
||||
dispatcher = NotificationDispatcher()
|
||||
target_configs = _build_target_configs(event, link_data, provider.config or {})
|
||||
if target_configs:
|
||||
results = await dispatcher.dispatch(event, target_configs)
|
||||
for r in results:
|
||||
if r.get("success"):
|
||||
dispatched += 1
|
||||
else:
|
||||
_LOGGER.error(
|
||||
"Notification failed for tracker %d: %s",
|
||||
tracker.id, r.get("error", "unknown"),
|
||||
)
|
||||
|
||||
await session.commit()
|
||||
# --- Dispatch ---
|
||||
dispatched = await _dispatch_webhook_event(
|
||||
engine=engine,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider.name,
|
||||
provider_config=provider.config or {},
|
||||
event=event,
|
||||
detail_keys=(
|
||||
"sender", "card_name", "board_name",
|
||||
"list_name", "old_list_name", "new_list_name",
|
||||
"comment_text", "task_name", "attachment_name",
|
||||
"label_name",
|
||||
),
|
||||
)
|
||||
|
||||
return {"ok": True, "dispatched": dispatched}
|
||||
|
||||
@@ -424,7 +425,7 @@ async def generic_webhook(provider_id: int, request: Request):
|
||||
# Parse JSON payload
|
||||
try:
|
||||
payload = await request.json()
|
||||
except Exception:
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
if store_payloads:
|
||||
async with AsyncSession(get_engine()) as log_session:
|
||||
await _save_webhook_log(
|
||||
@@ -451,70 +452,28 @@ async def generic_webhook(provider_id: int, request: Request):
|
||||
source_ip = request.client.host if request.client else ""
|
||||
event.extra["source_ip"] = source_ip
|
||||
|
||||
# --- Find trackers for this provider and dispatch ---
|
||||
dispatched = 0
|
||||
async with AsyncSession(engine) as session:
|
||||
tracker_result = await session.exec(
|
||||
select(NotificationTracker).where(
|
||||
NotificationTracker.provider_id == provider_id,
|
||||
NotificationTracker.enabled == True,
|
||||
)
|
||||
)
|
||||
trackers = tracker_result.all()
|
||||
# --- Dispatch ---
|
||||
dispatched = await _dispatch_webhook_event(
|
||||
engine=engine,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider_name,
|
||||
provider_config=provider_config,
|
||||
event=event,
|
||||
detail_keys=(
|
||||
"event_type_raw", "source_ip",
|
||||
),
|
||||
)
|
||||
|
||||
for tracker in trackers:
|
||||
filters = tracker.filters or {}
|
||||
if not _passes_filters(event, filters):
|
||||
_LOGGER.debug(
|
||||
"Event filtered out for tracker %d (%s)", tracker.id, tracker.name
|
||||
)
|
||||
continue
|
||||
|
||||
link_data = await load_link_data(session, tracker.id)
|
||||
if not link_data:
|
||||
continue
|
||||
|
||||
# Log event
|
||||
session.add(EventLog(
|
||||
tracker_id=tracker.id,
|
||||
tracker_name=tracker.name,
|
||||
provider_id=provider_id,
|
||||
provider_name=provider_name,
|
||||
event_type=event.event_type.value,
|
||||
collection_id=event.collection_id,
|
||||
collection_name=event.collection_name,
|
||||
assets_count=0,
|
||||
details={
|
||||
"provider_type": "webhook",
|
||||
"event_type_raw": event.extra.get("event_type_raw", ""),
|
||||
"source_ip": source_ip,
|
||||
},
|
||||
))
|
||||
|
||||
# Dispatch to targets
|
||||
dispatcher = NotificationDispatcher()
|
||||
target_configs = _build_target_configs(event, link_data, provider_config)
|
||||
if target_configs:
|
||||
results = await dispatcher.dispatch(event, target_configs)
|
||||
for r in results:
|
||||
if r.get("success"):
|
||||
dispatched += 1
|
||||
else:
|
||||
_LOGGER.error(
|
||||
"Notification failed for tracker %d: %s",
|
||||
tracker.id, r.get("error", "unknown"),
|
||||
)
|
||||
|
||||
# Log matched payload
|
||||
if store_payloads:
|
||||
# Log matched payload (separate session — dispatch already committed)
|
||||
if store_payloads:
|
||||
async with AsyncSession(engine) as log_session:
|
||||
await _save_webhook_log(
|
||||
session, provider_id, request.method, safe_headers,
|
||||
log_session, provider_id, request.method, safe_headers,
|
||||
payload, "matched" if dispatched > 0 else "unmatched",
|
||||
extracted_fields=dict(event.extra),
|
||||
max_count=max_stored,
|
||||
)
|
||||
|
||||
await session.commit()
|
||||
await log_session.commit()
|
||||
|
||||
return {"ok": True, "dispatched": dispatched}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user