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
This commit is contained in:
10
docs/API.md
10
docs/API.md
@@ -203,7 +203,14 @@ List library series that have missing episodes.
|
|||||||
| `page` | int | 1 | Page number (must be positive) |
|
| `page` | int | 1 | Page number (must be positive) |
|
||||||
| `per_page` | int | 20 | Items per page (max 1000) |
|
| `per_page` | int | 20 | Items per page (max 1000) |
|
||||||
| `sort_by` | string | null | Sort field: `title`, `id`, `name`, `missing_episodes` |
|
| `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):**
|
**Response (200 OK):**
|
||||||
|
|
||||||
@@ -221,6 +228,7 @@ List library series that have missing episodes.
|
|||||||
```
|
```
|
||||||
|
|
||||||
**Example with filter:**
|
**Example with filter:**
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
GET /api/anime?filter=no_episodes
|
GET /api/anime?filter=no_episodes
|
||||||
```
|
```
|
||||||
|
|||||||
@@ -337,10 +337,8 @@ async def list_anime(
|
|||||||
# Get all series from database to fetch NFO metadata
|
# Get all series from database to fetch NFO metadata
|
||||||
# and episode counts
|
# and episode counts
|
||||||
from src.server.database.connection import get_sync_session
|
from src.server.database.connection import get_sync_session
|
||||||
from src.server.database.models import (
|
from src.server.database.models import AnimeSeries as DBAnimeSeries
|
||||||
AnimeSeries as DBAnimeSeries,
|
from src.server.database.models import Episode
|
||||||
Episode
|
|
||||||
)
|
|
||||||
|
|
||||||
session = get_sync_session()
|
session = get_sync_session()
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ import logging
|
|||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
from typing import List, Optional
|
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.ext.asyncio import AsyncSession
|
||||||
from sqlalchemy.orm import Session, selectinload
|
from sqlalchemy.orm import Session, selectinload
|
||||||
|
|
||||||
@@ -258,11 +258,15 @@ class AnimeSeriesService:
|
|||||||
limit: Optional[int] = None,
|
limit: Optional[int] = None,
|
||||||
offset: int = 0,
|
offset: int = 0,
|
||||||
) -> List[AnimeSeries]:
|
) -> 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:
|
Since episodes in the database represent MISSING episodes
|
||||||
- No episodes exist in the database, OR
|
(from episodeDict), this returns series that have episodes
|
||||||
- All episodes have is_downloaded=False
|
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:
|
Args:
|
||||||
db: Database session
|
db: Database session
|
||||||
@@ -270,31 +274,20 @@ class AnimeSeriesService:
|
|||||||
offset: Offset for pagination
|
offset: Offset for pagination
|
||||||
|
|
||||||
Returns:
|
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 find series IDs with at least one undownloaded episode
|
||||||
|
undownloaded_series_ids = (
|
||||||
# Subquery to count downloaded episodes per series
|
select(Episode.series_id)
|
||||||
downloaded_count = (
|
.where(Episode.is_downloaded == False)
|
||||||
select(
|
.distinct()
|
||||||
Episode.series_id,
|
|
||||||
func.count(Episode.id).label('downloaded_count')
|
|
||||||
)
|
|
||||||
.where(Episode.is_downloaded.is_(True))
|
|
||||||
.group_by(Episode.series_id)
|
|
||||||
.subquery()
|
.subquery()
|
||||||
)
|
)
|
||||||
|
|
||||||
# Select series that either have no episodes or no downloaded episodes
|
# Select series that have undownloaded episodes
|
||||||
query = (
|
query = (
|
||||||
select(AnimeSeries)
|
select(AnimeSeries)
|
||||||
.outerjoin(downloaded_count, AnimeSeries.id == downloaded_count.c.series_id)
|
.where(AnimeSeries.id.in_(select(undownloaded_series_ids.c.series_id)))
|
||||||
.where(
|
|
||||||
or_(
|
|
||||||
downloaded_count.c.downloaded_count == None,
|
|
||||||
downloaded_count.c.downloaded_count == 0
|
|
||||||
)
|
|
||||||
)
|
|
||||||
.order_by(AnimeSeries.name)
|
.order_by(AnimeSeries.name)
|
||||||
.offset(offset)
|
.offset(offset)
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -1,10 +1,6 @@
|
|||||||
"""Tests for series filtering functionality."""
|
"""Tests for series filtering functionality."""
|
||||||
import pytest
|
import pytest
|
||||||
from sqlalchemy.ext.asyncio import (
|
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
|
||||||
AsyncSession,
|
|
||||||
async_sessionmaker,
|
|
||||||
create_async_engine,
|
|
||||||
)
|
|
||||||
|
|
||||||
from src.server.database.models import Base
|
from src.server.database.models import Base
|
||||||
from src.server.database.service import AnimeSeriesService, EpisodeService
|
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 def test_get_series_with_no_episodes_no_downloaded_episodes(
|
||||||
async_session: AsyncSession
|
async_session: AsyncSession
|
||||||
):
|
):
|
||||||
"""Test that series with no downloaded episodes are returned."""
|
"""Test that series with no downloaded episodes are returned.
|
||||||
# Create a series with no episodes
|
|
||||||
|
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(
|
series1 = await AnimeSeriesService.create(
|
||||||
async_session,
|
async_session,
|
||||||
key="test-series-1",
|
key="test-series-1",
|
||||||
@@ -71,7 +73,7 @@ async def test_get_series_with_no_episodes_no_downloaded_episodes(
|
|||||||
site="https://example.com/test1",
|
site="https://example.com/test1",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Create a series with undownloaded episodes
|
# Create a series with undownloaded episodes (MISSING - should appear)
|
||||||
series2 = await AnimeSeriesService.create(
|
series2 = await AnimeSeriesService.create(
|
||||||
async_session,
|
async_session,
|
||||||
key="test-series-2",
|
key="test-series-2",
|
||||||
@@ -87,7 +89,7 @@ async def test_get_series_with_no_episodes_no_downloaded_episodes(
|
|||||||
is_downloaded=False,
|
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(
|
series3 = await AnimeSeriesService.create(
|
||||||
async_session,
|
async_session,
|
||||||
key="test-series-3",
|
key="test-series-3",
|
||||||
@@ -105,23 +107,26 @@ async def test_get_series_with_no_episodes_no_downloaded_episodes(
|
|||||||
|
|
||||||
await async_session.commit()
|
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(
|
result = await AnimeSeriesService.get_series_with_no_episodes(
|
||||||
async_session
|
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}
|
result_ids = {s.id for s in result}
|
||||||
assert series1.id in result_ids
|
assert series1.id not in result_ids # No episodes in DB
|
||||||
assert series2.id in result_ids
|
assert series2.id in result_ids # Has missing episodes
|
||||||
assert series3.id not in result_ids
|
assert series3.id not in result_ids # Has downloaded episodes
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_get_series_with_no_episodes_mixed_downloads(
|
async def test_get_series_with_no_episodes_mixed_downloads(
|
||||||
async_session: AsyncSession
|
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
|
# Create series with some downloaded and some undownloaded episodes
|
||||||
series = await AnimeSeriesService.create(
|
series = await AnimeSeriesService.create(
|
||||||
async_session,
|
async_session,
|
||||||
@@ -140,7 +145,7 @@ async def test_get_series_with_no_episodes_mixed_downloads(
|
|||||||
is_downloaded=True,
|
is_downloaded=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Add undownloaded episode
|
# Add undownloaded episode (MISSING)
|
||||||
await EpisodeService.create(
|
await EpisodeService.create(
|
||||||
async_session,
|
async_session,
|
||||||
series_id=series.id,
|
series_id=series.id,
|
||||||
@@ -151,30 +156,86 @@ async def test_get_series_with_no_episodes_mixed_downloads(
|
|||||||
|
|
||||||
await async_session.commit()
|
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(
|
result = await AnimeSeriesService.get_series_with_no_episodes(
|
||||||
async_session
|
async_session
|
||||||
)
|
)
|
||||||
|
|
||||||
# Should NOT include series with at least one downloaded episode
|
# Should return the series because it has missing episodes
|
||||||
result_ids = {s.id for s in result}
|
assert len(result) == 1
|
||||||
assert series.id not in result_ids
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_get_series_with_no_episodes_pagination(
|
async def test_get_series_with_no_episodes_pagination(
|
||||||
async_session: AsyncSession
|
async_session: AsyncSession
|
||||||
):
|
):
|
||||||
"""Test pagination works correctly."""
|
"""Test pagination works correctly.
|
||||||
# Create multiple series without downloaded episodes
|
|
||||||
|
Note: Series with no episodes in DB won't appear.
|
||||||
|
"""
|
||||||
|
# Create multiple series with missing episodes
|
||||||
for i in range(5):
|
for i in range(5):
|
||||||
await AnimeSeriesService.create(
|
series = await AnimeSeriesService.create(
|
||||||
async_session,
|
async_session,
|
||||||
key=f"test-series-{i}",
|
key=f"test-series-{i}",
|
||||||
name=f"Test Series {i}",
|
name=f"Test Series {i}",
|
||||||
folder=f"Test Series {i} (2024)",
|
folder=f"Test Series {i} (2024)",
|
||||||
site=f"https://example.com/test{i}",
|
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()
|
await async_session.commit()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user