From a58127a0d3a3a5eaa45bc26f0fe5a7efa9197a26 Mon Sep 17 00:00:00 2001 From: Steve Lacey Date: Mon, 3 Oct 2022 16:59:28 +0700 Subject: [PATCH] Support custom actions via ActionAPI --- .editorconfig | 2 +- README.md | 52 +++++++++++++++++++++++--- pytest.ini | 2 +- tests/models.py | 19 ++++++++++ tests/test_actions.py | 53 +++++++++++++++++++++++++++ tests/test_exceptions.py | 16 ++++++++ tests/test_permissions.py | 8 ++-- tests/test_views.py | 25 +++++++------ tests/urls.py | 4 +- tests/views.py | 18 ++++++--- worf/exceptions.py | 75 +++++++++++++------------------------- worf/permissions.py | 14 +++---- worf/validators.py | 5 +-- worf/views/__init__.py | 1 + worf/views/action.py | 33 +++++++++++++++++ worf/views/base.py | 77 ++++++++++++++++++++++----------------- worf/views/detail.py | 7 ---- worf/views/list.py | 76 ++++++++++++-------------------------- 18 files changed, 301 insertions(+), 186 deletions(-) create mode 100644 tests/test_actions.py create mode 100644 tests/test_exceptions.py create mode 100644 worf/views/action.py diff --git a/.editorconfig b/.editorconfig index 204cfd7..f0e752d 100644 --- a/.editorconfig +++ b/.editorconfig @@ -5,7 +5,7 @@ indent_style = space indent_size = 2 end_of_line = lf charset = utf-8 -max_line_length = 80 +max_line_length = 88 trim_trailing_whitespace = true insert_final_newline = true diff --git a/README.md b/README.md index 835062f..17b3a75 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,8 @@ Table of contents - [DetailAPI](#detailapi) - [CreateAPI](#createapi) - [UpdateAPI](#updateapi) + - [ActionAPI](#actionapi) + - [DeleteAPI](#deleteapi) - [Browsable API](#browsable-api) - [Bundle loading](#bundle-loading) - [Debugging](#debugging) @@ -115,17 +117,18 @@ class BookSerializer(Serializer): ```py # views.py from worf.permissions import Authenticated -from worf.views import DetailAPI, ListAPI, UpdateAPI +from worf.views import ActionAPI, DeleteAPI, DetailAPI, ListAPI, UpdateAPI class BookList(CreateAPI, ListAPI): model = Book serializer = BookSerializer(only=["id", "title"]) permissions = [Authenticated] -class BookDetail(UpdateAPI, DetailAPI): +class BookDetail(ActionAPI, DeleteAPI, UpdateAPI, DetailAPI): model = Book serializer = BookSerializer permissions = [Authenticated] + actions = ["publish"] ``` ```py @@ -133,6 +136,7 @@ class BookDetail(UpdateAPI, DetailAPI): path("api/", include([ path("books/", BookList.as_view()), path("books//", BookDetail.as_view()), + path("books///", BookDetail.as_view()), ])), ``` @@ -206,7 +210,7 @@ Provides the basic functionality of API views. | permissions | list | [] | List of permissions classes. | | serializer | object | None | Serializer class or instance. | -*Note:* it is not recommended to use this abstract view directly. +**Note:** it is not recommended to use this abstract view directly. ### ListAPI @@ -214,11 +218,9 @@ Provides the basic functionality of API views. | Name | Type | Default | Description | | ----------------- | ------ | ------------------- | -------------------------------------------------------------------------------------- | | queryset | object | model.objects.all() | Queryset used to retrieve the results. | -| filters | dict | {} | Filters to apply to queryset. *Deprecated:* use `queryset` instead. | | lookup_field | str | None | Filter `queryset` based on a URL param, `lookup_url_kwarg` is required if this is set. | | lookup_url_kwarg | str | None | Filter `queryset` based on a URL param, `lookup_field` is required if this is set. | | payload_key | str | verbose_name_plural | Use in order to rename the key for the results array. | -| list_serializer | object | serializer | Serializer class or instance. | | ordering | list | [] | List of fields to default the queryset order by. | | filter_fields | list | [] | List of fields to support filtering via query params. | | search_fields | list | [] | List of fields to full text search via the `q` query param. | @@ -251,7 +253,6 @@ Use `per_page` to set custom limit for pagination. Default 25. | queryset | object | model.objects.all() | Queryset used to retrieve the results. | | lookup_field | str | id | Lookup field used to filter the model. | | lookup_url_kwarg | str | id | Name of the parameter passed to the view by the URL route. | -| detail_serializer | object | serializer | Serializer class or instance. | This `get_instance()` method uses `lookup_field` and `lookup_url_kwargs` to return a model instance. @@ -297,6 +298,45 @@ Validation of update fields is delegated to the serializer, any fields that are writeable should be within the `fields` definition of the serializer, and not marked as `dump_only` (read-only). +### ActionAPI + +| Name | Type | Default | Description | +| ------------------- | ------ | ------------------- | ---------------------------------------------------------- | +| queryset | object | model.objects.all() | Queryset used to retrieve the results. | +| lookup_field | str | id | Lookup field used to filter the model. | +| lookup_url_kwarg | str | id | Name of the parameter passed to the view by the URL route. | +| actions | list | [] | List of action methods to support. | + +Adds `put` endpoints keyed by a route param, mix this into a `DetailAPI` view: + +```py +class BookDetailAPI(ActionAPI, DetailAPI): + model = Book + serializer = BookSerializer + actions = ["publish"] +``` + +Actions must exist as a method on either the model or the view, they are passed the +contents of the bundle as kwargs, and if the method accepts a `user` kwarg then +`request.user` will be passed through too. + +### DeleteAPI + +| Name | Type | Default | Description | +| ------------------- | ------ | ------------------- | ---------------------------------------------------------- | +| queryset | object | model.objects.all() | Queryset used to retrieve the results. | +| lookup_field | str | id | Lookup field used to filter the model. | +| lookup_url_kwarg | str | id | Name of the parameter passed to the view by the URL route. | + +Adds a `delete` method to handle deletes, mix this into a `DetailAPI`. + +```py +class BookDetailAPI(DeleteAPI, DetailAPI): + model = Book +``` + +Deletes return a 204 no content response, no serializer is required. + Browsable API ------------- diff --git a/pytest.ini b/pytest.ini index 97ebc3d..5c2b98b 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,7 +1,7 @@ [pytest] addopts = --cov - --cov-fail-under 92 + --cov-fail-under 93.5 --cov-report term:skip-covered --cov-report html --no-cov-on-fail diff --git a/tests/models.py b/tests/models.py index 193c123..93b32b2 100644 --- a/tests/models.py +++ b/tests/models.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import User from django.db import models +from django.utils.timezone import now class Profile(models.Model): @@ -33,6 +34,12 @@ class Profile(models.Model): last_active = models.DateField(blank=True, null=True) created_at = models.DateTimeField(blank=True, null=True) + is_subscribed = models.BooleanField(blank=True, null=True) + subscribed_at = models.DateTimeField(blank=True, null=True) + subscribed_by = models.ForeignKey( + User, blank=True, null=True, on_delete=models.SET_NULL + ) + def get_avatar_url(self): return self.avatar.url if self.avatar else self.get_gravatar_url() @@ -42,6 +49,18 @@ def get_gravatar_hash(self): def get_gravatar_url(self, default="identicon", size=512): return f"https://www.gravatar.com/avatar/{self.get_gravatar_hash()}?d={default}&s={size}" + def subscribe(self, user=None): + self.is_subscribed = True + self.subscribed_at = now() + self.subscribed_by = user + self.save() + + def unsubscribe(self): + self.is_subscribed = False + self.subscribed_at = None + self.subscribed_by = None + self.save() + class Role(models.Model): name = models.CharField(max_length=100) diff --git a/tests/test_actions.py b/tests/test_actions.py new file mode 100644 index 0000000..366d204 --- /dev/null +++ b/tests/test_actions.py @@ -0,0 +1,53 @@ +from tests import parametrize + + +def test_action_model_func(user_client, profile): + response = user_client.put(f"/profiles/{profile.pk}/unsubscribe/") + result = response.json() + assert response.status_code == 200, result + profile.refresh_from_db() + assert profile.is_subscribed is False + assert profile.subscribed_at is None + assert profile.subscribed_by is None + + +def test_action_model_func_with_user_argument(user_client, profile, user): + response = user_client.put(f"/profiles/{profile.pk}/subscribe/") + result = response.json() + assert response.status_code == 200, result + profile.refresh_from_db() + assert profile.is_subscribed is True + assert profile.subscribed_at is not None + assert profile.subscribed_by == user + + +def test_action_view_func(user_client, profile, user): + response = user_client.put(f"/profiles/{profile.pk}/resubscribe/") + result = response.json() + assert response.status_code == 200, result + profile.refresh_from_db() + assert profile.is_subscribed is True + assert profile.subscribed_at is not None + assert profile.subscribed_by == user + + +def test_invalid_action(user_client, profile): + response = user_client.put(f"/profiles/{profile.pk}/invalid-action/") + result = response.json() + assert response.status_code == 400, result + assert result["message"] == "Invalid action: invalid-action" + + +def test_invalid_arguments(user_client, profile): + kwargs = dict(text="I love newsletters") + response = user_client.put(f"/profiles/{profile.pk}/subscribe/", kwargs) + result = response.json() + assert response.status_code == 400, result + message = "Invalid arguments: subscribe() got an unexpected keyword argument 'text'" + assert result["message"] == message + + +@parametrize("method", ["GET", "DELETE", "PATCH", "POST"]) +def test_invalid_method(user_client, method, profile): + response = user_client.generic(method, f"/profiles/{profile.pk}/subscribe/") + assert response.status_code == 405 diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 0000000..36a05f4 --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,16 @@ +from tests import parametrize +from worf import exceptions + + +@parametrize( + dict(e=exceptions.ActionError("test")), + dict(e=exceptions.AuthenticationError("test")), + dict(e=exceptions.DataConflict("test")), + dict(e=exceptions.NamingThingsError("test")), + dict(e=exceptions.NotFound("test")), + dict(e=exceptions.PermissionsError("test")), + dict(e=exceptions.SerializerError("test")), + dict(e=exceptions.WorfError("test")), +) +def test_exception(e): + assert e.message == str(e) == "test" diff --git a/tests/test_permissions.py b/tests/test_permissions.py index e5b1a9e..9f644f8 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -2,7 +2,7 @@ from django.contrib.auth.models import AnonymousUser, User -from worf.exceptions import HTTP401, HTTP404 +from worf.exceptions import AuthenticationError, NotFound from worf.permissions import Authenticated, PublicEndpoint, Staff @@ -11,7 +11,7 @@ def test_authenticated(db, rf): request = rf.get("/") request.user = AnonymousUser() - with pytest.raises(HTTP401): + with pytest.raises(AuthenticationError): assert permission(request) is None request.user = User.objects.create(username="test", password="test") @@ -32,12 +32,12 @@ def test_staff(db, rf): request = rf.get("/") request.user = AnonymousUser() - with pytest.raises(HTTP404): + with pytest.raises(NotFound): assert permission(request) is None request.user = User.objects.create(username="test", password="test") - with pytest.raises(HTTP404): + with pytest.raises(NotFound): assert permission(request) is None request.user.is_staff = True diff --git a/tests/test_views.py b/tests/test_views.py index 0c885eb..1f909f0 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -18,7 +18,7 @@ def test_profile_not_found(client, db, profile, user): response = client.get(f"/profiles/{uuid4()}/") result = response.json() assert response.status_code == 404, result - assert result["message"] == "Not Found" + assert result["message"] == "Not found" def test_profile_delete(client, db, profile, user): @@ -35,6 +35,14 @@ def test_profile_list(client, db, profile, user): assert result["profiles"][0]["username"] == user.username +@parametrize("page", [-1, 0, 1, 2]) +def test_profile_list_pages(client, db, page): + response = client.get("/profiles/", dict(page=page)) + result = response.json() + assert response.status_code == 200, result + assert len(result["profiles"]) == 0 + + def test_profile_list_filters(client, db, profile, url, user): response = client.get(url("/profiles/", {"name": user.name})) result = response.json() @@ -120,13 +128,6 @@ def test_profile_list_not_in_filters(client, db, profile, url, user): assert len(result["profiles"]) == 0 -def test_profile_list_subset_filters(client, db, profile, url, user): - response = client.get(url("/profiles/subset/", {"name": user.name})) - result = response.json() - assert response.status_code == 200, result - assert len(result["profiles"]) == 0 - - @patch("django.core.files.storage.FileSystemStorage.save") def test_profile_multipart_create(mock_save, client, db, role, user): avatar = SimpleUploadedFile("avatar.jpg", b"", content_type="image/jpeg") @@ -290,21 +291,21 @@ def test_profile_update_m2m_through_required_fields(client, db, method, profile, def test_staff_detail(admin_client, profile, user): - response = admin_client.get(f"/profiles/{profile.pk}/staff/") + response = admin_client.get(f"/staff/{profile.pk}/") result = response.json() assert response.status_code == 200, result assert result["username"] == user.username def test_staff_detail_is_not_found_for_user(user_client, profile, user): - response = user_client.get(f"/profiles/{profile.pk}/staff/") + response = user_client.get(f"/staff/{profile.pk}/") result = response.json() assert response.status_code == 404, result - assert result["message"] == "Not Found" + assert result["message"] == "Not found" def test_staff_detail_is_unauthorized_for_guest(client, db, profile, user): - response = client.get(f"/profiles/{profile.pk}/staff/") + response = client.get(f"/staff/{profile.pk}/") result = response.json() assert response.status_code == 401, result assert result["message"] == "Unauthorized" diff --git a/tests/urls.py b/tests/urls.py index 5751129..2db0618 100644 --- a/tests/urls.py +++ b/tests/urls.py @@ -4,9 +4,9 @@ urlpatterns = [ path("profiles/", views.ProfileList.as_view()), - path("profiles/subset/", views.ProfileListSubSet.as_view()), path("profiles//", views.ProfileDetail.as_view()), - path("profiles//staff/", views.StaffDetail.as_view()), + path("profiles///", views.ProfileDetail.as_view()), + path("staff//", views.StaffDetail.as_view()), path("user/", views.UserSelf.as_view()), path("users/", views.UserList.as_view()), path("users//", views.UserDetail.as_view()), diff --git a/tests/views.py b/tests/views.py index 0d5e6d2..1b20085 100644 --- a/tests/views.py +++ b/tests/views.py @@ -7,7 +7,7 @@ from tests.serializers import ProfileSerializer, UserSerializer from worf.exceptions import AuthenticationError from worf.permissions import Authenticated, PublicEndpoint, Staff -from worf.views import CreateAPI, DeleteAPI, DetailAPI, ListAPI, UpdateAPI +from worf.views import ActionAPI, CreateAPI, DeleteAPI, DetailAPI, ListAPI, UpdateAPI class ProfileList(CreateAPI, ListAPI): @@ -35,14 +35,20 @@ class ProfileList(CreateAPI, ListAPI): ] -class ProfileListSubSet(ProfileList): - queryset = ProfileList.queryset.none() - - -class ProfileDetail(DeleteAPI, UpdateAPI, DetailAPI): +class ProfileDetail(ActionAPI, DeleteAPI, UpdateAPI, DetailAPI): model = Profile serializer = ProfileSerializer permissions = [PublicEndpoint] + actions = [ + "resubscribe", + "subscribe", + "unsubscribe", + ] + + def resubscribe(self, request, *args, **kwargs): + profile = self.get_instance() + profile.subscribe(user=request.user) + return profile def validate_phone(self, value): try: diff --git a/worf/exceptions.py b/worf/exceptions.py index 574dcec..3101e9f 100644 --- a/worf/exceptions.py +++ b/worf/exceptions.py @@ -1,66 +1,41 @@ -class HTTPException(Exception): - def __init__(self, message=None): - super().__init__(message) - self.message = message or self.message +from dataclasses import dataclass -class HTTP400(HTTPException): - message = "Bad Request" - status = 400 +@dataclass(frozen=True) +class WorfError(Exception): + message: str -class HTTP401(HTTPException): - message = "Unauthorized" - status = 401 +@dataclass(frozen=True) +class ActionError(WorfError): + message: str -class HTTP404(HTTPException): - message = "Not Found" - status = 404 +@dataclass(frozen=True) +class AuthenticationError(WorfError): + message: str = "Unauthorized" -class HTTP409(HTTPException): - message = "Conflict" - status = 409 +@dataclass(frozen=True) +class DataConflict(WorfError): + message: str = "Conflict" -class HTTP410(HTTPException): - message = "Gone" - status = 410 +@dataclass(frozen=True) +class NamingThingsError(WorfError, ValueError): + message: str -class HTTP420(HTTPException): - message = "Enhance Your Calm" - status = 420 +@dataclass(frozen=True) +class NotFound(WorfError): + message: str = "Not found" -class HTTP422(HTTPException): - message = "Unprocessable Entity" - status = 422 +@dataclass(frozen=True) +class PermissionsError(WorfError): + message: str -HTTP_EXCEPTIONS = (HTTP400, HTTP401, HTTP404, HTTP409, HTTP410, HTTP420, HTTP422) - - -class AuthenticationError(Exception): - def __init__(self, message): - super().__init__(message) - self.message = message - - -class NamingThingsError(ValueError): - pass - - -class PermissionsError(Exception): - pass - - -class NotImplementedInWorfYet(NotImplementedError): - pass - - -class SerializerError(ValueError): - def __init__(self, message): - super().__init__(message) - self.message = message +@dataclass(frozen=True) +class SerializerError(WorfError, ValueError): + message: str diff --git a/worf/permissions.py b/worf/permissions.py index 34bb5e9..fdce219 100644 --- a/worf/permissions.py +++ b/worf/permissions.py @@ -1,12 +1,10 @@ -from worf.exceptions import HTTP401, HTTP404 +from worf.exceptions import AuthenticationError, NotFound class Authenticated: def __call__(self, request, **kwargs): - if request.user.is_authenticated: - return - - raise HTTP401() + if not request.user.is_authenticated: + raise AuthenticationError() class PublicEndpoint: @@ -16,7 +14,5 @@ def __call__(self, request, **kwargs): class Staff: def __call__(self, request, **kwargs): - if request.user.is_authenticated and request.user.is_staff: - return - - raise HTTP404() + if not request.user.is_authenticated or not request.user.is_staff: + raise NotFound() diff --git a/worf/validators.py b/worf/validators.py index c4d05af..2c4cdb1 100644 --- a/worf/validators.py +++ b/worf/validators.py @@ -7,7 +7,6 @@ from django.utils.dateparse import parse_datetime from worf.conf import settings -from worf.exceptions import NotImplementedInWorfYet class ValidateFields: @@ -149,7 +148,7 @@ def validate_bundle(self, key): @param key: the model attribute to check against. - @raise NotImplementedInWorfYet: If the field type is not currently supported + @raise NotImplementedError: If the field type is not currently supported @raise ValidationError: If this is a write and `key` is not serializer editable @raise ValidationError: If a value fails to pass validation @@ -244,4 +243,4 @@ def validate_bundle(self, key): message = f"{field.get_internal_type()} has no validation method for {key}" if settings.WORF_DEBUG: message += f":: Received {self.bundle[key]}" - raise NotImplementedInWorfYet(message) + raise NotImplementedError(message) diff --git a/worf/views/__init__.py b/worf/views/__init__.py index 02b1ba6..801a04d 100644 --- a/worf/views/__init__.py +++ b/worf/views/__init__.py @@ -1,3 +1,4 @@ +from worf.views.action import ActionAPI # noqa from worf.views.base import AbstractBaseAPI, APIResponse # noqa from worf.views.create import CreateAPI # noqa from worf.views.delete import DeleteAPI # noqa diff --git a/worf/views/action.py b/worf/views/action.py new file mode 100644 index 0000000..75ec997 --- /dev/null +++ b/worf/views/action.py @@ -0,0 +1,33 @@ +from worf.exceptions import ActionError +from worf.lookups import FindInstance +from worf.views.base import AbstractBaseAPI + + +class ActionAPI(FindInstance, AbstractBaseAPI): + actions = [] + + def dispatch(self, request, *args, **kwargs): + if "action" in kwargs and request.method != "PUT": + return self.http_method_not_allowed(request, *args, **kwargs) + return super().dispatch(request, *args, **kwargs) + + def perform_action(self, request, *args, **kwargs): + action = kwargs["action"].replace("-", "_") + if action not in self.actions: + raise ActionError(f"Invalid action: {kwargs['action']}") + instance = self.get_instance() + if hasattr(self, action): + return getattr(self, action)(request, **self.bundle, user=request.user) + try: + getattr(instance, action)(**self.bundle, user=request.user) + except TypeError as e: + if "unexpected keyword argument 'user'" not in str(e): + raise ActionError(f"Invalid arguments: {e}") + getattr(instance, action)(**self.bundle) + return instance + + def put(self, request, *args, **kwargs): + if "action" in kwargs: + self.perform_action(request, *args, **kwargs) + return self.render_to_response() + return super().put(request, *args, **kwargs) diff --git a/worf/views/base.py b/worf/views/base.py index 7053340..fdf4667 100644 --- a/worf/views/base.py +++ b/worf/views/base.py @@ -17,10 +17,13 @@ from worf.casing import camel_to_snake, snake_to_camel from worf.conf import settings from worf.exceptions import ( - HTTP_EXCEPTIONS, + ActionError, AuthenticationError, + DataConflict, + NotFound, PermissionsError, SerializerError, + WorfError, ) from worf.renderers import render_response from worf.serializers import SerializeModels @@ -83,51 +86,59 @@ def name(self): return snake_to_camel(verbose_name_plural.replace(" ", "_").lower()) def dispatch(self, request, *args, **kwargs): - method = request.method.lower() - handler = self.http_method_not_allowed - - if method in self.http_method_names: - handler = getattr(self, method, self.http_method_not_allowed) + try: + response = self.perform_dispatch(request, *args, **kwargs) + except ActionError as e: + response = self.render_error(e.message, 400) + except AuthenticationError as e: + response = self.render_error(e.message, 401) + except DataConflict as e: + response = self.render_error(e.message, 409) + except NotFound as e: + response = self.render_error(e.message, 404) + except SerializerError as e: + response = self.render_error(e.message, 400) + except ValidationError as e: + response = self.render_error(e.message, 422) + return response + def perform_dispatch(self, request, *args, **kwargs): try: + handler = self.get_handler(request, *args, **kwargs) self.check_permissions() self.set_bundle_from_request(request) - return handler(request, *args, **kwargs) - except HTTP_EXCEPTIONS as e: - message = e.message - status = e.status - except AuthenticationError as e: - message = e.message - status = 401 + response = handler(request, *args, **kwargs) except ObjectDoesNotExist as e: if self.model and not isinstance(e, self.model.DoesNotExist): raise e - message = "Not Found" - status = 404 - except RequestDataTooBig: + raise NotFound from e + except RequestDataTooBig as e: self.request._body = self.request.read(None) # prevent further raises - message = f"Max upload size is {filesizeformat(settings.DATA_UPLOAD_MAX_MEMORY_SIZE)}" - status = 422 - except SerializerError as e: - message = e.message - status = 400 - except ValidationError as e: - message = e.message - status = 422 + max_upload_size = filesizeformat(settings.DATA_UPLOAD_MAX_MEMORY_SIZE) + raise ValidationError(f"Max upload size is {max_upload_size}") from e + return response + def render_error(self, message, status): return self.render_to_response(dict(message=message), status) def check_permissions(self): - for perm in self.permissions: - try: + try: + for perm in self.permissions: perm()(self.request, **self.kwargs) - except HTTP_EXCEPTIONS as e: - if settings.WORF_DEBUG: - raise PermissionsError( - f"Permission check {perm.__module__}.{perm.__name__} raised {e.__class__.__name__}. " - f"You'd normally see a {e.status} here but WORF_DEBUG=True." - ) from e - raise e + except WorfError as e: + if settings.WORF_DEBUG: + raise PermissionsError( + f"Permission check {perm.__module__}.{perm.__name__} raised {e.__class__.__name__}. " + f"You'd normally see a {e.status} here but WORF_DEBUG=True." + ) from e + raise e + + def get_handler(self, request, *args, **kwargs): + method = request.method.lower() + handler = self.http_method_not_allowed + if method in self.http_method_names: + handler = getattr(self, method, self.http_method_not_allowed) + return handler def get_instance(self): return self.instance if hasattr(self, "instance") else None diff --git a/worf/views/detail.py b/worf/views/detail.py index d0cb345..14cca1c 100644 --- a/worf/views/detail.py +++ b/worf/views/detail.py @@ -4,16 +4,9 @@ class DetailAPI(FindInstance, AbstractBaseAPI): - detail_serializer = None - def get(self, *args, **kwargs): return self.render_to_response() - def get_serializer(self): - if self.detail_serializer and self.request.method == "GET": - return self.detail_serializer(**self.get_serializer_kwargs()) - return super().get_serializer() - class DetailUpdateAPI(UpdateAPI, DetailAPI): pass diff --git a/worf/views/list.py b/worf/views/list.py index 85c0f99..6c4cefc 100644 --- a/worf/views/list.py +++ b/worf/views/list.py @@ -8,7 +8,6 @@ from worf.casing import camel_to_snake from worf.conf import settings -from worf.exceptions import HTTP420 from worf.filters import apply_filterset, generate_filterset from worf.shortcuts import list_param from worf.views.base import AbstractBaseAPI @@ -17,14 +16,12 @@ class ListAPI(AbstractBaseAPI): lookup_url_kwarg = "id" # default incase lookup_field is set - filters = {} ordering = [] filter_fields = [] search_fields = [] sort_fields = [] queryset = None filter_set = None - list_serializer = None count = 0 page_num = 1 per_page = 25 @@ -36,9 +33,6 @@ def __init__(self, *args, **kwargs): codepath = self.codepath - if not isinstance(self.filters, dict): # pragma: no cover - raise ImproperlyConfigured(f"{codepath}.filters must be type: dict") - if not isinstance(self.ordering, list): # pragma: no cover raise ImproperlyConfigured(f"{codepath}.ordering must be type: list") @@ -65,15 +59,9 @@ def __init__(self, *args, **kwargs): def get(self, *args, **kwargs): return self.render_to_response() - def _set_base_lookup_kwargs(self): - # Filters set directly on the class - self.lookup_kwargs.update(self.filters) - - # Filters set via URL + def set_base_lookup_kwargs(self): if hasattr(self, "lookup_field") and hasattr(self, "lookup_url_kwarg"): - self.lookup_kwargs.update( - {self.lookup_field: self.kwargs[self.lookup_url_kwarg]} - ) + self.lookup_kwargs[self.lookup_field] = self.kwargs[self.lookup_url_kwarg] def set_search_lookup_kwargs(self): """ @@ -99,7 +87,7 @@ def set_search_lookup_kwargs(self): ) self.search_query = reduce(operator.or_, search_icontains) - if not self.filter_fields or not self.bundle: + if not self.filter_fields or not self.bundle: # pragma: no cover return for key in self.bundle.keys(): @@ -126,7 +114,7 @@ def get_processed_queryset(self): self.lookup_kwargs = {} self.search_query = Q() - self._set_base_lookup_kwargs() + self.set_base_lookup_kwargs() self.set_search_lookup_kwargs() lookups = self.lookup_kwargs.items() @@ -134,29 +122,22 @@ def get_processed_queryset(self): list_kwargs = {k: v for k, v in lookups if isinstance(v, list)} ordering = self.get_ordering() - queryset = self.get_queryset() + queryset = ( + apply_filterset(self.filter_set, self.get_queryset(), filterset_kwargs) + .filter(self.search_query) + .distinct() + ) - try: - queryset = ( - apply_filterset(self.filter_set, queryset, filterset_kwargs) - .filter(self.search_query) - .distinct() - ) + for key, value in list_kwargs.items(): + for item in value: + queryset = ( + queryset.exclude(**{key.rstrip("!"): item}) + if key.endswith("!") + else queryset.filter(**{key: item}) + ) - for key, value in list_kwargs.items(): - for item in value: - queryset = ( - queryset.exclude(**{key.rstrip("!"): item}) - if key.endswith("!") - else queryset.filter(**{key: item}) - ) - - if ordering: - queryset = queryset.order_by(*ordering) - except TypeError as e: # pragma: no cover - debugging - if settings.WORF_DEBUG: - raise HTTP420(f"Error, {self.lookup_kwargs}, {e.__cause__}") - raise e + if ordering: + queryset = queryset.order_by(*ordering) return queryset @@ -174,16 +155,11 @@ def get_ordering(self): def get_sort_field(self, field, descending=False): return OrderBy(F(field), descending=descending) - def get_serializer(self): - if self.list_serializer and self.request.method == "GET": - return self.list_serializer(**self.get_serializer_kwargs()) - return super().get_serializer() - def paginated_results(self): queryset = self.get_processed_queryset() request = self.request - if settings.WORF_DEBUG: + if settings.WORF_DEBUG: # pragma: no cover try: self.query = str(queryset.query) except EmptyResultSet: @@ -212,17 +188,13 @@ def serialize(self): payload = {str(self.name): serializer(many=True).dump(self.paginated_results())} if self.per_page: - payload.update( - { - "pagination": dict( - count=self.count, - pages=self.num_pages, - page=self.page_num, - ) - } + payload["pagination"] = dict( + count=self.count, + pages=self.num_pages, + page=self.page_num, ) - if settings.WORF_DEBUG: + if settings.WORF_DEBUG: # pragma: no cover payload["debug"] = { "bundle": self.bundle, "lookup_kwargs": getattr(self, "lookup_kwargs", {}),