Patch: Request retry, admin-level requests, and various fixes (#620)

- Add request retry in the case of a download failure, admins will be prompted to attach a new file to the request
- Add admin-level "add to requests" button in the release modal
This commit is contained in:
Alex
2026-02-16 12:27:56 +00:00
committed by GitHub
parent 1931eb96a5
commit ccb39e674e
20 changed files with 550 additions and 71 deletions
+1
View File
@@ -97,6 +97,7 @@ class DownloadTask:
# User association (multi-user support)
user_id: Optional[int] = None # DB user ID who queued this download
username: Optional[str] = None # Username for {User} template variable
request_id: Optional[int] = None # Origin request ID when queued from request fulfilment
# Runtime state
priority: int = 0
+61 -1
View File
@@ -455,8 +455,11 @@ def fulfil_request(
if requester is None:
raise RequestServiceError("Requesting user not found", status_code=404)
queued_release_data = dict(selected_release_data)
queued_release_data["_request_id"] = request_id
success, error = queue_release(
selected_release_data,
queued_release_data,
0,
user_id=request_row["user_id"],
username=requester.get("username"),
@@ -476,9 +479,66 @@ def fulfil_request(
release_data=selected_release_data,
delivery_state="queued",
delivery_updated_at=_now_timestamp(),
last_failure_reason=None,
admin_note=normalized_admin_note,
reviewed_by=admin_user_id,
reviewed_at=_now_timestamp(),
)
except ValueError as exc:
raise RequestServiceError(str(exc), status_code=409, code="stale_transition") from exc
def reopen_failed_request(
user_db: "UserDB",
*,
request_id: int,
failure_reason: str | None = None,
) -> dict[str, Any] | None:
"""Reopen a failed fulfilled request so admins can re-approve with a new release."""
normalized_failure_reason = None
if isinstance(failure_reason, str):
normalized_failure_reason = failure_reason.strip() or None
with user_db._lock:
conn = user_db._connect()
try:
current_row = conn.execute(
"SELECT * FROM download_requests WHERE id = ?",
(request_id,),
).fetchone()
current_request = user_db._parse_request_row(current_row)
if current_request is None:
return None
if current_request.get("status") != "fulfilled":
return None
current_delivery_state = _existing_delivery_state(current_request)
# Terminal hook callbacks can run before delivery-state sync persists "error".
# Allow reopening fulfilled requests unless they are already complete.
if current_delivery_state == "complete":
return None
if current_delivery_state not in {"error", "cancelled"} and normalized_failure_reason is None:
return None
conn.execute(
"""
UPDATE download_requests
SET status = 'pending',
delivery_state = 'none',
delivery_updated_at = NULL,
release_data = NULL,
last_failure_reason = ?,
reviewed_by = NULL,
reviewed_at = NULL
WHERE id = ?
""",
(normalized_failure_reason, request_id),
)
updated_row = conn.execute(
"SELECT * FROM download_requests WHERE id = ?",
(request_id,),
).fetchone()
conn.commit()
return user_db._parse_request_row(updated_row)
finally:
conn.close()
+3
View File
@@ -200,6 +200,8 @@ class UserDB:
)
if "delivery_updated_at" not in column_names:
conn.execute("ALTER TABLE download_requests ADD COLUMN delivery_updated_at TIMESTAMP")
if "last_failure_reason" not in column_names:
conn.execute("ALTER TABLE download_requests ADD COLUMN last_failure_reason TEXT")
conn.execute(
"""
@@ -614,6 +616,7 @@ class UserDB:
"reviewed_at",
"delivery_state",
"delivery_updated_at",
"last_failure_reason",
}
def update_request(
+6
View File
@@ -178,6 +178,10 @@ def queue_release(
try:
source = release_data.get('source', 'direct_download')
extra = release_data.get('extra', {})
raw_request_id = release_data.get('_request_id')
request_id: Optional[int] = None
if isinstance(raw_request_id, int) and raw_request_id > 0:
request_id = raw_request_id
# Get author, year, preview, and content_type from top-level (preferred) or extra (fallback)
author = release_data.get('author') or extra.get('author')
@@ -225,6 +229,7 @@ def queue_release(
priority=priority,
user_id=user_id,
username=username,
request_id=request_id,
)
if not book_queue.add(task):
@@ -327,6 +332,7 @@ def _task_to_dict(task: DownloadTask) -> Dict[str, Any]:
'download_path': task.download_path,
'user_id': task.user_id,
'username': task.username,
'request_id': task.request_id,
}
+36 -12
View File
@@ -46,6 +46,7 @@ from shelfmark.core.request_policy import (
resolve_policy_mode,
)
from shelfmark.core.requests_service import (
reopen_failed_request,
sync_delivery_states_from_queue_status,
)
from shelfmark.core.activity_service import ActivityService, build_download_item_key
@@ -1080,9 +1081,6 @@ def _notify_admin_for_terminal_download_status(*, task_id: str, status: QueueSta
def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task: Any) -> None:
_notify_admin_for_terminal_download_status(task_id=task_id, status=status, task=task)
if activity_service is None:
return
final_status = _queue_status_to_final_activity_status(status)
if final_status is None:
return
@@ -1129,19 +1127,45 @@ def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task:
if linked_request is not None:
snapshot["request"] = linked_request
if activity_service is not None:
try:
activity_service.record_terminal_snapshot(
user_id=owner_user_id,
item_type="download",
item_key=build_download_item_key(task_id),
origin=origin,
final_status=final_status,
snapshot=snapshot,
request_id=request_id,
source_id=task_id,
)
except Exception as exc:
logger.warning("Failed to record terminal download snapshot for task %s: %s", task_id, exc)
if user_db is None or linked_request is None or request_id is None or status != QueueStatus.ERROR:
return
raw_error_message = getattr(task, "status_message", None)
fallback_reason = (
raw_error_message.strip()
if isinstance(raw_error_message, str) and raw_error_message.strip()
else "Download failed"
)
try:
activity_service.record_terminal_snapshot(
user_id=owner_user_id,
item_type="download",
item_key=build_download_item_key(task_id),
origin=origin,
final_status=final_status,
snapshot=snapshot,
reopened_request = reopen_failed_request(
user_db,
request_id=request_id,
source_id=task_id,
failure_reason=fallback_reason,
)
if reopened_request is not None:
_emit_request_update_events([reopened_request])
except Exception as exc:
logger.warning("Failed to record terminal download snapshot for task %s: %s", task_id, exc)
logger.warning(
"Failed to reopen request %s after terminal download error %s: %s",
request_id,
task_id,
exc,
)
def _task_owned_by_actor(task: Any, *, actor_user_id: int | None, actor_username: str | None) -> bool:
+81 -3
View File
@@ -226,6 +226,43 @@ function App() {
socket,
});
const dismissedDownloadTaskIds = useMemo(() => {
const result = new Set<string>();
for (const key of dismissedActivityKeys) {
if (typeof key !== 'string' || !key.startsWith('download:')) {
continue;
}
const taskId = key.substring('download:'.length).trim();
if (taskId) {
result.add(taskId);
}
}
return result;
}, [dismissedActivityKeys]);
const isDownloadTaskDismissed = useCallback((taskId: string) => {
return dismissedDownloadTaskIds.has(taskId);
}, [dismissedDownloadTaskIds]);
const statusForButtonState = useMemo(() => {
if (!currentStatus.complete || dismissedDownloadTaskIds.size === 0) {
return currentStatus;
}
const filteredComplete = Object.fromEntries(
Object.entries(currentStatus.complete).filter(([taskId]) => !dismissedDownloadTaskIds.has(taskId))
) as Record<string, Book>;
if (Object.keys(filteredComplete).length === Object.keys(currentStatus.complete).length) {
return currentStatus;
}
return {
...currentStatus,
complete: filteredComplete,
};
}, [currentStatus, dismissedDownloadTaskIds]);
const showRequestsTab = useMemo(() => {
if (requestRoleIsAdmin) {
return true;
@@ -947,6 +984,23 @@ function App() {
[openRequestConfirmation, refreshRequestPolicy]
);
const handleReleaseBookRequest = useCallback(
async (book: Book, modalContentType: ContentType): Promise<void> => {
void refreshRequestPolicy();
const normalizedContentType = toContentType(modalContentType);
openRequestConfirmation({
book_data: buildMetadataBookRequestData(book, normalizedContentType),
release_data: null,
context: {
source: '*',
content_type: normalizedContentType,
request_level: 'book',
},
});
},
[openRequestConfirmation, refreshRequestPolicy]
);
const handleReleaseModalPolicyRefresh = useCallback(() => {
return refreshRequestPolicy({ force: true });
}, [refreshRequestPolicy]);
@@ -1044,19 +1098,38 @@ function App() {
const getDirectActionButtonState = useCallback(
(bookId: string): ButtonStateInfo => {
const baseState = getButtonState(bookId);
if (baseState.state === 'complete' && isDownloadTaskDismissed(bookId)) {
return applyDirectPolicyModeToButtonState(
{ text: 'Download', state: 'download' },
getDirectPolicyMode()
);
}
const mode = getDirectPolicyMode();
return applyDirectPolicyModeToButtonState(baseState, mode);
},
[getButtonState, getDirectPolicyMode]
[getButtonState, getDirectPolicyMode, isDownloadTaskDismissed]
);
const getUniversalActionButtonState = useCallback(
(bookId: string): ButtonStateInfo => {
const baseState = getUniversalButtonState(bookId);
const trackedReleaseIds = bookToReleaseMap[bookId] || [];
const allTrackedReleasesDismissed = trackedReleaseIds.length > 0 &&
trackedReleaseIds.every((releaseId) => isDownloadTaskDismissed(releaseId));
if (
baseState.state === 'complete' &&
(isDownloadTaskDismissed(bookId) || allTrackedReleasesDismissed)
) {
return applyUniversalPolicyModeToButtonState(
{ text: 'Get', state: 'download' },
getUniversalDefaultPolicyMode()
);
}
const mode = getUniversalDefaultPolicyMode();
return applyUniversalPolicyModeToButtonState(baseState, mode);
},
[getUniversalButtonState, getUniversalDefaultPolicyMode]
[bookToReleaseMap, getUniversalButtonState, getUniversalDefaultPolicyMode, isDownloadTaskDismissed]
);
const bookLanguages = config?.book_languages || DEFAULT_LANGUAGES;
@@ -1263,6 +1336,11 @@ function App() {
onClose={handleReleaseModalClose}
onDownload={isBrowseFulfilMode ? handleBrowseFulfilDownload : handleReleaseDownload}
onRequestRelease={isBrowseFulfilMode ? undefined : handleReleaseRequest}
onRequestBook={
isBrowseFulfilMode || !requestRoleIsAdmin
? undefined
: handleReleaseBookRequest
}
getPolicyModeForSource={isBrowseFulfilMode ? () => 'download' : (source, ct) => getSourceMode(source, ct)}
onPolicyRefresh={handleReleaseModalPolicyRefresh}
supportedFormats={supportedFormats}
@@ -1270,7 +1348,7 @@ function App() {
contentType={activeReleaseContentType}
defaultLanguages={defaultLanguageCodes}
bookLanguages={bookLanguages}
currentStatus={currentStatus}
currentStatus={statusForButtonState}
defaultReleaseSource={config?.default_release_source}
onSearchSeries={isBrowseFulfilMode ? undefined : handleSearchSeries}
/>
@@ -194,6 +194,7 @@ interface ReleaseModalProps {
onClose: () => void;
onDownload: (book: Book, release: Release, contentType: ContentType) => Promise<void>;
onRequestRelease?: (book: Book, release: Release, contentType: ContentType) => Promise<void>;
onRequestBook?: (book: Book, contentType: ContentType) => Promise<void>;
getPolicyModeForSource?: (source: string, contentType: ContentType) => RequestPolicyMode;
onPolicyRefresh?: () => Promise<unknown>;
supportedFormats: string[];
@@ -622,6 +623,7 @@ export const ReleaseModal = ({
onClose,
onDownload,
onRequestRelease,
onRequestBook,
getPolicyModeForSource,
onPolicyRefresh,
supportedFormats,
@@ -638,6 +640,7 @@ export const ReleaseModal = ({
? supportedAudiobookFormats
: supportedFormats;
const [isClosing, setIsClosing] = useState(false);
const [isRequestingBook, setIsRequestingBook] = useState(false);
// Available sources from plugin registry
const [availableSources, setAvailableSources] = useState<ReleaseSource[]>([]);
@@ -699,6 +702,19 @@ export const ReleaseModal = ({
}, 150);
}, [onClose]);
const handleRequestBook = useCallback(async (): Promise<void> => {
if (!book || !onRequestBook || isRequestingBook) {
return;
}
setIsRequestingBook(true);
try {
await onRequestBook(book, contentType);
handleClose();
} finally {
setIsRequestingBook(false);
}
}, [book, onRequestBook, isRequestingBook, contentType, handleClose]);
// Handle ESC key
useEffect(() => {
const handleEscape = (e: KeyboardEvent) => {
@@ -1511,6 +1527,21 @@ export const ReleaseModal = ({
</svg>
</a>
)}
{onRequestBook && (
<button
type="button"
onClick={() => {
void handleRequestBook();
}}
disabled={isRequestingBook}
className="inline-flex items-center gap-1 px-2 py-1 text-xs font-medium text-emerald-600 dark:text-emerald-400 bg-emerald-50 dark:bg-emerald-900/20 rounded-full hover:bg-emerald-100 dark:hover:bg-emerald-900/40 transition-colors disabled:cursor-not-allowed disabled:opacity-60"
>
<svg className="w-3 h-3" fill="none" stroke="currentColor" viewBox="0 0 24 24" strokeWidth={2}>
<path strokeLinecap="round" strokeLinejoin="round" d="M12 4.5v15m7.5-7.5h-15" />
</svg>
{isRequestingBook ? 'Adding...' : 'Add to requests'}
</button>
)}
</div>
</div>
</div>
@@ -86,7 +86,7 @@ const actionKey = (action: ActivityCardAction): string => {
const actionUiConfig = (
action: ActivityCardAction
): { title: string; className: string; icon: 'cross' | 'check' | 'stop' } => {
): { title: string; className: string; icon: 'cross' | 'check' | 'stop' | 'retry' } => {
switch (action.kind) {
case 'download-remove':
return {
@@ -139,7 +139,7 @@ const actionUiConfig = (
}
};
const ActionIcon = ({ icon }: { icon: 'cross' | 'check' | 'stop' }) => {
const ActionIcon = ({ icon }: { icon: 'cross' | 'check' | 'stop' | 'retry' }) => {
if (icon === 'stop') {
return (
<svg className="w-4 h-4" viewBox="0 0 24 24" fill="currentColor" aria-hidden="true">
@@ -154,6 +154,13 @@ const ActionIcon = ({ icon }: { icon: 'cross' | 'check' | 'stop' }) => {
</svg>
);
}
if (icon === 'retry') {
return (
<svg className="w-4 h-4" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="1.8" aria-hidden="true">
<path strokeLinecap="round" strokeLinejoin="round" d="M18.363 5.634A8.997 9.002 29.494 0 0 7.5 4.206 8.997 9.002 29.494 0 0 3.306 14.33 8.997 9.002 29.494 0 0 11.996 21a8.997 9.002 29.494 0 0 8.694-6.673m-2.327-8.693L20.87 8.14m.017-4.994v5.015m0 0h-5.013" />
</svg>
);
}
return (
<svg className="w-4 h-4" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2.25" aria-hidden="true">
<path strokeLinecap="round" strokeLinejoin="round" d="M6 18L18 6M6 6l12 12" />
@@ -317,7 +324,7 @@ export const ActivityCard = ({
}
return () => observer.disconnect();
}, [item.title, item.author]);
}, [item.title, item.author, isRequestDetailsOpen, isRequestRejectOpen]);
const reviewRecord = item.requestRecord;
const reviewApproveHandler = onRequestReviewApprove || onRequestApprove;
@@ -416,10 +423,13 @@ export const ActivityCard = ({
const requiresBrowseBeforeApprove =
reviewRecord?.request_level === 'book' || !hasAttachedRelease;
const showSourceField = reviewRecord?.request_level === 'release';
const isRetryAfterFailure = Boolean(toOptionalText(reviewRecord?.last_failure_reason));
const approveLabel =
requiresBrowseBeforeApprove
? 'Browse Releases To Approve'
? isRetryAfterFailure
? 'Browse Releases To Retry'
: 'Browse Releases To Approve'
: 'Approve Attached File';
const provider = toOptionalText(bookData.provider)?.toLowerCase();
@@ -509,7 +519,7 @@ export const ActivityCard = ({
{isSelected && (
<span
aria-hidden="true"
className="absolute left-0 top-2 bottom-2 w-1 bg-sky-500/80"
className="absolute left-0 top-2 bottom-2 w-1 bg-gray-400/80 dark:bg-gray-500/80"
/>
)}
<div className="flex gap-3 items-start">
@@ -534,6 +544,8 @@ export const ActivityCard = ({
content={!isDetailsExpanded && titleOverflow ? titleAuthorLine : undefined}
delay={0}
position="bottom"
triggerClassName="block max-w-full"
alwaysWrap
>
<p ref={titleLineRef} className={titleLineClassName}>
<span className="font-semibold">{titleNode}</span>
@@ -544,19 +556,27 @@ export const ActivityCard = ({
<div className="flex-shrink-0 inline-flex items-center gap-1 -my-1">
{actions.map((action) => {
const config = actionUiConfig(action);
const icon =
action.kind === 'request-approve' && isRetryAfterFailure
? 'retry'
: config.icon;
const actionTitle =
action.kind === 'request-approve' && isRetryAfterFailure
? 'Retry'
: config.title;
return (
<Tooltip
key={actionKey(action)}
content={config.title}
content={actionTitle}
delay={0}
position="bottom"
>
<IconButton
title={config.title}
title={actionTitle}
className={config.className}
onClick={() => runAction(action)}
>
<ActionIcon icon={config.icon} />
<ActionIcon icon={icon} />
</IconButton>
</Tooltip>
);
@@ -564,7 +584,7 @@ export const ActivityCard = ({
{showRequestDetailsToggle && onRequestDetailsToggle && (
<IconButton
title={isDetailsExpanded ? 'Hide details' : 'Show details'}
className="text-gray-500 hover:bg-gray-100 dark:hover:bg-gray-700"
className="text-gray-500 hover-action"
onClick={onRequestDetailsToggle}
>
<svg
@@ -666,8 +686,12 @@ export const ActivityCard = ({
) : (
<p className="text-xs opacity-70">
{reviewRecord?.request_level === 'book'
? 'This is a book-level request without an attached file. Choose a release before approval.'
: 'No attached release data is available. Choose a release before approval.'}
? isRetryAfterFailure
? 'Previous download failed. Choose a release before re-approving.'
: 'This is a book-level request without an attached file. Choose a release before approval.'
: isRetryAfterFailure
? 'Previous download failed and the attached release was cleared. Choose a release before re-approving.'
: 'No attached release data is available. Choose a release before approval.'}
</p>
)}
@@ -367,6 +367,23 @@ export const ActivitySidebar = ({
});
const mergedByDownloadId = new Map<string, ActivityItem>();
const reopenedRequestIds = new Set<number>();
visibleRequestItems.forEach((item) => {
if (item.kind !== 'request' || typeof item.requestId !== 'number') {
return;
}
const requestRecord = item.requestRecord;
const failureReason = requestRecord?.last_failure_reason;
if (
requestRecord?.status === 'pending' &&
typeof failureReason === 'string' &&
failureReason.trim().length > 0
) {
reopenedRequestIds.add(item.requestId);
}
});
const nextRequestItems = visibleRequestItems.map((requestItem) => {
const linkedDownloadId = getLinkedDownloadIdFromRequestItem(requestItem);
if (!linkedDownloadId) {
@@ -391,6 +408,15 @@ export const ActivitySidebar = ({
return downloadItem;
}
return mergedByDownloadId.get(downloadId) || downloadItem;
}).filter((downloadItem) => {
if (
typeof downloadItem.requestId === 'number' &&
reopenedRequestIds.has(downloadItem.requestId) &&
(downloadItem.visualStatus === 'error' || downloadItem.visualStatus === 'cancelled')
) {
return false;
}
return true;
});
return {
@@ -63,16 +63,23 @@ const getRequestBadge = (item: ActivityItem, isAdmin: boolean): ActivityCardBadg
const requestVisualStatus = item.requestRecord
? toRequestVisualStatus(item.requestRecord.status)
: item.visualStatus;
const failureReason = item.requestRecord?.last_failure_reason?.trim() || null;
const hasFailureReason = requestVisualStatus === 'pending' && Boolean(failureReason);
const hasInFlightLinkedDownload = (
item.kind === 'download' &&
requestVisualStatus === 'fulfilled' &&
isActiveDownloadStatus(item.visualStatus)
);
const visualStatus = hasInFlightLinkedDownload ? 'resolving' : requestVisualStatus;
let visualStatus: ActivityVisualStatus = hasInFlightLinkedDownload ? 'resolving' : requestVisualStatus;
if (hasFailureReason) {
visualStatus = 'error';
}
let text = item.statusLabel;
if (hasInFlightLinkedDownload) {
text = 'Approved';
} else if (hasFailureReason) {
text = failureReason as string;
} else if (requestVisualStatus === 'pending') {
text = getPendingRequestText(item, isAdmin);
} else if (requestVisualStatus === 'fulfilled') {
@@ -66,6 +66,13 @@ const getDownloadProgress = (status: ActivityVisualStatus, bookProgress: unknown
export const downloadToActivityItem = (book: Book, statusKey: DownloadStatusKey): ActivityItem => {
const visualStatus = statusKeyToVisualStatus(statusKey);
const requestId = (
typeof book.request_id === 'number' &&
Number.isFinite(book.request_id) &&
book.request_id > 0
)
? Math.trunc(book.request_id)
: undefined;
const metaLine = joinMetaParts([
toOptionalText(book.format)?.toUpperCase(),
toOptionalText(book.size),
@@ -92,6 +99,7 @@ export const downloadToActivityItem = (book: Book, statusKey: DownloadStatusKey)
downloadBookId: book.id,
downloadPath: toOptionalText(book.download_path),
sizeRaw: toOptionalText(book.size),
requestId,
};
};
@@ -52,6 +52,7 @@ interface SettingsContentProps {
customFieldContext?: {
authMode?: string;
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
onRefreshOverrideSummary?: () => void;
};
}
@@ -405,6 +406,7 @@ export const SettingsContent = ({
disabledReason: disabledState.reason,
authMode: customFieldContext?.authMode,
onShowToast: customFieldContext?.onShowToast,
onRefreshOverrideSummary: customFieldContext?.onRefreshOverrideSummary,
})
: renderField(
field,
@@ -44,6 +44,7 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
// Track previous isOpen state to detect modal open transition
const prevIsOpenRef = useRef(false);
const overrideSummaryRequestIdRef = useRef(0);
// Check for mobile viewport
useEffect(() => {
@@ -129,32 +130,34 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
};
}, [isOpen, selectedTab]);
const refreshOverrideSummaryForTab = useCallback(async (tabName: string) => {
const requestId = ++overrideSummaryRequestIdRef.current;
try {
const data = await getAdminSettingsOverridesSummary(tabName);
if (overrideSummaryRequestIdRef.current !== requestId) {
return;
}
setTabOverrideSummaries((prev) => ({
...prev,
[tabName]: data.keys || {},
}));
} catch {
if (overrideSummaryRequestIdRef.current !== requestId) {
return;
}
setTabOverrideSummaries((prev) => ({
...prev,
[tabName]: {},
}));
}
}, []);
useEffect(() => {
if (!isOpen || !selectedTab) {
return;
}
let cancelled = false;
getAdminSettingsOverridesSummary(selectedTab)
.then((data) => {
if (cancelled) return;
setTabOverrideSummaries((prev) => ({
...prev,
[selectedTab]: data.keys || {},
}));
})
.catch(() => {
if (cancelled) return;
setTabOverrideSummaries((prev) => ({
...prev,
[selectedTab]: {},
}));
});
return () => {
cancelled = true;
};
}, [isOpen, selectedTab]);
void refreshOverrideSummaryForTab(selectedTab);
}, [isOpen, selectedTab, refreshOverrideSummaryForTab]);
// Reset to first tab when modal transitions from closed to open
useEffect(() => {
@@ -181,10 +184,18 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
setShowMobileDetail(false);
}, []);
const handleRefreshCurrentTabOverrideSummary = useCallback(() => {
if (!selectedTab) {
return;
}
void refreshOverrideSummaryForTab(selectedTab);
}, [selectedTab, refreshOverrideSummaryForTab]);
const handleSave = useCallback(async () => {
if (!selectedTab) return;
const result = await saveTab(selectedTab);
if (result.success) {
void refreshOverrideSummaryForTab(selectedTab);
onShowToast?.(result.message, 'success');
// Notify parent that settings were saved so it can refresh config
onSettingsSaved?.();
@@ -197,7 +208,7 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
} else {
onShowToast?.(result.message, 'error');
}
}, [selectedTab, saveTab, onShowToast, onSettingsSaved]);
}, [selectedTab, saveTab, onShowToast, onSettingsSaved, refreshOverrideSummaryForTab]);
const handleAction = useCallback(
async (actionKey: string) => {
@@ -212,9 +223,13 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
}
return { success: true, message: 'Opening Users tab...' };
}
return executeAction(selectedTab, actionKey);
const result = await executeAction(selectedTab, actionKey);
if (result.success) {
void refreshOverrideSummaryForTab(selectedTab);
}
return result;
},
[selectedTab, executeAction, isMobile, setSelectedTab]
[selectedTab, executeAction, isMobile, setSelectedTab, refreshOverrideSummaryForTab]
);
// Memoize the field change handler to prevent creating new functions on every render
@@ -285,6 +300,7 @@ export const SettingsModal = ({ isOpen, authMode, onClose, onShowToast, onSettin
customFieldContext={{
authMode: usersAuthMode,
onShowToast,
onRefreshOverrideSummary: handleRefreshCurrentTabOverrideSummary,
}}
/>
))
@@ -20,8 +20,10 @@ export const UsersManagementField = ({
onUiStateChange,
authMode,
onShowToast,
onRefreshOverrideSummary,
}: CustomSettingsFieldRendererProps) => {
const { route, openCreate, openEdit, openEditOverrides, backToList } = useUsersPanelState();
const activeEditRequestIdRef = useRef(0);
const {
users,
@@ -80,12 +82,30 @@ export const UsersManagementField = ({
onEditSaveSuccess: clearEditState,
});
const invalidateEditContextRequest = useCallback(() => {
activeEditRequestIdRef.current += 1;
}, []);
useEffect(() => {
return () => {
invalidateEditContextRequest();
};
}, [invalidateEditContextRequest]);
const startEditing = async (user: AdminUser) => {
const requestId = activeEditRequestIdRef.current + 1;
activeEditRequestIdRef.current = requestId;
beginEditing(user);
try {
const context = await fetchUserEditContext(user.id);
if (activeEditRequestIdRef.current !== requestId) {
return;
}
applyUserEditContext(context);
} catch {
if (activeEditRequestIdRef.current !== requestId) {
return;
}
resetEditContext();
}
};
@@ -94,6 +114,7 @@ export const UsersManagementField = ({
const handleBackToList = () => {
onUiStateChange('routeKind', 'list');
invalidateEditContextRequest();
clearEditState();
backToList();
};
@@ -107,6 +128,7 @@ export const UsersManagementField = ({
const handleCreate = async () => {
const ok = await createUser();
if (ok) {
onRefreshOverrideSummary?.();
backToList();
}
};
@@ -125,7 +147,10 @@ export const UsersManagementField = ({
};
const handleSyncCwa = async () => {
await syncCwaUsers();
const ok = await syncCwaUsers();
if (ok) {
onRefreshOverrideSummary?.();
}
};
const handleBackToEdit = () => {
@@ -151,9 +176,10 @@ export const UsersManagementField = ({
const handleSaveUserEdit = useCallback(async () => {
const ok = await saveEditedUser({ includeSettings: false });
if (ok) {
onRefreshOverrideSummary?.();
backToList();
}
}, [backToList, saveEditedUser]);
}, [backToList, onRefreshOverrideSummary, saveEditedUser]);
const handleSaveUserOverrides = useCallback(async () => {
const ok = await saveEditedUser({
@@ -162,9 +188,10 @@ export const UsersManagementField = ({
includeSettings: true,
});
if (ok) {
onRefreshOverrideSummary?.();
backToList();
}
}, [backToList, saveEditedUser]);
}, [backToList, onRefreshOverrideSummary, saveEditedUser]);
const handleSaveUserOverridesRef = useRef(handleSaveUserOverrides);
useEffect(() => {
@@ -182,6 +209,14 @@ export const UsersManagementField = ({
return testAdminUserNotificationPreferences(editingUser.id, routes);
}, [editingUser]);
const handleDeleteUser = useCallback(async (userId: number) => {
const ok = await deleteUser(userId);
if (ok) {
onRefreshOverrideSummary?.();
}
return ok;
}, [deleteUser, onRefreshOverrideSummary]);
useEffect(() => {
if (route.kind !== 'edit-overrides') {
onUiStateChange('hasChanges', false);
@@ -227,7 +262,7 @@ export const UsersManagementField = ({
users={users}
loadingUsers={loading}
loadError={loadError}
onRetryLoadUsers={() => void fetchUsers()}
onRetryLoadUsers={() => void fetchUsers({ force: true })}
onCreate={openCreate}
showCreateForm={route.kind === 'create'}
createForm={createForm}
@@ -250,7 +285,7 @@ export const UsersManagementField = ({
downloadDefaults={downloadDefaults}
onOpenOverrides={handleOpenOverrides}
onEdit={handleEdit}
onDelete={deleteUser}
onDelete={handleDeleteUser}
deletingUserId={deletingUserId}
onSyncCwa={handleSyncCwa}
syncingCwa={syncingCwa}
@@ -12,6 +12,7 @@ export interface CustomSettingsFieldRendererProps {
disabledReason?: string;
authMode?: string;
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
onRefreshOverrideSummary?: () => void;
}
export interface CustomSettingsFieldLayout {
@@ -13,7 +13,7 @@ import { buildUserSettingsPayload } from './settingsPayload';
const MIN_PASSWORD_LENGTH = 4;
interface UseUserMutationsParams {
onShowToast?: (message: string, type: 'success' | 'error' | 'info') => void;
fetchUsers: () => Promise<AdminUser[]>;
fetchUsers: (options?: { force?: boolean }) => Promise<AdminUser[]>;
users: AdminUser[];
createForm: CreateUserFormState;
resetCreateForm: () => void;
@@ -86,7 +86,7 @@ export const useUserMutations = ({
});
resetCreateForm();
onShowToast?.(`Local user ${created.username} created`, 'success');
await fetchUsers();
await fetchUsers({ force: true });
return true;
} catch (err) {
return fail(err instanceof Error ? err.message : 'Failed to create user');
@@ -144,7 +144,7 @@ export const useUserMutations = ({
includeSettings && !includeProfile && !includePassword ? 'User preferences updated' : 'User updated',
'success',
);
const refreshedUsers = await fetchUsers();
const refreshedUsers = await fetchUsers({ force: true });
if (includeProfile && localAdminsBeforeSave > 0 && countLocalPasswordAdmins(refreshedUsers) === 0) {
onShowToast?.(
"No local admin accounts remain. Authentication will fall back to 'No Authentication' until a local admin is created.",
@@ -167,7 +167,7 @@ export const useUserMutations = ({
try {
await deleteAdminUser(userId);
onShowToast?.('User deleted', 'success');
const refreshedUsers = await fetchUsers();
const refreshedUsers = await fetchUsers({ force: true });
if (deletedUser && deletedUser.auth_source !== 'builtin') {
onShowToast?.(
`${authSourceLabel[deletedUser.auth_source]} users may be re-provisioned by your authentication source on a future login or sync.`,
@@ -194,7 +194,7 @@ export const useUserMutations = ({
try {
const result = await syncAdminCwaUsers();
onShowToast?.(result.message || 'Users synced from CWA', 'success');
await fetchUsers();
await fetchUsers({ force: true });
return true;
} catch (err) {
return fail(err instanceof Error ? err.message : 'Failed to sync users from CWA');
@@ -27,8 +27,12 @@ const shouldSuppressAccessToast = (message: string): boolean =>
const toLoadErrorMessage = (err: unknown): string =>
err instanceof Error ? err.message : 'Failed to load users';
const loadUsersIntoCache = async (): Promise<AdminUser[]> => {
if (cachedUsers !== null) {
interface LoadUsersOptions {
force?: boolean;
}
const loadUsersIntoCache = async ({ force = false }: LoadUsersOptions = {}): Promise<AdminUser[]> => {
if (!force && cachedUsers !== null) {
return cachedUsers;
}
if (usersCacheLoadPromise) {
@@ -98,14 +102,14 @@ export const useUsersFetch = ({ onShowToast }: UseUsersFetchParams) => {
const [loading, setLoading] = useState<boolean>(() => cachedUsers === null);
const [loadError, setLoadError] = useState<string | null>(() => cachedLoadError);
const fetchUsers = useCallback(async (): Promise<AdminUser[]> => {
const hasCachedResult = cachedUsers !== null;
const fetchUsers = useCallback(async ({ force = false }: LoadUsersOptions = {}): Promise<AdminUser[]> => {
const hasCachedResult = !force && cachedUsers !== null;
try {
if (!hasCachedResult) {
setLoading(true);
}
setLoadError(null);
const data = await loadUsersIntoCache();
const data = await loadUsersIntoCache({ force });
setUsers(data);
return data;
} catch (err) {
+64 -3
View File
@@ -8,6 +8,8 @@ interface TooltipProps {
delay?: number;
className?: string;
unstyled?: boolean;
triggerClassName?: string;
alwaysWrap?: boolean;
}
export function Tooltip({
@@ -17,15 +19,22 @@ export function Tooltip({
delay = 200,
className = '',
unstyled = false,
triggerClassName = 'inline-flex max-w-full',
alwaysWrap = false,
}: TooltipProps) {
const [isVisible, setIsVisible] = useState(false);
const [coords, setCoords] = useState<{ top: number; left: number } | null>(null);
const triggerRef = useRef<HTMLDivElement>(null);
const tooltipRef = useRef<HTMLDivElement>(null);
const timeoutRef = useRef<NodeJS.Timeout | null>(null);
const hasContent = Boolean(content);
const isPlainTextContent = typeof content === 'string' || typeof content === 'number';
const spacing = 6;
const showTooltip = () => {
if (!hasContent) {
return;
}
if (timeoutRef.current) clearTimeout(timeoutRef.current);
timeoutRef.current = setTimeout(() => {
if (triggerRef.current) {
@@ -67,13 +76,64 @@ export function Tooltip({
setCoords(null);
};
useEffect(() => {
if (!isVisible || !coords || !tooltipRef.current) {
return;
}
const tooltipRect = tooltipRef.current.getBoundingClientRect();
const viewportPadding = 6;
let deltaX = 0;
let deltaY = 0;
if (tooltipRect.width + viewportPadding * 2 > window.innerWidth) {
deltaX = viewportPadding - tooltipRect.left;
} else {
if (tooltipRect.left < viewportPadding) {
deltaX = viewportPadding - tooltipRect.left;
} else if (tooltipRect.right > window.innerWidth - viewportPadding) {
deltaX = (window.innerWidth - viewportPadding) - tooltipRect.right;
}
}
if (tooltipRect.height + viewportPadding * 2 > window.innerHeight) {
deltaY = viewportPadding - tooltipRect.top;
} else {
if (tooltipRect.top < viewportPadding) {
deltaY = viewportPadding - tooltipRect.top;
} else if (tooltipRect.bottom > window.innerHeight - viewportPadding) {
deltaY = (window.innerHeight - viewportPadding) - tooltipRect.bottom;
}
}
if (deltaX !== 0 || deltaY !== 0) {
setCoords((current) => {
if (!current) {
return current;
}
return {
top: current.top + deltaY,
left: current.left + deltaX,
};
});
}
}, [coords, isVisible]);
useEffect(() => {
if (!hasContent) {
setIsVisible(false);
setCoords(null);
}
}, [hasContent]);
useEffect(() => {
return () => {
if (timeoutRef.current) clearTimeout(timeoutRef.current);
};
}, []);
if (!content) {
if (!hasContent && !alwaysWrap) {
return <>{children}</>;
}
@@ -96,12 +156,13 @@ export function Tooltip({
onMouseLeave={hideTooltip}
onFocusCapture={showTooltip}
onBlurCapture={hideTooltip}
className="inline-flex max-w-full"
className={triggerClassName}
>
{children}
</div>
{isVisible && coords && createPortal(
{hasContent && isVisible && coords && createPortal(
<div
ref={tooltipRef}
role="tooltip"
className={`fixed z-[9999] pointer-events-none ${tooltipSizeClass} ${transformClass} ${className}`}
style={{
+2
View File
@@ -16,6 +16,7 @@ export interface DisplayField {
// Book data types
export interface Book {
id: string;
request_id?: number;
title: string;
author: string;
year?: string;
@@ -199,6 +200,7 @@ export interface RequestRecord {
status: 'pending' | 'fulfilled' | 'rejected' | 'cancelled';
delivery_state?: 'none' | 'unknown' | 'queued' | 'resolving' | 'locating' | 'downloading' | 'complete' | 'error' | 'cancelled';
delivery_updated_at?: string | null;
last_failure_reason?: string | null;
source_hint: string | null;
content_type: ContentType;
request_level: 'book' | 'release';
+90
View File
@@ -17,6 +17,7 @@ from shelfmark.core.requests_service import (
normalize_delivery_state,
normalize_request_level,
normalize_request_status,
reopen_failed_request,
reject_request,
sync_delivery_states_from_queue_status,
validate_request_level_payload,
@@ -389,6 +390,8 @@ def test_fulfil_request_queues_as_requesting_user(user_db):
assert captured["user_id"] == alice["id"]
assert captured["username"] == "alice"
assert isinstance(captured["release_data"], dict)
assert captured["release_data"]["_request_id"] == created["id"]
assert "_request_id" not in (fulfilled["release_data"] or {})
def test_fulfil_request_rejects_when_state_changes_after_queue_dispatch(user_db):
@@ -476,6 +479,93 @@ def test_fulfil_book_level_request_stores_selected_release_data(user_db):
assert captured["username"] == "alice"
def test_reopen_failed_request_reverts_to_pending_from_queued_and_clears_on_refulfil(user_db):
alice = user_db.create_user(username="alice")
admin = user_db.create_user(username="admin", role="admin")
created = create_request(
user_db,
user_id=alice["id"],
source_hint="prowlarr",
content_type="ebook",
request_level="release",
policy_mode="request_release",
book_data=_book_data(),
release_data=_release_data(),
)
fulfilled = fulfil_request(
user_db,
request_id=created["id"],
admin_user_id=admin["id"],
queue_release=lambda *_args, **_kwargs: (True, None),
)
assert fulfilled["status"] == "fulfilled"
assert fulfilled["delivery_state"] == "queued"
assert fulfilled["reviewed_by"] == admin["id"]
reopened = reopen_failed_request(
user_db,
request_id=created["id"],
failure_reason="Download failed: Timeout",
)
assert reopened is not None
assert reopened["status"] == "pending"
assert reopened["delivery_state"] == "none"
assert reopened["release_data"] is None
assert reopened["last_failure_reason"] == "Download failed: Timeout"
assert reopened["reviewed_by"] is None
assert reopened["reviewed_at"] is None
replacement_release = _release_data()
replacement_release["source_id"] = "release-456"
refulfilled = fulfil_request(
user_db,
request_id=created["id"],
admin_user_id=admin["id"],
queue_release=lambda *_args, **_kwargs: (True, None),
release_data=replacement_release,
)
assert refulfilled["status"] == "fulfilled"
assert refulfilled["release_data"]["source_id"] == "release-456"
assert refulfilled["last_failure_reason"] is None
def test_reopen_failed_request_does_not_reopen_completed_delivery(user_db):
alice = user_db.create_user(username="alice")
admin = user_db.create_user(username="admin", role="admin")
created = create_request(
user_db,
user_id=alice["id"],
source_hint="prowlarr",
content_type="ebook",
request_level="release",
policy_mode="request_release",
book_data=_book_data(),
release_data=_release_data(),
)
fulfilled = fulfil_request(
user_db,
request_id=created["id"],
admin_user_id=admin["id"],
queue_release=lambda *_args, **_kwargs: (True, None),
)
assert fulfilled["status"] == "fulfilled"
user_db.update_request(
created["id"],
delivery_state="complete",
delivery_updated_at="2026-01-01T00:00:00+00:00",
)
reopened = reopen_failed_request(
user_db,
request_id=created["id"],
failure_reason="Download failed: Timeout",
)
assert reopened is None
def test_sync_delivery_states_from_queue_status_updates_matching_fulfilled_requests(user_db):
alice = user_db.create_user(username="alice")
bob = user_db.create_user(username="bob")