From 04f26d5cfc9280d705fc21e417bf0c1c0465d61a Mon Sep 17 00:00:00 2001 From: Lukas Date: Fri, 23 Jan 2026 19:11:41 +0100 Subject: [PATCH] fix: Correct series filter logic for no_episodes Critical bug fix: The filter was returning the wrong series because of a misunderstanding of the episode table semantics. ISSUE: - Episodes table contains MISSING episodes (from episodeDict) - is_downloaded=False means episode file not found in folder - Original query logic was backwards - returned series with NO missing episodes instead of series WITH missing episodes SOLUTION: - Simplified query to directly check for episodes with is_downloaded=False - Changed from complex join with count aggregation to simple subquery - Now correctly returns series that have at least one undownloaded episode CHANGES: - src/server/database/service.py: Rewrote get_series_with_no_episodes() method with corrected logic and clearer documentation - tests/unit/test_series_filter.py: Updated test expectations to match corrected behavior with detailed comments explaining episode semantics - docs/API.md: Enhanced documentation explaining filter behavior and episode table meaning TESTS: All 5 unit tests pass with corrected logic --- docs/API.md | 116 +++++++++++++++++-------------- src/server/api/anime.py | 6 +- src/server/database/service.py | 41 +++++------ tests/unit/test_series_filter.py | 107 ++++++++++++++++++++++------ 4 files changed, 165 insertions(+), 105 deletions(-) diff --git a/docs/API.md b/docs/API.md index 40b4211..2f2bff4 100644 --- a/docs/API.md +++ b/docs/API.md @@ -34,11 +34,11 @@ Authorization: Bearer **Public Endpoints (no authentication required):** -- `/api/auth/*` - Authentication endpoints -- `/api/health` - Health check endpoints -- `/api/docs`, `/api/redoc` - API documentation -- `/static/*` - Static files -- `/`, `/login`, `/setup`, `/queue` - UI pages +- `/api/auth/*` - Authentication endpoints +- `/api/health` - Health check endpoints +- `/api/docs`, `/api/redoc` - API documentation +- `/static/*` - Static files +- `/`, `/login`, `/setup`, `/queue` - UI pages Source: [src/server/middleware/auth.py](../src/server/middleware/auth.py#L39-L52) @@ -91,7 +91,7 @@ Initial setup endpoint to configure the master password. Can only be called once **Errors:** -- `400 Bad Request` - Master password already configured or invalid password +- `400 Bad Request` - Master password already configured or invalid password Source: [src/server/api/auth.py](../src/server/api/auth.py#L28-L90) @@ -120,8 +120,8 @@ Validate master password and return JWT token. **Errors:** -- `401 Unauthorized` - Invalid credentials -- `429 Too Many Requests` - Account locked due to failed attempts +- `401 Unauthorized` - Invalid credentials +- `429 Too Many Requests` - Account locked due to failed attempts Source: [src/server/api/auth.py](../src/server/api/auth.py#L93-L124) @@ -203,7 +203,14 @@ List library series that have missing episodes. | `page` | int | 1 | Page number (must be positive) | | `per_page` | int | 20 | Items per page (max 1000) | | `sort_by` | string | null | Sort field: `title`, `id`, `name`, `missing_episodes` | -| `filter` | string | null | Filter: `no_episodes` (shows only series with no downloaded episodes in folder) | +| `filter` | string | null | Filter: `no_episodes` (shows only series with missing episodes - episodes in DB that haven't been downloaded yet) | + +**Filter Details:** + +- `no_episodes`: Returns series that have at least one episode in the database with `is_downloaded=False` +- Episodes in the database represent MISSING episodes (from episodeDict during scanning) +- `is_downloaded=False` means the episode file was not found in the folder +- This effectively shows series where no video files were found for missing episodes **Response (200 OK):** @@ -221,6 +228,7 @@ List library series that have missing episodes. ``` **Example with filter:** + ```bash GET /api/anime?filter=no_episodes ``` @@ -311,10 +319,10 @@ Add a new series to the library with automatic database persistence, folder crea **Folder Name Sanitization:** -- Removes invalid filesystem characters: `< > : " / \ | ? *` -- Trims leading/trailing whitespace and dots -- Preserves Unicode characters (for Japanese titles) -- Example: `"Attack on Titan: Final Season"` → `"Attack on Titan Final Season"` +- Removes invalid filesystem characters: `< > : " / \ | ? *` +- Trims leading/trailing whitespace and dots +- Preserves Unicode characters (for Japanese titles) +- Example: `"Attack on Titan: Final Season"` → `"Attack on Titan Final Season"` Source: [src/server/api/anime.py](../src/server/api/anime.py#L604-L710) @@ -819,8 +827,8 @@ These endpoints manage tvshow.nfo metadata files and associated media (poster, l **Prerequisites:** -- TMDB API key must be configured in settings -- NFO service returns 503 if API key not configured +- TMDB API key must be configured in settings +- NFO service returns 503 if API key not configured ### GET /api/nfo/{serie_id}/check @@ -830,7 +838,7 @@ Check if NFO file and media files exist for a series. **Path Parameters:** -- `serie_id` (string): Series identifier +- `serie_id` (string): Series identifier **Response (200 OK):** @@ -853,9 +861,9 @@ Check if NFO file and media files exist for a series. **Errors:** -- `401 Unauthorized` - Not authenticated -- `404 Not Found` - Series not found -- `503 Service Unavailable` - TMDB API key not configured +- `401 Unauthorized` - Not authenticated +- `404 Not Found` - Series not found +- `503 Service Unavailable` - TMDB API key not configured Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L90-L147) @@ -867,7 +875,7 @@ Create NFO file and download media for a series. **Path Parameters:** -- `serie_id` (string): Series identifier +- `serie_id` (string): Series identifier **Request Body:** @@ -884,12 +892,12 @@ Create NFO file and download media for a series. **Fields:** -- `serie_name` (string, optional): Series name for TMDB search (defaults to folder name) -- `year` (integer, optional): Series year to help narrow TMDB search -- `download_poster` (boolean, default: true): Download poster.jpg -- `download_logo` (boolean, default: true): Download logo.png -- `download_fanart` (boolean, default: true): Download fanart.jpg -- `overwrite_existing` (boolean, default: false): Overwrite existing NFO +- `serie_name` (string, optional): Series name for TMDB search (defaults to folder name) +- `year` (integer, optional): Series year to help narrow TMDB search +- `download_poster` (boolean, default: true): Download poster.jpg +- `download_logo` (boolean, default: true): Download logo.png +- `download_fanart` (boolean, default: true): Download fanart.jpg +- `overwrite_existing` (boolean, default: false): Overwrite existing NFO **Response (200 OK):** @@ -912,10 +920,10 @@ Create NFO file and download media for a series. **Errors:** -- `401 Unauthorized` - Not authenticated -- `404 Not Found` - Series not found -- `409 Conflict` - NFO already exists (use `overwrite_existing: true`) -- `503 Service Unavailable` - TMDB API error or key not configured +- `401 Unauthorized` - Not authenticated +- `404 Not Found` - Series not found +- `409 Conflict` - NFO already exists (use `overwrite_existing: true`) +- `503 Service Unavailable` - TMDB API error or key not configured Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L150-L240) @@ -927,11 +935,11 @@ Update existing NFO file with fresh TMDB data. **Path Parameters:** -- `serie_id` (string): Series identifier +- `serie_id` (string): Series identifier **Query Parameters:** -- `download_media` (boolean, default: true): Re-download media files +- `download_media` (boolean, default: true): Re-download media files **Response (200 OK):** @@ -954,9 +962,9 @@ Update existing NFO file with fresh TMDB data. **Errors:** -- `401 Unauthorized` - Not authenticated -- `404 Not Found` - Series or NFO not found (use create endpoint) -- `503 Service Unavailable` - TMDB API error +- `401 Unauthorized` - Not authenticated +- `404 Not Found` - Series or NFO not found (use create endpoint) +- `503 Service Unavailable` - TMDB API error Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L243-L325) @@ -968,7 +976,7 @@ Get NFO file XML content for a series. **Path Parameters:** -- `serie_id` (string): Series identifier +- `serie_id` (string): Series identifier **Response (200 OK):** @@ -984,8 +992,8 @@ Get NFO file XML content for a series. **Errors:** -- `401 Unauthorized` - Not authenticated -- `404 Not Found` - Series or NFO not found +- `401 Unauthorized` - Not authenticated +- `404 Not Found` - Series or NFO not found Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L328-L397) @@ -997,7 +1005,7 @@ Get media files status for a series. **Path Parameters:** -- `serie_id` (string): Series identifier +- `serie_id` (string): Series identifier **Response (200 OK):** @@ -1014,8 +1022,8 @@ Get media files status for a series. **Errors:** -- `401 Unauthorized` - Not authenticated -- `404 Not Found` - Series not found +- `401 Unauthorized` - Not authenticated +- `404 Not Found` - Series not found Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L400-L447) @@ -1027,7 +1035,7 @@ Download missing media files for a series. **Path Parameters:** -- `serie_id` (string): Series identifier +- `serie_id` (string): Series identifier **Request Body:** @@ -1054,9 +1062,9 @@ Download missing media files for a series. **Errors:** -- `401 Unauthorized` - Not authenticated -- `404 Not Found` - Series or NFO not found (NFO required for TMDB ID) -- `503 Service Unavailable` - TMDB API error +- `401 Unauthorized` - Not authenticated +- `404 Not Found` - Series or NFO not found (NFO required for TMDB ID) +- `503 Service Unavailable` - TMDB API error Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L450-L519) @@ -1079,10 +1087,10 @@ Batch create NFO files for multiple series. **Fields:** -- `serie_ids` (array of strings): Series identifiers to process -- `download_media` (boolean, default: true): Download media files -- `skip_existing` (boolean, default: true): Skip series with existing NFOs -- `max_concurrent` (integer, 1-10, default: 3): Number of concurrent operations +- `serie_ids` (array of strings): Series identifiers to process +- `download_media` (boolean, default: true): Download media files +- `skip_existing` (boolean, default: true): Skip series with existing NFOs +- `max_concurrent` (integer, 1-10, default: 3): Number of concurrent operations **Response (200 OK):** @@ -1120,8 +1128,8 @@ Batch create NFO files for multiple series. **Errors:** -- `401 Unauthorized` - Not authenticated -- `503 Service Unavailable` - TMDB API key not configured +- `401 Unauthorized` - Not authenticated +- `503 Service Unavailable` - TMDB API key not configured Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L522-L634) @@ -1158,8 +1166,8 @@ Get list of series without NFO files. **Errors:** -- `401 Unauthorized` - Not authenticated -- `503 Service Unavailable` - TMDB API key not configured +- `401 Unauthorized` - Not authenticated +- `503 Service Unavailable` - TMDB API key not configured Source: [src/server/api/nfo.py](../src/server/api/nfo.py#L637-L684) @@ -1352,7 +1360,7 @@ Clients can join/leave rooms to receive specific updates. **Available Rooms:** -- `downloads` - Download progress and status updates +- `downloads` - Download progress and status updates ### Server Message Format diff --git a/src/server/api/anime.py b/src/server/api/anime.py index 56f1d26..853e165 100644 --- a/src/server/api/anime.py +++ b/src/server/api/anime.py @@ -337,10 +337,8 @@ async def list_anime( # Get all series from database to fetch NFO metadata # and episode counts from src.server.database.connection import get_sync_session - from src.server.database.models import ( - AnimeSeries as DBAnimeSeries, - Episode - ) + from src.server.database.models import AnimeSeries as DBAnimeSeries + from src.server.database.models import Episode session = get_sync_session() try: diff --git a/src/server/database/service.py b/src/server/database/service.py index 46f1eb4..2e5a4ef 100644 --- a/src/server/database/service.py +++ b/src/server/database/service.py @@ -26,7 +26,7 @@ import logging from datetime import datetime, timedelta, timezone from typing import List, Optional -from sqlalchemy import delete, select, update +from sqlalchemy import Integer, delete, select, update from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import Session, selectinload @@ -258,11 +258,15 @@ class AnimeSeriesService: limit: Optional[int] = None, offset: int = 0, ) -> List[AnimeSeries]: - """Get anime series that have no downloaded episodes in folder. + """Get anime series that have no episodes found in folder. - Returns series where either: - - No episodes exist in the database, OR - - All episodes have is_downloaded=False + Since episodes in the database represent MISSING episodes + (from episodeDict), this returns series that have episodes + in the DB with is_downloaded=False, meaning they have missing + episodes and no files were found in the folder for those episodes. + + Returns series where: + - At least one episode exists in database with is_downloaded=False Args: db: Database session @@ -270,31 +274,20 @@ class AnimeSeriesService: offset: Offset for pagination Returns: - List of AnimeSeries instances with no downloaded episodes + List of AnimeSeries with missing episodes (not in folder) """ - from sqlalchemy import func, or_ - - # Subquery to count downloaded episodes per series - downloaded_count = ( - select( - Episode.series_id, - func.count(Episode.id).label('downloaded_count') - ) - .where(Episode.is_downloaded.is_(True)) - .group_by(Episode.series_id) + # Subquery to find series IDs with at least one undownloaded episode + undownloaded_series_ids = ( + select(Episode.series_id) + .where(Episode.is_downloaded == False) + .distinct() .subquery() ) - # Select series that either have no episodes or no downloaded episodes + # Select series that have undownloaded episodes query = ( select(AnimeSeries) - .outerjoin(downloaded_count, AnimeSeries.id == downloaded_count.c.series_id) - .where( - or_( - downloaded_count.c.downloaded_count == None, - downloaded_count.c.downloaded_count == 0 - ) - ) + .where(AnimeSeries.id.in_(select(undownloaded_series_ids.c.series_id))) .order_by(AnimeSeries.name) .offset(offset) ) diff --git a/tests/unit/test_series_filter.py b/tests/unit/test_series_filter.py index fc35e49..396abbf 100644 --- a/tests/unit/test_series_filter.py +++ b/tests/unit/test_series_filter.py @@ -1,10 +1,6 @@ """Tests for series filtering functionality.""" import pytest -from sqlalchemy.ext.asyncio import ( - AsyncSession, - async_sessionmaker, - create_async_engine, -) +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from src.server.database.models import Base from src.server.database.service import AnimeSeriesService, EpisodeService @@ -61,8 +57,14 @@ async def test_get_series_with_no_episodes_empty_database( async def test_get_series_with_no_episodes_no_downloaded_episodes( async_session: AsyncSession ): - """Test that series with no downloaded episodes are returned.""" - # Create a series with no episodes + """Test that series with no downloaded episodes are returned. + + Episodes in DB represent MISSING episodes, so: + - Series with episodes in DB (is_downloaded=False) = no files in folder + - Series with no episodes in DB = all episodes downloaded or no info + """ + # Create a series with NO episodes in DB + # (all downloaded or no episodes info) series1 = await AnimeSeriesService.create( async_session, key="test-series-1", @@ -71,7 +73,7 @@ async def test_get_series_with_no_episodes_no_downloaded_episodes( site="https://example.com/test1", ) - # Create a series with undownloaded episodes + # Create a series with undownloaded episodes (MISSING - should appear) series2 = await AnimeSeriesService.create( async_session, key="test-series-2", @@ -87,7 +89,7 @@ async def test_get_series_with_no_episodes_no_downloaded_episodes( is_downloaded=False, ) - # Create a series with downloaded episodes (should not be in result) + # Create a series with downloaded episodes (should not appear) series3 = await AnimeSeriesService.create( async_session, key="test-series-3", @@ -105,23 +107,26 @@ async def test_get_series_with_no_episodes_no_downloaded_episodes( await async_session.commit() - # Query for series with no downloaded episodes + # Query for series with no episodes in folder result = await AnimeSeriesService.get_series_with_no_episodes( async_session ) - # Should return series1 and series2 but not series3 + # Should return only series2 (has missing episodes) result_ids = {s.id for s in result} - assert series1.id in result_ids - assert series2.id in result_ids - assert series3.id not in result_ids + assert series1.id not in result_ids # No episodes in DB + assert series2.id in result_ids # Has missing episodes + assert series3.id not in result_ids # Has downloaded episodes @pytest.mark.asyncio async def test_get_series_with_no_episodes_mixed_downloads( async_session: AsyncSession ): - """Test series with mixed downloaded/undownloaded episodes.""" + """Test series with mixed downloaded/undownloaded episodes. + + Series with ANY missing episodes (is_downloaded=False) should appear. + """ # Create series with some downloaded and some undownloaded episodes series = await AnimeSeriesService.create( async_session, @@ -140,7 +145,7 @@ async def test_get_series_with_no_episodes_mixed_downloads( is_downloaded=True, ) - # Add undownloaded episode + # Add undownloaded episode (MISSING) await EpisodeService.create( async_session, series_id=series.id, @@ -151,30 +156,86 @@ async def test_get_series_with_no_episodes_mixed_downloads( await async_session.commit() - # Query for series with no downloaded episodes + # Query for series with no episodes in folder result = await AnimeSeriesService.get_series_with_no_episodes( async_session ) - # Should NOT include series with at least one downloaded episode - result_ids = {s.id for s in result} - assert series.id not in result_ids + # Should return the series because it has missing episodes + assert len(result) == 1 + assert result[0].id == series.id + + +@pytest.mark.asyncio +async def test_get_series_with_no_episodes_mixed_seasons( + async_session: AsyncSession +): + """Test series with some seasons downloaded, some not. + + If ANY episode is still missing (is_downloaded=False), series should appear. + """ + series = await AnimeSeriesService.create( + async_session, + key="test-series", + name="Test Series", + folder="Test Series (2024)", + site="https://example.com/test", + ) + + # Season 1: all episodes downloaded + await EpisodeService.create( + async_session, + series_id=series.id, + season=1, + episode_number=1, + is_downloaded=True, + ) + + # Season 2: has missing episode + await EpisodeService.create( + async_session, + series_id=series.id, + season=2, + episode_number=1, + is_downloaded=False, + ) + + await async_session.commit() + + result = await AnimeSeriesService.get_series_with_no_episodes( + async_session + ) + + # Should return the series because season 2 has missing episodes + assert len(result) == 1 + assert result[0].id == series.id @pytest.mark.asyncio async def test_get_series_with_no_episodes_pagination( async_session: AsyncSession ): - """Test pagination works correctly.""" - # Create multiple series without downloaded episodes + """Test pagination works correctly. + + Note: Series with no episodes in DB won't appear. + """ + # Create multiple series with missing episodes for i in range(5): - await AnimeSeriesService.create( + series = await AnimeSeriesService.create( async_session, key=f"test-series-{i}", name=f"Test Series {i}", folder=f"Test Series {i} (2024)", site=f"https://example.com/test{i}", ) + # Add missing episode + await EpisodeService.create( + async_session, + series_id=series.id, + season=1, + episode_number=1, + is_downloaded=False, + ) await async_session.commit()