From 685c35d552c1176ce3d452278069ebbdff1af8ec Mon Sep 17 00:00:00 2001 From: Alex <25013571+alexhb1@users.noreply.github.com> Date: Fri, 13 Mar 2026 14:33:10 +0000 Subject: [PATCH] Hardcover list separation + Browser download fix (#745) - Added full Hardcover reading status types into the list selector - Split reading status entries from dedicated lists - Added option to disable the automatic removal of books when downloaded from a Hardcover list - Fixed browser download not firing when the completed state was triggered in specific cases --- shelfmark/main.py | 9 +- shelfmark/metadata_providers/hardcover.py | 89 ++++++++++++------- src/frontend/src/App.tsx | 18 ++-- .../src/components/BookTargetDropdown.tsx | 28 +++++- src/frontend/src/components/DropdownList.tsx | 76 +++++++++------- src/frontend/src/components/SearchBar.tsx | 57 ++++++------ .../src/components/shared/DynamicDropdown.tsx | 1 + src/frontend/src/services/api.ts | 4 +- src/frontend/src/types/index.ts | 2 + tests/metadata/test_hardcover_lists.py | 64 +++++++------ 10 files changed, 221 insertions(+), 127 deletions(-) diff --git a/shelfmark/main.py b/shelfmark/main.py index 8937800e..9dcbb40e 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -1016,6 +1016,7 @@ def api_config() -> Union[Response, Tuple[Response, int]]: "show_release_source_links": app_config.get("SHOW_RELEASE_SOURCE_LINKS", True), "books_output_mode": app_config.get("BOOKS_OUTPUT_MODE", "folder"), "auto_open_downloads_sidebar": app_config.get("AUTO_OPEN_DOWNLOADS_SIDEBAR", True), + "hardcover_auto_remove_on_download": app_config.get("HARDCOVER_AUTO_REMOVE_ON_DOWNLOAD", True), "download_to_browser_content_types": app_config.get( "DOWNLOAD_TO_BROWSER_CONTENT_TYPES", [], @@ -2375,11 +2376,15 @@ def api_metadata_book_targets_update(provider: str, book_id: str) -> Union[Respo return jsonify({"error": "selected must be a boolean"}), 400 result = prov.set_book_target_state(book_id, target, selected) - return jsonify({ + response: dict = { "success": True, "changed": bool(result.get("changed", True)), "selected": selected, - }) + } + deselected = result.get("deselected_target") + if isinstance(deselected, str) and deselected: + response["deselected_target"] = deselected + return jsonify(response) @app.route('/api/releases', methods=['GET']) diff --git a/shelfmark/metadata_providers/hardcover.py b/shelfmark/metadata_providers/hardcover.py index 6b86fed3..5ada37ff 100644 --- a/shelfmark/metadata_providers/hardcover.py +++ b/shelfmark/metadata_providers/hardcover.py @@ -104,7 +104,22 @@ query GetUserLists { me { id username - want_to_read_books: user_books_aggregate(where: {status_id: {_eq: 1}}) { + want_to_read_count: user_books_aggregate(where: {status_id: {_eq: 1}}) { + aggregate { + count(columns: [book_id], distinct: true) + } + } + currently_reading_count: user_books_aggregate(where: {status_id: {_eq: 2}}) { + aggregate { + count(columns: [book_id], distinct: true) + } + } + read_count: user_books_aggregate(where: {status_id: {_eq: 3}}) { + aggregate { + count(columns: [book_id], distinct: true) + } + } + did_not_finish_count: user_books_aggregate(where: {status_id: {_eq: 5}}) { aggregate { count(columns: [book_id], distinct: true) } @@ -371,16 +386,17 @@ query GetSeriesBooks($seriesId: Int!) { } """ -HARDCOVER_WANT_TO_READ_STATUS_ID = 1 HARDCOVER_STATUS_PREFIX = "status:" -HARDCOVER_STATUS_URL_SLUGS: dict[int, str] = { - 1: "want-to-read", - 2: "currently-reading", - 3: "read", - 5: "did-not-finish", -} +HARDCOVER_STATUSES: list[dict] = [ + {"id": 1, "label": "Want to Read", "slug": "want-to-read", "query_key": "want_to_read_count"}, + {"id": 2, "label": "Currently Reading", "slug": "currently-reading", "query_key": "currently_reading_count"}, + {"id": 3, "label": "Read", "slug": "read", "query_key": "read_count"}, + {"id": 5, "label": "Did Not Finish", "slug": "did-not-finish", "query_key": "did_not_finish_count"}, +] +HARDCOVER_STATUS_URL_SLUGS: dict[int, str] = {s["id"]: s["slug"] for s in HARDCOVER_STATUSES} +HARDCOVER_STATUS_GROUP = "Reading Status" HARDCOVER_LIST_ID_PREFIX = "id:" -HARDCOVER_WRITABLE_TARGET_GROUPS = {"My Books", "My Lists"} +HARDCOVER_WRITABLE_TARGET_GROUPS = {HARDCOVER_STATUS_GROUP, "My Lists"} @dataclass(frozen=True) @@ -1539,26 +1555,27 @@ class HardcoverProvider(MetadataProvider): except (TypeError, ValueError): return name - want_to_read_count_data = me_data.get("want_to_read_books", {}) - want_to_read_aggregate = ( - want_to_read_count_data.get("aggregate", {}) - if isinstance(want_to_read_count_data, dict) - else {} - ) - want_to_read_count = ( - want_to_read_aggregate.get("count") - if isinstance(want_to_read_aggregate, dict) - else None - ) - want_to_read_value = f"{HARDCOVER_STATUS_PREFIX}{HARDCOVER_WANT_TO_READ_STATUS_ID}" - seen_values.add(want_to_read_value) - options.append( - { - "value": want_to_read_value, - "label": _format_label("Want to Read", want_to_read_count), - "group": "My Books", - } - ) + for status in HARDCOVER_STATUSES: + count_data = me_data.get(status["query_key"], {}) + aggregate = ( + count_data.get("aggregate", {}) + if isinstance(count_data, dict) + else {} + ) + count = ( + aggregate.get("count") + if isinstance(aggregate, dict) + else None + ) + value = f"{HARDCOVER_STATUS_PREFIX}{status['id']}" + seen_values.add(value) + options.append( + { + "value": value, + "label": _format_label(status["label"], count), + "group": HARDCOVER_STATUS_GROUP, + } + ) for list_item in me_data.get("lists", []): if not isinstance(list_item, dict): @@ -1652,6 +1669,7 @@ class HardcoverProvider(MetadataProvider): state = self._fetch_book_target_state(book_id_int) status_ids_to_invalidate: set[int] = set() list_ids_to_invalidate: set[int] = set() + deselected_target: Optional[str] = None if selected_target.startswith(HARDCOVER_STATUS_PREFIX): status_id = self._parse_prefixed_int(selected_target, "status target") @@ -1660,6 +1678,8 @@ class HardcoverProvider(MetadataProvider): if changed: if previous_status_id is not None: status_ids_to_invalidate.add(previous_status_id) + if selected and previous_status_id != status_id: + deselected_target = f"{HARDCOVER_STATUS_PREFIX}{previous_status_id}" status_ids_to_invalidate.add(status_id) elif selected_target.startswith(HARDCOVER_LIST_ID_PREFIX): list_id = self._parse_prefixed_int(selected_target, "list target") @@ -1676,7 +1696,10 @@ class HardcoverProvider(MetadataProvider): list_ids=list_ids_to_invalidate, ) - return {"changed": changed} + result_data: Dict[str, Any] = {"changed": changed} + if deselected_target: + result_data["deselected_target"] = deselected_target + return result_data @staticmethod def _unwrap_me_data(result: Optional[Dict]) -> Dict: @@ -2716,4 +2739,10 @@ def hardcover_settings(): description="Filter out books with a release year in the future", default=False, ), + CheckboxField( + key="HARDCOVER_AUTO_REMOVE_ON_DOWNLOAD", + label="Auto-Remove from List on Download", + description="Automatically remove a book from the active Hardcover list when you download it", + default=True, + ), ] diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index 137cea4f..994c61e7 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -591,13 +591,11 @@ function App() { }); // Check for completed items - const prevDownloadingIds = new Set(Object.keys(prevDownloading)); - const prevResolvingIds = new Set(Object.keys(prev.resolving || {})); - const prevQueuedIds = new Set(Object.keys(prevQueued)); + const prevComplete = prev.complete || {}; const currComplete = curr.complete || {}; Object.keys(currComplete).forEach(bookId => { - if (prevDownloadingIds.has(bookId) || prevQueuedIds.has(bookId)) { + if (!prevComplete[bookId]) { const book = currComplete[bookId]; showToast(`${book.title || 'Book'} completed`, 'success'); @@ -621,9 +619,10 @@ function App() { }); // Check for failed items + const prevError = prev.error || {}; const currError = curr.error || {}; Object.keys(currError).forEach(bookId => { - if (prevDownloadingIds.has(bookId) || prevResolvingIds.has(bookId) || prevQueuedIds.has(bookId)) { + if (!prevError[bookId]) { const book = currError[bookId]; const errorMsg = book.status_message || 'Download failed'; showToast(`${book.title || 'Book'}: ${errorMsg}`, 'error'); @@ -1115,18 +1114,19 @@ function App() { metadataConfigRef.current = activeMetadataConfig; const removeBookFromActiveList = useCallback((book: Book) => { + if (config?.hardcover_auto_remove_on_download === false) return; if (!bookSupportsTargets(book)) return; const activeList = searchFieldValuesRef.current.hardcover_list; if (!activeList) return; const target = String(activeList); - // Only auto-remove from lists the user owns (My Books / My Lists) + // Only auto-remove from lists the user owns (Reading Status / My Lists) const listField = metadataConfigRef.current?.search_fields.find( (f) => f.key === 'hardcover_list' && f.type === 'DynamicSelectSearchField', ); if (listField && listField.type === 'DynamicSelectSearchField') { const group = getDynamicOptionGroup(listField.options_endpoint, target); - if (group && group !== 'My Books' && group !== 'My Lists') return; + if (group && group !== 'Reading Status' && group !== 'My Lists') return; } void setBookTargetState(book.provider!, book.provider_id!, target, false).then((result) => { @@ -1141,7 +1141,7 @@ function App() { showToast(`Removed from ${listName || 'list'}`, 'info'); } }).catch(() => {}); - }, [showToast]); + }, [config?.hardcover_auto_remove_on_download, showToast]); const executeBookDownload = useCallback( async (book: Book, onBehalfOfUserId?: number): Promise => { @@ -2125,7 +2125,7 @@ function App() { bottom: 0, left: 0, right: '25rem', - zIndex: 40, + zIndex: 20, } : { paddingTop: `${headerHeight}px` } } diff --git a/src/frontend/src/components/BookTargetDropdown.tsx b/src/frontend/src/components/BookTargetDropdown.tsx index 2e34307d..90096339 100644 --- a/src/frontend/src/components/BookTargetDropdown.tsx +++ b/src/frontend/src/components/BookTargetDropdown.tsx @@ -51,14 +51,23 @@ const renderSummary = (selectedOptions: DropdownListOption[]) => { ); }; +const STATUS_PREFIX = 'status:'; + +const isStatusTarget = (value: string): boolean => value.startsWith(STATUS_PREFIX); + const updateOptionChecked = ( prev: BookTargetOption[], target: string, checked: boolean, ): BookTargetOption[] => - prev.map((option) => - option.value === target ? { ...option, checked } : option, - ); + prev.map((option) => { + if (option.value === target) return { ...option, checked }; + // Statuses are mutually exclusive — uncheck other statuses when one is selected + if (checked && isStatusTarget(target) && isStatusTarget(option.value)) { + return { ...option, checked: false }; + } + return option; + }); export const BookTargetDropdown = ({ provider, @@ -136,6 +145,7 @@ export const BookTargetDropdown = ({ value: option.value, label: option.label, description: option.description, + group: option.group, disabled: !option.writable || pendingTargets.has(option.value), })); }, [isLoading, loadError, options, pendingTargets]); @@ -176,6 +186,16 @@ export const BookTargetDropdown = ({ target: toggledTarget, selected: result.selected, }); + // When a status was implicitly deselected, sync other instances + if (result.deselectedTarget) { + setOptions((prev) => updateOptionChecked(prev, result.deselectedTarget!, false)); + emitBookTargetChange({ + provider, + bookId, + target: result.deselectedTarget, + selected: false, + }); + } const label = stripCountSuffix(toggledOption.label); onShowToast?.( `${result.selected ? 'Added to' : 'Removed from'} ${label}`, @@ -232,7 +252,7 @@ export const BookTargetDropdown = ({ options={dropdownOptions} value={selectedValues} onChange={handleChange} - placeholder={isLoading ? 'Loading…' : 'Lists & Want to Read'} + placeholder={isLoading ? 'Loading…' : 'Hardcover'} widthClassName={variant !== 'default' ? 'w-auto' : widthClassName} buttonClassName={variant !== 'default' ? '' : 'py-1.5 leading-none'} panelClassName={variant !== 'default' ? 'w-56' : undefined} diff --git a/src/frontend/src/components/DropdownList.tsx b/src/frontend/src/components/DropdownList.tsx index 3dc6029b..6ee72af4 100644 --- a/src/frontend/src/components/DropdownList.tsx +++ b/src/frontend/src/components/DropdownList.tsx @@ -7,6 +7,7 @@ export interface DropdownListOption { description?: string; disabled?: boolean; icon?: ReactNode; + group?: string; } interface DropdownListProps { @@ -115,37 +116,50 @@ export const DropdownList = ({ renderTrigger={renderTrigger} onOpenChange={onOpenChange} > - {({ close }) => ( -
- {options.map(option => ( - - ))} -
- )} + {({ close }) => { + let lastGroup: string | undefined; + return ( +
+ {options.map(option => { + const showGroupHeader = option.group != null && option.group !== lastGroup; + if (option.group != null) lastGroup = option.group; + return ( +
+ {showGroupHeader && ( +
+ {option.group} +
+ )} + +
+ ); + })} +
+ ); + }} ); }; diff --git a/src/frontend/src/components/SearchBar.tsx b/src/frontend/src/components/SearchBar.tsx index b2379df7..ea699421 100644 --- a/src/frontend/src/components/SearchBar.tsx +++ b/src/frontend/src/components/SearchBar.tsx @@ -274,7 +274,7 @@ export const SearchBar = forwardRef(({ fetchFieldOptions(dynamicEndpoint).then((loaded) => { if (cancelled) return; - setDynamicOptions(loaded.map((o) => ({ value: o.value, label: o.label }))); + setDynamicOptions(loaded.map((o) => ({ value: o.value, label: o.label, group: o.group }))); setIsDynamicLoading(false); }).catch(() => { if (cancelled) return; @@ -832,34 +832,41 @@ export const SearchBar = forwardRef(({ aria-label={effectiveInputAriaLabel} >
- {selectOptions.map((option) => { + {selectOptions.map((option, index) => { const currentValue = typeof value === 'string' ? value : String(value ?? ''); const isSelected = option.value === currentValue; + const showGroupHeader = option.group != null && (index === 0 || option.group !== selectOptions[index - 1]?.group); return ( - + +
); })} diff --git a/src/frontend/src/components/shared/DynamicDropdown.tsx b/src/frontend/src/components/shared/DynamicDropdown.tsx index ca5a2cb8..17bf4cc9 100644 --- a/src/frontend/src/components/shared/DynamicDropdown.tsx +++ b/src/frontend/src/components/shared/DynamicDropdown.tsx @@ -22,6 +22,7 @@ const buildOptions = ( value: option.value, label: option.label, description: option.description, + group: option.group, })); }; diff --git a/src/frontend/src/services/api.ts b/src/frontend/src/services/api.ts index 9321754a..b7ad3ac6 100644 --- a/src/frontend/src/services/api.ts +++ b/src/frontend/src/services/api.ts @@ -257,6 +257,7 @@ export interface BookTargetOption { export interface BookTargetStateResult { changed: boolean; selected: boolean; + deselectedTarget?: string; } // Search metadata providers and normalize to Book format @@ -423,7 +424,7 @@ export const setBookTargetState = async ( target: string, selected: boolean, ): Promise => { - const response = await fetchJSON<{ changed?: unknown; selected?: unknown }>( + const response = await fetchJSON<{ changed?: unknown; selected?: unknown; deselected_target?: unknown }>( `${API_BASE}/metadata/book/${encodeURIComponent(provider)}/${encodeURIComponent(bookId)}/targets`, { method: 'PUT', @@ -434,6 +435,7 @@ export const setBookTargetState = async ( return { changed: response.changed === true, selected: response.selected === true, + deselectedTarget: typeof response.deselected_target === 'string' ? response.deselected_target : undefined, }; }; diff --git a/src/frontend/src/types/index.ts b/src/frontend/src/types/index.ts index 95e5e267..f51847b2 100644 --- a/src/frontend/src/types/index.ts +++ b/src/frontend/src/types/index.ts @@ -115,6 +115,7 @@ export interface Toast { export interface SortOption { value: string; label: string; + group?: string; } // Search field types (mirror backend search field types) @@ -264,6 +265,7 @@ export interface AppConfig { show_release_source_links: boolean; books_output_mode: BooksOutputMode; auto_open_downloads_sidebar: boolean; // Auto-open sidebar when download is queued + hardcover_auto_remove_on_download: boolean; // Auto-remove from active Hardcover list on download download_to_browser_content_types: string[]; // Auto-download completed files to browser for selected content types settings_enabled: boolean; // Whether config directory is mounted and writable onboarding_complete: boolean; // Whether the user has completed initial setup diff --git a/tests/metadata/test_hardcover_lists.py b/tests/metadata/test_hardcover_lists.py index 213bbeb2..da35a1c5 100644 --- a/tests/metadata/test_hardcover_lists.py +++ b/tests/metadata/test_hardcover_lists.py @@ -1,7 +1,7 @@ from shelfmark.core.cache import cache_key from shelfmark.metadata_providers import MetadataSearchOptions, SearchResult from shelfmark.metadata_providers.hardcover import ( - HARDCOVER_WANT_TO_READ_STATUS_ID, + HARDCOVER_STATUS_GROUP, HardcoverProvider, ) @@ -23,7 +23,7 @@ class CacheStub: class TestHardcoverLists: - def test_fetch_user_lists_includes_want_to_read_shelf(self, monkeypatch): + def test_fetch_user_lists_includes_all_status_shelves(self, monkeypatch): provider = HardcoverProvider(api_key="test-token") monkeypatch.setattr( @@ -32,11 +32,10 @@ class TestHardcoverLists: lambda query, variables: { "me": { "username": "alex", - "want_to_read_books": { - "aggregate": { - "count": 7, - } - }, + "want_to_read_count": {"aggregate": {"count": 7}}, + "currently_reading_count": {"aggregate": {"count": 3}}, + "read_count": {"aggregate": {"count": 20}}, + "did_not_finish_count": {"aggregate": {"count": 1}}, "lists": [ { "id": 42, @@ -53,11 +52,26 @@ class TestHardcoverLists: options = provider._fetch_user_lists() assert options[0] == { - "value": f"status:{HARDCOVER_WANT_TO_READ_STATUS_ID}", + "value": "status:1", "label": "Want to Read (7)", - "group": "My Books", + "group": HARDCOVER_STATUS_GROUP, } assert options[1] == { + "value": "status:2", + "label": "Currently Reading (3)", + "group": HARDCOVER_STATUS_GROUP, + } + assert options[2] == { + "value": "status:3", + "label": "Read (20)", + "group": HARDCOVER_STATUS_GROUP, + } + assert options[3] == { + "value": "status:5", + "label": "Did Not Finish (1)", + "group": HARDCOVER_STATUS_GROUP, + } + assert options[4] == { "value": "id:42", "label": "Sci-Fi Favourites (12)", "group": "My Lists", @@ -81,13 +95,13 @@ class TestHardcoverLists: query="", page=2, limit=20, - fields={"hardcover_list": f"status:{HARDCOVER_WANT_TO_READ_STATUS_ID}"}, + fields={"hardcover_list": "status:1"}, ) ) assert result == expected assert captured == { - "status_id": HARDCOVER_WANT_TO_READ_STATUS_ID, + "status_id": 1, "page": 2, "limit": 20, } @@ -134,13 +148,13 @@ class TestHardcoverLists: monkeypatch.setattr(provider, "_execute_query", fake_execute) result = provider._fetch_current_user_books_by_status( - HARDCOVER_WANT_TO_READ_STATUS_ID, + 1, page=1, limit=10, ) assert captured["variables"] == { - "statusId": HARDCOVER_WANT_TO_READ_STATUS_ID, + "statusId": 1, "limit": 10, "offset": 0, } @@ -159,9 +173,9 @@ class TestHardcoverLists: "get_user_lists", lambda: [ { - "value": f"status:{HARDCOVER_WANT_TO_READ_STATUS_ID}", + "value": "status:1", "label": "Want to Read (7)", - "group": "My Books", + "group": HARDCOVER_STATUS_GROUP, }, { "value": "id:42", @@ -181,7 +195,7 @@ class TestHardcoverLists: "_execute_query", lambda query, variables, raise_on_error=False: { "me": { - "user_books": [{"id": 55, "status_id": HARDCOVER_WANT_TO_READ_STATUS_ID}], + "user_books": [{"id": 55, "status_id": 1}], "lists": [ {"id": 42, "list_books": [{"id": 500}]}, {"id": 99, "list_books": [{"id": 900}]}, @@ -194,9 +208,9 @@ class TestHardcoverLists: assert options == [ { - "value": f"status:{HARDCOVER_WANT_TO_READ_STATUS_ID}", + "value": "status:1", "label": "Want to Read (7)", - "group": "My Books", + "group": HARDCOVER_STATUS_GROUP, "checked": True, "writable": True, }, @@ -219,9 +233,9 @@ class TestHardcoverLists: "get_user_lists", lambda: [ { - "value": f"status:{HARDCOVER_WANT_TO_READ_STATUS_ID}", + "value": "status:1", "label": "Want to Read (7)", - "group": "My Books", + "group": HARDCOVER_STATUS_GROUP, } ], ) @@ -242,7 +256,7 @@ class TestHardcoverLists: "update_user_book": { "id": 77, "error": None, - "user_book": {"id": 77, "book_id": 123, "status_id": HARDCOVER_WANT_TO_READ_STATUS_ID}, + "user_book": {"id": 77, "book_id": 123, "status_id": 1}, } } raise AssertionError(f"Unexpected query: {query}") @@ -251,21 +265,21 @@ class TestHardcoverLists: result = provider.set_book_target_state( "123", - f"status:{HARDCOVER_WANT_TO_READ_STATUS_ID}", + "status:1", True, ) - assert result == {"changed": True} + assert result == {"changed": True, "deselected_target": "status:2"} assert captured["variables"] == { "userBookId": 77, - "statusId": HARDCOVER_WANT_TO_READ_STATUS_ID, + "statusId": 1, } assert cache_stub.invalidated == [ cache_key("hardcover:user_lists", "user-123"), ] assert set(cache_stub.invalidated_prefixes) == { cache_key("hardcover:user_books:status", "user-123", 2), - cache_key("hardcover:user_books:status", "user-123", HARDCOVER_WANT_TO_READ_STATUS_ID), + cache_key("hardcover:user_books:status", "user-123", 1), } def test_set_book_target_state_removes_list_membership(self, monkeypatch):