diff --git a/CHANGELOG.md b/CHANGELOG.md index b7116d3..165cc89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to Voicebox will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- **Profile Name Validation** - Added proper validation to prevent duplicate profile names ([#134](https://github.com/jamiepine/voicebox/issues/134)) + - Users now receive clear error messages when attempting to create or update profiles with duplicate names + - Improved error handling in create and update profile API endpoints + - Added comprehensive test suite for duplicate name validation + ## [0.1.0] - 2026-01-25 ### Added diff --git a/backend/main.py b/backend/main.py index e218d23..c3ad18f 100644 --- a/backend/main.py +++ b/backend/main.py @@ -221,7 +221,10 @@ async def create_profile( """Create a new voice profile.""" try: return await profiles.create_profile(data, db) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) except Exception as e: + # Fallback for unexpected errors raise HTTPException(status_code=400, detail=str(e)) @@ -277,10 +280,13 @@ async def update_profile( db: Session = Depends(get_db), ): """Update a voice profile.""" - profile = await profiles.update_profile(profile_id, data, db) - if not profile: - raise HTTPException(status_code=404, detail="Profile not found") - return profile + try: + profile = await profiles.update_profile(profile_id, data, db) + if not profile: + raise HTTPException(status_code=404, detail="Profile not found") + return profile + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) @app.delete("/profiles/{profile_id}") diff --git a/backend/profiles.py b/backend/profiles.py index cda6bee..7b4cc93 100644 --- a/backend/profiles.py +++ b/backend/profiles.py @@ -38,14 +38,22 @@ async def create_profile( ) -> VoiceProfileResponse: """ Create a new voice profile. - + Args: data: Profile creation data db: Database session - + Returns: Created profile + + Raises: + ValueError: If a profile with the same name already exists """ + # Check if profile name already exists + existing_profile = db.query(DBVoiceProfile).filter_by(name=data.name).first() + if existing_profile: + raise ValueError(f"A profile with the name '{data.name}' already exists. Please choose a different name.") + # Create profile in database db_profile = DBVoiceProfile( id=str(uuid.uuid4()), @@ -55,15 +63,15 @@ async def create_profile( created_at=datetime.utcnow(), updated_at=datetime.utcnow(), ) - + db.add(db_profile) db.commit() db.refresh(db_profile) - + # Create profile directory profile_dir = _get_profiles_dir() / db_profile.id profile_dir.mkdir(parents=True, exist_ok=True) - + return VoiceProfileResponse.model_validate(db_profile) @@ -191,28 +199,37 @@ async def update_profile( ) -> Optional[VoiceProfileResponse]: """ Update a voice profile. - + Args: profile_id: Profile ID data: Updated profile data db: Database session - + Returns: Updated profile or None if not found + + Raises: + ValueError: If a profile with the same name already exists (different profile) """ profile = db.query(DBVoiceProfile).filter_by(id=profile_id).first() if not profile: return None - + + # Check if the new name conflicts with another profile + if profile.name != data.name: + existing_profile = db.query(DBVoiceProfile).filter_by(name=data.name).first() + if existing_profile: + raise ValueError(f"A profile with the name '{data.name}' already exists. Please choose a different name.") + # Update fields profile.name = data.name profile.description = data.description profile.language = data.language profile.updated_at = datetime.utcnow() - + db.commit() db.refresh(profile) - + return VoiceProfileResponse.model_validate(profile) diff --git a/backend/tests/test_profile_duplicate_names.py b/backend/tests/test_profile_duplicate_names.py new file mode 100644 index 0000000..81d4f7e --- /dev/null +++ b/backend/tests/test_profile_duplicate_names.py @@ -0,0 +1,217 @@ +""" +Tests for profile duplicate name validation. + +This test suite verifies that the application correctly handles +duplicate profile names and provides user-friendly error messages. +""" + +import pytest +import tempfile +import shutil +from pathlib import Path +from sqlalchemy import create_engine +from sqlalchemy.orm import sessionmaker + +# Add parent directory to path to import backend modules +import sys +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from database import Base, VoiceProfile as DBVoiceProfile +from models import VoiceProfileCreate +from profiles import create_profile, update_profile + + +@pytest.fixture +def test_db(): + """Create a temporary test database.""" + # Create temporary directory for test database + temp_dir = tempfile.mkdtemp() + db_path = Path(temp_dir) / "test.db" + + # Create engine and session + engine = create_engine(f"sqlite:///{db_path}") + Base.metadata.create_all(bind=engine) + SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) + + db = SessionLocal() + + yield db + + # Cleanup + db.close() + shutil.rmtree(temp_dir) + + +@pytest.fixture +def mock_profiles_dir(monkeypatch, tmp_path): + """Mock the profiles directory to use a temporary path.""" + import profiles + monkeypatch.setattr(profiles, '_get_profiles_dir', lambda: tmp_path) + return tmp_path + + +@pytest.mark.asyncio +async def test_create_profile_duplicate_name_raises_error(test_db, mock_profiles_dir): + """Test that creating a profile with a duplicate name raises a ValueError.""" + # Create first profile + profile_data_1 = VoiceProfileCreate( + name="Test Profile", + description="First profile", + language="en" + ) + + profile_1 = await create_profile(profile_data_1, test_db) + assert profile_1.name == "Test Profile" + + # Try to create second profile with same name + profile_data_2 = VoiceProfileCreate( + name="Test Profile", + description="Second profile", + language="en" + ) + + with pytest.raises(ValueError) as exc_info: + await create_profile(profile_data_2, test_db) + + # Verify error message is user-friendly + assert "already exists" in str(exc_info.value) + assert "Test Profile" in str(exc_info.value) + assert "choose a different name" in str(exc_info.value).lower() + + +@pytest.mark.asyncio +async def test_create_profile_different_names_succeeds(test_db, mock_profiles_dir): + """Test that creating profiles with different names succeeds.""" + # Create first profile + profile_data_1 = VoiceProfileCreate( + name="Profile One", + description="First profile", + language="en" + ) + + profile_1 = await create_profile(profile_data_1, test_db) + assert profile_1.name == "Profile One" + + # Create second profile with different name + profile_data_2 = VoiceProfileCreate( + name="Profile Two", + description="Second profile", + language="en" + ) + + profile_2 = await create_profile(profile_data_2, test_db) + assert profile_2.name == "Profile Two" + + # Verify both profiles exist + assert profile_1.id != profile_2.id + + +@pytest.mark.asyncio +async def test_update_profile_to_duplicate_name_raises_error(test_db, mock_profiles_dir): + """Test that updating a profile to a duplicate name raises a ValueError.""" + # Create two profiles with different names + profile_data_1 = VoiceProfileCreate( + name="Profile A", + description="First profile", + language="en" + ) + profile_1 = await create_profile(profile_data_1, test_db) + + profile_data_2 = VoiceProfileCreate( + name="Profile B", + description="Second profile", + language="en" + ) + profile_2 = await create_profile(profile_data_2, test_db) + + # Try to update profile_2 to use profile_1's name + update_data = VoiceProfileCreate( + name="Profile A", # Duplicate name + description="Updated description", + language="en" + ) + + with pytest.raises(ValueError) as exc_info: + await update_profile(profile_2.id, update_data, test_db) + + # Verify error message is user-friendly + assert "already exists" in str(exc_info.value) + assert "Profile A" in str(exc_info.value) + + +@pytest.mark.asyncio +async def test_update_profile_keep_same_name_succeeds(test_db, mock_profiles_dir): + """Test that updating a profile while keeping the same name succeeds.""" + # Create profile + profile_data = VoiceProfileCreate( + name="My Profile", + description="Original description", + language="en" + ) + profile = await create_profile(profile_data, test_db) + + # Update profile with same name but different description + update_data = VoiceProfileCreate( + name="My Profile", # Same name + description="Updated description", + language="en" + ) + + updated_profile = await update_profile(profile.id, update_data, test_db) + + # Verify update succeeded + assert updated_profile is not None + assert updated_profile.id == profile.id + assert updated_profile.name == "My Profile" + assert updated_profile.description == "Updated description" + + +@pytest.mark.asyncio +async def test_update_profile_to_new_unique_name_succeeds(test_db, mock_profiles_dir): + """Test that updating a profile to a new unique name succeeds.""" + # Create profile + profile_data = VoiceProfileCreate( + name="Original Name", + description="Profile description", + language="en" + ) + profile = await create_profile(profile_data, test_db) + + # Update profile with new unique name + update_data = VoiceProfileCreate( + name="New Unique Name", + description="Updated description", + language="en" + ) + + updated_profile = await update_profile(profile.id, update_data, test_db) + + # Verify update succeeded + assert updated_profile is not None + assert updated_profile.id == profile.id + assert updated_profile.name == "New Unique Name" + + +@pytest.mark.asyncio +async def test_case_sensitive_names_allowed(test_db, mock_profiles_dir): + """Test that profile names are case-sensitive (e.g., 'Test' and 'test' are different).""" + # Create profile with lowercase name + profile_data_1 = VoiceProfileCreate( + name="test profile", + description="Lowercase", + language="en" + ) + profile_1 = await create_profile(profile_data_1, test_db) + + # Create profile with different case + profile_data_2 = VoiceProfileCreate( + name="Test Profile", + description="Title case", + language="en" + ) + profile_2 = await create_profile(profile_data_2, test_db) + + # Both should succeed since SQLite unique constraint is case-sensitive by default + assert profile_1.name == "test profile" + assert profile_2.name == "Test Profile" + assert profile_1.id != profile_2.id