feat: Change API status code for not found bookmarks (#387) (#418)

backend/app/api/api_v1/endpoints/bookmarks.py

@@ -1,4 +1,6 @@
-from fastapi import APIRouter, Depends, HTTPException
+from typing import Annotated
+
+from fastapi import APIRouter, Depends, Header, HTTPException, Response
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app import crud, schemas
@@ -114,6 +116,7 @@ async def get_bookmark_for_user(
     obj_id: str,
     db: AsyncSession = Depends(deps.get_db),
     user: User = Depends(current_active_user),
+    user_agent: Annotated[str | None, Header()] = None,
 ):
     """
     Get a bookmark for a current user by obj_type and obj_id.
@@ -124,12 +127,18 @@ async def get_bookmark_for_user(
     :type obj_id: uuid
     :return: bookmark
     :rtype: dict
+    :raises HTTPException 404: if bookmark not found
+    :note: if user_agent is browser, return 204
     """
     result = await crud.bookmark.get_by_user_and_obj(
         db, user_id=user.id, obj_type=obj_type, obj_id=obj_id
     )
     if not result:  # pragma: no cover
-        raise HTTPException(status_code=404, detail="Bookmark not found")
+        # if user_agent is browser, return 204, else 404
+        if user_agent and "Mozilla" in user_agent:
+            raise HTTPException(status_code=204)
+        else:
+            raise HTTPException(status_code=404, detail="Bookmark not found")
     else:
         return result
 
@@ -140,6 +149,7 @@ async def delete_bookmark_for_user(
     obj_id: str,
     db: AsyncSession = Depends(deps.get_db),
     user: User = Depends(current_active_user),
+    user_agent: Annotated[str | None, Header()] = None,
 ):
     """
     Delete a bookmark for a current user by obj_type and obj_id.
@@ -150,12 +160,17 @@ async def delete_bookmark_for_user(
     :type obj_id: uuid
     :return: bookmark which was deleted
     :rtype: dict
-    :raises HTTPException: if bookmark not found
+    :raises HTTPException 404: if bookmark not found
+    :note: if user_agent is browser, return 204 Response
     """
     bookmark = await crud.bookmark.get_by_user_and_obj(
         db, user_id=user.id, obj_type=obj_type, obj_id=obj_id
     )
     if bookmark:
         return await crud.bookmark.remove(db, id=bookmark.id)
     else:  # pragma: no cover
-        raise HTTPException(status_code=404, detail="Bookmark not found")
+        # if user_agent is browser, return 204, else 404
+        if user_agent and "Mozilla" in user_agent:
+            return Response(status_code=204)
+        else:
+            raise HTTPException(status_code=404, detail="Bookmark not found")

backend/tests/api/api_v1/test_bookmarks.py

@@ -498,10 +498,31 @@ async def test_get_no_bookmarks(
         f"{settings.API_V1_STR}/bookmarks/get?obj_type=gene&obj_id={obj_names.gene[0]}"
     )
     # assert:
+    # Status code is 404 because we use internal agent
     assert response.status_code == 404
     assert response.json() == {"detail": "Bookmark not found"}
 
 
+@pytest.mark.anyio
+@pytest.mark.parametrize("test_user, client_user", [(SUPER, SUPER)], indirect=True)
+async def test_get_no_bookmark_with_browser_header(
+    db_session: AsyncSession, client_user: TestClient, obj_names: ObjNames
+):
+    """
+    Test getting a bookmark as superuser when there are no bookmarks by simulating the browser
+    behaviour.
+    """
+    _ = db_session
+    # act:
+    response = client_user.get(
+        f"{settings.API_V1_STR}/bookmarks/get?obj_type=gene&obj_id={obj_names.gene[0]}",
+        headers={"user-agent": "Mozilla/5.0"},
+    )
+    # assert:
+    # Status code is 204 because we use browser agent
+    assert response.status_code == 204
+
+
 # ------------------------------------------------------------------------------
 # api/v1/bookmarks/delete
 # ------------------------------------------------------------------------------
@@ -591,5 +612,26 @@ async def test_delete_no_bookmarks(
         f"{settings.API_V1_STR}/bookmarks/delete?obj_type=gene&obj_id={obj_names.gene[0]}"
     )
     # assert:
+    # Status code is 404 because we use internal agent
     assert response.status_code == 404
     assert response.json() == {"detail": "Bookmark not found"}
+
+
+@pytest.mark.anyio
+@pytest.mark.parametrize("test_user, client_user", [(SUPER, SUPER)], indirect=True)
+async def test_delete_no_bookmark_with_browser_header(
+    db_session: AsyncSession, client_user: TestClient, obj_names: ObjNames
+):
+    """
+    Test deleting a bookmark as superuser when there are no bookmarks by simulating the browser
+    behaviour.
+    """
+    _ = db_session
+    # act:
+    response = client_user.delete(
+        f"{settings.API_V1_STR}/bookmarks/delete?obj_type=gene&obj_id={obj_names.gene[0]}",
+        headers={"user-agent": "Mozilla/5.0"},
+    )
+    # assert:
+    # Status code is 204 because we use browser agent
+    assert response.status_code == 204

frontend/src/api/bookmarks.ts

@@ -43,6 +43,9 @@ export class BookmarksClient {
       mode: 'cors',
       credentials: 'include'
     })
+    if (response.status === 204) {
+      return null
+    }
     return await response.json()
   }
 

frontend/src/stores/bookmarks.ts

@@ -66,13 +66,14 @@ export const useBookmarksStore = defineStore('bookmarks', () => {
     try {
       const client = new BookmarksClient()
       const response = await client.fetchBookmark(obj_type, obj_id)
-      if (response.detail === 'Unauthorized') {
-        storeState.value = StoreState.Error
+      if (response === null) {
         return null
-      } else if (response.detail === 'Bookmark not found') {
+      } else if (response.detail === 'Unauthorized') {
+        storeState.value = StoreState.Error
         return null
+      } else {
+        return response
       }
-      return response
     } catch (e) {
       storeState.value = StoreState.Error
       return null