diff --git a/openedx_tagging/core/tagging/api.py b/openedx_tagging/core/tagging/api.py index 7609d533..c0b156ee 100644 --- a/openedx_tagging/core/tagging/api.py +++ b/openedx_tagging/core/tagging/api.py @@ -88,13 +88,13 @@ def get_root_tags(taxonomy: Taxonomy) -> list[Tag]: return taxonomy.cast().get_tags(only_roots=True) -def get_children_tags(taxonomy: Taxonomy, parent_tag_id: str) -> list[Tag]: +def get_children_tags(taxonomy: Taxonomy, parent_tag_id: int) -> list[Tag]: """ Returns a list of children tags for the given parent tag. Note that if the taxonomy allows free-text tags, then the returned list will be empty. """ - return list(taxonomy.cast().get_children_tags(parent_tag_id)) + return taxonomy.cast().get_children_tags(parent_tag_id) def resync_object_tags(object_tags: QuerySet | None = None) -> int: diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index 0f2e2f7f..4121f74b 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -270,7 +270,7 @@ def copy(self, taxonomy: Taxonomy) -> Taxonomy: def get_tags( self, - tag_set: models.QuerySet = None, + tag_set: models.QuerySet | None = None, only_roots: bool = False, ) -> list[Tag]: """ @@ -320,16 +320,16 @@ def get_tags( break return tags - def get_children_tags(self, parent_tag_id: int) -> List[Tag]: + def get_children_tags(self, parent_tag_id: int) -> list[Tag]: """ Returns a list of children tags of `parent_tag_id` in the current taxonomy. """ if self.allow_free_text: return [] - return self.tag_set.filter(parent=parent_tag_id).order_by( + return list(self.tag_set.filter(parent=parent_tag_id).order_by( "parent__value", "value", "id" - ) + )) def validate_object_tag( self, diff --git a/openedx_tagging/core/tagging/models/system_defined.py b/openedx_tagging/core/tagging/models/system_defined.py index 1cd79d30..0807a8d9 100644 --- a/openedx_tagging/core/tagging/models/system_defined.py +++ b/openedx_tagging/core/tagging/models/system_defined.py @@ -243,7 +243,7 @@ class Meta: def get_tags( self, - tag_set: models.QuerySet = None, + tag_set: models.QuerySet | None = None, only_roots: bool = False, ) -> list[Tag]: """ diff --git a/openedx_tagging/core/tagging/rest_api/paginators.py b/openedx_tagging/core/tagging/rest_api/paginators.py index ab36a5ac..195978bd 100644 --- a/openedx_tagging/core/tagging/rest_api/paginators.py +++ b/openedx_tagging/core/tagging/rest_api/paginators.py @@ -1,4 +1,4 @@ -from edx_rest_framework_extensions.paginators import DefaultPagination +from edx_rest_framework_extensions.paginators import DefaultPagination # type: ignore from rest_framework.response import Response # From this point, the tags begin to be paginated diff --git a/openedx_tagging/core/tagging/rest_api/v1/permissions.py b/openedx_tagging/core/tagging/rest_api/v1/permissions.py index c9518d4f..f19de945 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/permissions.py +++ b/openedx_tagging/core/tagging/rest_api/v1/permissions.py @@ -1,10 +1,8 @@ """ Tagging permissions """ - +import rules # type: ignore from rest_framework.permissions import DjangoObjectPermissions -from openedx_tagging.core.tagging.models import Tag, Taxonomy -import rules class TaxonomyObjectPermissions(DjangoObjectPermissions): diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 8ec5e096..176741a2 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -5,7 +5,7 @@ from rest_framework import serializers from rest_framework.reverse import reverse -from openedx_tagging.core.tagging.models import Taxonomy, Tag, ObjectTag +from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy class TaxonomyListQueryParamsSerializer(serializers.Serializer): diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 2628eecc..6910da23 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -5,42 +5,34 @@ from django.http import Http404 from rest_framework import mixins from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError - -from ...models import Taxonomy -from ...rules import ChangeObjectTagPermissionItem -from rest_framework.viewsets import ModelViewSet, GenericViewSet from rest_framework.generics import ListAPIView +from rest_framework.viewsets import GenericViewSet, ModelViewSet + +from openedx_tagging.core.tagging.models.base import Tag from ...api import ( create_taxonomy, - get_taxonomy, - get_taxonomies, + get_children_tags, get_object_tags, - tag_object, get_root_tags, - get_children_tags, -) -from .permissions import ( - TaxonomyObjectPermissions, - TagListPermissions, - ObjectTagObjectPermissions, + get_taxonomies, + get_taxonomy, + tag_object, ) +from ...models import Taxonomy +from ...rules import ChangeObjectTagPermissionItem +from ..paginators import TAGS_THRESHOLD, DisabledTagsPagination, TagsPagination +from .permissions import ObjectTagObjectPermissions, TagListPermissions, TaxonomyObjectPermissions from .serializers import ( ObjectTagListQueryParamsSerializer, ObjectTagSerializer, ObjectTagUpdateBodySerializer, ObjectTagUpdateQueryParamsSerializer, - TaxonomyListQueryParamsSerializer, - TaxonomySerializer, TagsSerializer, TagsWithSubTagsSerializer, + TaxonomyListQueryParamsSerializer, + TaxonomySerializer, ) -from ..paginators import ( - TAGS_THRESHOLD, - TagsPagination, - DisabledTagsPagination, -) -from openedx_tagging.core.tagging.models.base import Tag class TaxonomyView(ModelViewSet): @@ -323,7 +315,7 @@ def update(self, request, object_id, partial=False): raise ValidationError(e) return self.retrieve(request, object_id) - + class TaxonomyTagsView(ListAPIView): """ @@ -370,7 +362,7 @@ def get_serializer_class(self): else: return TagsWithSubTagsSerializer - def get_taxonomy(self, pk: str): + def get_taxonomy(self, pk: int) -> Taxonomy: """ Get the taxonomy from `pk` or raise 404 """ @@ -380,7 +372,11 @@ def get_taxonomy(self, pk: str): self.check_object_permissions(self.request, taxonomy) return taxonomy - def get_matching_tags(self, taxonomy_id: int, parent_tag_id: str = None): + def get_matching_tags( + self, + taxonomy_id: int, + parent_tag_id: str | None = None + ) -> tuple[list[Tag], bool]: """ Returns a list of tags for the given taxonomy. Also returns a boolean to identify if the pagination is enabled. @@ -394,16 +390,20 @@ def get_matching_tags(self, taxonomy_id: int, parent_tag_id: str = None): """ taxonomy = self.get_taxonomy(taxonomy_id) if parent_tag_id: - return get_children_tags(taxonomy, parent_tag_id), True + return get_children_tags(taxonomy, int(parent_tag_id)), True else: pagination_enabled = taxonomy.tag_set.count() > TAGS_THRESHOLD return get_root_tags(taxonomy), pagination_enabled - def get_queryset(self): + def get_queryset(self) -> list[Tag]: # type: ignore """ Builds and returns the queryset to be paginated + + The return type is not a QuerySet because the tagging python api functions + returns lists, and on this point convert the list to a query set + is an unnecesary operation. """ - pk = str(self.kwargs.get("pk")) + pk = self.kwargs.get("pk") parent_tag_id = self.request.query_params.get("parent_tag_id", None) result, self.pagination_enabled = self.get_matching_tags( pk, parent_tag_id=parent_tag_id diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 81664121..98981b83 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -401,7 +401,7 @@ def test_delete_taxonomy_404(self): self.client.force_authenticate(user=self.staff) response = self.client.delete(url) assert response.status_code == status.HTTP_404_NOT_FOUND - + @ddt.ddt class TestObjectTagViewSet(APITestCase): @@ -791,7 +791,7 @@ def test_tag_object_without_permission(self, user_attr, expected_status): response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") assert response.status_code == expected_status - + class TestTaxonomyTagsView(TestTaxonomyViewMixin): """ @@ -815,7 +815,7 @@ def setUp(self): return super().setUp() - def _create_tag(self, depth: int, parent: Tag = None): + def _create_tag(self, depth: int, parent: Tag | None = None): """ Creates tags and children in a recursive way. """