feat(requests): let a book be requested alongside downloads (#1579)

In Download mode there is no way to ask for a book that has nothing to
download yet. The release modal has an "Add to requests" chip for
admins, which sends a book-level request, and when the default mode is
Download the server answers it with "Download policy requires a concrete
release". That is #995, where @expostarv2 used to bookmark unreleased
books this way. #1019 is the other side of it: @punkincognito wants a
less technical user to be able to download from one source and, when
that fails, ask for the book without choosing a release.

A book-level request in Download mode now becomes a normal pending book
request, stored with the mode `request_book`, when the actor is an admin
or when a new setting allows it: **Allow book requests alongside
downloads** (`REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD`, off by
default, per-user overridable). A request that names a release still
goes straight to the download queue as before, and with the setting off
a non-admin gets the same error as today, so nothing changes for an
existing install unless it turns the setting on.

The policy endpoint reports the setting, and the release modal shows
"Add to requests" to a non-admin when requests are on and the setting
allows it (admins keep it as before, and it stays off while fulfilling a
request). The small `canRequestBookFromReleaseModal` helper holds that
rule so it can be tested.

I chose a setting over making this the default for everyone because it
changes what Download mode means, which is yours to decide. The other
way to read #1019 is a per-source rule, "Request Book" for Prowlarr and
Download for the direct sources. That is not here: the rules matrix
excludes Request Book today, the release modal shows such a source as
unavailable, and it needs more UI work than I can check without trying
it in a browser. This PR covers the half that works with the existing
flow, and the rule version can follow if you want it.

Tests: four route tests (a user with the setting on, an admin with it
off, a release request that must still queue a download, and the policy
endpoint reporting the setting), and a frontend test for the helper. The
route tests that depend on the change fail on current main. Full backend
and frontend suites pass, with ruff, oxlint and tsc clean. I have not
tried it in a browser, so this is a draft until someone has looked at
the modal.


Closes #995. #1019 stays open for the per-source rule.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **New Features**
* Added an off-by-default, user-overridable setting that lets users with
download access request a book without selecting a release. These
requests remain pending for admin action, and admins can always make
book-level requests.
* Eligible users can make book-level requests under the download policy
when the setting is enabled. This option is unavailable in browse-fulfil
mode.
* Requests that include release details continue to queue downloads
rather than create book-level requests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
splitsec2 authored and GitHub committed 2026-11-08 17:56:10 -07:00
1 parent c38f4c6522
commit 8c06862ec7
12 files changed
+331 -16

No files matched your search

+1
View File
@@ -98,6 +98,7 @@ Each content type (ebook, audiobook) has a default mode that sets the baseline:
| Request Rules | Per-source overrides (see below) | None |
| Max Pending Requests Per User | Open request limit per user | 20 |
| Allow Notes on Requests | Let users attach a note when submitting | On |
| Allow book requests alongside downloads | Let users who can download also request a book without choosing a release, for a book nobody has released yet or one no source has. The request waits for an admin, and the user gets the Requests tab to follow or cancel it. Admins can always do this | Off |
### Request Rules
+12
View File
@@ -450,4 +450,16 @@ def users_settings() -> list[SettingsField]:
user_overridable=True,
show_when={"field": "REQUESTS_ENABLED", "value": True},
),
CheckboxField(
key="REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD",
label="Allow book requests alongside downloads",
description=(
"Let users who can download also request a book without choosing a release, for "
"a book nobody has released yet or one no source has. The request waits for an "
"admin. Admins can always do this."
),
default=False,
user_overridable=True,
show_when={"field": "REQUESTS_ENABLED", "value": True},
),
]
+1
View File
@@ -70,6 +70,7 @@ REQUEST_POLICY_KEYS = frozenset(
"REQUEST_POLICY_RULES",
"MAX_PENDING_REQUESTS_PER_USER",
"REQUESTS_ALLOW_NOTES",
"REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD",
}
)
+23
View File
@@ -304,6 +304,10 @@ def _prepare_request_create_arguments(
user_db: UserDB,
data: dict[str, Any],
) -> dict[str, Any]:
"""Check a request payload against the policy and build create_request's arguments.
A book-level request in Download mode becomes a pending one when the actor may ask for it.
"""
db_user_id, db_gate = _require_db_user_id()
if db_gate is not None or db_user_id is None:
msg = "User identity is unavailable for request workflow"
@@ -404,6 +408,21 @@ def _prepare_request_create_arguments(
)
requested_level = str(request_level).strip().lower() if isinstance(request_level, str) else ""
if (
resolved_mode == PolicyMode.DOWNLOAD
and requested_level == "book"
and release_data is None
and (
bool(session.get("is_admin", False))
or coerce_bool(
effective.get("REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD"), default=False
)
)
):
# Everything can be downloaded here, but the user asked for the book itself (nothing to
# download yet, or no source has it): leave it for an admin instead of refusing it.
resolved_mode = PolicyMode.REQUEST_BOOK
if resolved_mode == PolicyMode.REQUEST_BOOK and requested_level != "book":
msg = "Policy requires book-level requests"
raise RequestServiceError(
@@ -571,6 +590,7 @@ def register_request_routes(
@app.route("/api/request-policy", methods=["GET"])
def api_request_policy() -> ResponseReturnValue:
"""The signed-in user's request policy, as the frontend needs it to label actions."""
auth_gate = _require_request_endpoints_available(resolve_auth_mode)
if auth_gate is not None:
return auth_gate
@@ -624,6 +644,9 @@ def register_request_routes(
"requests_enabled": requests_enabled,
"is_admin": is_admin,
"allow_notes": coerce_bool(effective.get("REQUESTS_ALLOW_NOTES"), default=True),
"allow_book_request_with_download": coerce_bool(
effective.get("REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD"), default=False
),
"defaults": {
"ebook": (
default_ebook_mode.value
+31 -16
View File
@@ -109,6 +109,8 @@ import {
import {
applyDirectPolicyModeToButtonState,
applyUniversalPolicyModeToButtonState,
canRequestBookFromReleaseModal,
shouldShowRequestsTab,
} from './utils/requestPolicyUi';
import { getSearchByPreference, setSearchByPreference } from './utils/searchByPreference';
import { buildUrlSearchHash } from './utils/urlSearchHash';
@@ -264,6 +266,7 @@ const AdminSettingsWarmupMount = () => {
return null;
};
/** The application shell: search, results, the release modal, activity and settings. */
function App() {
const location = useLocation();
const { toasts, showToast, removeToast } = useToast();
@@ -322,6 +325,7 @@ function App() {
getSourceMode,
requestsEnabled: requestsPolicyEnabled,
allowNotes: allowRequestNotes,
allowBookRequestWithDownload,
refresh: refreshRequestPolicy,
} = useRequestPolicy({
enabled: isAuthenticated,
@@ -455,20 +459,23 @@ function App() {
};
}, [activityStatus, currentStatus, dismissedDownloadTaskIds]);
const showRequestsTab = useMemo(() => {
if (requestRoleIsAdmin) {
return true;
}
if (!isAuthenticated || !requestsPolicyEnabled) {
return false;
}
if (!requestPolicy) {
return false;
}
return !(
requestPolicy.defaults.ebook === 'download' && requestPolicy.defaults.audiobook === 'download'
);
}, [requestRoleIsAdmin, isAuthenticated, requestsPolicyEnabled, requestPolicy]);
const showRequestsTab = useMemo(
() =>
shouldShowRequestsTab({
isAdmin: requestRoleIsAdmin,
isAuthenticated,
requestsEnabled: requestsPolicyEnabled,
defaults: requestPolicy?.defaults ?? null,
allowBookRequestWithDownload,
}),
[
requestRoleIsAdmin,
isAuthenticated,
requestsPolicyEnabled,
requestPolicy,
allowBookRequestWithDownload,
],
);
// Search state and handlers
const {
@@ -2671,7 +2678,15 @@ function App() {
onDownload={isBrowseFulfilMode ? handleBrowseFulfilDownload : handleReleaseDownload}
onRequestRelease={isBrowseFulfilMode ? undefined : handleReleaseRequest}
onRequestBook={
isBrowseFulfilMode || !requestRoleIsAdmin ? undefined : handleReleaseBookRequest
canRequestBookFromReleaseModal({
isBrowseFulfilMode,
isAdmin: requestRoleIsAdmin,
requestsEnabled: requestsPolicyEnabled,
allowBookRequestWithDownload,
bookRequestMode: getSourceMode('*', activeReleaseContentType),
})
? handleReleaseBookRequest
: undefined
}
getPolicyModeForSource={
isBrowseFulfilMode ? () => 'download' : (source, ct) => getSourceMode(source, ct)
@@ -2776,7 +2791,7 @@ function App() {
pendingRequestCount={pendingRequestCount}
showRequestsTab={showRequestsTab}
isRequestsLoading={isActivitySnapshotLoading}
onRequestCancel={showRequestsTab ? handleRequestCancel : undefined}
onRequestCancel={handleRequestCancel}
onRequestApprove={requestRoleIsAdmin ? handleRequestApprove : undefined}
onRequestReject={requestRoleIsAdmin ? handleRequestReject : undefined}
onRequestReopen={requestRoleIsAdmin ? handleRequestReopen : undefined}
@@ -31,6 +31,7 @@ const REQUEST_POLICY_OVERRIDE_KEYS: Array<keyof PerUserSettings> = [
'REQUEST_POLICY_RULES',
'MAX_PENDING_REQUESTS_PER_USER',
'REQUESTS_ALLOW_NOTES',
'REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD',
];
const requestPolicyHeading: HeadingFieldConfig = {
@@ -22,6 +22,7 @@ export interface PerUserSettings {
REQUEST_POLICY_RULES?: Array<Record<string, unknown>>;
MAX_PENDING_REQUESTS_PER_USER?: number;
REQUESTS_ALLOW_NOTES?: boolean;
REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD?: boolean;
}
export interface CreateUserFormState {
@@ -22,11 +22,15 @@ interface UseRequestPolicyReturn {
isAdmin: boolean;
requestsEnabled: boolean;
allowNotes: boolean;
allowBookRequestWithDownload: boolean;
getDefaultMode: (contentType: string) => RequestPolicyMode;
getSourceMode: (source: string, contentType: string) => RequestPolicyMode;
refresh: (options?: { force?: boolean }) => Promise<RequestPolicyResponse | null>;
}
/**
* Fetch and cache the user's request policy, and resolve the mode for a source and content type.
*/
export function useRequestPolicy({
enabled,
isAdmin,
@@ -102,6 +106,9 @@ export function useRequestPolicy({
isAdmin: enabled && policy ? policy.is_admin : isAdmin,
requestsEnabled: enabled ? (policy?.requests_enabled ?? false) : false,
allowNotes: enabled ? (policy?.allow_notes ?? true) : true,
allowBookRequestWithDownload: enabled
? (policy?.allow_book_request_with_download ?? false)
: false,
getDefaultMode,
getSourceMode,
refresh,
@@ -4,8 +4,103 @@ import type { ButtonStateInfo } from '../types/index';
import {
applyDirectPolicyModeToButtonState,
applyUniversalPolicyModeToButtonState,
canRequestBookFromReleaseModal,
shouldShowRequestsTab,
} from '../utils/requestPolicyUi';
describe('canRequestBookFromReleaseModal', () => {
const base = {
isBrowseFulfilMode: false,
isAdmin: false,
requestsEnabled: true,
allowBookRequestWithDownload: false,
bookRequestMode: 'download' as const,
};
it('is always on for an admin', () => {
expect(canRequestBookFromReleaseModal({ ...base, isAdmin: true })).toBe(true);
expect(canRequestBookFromReleaseModal({ ...base, isAdmin: true, requestsEnabled: false })).toBe(
true,
);
});
it('is off for a user unless the setting allows it', () => {
expect(canRequestBookFromReleaseModal(base)).toBe(false);
expect(canRequestBookFromReleaseModal({ ...base, allowBookRequestWithDownload: true })).toBe(
true,
);
});
it('is off for a user whose policy for this content type is not Download', () => {
const allowed = { ...base, allowBookRequestWithDownload: true };
expect(canRequestBookFromReleaseModal({ ...allowed, bookRequestMode: 'request_release' })).toBe(
false,
);
expect(canRequestBookFromReleaseModal({ ...allowed, bookRequestMode: 'request_book' })).toBe(
false,
);
expect(canRequestBookFromReleaseModal({ ...allowed, bookRequestMode: 'blocked' })).toBe(false);
});
it('does not depend on the policy mode for an admin', () => {
expect(
canRequestBookFromReleaseModal({ ...base, isAdmin: true, bookRequestMode: 'blocked' }),
).toBe(true);
});
it('needs requests to be on for a user', () => {
expect(
canRequestBookFromReleaseModal({
...base,
requestsEnabled: false,
allowBookRequestWithDownload: true,
}),
).toBe(false);
});
it('is off while fulfilling a request, even for an admin', () => {
expect(
canRequestBookFromReleaseModal({ ...base, isAdmin: true, isBrowseFulfilMode: true }),
).toBe(false);
});
});
describe('shouldShowRequestsTab', () => {
const allDownload = { ebook: 'download' as const, audiobook: 'download' as const };
const base = {
isAdmin: false,
isAuthenticated: true,
requestsEnabled: true,
defaults: allDownload,
allowBookRequestWithDownload: false,
};
it('is hidden for a user who can only download', () => {
expect(shouldShowRequestsTab(base)).toBe(false);
});
it('shows for a user who may request a book alongside downloads', () => {
expect(shouldShowRequestsTab({ ...base, allowBookRequestWithDownload: true })).toBe(true);
});
it('shows when either default asks for requests', () => {
expect(
shouldShowRequestsTab({
...base,
defaults: { ebook: 'download', audiobook: 'request_book' },
}),
).toBe(true);
});
it('is hidden with requests off or no policy, and always shown to an admin', () => {
const allowed = { ...base, allowBookRequestWithDownload: true };
expect(shouldShowRequestsTab({ ...allowed, requestsEnabled: false })).toBe(false);
expect(shouldShowRequestsTab({ ...allowed, defaults: null })).toBe(false);
expect(shouldShowRequestsTab({ ...allowed, isAuthenticated: false })).toBe(false);
expect(shouldShowRequestsTab({ ...base, isAdmin: true, requestsEnabled: false })).toBe(true);
});
});
describe('requestPolicyUi', () => {
const baseDownload: ButtonStateInfo = { text: 'Download', state: 'download' };
+1
View File
@@ -217,6 +217,7 @@ export interface RequestPolicyResponse {
requests_enabled: boolean;
is_admin: boolean;
allow_notes: boolean;
allow_book_request_with_download?: boolean;
defaults: RequestPolicyDefaults;
rules: Array<Record<string, unknown>>;
source_modes: RequestPolicySourceMode[];
+49
View File
@@ -1,5 +1,54 @@
import type { ButtonStateInfo, RequestPolicyMode } from '../types';
/**
* Whether the release modal offers a book-level request ("Add to requests"). Admins always get
* it. Anyone else gets it only when requests are on, the setting lets users who can download
* also ask for a book without choosing a release, and their policy for this content type is
* Download. The server turns a book-level request into a pending one only in Download mode, so
* the action would be refused (Blocked) or mean something else (Request Release) otherwise.
*/
export const canRequestBookFromReleaseModal = ({
isBrowseFulfilMode,
isAdmin,
requestsEnabled,
allowBookRequestWithDownload,
bookRequestMode,
}: {
isBrowseFulfilMode: boolean;
isAdmin: boolean;
requestsEnabled: boolean;
allowBookRequestWithDownload: boolean;
bookRequestMode: RequestPolicyMode;
}): boolean => {
if (isBrowseFulfilMode) return false;
if (isAdmin) return true;
return requestsEnabled && allowBookRequestWithDownload && bookRequestMode === 'download';
};
/**
* Whether the activity sidebar shows a Requests tab. Admins always see it. A user sees it when
* requests are on and either default asks for requests, or when they may request a book
* alongside downloads, since those requests then need a place to be followed and cancelled.
*/
export const shouldShowRequestsTab = ({
isAdmin,
isAuthenticated,
requestsEnabled,
defaults,
allowBookRequestWithDownload,
}: {
isAdmin: boolean;
isAuthenticated: boolean;
requestsEnabled: boolean;
defaults: { ebook: RequestPolicyMode; audiobook: RequestPolicyMode } | null;
allowBookRequestWithDownload: boolean;
}): boolean => {
if (isAdmin) return true;
if (!isAuthenticated || !requestsEnabled || !defaults) return false;
if (allowBookRequestWithDownload) return true;
return !(defaults.ebook === 'download' && defaults.audiobook === 'download');
};
export const applyDirectPolicyModeToButtonState = (
baseState: ButtonStateInfo,
mode: RequestPolicyMode,
+109
View File
@@ -48,9 +48,12 @@ def _policy(
default_audiobook: str = "download",
max_pending_requests_per_user: int = 20,
requests_allow_notes: bool = True,
allow_book_request_with_download: bool = False,
rules: list[dict] | None = None,
) -> dict:
"""A request-policy settings dict with the given values, for patching the policy loader."""
return {
"REQUESTS_ALLOW_BOOK_REQUEST_WITH_DOWNLOAD": allow_book_request_with_download,
"REQUESTS_ENABLED": requests_enabled,
"REQUEST_POLICY_DEFAULT_EBOOK": default_ebook,
"REQUEST_POLICY_DEFAULT_AUDIOBOOK": default_audiobook,
@@ -2126,6 +2129,112 @@ class TestRequestCreationEdgeCases:
mock_queue_release.assert_not_called()
assert main_module.user_db.list_requests(user_id=user["id"]) == []
@staticmethod
def _book_level_request_payload() -> dict:
return {
"book_data": {
"title": "Not Released Yet",
"author": "Shelfmark",
"content_type": "ebook",
"provider": "openlibrary",
"provider_id": "not-released-yet",
},
"context": {"source": "*", "content_type": "ebook", "request_level": "book"},
}
def _post_book_request(self, main_module, client, policy):
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
with patch.object(
main_module, "load_users_request_policy_settings", return_value=policy
):
with patch(
"shelfmark.core.request_routes.load_users_request_policy_settings",
return_value=policy,
):
with patch.object(main_module.backend, "queue_release") as mock_queue_release:
resp = client.post("/api/requests", json=self._book_level_request_payload())
return resp, mock_queue_release
def test_a_book_request_is_kept_in_download_mode_when_the_setting_allows_it(
self, main_module, client
):
"""Download mode left no way to ask for a book nobody has released yet (#995, #1019)."""
user = _create_user(main_module, prefix="reader")
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
policy = _policy(default_ebook="download", allow_book_request_with_download=True)
resp, mock_queue_release = self._post_book_request(main_module, client, policy)
assert resp.status_code == 201
assert resp.json["status"] == "pending"
assert resp.json["policy_mode"] == "request_book"
assert resp.json["request_level"] == "book"
mock_queue_release.assert_not_called()
assert len(main_module.user_db.list_requests(user_id=user["id"])) == 1
def test_an_admin_can_request_a_book_in_download_mode_without_the_setting(
self, main_module, client
):
admin = _create_user(main_module, prefix="admin", role="admin")
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
policy = _policy(default_ebook="download", allow_book_request_with_download=False)
resp, mock_queue_release = self._post_book_request(main_module, client, policy)
assert resp.status_code == 201
assert resp.json["policy_mode"] == "request_book"
mock_queue_release.assert_not_called()
def test_the_setting_does_not_turn_a_release_request_into_a_book_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)
policy = _policy(default_ebook="download", allow_book_request_with_download=True)
payload = self._book_level_request_payload()
payload["release_data"] = {"source": "direct_download", "source_id": "abc", "title": "T"}
payload["context"] = {
"source": "direct_download",
"content_type": "ebook",
"request_level": "release",
}
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
with patch.object(
main_module, "load_users_request_policy_settings", return_value=policy
):
with patch(
"shelfmark.core.request_routes.load_users_request_policy_settings",
return_value=policy,
):
with patch.object(
main_module.backend, "queue_release", return_value=(True, None)
) as mock_queue_release:
resp = client.post("/api/requests", json=payload)
assert resp.status_code == 200
assert resp.json["kind"] == "download"
mock_queue_release.assert_called_once()
assert main_module.user_db.list_requests(user_id=user["id"]) == []
@pytest.mark.parametrize("allowed", [True, False])
def test_policy_endpoint_reports_the_book_request_setting(self, main_module, client, allowed):
user = _create_user(main_module, prefix="reader")
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
policy = _policy(default_ebook="download", allow_book_request_with_download=allowed)
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
with patch.object(
main_module, "load_users_request_policy_settings", return_value=policy
):
with patch(
"shelfmark.core.request_routes.load_users_request_policy_settings",
return_value=policy,
):
resp = client.get("/api/request-policy")
assert resp.json["allow_book_request_with_download"] is allowed
def test_blocked_policy_returns_403(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)