Skip to content

Commit

Permalink
more linting and api polishing, introduce pytest, unfinished commit
Browse files Browse the repository at this point in the history
  • Loading branch information
tykling committed Apr 11, 2024
1 parent a99cd88 commit 31117b9
Show file tree
Hide file tree
Showing 13 changed files with 257 additions and 196 deletions.
7 changes: 7 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ dev = [
test = [
"coverage==7.2.2",
"factory-boy==3.2.1",
"pytest-django==4.8.0",
]

[project.urls]
Expand Down Expand Up @@ -95,3 +96,9 @@ convention = "google"
"*/migrations/*" = [
"D" # https://docs.astral.sh/ruff/rules/#pydocstyle-d
]

[tool.pytest.ini_options]
DJANGO_SETTINGS_MODULE = "bma.settings"
python_files = ["tests.py"]
log_cli = 1
log_cli_level = "DEBUG"
49 changes: 30 additions & 19 deletions src/albums/api.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
"""The albums API."""
import logging
import operator
import uuid
from functools import reduce

from django.core.exceptions import ValidationError
from django.db.models import Q
from django.http import HttpRequest
from django.shortcuts import get_object_or_404
from guardian.shortcuts import assign_perm
from ninja import Query
from ninja import Router
from utils.api import wrap_response
from utils.api import AlbumApiResponseType
from utils.schema import ApiMessageSchema
from utils.schema import ApiResponseSchema

from .filters import AlbumFilters
from .models import Album
Expand All @@ -25,31 +26,39 @@
router = Router()

# https://django-ninja.rest-framework.com/guides/input/query-params/#using-schema
query = Query(...) # type: ignore
query = Query(...) # type: ignore[type-arg]


@router.post(
"/create/",
response={201: SingleAlbumResponseSchema},
response={
201: SingleAlbumResponseSchema,
422: ApiMessageSchema,
},
summary="Create a new album",
)
def album_create(request: HttpRequest, payload: AlbumRequestSchema) -> tuple[int, ApiResponseSchema]:
def album_create(request: HttpRequest, payload: AlbumRequestSchema) -> AlbumApiResponseType:
"""Use this endpoint to create a new album, with or without files."""
album = Album()
for k, v in payload.dict().items():
if k == "files":
album.files.set(v)
else:
setattr(album, k, v)
album.owner = request.user # type: ignore
album.owner = request.user # type: ignore[assignment]

try:
album.full_clean()
except ValidationError:
logger.exception("validation failed")
return 422, ApiMessageSchema(message="Validation error")

album.save()

# assign permissions
# assign permissions and return response
assign_perm("change_album", request.user, album)
assign_perm("delete_album", request.user, album)

# return response
return 200, wrap_response(album)
return 201, {"bma_response": album}


@router.get(
Expand All @@ -58,10 +67,10 @@ def album_create(request: HttpRequest, payload: AlbumRequestSchema) -> tuple[int
summary="Return an album.",
auth=None,
)
def album_get(request: HttpRequest, album_uuid: uuid.UUID) -> tuple[int, ApiResponseSchema]:
def album_get(request: HttpRequest, album_uuid: uuid.UUID) -> AlbumApiResponseType:
"""Return an album."""
album = get_object_or_404(Album, uuid=album_uuid)
return 200, wrap_response(album)
return 200, {"bma_response": album}


@router.get(
Expand All @@ -70,7 +79,7 @@ def album_get(request: HttpRequest, album_uuid: uuid.UUID) -> tuple[int, ApiResp
summary="Return a list of albums.",
auth=None,
)
def album_list(request: HttpRequest, filters: AlbumFilters = query) -> tuple[int, ApiResponseSchema]:
def album_list(request: HttpRequest, filters: AlbumFilters = query) -> AlbumApiResponseType:
"""Return a list of albums."""
albums = Album.objects.all()

Expand Down Expand Up @@ -98,7 +107,7 @@ def album_list(request: HttpRequest, filters: AlbumFilters = query) -> tuple[int
if filters.limit:
albums = albums[: filters.limit]

return 200, wrap_response(albums)
return 200, {"bma_response": albums}


@router.put(
Expand Down Expand Up @@ -127,8 +136,9 @@ def album_update(
request: HttpRequest,
album_uuid: uuid.UUID,
payload: AlbumRequestSchema,
*,
check: bool = False,
) -> tuple[int, ApiMessageSchema | ApiResponseSchema]:
) -> AlbumApiResponseType:
"""Update (PATCH) or replace (PUT) an Album."""
album = get_object_or_404(Album, uuid=album_uuid)
if not request.user.has_perm("change_album", album):
Expand All @@ -150,12 +160,12 @@ def album_update(
if attr == "files":
# handle the m2m seperate
continue
else:
setattr(album, attr, value)
# set the attribute on the album
setattr(album, attr, value)
album.save()
if "files" in payload.dict():
album.files.set(payload.dict()["files"])
return 200, wrap_response(album)
return 200, {"bma_response": album}


@router.delete(
Expand All @@ -164,8 +174,9 @@ def album_update(
summary="Delete an album.",
)
def album_delete(
request: HttpRequest, album_uuid: uuid.UUID, check: bool = False
request: HttpRequest, album_uuid: uuid.UUID, *, check: bool = False
) -> tuple[int, ApiMessageSchema | None]:
"""Delete an album."""
album = get_object_or_404(Album, uuid=album_uuid)
if not request.user.has_perm("delete_album", album):
# no permission
Expand Down
3 changes: 2 additions & 1 deletion src/albums/schema.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Schemas for album API calls."""
import uuid
from collections.abc import Sequence

from django.http import HttpRequest
from django.urls import reverse
Expand All @@ -14,7 +15,7 @@ class AlbumRequestSchema(ModelSchema):

title: str = ""
description: str = ""
files: list[uuid.UUID]
files: Sequence[uuid.UUID] = []

class Config:
"""Set model and fields."""
Expand Down
30 changes: 15 additions & 15 deletions src/albums/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def test_album_create(
content_type="application/json",
)
assert response.status_code == 201
self.album_uuid = response.json()["uuid"]
self.album_uuid = response.json()["bma_response"]["uuid"]

def test_album_create_with_files(
self,
Expand Down Expand Up @@ -84,9 +84,9 @@ def test_album_update(self):
content_type="application/json",
)
assert response.status_code == 200
assert len(response.json()["files"]) == 2
assert response.json()["title"] == "new title"
assert response.json()["description"] == "description here"
assert len(response.json()["bma_response"]["files"]) == 2
assert response.json()["bma_response"]["title"] == "new title"
assert response.json()["bma_response"]["description"] == "description here"

# update the album with more files
response = self.client.patch(
Expand All @@ -96,7 +96,7 @@ def test_album_update(self):
content_type="application/json",
)
assert response.status_code == 200
assert len(response.json()["files"]) == 10
assert len(response.json()["bma_response"]["files"]) == 10

# update to remove all files
response = self.client.patch(
Expand All @@ -106,7 +106,7 @@ def test_album_update(self):
content_type="application/json",
)
assert response.status_code == 200
assert len(response.json()["files"]) == 0
assert len(response.json()["bma_response"]["files"]) == 0

def test_album_delete(self):
"""Test deleting an album."""
Expand Down Expand Up @@ -154,31 +154,31 @@ def test_album_list(self):
self.test_album_create_with_files(title=f"album{i}")
response = self.client.get(reverse("api-v1-json:album_list"), headers={"authorization": self.user1.auth})
assert response.status_code == 200
assert len(response.json()) == 10
assert len(response.json()["bma_response"]) == 10

# test the file filter with files in different albums
response = self.client.get(
reverse("api-v1-json:album_list"),
data={"files": [self.files[0], response.json()[1]["files"][0]]},
data={"files": [self.files[0], response.json()["bma_response"][1]["files"][0]]},
headers={"authorization": self.user1.auth},
)
assert response.status_code == 200
assert len(response.json()) == 0
assert len(response.json()["bma_response"]) == 0
# test with files in the same album
response = self.client.get(
reverse("api-v1-json:album_list"),
data={"files": [self.files[0], self.files[1]]},
headers={"authorization": self.user1.auth},
)
assert response.status_code == 200
assert len(response.json()) == 1
assert len(response.json()["bma_response"]) == 1

# test search
response = self.client.get(
reverse("api-v1-json:album_list"), data={"search": "album4"}, headers={"authorization": self.user1.auth}
)
assert response.status_code == 200
assert len(response.json()) == 1
assert len(response.json()["bma_response"]) == 1

# test sorting
response = self.client.get(
Expand All @@ -187,8 +187,8 @@ def test_album_list(self):
headers={"authorization": self.user1.auth},
)
assert response.status_code == 200
assert len(response.json()) == 10
assert response.json()[0]["title"] == "album9"
assert len(response.json()["bma_response"]) == 10
assert response.json()["bma_response"][0]["title"] == "album9"

# test offset
response = self.client.get(
Expand All @@ -197,5 +197,5 @@ def test_album_list(self):
headers={"authorization": self.user1.auth},
)
assert response.status_code == 200
assert len(response.json()) == 5
assert response.json()[0]["title"] == "album5"
assert len(response.json()["bma_response"]) == 5
assert response.json()["bma_response"][0]["title"] == "album5"
1 change: 0 additions & 1 deletion src/bma/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
version="1",
# we require CSRF but disable it for non-session auth requests
# inside ExemptOauthFromCSRFMiddleware
csrf=True,
parser=ORJSONParser(),
renderer=ORJSONRenderer(),
urls_namespace="api-v1-json",
Expand Down
Loading

0 comments on commit 31117b9

Please sign in to comment.