Song and category edit fixes + tests

routes/song_category_router.py

@@ -1,3 +1,5 @@
+from sqlalchemy import func
+
 from database import DB_dependency
 from api_schemas.song_category_schemas import SongCategoryCreate, SongCategoryRead
 from db_models.song_category_model import SongCategory_DB
@@ -23,8 +25,12 @@ def get_song_category(category_id: int, db: DB_dependency):
 
 @song_category_router.post("/", response_model=SongCategoryRead, dependencies=[Permission.require("manage", "Song")])
 def create_song_category(song_category_data: SongCategoryCreate, db: DB_dependency):
-    num_existing = db.query(SongCategory_DB).filter(SongCategory_DB.name == song_category_data.name).count()
-    if num_existing > 0:
+    same_title = (
+        db.query(SongCategory_DB.id)
+        .filter(func.lower(SongCategory_DB.name) == func.lower(song_category_data.name))
+        .first()
+    )
+    if same_title is not None:
         raise HTTPException(status.HTTP_400_BAD_REQUEST, detail="This song category already exists")
     songcategory = SongCategory_DB(name=song_category_data.name)
     db.add(songcategory)
@@ -56,6 +62,14 @@ def update_song_category(category_id: int, category_data: SongCategoryCreate, db
     category = db.query(SongCategory_DB).filter_by(id=category_id).one_or_none()
     if category is None:
         raise HTTPException(status.HTTP_404_NOT_FOUND)
+    same_title = (
+        db.query(SongCategory_DB.id)
+        .filter(func.lower(SongCategory_DB.name) == func.lower(category_data.name), SongCategory_DB.id != category_id)
+        .first()
+    )
+    if same_title is not None:
+        raise HTTPException(status.HTTP_400_BAD_REQUEST, detail="This song category already exists")
+
     category.name = category_data.name
     db.commit()
     return category

routes/song_router.py

@@ -1,10 +1,10 @@
 from fastapi import APIRouter, HTTPException, status
+from sqlalchemy import func
 from api_schemas.song_schemas import SongCreate, SongRead
 from database import DB_dependency
 from db_models.song_model import Song_DB
 from user.permission import Permission
 
-
 song_router = APIRouter()
 
 
@@ -29,8 +29,8 @@ def get_song(song_id: int, db: DB_dependency):
 
 @song_router.post("/", response_model=SongRead, dependencies=[Permission.require("manage", "Song")])
 def create_song(song_data: SongCreate, db: DB_dependency):
-    num_existing = db.query(Song_DB).filter(Song_DB.title == song_data.title).count()
-    if num_existing > 0:
+    same_title = db.query(Song_DB.id).filter(func.lower(Song_DB.title) == func.lower(song_data.title)).first()
+    if same_title is not None:
         raise HTTPException(status.HTTP_400_BAD_REQUEST, detail="This song already exists")
     song = Song_DB(
         title=song_data.title,
@@ -61,8 +61,12 @@ def update_song(song_id: int, song_data: SongCreate, db: DB_dependency):
         raise HTTPException(status.HTTP_404_NOT_FOUND)
 
     # simply check if the title is being changed to an already existing title, if so, throw an error.
-    num_existing = db.query(Song_DB).filter(Song_DB.title == song_data.title).count()
-    if num_existing > 0:
+    same_title = (
+        db.query(Song_DB.id)
+        .filter(func.lower(Song_DB.title) == func.lower(song_data.title), Song_DB.id != song_id)
+        .first()
+    )
+    if same_title is not None:
         raise HTTPException(status.HTTP_400_BAD_REQUEST, detail="This song already exists")
     # Allow for partial updates (kinda). We handle melody separately since we want to allow for None
     for var, value in vars(song_data).items():

tests/basic_factories.py

@@ -50,6 +50,21 @@ def auth_headers(token):
     return {"Authorization": f"Bearer {token}"}
 
 
+def category_data(name="Testkategori"):
+    return {"name": name}
+
+
+def song_data(category_id, **kwargs):
+    data = {
+        "title": "Testsång",
+        "author": "Testförfattare",
+        "melody": "Testmelodi",
+        "content": "Testsångtext",
+        "category_id": category_id,
+    }
+    return {**data, **kwargs}
+
+
 def council_data_factory(**kwargs):
     """Factory for council create/update payloads."""
     default_data = {

tests/basic_fixtures.py

@@ -296,6 +296,24 @@ def event(client, admin_token, admin_council_id):
     return response.json()
 
 
+@pytest.fixture()
+def song_category(client, admin_token):
+    """Create and return a song category."""
+
+    response = client.post("/songs-category/", json=category_data(), headers=auth_headers(admin_token))
+    assert response.status_code in (200, 201), response.text
+    return response.json()
+
+
+@pytest.fixture()
+def song(client, admin_token, song_category):
+    """Create and return a song in the shared song category."""
+
+    response = client.post("/songs/", json=song_data(song_category["id"]), headers=auth_headers(admin_token))
+    assert response.status_code in (200, 201), response.text
+    return response.json()
+
+
 @pytest.fixture()
 def nollning_event(client, admin_token, admin_council_id):
     """Create and return a nollning event which only accepts groups of type "Mentor"."""

tests/test_song_categories.py

@@ -0,0 +1,115 @@
+# type: ignore
+import pytest
+from .basic_factories import auth_headers, category_data
+
+
+class TestCreateSongCategory:
+    """Test POST /songs-category/ endpoint"""
+
+    def test_create_song_category_success(self, client, admin_token):
+        response = client.post(
+            "/songs-category/",
+            json=category_data("Visor"),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code in (200, 201), response.text
+        assert response.json()["name"] == "Visor"
+
+    def test_create_song_category_duplicate_is_case_insensitive(self, client, admin_token, song_category):
+        response = client.post(
+            "/songs-category/",
+            json=category_data(song_category["name"].upper()),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 400
+
+    @pytest.mark.parametrize("token_fixture", ["member_token", "non_member_token"])
+    def test_create_song_category_forbidden(self, client, request, token_fixture):
+        response = client.post(
+            "/songs-category/",
+            json=category_data("Forbidden"),
+            headers=auth_headers(request.getfixturevalue(token_fixture)),
+        )
+
+        assert response.status_code == 403
+
+    def test_create_song_category_unauthenticated(self, client):
+        response = client.post("/songs-category/", json=category_data("Unauthenticated"))
+
+        assert response.status_code == 401
+
+
+class TestGetSongCategories:
+    """Test GET /songs-category/ and GET /songs-category/{category_id} endpoints"""
+
+    def test_get_all_song_categories(self, client, song_category):
+        response = client.get("/songs-category/")
+
+        assert response.status_code == 200
+        assert song_category["id"] in [category["id"] for category in response.json()]
+
+    def test_get_single_song_category(self, client, song_category):
+        response = client.get(f"/songs-category/{song_category['id']}")
+
+        assert response.status_code == 200
+        assert response.json() == song_category
+
+    def test_get_missing_song_category(self, client):
+        response = client.get("/songs-category/999999")
+
+        assert response.status_code == 404
+
+
+class TestUpdateSongCategory:
+    """Test PATCH /songs-category/{category_id} endpoint"""
+
+    def test_update_song_category_success(self, client, admin_token, song_category):
+        response = client.patch(
+            f"/songs-category/{song_category['id']}",
+            json=category_data("Updated Category"),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 200
+        assert response.json()["name"] == "Updated Category"
+
+    def test_update_song_category_duplicate_is_case_insensitive(self, client, admin_token, song_category):
+        other = client.post(
+            "/songs-category/",
+            json=category_data("Other Category"),
+            headers=auth_headers(admin_token),
+        ).json()
+
+        response = client.patch(
+            f"/songs-category/{other['id']}",
+            json=category_data(song_category["name"].lower()),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 400
+
+    def test_update_song_category_forbidden(self, client, member_token, song_category):
+        response = client.patch(
+            f"/songs-category/{song_category['id']}",
+            json=category_data("Forbidden Update"),
+            headers=auth_headers(member_token),
+        )
+
+        assert response.status_code == 403
+
+
+class TestDeleteSongCategory:
+    """Test DELETE /songs-category/{category_id} endpoint"""
+
+    def test_delete_song_category_success(self, client, admin_token, song_category):
+        response = client.delete(f"/songs-category/{song_category['id']}", headers=auth_headers(admin_token))
+
+        assert response.status_code == 200
+        assert client.get(f"/songs-category/{song_category['id']}").status_code == 404
+
+    def test_delete_song_category_forbidden(self, client, member_token, song_category):
+        response = client.delete(f"/songs-category/{song_category['id']}", headers=auth_headers(member_token))
+
+        assert response.status_code == 403

tests/test_songs.py

@@ -0,0 +1,156 @@
+# type: ignore
+import pytest
+from .basic_factories import auth_headers, song_data
+
+
+class TestCreateSong:
+    """Test POST /songs/ endpoint"""
+
+    def test_create_song_success(self, client, admin_token, song_category):
+        data = song_data(song_category["id"])
+        response = client.post("/songs/", json=data, headers=auth_headers(admin_token))
+
+        assert response.status_code in (200, 201), response.text
+        created = response.json()
+        assert created["title"] == data["title"]
+        assert created["author"] == data["author"]
+        assert created["content"] == data["content"]
+        assert created["category"]["id"] == song_category["id"]
+        assert created["views"] == 0
+
+    def test_create_song_duplicate_title_is_rejected(self, client, admin_token, song):
+        response = client.post(
+            "/songs/",
+            json=song_data(song["category"]["id"]),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 400
+
+    def test_create_song_duplicate_title_is_case_insensitive(self, client, admin_token, song):
+        response = client.post(
+            "/songs/",
+            json=song_data(song["category"]["id"], title=song["title"].upper()),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 400
+
+    @pytest.mark.parametrize("token_fixture", ["member_token", "non_member_token"])
+    def test_create_song_forbidden(self, client, request, song_category, token_fixture):
+        response = client.post(
+            "/songs/",
+            json=song_data(song_category["id"]),
+            headers=auth_headers(request.getfixturevalue(token_fixture)),
+        )
+
+        assert response.status_code == 403
+
+    def test_create_song_unauthenticated(self, client, song_category):
+        response = client.post("/songs/", json=song_data(song_category["id"]))
+
+        assert response.status_code == 401
+
+
+class TestGetSongs:
+    """Test GET /songs/ and GET /songs/{song_id} endpoints"""
+
+    def test_get_all_songs(self, client, song):
+        response = client.get("/songs/")
+
+        assert response.status_code == 200
+        assert song["id"] in [listed["id"] for listed in response.json()]
+
+    def test_get_single_song_increments_views(self, client, song):
+        response = client.get(f"/songs/{song['id']}")
+
+        assert response.status_code == 200
+        assert response.json()["title"] == song["title"]
+        assert response.json()["views"] == 1
+
+    def test_get_missing_song(self, client):
+        response = client.get("/songs/999999")
+
+        assert response.status_code == 404
+
+
+class TestUpdateSong:
+    """Test PATCH /songs/{song_id} endpoint"""
+
+    def test_update_song_success(self, client, admin_token, song):
+        response = client.patch(
+            f"/songs/{song['id']}",
+            json=song_data(
+                song["category"]["id"],
+                title="Updated Song",
+                author=None,
+                melody=None,
+                content="Updated lyrics",
+            ),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 200
+        updated = response.json()
+        assert updated["title"] == "Updated Song"
+        assert updated["content"] == "Updated lyrics"
+        assert updated["author"] == song["author"]
+        assert updated["melody"] is None
+
+    def test_update_song_keeping_own_title(self, client, admin_token, song):
+        response = client.patch(
+            f"/songs/{song['id']}",
+            json=song_data(song["category"]["id"], title=song["title"], content="Updated lyrics"),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 200, response.text
+        assert response.json()["title"] == song["title"]
+        assert response.json()["content"] == "Updated lyrics"
+
+    def test_update_song_duplicate_title_is_rejected(self, client, admin_token, song, song_category):
+        other = client.post(
+            "/songs/",
+            json=song_data(song_category["id"], title="Other Song"),
+            headers=auth_headers(admin_token),
+        ).json()
+
+        response = client.patch(
+            f"/songs/{other['id']}",
+            json=song_data(song_category["id"], title=song["title"].upper()),
+            headers=auth_headers(admin_token),
+        )
+
+        assert response.status_code == 400
+
+    def test_update_song_forbidden(self, client, member_token, song):
+        response = client.patch(
+            f"/songs/{song['id']}",
+            json=song_data(song["category"]["id"], title="Forbidden Update"),
+            headers=auth_headers(member_token),
+        )
+
+        assert response.status_code == 403
+
+
+class TestDeleteSong:
+    """Test DELETE /songs/{song_id} endpoint"""
+
+    def test_delete_song_success(self, client, admin_token, song):
+        response = client.delete(f"/songs/{song['id']}", headers=auth_headers(admin_token))
+
+        assert response.status_code == 200
+        assert client.get(f"/songs/{song['id']}").status_code == 404
+
+    def test_delete_song_forbidden(self, client, member_token, song):
+        response = client.delete(f"/songs/{song['id']}", headers=auth_headers(member_token))
+
+        assert response.status_code == 403
+
+    def test_delete_category_detaches_song(self, client, admin_token, song, song_category):
+        response = client.delete(f"/songs-category/{song_category['id']}", headers=auth_headers(admin_token))
+
+        assert response.status_code == 200
+        song_response = client.get(f"/songs/{song['id']}")
+        assert song_response.status_code == 200
+        assert song_response.json()["category"] is None