diff --git a/shelfmark/config/users_settings.py b/shelfmark/config/users_settings.py index c54f36c3..3e646275 100644 --- a/shelfmark/config/users_settings.py +++ b/shelfmark/config/users_settings.py @@ -62,7 +62,7 @@ _SELF_SETTINGS_SECTION_OPTIONS = [ { "value": "search", "label": "Search Preferences", - "description": "Show personal search mode and provider settings.", + "description": "Show personal search mode, language, and provider settings.", }, { "value": "notifications", @@ -78,7 +78,7 @@ _SEARCH_PREFERENCE_PROVIDER_KEYS = { "METADATA_PROVIDER_AUDIOBOOK", "METADATA_PROVIDER_COMBINED", } -_SEARCH_PREFERENCE_VALIDATABLE_KEYS = { +SEARCH_PREFERENCE_VALIDATABLE_KEYS = { "SEARCH_MODE", "BOOK_LANGUAGE", "DEFAULT_RELEASE_SOURCE", @@ -183,9 +183,11 @@ def _get_request_policy_rule_columns() -> list[dict[str, object]]: def _validate_book_languages(value: Any) -> tuple[Any, str | None]: """Validate a per-user default language list against the known languages. - Accepts the list the settings UI sends as well as the comma-separated string the - env var uses. An empty list is a deliberate override meaning "no default language - filter", so it is kept as-is; ``None`` clears the override further up the chain. + Accepts the list the settings UI sends as well as a comma-separated string, so an + API client can spell the value the way the env var does. Blank entries are skipped + rather than rejected, which makes "" and "en," mean the same as [] and ["en"]. An + empty result is a deliberate override meaning "no default language filter", so it + is kept as-is; ``None`` clears the override further up the chain. """ entries = value.split(",") if isinstance(value, str) else value if not isinstance(entries, (list, tuple)): @@ -193,6 +195,8 @@ def _validate_book_languages(value: Any) -> tuple[Any, str | None]: normalized: list[str] = [] for entry in entries: + if entry is None or (isinstance(entry, str) and not entry.strip()): + continue code = normalize_language(entry) if code is None: return value, f"BOOK_LANGUAGE contains an unsupported language: {entry}" @@ -204,7 +208,7 @@ def _validate_book_languages(value: Any) -> tuple[Any, str | None]: def validate_search_preference_value(key: str, value: Any) -> tuple[Any, str | None]: """Validate and normalize a search preference value for user overrides.""" - if key not in _SEARCH_PREFERENCE_VALIDATABLE_KEYS: + if key not in SEARCH_PREFERENCE_VALIDATABLE_KEYS: return value, None if value is None: @@ -325,7 +329,7 @@ def _on_save_users(values: dict[str, object]) -> dict[str, object]: } values["REQUEST_POLICY_RULES"] = normalized_rules - for key in _SEARCH_PREFERENCE_VALIDATABLE_KEYS: + for key in SEARCH_PREFERENCE_VALIDATABLE_KEYS: if key not in values: continue normalized_value, validation_error = validate_search_preference_value(key, values[key]) diff --git a/shelfmark/core/admin_settings_routes.py b/shelfmark/core/admin_settings_routes.py index a5c2f0a7..b4d33905 100644 --- a/shelfmark/core/admin_settings_routes.py +++ b/shelfmark/core/admin_settings_routes.py @@ -11,7 +11,10 @@ from shelfmark.config.notifications_settings import ( is_valid_notification_url, normalize_notification_routes, ) -from shelfmark.config.users_settings import validate_search_preference_value +from shelfmark.config.users_settings import ( + SEARCH_PREFERENCE_VALIDATABLE_KEYS, + validate_search_preference_value, +) from shelfmark.core.config import config as app_config from shelfmark.core.request_policy import parse_policy_mode, validate_policy_rules from shelfmark.core.settings_registry import load_config_file @@ -91,14 +94,9 @@ def validate_user_settings( if search_validation_error: errors.append(search_validation_error) continue - if key in { - "SEARCH_MODE", - "BOOK_LANGUAGE", - "METADATA_PROVIDER", - "METADATA_PROVIDER_AUDIOBOOK", - "DEFAULT_RELEASE_SOURCE", - "DEFAULT_RELEASE_SOURCE_AUDIOBOOK", - }: + # Every key the search validator recognises keeps its normalized value; + # a hand-maintained subset here silently dropped normalization for the rest. + if key in SEARCH_PREFERENCE_VALIDATABLE_KEYS: valid[key] = normalized_search_value continue diff --git a/shelfmark/release_sources/prowlarr/handler.py b/shelfmark/release_sources/prowlarr/handler.py index 9ea51914..cf432ad7 100644 --- a/shelfmark/release_sources/prowlarr/handler.py +++ b/shelfmark/release_sources/prowlarr/handler.py @@ -297,10 +297,11 @@ class ProwlarrHandler(ExternalClientHandler): search_title=title, search_author=task.author, ) + # No language default here on purpose: this re-finds one exact release by its + # guid, and Prowlarr does not filter on plan.languages anyway. plan = build_release_search_plan( book, indexers=[indexer] if indexer is not None else None, - user_id=task.user_id, ) source = ProwlarrSource() diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index 2558edf1..65a572b7 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -87,6 +87,7 @@ import { bookSupportsTargets } from './utils/bookTargetLoader'; import { buildSearchQuery } from './utils/buildSearchQuery'; import { wasDownloadQueuedAfterResponseError } from './utils/downloadRecovery'; import { getDynamicOptionGroup } from './utils/dynamicFieldOptions'; +import { resolveDefaultLanguageCodes } from './utils/languageFilters'; import { getConfiguredMetadataProviderForContentType } from './utils/metadataProviders'; import { getEffectiveMetadataSort } from './utils/metadataSort'; import { isRecord } from './utils/objectHelpers'; @@ -1927,10 +1928,7 @@ function App() { ); const supportedFormats = config?.supported_formats || DEFAULT_SUPPORTED_FORMATS; const defaultLanguageCodes = useMemo( - () => - config?.default_language && config.default_language.length > 0 - ? config.default_language - : [bookLanguages[0]?.code || 'en'], + () => resolveDefaultLanguageCodes(config?.default_language, bookLanguages), [config?.default_language, bookLanguages], ); diff --git a/src/frontend/src/components/UrlSearchBootstrapMount.tsx b/src/frontend/src/components/UrlSearchBootstrapMount.tsx index 89591e38..68842527 100644 --- a/src/frontend/src/components/UrlSearchBootstrapMount.tsx +++ b/src/frontend/src/components/UrlSearchBootstrapMount.tsx @@ -3,6 +3,7 @@ import type { Dispatch, SetStateAction } from 'react'; import { useMountEffect } from '@/hooks/useMountEffect'; import type { AppConfig, AdvancedFilterState, ContentType, SearchMode, SortOption } from '@/types'; import { buildSearchQuery } from '@/utils/buildSearchQuery'; +import { resolveDefaultLanguageCodes } from '@/utils/languageFilters'; import { getEffectiveMetadataSort } from '@/utils/metadataSort'; import type { ParsedUrlSearch } from '@/utils/parseUrlSearchParams'; @@ -73,10 +74,10 @@ export const UrlSearchBootstrapMount = ({ } const bookLanguages = config.book_languages || []; - const defaultLanguageCodes = - config.default_language && config.default_language.length > 0 - ? config.default_language - : [bookLanguages[0]?.code || 'en']; + const defaultLanguageCodes = resolveDefaultLanguageCodes( + config.default_language, + bookLanguages, + ); if (parsedParams.searchInput) { setSearchInput(parsedParams.searchInput); diff --git a/src/frontend/src/components/settings/users/UserOverridesSection.tsx b/src/frontend/src/components/settings/users/UserOverridesSection.tsx index 20d497ec..aac46cd7 100644 --- a/src/frontend/src/components/settings/users/UserOverridesSection.tsx +++ b/src/frontend/src/components/settings/users/UserOverridesSection.tsx @@ -9,8 +9,8 @@ import { HeadingField, MultiSelectField, SelectField, TextField } from '../field import { FieldWrapper } from '../shared'; import { getFieldByKey, + resolveListOverride, toNormalizedLowercaseTextValue, - toStringListValue, toTextValue, } from './fieldHelpers'; import type { PerUserSettings } from './types'; @@ -180,12 +180,6 @@ export const UserOverridesSection = ({ label: 'Email Recipient', description: 'Email address used for this user in Email output mode.', }; - const browserDownloadGlobalValue = toStringListValue( - globalValues.DOWNLOAD_TO_BROWSER_CONTENT_TYPES, - ); - const browserDownloadUserValue = toStringListValue( - userSettings.DOWNLOAD_TO_BROWSER_CONTENT_TYPES, - ); const isOverridden = (key: DeliverySettingKey): boolean => { if ( @@ -201,10 +195,12 @@ export const UserOverridesSection = ({ return userValue !== globalValue; }; - const isBrowserDownloadOverridden = - Object.prototype.hasOwnProperty.call(userSettings, 'DOWNLOAD_TO_BROWSER_CONTENT_TYPES') && - userSettings.DOWNLOAD_TO_BROWSER_CONTENT_TYPES !== null && - JSON.stringify(browserDownloadUserValue) !== JSON.stringify(browserDownloadGlobalValue); + const { value: browserDownloadContentTypes, isOverridden: isBrowserDownloadOverridden } = + resolveListOverride( + userSettings.DOWNLOAD_TO_BROWSER_CONTENT_TYPES, + globalValues.DOWNLOAD_TO_BROWSER_CONTENT_TYPES, + Object.prototype.hasOwnProperty.call(userSettings, 'DOWNLOAD_TO_BROWSER_CONTENT_TYPES'), + ); const resetKeys = (keys: DeliverySettingKey[]) => { setUserSettings((prev) => { @@ -233,9 +229,6 @@ export const UserOverridesSection = ({ const outputModeValue = readValue('BOOKS_OUTPUT_MODE', 'folder'); const effectiveOutputMode = normalizeMode(outputModeValue); - const browserDownloadContentTypes = isBrowserDownloadOverridden - ? browserDownloadUserValue - : browserDownloadGlobalValue; const destinationValue = readValue('DESTINATION'); const destinationAudiobookValue = readValue('DESTINATION_AUDIOBOOK'); const libraryValue = readValue('BOOKLORE_LIBRARY_ID'); diff --git a/src/frontend/src/components/settings/users/UserSearchPreferencesSection.tsx b/src/frontend/src/components/settings/users/UserSearchPreferencesSection.tsx index 88183fe1..d7c7da01 100644 --- a/src/frontend/src/components/settings/users/UserSearchPreferencesSection.tsx +++ b/src/frontend/src/components/settings/users/UserSearchPreferencesSection.tsx @@ -8,8 +8,8 @@ import { HeadingField, MultiSelectField, SelectField } from '../fields'; import { FieldWrapper } from '../shared'; import { getFieldByKey, + resolveListOverride, toNormalizedLowercaseTextValue, - toStringListValue, toTextValue, } from './fieldHelpers'; import type { PerUserSettings } from './types'; @@ -141,15 +141,11 @@ export const UserSearchPreferencesSection = ({ ); const bookLanguageField = getFieldByKey(fields, 'BOOK_LANGUAGE', fallbackBookLanguageField); - const bookLanguageGlobalValue = toStringListValue(globalValues.BOOK_LANGUAGE); - const bookLanguageUserValue = toStringListValue(userSettings.BOOK_LANGUAGE); - const isBookLanguageOverridden = - Object.prototype.hasOwnProperty.call(userSettings, 'BOOK_LANGUAGE') && - userSettings.BOOK_LANGUAGE !== null && - JSON.stringify(bookLanguageUserValue) !== JSON.stringify(bookLanguageGlobalValue); - const bookLanguageValue = isBookLanguageOverridden - ? bookLanguageUserValue - : bookLanguageGlobalValue; + const { value: bookLanguageValue, isOverridden: isBookLanguageOverridden } = resolveListOverride( + userSettings.BOOK_LANGUAGE, + globalValues.BOOK_LANGUAGE, + Object.prototype.hasOwnProperty.call(userSettings, 'BOOK_LANGUAGE'), + ); const isOverridden = (key: SearchSettingKey): boolean => { if ( diff --git a/src/frontend/src/components/settings/users/fieldHelpers.ts b/src/frontend/src/components/settings/users/fieldHelpers.ts index 4e4649e6..f4d006f3 100644 --- a/src/frontend/src/components/settings/users/fieldHelpers.ts +++ b/src/frontend/src/components/settings/users/fieldHelpers.ts @@ -31,13 +31,33 @@ export const toNormalizedLowercaseTextValue = (value: unknown): string => { return toTrimmedTextValue(value).toLowerCase(); }; -export const toStringListValue = (value: unknown): string[] => { +const toStringListValue = (value: unknown): string[] => { if (!Array.isArray(value)) { return []; } return value.map((entry) => toTrimmedTextValue(entry)).filter((entry) => entry.length > 0); }; +/** + * Resolve a list-valued per-user override against its global value. + * + * A key absent from userSettings, or set to null, is not an override. A stored list + * that matches the global one is treated as inherited, matching how + * buildUserSettingsPayload clears it on save. + */ +export const resolveListOverride = ( + userValue: unknown, + globalValue: unknown, + hasUserKey: boolean, +): { value: string[]; isOverridden: boolean } => { + const globalList = toStringListValue(globalValue); + const userList = toStringListValue(userValue); + const isOverridden = + hasUserKey && userValue !== null && JSON.stringify(userList) !== JSON.stringify(globalList); + + return { value: isOverridden ? userList : globalList, isOverridden }; +}; + export const toComparableValue = (value: unknown): string => { if (value === null || value === undefined) { return ''; diff --git a/src/frontend/src/tests/languageFilters.test.ts b/src/frontend/src/tests/languageFilters.test.ts index 7e76dd8e..f05fcd17 100644 --- a/src/frontend/src/tests/languageFilters.test.ts +++ b/src/frontend/src/tests/languageFilters.test.ts @@ -7,6 +7,7 @@ import { buildLanguageNormalizer, getReleaseSearchLanguageParams, releaseLanguageMatchesFilter, + resolveDefaultLanguageCodes, } from '../utils/languageFilters'; const supportedLanguages: Language[] = [ @@ -67,3 +68,36 @@ describe('releaseLanguageMatchesFilter', () => { expect(visibleLanguages).toHaveLength(48); }); }); + +describe('resolveDefaultLanguageCodes', () => { + it('keeps an explicitly empty default as "no default filter"', () => { + // The backend stores [] to mean "do not filter"; substituting the first + // supported language here would make the UI filter where the server does not. + expect(resolveDefaultLanguageCodes([], supportedLanguages)).toEqual([]); + }); + + it('leaves a configured default untouched', () => { + expect(resolveDefaultLanguageCodes(['de', 'hu'], supportedLanguages)).toEqual(['de', 'hu']); + }); + + it('falls back to the first supported language only when nothing is configured', () => { + expect(resolveDefaultLanguageCodes(undefined, supportedLanguages)).toEqual(['en']); + expect(resolveDefaultLanguageCodes(null, [])).toEqual(['en']); + }); + + it('sends no language filter when an empty default is the whole selection', () => { + const defaults = resolveDefaultLanguageCodes([], supportedLanguages); + + expect( + getReleaseSearchLanguageParams([LANGUAGE_OPTION_DEFAULT], supportedLanguages, defaults), + ).toBe(undefined); + }); + + it('does not smuggle the first language into a Default+German selection', () => { + const defaults = resolveDefaultLanguageCodes([], supportedLanguages); + + expect( + getReleaseSearchLanguageParams([LANGUAGE_OPTION_DEFAULT, 'de'], supportedLanguages, defaults), + ).toEqual(['de']); + }); +}); diff --git a/src/frontend/src/utils/languageFilters.ts b/src/frontend/src/utils/languageFilters.ts index c7ec79c8..fba08b51 100644 --- a/src/frontend/src/utils/languageFilters.ts +++ b/src/frontend/src/utils/languageFilters.ts @@ -27,6 +27,24 @@ export const normalizeLanguageSelection = (selected: string[]): string[] => { return unique.length ? unique : [LANGUAGE_OPTION_DEFAULT]; }; +/** + * Resolve the language codes the "Default" filter option stands for. + * + * An explicitly empty list is a deliberate "no default filter" and is returned as-is; + * only a missing value falls back to the first supported language. Substituting a + * language for the empty list would make the UI filter by a language the backend + * does not apply. + */ +export const resolveDefaultLanguageCodes = ( + configuredDefault: string[] | null | undefined, + supportedLanguages: Language[], +): string[] => { + if (Array.isArray(configuredDefault)) { + return configuredDefault; + } + return [supportedLanguages[0]?.code || 'en']; +}; + export const getLanguageFilterValues = ( selection: string[], supportedLanguages: Language[], diff --git a/tests/config/test_search_mode_settings.py b/tests/config/test_search_mode_settings.py index 17c29f86..b6699297 100644 --- a/tests/config/test_search_mode_settings.py +++ b/tests/config/test_search_mode_settings.py @@ -1,8 +1,17 @@ """Tests for search mode settings definitions.""" +import json + +import pytest + from shelfmark.config.settings import search_mode_settings +def _search_mode_field(key: str): + fields = {field.key: field for field in search_mode_settings() if hasattr(field, "key")} + return fields[key] + + def test_search_mode_settings_include_release_source_links_toggle(): fields = {field.key: field for field in search_mode_settings() if hasattr(field, "key")} @@ -21,3 +30,47 @@ def test_book_language_is_user_overridable(): assert field.label == "Default Book Languages" assert field.default == ["en"] assert field.user_overridable is True + + +def test_book_language_stored_under_the_general_tab_still_resolves(tmp_path): + """BOOK_LANGUAGE moved from the General tab to Search Mode with no migration. + + That is only safe because both tabs persist into the same settings.json, so an + install that stored the value while the field lived on General keeps it. If the + two tabs ever get separate files, every existing install silently falls back to + the ["en"] default instead. + """ + from shelfmark.core.settings_registry import _get_config_file_path, get_setting_value + + (tmp_path / "settings.json").write_text(json.dumps({"BOOK_LANGUAGE": ["de", "fr"]})) + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.delenv("BOOK_LANGUAGE", raising=False) + monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", tmp_path) + + assert _get_config_file_path("search_mode") == _get_config_file_path("general") + assert get_setting_value(_search_mode_field("BOOK_LANGUAGE"), "search_mode") == ["de", "fr"] + + +def test_book_language_uses_its_default_on_a_fresh_install(tmp_path): + from shelfmark.core.settings_registry import get_setting_value + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.delenv("BOOK_LANGUAGE", raising=False) + monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", tmp_path) + + assert get_setting_value(_search_mode_field("BOOK_LANGUAGE"), "search_mode") == ["en"] + + +def test_book_language_env_var_beats_the_stored_value(tmp_path): + from shelfmark.core.settings_registry import get_setting_value, is_value_from_env + + (tmp_path / "settings.json").write_text(json.dumps({"BOOK_LANGUAGE": ["de", "fr"]})) + field = _search_mode_field("BOOK_LANGUAGE") + + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setenv("BOOK_LANGUAGE", "es,it") + monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", tmp_path) + + assert is_value_from_env(field) is True + assert get_setting_value(field, "search_mode") == ["es", "it"] diff --git a/tests/config/test_users_settings.py b/tests/config/test_users_settings.py index 8b77b9a5..38e5ffd0 100644 --- a/tests/config/test_users_settings.py +++ b/tests/config/test_users_settings.py @@ -90,7 +90,7 @@ def test_visible_self_settings_sections_field_defaults_and_options(): { "value": "search", "label": "Search Preferences", - "description": "Show personal search mode and provider settings.", + "description": "Show personal search mode, language, and provider settings.", }, { "value": "notifications", diff --git a/tests/core/test_admin_users_api.py b/tests/core/test_admin_users_api.py index ee6d00a4..4dd27745 100644 --- a/tests/core/test_admin_users_api.py +++ b/tests/core/test_admin_users_api.py @@ -534,6 +534,46 @@ class TestAdminUserUpdateEndpoint: ) assert user_db.get_user_settings(user["id"]) == {} + def test_update_user_settings_skips_blank_book_languages(self, admin_client, user_db): + """A trailing comma or a blank slot means "nothing there", not an unknown language.""" + user = user_db.create_user(username="alice") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={"settings": {"BOOK_LANGUAGE": "en,"}}, + ) + assert resp.status_code == 200 + assert user_db.get_user_settings(user["id"])["BOOK_LANGUAGE"] == ["en"] + + def test_update_user_settings_normalizes_every_validated_search_key( + self, admin_client, user_db + ): + """Keys the search validator recognises keep their normalized value. + + These three were validated but then stored raw, because the write-back was + gated on a hand-maintained subset of the validated keys. A padded provider + name was accepted and then persisted with its padding, so every later lookup + of it failed. + """ + user = user_db.create_user(username="alice") + + resp = admin_client.put( + f"/api/admin/users/{user['id']}", + json={ + "settings": { + "METADATA_PROVIDER_COMBINED": " openlibrary ", + "SHOW_COMBINED_SELECTOR": "yes", + "FORCE_COMBINED_SEARCH": "", + } + }, + ) + assert resp.status_code == 200 + + settings = user_db.get_user_settings(user["id"]) + assert settings["METADATA_PROVIDER_COMBINED"] == "openlibrary" + assert settings["SHOW_COMBINED_SELECTOR"] is True + assert settings["FORCE_COMBINED_SEARCH"] is False + def test_update_user_settings_accepts_notification_overrides(self, admin_client, user_db): user = user_db.create_user(username="alice")