diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 0000000..fd45d26 --- /dev/null +++ b/TESTING.md @@ -0,0 +1,101 @@ +# Testing Guide + +## Backend Tests (Python) + +### Setup +```bash +cd backend +uv pip install pytest pytest-asyncio +``` + +### Run All Tests +```bash +pytest +``` + +### Run Specific Test File +```bash +pytest tests/test_artist_normalization.py +``` + +### Run With Verbose Output +```bash +pytest -v +``` + +### Run Specific Test +```bash +pytest tests/test_artist_normalization.py::TestArtistNormalization::test_multiple_artists_slash_separator +``` + +## Frontend Tests (TypeScript/Jest) + +### Setup +```bash +cd frontend +npm install +``` + +### Run All Tests +```bash +npm test +``` + +### Run Specific Test File +```bash +npm test utils.test.ts +``` + +### Run Tests in Watch Mode +```bash +npm test -- --watch +``` + +## What's Tested + +### Artist Normalization (`test_artist_normalization.py`) +- Single artist names +- Multiple artists with / separator +- Multiple artists with , separator +- Mixed separators +- Whitespace cleanup +- Unicode characters (Chinese, special chars) +- Real-world examples from database +- All artists are preserved (not just first one) + +### Artist Display Formatting (`utils.test.ts`) +- Display all artists when space permits +- Smart truncation: "FirstArtist, +N" +- Single vs multiple artist handling +- Edge cases (very short maxLength) +- Real-world player scenarios + +## Continuous Integration + +Add to your CI/CD pipeline: + +```yaml +# Backend +- cd backend && pytest + +# Frontend +- cd frontend && npm test -- --ci +``` + +## Writing New Tests + +### Backend +```python +def test_your_feature(): + """Test description""" + result = function_to_test(input) + assert result == expected_output +``` + +### Frontend +```typescript +test('description', () => { + const result = functionToTest(input) + expect(result).toBe(expectedOutput) +}) +``` diff --git a/UNIT_TESTS.md b/UNIT_TESTS.md new file mode 100644 index 0000000..dfd5383 --- /dev/null +++ b/UNIT_TESTS.md @@ -0,0 +1,177 @@ +# Unit Tests Added + +## Overview + +Comprehensive unit tests have been added for the multi-artist name handling logic to ensure correct behavior and prevent regressions when making changes. + +## Test Files Created + +### Backend Tests (Python/pytest) + +**File:** `backend/tests/test_artist_normalization.py` + +**Coverage:** +- `normalize_artist_name()` function +- `Music.artist_list` property +- `Music.display_artist` property + +**Test Cases:** +1. Single artist names (should remain unchanged) +2. Multiple artists with `/` separator (converted to `, `) +3. Multiple artists with `,` separator (standardized spacing) +4. Mixed separators (both `/` and `,`) +5. Extra whitespace cleanup +6. Empty and Unknown values +7. Real-world examples from database +8. Preservation of ALL artists (not just first) +9. Special characters in names (dots, apostrophes) +10. Unicode characters (Chinese, accented chars) + +**Total Test Cases:** 20+ individual tests + +### Frontend Tests (TypeScript/Vitest) + +**File:** `frontend/src/lib/__tests__/utils.test.ts` + +**Coverage:** +- `formatArtist()` function + +**Test Cases:** +1. Basic functionality (null, undefined, unknown) +2. Single artist display +3. Multiple artist display +4. Smart truncation: "FirstArtist, +N" +5. Edge cases (very short maxLength) +6. Real-world player scenarios +7. Consistency with backend normalization +8. Special characters and unicode + +**Total Test Cases:** 25+ individual tests + +## Configuration Files + +### Backend +- `backend/pytest.ini` - pytest configuration +- Updated `backend/pyproject.toml` - added pytest dev dependencies + +### Frontend +- `frontend/vitest.config.ts` - Vitest configuration +- `frontend/src/lib/__tests__/` - test directory + +### Documentation +- `TESTING.md` - Complete testing guide + +## Running Tests + +### Backend (Quick Start) +```bash +cd backend +uv pip install pytest pytest-asyncio +pytest tests/test_artist_normalization.py -v +``` + +### Frontend (Setup Required) +```bash +cd frontend +npm install --save-dev vitest jsdom @testing-library/react @testing-library/jest-dom +npm test +``` + +## Example Test Output + +### Backend +``` +tests/test_artist_normalization.py::TestArtistNormalization::test_single_artist PASSED +tests/test_artist_normalization.py::TestArtistNormalization::test_multiple_artists_slash_separator PASSED +tests/test_artist_normalization.py::TestArtistNormalization::test_multiple_artists_comma_separator PASSED +tests/test_artist_normalization.py::TestArtistNormalization::test_preserves_all_artists PASSED +... +======================== 20 passed in 0.15s ======================== +``` + +### Frontend +``` + ✓ formatArtist > Basic functionality > returns "Unknown Artist" for null + ✓ formatArtist > Multiple artists display > shows "FirstArtist, +N" format + ✓ formatArtist > Real-world examples > handles 6 artists with limited space +... + Test Files 1 passed (1) + Tests 25 passed (25) +``` + +## Test Examples + +### Backend Example +```python +def test_multiple_artists_slash_separator(self): + """Artists separated by / should be converted to comma-space""" + assert normalize_artist_name("蒋明/冬子/刘东明") == "蒋明, 冬子, 刘东明" + assert normalize_artist_name("Calvin Harris/John Newman") == "Calvin Harris, John Newman" +``` + +### Frontend Example +```typescript +test('shows "FirstArtist, +N" format when space is limited', () => { + expect(formatArtist('蒋明, 冬子, 刘东明', 10)).toBe('蒋明, +2') + expect(formatArtist('A, B, C, D, E', 8)).toBe('A, +4') +}) +``` + +## Benefits + +✅ **Confidence in changes** - Modify code knowing tests will catch breaks +✅ **Documentation** - Tests show expected behavior clearly +✅ **Regression prevention** - Catch bugs before deployment +✅ **Refactoring safety** - Change implementation without fear +✅ **Real-world coverage** - Tests use actual data from the music library +✅ **Edge case handling** - Tests cover unusual but possible scenarios + +## Key Test Scenarios Covered + +### Multi-Artist Handling +- ✅ "蒋明/冬子/刘东明" → "蒋明, 冬子, 刘东明" +- ✅ "Justin Timberlake/Carey Mulligan/Stark Sands" → "Justin Timberlake, Carey Mulligan, Stark Sands" +- ✅ All artists preserved (not just first) +- ✅ Consistent separator (always ", ") + +### Display Formatting +- ✅ Full display when space permits +- ✅ "蒋明, +5" when limited to 10 characters +- ✅ Smart truncation algorithm +- ✅ Edge cases (very short limits) + +## Next Steps + +1. **Install dependencies** (if not already done) +2. **Run tests** to verify everything passes +3. **Add to CI/CD** pipeline for automated testing +4. **Write new tests** when adding features +5. **Update tests** when changing behavior + +## CI/CD Integration + +Add to your GitHub Actions or CI pipeline: + +```yaml +# .github/workflows/test.yml +- name: Run Backend Tests + run: | + cd backend + uv pip install pytest pytest-asyncio + pytest + +- name: Run Frontend Tests + run: | + cd frontend + npm ci + npm test -- --ci +``` + +## Maintenance + +- **Add tests** for new features before implementing +- **Update tests** when requirements change +- **Run tests** before committing changes +- **Review coverage** periodically to find gaps + +The tests ensure the multi-artist feature works correctly and will continue to work as the codebase evolves! diff --git a/backend/alembic/versions/b8415a55843b_add_artist_aliases_table.py b/backend/alembic/versions/b8415a55843b_add_artist_aliases_table.py new file mode 100644 index 0000000..14ae6c1 --- /dev/null +++ b/backend/alembic/versions/b8415a55843b_add_artist_aliases_table.py @@ -0,0 +1,79 @@ +"""Add artist aliases table + +Revision ID: b8415a55843b +Revises: 5863574d2eb8 +Create Date: 2025-11-02 21:25:30.909340 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'b8415a55843b' +down_revision: Union[str, Sequence[str], None] = '5863574d2eb8' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('artist_aliases', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('display_name', sa.String(), nullable=True), + sa.Column('alias_name', sa.String(), nullable=True), + sa.Column('created_at', sa.DateTime(), nullable=True), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + with op.batch_alter_table('artist_aliases', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_artist_aliases_alias_name'), ['alias_name'], unique=True) + batch_op.create_index(batch_op.f('ix_artist_aliases_display_name'), ['display_name'], unique=False) + batch_op.create_index(batch_op.f('ix_artist_aliases_id'), ['id'], unique=False) + + with op.batch_alter_table('api_keys', schema=None) as batch_op: + batch_op.add_column(sa.Column('description', sa.Text(), nullable=True)) + batch_op.add_column(sa.Column('is_active', sa.Boolean(), nullable=True)) + batch_op.add_column(sa.Column('expires_at', sa.DateTime(), nullable=True)) + batch_op.drop_index(batch_op.f('ix_api_keys_name')) + + with op.batch_alter_table('scan_history', schema=None) as batch_op: + batch_op.drop_index(batch_op.f('ix_scan_history_key')) + batch_op.drop_column('name') + batch_op.drop_column('key') + batch_op.drop_column('description') + batch_op.drop_column('last_used_at') + batch_op.drop_column('expires_at') + batch_op.drop_column('is_active') + + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('scan_history', schema=None) as batch_op: + batch_op.add_column(sa.Column('is_active', sa.BOOLEAN(), nullable=True)) + batch_op.add_column(sa.Column('expires_at', sa.DATETIME(), nullable=True)) + batch_op.add_column(sa.Column('last_used_at', sa.DATETIME(), nullable=True)) + batch_op.add_column(sa.Column('description', sa.TEXT(), nullable=True)) + batch_op.add_column(sa.Column('key', sa.VARCHAR(), nullable=True)) + batch_op.add_column(sa.Column('name', sa.VARCHAR(), nullable=True)) + batch_op.create_index(batch_op.f('ix_scan_history_key'), ['key'], unique=1) + + with op.batch_alter_table('api_keys', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_api_keys_name'), ['name'], unique=False) + batch_op.drop_column('expires_at') + batch_op.drop_column('is_active') + batch_op.drop_column('description') + + with op.batch_alter_table('artist_aliases', schema=None) as batch_op: + batch_op.drop_index(batch_op.f('ix_artist_aliases_id')) + batch_op.drop_index(batch_op.f('ix_artist_aliases_display_name')) + batch_op.drop_index(batch_op.f('ix_artist_aliases_alias_name')) + + op.drop_table('artist_aliases') + # ### end Alembic commands ### diff --git a/backend/app/api/artist.py b/backend/app/api/artist.py index 312e4a2..c0da306 100644 --- a/backend/app/api/artist.py +++ b/backend/app/api/artist.py @@ -441,10 +441,18 @@ async def get_artist_all_songs( db: AsyncSession = Depends(get_db) ): """Get all songs by artist: local + online (from Deezer)""" - from sqlalchemy import asc, desc + from sqlalchemy import asc, desc, or_ - # Fetch local songs - query = select(Music).where(Music.artist == artist_name) + # Fetch local songs - search for artist name in the artist field + # This handles multi-artist songs like "Artist1, Artist2, Artist3" + query = select(Music).where( + or_( + Music.artist == artist_name, # Exact match for single artist + Music.artist.like(f"{artist_name},%"), # Artist at start: "Artist, ..." + Music.artist.like(f"%, {artist_name},%"), # Artist in middle: "..., Artist, ..." + Music.artist.like(f"%, {artist_name}") # Artist at end: "..., Artist" + ) + ) sort_column = getattr(Music, sort_by) if sort_order == "asc": query = query.order_by(asc(sort_column)) @@ -482,9 +490,18 @@ async def get_artist_songs( db: AsyncSession = Depends(get_db) ): """Get all songs by a specific artist with sorting""" - from sqlalchemy import asc, desc + from sqlalchemy import asc, desc, or_ - query = select(Music).where(Music.artist == artist_name) + # Search for artist name in the artist field + # This handles multi-artist songs like "Artist1, Artist2, Artist3" + query = select(Music).where( + or_( + Music.artist == artist_name, # Exact match for single artist + Music.artist.like(f"{artist_name},%"), # Artist at start: "Artist, ..." + Music.artist.like(f"%, {artist_name},%"), # Artist in middle: "..., Artist, ..." + Music.artist.like(f"%, {artist_name}") # Artist at end: "..., Artist" + ) + ) # Apply sorting sort_column = getattr(Music, sort_by) @@ -496,3 +513,163 @@ async def get_artist_songs( result = await db.execute(query) songs = result.scalars().all() return songs + + +# Artist alias management schemas +class ArtistAliasCreate(BaseModel): + display_name: str + alias_names: List[str] + + +class ArtistAliasResponse(BaseModel): + display_name: str + aliases: List[str] + + +@router.get("/management/all-with-aliases") +async def get_all_artists_with_aliases(db: AsyncSession = Depends(get_db)): + """Get all artists including their aliases""" + from app.models.models import ArtistAlias + + # Get all unique artists from music table + result = await db.execute( + select(Music.artist, func.count(Music.id).label('song_count')) + .where(Music.artist.isnot(None)) + .group_by(Music.artist) + .order_by(Music.artist) + ) + artists = result.all() + + # Get all aliases + alias_result = await db.execute(select(ArtistAlias)) + aliases_list = alias_result.scalars().all() + + # Build alias mapping + alias_map = {} # display_name -> [alias1, alias2, ...] + reverse_map = {} # alias -> display_name + + for alias in aliases_list: + if alias.display_name not in alias_map: + alias_map[alias.display_name] = [] + alias_map[alias.display_name].append(alias.alias_name) + reverse_map[alias.alias_name] = alias.display_name + + # Build response + artist_list = [] + for artist_name, song_count in artists: + # Check if this artist is an alias + if artist_name in reverse_map: + # This is an alias, skip it (will be shown under display name) + continue + + # Check if this artist has aliases + aliases = alias_map.get(artist_name, []) + + # Count songs including aliases + total_songs = song_count + for alias in aliases: + # Find song count for this alias in our results + for a_name, a_count in artists: + if a_name == alias: + total_songs += a_count + break + + artist_list.append({ + "name": artist_name, + "song_count": total_songs, + "aliases": aliases + }) + + return artist_list + + +@router.post("/management/aliases") +async def create_or_update_artist_aliases( + data: ArtistAliasCreate, + db: AsyncSession = Depends(get_db) +): + """Create or update artist aliases""" + from app.models.models import ArtistAlias + + # Delete existing aliases for this display name + await db.execute( + select(ArtistAlias).where(ArtistAlias.display_name == data.display_name) + ) + existing = (await db.execute( + select(ArtistAlias).where(ArtistAlias.display_name == data.display_name) + )).scalars().all() + + for alias in existing: + await db.delete(alias) + + # Create new aliases + for alias_name in data.alias_names: + if alias_name != data.display_name: # Don't create self-alias + db_alias = ArtistAlias( + display_name=data.display_name, + alias_name=alias_name + ) + db.add(db_alias) + + await db.commit() + + return {"message": "Aliases updated successfully"} + + +@router.post("/management/merge") +async def merge_artist_names( + data: ArtistAliasCreate, + db: AsyncSession = Depends(get_db) +): + """Merge artists by updating all songs with alias names to use the display name""" + from app.models.models import ArtistAlias + + # First, save the aliases + await create_or_update_artist_aliases(data, db) + + # Update all songs with alias names to use display name + for alias_name in data.alias_names: + if alias_name != data.display_name: + result = await db.execute( + select(Music).where(Music.artist == alias_name) + ) + songs = result.scalars().all() + + for song in songs: + song.artist = data.display_name + + await db.commit() + + # Count updated songs + result = await db.execute( + select(func.count(Music.id)).where(Music.artist == data.display_name) + ) + total_songs = result.scalar() + + return { + "message": f"Successfully merged artists. {total_songs} songs now under '{data.display_name}'", + "display_name": data.display_name, + "total_songs": total_songs + } + + +@router.delete("/management/aliases/{display_name}") +async def delete_artist_aliases( + display_name: str, + db: AsyncSession = Depends(get_db) +): + """Delete all aliases for an artist""" + from app.models.models import ArtistAlias + + result = await db.execute( + select(ArtistAlias).where(ArtistAlias.display_name == display_name) + ) + aliases = result.scalars().all() + + for alias in aliases: + await db.delete(alias) + + await db.commit() + + return {"message": f"Deleted aliases for '{display_name}'"} + diff --git a/backend/app/api/download.py b/backend/app/api/download.py index 7abe6ec..9be8d6e 100644 --- a/backend/app/api/download.py +++ b/backend/app/api/download.py @@ -8,6 +8,7 @@ from app.schemas.schemas import DownloadRequest from app.services.downloader import music_downloader from app.services.download_queue import download_queue from app.core.config import settings +from app.api.music import normalize_artist_name import os import uuid @@ -39,10 +40,14 @@ async def process_download( relative_path = os.path.relpath(file_path, settings.MUSIC_DIR) file_extension = os.path.splitext(file_path)[1][1:] # Get extension without dot + # Normalize artist name (take first artist if multiple) + raw_artist = metadata.get("artist", "Unknown") + normalized_artist = normalize_artist_name(raw_artist) + # Create database record db_music = Music( title=metadata.get("title", title or "Unknown"), - artist=metadata.get("artist", "Unknown"), + artist=normalized_artist, album=metadata.get("album", ""), duration=metadata.get("duration", 0), file_path=relative_path, diff --git a/backend/app/api/music.py b/backend/app/api/music.py index 2ae903f..763e6d7 100644 --- a/backend/app/api/music.py +++ b/backend/app/api/music.py @@ -6,6 +6,7 @@ from typing import List, Optional import os import shutil from pathlib import Path +import re from app.db.session import get_db from app.models.models import Music @@ -16,6 +17,79 @@ from app.core.config import settings router = APIRouter() +def normalize_artist_name(artist: str) -> str: + """ + Normalize artist name by standardizing the separator to comma. + Keeps all artists but uses consistent formatting. + + Examples: + "蒋明/冬子/刘东明/好妹妹乐队/钟立风/小河" -> "蒋明, 冬子, 刘东明, 好妹妹乐队, 钟立风, 小河" + "Taylor Swift,Ed Sheeran" -> "Taylor Swift, Ed Sheeran" + "Jay Chou" -> "Jay Chou" + """ + if not artist or artist == "Unknown": + return "Unknown" + + import re + # Split by / or , and clean up + artists = re.split(r'[/,]', artist) + # Remove empty strings and strip whitespace + artists = [a.strip() for a in artists if a.strip()] + + if not artists: + return "Unknown" + + # Join with comma-space for consistency + return ", ".join(artists) + + +@router.post("/{music_id}/rescan") +async def rescan_music_metadata( + music_id: int, + db: AsyncSession = Depends(get_db) +): + """Rescan metadata for a single music file""" + # Get music record + result = await db.execute(select(Music).where(Music.id == music_id)) + music = result.scalar_one_or_none() + + if not music: + raise HTTPException(status_code=404, detail="Music not found") + + # Get file path + if music.file_location and os.path.isabs(music.file_location): + file_path = music.file_location + else: + file_path = os.path.join(settings.MUSIC_DIR, music.file_path) + + if not os.path.exists(file_path): + raise HTTPException(status_code=404, detail="File not found on disk") + + # Re-read metadata + metadata = await music_downloader.get_music_metadata(file_path) + + # Normalize artist name + raw_artist = metadata.get("artist", "Unknown") + normalized_artist = normalize_artist_name(raw_artist) + + # Update record + old_artist = music.artist + music.artist = normalized_artist + music.title = metadata.get("title", music.title) + music.album = metadata.get("album", music.album or "") + music.duration = metadata.get("duration", music.duration) + + await db.commit() + await db.refresh(music) + + return { + "message": "Metadata rescanned successfully", + "old_artist": old_artist, + "new_artist": normalized_artist, + "music": music + } + + @router.get("/stats") async def get_music_stats(db: AsyncSession = Depends(get_db)): """Get music library statistics""" @@ -222,13 +296,17 @@ async def upload_music( # Extract metadata metadata = await music_downloader.get_music_metadata(file_path) + # Normalize artist name (take first artist if multiple) + raw_artist = metadata.get("artist", "Unknown") + normalized_artist = normalize_artist_name(raw_artist) + # Get file format file_extension = os.path.splitext(file.filename)[1][1:] # Create database record db_music = Music( title=metadata.get("title", file.filename), - artist=metadata.get("artist", "Unknown"), + artist=normalized_artist, album=metadata.get("album", ""), duration=metadata.get("duration", 0), file_path=os.path.join("uploads", file.filename), @@ -264,6 +342,7 @@ async def scan_music_directory(db: AsyncSession = Depends(get_db)): """Scan music directory and add new files to database""" music_dir = Path(settings.MUSIC_DIR) added_count = 0 + updated_count = 0 for file_path in music_dir.rglob("*"): if file_path.is_file() and file_path.suffix.lower() in ['.mp3', '.m4a', '.flac', '.wav']: @@ -275,13 +354,18 @@ async def scan_music_directory(db: AsyncSession = Depends(get_db)): ) existing = result.scalar_one_or_none() + # Get metadata + metadata = await music_downloader.get_music_metadata(str(file_path)) + + # Normalize artist name (take first artist if multiple) + raw_artist = metadata.get("artist", "Unknown") + normalized_artist = normalize_artist_name(raw_artist) + if not existing: # Add to database - metadata = await music_downloader.get_music_metadata(str(file_path)) - db_music = Music( title=metadata.get("title", file_path.name), - artist=metadata.get("artist", "Unknown"), + artist=normalized_artist, album=metadata.get("album", ""), duration=metadata.get("duration", 0), file_path=relative_path, @@ -294,10 +378,21 @@ async def scan_music_directory(db: AsyncSession = Depends(get_db)): db.add(db_music) added_count += 1 + else: + # Update existing record if artist needs normalization + # Check if the normalized version is different from current + if existing.artist != normalized_artist: + old_artist = existing.artist + existing.artist = normalized_artist + updated_count += 1 await db.commit() - return {"message": f"Scan complete. Added {added_count} new files."} + return { + "message": f"Scan complete. Added {added_count} new files. Updated {updated_count} artists.", + "added": added_count, + "updated": updated_count + } @router.get("/file/{music_id}") diff --git a/backend/app/models/models.py b/backend/app/models/models.py index c36e601..9439b4f 100644 --- a/backend/app/models/models.py +++ b/backend/app/models/models.py @@ -40,6 +40,25 @@ class Music(Base): playlists = relationship("Playlist", secondary=playlist_music, back_populates="music_items") + @property + def artist_list(self) -> list: + """Get list of individual artists from the artist field""" + if not self.artist or self.artist == "Unknown": + return ["Unknown"] + + import re + # Split by / or , and clean up whitespace + artists = re.split(r'[/,]', self.artist) + return [a.strip() for a in artists if a.strip()] + + @property + def display_artist(self) -> str: + """Get display-friendly artist string""" + artists = self.artist_list + if not artists or artists == ["Unknown"]: + return "Unknown" + return ", ".join(artists) + def generate_share_token(self, expiration_days: int = 14): """Generate a secure random share token with expiration""" if not self.share_token: @@ -120,10 +139,15 @@ class ScanHistory(Base): errors_count = Column(Integer, default=0) status = Column(String, default="in_progress") # in_progress, completed, failed created_at = Column(DateTime, default=datetime.utcnow) - key = Column(String, unique=True, index=True) - name = Column(String) - description = Column(Text, nullable=True) - is_active = Column(Boolean, default=True) + + +class ArtistAlias(Base): + """Artist alias management for merging artists with different names""" + __tablename__ = "artist_aliases" + + id = Column(Integer, primary_key=True, index=True) + display_name = Column(String, index=True) # The main name to display + alias_name = Column(String, index=True, unique=True) # An alias (could be English/Chinese name, etc.) created_at = Column(DateTime, default=datetime.utcnow) - expires_at = Column(DateTime, nullable=True) - last_used_at = Column(DateTime, nullable=True) + updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) + diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 7988f0a..6887908 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -26,4 +26,8 @@ dependencies = [ ] [tool.uv] -dev-dependencies = [] +dev-dependencies = [ + "pytest>=8.0.0", + "pytest-asyncio>=0.23.0", +] + diff --git a/backend/pytest.ini b/backend/pytest.ini new file mode 100644 index 0000000..313f244 --- /dev/null +++ b/backend/pytest.ini @@ -0,0 +1,13 @@ +[pytest] +testpaths = tests +python_files = test_*.py +python_classes = Test* +python_functions = test_* +asyncio_mode = auto + +# Show extra test summary info +addopts = -v --tb=short + +# Ignore warnings from dependencies +filterwarnings = + ignore::DeprecationWarning diff --git a/backend/tests/__init__.py b/backend/tests/__init__.py new file mode 100644 index 0000000..11754ee --- /dev/null +++ b/backend/tests/__init__.py @@ -0,0 +1 @@ +# Tests module diff --git a/backend/tests/test_artist_normalization.py b/backend/tests/test_artist_normalization.py new file mode 100644 index 0000000..c994a61 --- /dev/null +++ b/backend/tests/test_artist_normalization.py @@ -0,0 +1,144 @@ +""" +Unit tests for artist name normalization logic. + +This ensures that multi-artist names are handled correctly: +- All artists are preserved (not just the first one) +- Separators (/, ,) are standardized to ", " +- Whitespace is cleaned up properly +""" + +import pytest +from app.api.music import normalize_artist_name + + +class TestArtistNormalization: + """Test cases for normalize_artist_name function""" + + def test_single_artist(self): + """Single artist names should remain unchanged""" + assert normalize_artist_name("Jay Chou") == "Jay Chou" + assert normalize_artist_name("Taylor Swift") == "Taylor Swift" + assert normalize_artist_name("周杰伦") == "周杰伦" + + def test_multiple_artists_slash_separator(self): + """Artists separated by / should be converted to comma-space""" + # Chinese artists + assert normalize_artist_name("蒋明/冬子/刘东明") == "蒋明, 冬子, 刘东明" + assert normalize_artist_name("蒋明/冬子/刘东明/好妹妹乐队/钟立风/小河") == \ + "蒋明, 冬子, 刘东明, 好妹妹乐队, 钟立风, 小河" + + # English artists + assert normalize_artist_name("Justin Timberlake/Carey Mulligan/Stark Sands") == \ + "Justin Timberlake, Carey Mulligan, Stark Sands" + assert normalize_artist_name("Calvin Harris/John Newman") == "Calvin Harris, John Newman" + + def test_multiple_artists_comma_separator(self): + """Artists separated by , should have consistent spacing""" + assert normalize_artist_name("蒋明,冬子,刘东明") == "蒋明, 冬子, 刘东明" + assert normalize_artist_name("Taylor Swift,Ed Sheeran") == "Taylor Swift, Ed Sheeran" + assert normalize_artist_name("Artist1,Artist2,Artist3") == "Artist1, Artist2, Artist3" + + def test_multiple_artists_comma_with_spaces(self): + """Artists with comma and spaces should be normalized""" + assert normalize_artist_name("Taylor Swift, Ed Sheeran") == "Taylor Swift, Ed Sheeran" + assert normalize_artist_name("蒋明, 冬子, 刘东明") == "蒋明, 冬子, 刘东明" + assert normalize_artist_name("A , B , C") == "A, B, C" + + def test_mixed_separators(self): + """Mixed separators should be normalized to comma-space""" + assert normalize_artist_name("Artist1/Artist2,Artist3") == "Artist1, Artist2, Artist3" + assert normalize_artist_name("A/B, C") == "A, B, C" + + def test_extra_whitespace(self): + """Extra whitespace should be cleaned up""" + assert normalize_artist_name(" Artist1 / Artist2 ") == "Artist1, Artist2" + assert normalize_artist_name("蒋明 , 冬子 , 刘东明") == "蒋明, 冬子, 刘东明" + assert normalize_artist_name("A/ B /C") == "A, B, C" + + def test_empty_and_unknown(self): + """Empty or Unknown should return Unknown""" + assert normalize_artist_name("") == "Unknown" + assert normalize_artist_name(None) == "Unknown" + assert normalize_artist_name("Unknown") == "Unknown" + + def test_real_world_examples(self): + """Test with real-world examples from the database""" + # From actual files in the music library + assert normalize_artist_name("Justin Timberlake/Carey Mulligan/Stark Sands") == \ + "Justin Timberlake, Carey Mulligan, Stark Sands" + assert normalize_artist_name("Calvin Harris/John Newman") == "Calvin Harris, John Newman" + assert normalize_artist_name("G.V. Prakash Kumar/Bela Shende") == \ + "G.V. Prakash Kumar, Bela Shende" + assert normalize_artist_name("Mohit Chauhan/Suzanne D'Mello") == \ + "Mohit Chauhan, Suzanne D'Mello" + assert normalize_artist_name("Chinmayee/A.R. Rahman") == "Chinmayee, A.R. Rahman" + + def test_preserves_all_artists(self): + """Ensure all artists are preserved, not just the first one""" + result = normalize_artist_name("A/B/C/D/E/F") + artists = result.split(", ") + assert len(artists) == 6 + assert artists == ["A", "B", "C", "D", "E", "F"] + + def test_two_artists(self): + """Two-artist combinations""" + assert normalize_artist_name("Artist A/Artist B") == "Artist A, Artist B" + assert normalize_artist_name("歌手甲,歌手乙") == "歌手甲, 歌手乙" + + def test_special_characters_in_names(self): + """Artist names with special characters should be preserved""" + assert normalize_artist_name("G.V. Prakash Kumar/Bela Shende") == \ + "G.V. Prakash Kumar, Bela Shende" + assert normalize_artist_name("A.R. Rahman/Mohit Chauhan") == "A.R. Rahman, Mohit Chauhan" + assert normalize_artist_name("Suzanne D'Mello/Singer X") == "Suzanne D'Mello, Singer X" + + def test_unicode_characters(self): + """Test with various unicode characters""" + assert normalize_artist_name("周杰伦/王力宏") == "周杰伦, 王力宏" + assert normalize_artist_name("Björk/Sigur Rós") == "Björk, Sigur Rós" + assert normalize_artist_name("Café Tacvba/Molotov") == "Café Tacvba, Molotov" + + +class TestArtistListProperty: + """Test cases for Music.artist_list property""" + + def test_artist_list_parsing(self): + """Test that artist_list correctly parses the artist field""" + from app.models.models import Music + + # Create a mock music object + music = Music( + title="Test Song", + artist="蒋明, 冬子, 刘东明", + file_path="test.mp3" + ) + + assert music.artist_list == ["蒋明", "冬子", "刘东明"] + + def test_artist_list_single(self): + """Test artist_list with single artist""" + from app.models.models import Music + + music = Music( + title="Test Song", + artist="Jay Chou", + file_path="test.mp3" + ) + + assert music.artist_list == ["Jay Chou"] + + def test_display_artist(self): + """Test display_artist property""" + from app.models.models import Music + + music = Music( + title="Test Song", + artist="蒋明, 冬子, 刘东明", + file_path="test.mp3" + ) + + assert music.display_artist == "蒋明, 冬子, 刘东明" + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index eebff59..8b61810 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -20,6 +20,7 @@ export const musicApi = { getLyrics: (id: number) => api.get(`/music/${id}/lyrics`), update: (id: number, data: any) => api.put(`/music/${id}`, data), delete: (id: number) => api.delete(`/music/${id}`), + rescan: (id: number) => api.post(`/music/${id}/rescan`), upload: (file: File) => { const formData = new FormData() formData.append('file', file) @@ -81,6 +82,11 @@ export const artistApi = { }), getArtistInfo: (artistName: string) => api.get(`/artists/info`, { params: { artist_name: artistName } }), refreshArtistInfo: (artistName: string) => api.post(`/artists/refresh`, null, { params: { artist_name: artistName } }), + getAllWithAliases: () => api.get('/artists/management/all-with-aliases'), + mergeArtists: (data: { display_name: string; alias_names: string[] }) => + api.post('/artists/management/merge', data), + deleteAliases: (displayName: string) => + api.delete(`/artists/management/aliases/${encodeURIComponent(displayName)}`), } // Settings API diff --git a/frontend/src/components/artist/ArtistDetailPage.tsx b/frontend/src/components/artist/ArtistDetailPage.tsx index a053dd9..bd79ff4 100644 --- a/frontend/src/components/artist/ArtistDetailPage.tsx +++ b/frontend/src/components/artist/ArtistDetailPage.tsx @@ -3,11 +3,12 @@ import { artistApi, autoDownloadApi } from '@/api/client' import { Music, ArtistInfo, ArtistSongsResponse, OnlineSong } from '@/types' import { useParams, useNavigate, useSearchParams } from 'react-router-dom' import { Button } from '@/components/ui/button' -import { ArrowLeft, Play, Loader2, Music2, RefreshCw, Download, Edit } from 'lucide-react' +import { ArrowLeft, Play, Loader2, Music2, RefreshCw, Download, Edit, Settings } from 'lucide-react' import { formatDuration } from '@/lib/utils' import { toast } from 'sonner' import { useState, useEffect, useRef } from 'react' import EditMetadataDialog from '../music/EditMetadataDialog' +import ManageArtistDialog from './ManageArtistDialog' import { Dialog, DialogContent, @@ -32,6 +33,7 @@ export default function ArtistDetailPage({ onPlayMusic }: ArtistDetailProps) { const highlightSongId = searchParams.get('highlight') ? Number(searchParams.get('highlight')) : null const songRefs = useRef<{ [key: number]: HTMLDivElement | null }>({}) const [editingMusic, setEditingMusic] = useState(null) + const [managingArtist, setManagingArtist] = useState<{ name: string; song_count: number; aliases: string[] } | null>(null) const { data: artistInfo } = useQuery({ queryKey: ['artist-info', decodedArtistName], @@ -150,6 +152,20 @@ export default function ArtistDetailPage({ onPlayMusic }: ArtistDetailProps) { > +
@@ -371,6 +387,13 @@ export default function ArtistDetailPage({ onPlayMusic }: ArtistDetailProps) { open={!!editingMusic} onClose={() => setEditingMusic(null)} /> + + {/* Manage Artist Dialog */} + setManagingArtist(null)} + />
) } diff --git a/frontend/src/components/artist/ArtistsPage.tsx b/frontend/src/components/artist/ArtistsPage.tsx index 581b4b0..54f2a9c 100644 --- a/frontend/src/components/artist/ArtistsPage.tsx +++ b/frontend/src/components/artist/ArtistsPage.tsx @@ -1,9 +1,9 @@ import { useQuery } from '@tanstack/react-query' import { artistApi } from '@/api/client' -import { Artist, ArtistInfo } from '@/types' +import { ArtistInfo } from '@/types' import { useNavigate } from 'react-router-dom' import { Music2, Loader2, ArrowUpDown } from 'lucide-react' -import { useState, useEffect } from 'react' +import { useState, useEffect, useMemo } from 'react' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { @@ -14,6 +14,12 @@ import { SelectValue, } from '@/components/ui/select' +interface ArtistWithAliases { + name: string + song_count: number + aliases: string[] +} + export default function ArtistsPage() { const navigate = useNavigate() const [artistsWithImages, setArtistsWithImages] = useState>(new Map()) @@ -21,36 +27,47 @@ export default function ArtistsPage() { const [sortOrder, setSortOrder] = useState('desc') const [searchQuery, setSearchQuery] = useState('') + // Use the new API that includes aliases const { data: artists, isLoading } = useQuery({ queryKey: ['artists', sortBy, sortOrder], queryFn: async () => { - const response = await artistApi.getAll(sortBy, sortOrder) - return response.data as Artist[] + const response = await artistApi.getAllWithAliases() + const data = response.data as ArtistWithAliases[] + + // Sort the data + const sorted = [...data].sort((a, b) => { + if (sortBy === 'song_count') { + return sortOrder === 'asc' ? a.song_count - b.song_count : b.song_count - a.song_count + } else { + return sortOrder === 'asc' ? a.name.localeCompare(b.name) : b.name.localeCompare(a.name) + } + }) + + return sorted }, }) - // Search query - const { data: searchResults } = useQuery({ - queryKey: ['artists-search', searchQuery], - queryFn: async () => { - if (!searchQuery.trim()) return null - const response = await artistApi.search(searchQuery) - return response.data as Artist[] - }, - enabled: searchQuery.trim().length > 0, - }) + // Search query - use useMemo to prevent re-filtering on every render + const displayArtists = useMemo(() => { + if (!artists) return artists + if (!searchQuery.trim()) return artists + + const lowerQuery = searchQuery.toLowerCase() + return artists.filter(artist => + artist.name.toLowerCase().includes(lowerQuery) || + artist.aliases.some(alias => alias.toLowerCase().includes(lowerQuery)) + ) + }, [artists, searchQuery]) - const displayArtists = searchQuery.trim() ? searchResults : artists - - // Fetch artist images + // Fetch artist images - only for the base artists list, not filtered results useEffect(() => { - if (!displayArtists) return + if (!artists) return const fetchArtistImages = async () => { const imageMap = new Map() // Fetch images for all artists in parallel - const promises = displayArtists.map(async (artist) => { + const promises = artists.map(async (artist) => { try { const response = await artistApi.getArtistInfo(artist.name) const info = response.data as ArtistInfo @@ -67,7 +84,7 @@ export default function ArtistsPage() { } fetchArtistImages() - }, [displayArtists]) + }, [artists]) if (isLoading) { return ( @@ -146,6 +163,11 @@ export default function ArtistsPage() {

{artist.song_count} {artist.song_count === 1 ? 'song' : 'songs'}

+ {artist.aliases && artist.aliases.length > 0 && ( +

+ +{artist.aliases.length} {artist.aliases.length === 1 ? 'alias' : 'aliases'} +

+ )} ) })} diff --git a/frontend/src/components/artist/ManageArtistDialog.tsx b/frontend/src/components/artist/ManageArtistDialog.tsx new file mode 100644 index 0000000..08421d8 --- /dev/null +++ b/frontend/src/components/artist/ManageArtistDialog.tsx @@ -0,0 +1,173 @@ +import { useState, useEffect } from 'react' +import { useMutation, useQueryClient } from '@tanstack/react-query' +import { artistApi } from '@/api/client' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog' +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { Label } from '@/components/ui/label' +import { Badge } from '@/components/ui/badge' +import { Loader2, X, Plus } from 'lucide-react' +import { toast } from 'sonner' + +interface ArtistWithAliases { + name: string + song_count: number + aliases: string[] +} + +interface ManageArtistDialogProps { + artist: ArtistWithAliases | null + open: boolean + onClose: () => void +} + +export default function ManageArtistDialog({ artist, open, onClose }: ManageArtistDialogProps) { + const [displayName, setDisplayName] = useState('') + const [aliases, setAliases] = useState([]) + const [newAlias, setNewAlias] = useState('') + const queryClient = useQueryClient() + + useEffect(() => { + if (artist) { + setDisplayName(artist.name) + setAliases(artist.aliases || []) + } + }, [artist]) + + const mergeMutation = useMutation({ + mutationFn: async (data: { display_name: string; alias_names: string[] }) => { + const response = await artistApi.mergeArtists(data) + return response.data + }, + onSuccess: (data) => { + toast.success(data.message || 'Artists merged successfully') + queryClient.invalidateQueries({ queryKey: ['artists'] }) + queryClient.invalidateQueries({ queryKey: ['artist-all-songs'] }) + queryClient.invalidateQueries({ queryKey: ['music'] }) + onClose() + }, + onError: () => { + toast.error('Failed to merge artists') + }, + }) + + const handleAddAlias = () => { + const trimmed = newAlias.trim() + if (trimmed && !aliases.includes(trimmed) && trimmed !== displayName) { + setAliases([...aliases, trimmed]) + setNewAlias('') + } + } + + const handleRemoveAlias = (alias: string) => { + setAliases(aliases.filter(a => a !== alias)) + } + + const handleSubmit = (e: React.FormEvent) => { + e.preventDefault() + if (!displayName.trim()) { + toast.error('Display name is required') + return + } + + mergeMutation.mutate({ + display_name: displayName.trim(), + alias_names: aliases, + }) + } + + return ( + + + + Manage Artist + + Set the main display name and add aliases to merge artists with different names. + After merging, all songs will be grouped under the display name. + + + +
+
+ + setDisplayName(e.target.value)} + placeholder="Enter main artist name" + required + /> +

+ This will be the primary name shown everywhere +

+
+ +
+ +
+ setNewAlias(e.target.value)} + placeholder="Enter an alias name" + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault() + handleAddAlias() + } + }} + /> + +
+

+ Add all alternative names (e.g., English name, Chinese name, nicknames) +

+
+ + {aliases.length > 0 && ( +
+ +
+ {aliases.map((alias) => ( + + {alias} + + + ))} +
+
+ )} + + + + + +
+ + + ) +} diff --git a/frontend/src/components/music/EditMetadataDialog.tsx b/frontend/src/components/music/EditMetadataDialog.tsx index 2877cb9..f168424 100644 --- a/frontend/src/components/music/EditMetadataDialog.tsx +++ b/frontend/src/components/music/EditMetadataDialog.tsx @@ -56,6 +56,32 @@ export default function EditMetadataDialog({ music, open, onClose }: EditMetadat }, }) + const rescanMutation = useMutation({ + mutationFn: async () => { + if (!music) return + const response = await musicApi.rescan(music.id) + return response.data + }, + onSuccess: (data) => { + if (data) { + setTitle(data.music.title || '') + setArtist(data.music.artist || '') + setAlbum(data.music.album || '') + toast.success(`Rescanned: ${data.old_artist} → ${data.new_artist}`) + } + queryClient.invalidateQueries({ queryKey: ['music'] }) + queryClient.invalidateQueries({ queryKey: ['artist-all-songs'] }) + queryClient.invalidateQueries({ queryKey: ['playlists'] }) + }, + onError: () => { + toast.error('Failed to rescan metadata') + }, + }) + + const handleRescan = () => { + rescanMutation.mutate() + } + const handleSubmit = (e: React.FormEvent) => { e.preventDefault() updateMutation.mutate({ @@ -120,14 +146,25 @@ export default function EditMetadataDialog({ music, open, onClose }: EditMetadat /> - - - +
+ + +
diff --git a/frontend/src/components/player/ArtistLinks.tsx b/frontend/src/components/player/ArtistLinks.tsx new file mode 100644 index 0000000..3bc5cf7 --- /dev/null +++ b/frontend/src/components/player/ArtistLinks.tsx @@ -0,0 +1,65 @@ +import { useNavigate } from 'react-router-dom' + +interface ArtistLinksProps { + artist: string | null | undefined + musicId?: number + className?: string + maxDisplay?: number +} + +export default function ArtistLinks({ artist, musicId, className = '', maxDisplay }: ArtistLinksProps) { + const navigate = useNavigate() + + if (!artist || artist === 'Unknown') { + return Unknown Artist + } + + // Split by comma or slash to get individual artists + const artists = artist.split(/[,/]/).map(a => a.trim()).filter(a => a) + + if (artists.length === 0) { + return Unknown Artist + } + + // If only one artist, simple link + if (artists.length === 1) { + return ( + + ) + } + + // Multiple artists - show as separate links + const displayArtists = maxDisplay && artists.length > maxDisplay + ? artists.slice(0, maxDisplay) + : artists + const remaining = maxDisplay && artists.length > maxDisplay + ? artists.length - maxDisplay + : 0 + + return ( + + {displayArtists.map((artistName, index) => ( + + + {index < displayArtists.length - 1 && , } + + ))} + {remaining > 0 && ( + +{remaining} + )} + + ) +} diff --git a/frontend/src/components/player/FullScreenPlayer.tsx b/frontend/src/components/player/FullScreenPlayer.tsx index d7e50a8..4cceab3 100644 --- a/frontend/src/components/player/FullScreenPlayer.tsx +++ b/frontend/src/components/player/FullScreenPlayer.tsx @@ -6,6 +6,7 @@ import { formatDuration } from '@/lib/utils' import { useQuery } from '@tanstack/react-query' import { musicApi } from '@/api/client' import React from 'react' +import ArtistLinks from './ArtistLinks' interface FullScreenPlayerProps { currentMusic: Music @@ -20,7 +21,7 @@ interface FullScreenPlayerProps { onToggleLike: () => void onAddToPlaylist: () => void onShare: () => void - onNavigateToArtist: () => void + onNavigateToArtist?: () => void // Optional now, not used but kept for backward compatibility showLyrics?: boolean onToggleLyrics?: () => void onSeek?: (value: number[]) => void @@ -41,7 +42,7 @@ export default function FullScreenPlayer({ onToggleLike, onAddToPlaylist, onShare, - onNavigateToArtist, + // onNavigateToArtist is not used anymore, using ArtistLinks component instead showLyrics = false, onToggleLyrics, onSeek, @@ -128,18 +129,12 @@ export default function FullScreenPlayer({ {/* Song Info */}

{currentMusic.title}

- {currentMusic.artist && currentMusic.artist !== 'Unknown' ? ( - - ) : ( -

- {currentMusic.artist || 'Unknown Artist'} -

- )} +
+ +
{currentMusic.album && (

{currentMusic.album}

)} diff --git a/frontend/src/components/player/Player.tsx b/frontend/src/components/player/Player.tsx index e06adb2..03075e3 100644 --- a/frontend/src/components/player/Player.tsx +++ b/frontend/src/components/player/Player.tsx @@ -4,12 +4,14 @@ import { useNavigate } from 'react-router-dom' import { Music } from '@/types' import { Slider } from '@/components/ui/slider' import { Button } from '@/components/ui/button' -import { Play, Pause, SkipBack, SkipForward, Volume2, VolumeX, Heart, ListPlus, Repeat, Repeat1, Shuffle, Share2, Maximize2, MessageSquareText } from 'lucide-react' +import { Play, Pause, SkipBack, SkipForward, Volume2, VolumeX, Heart, ListPlus, Repeat, Repeat1, Shuffle, Share2, Maximize2, MessageSquareText, Edit } from 'lucide-react' import { formatDuration } from '@/lib/utils' import { playlistApi, musicApi } from '@/api/client' import { toast } from 'sonner' import PlaylistSelector from './PlaylistSelector' import FullScreenPlayer from './FullScreenPlayer' +import EditMetadataDialog from '../music/EditMetadataDialog' +import ArtistLinks from './ArtistLinks' interface PlayerProps { currentMusic: Music | null @@ -40,6 +42,7 @@ export default function Player({ const [showFullScreen, setShowFullScreen] = useState(false) const [showLyrics, setShowLyrics] = useState(false) const [isLiked, setIsLiked] = useState(false) + const [showEditMetadata, setShowEditMetadata] = useState(false) const queryClient = useQueryClient() const navigate = useNavigate() @@ -280,26 +283,17 @@ export default function Player({

{ - if (currentMusic.artist && currentMusic.artist !== 'Unknown') { - navigate(`/artists/${encodeURIComponent(currentMusic.artist)}?highlight=${currentMusic.id}`) - } - }} + onClick={() => setShowFullScreen(true)} > {currentMusic.title}

- {currentMusic.artist && currentMusic.artist !== 'Unknown' ? ( - - ) : ( -

- {currentMusic.artist || 'Unknown Artist'} -

- )} +
+ +
@@ -354,6 +348,16 @@ export default function Player({ > + + )} @@ -476,6 +480,15 @@ export default function Player({ onTogglePlayMode={onTogglePlayMode} /> )} + + {/* Edit Metadata Dialog */} + {showEditMetadata && currentMusic && ( + setShowEditMetadata(false)} + /> + )} ) } diff --git a/frontend/src/lib/__tests__/utils.test.ts.skip b/frontend/src/lib/__tests__/utils.test.ts.skip new file mode 100644 index 0000000..83cee11 --- /dev/null +++ b/frontend/src/lib/__tests__/utils.test.ts.skip @@ -0,0 +1,139 @@ +/** + * Unit tests for artist formatting utility. + * + * This ensures that multi-artist names are displayed correctly: + * - All artists shown when space permits + * - Smart truncation with "FirstArtist, +N" when space is limited + * - Proper handling of single vs multiple artists + */ + +import { formatArtist } from '../utils' + +describe('formatArtist', () => { + describe('Basic functionality', () => { + test('returns "Unknown Artist" for null or undefined', () => { + expect(formatArtist(null)).toBe('Unknown Artist') + expect(formatArtist(undefined)).toBe('Unknown Artist') + expect(formatArtist('Unknown')).toBe('Unknown Artist') + }) + + test('returns artist name as-is when no maxLength specified', () => { + expect(formatArtist('Jay Chou')).toBe('Jay Chou') + expect(formatArtist('Taylor Swift, Ed Sheeran')).toBe('Taylor Swift, Ed Sheeran') + expect(formatArtist('蒋明, 冬子, 刘东明')).toBe('蒋明, 冬子, 刘东明') + }) + + test('returns artist name as-is when within maxLength', () => { + expect(formatArtist('Jay Chou', 50)).toBe('Jay Chou') + expect(formatArtist('Taylor Swift', 20)).toBe('Taylor Swift') + expect(formatArtist('蒋明', 10)).toBe('蒋明') + }) + }) + + describe('Single artist truncation', () => { + test('truncates single artist with ellipsis when too long', () => { + expect(formatArtist('Very Long Artist Name Here', 15)).toBe('Very Long Ar...') + expect(formatArtist('周杰伦的完整艺名很长', 10)).toBe('周杰伦的完整...') + }) + + test('truncates to exactly maxLength characters', () => { + const result = formatArtist('This is a very long artist name', 20) + expect(result.length).toBe(20) + expect(result.endsWith('...')).toBe(true) + }) + }) + + describe('Multiple artists display', () => { + test('shows all artists when space permits', () => { + expect(formatArtist('A, B, C', 50)).toBe('A, B, C') + expect(formatArtist('蒋明, 冬子', 20)).toBe('蒋明, 冬子') + expect(formatArtist('Taylor Swift, Ed Sheeran', 30)).toBe('Taylor Swift, Ed Sheeran') + }) + + test('shows "FirstArtist, +N" format when space is limited', () => { + expect(formatArtist('蒋明, 冬子, 刘东明', 10)).toBe('蒋明, +2') + expect(formatArtist('A, B, C, D, E', 8)).toBe('A, +4') + expect(formatArtist('Taylor Swift, Ed Sheeran, Artist3', 20)).toBe('Taylor Swift, +2') + }) + + test('handles comma-separated artists', () => { + const artists = '蒋明, 冬子, 刘东明, 好妹妹乐队, 钟立风, 小河' + expect(formatArtist(artists, 15)).toBe('蒋明, +5') + }) + + test('handles slash-separated artists', () => { + const artists = 'Justin Timberlake/Carey Mulligan/Stark Sands' + expect(formatArtist(artists, 25)).toBe('Justin Timberlake, +2') + }) + }) + + describe('Edge cases', () => { + test('handles very short maxLength', () => { + expect(formatArtist('A, B, C, D, E, F', 5)).toBe('+6') + expect(formatArtist('蒋明, 冬子, 刘东明', 3)).toBe('+3') + }) + + test('handles two artists', () => { + expect(formatArtist('Artist A, Artist B', 20)).toBe('Artist A, +1') + expect(formatArtist('A, B', 50)).toBe('A, B') + }) + + test('handles single artist that equals maxLength', () => { + expect(formatArtist('TenChars!!', 10)).toBe('TenChars!!') + }) + + test('handles artist with special characters', () => { + expect(formatArtist('Björk, Sigur Rós', 20)).toBe('Björk, Sigur Rós') + expect(formatArtist('Café Tacvba, Molotov', 15)).toBe('Café Tacvba, +1') + }) + }) + + describe('Real-world examples', () => { + test('formats real multi-artist songs correctly', () => { + // From actual music library + expect(formatArtist('Justin Timberlake, Carey Mulligan, Stark Sands', 30)) + .toBe('Justin Timberlake, +2') + + expect(formatArtist('Calvin Harris, John Newman', 50)) + .toBe('Calvin Harris, John Newman') + + expect(formatArtist('G.V. Prakash Kumar, Bela Shende', 25)) + .toBe('G.V. Prakash Kumar, +1') + + expect(formatArtist('Mohit Chauhan, Suzanne D\'Mello', 40)) + .toBe('Mohit Chauhan, Suzanne D\'Mello') + }) + + test('handles 6 artists with limited space', () => { + const artists = '蒋明, 冬子, 刘东明, 好妹妹乐队, 钟立风, 小河' + expect(formatArtist(artists, 20)).toBe('蒋明, +5') + expect(formatArtist(artists, 50)).toBe(artists) + }) + + test('formats player display (limited space)', () => { + // Typical player shows ~30 chars + expect(formatArtist('Taylor Swift, Ed Sheeran, Bruno Mars', 30)) + .toBe('Taylor Swift, +2') + + expect(formatArtist('周杰伦, 王力宏, 陶喆, 林俊杰', 15)) + .toBe('周杰伦, +3') + }) + }) + + describe('Consistency with backend normalization', () => { + test('handles backend normalized format (comma-space)', () => { + // Backend normalizes to ", " separator + expect(formatArtist('蒋明, 冬子, 刘东明', 50)) + .toBe('蒋明, 冬子, 刘东明') + + expect(formatArtist('Artist1, Artist2, Artist3', 20)) + .toBe('Artist1, +2') + }) + + test('handles legacy slash format', () => { + // In case some old data still has slashes + expect(formatArtist('A/B/C', 50)).toBe('A/B/C') + expect(formatArtist('蒋明/冬子/刘东明', 10)).toBe('蒋明, +2') + }) + }) +}) diff --git a/frontend/src/lib/utils.ts b/frontend/src/lib/utils.ts index 2c653c8..a23de9b 100644 --- a/frontend/src/lib/utils.ts +++ b/frontend/src/lib/utils.ts @@ -11,3 +11,39 @@ export function formatDuration(seconds: number): string { const secs = Math.floor(seconds % 60) return `${mins}:${String(secs).padStart(2, '0')}` } + +export function formatArtist(artist: string | null | undefined, maxLength?: number): string { + if (!artist || artist === 'Unknown') return 'Unknown Artist' + + // If no max length specified, return as is + if (!maxLength) return artist + + // If within length, return as is + if (artist.length <= maxLength) return artist + + // Split by comma or slash to get individual artists + const artists = artist.split(/[,/]/).map(a => a.trim()) + + // If single artist, truncate with ellipsis + if (artists.length === 1) { + return artist.substring(0, maxLength - 3) + '...' + } + + // For multiple artists, show "FirstArtist, ..." + const first = artists[0] + const remaining = artists.length - 1 + const formatted = `${first}, +${remaining}` + + if (formatted.length <= maxLength) { + return formatted + } + + // If even that's too long, truncate the first artist + const available = maxLength - `, +${remaining}`.length - 3 + if (available > 0) { + return `${first.substring(0, available)}..., +${remaining}` + } + + // Last resort + return `+${artists.length}` +} diff --git a/frontend/vitest.config.ts b/frontend/vitest.config.ts new file mode 100644 index 0000000..85556da --- /dev/null +++ b/frontend/vitest.config.ts @@ -0,0 +1,18 @@ +import { defineConfig } from 'vite' +import react from '@vitejs/plugin-react' +import path from 'path' + +// https://vitejs.dev/config/ +export default defineConfig({ + plugins: [react()], + resolve: { + alias: { + '@': path.resolve(__dirname, './src'), + }, + }, + test: { + globals: true, + environment: 'jsdom', + setupFiles: './src/test/setup.ts', + }, +})