From 65e3325e514be32f16e46837eb8d61a00a6793ec Mon Sep 17 00:00:00 2001 From: Roland Date: Wed, 7 Oct 2026 17:06:01 -0600 Subject: [PATCH] Add Playlists to the Other search tab Domains and Playlists are Collections, so the Other tab searches both with the existing /api/collections/search (by kind) instead of a Domain-only endpoint. The search and the estimate share search_collections_query. The search now loads each Collection's Downloads up front instead of once per Collection. Co-Authored-By: Claude Opus 5.5 --- app/src/api.js | 10 ++-- app/src/components/OtherSearchView.test.js | 47 ++++++++++------ app/src/components/Search.js | 26 ++++++--- app/src/hooks/customHooks.js | 28 ++++++---- modules/archive/api.py | 11 ---- modules/archive/lib.py | 27 +-------- modules/archive/schema.py | 6 -- modules/archive/test/test_domain_search.py | 46 ---------------- modules/repos/test/test_search.py | 6 +- wrolpi/collections/lib.py | 55 ++++++++----------- .../test/test_collection_search.py | 47 ++++++++++++++++ wrolpi/common.py | 18 ++++-- 12 files changed, 160 insertions(+), 167 deletions(-) delete mode 100644 modules/archive/test/test_domain_search.py create mode 100644 wrolpi/collections/test/test_collection_search.py diff --git a/app/src/api.js b/app/src/api.js index 3d8a6488..8b2a421b 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -2129,13 +2129,13 @@ export async function searchChannels(tagNames, searchStr) { } } -// Domains whose name matches `searchStr` and/or which have any of the tags. -export async function searchDomains(tagNames, searchStr) { - const body = {tag_names: tagNames, search_str: searchStr}; - const response = await apiPost(`${ARCHIVES_API}/domains/search`, body); +// Collections of `kind` whose name matches `searchStr` and/or which have any of the tags. +export async function searchCollections(kind, tagNames, searchStr) { + const body = {kind, tag_names: tagNames, search_str: searchStr}; + const response = await apiPost(`${COLLECTIONS_API}/search`, body); if (response.ok) { const content = await response.json(); - return {domains: content.domains}; + return {collections: content.collections}; } } diff --git a/app/src/components/OtherSearchView.test.js b/app/src/components/OtherSearchView.test.js index 468a6000..c4de3dc9 100644 --- a/app/src/components/OtherSearchView.test.js +++ b/app/src/components/OtherSearchView.test.js @@ -1,7 +1,7 @@ import React from 'react'; import {act, render, screen} from '../test-utils'; import {OtherSearchView} from './Search'; -import {searchChannels, searchDomains, searchRepos} from '../api'; +import {searchChannels, searchCollections, searchRepos} from '../api'; // What the search page's Other tab shows for each state of useSearchChannels. "No Channels" // must only appear once the server has confirmed there are none. @@ -9,7 +9,7 @@ import {searchChannels, searchDomains, searchRepos} from '../api'; jest.mock('../api', () => ({ ...jest.requireActual('../api'), searchChannels: jest.fn(), - searchDomains: jest.fn(), + searchCollections: jest.fn(), searchRepos: jest.fn(), })); @@ -19,8 +19,8 @@ beforeEach(() => { searchChannels.mockReset(); searchRepos.mockReset(); searchRepos.mockResolvedValue({repos: [], total: 0}); - searchDomains.mockReset(); - searchDomains.mockResolvedValue({domains: []}); + searchCollections.mockReset(); + searchCollections.mockResolvedValue({collections: []}); }); test('pending: shows a loader, not "No Channels"', () => { @@ -121,39 +121,54 @@ describe('Repos', () => { }); }); -describe('Domains', () => { - test('empty: shows "No Domains"', async () => { +// Domains and Playlists are Collections searched by kind. +describe.each([ + ['domain', 'Domains', 'example.com', '/archives?domain=example.com'], + ['playlist', 'Playlists', 'Road Trip', '/playlists/3'], +])('%s Collections', (kind, title, name, href) => { + const collectionsOf = (wanted, collections) => (k) => Promise.resolve({collections: k === wanted ? collections : []}); + + test(`empty: shows "No ${title}"`, async () => { + searchChannels.mockResolvedValue({channels: []}); + render(, {query: {searchParams: new URLSearchParams('q=nothing')}}); + await act(async () => { + }); + expect(screen.getByText(`No ${title}`)).toBeInTheDocument(); + }); + + test('nothing to search by: shows none without fetching', async () => { searchChannels.mockResolvedValue({channels: []}); render(); await act(async () => { }); - expect(screen.getByText('No Domains')).toBeInTheDocument(); + expect(screen.getByText(`No ${title}`)).toBeInTheDocument(); + expect(searchCollections).not.toHaveBeenCalled(); }); - test('failed: shows an error only in the Domains section', async () => { + test(`failed: shows an error only in the ${title} section`, async () => { searchChannels.mockResolvedValue({channels: [{id: 1, name: 'Cooking'}]}); - searchDomains.mockRejectedValue(new Error('boom')); + searchCollections.mockImplementation(k => k === kind ? Promise.reject(new Error('boom')) : Promise.resolve({collections: []})); const spy = jest.spyOn(console, 'error').mockImplementation(() => { }); try { - render(); + render(, {query: {searchParams: new URLSearchParams('q=x')}}); await act(async () => { }); expect(screen.getByText('Cooking')).toBeInTheDocument(); - expect(screen.getByText(/Could not fetch the domains/)).toBeInTheDocument(); + expect(screen.getByText(new RegExp(`Could not fetch the ${title.toLowerCase()}`))).toBeInTheDocument(); } finally { spy.mockRestore(); } }); - test('loaded: lists domains by the search, linked to their archives', async () => { + test('loaded: lists them by the search and tags, linked', async () => { searchChannels.mockResolvedValue({channels: []}); - searchDomains.mockResolvedValue({domains: [{id: 3, domain: 'example.com', tag_name: null}]}); - const searchParams = new URLSearchParams('q=example&tag=News'); + searchCollections.mockImplementation(collectionsOf(kind, [{id: 3, name, kind, tag_name: null}])); + const searchParams = new URLSearchParams('q=road&tag=News'); render(, {query: {searchParams}}); await act(async () => { }); - expect(searchDomains).toHaveBeenCalledWith(['News'], 'example'); - expect(screen.getByText('example.com').closest('a')).toHaveAttribute('href', '/archives?domain=example.com'); + expect(searchCollections).toHaveBeenCalledWith(kind, ['News'], 'road'); + expect(screen.getByText(name).closest('a')).toHaveAttribute('href', href); }); }); diff --git a/app/src/components/Search.js b/app/src/components/Search.js index 5358336c..6d76b1c7 100644 --- a/app/src/components/Search.js +++ b/app/src/components/Search.js @@ -8,7 +8,7 @@ import { useSearchDate, useSearchDeep, useSearchFilter, - useSearchDomains, + useSearchCollections, useSearchRepos, } from "../hooks/customHooks"; import {ShortcutHint} from "./ShortcutHint"; @@ -563,12 +563,12 @@ function SearchChannelPreview({channel}) { } -function SearchDomainPreview({domain}) { +function SearchCollectionPreview({collection, to}) { const {SingleTag} = React.useContext(TagsContext); return
- {domain.domain} - {domain.tag_name && } + {collection.name} + {collection.tag_name && }
} @@ -605,15 +605,16 @@ function OtherSearchSection({value, title, items, preview, emptyText, errorText} } -// `channels`, `domains`, and `repos` follow useSearchChannels, useSearchDomains, and useSearchRepos: null = pending, +// Each section's items follow useSearchChannels, useSearchCollections, or useSearchRepos: null = pending, // undefined = fetch failed, [] = none. export function OtherSearchView({loading}) { const {searchParams} = React.useContext(QueryContext); - const [activeValues, setActiveValues] = React.useState(['channels', 'domains', 'repos']); + const [activeValues, setActiveValues] = React.useState(['channels', 'domains', 'playlists', 'repos']); const activeTags = searchParams.getAll('tag'); const searchStr = searchParams.get('q') || ''; const {channels, loading: channelsLoading} = useSearchChannels(activeTags, searchStr); - const {domains} = useSearchDomains(searchStr, activeTags); + const {collections: domains} = useSearchCollections('domain', searchStr, activeTags); + const {collections: playlists} = useSearchCollections('playlist', searchStr, activeTags); const {repos} = useSearchRepos(searchStr, activeTags); if (loading || channelsLoading || channels === null) { @@ -635,10 +636,19 @@ export function OtherSearchView({loading}) { value='domains' title='Domains' items={domains} - preview={i => } + preview={i => } emptyText='No Domains' errorText='Could not fetch the domains.' /> + } + emptyText='No Playlists' + errorText='Could not fetch the playlists.' + /> { } } -// Domains whose name matches `searchStr` and/or which have any of the tags. `domains`: null = pending, -// undefined = fetch failed, [] = none. Only the newest request may write state. -export const useSearchDomains = (searchStr, tagNames) => { - const [domains, setDomains] = useState(null); +// Collections of `kind` whose name matches `searchStr` and/or which have any of the tags. `collections`: +// null = pending, undefined = fetch failed, [] = none. Nothing to search by matches nothing (the API would list +// every Collection of the kind). Only the newest request may write state. +export const useSearchCollections = (kind, searchStr, tagNames) => { + const [collections, setCollections] = useState(null); const requestGen = useRef(0); const tagKey = JSON.stringify(tagNames || []); useEffect(() => { const gen = ++requestGen.current; - setDomains(null); - searchDomains(JSON.parse(tagKey), searchStr) - .then(result => gen === requestGen.current && setDomains(result ? result.domains : undefined)) + const tags = JSON.parse(tagKey); + if (!(searchStr || '').trim() && _.isEmpty(tags)) { + setCollections([]); + return; + } + setCollections(null); + searchCollections(kind, tags, searchStr) + .then(result => gen === requestGen.current && setCollections(result ? result.collections : undefined)) .catch(e => { console.error(e); - if (gen === requestGen.current) setDomains(undefined); + if (gen === requestGen.current) setCollections(undefined); }); - }, [searchStr, tagKey]); + }, [kind, searchStr, tagKey]); - return {domains}; + return {collections}; } // Repos matching the search (their name and README) and/or the tags. `repos`: null = pending, diff --git a/modules/archive/api.py b/modules/archive/api.py index 01647d84..ee4db91b 100644 --- a/modules/archive/api.py +++ b/modules/archive/api.py @@ -68,17 +68,6 @@ async def search_archives(request: Request, body: schema.ArchiveSearchRequest): return json_response(ret) -@archive_bp.post('/domains/search') -@openapi.definition( - description='Search Domains by their name and/or Tags', - body=schema.DomainSearchRequest, -) -@validate(schema.DomainSearchRequest) -async def domain_search(request: Request, body: schema.DomainSearchRequest): - domains = lib.search_domains(request.ctx.session, body.tag_names, body.search_str) - return json_response(dict(domains=domains)) - - @archive_bp.post('/file_format') @openapi.definition( description='Preview the archive file format', diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 495de67a..e5562577 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -16,7 +16,7 @@ from urllib.parse import urlparse import pytz from sqlalchemy import asc, func, BigInteger -from sqlalchemy.orm import Query, Session +from sqlalchemy.orm import Session from modules.archive.models import Archive from wrolpi import dates @@ -34,7 +34,7 @@ from wrolpi.files.lib import handle_file_group_search_results, cached_search_tot search_filter_cache_key, count_file_groups from wrolpi.files.models import FileGroup from wrolpi.switches import register_switch_handler, ActivateSwitchMethod -from wrolpi.tags import Tag, tag_append_sub_select_where +from wrolpi.tags import tag_append_sub_select_where from wrolpi.vars import PYTEST, DOCKERIZED logger = logger.getChild(__name__) @@ -1555,29 +1555,6 @@ async def search_domains_by_name(session: Session, name: str, limit: int = 5) -> ] -def search_domains_query(session: Session, tag_names: List[str] = None, search_str: str = None) -> Optional[Query]: - """Domains whose name contains `search_str` and which have any of the Tags. None when there is nothing to search - by.""" - search_str = (search_str or '').strip() - if not tag_names and not search_str: - return None - query = session.query(Collection).filter(Collection.kind == 'domain') - if search_str: - query = query.filter(Collection.name.ilike(f'%{search_str}%')) - if tag_names: - query = query.join(Tag, Tag.id == Collection.tag_id).filter(Tag.name.in_(tag_names)) - return query - - -def search_domains(session: Session, tag_names: List[str] = None, search_str: str = None) -> List[dict]: - """Search Domains by their name and/or Tags.""" - query = search_domains_query(session, tag_names, search_str) - if query is None: - return [] - collections = query.order_by(func.lower(Collection.name)).all() - return [dict(id=i.id, domain=i.name, tag_name=i.tag_name) for i in collections] - - async def html_to_readability(html: str | bytes, url: str, timeout: int = 120): """Extract the readability dict from the provided HTML.""" with tempfile.NamedTemporaryFile('wb', suffix='.html') as singlefile_file: diff --git a/modules/archive/schema.py b/modules/archive/schema.py index edd0d6e1..b963a545 100644 --- a/modules/archive/schema.py +++ b/modules/archive/schema.py @@ -27,12 +27,6 @@ class ArchiveSearchRequest: deep: bool = False # When True, search all text including page contents (d_text) -@dataclass -class DomainSearchRequest: - tag_names: List[str] = field(default_factory=list) - search_str: Optional[str] = None - - @dataclass class ArchiveSearchResponse: file_groups: List[ArchiveDict] diff --git a/modules/archive/test/test_domain_search.py b/modules/archive/test/test_domain_search.py deleted file mode 100644 index 800c10ec..00000000 --- a/modules/archive/test/test_domain_search.py +++ /dev/null @@ -1,46 +0,0 @@ -from http import HTTPStatus - -import pytest - -from wrolpi.collections import Collection - - -@pytest.mark.asyncio -async def test_search_domains(async_client, test_session, tag_factory): - """Domains can be searched by their name and/or Tags, and the Other tab's estimate counts the same Domains.""" - tag1, tag2 = await tag_factory(), await tag_factory() - test_session.add_all([ - Collection(name='cooking.example.com', kind='domain', tag=tag1), - Collection(name='cooking.example.org', kind='domain'), - Collection(name='gardening.example.com', kind='domain', tag=tag2), - # Not a Domain. - Collection(name='cooking channel', kind='channel', tag=tag1), - ]) - test_session.commit() - - async def search(body): - request, response = await async_client.post('/api/archive/domains/search', json=body) - assert response.status_code == HTTPStatus.OK - return sorted(i['domain'] for i in response.json['domains']) - - cases = [ - (dict(search_str='cooking'), ['cooking.example.com', 'cooking.example.org']), - (dict(tag_names=[tag1.name]), ['cooking.example.com']), - # Any of the Tags. - (dict(tag_names=[tag1.name, tag2.name]), ['cooking.example.com', 'gardening.example.com']), - # The name and the Tags must both match. - (dict(search_str='example.com', tag_names=[tag2.name]), ['gardening.example.com']), - (dict(search_str='cooking', tag_names=[tag2.name]), []), - # Nothing to search by matches nothing. - (dict(search_str=''), []), - (dict(), []), - ] - for body, expected in cases: - assert await search(body) == expected, body - request, response = await async_client.post('/api/search_other_estimates', json=body) - assert response.status_code == HTTPStatus.OK - assert response.json['others']['domain_count'] == len(expected), body - - request, response = await async_client.post('/api/archive/domains/search', json=dict(tag_names=[tag1.name])) - domain, = response.json['domains'] - assert domain['tag_name'] == tag1.name diff --git a/modules/repos/test/test_search.py b/modules/repos/test/test_search.py index bd758690..9ff59f0d 100644 --- a/modules/repos/test/test_search.py +++ b/modules/repos/test/test_search.py @@ -116,13 +116,13 @@ async def test_search_api(test_session, async_client, test_wrolpi_config): # The global search's "Other" estimate counts Repos by the search, and by Tag. request, response = await async_client.post('/api/search_other_estimates', content=json.dumps(body)) - assert response.json['others'] == dict(channel_count=0, domain_count=0, repo_count=1) + assert response.json['others'] == dict(channel_count=0, domain_count=0, playlist_count=0, repo_count=1) body = dict(tag_names=['software']) request, response = await async_client.post('/api/search_other_estimates', content=json.dumps(body)) - assert response.json['others'] == dict(channel_count=0, domain_count=0, repo_count=1) + assert response.json['others'] == dict(channel_count=0, domain_count=0, playlist_count=0, repo_count=1) body = dict(tag_names=['software'], search_str='nothing') request, response = await async_client.post('/api/search_other_estimates', content=json.dumps(body)) - assert response.json['others'] == dict(channel_count=0, domain_count=0, repo_count=0) + assert response.json['others'] == dict(channel_count=0, domain_count=0, playlist_count=0, repo_count=0) # Repos are suggested by name. request, response = await async_client.post('/api/search_suggestions', content=json.dumps(dict(search_str='kiw'))) diff --git a/wrolpi/collections/lib.py b/wrolpi/collections/lib.py index 08cc055f..d18d77f0 100644 --- a/wrolpi/collections/lib.py +++ b/wrolpi/collections/lib.py @@ -8,7 +8,7 @@ import pathlib from typing import List, Optional, Dict from sqlalchemy import func -from sqlalchemy.orm import Session, selectinload, noload +from sqlalchemy.orm import Query, Session, selectinload, noload from wrolpi import flags from wrolpi.common import get_media_directory, logger, get_relative_to_media_directory, TRACE_LEVEL, background_task @@ -871,42 +871,35 @@ def delete_collection( return collection_dict +def search_collections_query( + session: Session, + kind: Optional[str] = None, + tag_names: Optional[List[str]] = None, + search_str: Optional[str] = None, +) -> Query: + """Collections of `kind` whose name contains `search_str` and which have any of the Tags. A missing filter + matches every Collection.""" + query = session.query(Collection) + if kind: + query = query.filter(Collection.kind == kind) + if tag_names: + query = query.join(Tag, Tag.id == Collection.tag_id).filter(Tag.name.in_(tag_names)) + if search_str: + query = query.filter(Collection.name.ilike(f'%{search_str}%')) + return query + + def search_collections( session: Session, kind: Optional[str] = None, tag_names: Optional[List[str]] = None, search_str: Optional[str] = None, ) -> List[dict]: - """ - Search collections by kind, tags, and search string. - - Args: - session: Database session - kind: Optional collection kind filter - tag_names: Optional list of tag names to filter by - search_str: Optional search string for collection names - - Returns: - List of matching collection dicts - """ - query = session.query(Collection) - - # Filter by kind - if kind: - query = query.filter(Collection.kind == kind) - - # Filter by tags - if tag_names: - query = query.join(Tag).filter(Tag.name.in_(tag_names)) - - # Filter by search string - if search_str: - query = query.filter(Collection.name.ilike(f'%{search_str}%')) - - # Order by name - query = query.order_by(Collection.name) - - collections = query.all() + """Search Collections by kind, Tags, and name (see `search_collections_query`).""" + collections = search_collections_query(session, kind, tag_names, search_str) \ + .options(*Collection.json_options()) \ + .order_by(Collection.name) \ + .all() return [collection.__json__() for collection in collections] diff --git a/wrolpi/collections/test/test_collection_search.py b/wrolpi/collections/test/test_collection_search.py new file mode 100644 index 00000000..bbd3709a --- /dev/null +++ b/wrolpi/collections/test/test_collection_search.py @@ -0,0 +1,47 @@ +from http import HTTPStatus + +import pytest + +from wrolpi.collections import Collection + + +@pytest.mark.asyncio +@pytest.mark.parametrize('kind', ['domain', 'playlist']) +async def test_search_collections_other_estimate(async_client, test_session, tag_factory, kind): + """Collections can be searched by kind, name, and/or Tags; the Other tab's estimate counts the same + Collections.""" + tag1, tag2 = await tag_factory(), await tag_factory() + test_session.add_all([ + Collection(name='cooking one', kind=kind, tag=tag1), + Collection(name='cooking two', kind=kind), + Collection(name='gardening', kind=kind, tag=tag2), + # Another kind is never counted. + Collection(name='cooking other', kind='channel' if kind == 'domain' else 'domain', tag=tag1), + ]) + test_session.commit() + + async def search(body): + request, response = await async_client.post('/api/collections/search', json=dict(body, kind=kind)) + assert response.status_code == HTTPStatus.OK + return sorted(i['name'] for i in response.json['collections']) + + cases = [ + (dict(search_str='cooking'), ['cooking one', 'cooking two']), + (dict(tag_names=[tag1.name]), ['cooking one']), + # Any of the Tags. + (dict(tag_names=[tag1.name, tag2.name]), ['cooking one', 'gardening']), + # The name and the Tags must both match. + (dict(search_str='garden', tag_names=[tag2.name]), ['gardening']), + (dict(search_str='cooking', tag_names=[tag2.name]), []), + ] + count_key = f'{kind}_count' + for body, expected in cases: + assert await search(body) == expected, body + request, response = await async_client.post('/api/search_other_estimates', json=body) + assert response.status_code == HTTPStatus.OK + assert response.json['others'][count_key] == len(expected), body + + # Nothing to search by: the Other tab shows none (the search endpoint lists every Collection of the kind). + for body in (dict(search_str=''), dict(search_str=' '), dict()): + request, response = await async_client.post('/api/search_other_estimates', json=body) + assert response.json['others'][count_key] == 0, body diff --git a/wrolpi/common.py b/wrolpi/common.py index fedb9cd3..11bd80c9 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -2870,17 +2870,25 @@ def extract_headlines(entries: List[str], search_str: str) -> List[Tuple[str, fl async def search_other_estimates(session: Session, tag_names: List[str], search_str: str = None) -> dict: - """Estimate other things that match the search or the Tags (Channels and Domains by their name, Repos by their - name and README).""" - from modules.archive.lib import search_domains_query + """Estimate other things that match the search or the Tags (Channels, Domains, and Playlists by their name, Repos + by their name and README).""" from modules.videos.channel.lib import search_channels_query from modules.repos.lib import count_repos + from wrolpi.collections.lib import search_collections_query + search_str = (search_str or '').strip() channels = search_channels_query(session, tag_names, search_str) - domains = search_domains_query(session, tag_names, search_str) + + def count_collections(kind: str) -> int: + # Nothing to search by matches nothing, like the other searches. + if not tag_names and not search_str: + return 0 + return search_collections_query(session, kind, tag_names, search_str).count() + others = dict( channel_count=channels.count() if channels is not None else 0, - domain_count=domains.count() if domains is not None else 0, + domain_count=count_collections('domain'), + playlist_count=count_collections('playlist'), repo_count=count_repos(session, search_str, tag_names), ) return others