mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 10:41:11 +01:00
Download history refactor pt3 (#706)
- Added canonical per-user visibility of requests and downloads via new activity view table. Users get fully independent activity and history views, while admins still see all. - Replaces janky frontend + backend combination
This commit is contained in:
+201
-184
@@ -2,11 +2,16 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import datetime, timezone
|
||||
from typing import Any, Callable, NamedTuple
|
||||
|
||||
from flask import Flask, jsonify, request, session
|
||||
|
||||
from shelfmark.core.activity_view_state_service import (
|
||||
ADMIN_VIEWER_SCOPE,
|
||||
NOAUTH_VIEWER_SCOPE,
|
||||
ActivityViewStateService,
|
||||
user_viewer_scope,
|
||||
)
|
||||
from shelfmark.core.download_history_service import ACTIVE_DOWNLOAD_STATUS, DownloadHistoryService, VALID_TERMINAL_STATUSES
|
||||
from shelfmark.core.logger import setup_logger
|
||||
from shelfmark.core.models import ACTIVE_QUEUE_STATUSES, QueueStatus, TERMINAL_QUEUE_STATUSES
|
||||
@@ -14,9 +19,7 @@ from shelfmark.core.request_validation import RequestStatus
|
||||
from shelfmark.core.request_helpers import (
|
||||
emit_ws_event,
|
||||
extract_release_source_id,
|
||||
normalize_optional_text,
|
||||
normalize_positive_int,
|
||||
now_utc_iso,
|
||||
populate_request_usernames,
|
||||
)
|
||||
from shelfmark.core.user_db import UserDB
|
||||
@@ -24,16 +27,6 @@ from shelfmark.core.user_db import UserDB
|
||||
logger = setup_logger(__name__)
|
||||
|
||||
|
||||
def _parse_timestamp(value: Any) -> float:
|
||||
if not isinstance(value, str) or not value.strip():
|
||||
return 0.0
|
||||
normalized = value.strip().replace("Z", "+00:00")
|
||||
try:
|
||||
return datetime.fromisoformat(normalized).timestamp()
|
||||
except ValueError:
|
||||
return 0.0
|
||||
|
||||
|
||||
def _require_authenticated(resolve_auth_mode: Callable[[], str]):
|
||||
auth_mode = resolve_auth_mode()
|
||||
if auth_mode == "none":
|
||||
@@ -116,6 +109,7 @@ class _ActorContext(NamedTuple):
|
||||
is_no_auth: bool
|
||||
is_admin: bool
|
||||
owner_scope: int | None
|
||||
viewer_scope: str
|
||||
|
||||
|
||||
def _resolve_activity_actor(
|
||||
@@ -128,27 +122,35 @@ def _resolve_activity_actor(
|
||||
Returns (actor, error_response). On success actor is non-None.
|
||||
"""
|
||||
if resolve_auth_mode() == "none":
|
||||
return _ActorContext(db_user_id=None, is_no_auth=True, is_admin=True, owner_scope=None), None
|
||||
return _ActorContext(
|
||||
db_user_id=None,
|
||||
is_no_auth=True,
|
||||
is_admin=True,
|
||||
owner_scope=None,
|
||||
viewer_scope=NOAUTH_VIEWER_SCOPE,
|
||||
), None
|
||||
|
||||
db_user_id, db_gate = _resolve_db_user_id(user_db=user_db)
|
||||
if db_user_id is None:
|
||||
return None, db_gate
|
||||
|
||||
is_admin = bool(session.get("is_admin"))
|
||||
viewer_scope = ADMIN_VIEWER_SCOPE if is_admin else user_viewer_scope(db_user_id)
|
||||
return _ActorContext(
|
||||
db_user_id=db_user_id,
|
||||
is_no_auth=False,
|
||||
is_admin=is_admin,
|
||||
owner_scope=None if is_admin else db_user_id,
|
||||
viewer_scope=viewer_scope,
|
||||
), None
|
||||
|
||||
|
||||
def _activity_ws_room(*, is_no_auth: bool, actor_db_user_id: int | None) -> str:
|
||||
def _activity_ws_room(actor: _ActorContext) -> str:
|
||||
"""Resolve the WebSocket room for activity events."""
|
||||
if is_no_auth:
|
||||
if actor.is_no_auth or actor.is_admin:
|
||||
return "admins"
|
||||
if actor_db_user_id is not None:
|
||||
return f"user_{actor_db_user_id}"
|
||||
if actor.db_user_id is not None:
|
||||
return f"user_{actor.db_user_id}"
|
||||
return "admins"
|
||||
|
||||
|
||||
@@ -162,6 +164,19 @@ def _check_item_ownership(actor: _ActorContext, row: dict[str, Any]) -> Any | No
|
||||
return None
|
||||
|
||||
|
||||
def _check_terminal_download(row: dict[str, Any]) -> Any | None:
|
||||
final_status = str(row.get("final_status") or "").strip().lower()
|
||||
if final_status not in VALID_TERMINAL_STATUSES:
|
||||
return jsonify({"error": "Only terminal downloads can be dismissed"}), 409
|
||||
return None
|
||||
|
||||
|
||||
def _check_terminal_request(row: dict[str, Any]) -> Any | None:
|
||||
if _request_terminal_status(row) is None:
|
||||
return jsonify({"error": "Only terminal requests can be dismissed"}), 409
|
||||
return None
|
||||
|
||||
|
||||
def _list_visible_requests(user_db: UserDB, *, is_admin: bool, db_user_id: int | None) -> list[dict[str, Any]]:
|
||||
if is_admin:
|
||||
request_rows = user_db.list_requests()
|
||||
@@ -233,29 +248,9 @@ def _build_download_status_from_db(
|
||||
download_payload["status_message"] = None
|
||||
status[final_status][task_id] = download_payload
|
||||
|
||||
# Include any queue items that don't have a DB row yet (race condition safety).
|
||||
# Only include active items — terminal items without a DB row are orphans
|
||||
# (e.g. admin cleared history) and should not be shown.
|
||||
for task_id, (bucket_key, queue_payload) in queue_index.items():
|
||||
if bucket_key in ACTIVE_QUEUE_STATUSES and task_id not in status.get(bucket_key, {}):
|
||||
status[bucket_key][task_id] = queue_payload
|
||||
|
||||
return status
|
||||
|
||||
|
||||
def _collect_active_download_task_ids(status: dict[str, dict[str, Any]]) -> set[str]:
|
||||
active_task_ids: set[str] = set()
|
||||
for bucket_key in ACTIVE_QUEUE_STATUSES:
|
||||
bucket = status.get(bucket_key)
|
||||
if not isinstance(bucket, dict):
|
||||
continue
|
||||
for task_id in bucket.keys():
|
||||
normalized_task_id = str(task_id).strip()
|
||||
if normalized_task_id:
|
||||
active_task_ids.add(normalized_task_id)
|
||||
return active_task_ids
|
||||
|
||||
|
||||
def _request_terminal_status(row: dict[str, Any]) -> str | None:
|
||||
request_status = row.get("status")
|
||||
if request_status == RequestStatus.PENDING:
|
||||
@@ -300,7 +295,11 @@ def _minimal_request_snapshot(request_row: dict[str, Any], request_id: int) -> d
|
||||
return {"kind": "request", "request": minimal_request}
|
||||
|
||||
|
||||
def _request_history_entry(request_row: dict[str, Any]) -> dict[str, Any] | None:
|
||||
def _request_history_entry(
|
||||
request_row: dict[str, Any],
|
||||
*,
|
||||
dismissed_at: str | None,
|
||||
) -> dict[str, Any] | None:
|
||||
request_id = normalize_positive_int(request_row.get("id"))
|
||||
if request_id is None:
|
||||
return None
|
||||
@@ -311,7 +310,7 @@ def _request_history_entry(request_row: dict[str, Any]) -> dict[str, Any] | None
|
||||
"user_id": request_row.get("user_id"),
|
||||
"item_type": "request",
|
||||
"item_key": item_key,
|
||||
"dismissed_at": request_row.get("dismissed_at"),
|
||||
"dismissed_at": dismissed_at,
|
||||
"snapshot": _minimal_request_snapshot(request_row, request_id),
|
||||
"origin": "request",
|
||||
"final_status": final_status,
|
||||
@@ -321,29 +320,13 @@ def _request_history_entry(request_row: dict[str, Any]) -> dict[str, Any] | None
|
||||
}
|
||||
|
||||
|
||||
def _dedupe_dismissed_entries(entries: list[dict[str, str]]) -> list[dict[str, str]]:
|
||||
seen: set[tuple[str, str]] = set()
|
||||
result: list[dict[str, str]] = []
|
||||
for entry in entries:
|
||||
item_type = str(entry.get("item_type") or "").strip().lower()
|
||||
item_key = str(entry.get("item_key") or "").strip()
|
||||
if item_type not in {"download", "request"} or not item_key:
|
||||
continue
|
||||
marker = (item_type, item_key)
|
||||
if marker in seen:
|
||||
continue
|
||||
seen.add(marker)
|
||||
result.append({"item_type": item_type, "item_key": item_key})
|
||||
return result
|
||||
|
||||
|
||||
def register_activity_routes(
|
||||
app: Flask,
|
||||
user_db: UserDB,
|
||||
*,
|
||||
activity_view_state_service: ActivityViewStateService,
|
||||
download_history_service: DownloadHistoryService,
|
||||
resolve_auth_mode: Callable[[], str],
|
||||
resolve_status_scope: Callable[[], tuple[bool, int | None, bool]],
|
||||
queue_status: Callable[..., dict[str, dict[str, Any]]],
|
||||
sync_request_delivery_states: Callable[..., list[dict[str, Any]]],
|
||||
emit_request_updates: Callable[[list[dict[str, Any]]], None],
|
||||
@@ -357,93 +340,65 @@ def register_activity_routes(
|
||||
if auth_gate is not None:
|
||||
return auth_gate
|
||||
|
||||
is_admin, db_user_id, can_access_status = resolve_status_scope()
|
||||
if not can_access_status:
|
||||
return (
|
||||
jsonify(
|
||||
{
|
||||
"error": "User identity unavailable for activity workflow",
|
||||
"code": "user_identity_unavailable",
|
||||
}
|
||||
),
|
||||
403,
|
||||
)
|
||||
actor, actor_error = _resolve_activity_actor(
|
||||
user_db=user_db,
|
||||
resolve_auth_mode=resolve_auth_mode,
|
||||
)
|
||||
if actor_error is not None:
|
||||
return actor_error
|
||||
|
||||
owner_user_scope = None if is_admin else db_user_id
|
||||
|
||||
live_queue = queue_status(user_id=owner_user_scope)
|
||||
|
||||
try:
|
||||
db_rows = download_history_service.get_undismissed(
|
||||
user_id=owner_user_scope,
|
||||
limit=200,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to load undismissed download rows: %s", exc)
|
||||
db_rows = []
|
||||
hidden_rows = activity_view_state_service.list_hidden(viewer_scope=actor.viewer_scope)
|
||||
hidden_item_keys = {str(row.get("item_key") or "").strip() for row in hidden_rows}
|
||||
dismissed_entries = [
|
||||
{
|
||||
"item_type": str(row.get("item_type") or "").strip().lower(),
|
||||
"item_key": str(row.get("item_key") or "").strip(),
|
||||
}
|
||||
for row in hidden_rows
|
||||
if str(row.get("item_type") or "").strip().lower() in {"download", "request"}
|
||||
and str(row.get("item_key") or "").strip()
|
||||
]
|
||||
live_queue = queue_status(user_id=actor.owner_scope)
|
||||
db_rows = download_history_service.list_recent(
|
||||
user_id=actor.owner_scope,
|
||||
limit=200,
|
||||
)
|
||||
visible_db_rows = [
|
||||
row
|
||||
for row in db_rows
|
||||
if f"download:{str(row.get('task_id') or '').strip()}" not in hidden_item_keys
|
||||
]
|
||||
|
||||
status = _build_download_status_from_db(
|
||||
db_rows=db_rows,
|
||||
db_rows=visible_db_rows,
|
||||
queue_status=live_queue,
|
||||
)
|
||||
|
||||
updated_requests = sync_request_delivery_states(
|
||||
user_db,
|
||||
queue_status=status,
|
||||
user_id=owner_user_scope,
|
||||
user_id=actor.owner_scope,
|
||||
)
|
||||
emit_request_updates(updated_requests)
|
||||
request_rows = _list_visible_requests(user_db, is_admin=is_admin, db_user_id=db_user_id)
|
||||
|
||||
dismissed: list[dict[str, str]] = []
|
||||
dismissed_task_ids: list[str] = []
|
||||
try:
|
||||
dismissed_task_ids = download_history_service.get_dismissed_keys(
|
||||
user_id=owner_user_scope,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to load dismissed download keys: %s", exc)
|
||||
|
||||
# Only clear stale dismissals when active downloads overlap dismissed keys.
|
||||
active_task_ids = _collect_active_download_task_ids(status)
|
||||
stale_dismissed = active_task_ids & set(dismissed_task_ids) if active_task_ids else set()
|
||||
if stale_dismissed:
|
||||
try:
|
||||
download_history_service.clear_dismissals_for_active(
|
||||
task_ids=stale_dismissed,
|
||||
user_id=owner_user_scope,
|
||||
)
|
||||
dismissed_task_ids = [tid for tid in dismissed_task_ids if tid not in stale_dismissed]
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to clear stale download dismissals for active tasks: %s", exc)
|
||||
|
||||
dismissed.extend(
|
||||
{"item_type": "download", "item_key": f"download:{task_id}"}
|
||||
for task_id in dismissed_task_ids
|
||||
request_rows = _list_visible_requests(
|
||||
user_db,
|
||||
is_admin=actor.is_admin,
|
||||
db_user_id=actor.db_user_id,
|
||||
)
|
||||
|
||||
# Keep request dismissal state on the request rows directly.
|
||||
try:
|
||||
dismissed_request_rows = user_db.list_dismissed_requests(user_id=owner_user_scope)
|
||||
for request_row in dismissed_request_rows:
|
||||
request_id = normalize_positive_int(request_row.get("id"))
|
||||
if request_id is None:
|
||||
continue
|
||||
dismissed.append({"item_type": "request", "item_key": f"request:{request_id}"})
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to load dismissed request keys: %s", exc)
|
||||
|
||||
if not is_admin and db_user_id is None:
|
||||
# In auth mode, if we can't identify a non-admin viewer, don't show dismissals.
|
||||
dismissed = []
|
||||
else:
|
||||
dismissed = _dedupe_dismissed_entries(dismissed)
|
||||
visible_request_rows: list[dict[str, Any]] = []
|
||||
for row in request_rows:
|
||||
request_id = normalize_positive_int(row.get("id"))
|
||||
if request_id is None:
|
||||
continue
|
||||
if f"request:{request_id}" in hidden_item_keys:
|
||||
continue
|
||||
visible_request_rows.append(row)
|
||||
|
||||
return jsonify(
|
||||
{
|
||||
"status": status,
|
||||
"requests": request_rows,
|
||||
"dismissed": dismissed,
|
||||
"requests": visible_request_rows,
|
||||
"dismissed": dismissed_entries,
|
||||
}
|
||||
)
|
||||
|
||||
@@ -476,18 +431,21 @@ def register_activity_routes(
|
||||
|
||||
existing = download_history_service.get_by_task_id(task_id)
|
||||
if existing is None:
|
||||
# Row already gone (e.g. admin cleared history) — treat as success
|
||||
dismissal_item = {"item_type": "download", "item_key": f"download:{task_id}"}
|
||||
else:
|
||||
ownership_gate = _check_item_ownership(actor, existing)
|
||||
if ownership_gate is not None:
|
||||
return ownership_gate
|
||||
return jsonify({"error": "Download not found"}), 404
|
||||
|
||||
download_history_service.dismiss(
|
||||
task_id=task_id,
|
||||
user_id=actor.owner_scope,
|
||||
)
|
||||
dismissal_item = {"item_type": "download", "item_key": f"download:{task_id}"}
|
||||
ownership_gate = _check_item_ownership(actor, existing)
|
||||
if ownership_gate is not None:
|
||||
return ownership_gate
|
||||
terminal_gate = _check_terminal_download(existing)
|
||||
if terminal_gate is not None:
|
||||
return terminal_gate
|
||||
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope=actor.viewer_scope,
|
||||
item_type="download",
|
||||
item_key=f"download:{task_id}",
|
||||
)
|
||||
dismissal_item = {"item_type": "download", "item_key": f"download:{task_id}"}
|
||||
|
||||
elif item_type == "request":
|
||||
request_id = normalize_positive_int(_parse_item_key(item_key, "request"))
|
||||
@@ -501,13 +459,20 @@ def register_activity_routes(
|
||||
ownership_gate = _check_item_ownership(actor, request_row)
|
||||
if ownership_gate is not None:
|
||||
return ownership_gate
|
||||
terminal_gate = _check_terminal_request(request_row)
|
||||
if terminal_gate is not None:
|
||||
return terminal_gate
|
||||
|
||||
user_db.update_request(request_id, dismissed_at=now_utc_iso())
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope=actor.viewer_scope,
|
||||
item_type="request",
|
||||
item_key=f"request:{request_id}",
|
||||
)
|
||||
dismissal_item = {"item_type": "request", "item_key": f"request:{request_id}"}
|
||||
else:
|
||||
return jsonify({"error": "item_type must be one of: download, request"}), 400
|
||||
|
||||
room = _activity_ws_room(is_no_auth=actor.is_no_auth, actor_db_user_id=actor.db_user_id)
|
||||
room = _activity_ws_room(actor)
|
||||
emit_ws_event(
|
||||
ws_manager,
|
||||
event_name="activity_update",
|
||||
@@ -541,8 +506,8 @@ def register_activity_routes(
|
||||
if not isinstance(items, list):
|
||||
return jsonify({"error": "items must be an array"}), 400
|
||||
|
||||
download_task_ids: list[str] = []
|
||||
request_ids: list[int] = []
|
||||
dismissal_items: list[dict[str, str]] = []
|
||||
missing_item_keys: list[str] = []
|
||||
|
||||
for item in items:
|
||||
if not isinstance(item, dict):
|
||||
@@ -557,11 +522,15 @@ def register_activity_routes(
|
||||
return jsonify({"error": "download item_key must be in the format download:<task_id>"}), 400
|
||||
existing = download_history_service.get_by_task_id(task_id)
|
||||
if existing is None:
|
||||
missing_item_keys.append(f"download:{task_id}")
|
||||
continue
|
||||
ownership_gate = _check_item_ownership(actor, existing)
|
||||
if ownership_gate is not None:
|
||||
return ownership_gate
|
||||
download_task_ids.append(task_id)
|
||||
terminal_gate = _check_terminal_download(existing)
|
||||
if terminal_gate is not None:
|
||||
return terminal_gate
|
||||
dismissal_items.append({"item_type": "download", "item_key": f"download:{task_id}"})
|
||||
continue
|
||||
|
||||
if item_type == "request":
|
||||
@@ -570,28 +539,36 @@ def register_activity_routes(
|
||||
return jsonify({"error": "request item_key must be in the format request:<id>"}), 400
|
||||
request_row = user_db.get_request(request_id)
|
||||
if request_row is None:
|
||||
missing_item_keys.append(f"request:{request_id}")
|
||||
continue
|
||||
ownership_gate = _check_item_ownership(actor, request_row)
|
||||
if ownership_gate is not None:
|
||||
return ownership_gate
|
||||
request_ids.append(request_id)
|
||||
terminal_gate = _check_terminal_request(request_row)
|
||||
if terminal_gate is not None:
|
||||
return terminal_gate
|
||||
dismissal_items.append({"item_type": "request", "item_key": f"request:{request_id}"})
|
||||
continue
|
||||
|
||||
return jsonify({"error": "item_type must be one of: download, request"}), 400
|
||||
|
||||
dismissed_download_count = download_history_service.dismiss_many(
|
||||
task_ids=download_task_ids,
|
||||
user_id=actor.owner_scope,
|
||||
if missing_item_keys:
|
||||
return (
|
||||
jsonify(
|
||||
{
|
||||
"error": "One or more activity items were not found",
|
||||
"missing_item_keys": missing_item_keys,
|
||||
}
|
||||
),
|
||||
404,
|
||||
)
|
||||
|
||||
dismissed_count = activity_view_state_service.dismiss_many(
|
||||
viewer_scope=actor.viewer_scope,
|
||||
items=dismissal_items,
|
||||
)
|
||||
|
||||
dismissed_request_count = user_db.dismiss_requests_batch(
|
||||
request_ids=request_ids,
|
||||
dismissed_at=now_utc_iso(),
|
||||
)
|
||||
|
||||
dismissed_count = dismissed_download_count + dismissed_request_count
|
||||
|
||||
room = _activity_ws_room(is_no_auth=actor.is_no_auth, actor_db_user_id=actor.db_user_id)
|
||||
room = _activity_ws_room(actor)
|
||||
emit_ws_event(
|
||||
ws_manager,
|
||||
event_name="activity_update",
|
||||
@@ -628,30 +605,70 @@ def register_activity_routes(
|
||||
if offset < 0:
|
||||
return jsonify({"error": "offset must be a non-negative integer"}), 400
|
||||
|
||||
# Fetch enough from each source to fill the requested page after merging.
|
||||
merge_limit = offset + limit
|
||||
download_history_rows = download_history_service.get_history(
|
||||
user_id=actor.owner_scope,
|
||||
limit=merge_limit,
|
||||
offset=0,
|
||||
history_rows = activity_view_state_service.list_history(
|
||||
viewer_scope=actor.viewer_scope,
|
||||
limit=limit,
|
||||
offset=offset,
|
||||
)
|
||||
dismissed_request_rows = user_db.list_dismissed_requests(user_id=actor.owner_scope, limit=merge_limit)
|
||||
request_history_rows = [
|
||||
entry
|
||||
for entry in (_request_history_entry(row) for row in dismissed_request_rows)
|
||||
if entry is not None
|
||||
]
|
||||
payload: list[dict[str, Any]] = []
|
||||
|
||||
combined = [*download_history_rows, *request_history_rows]
|
||||
combined.sort(
|
||||
key=lambda row: (
|
||||
_parse_timestamp(row.get("dismissed_at")),
|
||||
str(row.get("id") or ""),
|
||||
),
|
||||
reverse=True,
|
||||
)
|
||||
paged = combined[offset:offset + limit]
|
||||
return jsonify(paged)
|
||||
for history_row in history_rows:
|
||||
item_type = str(history_row.get("item_type") or "").strip().lower()
|
||||
item_key = str(history_row.get("item_key") or "").strip()
|
||||
dismissed_at = history_row.get("dismissed_at")
|
||||
|
||||
if not isinstance(dismissed_at, str) or not dismissed_at.strip():
|
||||
raise RuntimeError(f"Activity history state missing dismissed_at for {item_key}")
|
||||
|
||||
if item_type == "download":
|
||||
task_id = _parse_item_key(item_key, "download")
|
||||
if task_id is None:
|
||||
raise RuntimeError(f"Invalid activity history item_key: {item_key}")
|
||||
|
||||
download_row = download_history_service.get_by_task_id(task_id)
|
||||
if download_row is None:
|
||||
raise RuntimeError(f"Download history row not found for {item_key}")
|
||||
|
||||
if not actor.is_admin:
|
||||
owner_user_id = normalize_positive_int(download_row.get("user_id"))
|
||||
if owner_user_id != actor.db_user_id:
|
||||
raise RuntimeError(f"Viewer state out of scope for {item_key}")
|
||||
|
||||
payload.append(
|
||||
DownloadHistoryService.to_history_row(
|
||||
download_row,
|
||||
dismissed_at=dismissed_at,
|
||||
)
|
||||
)
|
||||
continue
|
||||
|
||||
if item_type == "request":
|
||||
request_id = normalize_positive_int(_parse_item_key(item_key, "request"))
|
||||
if request_id is None:
|
||||
raise RuntimeError(f"Invalid activity history item_key: {item_key}")
|
||||
|
||||
request_row = user_db.get_request(request_id)
|
||||
if request_row is None:
|
||||
raise RuntimeError(f"Request row not found for {item_key}")
|
||||
|
||||
if not actor.is_admin:
|
||||
owner_user_id = normalize_positive_int(request_row.get("user_id"))
|
||||
if owner_user_id != actor.db_user_id:
|
||||
raise RuntimeError(f"Viewer state out of scope for {item_key}")
|
||||
|
||||
populate_request_usernames([request_row], user_db)
|
||||
entry = _request_history_entry(
|
||||
request_row,
|
||||
dismissed_at=dismissed_at,
|
||||
)
|
||||
if entry is None:
|
||||
raise RuntimeError(f"Failed to build request history entry for {item_key}")
|
||||
payload.append(entry)
|
||||
continue
|
||||
|
||||
raise RuntimeError(f"Unknown activity history item_type: {item_type}")
|
||||
|
||||
return jsonify(payload)
|
||||
|
||||
@app.route("/api/activity/history", methods=["DELETE"])
|
||||
def api_activity_history_clear():
|
||||
@@ -666,18 +683,18 @@ def register_activity_routes(
|
||||
if actor_error is not None:
|
||||
return actor_error
|
||||
|
||||
deleted_downloads = download_history_service.clear_dismissed(user_id=actor.owner_scope)
|
||||
deleted_requests = user_db.delete_dismissed_requests(user_id=actor.owner_scope)
|
||||
deleted_count = deleted_downloads + deleted_requests
|
||||
cleared_count = activity_view_state_service.clear_history(
|
||||
viewer_scope=actor.viewer_scope,
|
||||
)
|
||||
|
||||
room = _activity_ws_room(is_no_auth=actor.is_no_auth, actor_db_user_id=actor.db_user_id)
|
||||
room = _activity_ws_room(actor)
|
||||
emit_ws_event(
|
||||
ws_manager,
|
||||
event_name="activity_update",
|
||||
room=room,
|
||||
payload={
|
||||
"kind": "history_cleared",
|
||||
"count": deleted_count,
|
||||
"count": cleared_count,
|
||||
},
|
||||
)
|
||||
return jsonify({"status": "cleared", "deleted_count": deleted_count})
|
||||
return jsonify({"status": "cleared", "cleared_count": cleared_count})
|
||||
|
||||
@@ -0,0 +1,310 @@
|
||||
"""Persistence helpers for per-viewer activity visibility state."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
import threading
|
||||
from typing import Any
|
||||
|
||||
from shelfmark.core.request_helpers import now_utc_iso
|
||||
|
||||
|
||||
VALID_ACTIVITY_ITEM_TYPES = frozenset({"download", "request"})
|
||||
ADMIN_VIEWER_SCOPE = "admin:shared"
|
||||
NOAUTH_VIEWER_SCOPE = "noauth:shared"
|
||||
USER_VIEWER_SCOPE_PREFIX = "user:"
|
||||
|
||||
|
||||
def user_viewer_scope(user_id: int) -> str:
|
||||
if not isinstance(user_id, int) or user_id < 1:
|
||||
raise ValueError("user_id must be a positive integer")
|
||||
return f"{USER_VIEWER_SCOPE_PREFIX}{user_id}"
|
||||
|
||||
|
||||
def normalize_viewer_scope(viewer_scope: Any) -> str:
|
||||
if not isinstance(viewer_scope, str) or not viewer_scope.strip():
|
||||
raise ValueError("viewer_scope must be a non-empty string")
|
||||
|
||||
normalized = viewer_scope.strip()
|
||||
if normalized in {ADMIN_VIEWER_SCOPE, NOAUTH_VIEWER_SCOPE}:
|
||||
return normalized
|
||||
|
||||
if not normalized.startswith(USER_VIEWER_SCOPE_PREFIX):
|
||||
raise ValueError(
|
||||
"viewer_scope must be one of: admin:shared, noauth:shared, or user:<id>"
|
||||
)
|
||||
|
||||
raw_user_id = normalized[len(USER_VIEWER_SCOPE_PREFIX):].strip()
|
||||
try:
|
||||
parsed_user_id = int(raw_user_id)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise ValueError("viewer_scope user id must be a positive integer") from exc
|
||||
|
||||
return user_viewer_scope(parsed_user_id)
|
||||
|
||||
|
||||
def _normalize_item_type(item_type: Any) -> str:
|
||||
if not isinstance(item_type, str) or not item_type.strip():
|
||||
raise ValueError("item_type must be a non-empty string")
|
||||
normalized = item_type.strip().lower()
|
||||
if normalized not in VALID_ACTIVITY_ITEM_TYPES:
|
||||
raise ValueError("item_type must be one of: download, request")
|
||||
return normalized
|
||||
|
||||
|
||||
def _normalize_item_key(item_key: Any, *, item_type: str) -> str:
|
||||
if not isinstance(item_key, str) or not item_key.strip():
|
||||
raise ValueError("item_key must be a non-empty string")
|
||||
|
||||
normalized = item_key.strip()
|
||||
expected_prefix = f"{item_type}:"
|
||||
if not normalized.startswith(expected_prefix):
|
||||
raise ValueError(f"item_key must be in the format {expected_prefix}<id>")
|
||||
if not normalized.split(":", 1)[1].strip():
|
||||
raise ValueError(f"item_key must be in the format {expected_prefix}<id>")
|
||||
return normalized
|
||||
|
||||
|
||||
class ActivityViewStateService:
|
||||
"""Service for per-viewer activity dismissal and history visibility."""
|
||||
|
||||
def __init__(self, db_path: str):
|
||||
self._db_path = db_path
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def _connect(self) -> sqlite3.Connection:
|
||||
conn = sqlite3.connect(self._db_path)
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA foreign_keys = ON")
|
||||
return conn
|
||||
|
||||
def list_hidden(
|
||||
self,
|
||||
*,
|
||||
viewer_scope: str,
|
||||
limit: int | None = None,
|
||||
) -> list[dict[str, Any]]:
|
||||
normalized_scope = normalize_viewer_scope(viewer_scope)
|
||||
normalized_limit = None if limit is None else max(1, int(limit))
|
||||
query = """
|
||||
SELECT item_type, item_key, dismissed_at, cleared_at
|
||||
FROM activity_view_state
|
||||
WHERE viewer_scope = ?
|
||||
AND dismissed_at IS NOT NULL
|
||||
ORDER BY COALESCE(cleared_at, dismissed_at) DESC, id DESC
|
||||
"""
|
||||
params: list[Any] = [normalized_scope]
|
||||
if normalized_limit is not None:
|
||||
query += "\nLIMIT ?"
|
||||
params.append(normalized_limit)
|
||||
|
||||
conn = self._connect()
|
||||
try:
|
||||
rows = conn.execute(query, params).fetchall()
|
||||
return [dict(row) for row in rows]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def list_history(
|
||||
self,
|
||||
*,
|
||||
viewer_scope: str,
|
||||
limit: int = 50,
|
||||
offset: int = 0,
|
||||
) -> list[dict[str, Any]]:
|
||||
normalized_scope = normalize_viewer_scope(viewer_scope)
|
||||
normalized_limit = max(1, min(int(limit), 5000))
|
||||
normalized_offset = max(0, int(offset))
|
||||
|
||||
conn = self._connect()
|
||||
try:
|
||||
rows = conn.execute(
|
||||
"""
|
||||
SELECT item_type, item_key, dismissed_at
|
||||
FROM activity_view_state
|
||||
WHERE viewer_scope = ?
|
||||
AND dismissed_at IS NOT NULL
|
||||
AND cleared_at IS NULL
|
||||
ORDER BY dismissed_at DESC, id DESC
|
||||
LIMIT ? OFFSET ?
|
||||
""",
|
||||
(normalized_scope, normalized_limit, normalized_offset),
|
||||
).fetchall()
|
||||
return [dict(row) for row in rows]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def dismiss(
|
||||
self,
|
||||
*,
|
||||
viewer_scope: str,
|
||||
item_type: str,
|
||||
item_key: str,
|
||||
) -> int:
|
||||
normalized_scope = normalize_viewer_scope(viewer_scope)
|
||||
normalized_type = _normalize_item_type(item_type)
|
||||
normalized_key = _normalize_item_key(item_key, item_type=normalized_type)
|
||||
dismissed_at = now_utc_iso()
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(
|
||||
"""
|
||||
INSERT INTO activity_view_state (
|
||||
viewer_scope,
|
||||
item_type,
|
||||
item_key,
|
||||
dismissed_at,
|
||||
cleared_at
|
||||
)
|
||||
VALUES (?, ?, ?, ?, NULL)
|
||||
ON CONFLICT(viewer_scope, item_type, item_key) DO UPDATE SET
|
||||
dismissed_at = excluded.dismissed_at,
|
||||
cleared_at = NULL
|
||||
""",
|
||||
(normalized_scope, normalized_type, normalized_key, dismissed_at),
|
||||
)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def dismiss_many(
|
||||
self,
|
||||
*,
|
||||
viewer_scope: str,
|
||||
items: list[dict[str, str]],
|
||||
) -> int:
|
||||
normalized_scope = normalize_viewer_scope(viewer_scope)
|
||||
if not items:
|
||||
return 0
|
||||
|
||||
seen: set[tuple[str, str]] = set()
|
||||
normalized_items: list[tuple[str, str]] = []
|
||||
for item in items:
|
||||
normalized_type = _normalize_item_type(item.get("item_type"))
|
||||
normalized_key = _normalize_item_key(item.get("item_key"), item_type=normalized_type)
|
||||
marker = (normalized_type, normalized_key)
|
||||
if marker in seen:
|
||||
continue
|
||||
seen.add(marker)
|
||||
normalized_items.append(marker)
|
||||
|
||||
if not normalized_items:
|
||||
return 0
|
||||
|
||||
dismissed_at = now_utc_iso()
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
total = 0
|
||||
for normalized_type, normalized_key in normalized_items:
|
||||
cursor = conn.execute(
|
||||
"""
|
||||
INSERT INTO activity_view_state (
|
||||
viewer_scope,
|
||||
item_type,
|
||||
item_key,
|
||||
dismissed_at,
|
||||
cleared_at
|
||||
)
|
||||
VALUES (?, ?, ?, ?, NULL)
|
||||
ON CONFLICT(viewer_scope, item_type, item_key) DO UPDATE SET
|
||||
dismissed_at = excluded.dismissed_at,
|
||||
cleared_at = NULL
|
||||
""",
|
||||
(normalized_scope, normalized_type, normalized_key, dismissed_at),
|
||||
)
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
total += max(rowcount, 0)
|
||||
conn.commit()
|
||||
return total
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def clear_history(self, *, viewer_scope: str) -> int:
|
||||
normalized_scope = normalize_viewer_scope(viewer_scope)
|
||||
cleared_at = now_utc_iso()
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(
|
||||
"""
|
||||
UPDATE activity_view_state
|
||||
SET cleared_at = ?
|
||||
WHERE viewer_scope = ?
|
||||
AND dismissed_at IS NOT NULL
|
||||
AND cleared_at IS NULL
|
||||
""",
|
||||
(cleared_at, normalized_scope),
|
||||
)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def clear_item_for_all_viewers(self, *, item_type: str, item_key: str) -> int:
|
||||
normalized_type = _normalize_item_type(item_type)
|
||||
normalized_key = _normalize_item_key(item_key, item_type=normalized_type)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(
|
||||
"""
|
||||
DELETE FROM activity_view_state
|
||||
WHERE item_type = ? AND item_key = ?
|
||||
""",
|
||||
(normalized_type, normalized_key),
|
||||
)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def delete_viewer_scope(self, *, viewer_scope: str) -> int:
|
||||
normalized_scope = normalize_viewer_scope(viewer_scope)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(
|
||||
"DELETE FROM activity_view_state WHERE viewer_scope = ?",
|
||||
(normalized_scope,),
|
||||
)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def delete_items(self, *, item_type: str, item_keys: list[str]) -> int:
|
||||
normalized_type = _normalize_item_type(item_type)
|
||||
normalized_keys = [
|
||||
_normalize_item_key(item_key, item_type=normalized_type)
|
||||
for item_key in item_keys
|
||||
]
|
||||
if not normalized_keys:
|
||||
return 0
|
||||
|
||||
placeholders = ",".join("?" for _ in normalized_keys)
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(
|
||||
f"""
|
||||
DELETE FROM activity_view_state
|
||||
WHERE item_type = ? AND item_key IN ({placeholders})
|
||||
""",
|
||||
(normalized_type, *normalized_keys),
|
||||
)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Persistence helpers for flat download terminal history."""
|
||||
"""Persistence helpers for canonical download activity rows."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -61,20 +61,8 @@ def _normalize_limit(value: Any, *, default: int, minimum: int, maximum: int) ->
|
||||
return parsed
|
||||
|
||||
|
||||
def _normalize_offset(value: Any, *, default: int) -> int:
|
||||
if value is None:
|
||||
return default
|
||||
try:
|
||||
parsed = int(value)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise ValueError("offset must be an integer") from exc
|
||||
if parsed < 0:
|
||||
return 0
|
||||
return parsed
|
||||
|
||||
|
||||
class DownloadHistoryService:
|
||||
"""Service for persisted terminal download history and dismissals."""
|
||||
"""Service for persisted canonical download activity rows."""
|
||||
|
||||
def __init__(self, db_path: str):
|
||||
self._db_path = db_path
|
||||
@@ -132,7 +120,7 @@ class DownloadHistoryService:
|
||||
return None
|
||||
|
||||
@classmethod
|
||||
def _to_history_row(cls, row: dict[str, Any]) -> dict[str, Any]:
|
||||
def to_history_row(cls, row: dict[str, Any], *, dismissed_at: str) -> dict[str, Any]:
|
||||
task_id = str(row.get("task_id") or "").strip()
|
||||
item_key = cls._to_item_key(task_id)
|
||||
download_payload = cls.to_download_payload(row)
|
||||
@@ -144,7 +132,7 @@ class DownloadHistoryService:
|
||||
"user_id": row.get("user_id"),
|
||||
"item_type": "download",
|
||||
"item_key": item_key,
|
||||
"dismissed_at": row.get("dismissed_at"),
|
||||
"dismissed_at": dismissed_at,
|
||||
"snapshot": {
|
||||
"kind": "download",
|
||||
"download": download_payload,
|
||||
@@ -287,7 +275,7 @@ class DownloadHistoryService:
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def get_undismissed(
|
||||
def list_recent(
|
||||
self,
|
||||
*,
|
||||
user_id: int | None,
|
||||
@@ -295,10 +283,10 @@ class DownloadHistoryService:
|
||||
) -> list[dict[str, Any]]:
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
normalized_limit = _normalize_limit(limit, default=200, minimum=1, maximum=1000)
|
||||
query = "SELECT * FROM download_history WHERE dismissed_at IS NULL"
|
||||
query = "SELECT * FROM download_history"
|
||||
params: list[Any] = []
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
query += " WHERE user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
query += " ORDER BY terminal_at DESC, id DESC LIMIT ?"
|
||||
params.append(normalized_limit)
|
||||
@@ -309,133 +297,3 @@ class DownloadHistoryService:
|
||||
return [dict(row) for row in rows]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def get_dismissed_keys(self, *, user_id: int | None, limit: int = 5000) -> list[str]:
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
normalized_limit = _normalize_limit(limit, default=5000, minimum=1, maximum=10000)
|
||||
query = "SELECT task_id FROM download_history WHERE dismissed_at IS NOT NULL"
|
||||
params: list[Any] = []
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
query += " ORDER BY dismissed_at DESC, id DESC LIMIT ?"
|
||||
params.append(normalized_limit)
|
||||
|
||||
conn = self._connect()
|
||||
try:
|
||||
rows = conn.execute(query, params).fetchall()
|
||||
keys: list[str] = []
|
||||
for row in rows:
|
||||
task_id = normalize_optional_text(row["task_id"])
|
||||
if task_id is not None:
|
||||
keys.append(task_id)
|
||||
return keys
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def dismiss(self, *, task_id: str, user_id: int | None) -> int:
|
||||
normalized_task_id = _normalize_task_id(task_id)
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
query = "UPDATE download_history SET dismissed_at = ? WHERE task_id = ?"
|
||||
params: list[Any] = [now_utc_iso(), normalized_task_id]
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(query, params)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def dismiss_many(self, *, task_ids: list[str], user_id: int | None) -> int:
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
normalized_task_ids = [_normalize_task_id(task_id) for task_id in task_ids]
|
||||
if not normalized_task_ids:
|
||||
return 0
|
||||
|
||||
placeholders = ",".join("?" for _ in normalized_task_ids)
|
||||
query = f"UPDATE download_history SET dismissed_at = ? WHERE task_id IN ({placeholders})"
|
||||
params: list[Any] = [now_utc_iso(), *normalized_task_ids]
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(query, params)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def get_history(self, *, user_id: int | None, limit: int, offset: int) -> list[dict[str, Any]]:
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
normalized_limit = _normalize_limit(limit, default=50, minimum=1, maximum=5000)
|
||||
normalized_offset = _normalize_offset(offset, default=0)
|
||||
|
||||
query = "SELECT * FROM download_history WHERE dismissed_at IS NOT NULL"
|
||||
params: list[Any] = []
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
query += " ORDER BY dismissed_at DESC, id DESC LIMIT ? OFFSET ?"
|
||||
params.extend([normalized_limit, normalized_offset])
|
||||
|
||||
conn = self._connect()
|
||||
try:
|
||||
rows = conn.execute(query, params).fetchall()
|
||||
payload: list[dict[str, Any]] = []
|
||||
for row in rows:
|
||||
row_dict = dict(row)
|
||||
payload.append(self._to_history_row(row_dict))
|
||||
return payload
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def clear_dismissed(self, *, user_id: int | None) -> int:
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
query = "DELETE FROM download_history WHERE dismissed_at IS NOT NULL"
|
||||
params: list[Any] = []
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(query, params)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def clear_dismissals_for_active(self, *, task_ids: set[str], user_id: int | None) -> int:
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
normalized_task_ids = [_normalize_task_id(task_id) for task_id in task_ids]
|
||||
if not normalized_task_ids:
|
||||
return 0
|
||||
|
||||
placeholders = ",".join("?" for _ in normalized_task_ids)
|
||||
query = f"UPDATE download_history SET dismissed_at = NULL WHERE task_id IN ({placeholders})"
|
||||
params: list[Any] = [*normalized_task_ids]
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(query, params)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
@@ -8,8 +8,11 @@ from threading import Lock, Event
|
||||
from typing import Dict, List, Optional, Tuple, Any, Callable
|
||||
|
||||
from shelfmark.core.config import config as app_config
|
||||
from shelfmark.core.logger import setup_logger
|
||||
from shelfmark.core.models import QueueStatus, QueueItem, DownloadTask, TERMINAL_QUEUE_STATUSES
|
||||
|
||||
logger = setup_logger(__name__)
|
||||
|
||||
|
||||
class BookQueue:
|
||||
"""Thread-safe download queue manager with priority support and cancellation."""
|
||||
@@ -55,8 +58,8 @@ class BookQueue:
|
||||
if hook is not None:
|
||||
try:
|
||||
hook(task_id, task)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as exc:
|
||||
logger.warning("Queue hook failed while adding task %s: %s", task_id, exc)
|
||||
return True
|
||||
|
||||
def get_next(self) -> Optional[Tuple[str, Event]]:
|
||||
@@ -294,8 +297,8 @@ class BookQueue:
|
||||
if hook is not None and hook_task is not None:
|
||||
try:
|
||||
hook(task_id, hook_task)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as exc:
|
||||
logger.warning("Queue hook failed while requeueing task %s: %s", task_id, exc)
|
||||
return True
|
||||
|
||||
def reorder_queue(self, task_priorities: Dict[str, int]) -> bool:
|
||||
|
||||
@@ -65,15 +65,9 @@ def coerce_int(value: Any, default: int) -> int:
|
||||
|
||||
|
||||
def normalize_optional_text(value: Any) -> str | None:
|
||||
"""Return a trimmed string or None for empty/missing input.
|
||||
|
||||
Non-string values are coerced via ``str()`` before stripping;
|
||||
``None`` short-circuits to ``None``.
|
||||
"""
|
||||
if value is None:
|
||||
return None
|
||||
"""Return a trimmed string or None for empty/non-string input."""
|
||||
if not isinstance(value, str):
|
||||
value = str(value)
|
||||
return None
|
||||
normalized = value.strip()
|
||||
return normalized or None
|
||||
|
||||
|
||||
@@ -123,15 +123,27 @@ def _resolve_title_from_book_data(book_data: Any) -> str:
|
||||
return "Unknown title"
|
||||
|
||||
|
||||
def _normalize_optional_source_id(value: Any) -> str | None:
|
||||
"""Normalize source identifiers while allowing integer provider ids."""
|
||||
if isinstance(value, bool) or value is None:
|
||||
return None
|
||||
if isinstance(value, int):
|
||||
value = str(value)
|
||||
return normalize_optional_text(value)
|
||||
|
||||
|
||||
def _build_direct_release_data_from_book_data(
|
||||
*,
|
||||
book_data: dict[str, Any],
|
||||
content_type: str,
|
||||
) -> dict[str, Any]:
|
||||
"""Build release-level payload fields for direct-download requests."""
|
||||
source_id = _normalize_optional_source_id(book_data.get("provider_id")) or _normalize_optional_source_id(
|
||||
book_data.get("id")
|
||||
)
|
||||
payload: dict[str, Any] = {
|
||||
"source": "direct_download",
|
||||
"source_id": book_data.get("provider_id") or book_data.get("id"),
|
||||
"source_id": source_id,
|
||||
"title": book_data.get("title"),
|
||||
"author": book_data.get("author"),
|
||||
"year": book_data.get("year"),
|
||||
@@ -169,8 +181,11 @@ def _normalize_direct_request_payload(
|
||||
if normalized_release_data.get("content_type") is None:
|
||||
normalized_release_data["content_type"] = content_type
|
||||
|
||||
if normalize_optional_text(normalized_release_data.get("source_id")) is None and isinstance(book_data, dict):
|
||||
fallback_source_id = normalize_optional_text(book_data.get("provider_id")) or normalize_optional_text(
|
||||
normalized_source_id = _normalize_optional_source_id(normalized_release_data.get("source_id"))
|
||||
if normalized_source_id is not None:
|
||||
normalized_release_data["source_id"] = normalized_source_id
|
||||
elif isinstance(book_data, dict):
|
||||
fallback_source_id = _normalize_optional_source_id(book_data.get("provider_id")) or _normalize_optional_source_id(
|
||||
book_data.get("id")
|
||||
)
|
||||
if fallback_source_id is not None:
|
||||
|
||||
+41
-93
@@ -7,6 +7,7 @@ import threading
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
from shelfmark.core.auth_modes import AUTH_SOURCE_BUILTIN, AUTH_SOURCE_SET
|
||||
from shelfmark.core.activity_view_state_service import user_viewer_scope
|
||||
from shelfmark.core.logger import setup_logger
|
||||
from shelfmark.core.request_helpers import normalize_optional_positive_int
|
||||
from shelfmark.core.models import QueueStatus
|
||||
@@ -57,8 +58,7 @@ CREATE TABLE IF NOT EXISTS download_requests (
|
||||
reviewed_by INTEGER REFERENCES users(id),
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
reviewed_at TIMESTAMP,
|
||||
delivery_updated_at TIMESTAMP,
|
||||
dismissed_at TIMESTAMP
|
||||
delivery_updated_at TIMESTAMP
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_download_requests_user_status_created_at
|
||||
@@ -86,19 +86,32 @@ CREATE TABLE IF NOT EXISTS download_history (
|
||||
status_message TEXT,
|
||||
download_path TEXT,
|
||||
queued_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
terminal_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||
dismissed_at TIMESTAMP
|
||||
terminal_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_download_history_user_status
|
||||
ON download_history (user_id, final_status, terminal_at DESC);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_download_history_dismissed
|
||||
ON download_history (dismissed_at) WHERE dismissed_at IS NOT NULL;
|
||||
CREATE INDEX IF NOT EXISTS idx_download_history_recent
|
||||
ON download_history (user_id, terminal_at DESC, id DESC);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_download_history_undismissed
|
||||
ON download_history (user_id, terminal_at DESC, id DESC)
|
||||
WHERE dismissed_at IS NULL;
|
||||
CREATE TABLE IF NOT EXISTS activity_view_state (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
viewer_scope TEXT NOT NULL,
|
||||
item_type TEXT NOT NULL,
|
||||
item_key TEXT NOT NULL,
|
||||
dismissed_at TIMESTAMP,
|
||||
cleared_at TIMESTAMP,
|
||||
UNIQUE(viewer_scope, item_type, item_key)
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_activity_view_state_history
|
||||
ON activity_view_state (viewer_scope, dismissed_at DESC, id DESC)
|
||||
WHERE dismissed_at IS NOT NULL AND cleared_at IS NULL;
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_activity_view_state_hidden
|
||||
ON activity_view_state (viewer_scope, item_type, item_key)
|
||||
WHERE dismissed_at IS NOT NULL;
|
||||
"""
|
||||
|
||||
|
||||
@@ -168,7 +181,6 @@ class UserDB:
|
||||
conn.executescript(_CREATE_TABLES_SQL)
|
||||
self._migrate_auth_source_column(conn)
|
||||
self._migrate_request_delivery_columns(conn)
|
||||
self._migrate_download_requests_dismissed_at(conn)
|
||||
self._migrate_download_history_queued_at(conn)
|
||||
conn.commit()
|
||||
# WAL mode must be changed outside an open transaction.
|
||||
@@ -224,17 +236,6 @@ class UserDB:
|
||||
"""
|
||||
)
|
||||
|
||||
def _migrate_download_requests_dismissed_at(self, conn: sqlite3.Connection) -> None:
|
||||
"""Ensure download_requests.dismissed_at exists for request dismissal history."""
|
||||
columns = conn.execute("PRAGMA table_info(download_requests)").fetchall()
|
||||
column_names = {str(col["name"]) for col in columns}
|
||||
if "dismissed_at" not in column_names:
|
||||
conn.execute("ALTER TABLE download_requests ADD COLUMN dismissed_at TIMESTAMP")
|
||||
conn.execute(
|
||||
"CREATE INDEX IF NOT EXISTS idx_download_requests_dismissed "
|
||||
"ON download_requests (dismissed_at) WHERE dismissed_at IS NOT NULL"
|
||||
)
|
||||
|
||||
def _migrate_download_history_queued_at(self, conn: sqlite3.Connection) -> None:
|
||||
"""Ensure download_history.queued_at exists for queue-time recording."""
|
||||
columns = conn.execute("PRAGMA table_info(download_history)").fetchall()
|
||||
@@ -349,6 +350,25 @@ class UserDB:
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
request_rows = conn.execute(
|
||||
"SELECT id FROM download_requests WHERE user_id = ?",
|
||||
(user_id,),
|
||||
).fetchall()
|
||||
request_item_keys = [f"request:{row['id']}" for row in request_rows]
|
||||
if request_item_keys:
|
||||
placeholders = ",".join("?" for _ in request_item_keys)
|
||||
conn.execute(
|
||||
f"""
|
||||
DELETE FROM activity_view_state
|
||||
WHERE item_type = 'request'
|
||||
AND item_key IN ({placeholders})
|
||||
""",
|
||||
request_item_keys,
|
||||
)
|
||||
conn.execute(
|
||||
"DELETE FROM activity_view_state WHERE viewer_scope = ?",
|
||||
(user_viewer_scope(user_id),),
|
||||
)
|
||||
conn.execute("UPDATE download_requests SET reviewed_by = NULL WHERE reviewed_by = ?", (user_id,))
|
||||
conn.execute("DELETE FROM users WHERE id = ?", (user_id,))
|
||||
conn.commit()
|
||||
@@ -600,7 +620,6 @@ class UserDB:
|
||||
"delivery_state",
|
||||
"delivery_updated_at",
|
||||
"last_failure_reason",
|
||||
"dismissed_at",
|
||||
}
|
||||
|
||||
def update_request(
|
||||
@@ -660,11 +679,6 @@ class UserDB:
|
||||
if delivery_updated_at is not None and not isinstance(delivery_updated_at, str):
|
||||
raise ValueError("delivery_updated_at must be a string when provided")
|
||||
|
||||
if "dismissed_at" in updates:
|
||||
dismissed_at = updates["dismissed_at"]
|
||||
if dismissed_at is not None and not isinstance(dismissed_at, str):
|
||||
raise ValueError("dismissed_at must be a string when provided")
|
||||
|
||||
if "content_type" in updates and not updates["content_type"]:
|
||||
raise ValueError("content_type is required")
|
||||
|
||||
@@ -785,69 +799,3 @@ class UserDB:
|
||||
return int(row["count"]) if row else 0
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def list_dismissed_requests(self, *, user_id: int | None, limit: int | None = None) -> List[Dict[str, Any]]:
|
||||
"""List dismissed requests, optionally scoped by owner user_id."""
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
params: list[Any] = []
|
||||
query = "SELECT * FROM download_requests WHERE dismissed_at IS NOT NULL"
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
query += " ORDER BY dismissed_at DESC, id DESC"
|
||||
if limit is not None and limit > 0:
|
||||
query += " LIMIT ?"
|
||||
params.append(limit)
|
||||
|
||||
conn = self._connect()
|
||||
try:
|
||||
rows = conn.execute(query, params).fetchall()
|
||||
results: List[Dict[str, Any]] = []
|
||||
for row in rows:
|
||||
parsed = self._parse_request_row(row)
|
||||
if parsed is not None:
|
||||
results.append(parsed)
|
||||
return results
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def dismiss_requests_batch(self, *, request_ids: list[int], dismissed_at: str) -> int:
|
||||
"""Set dismissed_at on multiple requests in a single UPDATE."""
|
||||
if not request_ids:
|
||||
return 0
|
||||
placeholders = ",".join("?" for _ in request_ids)
|
||||
query = f"UPDATE download_requests SET dismissed_at = ? WHERE id IN ({placeholders})"
|
||||
params: list[Any] = [dismissed_at, *request_ids]
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(query, params)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
def delete_dismissed_requests(self, *, user_id: int | None) -> int:
|
||||
"""Delete dismissed terminal requests, optionally scoped by owner user_id."""
|
||||
normalized_user_id = normalize_optional_positive_int(user_id, "user_id")
|
||||
params: list[Any] = []
|
||||
query = """
|
||||
DELETE FROM download_requests
|
||||
WHERE dismissed_at IS NOT NULL
|
||||
AND status IN ('fulfilled', 'rejected', 'cancelled')
|
||||
"""
|
||||
if normalized_user_id is not None:
|
||||
query += " AND user_id = ?"
|
||||
params.append(normalized_user_id)
|
||||
|
||||
with self._lock:
|
||||
conn = self._connect()
|
||||
try:
|
||||
cursor = conn.execute(query, params)
|
||||
conn.commit()
|
||||
rowcount = int(cursor.rowcount) if cursor.rowcount is not None else 0
|
||||
return max(rowcount, 0)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
+68
-2
@@ -50,9 +50,11 @@ from shelfmark.core.requests_service import (
|
||||
reopen_failed_request,
|
||||
sync_delivery_states_from_queue_status,
|
||||
)
|
||||
from shelfmark.core.activity_view_state_service import ActivityViewStateService
|
||||
from shelfmark.core.download_history_service import DownloadHistoryService
|
||||
from shelfmark.core.notifications import NotificationContext, NotificationEvent, notify_admin, notify_user
|
||||
from shelfmark.core.request_helpers import (
|
||||
emit_ws_event,
|
||||
coerce_bool,
|
||||
load_users_request_policy_settings,
|
||||
normalize_optional_text,
|
||||
@@ -129,10 +131,12 @@ from shelfmark.core.user_db import UserDB
|
||||
_user_db_path = _os.path.join(_os.environ.get("CONFIG_DIR", "/config"), "users.db")
|
||||
user_db: UserDB | None = None
|
||||
download_history_service: DownloadHistoryService | None = None
|
||||
activity_view_state_service: ActivityViewStateService | None = None
|
||||
try:
|
||||
user_db = UserDB(_user_db_path)
|
||||
user_db.initialize()
|
||||
download_history_service = DownloadHistoryService(_user_db_path)
|
||||
activity_view_state_service = ActivityViewStateService(_user_db_path)
|
||||
import shelfmark.config.users_settings as _ # noqa: F401 - registers users tab
|
||||
from shelfmark.core.oidc_routes import register_oidc_routes
|
||||
from shelfmark.core.admin_routes import register_admin_routes
|
||||
@@ -148,6 +152,7 @@ except (sqlite3.OperationalError, OSError) as e:
|
||||
)
|
||||
user_db = None
|
||||
download_history_service = None
|
||||
activity_view_state_service = None
|
||||
|
||||
# Start download coordinator
|
||||
backend.start()
|
||||
@@ -408,13 +413,13 @@ if user_db is not None:
|
||||
queue_release=lambda *args, **kwargs: backend.queue_release(*args, **kwargs),
|
||||
ws_manager=ws_manager,
|
||||
)
|
||||
if download_history_service is not None:
|
||||
if download_history_service is not None and activity_view_state_service is not None:
|
||||
register_activity_routes(
|
||||
app,
|
||||
user_db,
|
||||
activity_view_state_service=activity_view_state_service,
|
||||
download_history_service=download_history_service,
|
||||
resolve_auth_mode=lambda: get_auth_mode(),
|
||||
resolve_status_scope=lambda: _resolve_status_scope(),
|
||||
queue_status=lambda user_id=None: backend.queue_status(user_id=user_id),
|
||||
sync_request_delivery_states=sync_delivery_states_from_queue_status,
|
||||
emit_request_updates=lambda rows: _emit_request_update_events(rows),
|
||||
@@ -1164,6 +1169,24 @@ def _notify_admin_for_terminal_download_status(*, task_id: str, status: QueueSta
|
||||
)
|
||||
|
||||
|
||||
def _emit_activity_update_for_task(*, payload: dict[str, Any], task: Any) -> None:
|
||||
owner_user_id = normalize_positive_int(getattr(task, "user_id", None))
|
||||
emit_ws_event(
|
||||
ws_manager,
|
||||
event_name="activity_update",
|
||||
room="admins",
|
||||
payload=payload,
|
||||
)
|
||||
if owner_user_id is None:
|
||||
return
|
||||
emit_ws_event(
|
||||
ws_manager,
|
||||
event_name="activity_update",
|
||||
room=f"user_{owner_user_id}",
|
||||
payload=payload,
|
||||
)
|
||||
|
||||
|
||||
def _record_download_queued(task_id: str, task: Any) -> None:
|
||||
"""Persist initial download record when a task enters the queue."""
|
||||
if download_history_service is None:
|
||||
@@ -1197,6 +1220,32 @@ def _record_download_queued(task_id: str, task: Any) -> None:
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to record download at queue time for task %s: %s", task_id, exc)
|
||||
return
|
||||
|
||||
if activity_view_state_service is None:
|
||||
return
|
||||
|
||||
try:
|
||||
cleared_view_state = 0
|
||||
cleared_view_state += activity_view_state_service.clear_item_for_all_viewers(
|
||||
item_type="download",
|
||||
item_key=f"download:{task_id}",
|
||||
)
|
||||
if request_id is not None:
|
||||
cleared_view_state += activity_view_state_service.clear_item_for_all_viewers(
|
||||
item_type="request",
|
||||
item_key=f"request:{request_id}",
|
||||
)
|
||||
if cleared_view_state > 0:
|
||||
_emit_activity_update_for_task(
|
||||
task=task,
|
||||
payload={
|
||||
"kind": "activity_reset",
|
||||
"task_id": task_id,
|
||||
},
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to reset activity viewer state for task %s: %s", task_id, exc)
|
||||
|
||||
|
||||
def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task: Any) -> None:
|
||||
@@ -1206,6 +1255,7 @@ def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task:
|
||||
if final_status is None:
|
||||
return
|
||||
|
||||
finalized_download = False
|
||||
if download_history_service is not None:
|
||||
try:
|
||||
download_history_service.finalize_download(
|
||||
@@ -1214,9 +1264,20 @@ def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task:
|
||||
status_message=normalize_optional_text(getattr(task, "status_message", None)),
|
||||
download_path=normalize_optional_text(getattr(task, "download_path", None)),
|
||||
)
|
||||
finalized_download = True
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to finalize download history for task %s: %s", task_id, exc)
|
||||
|
||||
if finalized_download:
|
||||
_emit_activity_update_for_task(
|
||||
task=task,
|
||||
payload={
|
||||
"kind": "download_terminal",
|
||||
"task_id": task_id,
|
||||
"status": final_status,
|
||||
},
|
||||
)
|
||||
|
||||
if user_db is None or status != QueueStatus.ERROR:
|
||||
return
|
||||
|
||||
@@ -1237,6 +1298,11 @@ def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task:
|
||||
failure_reason=fallback_reason,
|
||||
)
|
||||
if reopened_request is not None:
|
||||
if activity_view_state_service is not None:
|
||||
activity_view_state_service.clear_item_for_all_viewers(
|
||||
item_type="request",
|
||||
item_key=f"request:{request_id}",
|
||||
)
|
||||
_emit_request_update_events([reopened_request])
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
|
||||
+16
-17
@@ -135,17 +135,6 @@ type PendingOnBehalfDownload =
|
||||
actingAsUser: ActingAsUserSelection;
|
||||
};
|
||||
|
||||
const mergeTerminalBucket = (
|
||||
persistedBucket: Record<string, Book> | undefined,
|
||||
realtimeBucket: Record<string, Book> | undefined
|
||||
): Record<string, Book> | undefined => {
|
||||
const merged = {
|
||||
...(persistedBucket || {}),
|
||||
...(realtimeBucket || {}),
|
||||
};
|
||||
return Object.keys(merged).length > 0 ? merged : undefined;
|
||||
};
|
||||
|
||||
function App() {
|
||||
const { toasts, showToast, removeToast } = useToast();
|
||||
const { socket } = useSocket();
|
||||
@@ -263,10 +252,12 @@ function App() {
|
||||
requestItems,
|
||||
dismissedActivityKeys,
|
||||
historyItems,
|
||||
activityHistoryLoaded,
|
||||
pendingRequestCount,
|
||||
isActivitySnapshotLoading,
|
||||
activityHistoryLoading,
|
||||
activityHistoryHasMore,
|
||||
prefetchActivityHistory,
|
||||
refreshActivitySnapshot,
|
||||
resetActivity,
|
||||
handleActivityTabChange,
|
||||
@@ -319,9 +310,9 @@ function App() {
|
||||
};
|
||||
}, [currentStatus, dismissedDownloadTaskIds]);
|
||||
|
||||
// Use real-time buckets for active work and merge persisted terminal buckets
|
||||
// so completed/errored entries survive restarts. Filter out dismissed items
|
||||
// so the sidebar counts stay consistent with the activity panel.
|
||||
// Use real-time buckets for active work and persisted activity snapshot
|
||||
// buckets for terminal history. Filter out dismissed items so the sidebar
|
||||
// counts stay consistent with the activity panel.
|
||||
const activitySidebarStatus = useMemo<StatusData>(() => {
|
||||
const filterDismissed = (
|
||||
bucket: Record<string, Book> | undefined
|
||||
@@ -338,9 +329,9 @@ function App() {
|
||||
resolving: currentStatus.resolving,
|
||||
locating: currentStatus.locating,
|
||||
downloading: currentStatus.downloading,
|
||||
complete: filterDismissed(mergeTerminalBucket(activityStatus.complete, currentStatus.complete)),
|
||||
error: filterDismissed(mergeTerminalBucket(activityStatus.error, currentStatus.error)),
|
||||
cancelled: filterDismissed(mergeTerminalBucket(activityStatus.cancelled, currentStatus.cancelled)),
|
||||
complete: filterDismissed(activityStatus.complete),
|
||||
error: filterDismissed(activityStatus.error),
|
||||
cancelled: filterDismissed(activityStatus.cancelled),
|
||||
};
|
||||
}, [activityStatus, currentStatus, dismissedDownloadTaskIds]);
|
||||
|
||||
@@ -430,6 +421,13 @@ function App() {
|
||||
const [sidebarPinnedOpen, setSidebarPinnedOpen] = useState(false);
|
||||
const [headerHeight, setHeaderHeight] = useState(0);
|
||||
const headerObserverRef = useRef<ResizeObserver | null>(null);
|
||||
useEffect(() => {
|
||||
if (!downloadsSidebarOpen) {
|
||||
return;
|
||||
}
|
||||
prefetchActivityHistory();
|
||||
}, [downloadsSidebarOpen, prefetchActivityHistory]);
|
||||
|
||||
const headerRef = useCallback((el: HTMLDivElement | null) => {
|
||||
if (headerObserverRef.current) {
|
||||
headerObserverRef.current.disconnect();
|
||||
@@ -1696,6 +1694,7 @@ function App() {
|
||||
requestItems={requestItems}
|
||||
dismissedItemKeys={dismissedActivityKeys}
|
||||
historyItems={historyItems}
|
||||
historyLoaded={activityHistoryLoaded}
|
||||
historyHasMore={activityHistoryHasMore}
|
||||
historyLoading={activityHistoryLoading}
|
||||
onHistoryLoadMore={handleActivityHistoryLoadMore}
|
||||
|
||||
@@ -402,17 +402,20 @@ function ShimmerBlock({ className }: { className: string }) {
|
||||
}
|
||||
|
||||
// Loading skeleton for releases - matches ReleaseRow layout
|
||||
// Renders enough rows to fill the container, fading out at the bottom via a gradient mask
|
||||
function ReleaseSkeleton() {
|
||||
// Render enough rows to cover tall viewports; overflow is hidden by the mask
|
||||
const rows = 8;
|
||||
return (
|
||||
<div className="divide-y divide-gray-200/60 dark:divide-gray-800/60">
|
||||
{[1, 2, 3, 4, 5].map((i) => (
|
||||
<div
|
||||
key={i}
|
||||
className="px-5 py-2"
|
||||
style={{
|
||||
opacity: 1 - (i - 1) * 0.15, // Fade out lower rows
|
||||
}}
|
||||
>
|
||||
<div
|
||||
className="divide-y divide-gray-200/60 dark:divide-gray-800/60 overflow-hidden"
|
||||
style={{
|
||||
maskImage: 'linear-gradient(to bottom, black 40%, transparent 100%)',
|
||||
WebkitMaskImage: 'linear-gradient(to bottom, black 40%, transparent 100%)',
|
||||
}}
|
||||
>
|
||||
{Array.from({ length: rows }, (_, i) => (
|
||||
<div key={i} className="px-5 py-2">
|
||||
<div className="grid grid-cols-[auto_1fr_auto] sm:grid-cols-[auto_minmax(0,2fr)_60px_80px_80px_auto] items-center gap-2 sm:gap-3">
|
||||
{/* Thumbnail skeleton */}
|
||||
<ShimmerBlock className="w-7 h-10 sm:w-10 sm:h-14" />
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useMemo, useRef, useState, type WheelEvent } from 'react';
|
||||
import { useCallback, useEffect, useMemo, useRef, useState, type WheelEvent } from 'react';
|
||||
import { RequestRecord, StatusData } from '../../types';
|
||||
import { downloadToActivityItem, DownloadStatusKey } from './activityMappers';
|
||||
import { ActivityItem } from './activityTypes';
|
||||
@@ -17,6 +17,7 @@ interface ActivitySidebarProps {
|
||||
requestItems: ActivityItem[];
|
||||
dismissedItemKeys?: string[];
|
||||
historyItems?: ActivityItem[];
|
||||
historyLoaded?: boolean;
|
||||
historyHasMore?: boolean;
|
||||
historyLoading?: boolean;
|
||||
onHistoryLoadMore?: () => void;
|
||||
@@ -247,6 +248,7 @@ export const ActivitySidebar = ({
|
||||
requestItems,
|
||||
dismissedItemKeys = [],
|
||||
historyItems = [],
|
||||
historyLoaded = false,
|
||||
historyHasMore = false,
|
||||
historyLoading = false,
|
||||
onHistoryLoadMore,
|
||||
@@ -274,6 +276,10 @@ export const ActivitySidebar = ({
|
||||
() => new Set(dismissedItemKeys),
|
||||
[dismissedItemKeys]
|
||||
);
|
||||
const handleTabChange = useCallback((nextTab: ActivityTabKey) => {
|
||||
setActiveTab(nextTab);
|
||||
onActiveTabChange?.(nextTab);
|
||||
}, [onActiveTabChange]);
|
||||
|
||||
useEffect(() => {
|
||||
const mediaQuery = window.matchMedia('(min-width: 1024px)');
|
||||
@@ -292,13 +298,9 @@ export const ActivitySidebar = ({
|
||||
|
||||
useEffect(() => {
|
||||
if (!showRequestsTab && activeTab === 'requests') {
|
||||
setActiveTab('all');
|
||||
handleTabChange('all');
|
||||
}
|
||||
}, [showRequestsTab, activeTab]);
|
||||
|
||||
useEffect(() => {
|
||||
onActiveTabChange?.(activeTab);
|
||||
}, [activeTab, onActiveTabChange]);
|
||||
}, [showRequestsTab, activeTab, handleTabChange]);
|
||||
|
||||
useEffect(() => {
|
||||
if (activeTab === 'downloads') {
|
||||
@@ -452,9 +454,10 @@ export const ActivitySidebar = ({
|
||||
}
|
||||
return requestStatus === 'fulfilled' && item.kind === 'request';
|
||||
})
|
||||
: activeTab === 'history'
|
||||
? historyItems
|
||||
: mergedDownloadItems;
|
||||
: activeTab === 'history'
|
||||
? historyItems
|
||||
: mergedDownloadItems;
|
||||
const isHistoryInitialLoad = activeTab === 'history' && !historyLoaded;
|
||||
|
||||
const availableUsers = useMemo(() => {
|
||||
const userMap = new Map<string, string>();
|
||||
@@ -702,12 +705,12 @@ export const ActivitySidebar = ({
|
||||
)}
|
||||
</Dropdown>
|
||||
)}
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setActiveTab((current) => (current === 'history' ? 'all' : 'history'))}
|
||||
className={`relative h-9 w-9 inline-flex items-center justify-center rounded-full hover-action transition-colors ${
|
||||
activeTab === 'history' ? 'text-sky-600 dark:text-sky-400' : ''
|
||||
}`}
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => handleTabChange(activeTab === 'history' ? 'all' : 'history')}
|
||||
className={`relative h-9 w-9 inline-flex items-center justify-center rounded-full hover-action transition-colors ${
|
||||
activeTab === 'history' ? 'text-sky-600 dark:text-sky-400' : ''
|
||||
}`}
|
||||
title={activeTab === 'history' ? 'Back to activity' : 'Open history'}
|
||||
aria-label={activeTab === 'history' ? 'Back to activity' : 'Open history'}
|
||||
aria-pressed={activeTab === 'history'}
|
||||
@@ -745,7 +748,7 @@ export const ActivitySidebar = ({
|
||||
<button
|
||||
type="button"
|
||||
ref={(el) => { tabRefs.current.all = el; }}
|
||||
onClick={() => setActiveTab('all')}
|
||||
onClick={() => handleTabChange('all')}
|
||||
className={`px-4 py-2.5 text-sm font-medium border-b-2 border-transparent transition-colors whitespace-nowrap ${
|
||||
activeTab === 'all'
|
||||
? 'text-sky-600 dark:text-sky-400'
|
||||
@@ -758,7 +761,7 @@ export const ActivitySidebar = ({
|
||||
<button
|
||||
type="button"
|
||||
ref={(el) => { tabRefs.current.downloads = el; }}
|
||||
onClick={() => setActiveTab('downloads')}
|
||||
onClick={() => handleTabChange('downloads')}
|
||||
className={`px-4 py-2.5 text-sm font-medium border-b-2 border-transparent transition-colors whitespace-nowrap ${
|
||||
activeTab === 'downloads'
|
||||
? 'text-sky-600 dark:text-sky-400'
|
||||
@@ -777,7 +780,7 @@ export const ActivitySidebar = ({
|
||||
<button
|
||||
type="button"
|
||||
ref={(el) => { tabRefs.current.requests = el; }}
|
||||
onClick={() => setActiveTab('requests')}
|
||||
onClick={() => handleTabChange('requests')}
|
||||
className={`px-4 py-2.5 text-sm font-medium border-b-2 border-transparent transition-colors whitespace-nowrap ${
|
||||
activeTab === 'requests'
|
||||
? 'text-sky-600 dark:text-sky-400'
|
||||
@@ -808,7 +811,7 @@ export const ActivitySidebar = ({
|
||||
{activeTab === 'requests'
|
||||
? isRequestsLoading ? 'Loading requests...' : 'No requests'
|
||||
: activeTab === 'history'
|
||||
? historyLoading ? 'Loading history...' : 'No history'
|
||||
? (historyLoading || isHistoryInitialLoad) ? 'Loading history...' : 'No history'
|
||||
: activeTab === 'downloads'
|
||||
? 'No downloads'
|
||||
: 'No activity'}
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import type { ActivityItem } from './activityTypes.js';
|
||||
|
||||
export const dedupeHistoryItems = (items: ActivityItem[]): ActivityItem[] => {
|
||||
const requestIdsWithDownloadRows = new Set<number>();
|
||||
|
||||
items.forEach((item) => {
|
||||
if (item.kind === 'download' && typeof item.requestId === 'number') {
|
||||
requestIdsWithDownloadRows.add(item.requestId);
|
||||
}
|
||||
});
|
||||
|
||||
if (!requestIdsWithDownloadRows.size) {
|
||||
return items;
|
||||
}
|
||||
|
||||
return items.filter((item) => {
|
||||
if (item.kind !== 'request' || typeof item.requestId !== 'number') {
|
||||
return true;
|
||||
}
|
||||
if (!requestIdsWithDownloadRows.has(item.requestId)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const requestStatus = item.requestRecord?.status;
|
||||
return requestStatus !== 'fulfilled' && item.visualStatus !== 'fulfilled';
|
||||
});
|
||||
};
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
downloadToActivityItem,
|
||||
requestToActivityItem,
|
||||
} from '../components/activity';
|
||||
import { dedupeHistoryItems } from '../components/activity/activityHistory.js';
|
||||
|
||||
const HISTORY_PAGE_SIZE = 50;
|
||||
|
||||
@@ -120,10 +121,12 @@ interface UseActivityResult {
|
||||
requestItems: ActivityItem[];
|
||||
dismissedActivityKeys: string[];
|
||||
historyItems: ActivityItem[];
|
||||
activityHistoryLoaded: boolean;
|
||||
pendingRequestCount: number;
|
||||
isActivitySnapshotLoading: boolean;
|
||||
activityHistoryLoading: boolean;
|
||||
activityHistoryHasMore: boolean;
|
||||
prefetchActivityHistory: () => void;
|
||||
refreshActivitySnapshot: () => Promise<void>;
|
||||
handleActivityTabChange: (tab: 'all' | 'downloads' | 'requests' | 'history') => void;
|
||||
resetActivity: () => void;
|
||||
@@ -217,6 +220,13 @@ export const useActivity = ({
|
||||
void refreshActivityHistory();
|
||||
}, [activityHistoryLoaded, activityHistoryLoading, refreshActivityHistory]);
|
||||
|
||||
const prefetchActivityHistory = useCallback(() => {
|
||||
if (activityHistoryLoaded || activityHistoryLoading) {
|
||||
return;
|
||||
}
|
||||
void refreshActivityHistory();
|
||||
}, [activityHistoryLoaded, activityHistoryLoading, refreshActivityHistory]);
|
||||
|
||||
const handleActivityHistoryLoadMore = useCallback(() => {
|
||||
if (!isAuthenticated || activityHistoryLoading || !activityHistoryHasMore) {
|
||||
return;
|
||||
@@ -282,44 +292,7 @@ export const useActivity = ({
|
||||
.map((row) => mapHistoryRowToActivityItem(row, isAdmin ? 'admin' : 'user'))
|
||||
.sort((left, right) => right.timestamp - left.timestamp);
|
||||
|
||||
// Request history entries with attached release metadata are approval artifacts,
|
||||
// not actual download outcomes. Keep history focused on concrete download results.
|
||||
const nonAttachedReleaseRequestItems = mappedItems.filter((item) => {
|
||||
if (item.kind !== 'request') {
|
||||
return true;
|
||||
}
|
||||
|
||||
const releaseData = item.requestRecord?.release_data;
|
||||
if (!releaseData || typeof releaseData !== 'object') {
|
||||
return true;
|
||||
}
|
||||
|
||||
return Object.keys(releaseData as Record<string, unknown>).length === 0;
|
||||
});
|
||||
|
||||
// Download dismissals already carry linked request context; hide redundant
|
||||
// fulfilled-request history rows that would otherwise appear as "Approved".
|
||||
const requestIdsWithDownloadRows = new Set<number>();
|
||||
nonAttachedReleaseRequestItems.forEach((item) => {
|
||||
if (item.kind === 'download' && typeof item.requestId === 'number') {
|
||||
requestIdsWithDownloadRows.add(item.requestId);
|
||||
}
|
||||
});
|
||||
|
||||
if (!requestIdsWithDownloadRows.size) {
|
||||
return nonAttachedReleaseRequestItems;
|
||||
}
|
||||
|
||||
return nonAttachedReleaseRequestItems.filter((item) => {
|
||||
if (item.kind !== 'request' || typeof item.requestId !== 'number') {
|
||||
return true;
|
||||
}
|
||||
if (!requestIdsWithDownloadRows.has(item.requestId)) {
|
||||
return true;
|
||||
}
|
||||
const requestStatus = item.requestRecord?.status;
|
||||
return requestStatus !== 'fulfilled' && item.visualStatus !== 'fulfilled';
|
||||
});
|
||||
return dedupeHistoryItems(mappedItems);
|
||||
},
|
||||
[activityHistoryRows, isAdmin]
|
||||
);
|
||||
@@ -421,10 +394,12 @@ export const useActivity = ({
|
||||
requestItems,
|
||||
dismissedActivityKeys,
|
||||
historyItems,
|
||||
activityHistoryLoaded,
|
||||
pendingRequestCount,
|
||||
isActivitySnapshotLoading,
|
||||
activityHistoryLoading,
|
||||
activityHistoryHasMore,
|
||||
prefetchActivityHistory,
|
||||
refreshActivitySnapshot,
|
||||
handleActivityTabChange,
|
||||
resetActivity,
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
import * as assert from 'node:assert/strict';
|
||||
import { describe, it } from 'node:test';
|
||||
import type { ActivityItem } from '../components/activity/activityTypes.js';
|
||||
import { dedupeHistoryItems } from '../components/activity/activityHistory.js';
|
||||
|
||||
const makeRequestItem = (overrides: Partial<ActivityItem> = {}): ActivityItem => ({
|
||||
id: 'request-42',
|
||||
kind: 'request',
|
||||
visualStatus: 'fulfilled',
|
||||
title: 'Request Title',
|
||||
author: 'Request Author',
|
||||
metaLine: 'Release request',
|
||||
statusLabel: 'Approved',
|
||||
timestamp: 1,
|
||||
requestId: 42,
|
||||
requestLevel: 'release',
|
||||
requestRecord: {
|
||||
id: 42,
|
||||
user_id: 7,
|
||||
status: 'fulfilled',
|
||||
source_hint: 'prowlarr',
|
||||
content_type: 'ebook',
|
||||
request_level: 'release',
|
||||
policy_mode: 'request_release',
|
||||
book_data: { title: 'Request Title', author: 'Request Author' },
|
||||
release_data: { source: 'prowlarr', source_id: 'release-42' },
|
||||
note: null,
|
||||
admin_note: null,
|
||||
reviewed_by: null,
|
||||
reviewed_at: null,
|
||||
created_at: '2026-02-13T12:00:00Z',
|
||||
updated_at: '2026-02-13T12:00:00Z',
|
||||
username: 'alice',
|
||||
},
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const makeDownloadItem = (overrides: Partial<ActivityItem> = {}): ActivityItem => ({
|
||||
id: 'download-42',
|
||||
kind: 'download',
|
||||
visualStatus: 'complete',
|
||||
title: 'Request Title',
|
||||
author: 'Request Author',
|
||||
metaLine: 'EPUB · 2 MB · Prowlarr',
|
||||
statusLabel: 'Complete',
|
||||
timestamp: 2,
|
||||
downloadBookId: 'download-42',
|
||||
requestId: 42,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe('dedupeHistoryItems', () => {
|
||||
it('keeps release-level request history rows when they are not duplicated by a download row', () => {
|
||||
const rejectedRequest = makeRequestItem({
|
||||
id: 'request-7',
|
||||
visualStatus: 'rejected',
|
||||
statusLabel: 'Rejected',
|
||||
requestId: 7,
|
||||
requestRecord: {
|
||||
...makeRequestItem().requestRecord!,
|
||||
id: 7,
|
||||
status: 'rejected',
|
||||
release_data: { source: 'prowlarr', source_id: 'release-7' },
|
||||
},
|
||||
});
|
||||
|
||||
const items = dedupeHistoryItems([rejectedRequest]);
|
||||
|
||||
assert.deepEqual(items, [rejectedRequest]);
|
||||
});
|
||||
|
||||
it('drops fulfilled request history rows when a linked download row is present', () => {
|
||||
const requestItem = makeRequestItem();
|
||||
const downloadItem = makeDownloadItem();
|
||||
|
||||
const items = dedupeHistoryItems([requestItem, downloadItem]);
|
||||
|
||||
assert.deepEqual(items, [downloadItem]);
|
||||
});
|
||||
|
||||
it('keeps fulfilled request history rows when no linked download row exists', () => {
|
||||
const requestItem = makeRequestItem();
|
||||
|
||||
const items = dedupeHistoryItems([requestItem]);
|
||||
|
||||
assert.deepEqual(items, [requestItem]);
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import uuid
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import ANY, patch
|
||||
|
||||
import pytest
|
||||
@@ -93,6 +94,13 @@ def _sample_status_payload() -> dict:
|
||||
}
|
||||
|
||||
|
||||
def _hidden_item_keys(main_module, *, viewer_scope: str) -> set[str]:
|
||||
return {
|
||||
row["item_key"]
|
||||
for row in main_module.activity_view_state_service.list_hidden(viewer_scope=viewer_scope)
|
||||
}
|
||||
|
||||
|
||||
class TestActivityRoutes:
|
||||
def test_snapshot_returns_status_requests_and_dismissed(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
@@ -162,10 +170,11 @@ class TestActivityRoutes:
|
||||
|
||||
assert clear_history_response.status_code == 200
|
||||
assert clear_history_response.json["status"] == "cleared"
|
||||
assert clear_history_response.json["deleted_count"] == 1
|
||||
assert clear_history_response.json["cleared_count"] == 1
|
||||
|
||||
assert history_after_clear.status_code == 200
|
||||
assert history_after_clear.json == []
|
||||
assert main_module.download_history_service.get_by_task_id("test-task") is not None
|
||||
|
||||
def test_dismiss_preserves_terminal_snapshot_without_live_queue_merge(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
@@ -197,7 +206,7 @@ class TestActivityRoutes:
|
||||
assert snapshot_download["author"] == "Recorded Author"
|
||||
assert snapshot_download["status_message"] is None
|
||||
|
||||
def test_clear_history_deletes_dismissed_requests_from_snapshot(self, main_module, client):
|
||||
def test_clear_history_hides_dismissed_requests_without_deleting_them(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
@@ -233,13 +242,15 @@ class TestActivityRoutes:
|
||||
|
||||
assert clear_history_response.status_code == 200
|
||||
assert clear_history_response.json["status"] == "cleared"
|
||||
assert clear_history_response.json["cleared_count"] == 1
|
||||
|
||||
assert history_after_clear.status_code == 200
|
||||
assert history_after_clear.json == []
|
||||
|
||||
assert snapshot_after_clear.status_code == 200
|
||||
assert all(row["id"] != request_row["id"] for row in snapshot_after_clear.json["requests"])
|
||||
assert {"item_type": "request", "item_key": request_key} not in snapshot_after_clear.json["dismissed"]
|
||||
assert {"item_type": "request", "item_key": request_key} in snapshot_after_clear.json["dismissed"]
|
||||
assert main_module.user_db.get_request(request_row["id"]) is not None
|
||||
|
||||
def test_admin_snapshot_includes_admin_viewer_dismissals(self, main_module, client):
|
||||
admin = _create_user(main_module, prefix="admin", role="admin")
|
||||
@@ -344,6 +355,75 @@ class TestActivityRoutes:
|
||||
assert response.status_code == 403
|
||||
assert response.json["code"] == "user_identity_unavailable"
|
||||
|
||||
def test_dismiss_returns_404_when_download_history_row_is_missing(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "download", "item_key": "download:missing-task"},
|
||||
)
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json["error"] == "Download not found"
|
||||
|
||||
def test_dismiss_rejects_active_download(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
main_module.download_history_service.record_download(
|
||||
task_id="active-dismiss-task",
|
||||
user_id=user["id"],
|
||||
username=user["username"],
|
||||
request_id=None,
|
||||
source="direct_download",
|
||||
source_display_name="Direct Download",
|
||||
title="Active Dismiss Task",
|
||||
author="Author",
|
||||
format="epub",
|
||||
size="1 MB",
|
||||
preview=None,
|
||||
content_type="ebook",
|
||||
origin="direct",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "download", "item_key": "download:active-dismiss-task"},
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
assert response.json["error"] == "Only terminal downloads can be dismissed"
|
||||
|
||||
def test_dismiss_rejects_pending_request(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
request_row = main_module.user_db.create_request(
|
||||
user_id=user["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data={
|
||||
"title": "Pending Request",
|
||||
"author": "Pending Author",
|
||||
"provider": "openlibrary",
|
||||
"provider_id": "pending-dismiss-1",
|
||||
},
|
||||
status="pending",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "request", "item_key": f"request:{request_row['id']}"},
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
assert response.json["error"] == "Only terminal requests can be dismissed"
|
||||
|
||||
def test_dismiss_emits_activity_update_to_user_room(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
@@ -369,6 +449,32 @@ class TestActivityRoutes:
|
||||
to=f"user_{user['id']}",
|
||||
)
|
||||
|
||||
def test_admin_dismiss_emits_activity_update_to_admin_room(self, main_module, client):
|
||||
admin = _create_user(main_module, prefix="admin", role="admin")
|
||||
owner = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
|
||||
_record_terminal_download(
|
||||
main_module,
|
||||
task_id="admin-dismiss-room-task",
|
||||
user_id=owner["id"],
|
||||
username=owner["username"],
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(main_module.ws_manager, "is_enabled", return_value=True):
|
||||
with patch.object(main_module.ws_manager.socketio, "emit") as mock_emit:
|
||||
response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "download", "item_key": "download:admin-dismiss-room-task"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
mock_emit.assert_called_once_with(
|
||||
"activity_update",
|
||||
ANY,
|
||||
to="admins",
|
||||
)
|
||||
|
||||
def test_dismiss_many_preserves_terminal_snapshots_without_live_queue_merge(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
@@ -414,6 +520,39 @@ class TestActivityRoutes:
|
||||
assert rows_by_key[f"download:{second_task_id}"]["snapshot"]["download"]["title"] == "Second Title"
|
||||
assert rows_by_key[f"download:{second_task_id}"]["snapshot"]["download"]["author"] == "Second Author"
|
||||
|
||||
def test_dismiss_many_returns_404_without_partial_dismiss_when_any_item_is_missing(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
existing_task_id = "dismiss-many-existing"
|
||||
_record_terminal_download(
|
||||
main_module,
|
||||
task_id=existing_task_id,
|
||||
user_id=user["id"],
|
||||
username=user["username"],
|
||||
title="Existing Title",
|
||||
author="Existing Author",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
response = client.post(
|
||||
"/api/activity/dismiss-many",
|
||||
json={
|
||||
"items": [
|
||||
{"item_type": "download", "item_key": f"download:{existing_task_id}"},
|
||||
{"item_type": "download", "item_key": "download:missing-bulk-task"},
|
||||
]
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 404
|
||||
assert response.json["error"] == "One or more activity items were not found"
|
||||
assert response.json["missing_item_keys"] == ["download:missing-bulk-task"]
|
||||
assert f"download:{existing_task_id}" not in _hidden_item_keys(
|
||||
main_module,
|
||||
viewer_scope=f"user:{user['id']}",
|
||||
)
|
||||
|
||||
def test_no_auth_dismiss_many_and_history_use_shared_identity(self, main_module):
|
||||
task_id = f"no-auth-{uuid.uuid4().hex[:10]}"
|
||||
item_key = f"download:{task_id}"
|
||||
@@ -474,8 +613,8 @@ class TestActivityRoutes:
|
||||
assert response.status_code == 200
|
||||
assert response.json["status"] == "dismissed"
|
||||
|
||||
dismissals = main_module.download_history_service.get_dismissed_keys(user_id=None)
|
||||
assert task_id in dismissals
|
||||
dismissals = _hidden_item_keys(main_module, viewer_scope="noauth:shared")
|
||||
assert item_key in dismissals
|
||||
|
||||
def test_no_auth_dismiss_many_uses_shared_identity_even_with_valid_session_db_user(
|
||||
self,
|
||||
@@ -643,7 +782,32 @@ class TestActivityRoutes:
|
||||
assert "active-downloading-task" in response.json["status"]["downloading"]
|
||||
assert response.json["status"]["downloading"]["active-downloading-task"]["progress"] == 0.5
|
||||
|
||||
def test_snapshot_clears_stale_download_dismissal_when_same_task_is_active(self, main_module, client):
|
||||
def test_snapshot_ignores_queue_only_active_download_without_history_row(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
active_status = _sample_status_payload()
|
||||
active_status["downloading"] = {
|
||||
"queue-only-task": {
|
||||
"id": "queue-only-task",
|
||||
"title": "Queue Only Task",
|
||||
"author": "Queue Author",
|
||||
"source": "direct_download",
|
||||
"progress": 0.5,
|
||||
"status_message": "Downloading 50%",
|
||||
"user_id": user["id"],
|
||||
"username": user["username"],
|
||||
}
|
||||
}
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(main_module.backend, "queue_status", return_value=active_status):
|
||||
response = client.get("/api/activity/snapshot")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert "queue-only-task" not in response.json["status"]["downloading"]
|
||||
|
||||
def test_queue_hook_clears_download_view_state_when_task_is_requeued(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
@@ -661,27 +825,31 @@ class TestActivityRoutes:
|
||||
json={"item_type": "download", "item_key": "download:task-reused-1"},
|
||||
)
|
||||
assert dismiss_response.status_code == 200
|
||||
assert "download:task-reused-1" in _hidden_item_keys(
|
||||
main_module,
|
||||
viewer_scope=f"user:{user['id']}",
|
||||
)
|
||||
|
||||
active_status = _sample_status_payload()
|
||||
active_status["downloading"] = {
|
||||
"task-reused-1": {
|
||||
"id": "task-reused-1",
|
||||
"title": "Reused Task",
|
||||
"author": "Author",
|
||||
"source": "direct_download",
|
||||
"added_time": 1,
|
||||
}
|
||||
}
|
||||
main_module._record_download_queued(
|
||||
"task-reused-1",
|
||||
SimpleNamespace(
|
||||
user_id=user["id"],
|
||||
username=user["username"],
|
||||
request_id=None,
|
||||
source="direct_download",
|
||||
title="Reused Task",
|
||||
author="Author",
|
||||
format="epub",
|
||||
size="1 MB",
|
||||
preview=None,
|
||||
content_type="ebook",
|
||||
),
|
||||
)
|
||||
|
||||
with patch.object(main_module.backend, "queue_status", return_value=active_status):
|
||||
snapshot_response = client.get("/api/activity/snapshot")
|
||||
|
||||
assert snapshot_response.status_code == 200
|
||||
assert {
|
||||
"item_type": "download",
|
||||
"item_key": "download:task-reused-1",
|
||||
} not in snapshot_response.json["dismissed"]
|
||||
assert "task-reused-1" not in main_module.download_history_service.get_dismissed_keys(user_id=user["id"])
|
||||
assert "download:task-reused-1" not in _hidden_item_keys(
|
||||
main_module,
|
||||
viewer_scope=f"user:{user['id']}",
|
||||
)
|
||||
|
||||
def test_dismiss_state_is_isolated_per_user(self, main_module, client):
|
||||
user_one = _create_user(main_module, prefix="reader-one")
|
||||
@@ -712,6 +880,60 @@ class TestActivityRoutes:
|
||||
assert snapshot_two.status_code == 200
|
||||
assert {"item_type": "download", "item_key": "download:shared-task"} not in snapshot_two.json["dismissed"]
|
||||
|
||||
def test_admin_dismiss_and_clear_do_not_affect_owner_view(self, main_module, client):
|
||||
admin = _create_user(main_module, prefix="admin", role="admin")
|
||||
owner = _create_user(main_module, prefix="reader")
|
||||
task_id = f"admin-owned-{uuid.uuid4().hex[:8]}"
|
||||
|
||||
_record_terminal_download(
|
||||
main_module,
|
||||
task_id=task_id,
|
||||
user_id=owner["id"],
|
||||
username=owner["username"],
|
||||
title="Admin Owned Task",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
|
||||
dismiss_response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "download", "item_key": f"download:{task_id}"},
|
||||
)
|
||||
assert dismiss_response.status_code == 200
|
||||
|
||||
admin_history = client.get("/api/activity/history?limit=10&offset=0")
|
||||
assert admin_history.status_code == 200
|
||||
assert any(row["item_key"] == f"download:{task_id}" for row in admin_history.json)
|
||||
|
||||
_set_session(client, user_id=owner["username"], db_user_id=owner["id"], is_admin=False)
|
||||
with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()):
|
||||
owner_snapshot_after_admin_dismiss = client.get("/api/activity/snapshot")
|
||||
assert owner_snapshot_after_admin_dismiss.status_code == 200
|
||||
assert task_id in owner_snapshot_after_admin_dismiss.json["status"]["complete"]
|
||||
assert {
|
||||
"item_type": "download",
|
||||
"item_key": f"download:{task_id}",
|
||||
} not in owner_snapshot_after_admin_dismiss.json["dismissed"]
|
||||
|
||||
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
|
||||
clear_response = client.delete("/api/activity/history")
|
||||
assert clear_response.status_code == 200
|
||||
assert clear_response.json["cleared_count"] >= 1
|
||||
|
||||
_set_session(client, user_id=owner["username"], db_user_id=owner["id"], is_admin=False)
|
||||
with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()):
|
||||
owner_snapshot_after_admin_clear = client.get("/api/activity/snapshot")
|
||||
owner_history = client.get("/api/activity/history?limit=10&offset=0")
|
||||
|
||||
assert owner_snapshot_after_admin_clear.status_code == 200
|
||||
assert task_id in owner_snapshot_after_admin_clear.json["status"]["complete"]
|
||||
assert {
|
||||
"item_type": "download",
|
||||
"item_key": f"download:{task_id}",
|
||||
} not in owner_snapshot_after_admin_clear.json["dismissed"]
|
||||
assert owner_history.status_code == 200
|
||||
assert owner_history.json == []
|
||||
|
||||
def test_admin_request_dismissal_is_shared_across_admin_users(self, main_module, client):
|
||||
admin_one = _create_user(main_module, prefix="admin-one", role="admin")
|
||||
admin_two = _create_user(main_module, prefix="admin-two", role="admin")
|
||||
@@ -749,6 +971,37 @@ class TestActivityRoutes:
|
||||
assert history_response.status_code == 200
|
||||
assert any(row["item_key"] == f"request:{request_row['id']}" for row in history_response.json)
|
||||
|
||||
def test_admin_request_history_includes_requester_username(self, main_module, client):
|
||||
admin = _create_user(main_module, prefix="admin", role="admin")
|
||||
owner = _create_user(main_module, prefix="request-owner")
|
||||
request_row = main_module.user_db.create_request(
|
||||
user_id=owner["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data={
|
||||
"title": "History Username Request",
|
||||
"author": "History Username Author",
|
||||
"provider": "openlibrary",
|
||||
"provider_id": "history-username-request-1",
|
||||
},
|
||||
status="rejected",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
|
||||
dismiss_response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "request", "item_key": f"request:{request_row['id']}"},
|
||||
)
|
||||
history_response = client.get("/api/activity/history?limit=10&offset=0")
|
||||
|
||||
assert dismiss_response.status_code == 200
|
||||
assert history_response.status_code == 200
|
||||
matching_rows = [row for row in history_response.json if row["item_key"] == f"request:{request_row['id']}"]
|
||||
assert len(matching_rows) == 1
|
||||
assert matching_rows[0]["snapshot"]["request"]["username"] == owner["username"]
|
||||
|
||||
def test_history_paging_is_stable_and_non_overlapping(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="history-user")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
@@ -762,7 +1015,11 @@ class TestActivityRoutes:
|
||||
username=user["username"],
|
||||
title=f"History Task {index}",
|
||||
)
|
||||
main_module.download_history_service.dismiss(task_id=task_id, user_id=user["id"])
|
||||
main_module.activity_view_state_service.dismiss(
|
||||
viewer_scope=f"user:{user['id']}",
|
||||
item_type="download",
|
||||
item_key=f"download:{task_id}",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
page_one = client.get("/api/activity/history?limit=2&offset=0")
|
||||
@@ -824,7 +1081,11 @@ class TestActivityRoutes:
|
||||
user_id=user["id"],
|
||||
username=user["username"],
|
||||
)
|
||||
main_module.download_history_service.dismiss(task_id="history-clear-task", user_id=user["id"])
|
||||
main_module.activity_view_state_service.dismiss(
|
||||
viewer_scope=f"user:{user['id']}",
|
||||
item_type="download",
|
||||
item_key="download:history-clear-task",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(main_module.ws_manager, "is_enabled", return_value=True):
|
||||
@@ -837,3 +1098,32 @@ class TestActivityRoutes:
|
||||
ANY,
|
||||
to=f"user_{user['id']}",
|
||||
)
|
||||
|
||||
def test_admin_clear_history_emits_activity_update_to_admin_room(self, main_module, client):
|
||||
admin = _create_user(main_module, prefix="admin", role="admin")
|
||||
owner = _create_user(main_module, prefix="reader")
|
||||
task_id = f"admin-history-clear-{uuid.uuid4().hex[:8]}"
|
||||
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
|
||||
_record_terminal_download(
|
||||
main_module,
|
||||
task_id=task_id,
|
||||
user_id=owner["id"],
|
||||
username=owner["username"],
|
||||
)
|
||||
main_module.activity_view_state_service.dismiss(
|
||||
viewer_scope="admin:shared",
|
||||
item_type="download",
|
||||
item_key=f"download:{task_id}",
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(main_module.ws_manager, "is_enabled", return_value=True):
|
||||
with patch.object(main_module.ws_manager.socketio, "emit") as mock_emit:
|
||||
response = client.delete("/api/activity/history")
|
||||
|
||||
assert response.status_code == 200
|
||||
mock_emit.assert_called_once_with(
|
||||
"activity_update",
|
||||
ANY,
|
||||
to="admins",
|
||||
)
|
||||
|
||||
@@ -4,7 +4,8 @@ from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import uuid
|
||||
from unittest.mock import patch
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import ANY, patch
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -162,6 +163,36 @@ class TestTerminalSnapshotCapture:
|
||||
finally:
|
||||
main_module.backend.book_queue.cancel_download(task_id)
|
||||
|
||||
def test_complete_transition_emits_activity_update_to_owner_and_admin_rooms(self, main_module):
|
||||
user = _create_user(main_module, prefix="snap-activity-update")
|
||||
task_id = f"activity-update-{uuid.uuid4().hex[:8]}"
|
||||
task = DownloadTask(
|
||||
task_id=task_id,
|
||||
source="direct_download",
|
||||
title="Activity Update Snapshot",
|
||||
user_id=user["id"],
|
||||
username=user["username"],
|
||||
)
|
||||
assert main_module.backend.book_queue.add(task) is True
|
||||
|
||||
try:
|
||||
with patch.object(main_module.ws_manager, "is_enabled", return_value=True):
|
||||
with patch.object(main_module.ws_manager.socketio, "emit") as mock_emit:
|
||||
main_module.backend.book_queue.update_status(task_id, QueueStatus.COMPLETE)
|
||||
|
||||
mock_emit.assert_any_call(
|
||||
"activity_update",
|
||||
ANY,
|
||||
to="admins",
|
||||
)
|
||||
mock_emit.assert_any_call(
|
||||
"activity_update",
|
||||
ANY,
|
||||
to=f"user_{user['id']}",
|
||||
)
|
||||
finally:
|
||||
main_module.backend.book_queue.cancel_download(task_id)
|
||||
|
||||
def test_error_transition_triggers_download_failed_notification(self, main_module):
|
||||
user = _create_user(main_module, prefix="snap-notify-error")
|
||||
task_id = f"notify-error-{uuid.uuid4().hex[:8]}"
|
||||
@@ -291,6 +322,43 @@ class TestTerminalSnapshotCapture:
|
||||
finally:
|
||||
main_module.backend.book_queue.cancel_download(task_id)
|
||||
|
||||
def test_queue_hook_emits_activity_update_when_requeue_clears_view_state(self, main_module):
|
||||
user = _create_user(main_module, prefix="snap-reset")
|
||||
task_id = f"reset-{uuid.uuid4().hex[:8]}"
|
||||
main_module.activity_view_state_service.dismiss(
|
||||
viewer_scope=f"user:{user['id']}",
|
||||
item_type="download",
|
||||
item_key=f"download:{task_id}",
|
||||
)
|
||||
|
||||
task = SimpleNamespace(
|
||||
user_id=user["id"],
|
||||
username=user["username"],
|
||||
request_id=None,
|
||||
source="direct_download",
|
||||
title="Reset Snapshot",
|
||||
author="Reset Author",
|
||||
format="epub",
|
||||
size="1 MB",
|
||||
preview=None,
|
||||
content_type="ebook",
|
||||
)
|
||||
|
||||
with patch.object(main_module.ws_manager, "is_enabled", return_value=True):
|
||||
with patch.object(main_module.ws_manager.socketio, "emit") as mock_emit:
|
||||
main_module._record_download_queued(task_id, task)
|
||||
|
||||
mock_emit.assert_any_call(
|
||||
"activity_update",
|
||||
ANY,
|
||||
to="admins",
|
||||
)
|
||||
mock_emit.assert_any_call(
|
||||
"activity_update",
|
||||
ANY,
|
||||
to=f"user_{user['id']}",
|
||||
)
|
||||
|
||||
def test_cancelled_transition_does_not_trigger_notification(self, main_module):
|
||||
user = _create_user(main_module, prefix="snap-notify-cancel")
|
||||
task_id = f"notify-cancel-{uuid.uuid4().hex[:8]}"
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
"""Tests for per-viewer activity visibility state."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def db_path():
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
yield os.path.join(tmpdir, "shelfmark.db")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def activity_view_state_service(db_path):
|
||||
from shelfmark.core.activity_view_state_service import ActivityViewStateService
|
||||
from shelfmark.core.user_db import UserDB
|
||||
|
||||
user_db = UserDB(db_path)
|
||||
user_db.initialize()
|
||||
return ActivityViewStateService(db_path)
|
||||
|
||||
|
||||
class TestActivityViewStateService:
|
||||
def test_dismiss_and_clear_history_are_viewer_scoped(self, activity_view_state_service):
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="user:1",
|
||||
item_type="download",
|
||||
item_key="download:first-task",
|
||||
)
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="user:1",
|
||||
item_type="request",
|
||||
item_key="request:12",
|
||||
)
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="admin:shared",
|
||||
item_type="download",
|
||||
item_key="download:first-task",
|
||||
)
|
||||
|
||||
user_hidden = activity_view_state_service.list_hidden(viewer_scope="user:1")
|
||||
assert {row["item_key"] for row in user_hidden} == {"download:first-task", "request:12"}
|
||||
|
||||
user_history = activity_view_state_service.list_history(viewer_scope="user:1", limit=10, offset=0)
|
||||
assert [row["item_key"] for row in user_history] == ["request:12", "download:first-task"]
|
||||
assert all(isinstance(row["dismissed_at"], str) for row in user_history)
|
||||
|
||||
cleared_count = activity_view_state_service.clear_history(viewer_scope="user:1")
|
||||
assert cleared_count == 2
|
||||
assert activity_view_state_service.list_history(viewer_scope="user:1", limit=10, offset=0) == []
|
||||
|
||||
user_hidden_after_clear = activity_view_state_service.list_hidden(viewer_scope="user:1")
|
||||
assert {row["item_key"] for row in user_hidden_after_clear} == {"download:first-task", "request:12"}
|
||||
|
||||
admin_history = activity_view_state_service.list_history(
|
||||
viewer_scope="admin:shared",
|
||||
limit=10,
|
||||
offset=0,
|
||||
)
|
||||
assert [row["item_key"] for row in admin_history] == ["download:first-task"]
|
||||
|
||||
def test_clear_item_for_all_viewers_removes_every_scope(self, activity_view_state_service):
|
||||
activity_view_state_service.dismiss_many(
|
||||
viewer_scope="user:1",
|
||||
items=[
|
||||
{"item_type": "download", "item_key": "download:shared-task"},
|
||||
{"item_type": "download", "item_key": "download:shared-task"},
|
||||
{"item_type": "request", "item_key": "request:7"},
|
||||
],
|
||||
)
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="admin:shared",
|
||||
item_type="download",
|
||||
item_key="download:shared-task",
|
||||
)
|
||||
|
||||
removed = activity_view_state_service.clear_item_for_all_viewers(
|
||||
item_type="download",
|
||||
item_key="download:shared-task",
|
||||
)
|
||||
assert removed == 2
|
||||
|
||||
user_hidden = activity_view_state_service.list_hidden(viewer_scope="user:1")
|
||||
assert [row["item_key"] for row in user_hidden] == ["request:7"]
|
||||
|
||||
admin_hidden = activity_view_state_service.list_hidden(viewer_scope="admin:shared")
|
||||
assert admin_hidden == []
|
||||
|
||||
def test_delete_viewer_scope_validates_and_removes_rows(self, activity_view_state_service):
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="user:99",
|
||||
item_type="download",
|
||||
item_key="download:task-99",
|
||||
)
|
||||
|
||||
deleted = activity_view_state_service.delete_viewer_scope(viewer_scope="user:99")
|
||||
assert deleted == 1
|
||||
assert activity_view_state_service.list_hidden(viewer_scope="user:99") == []
|
||||
|
||||
with pytest.raises(ValueError, match="positive integer"):
|
||||
activity_view_state_service.list_hidden(viewer_scope="user:0")
|
||||
|
||||
def test_list_hidden_returns_all_rows_by_default(self, activity_view_state_service):
|
||||
items = [
|
||||
{"item_type": "request", "item_key": f"request:{index}"}
|
||||
for index in range(1, 5002)
|
||||
]
|
||||
activity_view_state_service.dismiss_many(
|
||||
viewer_scope="user:1",
|
||||
items=items,
|
||||
)
|
||||
|
||||
all_hidden = activity_view_state_service.list_hidden(viewer_scope="user:1")
|
||||
limited_hidden = activity_view_state_service.list_hidden(viewer_scope="user:1", limit=10)
|
||||
|
||||
assert len(all_hidden) == 5001
|
||||
assert len(limited_hidden) == 10
|
||||
@@ -0,0 +1,53 @@
|
||||
"""Tests for queue hook failure handling."""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from shelfmark.core.models import DownloadTask
|
||||
from shelfmark.core.queue import BookQueue
|
||||
|
||||
|
||||
def _make_task(task_id: str = "task-1") -> DownloadTask:
|
||||
return DownloadTask(
|
||||
task_id=task_id,
|
||||
source="direct_download",
|
||||
title="Example Title",
|
||||
user_id=1,
|
||||
username="alice",
|
||||
)
|
||||
|
||||
|
||||
def test_add_logs_queue_hook_failures():
|
||||
queue = BookQueue()
|
||||
|
||||
def broken_hook(task_id: str, task: DownloadTask) -> None:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
queue.set_queue_hook(broken_hook)
|
||||
|
||||
with patch("shelfmark.core.queue.logger.warning") as mock_warning:
|
||||
assert queue.add(_make_task()) is True
|
||||
|
||||
mock_warning.assert_called_once()
|
||||
args = mock_warning.call_args.args
|
||||
assert args[0] == "Queue hook failed while adding task %s: %s"
|
||||
assert args[1] == "task-1"
|
||||
assert str(args[2]) == "boom"
|
||||
|
||||
|
||||
def test_enqueue_existing_logs_queue_hook_failures():
|
||||
queue = BookQueue()
|
||||
assert queue.add(_make_task("task-2")) is True
|
||||
|
||||
def broken_hook(task_id: str, task: DownloadTask) -> None:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
queue.set_queue_hook(broken_hook)
|
||||
|
||||
with patch("shelfmark.core.queue.logger.warning") as mock_warning:
|
||||
assert queue.enqueue_existing("task-2") is True
|
||||
|
||||
mock_warning.assert_called_once()
|
||||
args = mock_warning.call_args.args
|
||||
assert args[0] == "Queue hook failed while requeueing task %s: %s"
|
||||
assert args[1] == "task-2"
|
||||
assert str(args[2]) == "boom"
|
||||
@@ -0,0 +1,14 @@
|
||||
"""Tests for shared request helper normalization utilities."""
|
||||
|
||||
from shelfmark.core.request_helpers import normalize_optional_text
|
||||
|
||||
|
||||
def test_normalize_optional_text_trims_strings():
|
||||
assert normalize_optional_text(" hello ") == "hello"
|
||||
|
||||
|
||||
def test_normalize_optional_text_returns_none_for_empty_or_non_string_values():
|
||||
assert normalize_optional_text("") is None
|
||||
assert normalize_optional_text(None) is None
|
||||
assert normalize_optional_text(123) is None
|
||||
assert normalize_optional_text(False) is None
|
||||
+57
-72
@@ -70,6 +70,14 @@ class TestUserDBInitialization:
|
||||
assert cursor.fetchone() is not None
|
||||
conn.close()
|
||||
|
||||
def test_initialize_creates_activity_view_state_table(self, user_db, db_path):
|
||||
conn = sqlite3.connect(db_path)
|
||||
cursor = conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table' AND name='activity_view_state'"
|
||||
)
|
||||
assert cursor.fetchone() is not None
|
||||
conn.close()
|
||||
|
||||
def test_initialize_does_not_create_legacy_activity_tables(self, user_db, db_path):
|
||||
conn = sqlite3.connect(db_path)
|
||||
activity_log = conn.execute(
|
||||
@@ -115,7 +123,17 @@ class TestUserDBInitialization:
|
||||
).fetchall()
|
||||
index_names = {row[0] for row in rows}
|
||||
assert "idx_download_history_user_status" in index_names
|
||||
assert "idx_download_history_dismissed" in index_names
|
||||
assert "idx_download_history_recent" in index_names
|
||||
conn.close()
|
||||
|
||||
def test_initialize_creates_activity_view_state_indexes(self, user_db, db_path):
|
||||
conn = sqlite3.connect(db_path)
|
||||
rows = conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='activity_view_state'"
|
||||
).fetchall()
|
||||
index_names = {row[0] for row in rows}
|
||||
assert "idx_activity_view_state_history" in index_names
|
||||
assert "idx_activity_view_state_hidden" in index_names
|
||||
conn.close()
|
||||
|
||||
def test_initialize_enables_wal_mode(self, user_db, db_path):
|
||||
@@ -244,7 +262,7 @@ class TestUserDBInitialization:
|
||||
assert "REQUESTS_ALLOW_NOTES" not in column_names
|
||||
conn.close()
|
||||
|
||||
def test_initialize_migrates_download_requests_dismissed_at_column(self, db_path):
|
||||
def test_initialize_does_not_add_dismissed_at_to_download_requests(self, db_path):
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.executescript(
|
||||
"""
|
||||
@@ -297,7 +315,7 @@ class TestUserDBInitialization:
|
||||
conn.row_factory = sqlite3.Row
|
||||
columns = conn.execute("PRAGMA table_info(download_requests)").fetchall()
|
||||
column_names = {str(col["name"]) for col in columns}
|
||||
assert "dismissed_at" in column_names
|
||||
assert "dismissed_at" not in column_names
|
||||
conn.close()
|
||||
|
||||
def test_initialize_migrates_existing_install_without_backfill(self, db_path):
|
||||
@@ -445,13 +463,18 @@ class TestUserDBInitialization:
|
||||
|
||||
request_columns = conn.execute("PRAGMA table_info(download_requests)").fetchall()
|
||||
request_column_names = {str(col["name"]) for col in request_columns}
|
||||
assert "dismissed_at" in request_column_names
|
||||
assert "dismissed_at" not in request_column_names
|
||||
|
||||
history_table = conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table' AND name='download_history'"
|
||||
).fetchone()
|
||||
assert history_table is not None
|
||||
|
||||
view_state_table = conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table' AND name='activity_view_state'"
|
||||
).fetchone()
|
||||
assert view_state_table is not None
|
||||
|
||||
# No retroactive copy from legacy activity tables in the no-backfill plan.
|
||||
history_count = conn.execute("SELECT COUNT(*) AS count FROM download_history").fetchone()["count"]
|
||||
assert history_count == 0
|
||||
@@ -974,48 +997,14 @@ class TestDownloadRequests:
|
||||
|
||||
assert user_db.get_request(created["id"]) is None
|
||||
|
||||
def test_list_dismissed_requests_scopes_by_user(self, user_db):
|
||||
def test_delete_user_cleans_up_activity_view_state(self, user_db, db_path):
|
||||
from shelfmark.core.activity_view_state_service import ActivityViewStateService
|
||||
|
||||
activity_view_state_service = ActivityViewStateService(db_path)
|
||||
|
||||
alice = user_db.create_user(username="alice")
|
||||
bob = user_db.create_user(username="bob")
|
||||
first = user_db.create_request(
|
||||
user_id=alice["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data=self._book_data(),
|
||||
)
|
||||
second = user_db.create_request(
|
||||
user_id=bob["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data=self._book_data(),
|
||||
)
|
||||
third = user_db.create_request(
|
||||
user_id=alice["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data=self._book_data(),
|
||||
)
|
||||
|
||||
user_db.update_request(first["id"], dismissed_at="2026-01-01T10:00:00+00:00")
|
||||
user_db.update_request(second["id"], dismissed_at="2026-01-01T12:00:00+00:00")
|
||||
user_db.update_request(third["id"], dismissed_at="2026-01-01T11:00:00+00:00")
|
||||
|
||||
all_rows = user_db.list_dismissed_requests(user_id=None)
|
||||
assert [row["id"] for row in all_rows] == [second["id"], third["id"], first["id"]]
|
||||
|
||||
alice_rows = user_db.list_dismissed_requests(user_id=alice["id"])
|
||||
assert [row["id"] for row in alice_rows] == [third["id"], first["id"]]
|
||||
|
||||
bob_rows = user_db.list_dismissed_requests(user_id=bob["id"])
|
||||
assert [row["id"] for row in bob_rows] == [second["id"]]
|
||||
|
||||
def test_delete_dismissed_requests_scopes_by_user_and_only_deletes_terminal(self, user_db):
|
||||
alice = user_db.create_user(username="alice")
|
||||
bob = user_db.create_user(username="bob")
|
||||
alice_rejected = user_db.create_request(
|
||||
alice_request = user_db.create_request(
|
||||
user_id=alice["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
@@ -1023,41 +1012,37 @@ class TestDownloadRequests:
|
||||
book_data=self._book_data(),
|
||||
status="rejected",
|
||||
)
|
||||
bob_fulfilled = user_db.create_request(
|
||||
bob_request = user_db.create_request(
|
||||
user_id=bob["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data=self._book_data(),
|
||||
status="fulfilled",
|
||||
)
|
||||
alice_pending = user_db.create_request(
|
||||
user_id=alice["id"],
|
||||
content_type="ebook",
|
||||
request_level="book",
|
||||
policy_mode="request_book",
|
||||
book_data=self._book_data(),
|
||||
status="pending",
|
||||
status="rejected",
|
||||
)
|
||||
|
||||
user_db.update_request(alice_rejected["id"], dismissed_at="2026-01-01T10:00:00+00:00")
|
||||
user_db.update_request(bob_fulfilled["id"], dismissed_at="2026-01-01T11:00:00+00:00")
|
||||
user_db.update_request(alice_pending["id"], dismissed_at="2026-01-01T12:00:00+00:00")
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope=f"user:{alice['id']}",
|
||||
item_type="request",
|
||||
item_key=f"request:{alice_request['id']}",
|
||||
)
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="admin:shared",
|
||||
item_type="request",
|
||||
item_key=f"request:{alice_request['id']}",
|
||||
)
|
||||
activity_view_state_service.dismiss(
|
||||
viewer_scope="admin:shared",
|
||||
item_type="request",
|
||||
item_key=f"request:{bob_request['id']}",
|
||||
)
|
||||
|
||||
deleted_alice = user_db.delete_dismissed_requests(user_id=alice["id"])
|
||||
assert deleted_alice == 1
|
||||
assert user_db.get_request(alice_rejected["id"]) is None
|
||||
assert user_db.get_request(alice_pending["id"]) is not None
|
||||
assert user_db.get_request(bob_fulfilled["id"]) is not None
|
||||
user_db.delete_user(alice["id"])
|
||||
|
||||
deleted_all = user_db.delete_dismissed_requests(user_id=None)
|
||||
assert deleted_all == 1
|
||||
assert user_db.get_request(bob_fulfilled["id"]) is None
|
||||
assert user_db.get_request(alice_pending["id"]) is not None
|
||||
conn = sqlite3.connect(db_path)
|
||||
rows = conn.execute(
|
||||
"SELECT viewer_scope, item_key FROM activity_view_state ORDER BY viewer_scope, item_key"
|
||||
).fetchall()
|
||||
conn.close()
|
||||
|
||||
def test_request_dismissal_helpers_validate_user_scope(self, user_db):
|
||||
with pytest.raises(ValueError, match="user_id must be a positive integer"):
|
||||
user_db.list_dismissed_requests(user_id=0)
|
||||
|
||||
with pytest.raises(ValueError, match="user_id must be a positive integer"):
|
||||
user_db.delete_dismissed_requests(user_id=0)
|
||||
assert rows == [("admin:shared", f"request:{bob_request['id']}")]
|
||||
|
||||
Reference in New Issue
Block a user