Conventional dataset tag

docs/migration.md

@@ -129,6 +129,13 @@ The `limit` and `offset` parameters can now be used independently, you no longer
 to provide both if you wish to set only one.
 
 #### `POST /datasets/tag`
+
+???+ warning "This endpoint is deprecated"
+
+    The new tag endpoint is `POST` `/datasets/{identifier}/tags`.
+    The dataset identifier is provided as part of the path, the tag is still provided in the body.
+    This endpoint is provided for easier migration in case the response matters to you.
+
 When successful, the "tag" property in the returned response is now always a list, even if only one tag exists for the entity.
 For example, after tagging dataset 21 with the tag `"foo"`:
 ```diff

docs/roadmap.md

@@ -6,15 +6,20 @@ The sunset date of the PHP-based REST API will depend on the progress of this re
 ## Phase 1: Achieve Feature Parity
 
 The first goal is _rough_ feature parity.
-This means that we want to expose the same endpoints with the same general responses.
-We may deviate in case the original implementation had an oversight or a bug, and fix those along the way.
+This means that we want to expose the same endpoints with the same general responses, but:
+
+- we may deviate in case the original implementation had an oversight or a bug, and fix those along the way.
+- we may drop support for some admin-only or previously undocumented endpoints.
+- we may make other alterations at our discretion, e.g., improve consistency in responses or [RFC9457](https://www.rfc-editor.org/rfc/rfc9457.html) error handling. But we do not significantly restructure "happy path" responses or endpoint structure.
+
 We can be pragmatic about this, for example, copying over a complex query from the PHP API.
 This phase is considered complete when the output of the new REST API can be mapped to output of the old REST API for all endpoints the PHP-based API has and the [migration guide](migration.md) is done.
 
 
 ## Phase 2: Modernizing the API
 
 We are also working on incorporating modern standards like [RFC9457](https://www.rfc-editor.org/rfc/rfc9457.html) for error reporting, better use of JSON types, including authentication in the request's HTTP header instead of the URL, or changing HTTP methods or paths to better reflect the behavior of the endpoint.
+We may also consider schema changes to responses to improve e.g., usability or performance.
 
 ## Phase 3: Revisiting the Data(base)
 

src/routers/datasets.py

@@ -13,7 +13,7 @@
 from http import HTTPStatus
 from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple
 
-from fastapi import APIRouter, Body, Depends, Query
+from fastapi import APIRouter, Body, Depends, Path, Query
 from loguru import logger
 from sqlalchemy import bindparam, text
 
@@ -89,30 +89,45 @@ def _format_dataset_url(dataset: UntypedRow) -> str:
 
 @router.post(
     path="/tag",
+    deprecated=True,
 )
 async def tag_dataset(
     data_id: Annotated[Identifier, Body()],
     tag: Annotated[TagString, Body()],
     user: Annotated[User, Depends(fetch_user_or_raise)],
-    expdb_db: Annotated[AsyncSession, Depends(expdb_session)],
+    expdb: Annotated[AsyncSession, Depends(expdb_session)],
 ) -> dict[str, dict[str, Any]]:
+    """Add a tag to the dataset, this tag is publicly visible to all users."""
+    await tag_dataset_new(data_id, tag, user, expdb)
+
+    tags = await database.datasets.get_tags_for(data_id, expdb)
+
+    return {
+        "data_tag": {"id": str(data_id), "tag": tags},
+    }
+
+
+@router.post(
+    path="/{identifier}/tags",
+    status_code=HTTPStatus.NO_CONTENT,
+)
+async def tag_dataset_new(
+    identifier: Annotated[Identifier, Path()],
+    tag: Annotated[TagString, Body(embed=True)],
+    user: Annotated[User, Depends(fetch_user_or_raise)],
+    expdb: Annotated[AsyncSession, Depends(expdb_session)],
+) -> None:
     """Add a tag to the dataset, this tag is publicly visible to all users."""
     try:
-        await database.datasets.tag(data_id, tag, user_id=user.user_id, session=expdb_db)
+        await database.datasets.tag(identifier, tag, user_id=user.user_id, session=expdb)
     except ForeignKeyConstraintError:
-        msg = f"Dataset {data_id} not found."
+        msg = f"Dataset {identifier} not found."
         raise DatasetNotFoundError(msg, code=472) from None
     except DuplicatePrimaryKeyError:
-        msg = f"Dataset {data_id} already tagged with {tag!r}."
+        msg = f"Dataset {identifier} already tagged with {tag!r}."
         raise TagAlreadyExistsError(msg) from None
 
-    logger.info("Dataset {data_id} tagged '{tag}'.", data_id=data_id, tag=tag)
-
-    tags = await database.datasets.get_tags_for(data_id, expdb_db)
-
-    return {
-        "data_tag": {"id": str(data_id), "tag": tags},
-    }
+    logger.info("Dataset {identifier} tagged '{tag}'.", identifier=identifier, tag=tag)
 
 
 @router.post(path="/untag", deprecated=True)

tests/routers/dataset_tag_test.py

@@ -18,22 +18,41 @@
     from sqlalchemy.ext.asyncio import AsyncSession
 
 
-@pytest.mark.parametrize(
-    "key",
-    [None, ApiKey.INVALID],
-    ids=["no authentication", "invalid key"],
-)
-async def test_dataset_tag_rejects_unauthorized(key: ApiKey, py_api: httpx.AsyncClient) -> None:
-    apikey = "" if key is None else f"?api_key={key}"
+async def test_dataset_tag_requires_authorization(py_api: httpx.AsyncClient) -> None:
     any_dataset_identifier = 1
     response = await py_api.post(
-        f"/datasets/tag{apikey}",
+        "/datasets/tag",
         json={"data_id": any_dataset_identifier, "tag": "test"},
     )
     assert response.status_code == HTTPStatus.UNAUTHORIZED
 
 
-# ── Direct call tests: tag_dataset ──
+async def test_dataset_tag_json(py_api: httpx.AsyncClient, dataset_factory: DatasetFactory) -> None:
+    dataset_id = await dataset_factory()
+    response = await py_api.post(
+        f"/datasets/tag?api_key={ApiKey.SOME_USER}",
+        json={"data_id": dataset_id, "tag": "test"},
+    )
+    assert response.status_code == HTTPStatus.OK
+    expected_json = {
+        "data_tag": {
+            "id": str(dataset_id),
+            "tag": ["test"],
+        }
+    }
+    assert response.json() == expected_json
+
+
+async def test_dataset_tag_new_json(
+    py_api: httpx.AsyncClient, dataset_factory: DatasetFactory
+) -> None:
+    dataset_id = await dataset_factory()
+    response = await py_api.post(
+        f"/datasets/{dataset_id}/tags?api_key={ApiKey.SOME_USER}",
+        json={"tag": "test"},
+    )
+    assert response.status_code == HTTPStatus.NO_CONTENT, response.json()
+    assert not response.content
 
 
 @pytest.mark.mut
@@ -47,7 +66,7 @@ async def test_dataset_tag(
 ) -> None:
     dataset_id = await dataset_factory()
     tag = "test_tag"
-    result = await tag_dataset(data_id=dataset_id, tag=tag, user=user, expdb_db=expdb_session)
+    result = await tag_dataset(data_id=dataset_id, tag=tag, user=user, expdb=expdb_session)
     assert result == {"data_tag": {"id": str(dataset_id), "tag": [tag]}}
 
     tags = await get_tags_for(dataset_id=dataset_id, session=expdb_session)
@@ -59,9 +78,9 @@ async def test_dataset_tag_returns_existing_tags(
     expdb_session: AsyncSession, dataset_factory: DatasetFactory
 ) -> None:
     dataset_id = await dataset_factory()
-    await tag_dataset(data_id=dataset_id, tag="first", user=OWNER_USER, expdb_db=expdb_session)
+    await tag_dataset(data_id=dataset_id, tag="first", user=OWNER_USER, expdb=expdb_session)
     result = await tag_dataset(
-        data_id=dataset_id, tag="second", user=ADMIN_USER, expdb_db=expdb_session
+        data_id=dataset_id, tag="second", user=ADMIN_USER, expdb=expdb_session
     )
     assert result == {"data_tag": {"id": str(dataset_id), "tag": ["first", "second"]}}
 
@@ -72,10 +91,10 @@ async def test_dataset_tag_fails_if_tag_exists(
 ) -> None:
     tag = "repeated_tag"
     dataset_id = await dataset_factory()
-    await tag_dataset(data_id=dataset_id, tag=tag, user=OWNER_USER, expdb_db=expdb_session)
+    await tag_dataset(data_id=dataset_id, tag=tag, user=OWNER_USER, expdb=expdb_session)
 
     with pytest.raises(TagAlreadyExistsError) as e:
-        await tag_dataset(data_id=dataset_id, tag=tag, user=ADMIN_USER, expdb_db=expdb_session)
+        await tag_dataset(data_id=dataset_id, tag=tag, user=ADMIN_USER, expdb=expdb_session)
     assert str(dataset_id) in e.value.detail
     assert tag in e.value.detail
 
@@ -87,7 +106,7 @@ async def test_dataset_tag_fails_if_dataset_does_not_exist(expdb_session: AsyncS
             data_id=dataset_id,
             tag="foo",
             user=ADMIN_USER,
-            expdb_db=expdb_session,
+            expdb=expdb_session,
         )
     assert str(dataset_id) in e.value.detail
     dataset_not_found_in_tag_endpoint = 472