From d7e58726036b29f0fcc9d9e31098b81ebf475bf0 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 11 Aug 2023 12:50:11 -0500 Subject: [PATCH] feat: Single taxonomy view API for tags docs: ADR for pagination and repr of single taxonomy api feat: Initial view and pagination config feat: get_range_index added to paginator feat: View for get tags - Get paginated root tags - Get paginated children tags - Get all tags for small taxonomies - Tags structure for response chore: Adding permissions and docstrings style: quality feat: Search added to get tags view feat: searching tags docs: Adding comments on the view docs: Update ADR with search logic chore: fix quality checks chore: fix tests style: Nits on type hints and docstrings and comments fix: Delete all about range index in paginators --- .../0014-single-taxonomy-view-api.rst | 13 +- openedx_tagging/core/tagging/api.py | 50 ++- .../core/tagging/import_export/import_plan.py | 1 + .../core/tagging/import_export/parsers.py | 4 +- .../0007_tag_import_task_log_null_fix.py | 11 +- .../0008_taxonomy_description_not_null.py | 13 +- openedx_tagging/core/tagging/models/base.py | 54 ++- .../core/tagging/models/system_defined.py | 33 +- .../core/tagging/rest_api/paginators.py | 29 ++ .../core/tagging/rest_api/v1/permissions.py | 14 +- .../core/tagging/rest_api/v1/serializers.py | 89 ++++- .../core/tagging/rest_api/v1/urls.py | 9 +- .../core/tagging/rest_api/v1/views.py | 215 ++++++++++- openedx_tagging/core/tagging/rules.py | 13 +- .../openedx_tagging/core/tagging/test_api.py | 48 +++ .../core/tagging/test_models.py | 60 +++ .../core/tagging/test_views.py | 345 +++++++++++++++++- 17 files changed, 953 insertions(+), 48 deletions(-) create mode 100644 openedx_tagging/core/tagging/rest_api/paginators.py diff --git a/docs/decisions/0014-single-taxonomy-view-api.rst b/docs/decisions/0014-single-taxonomy-view-api.rst index a71a1ce7..ddd0de26 100644 --- a/docs/decisions/0014-single-taxonomy-view-api.rst +++ b/docs/decisions/0014-single-taxonomy-view-api.rst @@ -83,9 +83,18 @@ We will use the same view to perform a search with the same logic: **get_matching_tags(parent_tag_id: str = None, search_term: str = None)** -We can use ``search_term`` to perferom a search on root tags or children tags depending of ``parent_tag_id``. +We can use ``search_term`` to perform a search on all taxonomy tags or children tags depending of ``parent_tag_id``. +The result will be a pruned tree with the necessary tags to be able to reach the results from a root tag. +Ex. if in the result there may be a child tag of ``depth=2``, but the parents are not found in the result. +In this case, it is necessary to add the parent and the parent of the parent (root tag) to be able to show +the child tag that is in the result. For the search, ``SEARCH_TAGS_THRESHOLD`` will be used. (It is recommended that it be 20% of ``TAGS_THRESHOLD``). +It will work in the following way: + +- If ``search_result.count() < SEARCH_TAGS_THRESHOLD``, then it will return all tags on the result tree without pagination. +- Otherwise, it will return the roots of the result tree with pagination. Each root will have the entire pruned branch. + It will work in the same way of ``TAGS_THRESHOLD`` (see Views & Pagination) **Pros** @@ -190,4 +199,4 @@ can return all the tags in one page. So we can perform the tag search on the fro **Cons:** - It is not scalable. -- Sets limits of tags that can be created in the taxonomy. +- Sets limits of tags that can be created in the taxonomy. \ No newline at end of file diff --git a/openedx_tagging/core/tagging/api.py b/openedx_tagging/core/tagging/api.py index e0295df5..ca2cb1f1 100644 --- a/openedx_tagging/core/tagging/api.py +++ b/openedx_tagging/core/tagging/api.py @@ -79,6 +79,47 @@ def get_tags(taxonomy: Taxonomy) -> list[Tag]: return taxonomy.cast().get_tags() +def get_root_tags(taxonomy: Taxonomy) -> list[Tag]: + """ + Returns a list of the root tags for the given taxonomy. + + Note that if the taxonomy allows free-text tags, then the returned list will be empty. + """ + return list(taxonomy.cast().get_filtered_tags()) + + +def search_tags(taxonomy: Taxonomy, search_term: str) -> list[Tag]: + """ + Returns a list of all tags that contains `search_term` of the given taxonomy. + + Note that if the taxonomy allows free-text tags, then the returned list will be empty. + """ + return list( + taxonomy.cast().get_filtered_tags( + search_term=search_term, + search_in_all=True, + ) + ) + + +def get_children_tags( + taxonomy: Taxonomy, + parent_tag_id: int, + search_term: str | None = None, +) -> 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_filtered_tags( + parent_tag_id=parent_tag_id, + search_term=search_term, + ) + ) + + def resync_object_tags(object_tags: QuerySet | None = None) -> int: """ Reconciles ObjectTag entries with any changes made to their associated taxonomies and tags. @@ -98,8 +139,7 @@ def resync_object_tags(object_tags: QuerySet | None = None) -> int: def get_object_tags( - object_id: str, - taxonomy_id: str | None = None + object_id: str, taxonomy_id: str | None = None ) -> QuerySet[ObjectTag]: """ Returns a Queryset of object tags for a given object. @@ -124,10 +164,8 @@ def delete_object_tags(object_id: str): """ Delete all ObjectTag entries for a given object. """ - tags = ( - ObjectTag.objects.filter( - object_id=object_id, - ) + tags = ObjectTag.objects.filter( + object_id=object_id, ) tags.delete() diff --git a/openedx_tagging/core/tagging/import_export/import_plan.py b/openedx_tagging/core/tagging/import_export/import_plan.py index 774afcad..f58390df 100644 --- a/openedx_tagging/core/tagging/import_export/import_plan.py +++ b/openedx_tagging/core/tagging/import_export/import_plan.py @@ -16,6 +16,7 @@ class TagItem: """ Tag representation on the tag import plan """ + id: str value: str index: int | None = 0 diff --git a/openedx_tagging/core/tagging/import_export/parsers.py b/openedx_tagging/core/tagging/import_export/parsers.py index 1fb71473..b0132f3d 100644 --- a/openedx_tagging/core/tagging/import_export/parsers.py +++ b/openedx_tagging/core/tagging/import_export/parsers.py @@ -103,7 +103,9 @@ def _export_data(cls, tags: list[dict], taxonomy: Taxonomy) -> str: raise NotImplementedError @classmethod - def _parse_tags(cls, tags_data: list[dict]) -> tuple[list[TagItem], list[TagParserError]]: + def _parse_tags( + cls, tags_data: list[dict] + ) -> tuple[list[TagItem], list[TagParserError]]: """ Validate the required fields of each tag. diff --git a/openedx_tagging/core/tagging/migrations/0007_tag_import_task_log_null_fix.py b/openedx_tagging/core/tagging/migrations/0007_tag_import_task_log_null_fix.py index c4a4067a..48e27811 100644 --- a/openedx_tagging/core/tagging/migrations/0007_tag_import_task_log_null_fix.py +++ b/openedx_tagging/core/tagging/migrations/0007_tag_import_task_log_null_fix.py @@ -4,15 +4,16 @@ class Migration(migrations.Migration): - dependencies = [ - ('oel_tagging', '0006_auto_20230802_1631'), + ("oel_tagging", "0006_auto_20230802_1631"), ] operations = [ migrations.AlterField( - model_name='tagimporttask', - name='log', - field=models.TextField(blank=True, default=None, help_text='Action execution logs'), + model_name="tagimporttask", + name="log", + field=models.TextField( + blank=True, default=None, help_text="Action execution logs" + ), ), ] diff --git a/openedx_tagging/core/tagging/migrations/0008_taxonomy_description_not_null.py b/openedx_tagging/core/tagging/migrations/0008_taxonomy_description_not_null.py index 37b35282..73da9320 100644 --- a/openedx_tagging/core/tagging/migrations/0008_taxonomy_description_not_null.py +++ b/openedx_tagging/core/tagging/migrations/0008_taxonomy_description_not_null.py @@ -6,16 +6,19 @@ class Migration(migrations.Migration): - dependencies = [ - ('oel_tagging', '0007_tag_import_task_log_null_fix'), + ("oel_tagging", "0007_tag_import_task_log_null_fix"), ] operations = [ migrations.AlterField( - model_name='taxonomy', - name='description', - field=openedx_learning.lib.fields.MultiCollationTextField(blank=True, default='', help_text='Provides extra information for the user when applying tags from this taxonomy to an object.'), + model_name="taxonomy", + name="description", + field=openedx_learning.lib.fields.MultiCollationTextField( + blank=True, + default="", + help_text="Provides extra information for the user when applying tags from this taxonomy to an object.", + ), preserve_default=False, ), ] diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index 20f610e4..829909b6 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -268,7 +268,10 @@ def copy(self, taxonomy: Taxonomy) -> Taxonomy: self._taxonomy_class = taxonomy._taxonomy_class return self - def get_tags(self, tag_set: models.QuerySet | None = None) -> list[Tag]: + def get_tags( + self, + tag_set: models.QuerySet[Tag] | None = None, + ) -> list[Tag]: """ Returns a list of all Tags in the current taxonomy, from the root(s) down to TAXONOMY_MAX_DEPTH tags, in tree order. @@ -289,6 +292,7 @@ def get_tags(self, tag_set: models.QuerySet | None = None) -> list[Tag]: tag_set = self.tag_set.all() parents = None + for depth in range(TAXONOMY_MAX_DEPTH): filtered_tags = tag_set.prefetch_related("parent") if parents is None: @@ -310,6 +314,42 @@ def get_tags(self, tag_set: models.QuerySet | None = None) -> list[Tag]: break return tags + def get_filtered_tags( + self, + tag_set: models.QuerySet[Tag] | None = None, + parent_tag_id: int | None = None, + search_term: str | None = None, + search_in_all: bool = False, + ) -> models.QuerySet[Tag]: + """ + Returns a filtered QuerySet of tags. + By default returns the root tags of the given taxonomy + + Use `parent_tag_id` to return the children of a tag. + + Use `search_term` to filter the results by values that contains `search_term`. + + Set `search_in_all` to True to make the search in all tags on the given taxonomy. + + Note: This is mostly an 'internal' API and generally code outside of openedx_tagging + should use the APIs in openedx_tagging.api which in turn use this. + """ + if tag_set is None: + tag_set = self.tag_set.all() + + if self.allow_free_text: + return tag_set.none() + + if not search_in_all: + # If not search in all taxonomy, then apply parent filter. + tag_set = tag_set.filter(parent=parent_tag_id) + + if search_term: + # Apply search filter + tag_set = tag_set.filter(value__icontains=search_term) + + return tag_set.order_by("value", "id") + def validate_object_tag( self, object_tag: "ObjectTag", @@ -348,7 +388,9 @@ def _check_taxonomy( Subclasses can override this method to perform their own taxonomy validation checks. """ # Must be linked to this taxonomy - return (object_tag.taxonomy_id is not None) and object_tag.taxonomy_id == self.id + return ( + object_tag.taxonomy_id is not None + ) and object_tag.taxonomy_id == self.id def _check_tag( self, @@ -481,9 +523,11 @@ def autocomplete_tags( # Fetch tags that the object already has to exclude them from the result excluded_tags: list[str] = [] if object_id: - excluded_tags = list(self.objecttag_set.filter(object_id=object_id).values_list( - "_value", flat=True - )) + excluded_tags = list( + self.objecttag_set.filter(object_id=object_id).values_list( + "_value", flat=True + ) + ) return ( # Fetch object tags from this taxonomy whose value contains the search self.objecttag_set.filter(_value__icontains=search) diff --git a/openedx_tagging/core/tagging/models/system_defined.py b/openedx_tagging/core/tagging/models/system_defined.py index 275d6e97..c78e9aa8 100644 --- a/openedx_tagging/core/tagging/models/system_defined.py +++ b/openedx_tagging/core/tagging/models/system_defined.py @@ -10,7 +10,7 @@ from django.contrib.auth import get_user_model from django.db import models -from openedx_tagging.core.tagging.models.base import ObjectTag +from openedx_tagging.core.tagging.models.base import ObjectTag, Tag from .base import ObjectTag, Tag, Taxonomy @@ -241,7 +241,10 @@ class LanguageTaxonomy(SystemDefinedTaxonomy): class Meta: proxy = True - def get_tags(self, tag_set: models.QuerySet | None = None) -> list[Tag]: + def get_tags( + self, + tag_set: models.QuerySet[Tag] | None = None, + ) -> list[Tag]: """ Returns a list of all the available Language Tags, annotated with ``depth`` = 0. """ @@ -249,6 +252,32 @@ def get_tags(self, tag_set: models.QuerySet | None = None) -> list[Tag]: tag_set = self.tag_set.filter(external_id__in=available_langs) return super().get_tags(tag_set=tag_set) + def get_filtered_tags( + self, + tag_set: models.QuerySet[Tag] | None = None, + parent_tag_id: int | None = None, + search_term: str | None = None, + search_in_all: bool = False, + ) -> models.QuerySet[Tag]: + """ + Returns a filtered QuerySet of available Language Tags. + By default returns all the available Language Tags. + + `parent_tag_id` returns an empty result because all Language tags are root tags. + + Use `search_term` to filter the results by values that contains `search_term`. + """ + if parent_tag_id: + return self.tag_set.none() + + available_langs = self._get_available_languages() + tag_set = self.tag_set.filter(external_id__in=available_langs) + return super().get_filtered_tags( + tag_set=tag_set, + search_term=search_term, + search_in_all=search_in_all, + ) + def _get_available_languages(cls) -> set[str]: """ Get available languages from Django LANGUAGE. diff --git a/openedx_tagging/core/tagging/rest_api/paginators.py b/openedx_tagging/core/tagging/rest_api/paginators.py new file mode 100644 index 00000000..6e2fd99f --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/paginators.py @@ -0,0 +1,29 @@ +from edx_rest_framework_extensions.paginators import DefaultPagination # type: ignore[import] + +# From this point, the tags begin to be paginated +TAGS_THRESHOLD = 1000 + +# From this point, search tags begin to be paginated +SEARCH_TAGS_THRESHOLD = 200 + + +class TagsPagination(DefaultPagination): + """ + Custom pagination configuration for taxonomies + with a large number of tags. Used on the get tags API view. + """ + page_size = 10 + max_page_size = 300 + + +class DisabledTagsPagination(DefaultPagination): + """ + Custom pagination configuration for taxonomies + with a small number of tags. Used on the get tags API view + + This class allows to bring all the tags of the taxonomy. + It should be used if the number of tags within + the taxonomy does not exceed `TAGS_THRESHOLD`. + """ + page_size = TAGS_THRESHOLD + max_page_size = TAGS_THRESHOLD + 1 diff --git a/openedx_tagging/core/tagging/rest_api/v1/permissions.py b/openedx_tagging/core/tagging/rest_api/v1/permissions.py index 245fc3cb..2e7f921c 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/permissions.py +++ b/openedx_tagging/core/tagging/rest_api/v1/permissions.py @@ -1,7 +1,7 @@ """ Tagging permissions """ - +import rules # type: ignore[import] from rest_framework.permissions import DjangoObjectPermissions @@ -27,3 +27,15 @@ class ObjectTagObjectPermissions(DjangoObjectPermissions): "PATCH": ["%(app_label)s.change_%(model_name)s"], "DELETE": ["%(app_label)s.delete_%(model_name)s"], } + + +class TagListPermissions(DjangoObjectPermissions): + def has_permission(self, request, view): + if not request.user or ( + not request.user.is_authenticated and self.authenticated_users_only + ): + return False + return True + + def has_object_permission(self, request, view, obj): + return rules.has_perm("oel_tagging.list_tag", request.user, obj) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 787d3427..756d4a79 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -3,8 +3,9 @@ """ from rest_framework import serializers +from rest_framework.reverse import reverse -from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy +from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy class TaxonomyListQueryParamsSerializer(serializers.Serializer): @@ -70,4 +71,88 @@ class ObjectTagUpdateQueryParamsSerializer(serializers.Serializer): Serializer of the query params for the ObjectTag UPDATE view """ - taxonomy = serializers.PrimaryKeyRelatedField(queryset=Taxonomy.objects.all(), required=True) + taxonomy = serializers.PrimaryKeyRelatedField( + queryset=Taxonomy.objects.all(), required=True + ) + + +class TagsSerializer(serializers.ModelSerializer): + """ + Serializer for Tags + + Adds a link to get the sub tags + """ + + sub_tags_link = serializers.SerializerMethodField() + children_count = serializers.SerializerMethodField() + + class Meta: + model = Tag + fields = ( + "id", + "value", + "taxonomy_id", + "parent_id", + "sub_tags_link", + "children_count", + ) + + def get_sub_tags_link(self, obj): + if obj.children.count(): + query_params = f"?parent_tag_id={obj.id}" + url = ( + reverse("oel_tagging:taxonomy-tags", args=[str(obj.taxonomy_id)]) + + query_params + ) + request = self.context.get("request") + return request.build_absolute_uri(url) + + def get_children_count(self, obj): + return obj.children.count() + + +class TagsWithSubTagsSerializer(serializers.ModelSerializer): + """ + Serializer for Tags. + + Represents a tree with a list of sub tags + """ + + sub_tags = serializers.SerializerMethodField() + children_count = serializers.SerializerMethodField() + + class Meta: + model = Tag + fields = ( + "id", + "value", + "taxonomy_id", + "sub_tags", + "children_count", + ) + + def get_sub_tags(self, obj): + serializer = TagsWithSubTagsSerializer( + obj.children.all().order_by("value", "id"), + many=True, + read_only=True, + ) + return serializer.data + + def get_children_count(self, obj): + return obj.children.count() + + +class TagsForSearchSerializer(TagsWithSubTagsSerializer): + """ + Serializer for Tags + + Used to filter sub tags of a given tag + """ + + def get_sub_tags(self, obj): + serializer = TagsWithSubTagsSerializer(obj.sub_tags, many=True, read_only=True) + return serializer.data + + def get_children_count(self, obj): + return len(obj.sub_tags) diff --git a/openedx_tagging/core/tagging/rest_api/v1/urls.py b/openedx_tagging/core/tagging/rest_api/v1/urls.py index 02cb48e4..7b96cf98 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/urls.py +++ b/openedx_tagging/core/tagging/rest_api/v1/urls.py @@ -11,4 +11,11 @@ router.register("taxonomies", views.TaxonomyView, basename="taxonomy") router.register("object_tags", views.ObjectTagView, basename="object_tag") -urlpatterns = [path("", include(router.urls))] +urlpatterns = [ + path("", include(router.urls)), + path( + "taxonomies//tags/", + views.TaxonomyTagsView.as_view(), + name="taxonomy-tags", + ), +] diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py index 008859f5..52029e04 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/views.py +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -1,21 +1,39 @@ """ Tagging API Views """ +from __future__ import annotations + from django.db import models from django.http import Http404 from rest_framework import mixins from rest_framework.exceptions import MethodNotAllowed, PermissionDenied, ValidationError +from rest_framework.generics import ListAPIView from rest_framework.viewsets import GenericViewSet, ModelViewSet -from ...api import create_taxonomy, get_object_tags, get_taxonomies, get_taxonomy, tag_object +from openedx_tagging.core.tagging.models.base import Tag + +from ...api import ( + create_taxonomy, + get_children_tags, + get_object_tags, + get_root_tags, + get_taxonomies, + get_taxonomy, + search_tags, + tag_object, +) from ...models import Taxonomy from ...rules import ChangeObjectTagPermissionItem -from .permissions import ObjectTagObjectPermissions, TaxonomyObjectPermissions +from ..paginators import SEARCH_TAGS_THRESHOLD, TAGS_THRESHOLD, DisabledTagsPagination, TagsPagination +from .permissions import ObjectTagObjectPermissions, TagListPermissions, TaxonomyObjectPermissions from .serializers import ( ObjectTagListQueryParamsSerializer, ObjectTagSerializer, ObjectTagUpdateBodySerializer, ObjectTagUpdateQueryParamsSerializer, + TagsForSearchSerializer, + TagsSerializer, + TagsWithSubTagsSerializer, TaxonomyListQueryParamsSerializer, TaxonomySerializer, ) @@ -169,7 +187,12 @@ def perform_create(self, serializer) -> None: serializer.instance = create_taxonomy(**serializer.validated_data) -class ObjectTagView(mixins.RetrieveModelMixin, mixins.UpdateModelMixin, mixins.ListModelMixin, GenericViewSet): +class ObjectTagView( + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + mixins.ListModelMixin, + GenericViewSet, +): """ View to retrieve paginated ObjectTags for a provided Object ID (object_id). @@ -276,7 +299,9 @@ def update(self, request, object_id, partial=False): if partial: raise MethodNotAllowed("PATCH", detail="PATCH not allowed") - query_params = ObjectTagUpdateQueryParamsSerializer(data=request.query_params.dict()) + query_params = ObjectTagUpdateQueryParamsSerializer( + data=request.query_params.dict() + ) query_params.is_valid(raise_exception=True) taxonomy = query_params.validated_data.get("taxonomy", None) taxonomy = taxonomy.cast() @@ -289,7 +314,9 @@ def update(self, request, object_id, partial=False): ) if not request.user.has_perm(perm, perm_obj): - raise PermissionDenied("You do not have permission to change object tags for this taxonomy or object_id.") + raise PermissionDenied( + "You do not have permission to change object tags for this taxonomy or object_id." + ) body = ObjectTagUpdateBodySerializer(data=request.data) body.is_valid(raise_exception=True) @@ -301,3 +328,181 @@ def update(self, request, object_id, partial=False): raise ValidationError(e) return self.retrieve(request, object_id) + + +class TaxonomyTagsView(ListAPIView): + """ + View to list tags of a taxonomy. + + **List Query Parameters** + * pk (required) - The pk of the taxonomy to retrieve tags. + * parent_tag_id (optional) - Id of the tag to retrieve children tags. + * page (optional) - Page number (default: 1) + * page_size (optional) - Number of items per page (default: 10) + + **List Example Requests** + GET api/tagging/v1/taxonomy/:pk/tags - Get tags of taxonomy + GET api/tagging/v1/taxonomy/:pk/tags?parent_tag_id=30 - Get children tags of tag + + **List Query Returns** + * 200 - Success + * 400 - Invalid query parameter + * 403 - Permission denied + * 404 - Taxonomy not found + """ + + permission_classes = [TagListPermissions] + pagination_enabled = True + + def __init__(self): + # Initialized here to avoid errors on type hints + self.serializer_class = TagsSerializer + + def get_pagination_class(self): + """ + Get the corresponding class depending if the pagination is enabled. + + It is necessary to call this function before returning the data. + """ + if self.pagination_enabled: + return TagsPagination + else: + return DisabledTagsPagination + + def get_taxonomy(self, pk: int) -> Taxonomy: + """ + Get the taxonomy from `pk` or raise 404. + """ + taxonomy = get_taxonomy(pk) + if not taxonomy: + raise Http404("Taxonomy not found") + self.check_object_permissions(self.request, taxonomy) + return taxonomy + + def _build_search_tree(self, tags: list[Tag]) -> list[Tag]: + """ + Builds a tree with the result tags for a search. + + The retult is a pruned tree that contains + the path from root tags to tags that match the search. + """ + tag_ids = [tag.id for tag in tags] + + # Get missing parents. + # Not all parents are in the search result. + # This occurs when a child tag is on the search result, but its parent not, + # we need to add the parent to show the tree from the root to the child. + for tag in tags: + if tag.parent and tag.parent_id and tag.parent_id not in tag_ids: + tag_ids.append(tag.parent_id) + tags.append(tag.parent) # Our loop will iterate over this new parent tag too. + + groups: dict[int, list[Tag]] = {} + roots: list[Tag] = [] + + # Group tags by parent + for tag in tags: + if tag.parent_id is not None: + if tag.parent_id not in groups: + groups[tag.parent_id] = [] + groups[tag.parent_id].append(tag) + else: + roots.append(tag) + + for tag in tags: + # Used to serialize searched childrens + tag.sub_tags = groups.get(tag.id, []) # type: ignore[attr-defined] + + return roots + + def get_matching_tags( + self, + taxonomy_id: int, + parent_tag_id: str | None = None, + search_term: str | None = None, + ) -> list[Tag]: + """ + Returns a list of tags for the given taxonomy. + + The pagination can be enabled or disabled depending of the taxonomy size. + You can read the desicion '0014_*' to more info about this logic. + Also, determines the serializer to be used. + + Use `parent_tag_id` to get the children of the given tag. + + Use `search_term` to filter tags values that contains the given term. + """ + taxonomy = self.get_taxonomy(taxonomy_id) + if parent_tag_id: + # Get children of a tag. + + # If you need to get the children, then the roots are + # paginated, so we need to paginate the childrens too. + self.pagination_enabled = True + + # Normal serializer, with children link. + self.serializer_class = TagsSerializer + return get_children_tags( + taxonomy, + int(parent_tag_id), + search_term=search_term, + ) + else: + if search_term: + # Search tags + result = search_tags( + taxonomy, + search_term, + ) + # Checks the result size to determine whether + # to turn pagination on or off. + self.pagination_enabled = len(result) > SEARCH_TAGS_THRESHOLD + + # Use the special serializer to only show the tree + # of the search result. + self.serializer_class = TagsForSearchSerializer + + result = self._build_search_tree(result) + else: + # Get root tags of taxonomy + + # Checks the taxonomy size to determine whether + # to turn pagination on or off. + self.pagination_enabled = taxonomy.tag_set.count() > TAGS_THRESHOLD + + if self.pagination_enabled: + # If pagination is enabled, use the normal serializer + # with children link. + self.serializer_class = TagsSerializer + else: + # If pagination is disabled, use the special serializer + # to show children. In this case, we return all taxonomy tags + # in a tree structure. + self.serializer_class = TagsWithSubTagsSerializer + + result = get_root_tags(taxonomy) + + return result + + def get_queryset(self) -> list[Tag]: # type: ignore[override] + """ + Builds and returns the queryset to be paginated. + + The return type is not a QuerySet because the tagging python api functions + return lists, and on this point convert the list to a query set + is an unnecesary operation. + """ + pk = self.kwargs.get("pk") + parent_tag_id = self.request.query_params.get("parent_tag_id", None) + search_term = self.request.query_params.get("search_term", None) + + result = self.get_matching_tags( + pk, + parent_tag_id=parent_tag_id, + search_term=search_term, + ) + + # This function is not called automatically + self.pagination_class = self.get_pagination_class() + + return result diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 880fe19c..00ec8811 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -12,7 +12,9 @@ from .models import Tag, Taxonomy -UserType = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser] +UserType = Union[ + django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser +] # Global staff are taxonomy admins. @@ -74,7 +76,9 @@ def can_change_object_tag_objectid(_user: UserType, _object_id: str) -> bool: @rules.predicate -def can_change_object_tag(user: UserType, perm_obj: ChangeObjectTagPermissionItem | None = None) -> bool: +def can_change_object_tag( + user: UserType, perm_obj: ChangeObjectTagPermissionItem | None = None +) -> bool: """ Checks if the user has permissions to create or modify tags on the given taxonomy and object_id. """ @@ -84,7 +88,9 @@ def can_change_object_tag(user: UserType, perm_obj: ChangeObjectTagPermissionIte return True # Checks the permission for the taxonomy - taxonomy_perm = user.has_perm("oel_tagging.change_objecttag_taxonomy", perm_obj.taxonomy) + taxonomy_perm = user.has_perm( + "oel_tagging.change_objecttag_taxonomy", perm_obj.taxonomy + ) if not taxonomy_perm: return False @@ -109,6 +115,7 @@ def can_change_object_tag(user: UserType, perm_obj: ChangeObjectTagPermissionIte rules.add_perm("oel_tagging.change_tag", can_change_tag) rules.add_perm("oel_tagging.delete_tag", is_taxonomy_admin) rules.add_perm("oel_tagging.view_tag", rules.always_allow) +rules.add_perm("oel_tagging.list_tag", can_view_taxonomy) # ObjectTag rules.add_perm("oel_tagging.add_objecttag", can_change_object_tag) diff --git a/tests/openedx_tagging/core/tagging/test_api.py b/tests/openedx_tagging/core/tagging/test_api.py index 46e8c190..8f185e75 100644 --- a/tests/openedx_tagging/core/tagging/test_api.py +++ b/tests/openedx_tagging/core/tagging/test_api.py @@ -17,9 +17,17 @@ ("az", "Azerbaijani"), ("en", "English"), ("id", "Indonesian"), + ("ga", "Irish"), + ("pl", "Polish"), ("qu", "Quechua"), ("zu", "Zulu"), ] +# Languages that contains 'ish' +filtered_test_languages = [ + ("en", "English"), + ("ga", "Irish"), + ("pl", "Polish"), +] @ddt.ddt @@ -108,6 +116,46 @@ def test_get_tags(self) -> None: expected_langs = [lang[0] for lang in test_languages] assert langs == expected_langs + @override_settings(LANGUAGES=test_languages) + def test_get_root_tags(self): + assert tagging_api.get_root_tags(self.taxonomy) == self.domain_tags + assert tagging_api.get_root_tags(self.system_taxonomy) == self.system_tags + tags = tagging_api.get_root_tags(self.language_taxonomy) + langs = [tag.external_id for tag in tags] + expected_langs = [lang[0] for lang in test_languages] + assert langs == expected_langs + + @override_settings(LANGUAGES=test_languages) + def test_search_tags(self): + assert tagging_api.search_tags( + self.taxonomy, + search_term='eU' + ) == self.filtered_tags + + tags = tagging_api.search_tags(self.language_taxonomy, search_term='IsH') + langs = [tag.external_id for tag in tags] + expected_langs = [lang[0] for lang in filtered_test_languages] + assert langs == expected_langs + + def test_get_children_tags(self): + assert tagging_api.get_children_tags( + self.taxonomy, + self.animalia.id, + ) == self.phylum_tags + assert tagging_api.get_children_tags( + self.taxonomy, + self.animalia.id, + search_term='dA', + ) == self.filtered_phylum_tags + assert not tagging_api.get_children_tags( + self.system_taxonomy, + self.system_taxonomy_tag.id, + ) + assert not tagging_api.get_children_tags( + self.language_taxonomy, + self.english_tag, + ) + def check_object_tag( self, object_tag: ObjectTag, diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py index 4b6f644a..6cef2bcf 100644 --- a/tests/openedx_tagging/core/tagging/test_models.py +++ b/tests/openedx_tagging/core/tagging/test_models.py @@ -39,7 +39,9 @@ def setUp(self): self.eubacteria = get_tag("Eubacteria") self.chordata = get_tag("Chordata") self.mammalia = get_tag("Mammalia") + self.animalia = get_tag("Animalia") self.system_taxonomy_tag = get_tag("System Tag 1") + self.english_tag = get_tag("English") self.user_1 = get_user_model()( id=1, username="test_user_1", @@ -58,6 +60,12 @@ def setUp(self): get_tag("Bacteria"), get_tag("Eukaryota"), ] + # Domain tags that contains 'ar' + self.filtered_domain_tags = [ + get_tag("Archaea"), + get_tag("Eukaryota"), + ] + # Kingdom tags (depth=1) self.kingdom_tags = [ # Kingdoms of https://en.wikipedia.org/wiki/Archaea @@ -74,6 +82,7 @@ def setUp(self): get_tag("Plantae"), get_tag("Protista"), ] + # Phylum tags (depth=2) self.phylum_tags = [ # Some phyla of https://en.wikipedia.org/wiki/Animalia @@ -85,6 +94,19 @@ def setUp(self): get_tag("Placozoa"), get_tag("Porifera"), ] + # Phylum tags that contains 'da' + self.filtered_phylum_tags = [ + get_tag("Arthropoda"), + get_tag("Chordata"), + get_tag("Cnidaria"), + ] + + # Biology tags that contains 'eu' + self.filtered_tags = [ + get_tag("Eubacteria"), + get_tag("Eukaryota"), + get_tag("Euryarchaeida"), + ] self.system_tags = [ get_tag("System Tag 1"), @@ -220,11 +242,49 @@ def test_get_tags(self): *self.phylum_tags, ] + def test_get_root_tags(self): + assert list(self.taxonomy.get_filtered_tags()) == self.domain_tags + assert list( + self.taxonomy.get_filtered_tags(search_term='aR') + ) == self.filtered_domain_tags + def test_get_tags_free_text(self): self.taxonomy.allow_free_text = True with self.assertNumQueries(0): assert self.taxonomy.get_tags() == [] + def test_get_children_tags(self): + assert list( + self.taxonomy.get_filtered_tags(parent_tag_id=self.animalia.id) + ) == self.phylum_tags + assert list( + self.taxonomy.get_filtered_tags( + parent_tag_id=self.animalia.id, + search_term='dA', + ) + ) == self.filtered_phylum_tags + assert not list( + self.system_taxonomy.get_filtered_tags( + parent_tag_id=self.system_taxonomy_tag.id + ) + ) + + def test_get_children_tags_free_text(self): + self.taxonomy.allow_free_text = True + assert not list(self.taxonomy.get_filtered_tags( + parent_tag_id=self.animalia.id + )) + assert not list(self.taxonomy.get_filtered_tags( + parent_tag_id=self.animalia.id, + search_term='dA', + )) + + def test_search_tags(self): + assert list(self.taxonomy.get_filtered_tags( + search_term='eU', + search_in_all=True + )) == self.filtered_tags + def test_get_tags_shallow_taxonomy(self): taxonomy = Taxonomy.objects.create(name="Difficulty") tags = [ diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index a50ac3e6..99c55256 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -14,11 +14,13 @@ from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy +from openedx_tagging.core.tagging.rest_api.paginators import TagsPagination User = get_user_model() TAXONOMY_LIST_URL = "/tagging/rest_api/v1/taxonomies/" TAXONOMY_DETAIL_URL = "/tagging/rest_api/v1/taxonomies/{pk}/" +TAXONOMY_TAGS_URL = "/tagging/rest_api/v1/taxonomies/{pk}/tags/" OBJECT_TAGS_RETRIEVE_URL = "/tagging/rest_api/v1/object_tags/{object_id}/" @@ -28,8 +30,8 @@ def check_taxonomy( - data: dict, - id, # pylint: disable=redefined-builtin + data, + taxonomy_id, name, description="", enabled=True, @@ -40,9 +42,9 @@ def check_taxonomy( visible_to_authors=True, ): """ - Helper method to check expected fields of a Taxonomy + Check taxonomy data """ - assert data["id"] == id + assert data["id"] == taxonomy_id assert data["name"] == name assert data["description"] == description assert data["enabled"] == enabled @@ -53,12 +55,12 @@ def check_taxonomy( assert data["visible_to_authors"] == visible_to_authors -@ddt.ddt -class TestTaxonomyViewSet(APITestCase): +class TestTaxonomyViewMixin(APITestCase): """ - Test of the Taxonomy REST API + Mixin for taxonomy views. Adds users. """ - def setUp(self) -> None: + + def setUp(self): super().setUp() self.user = User.objects.create( @@ -72,6 +74,13 @@ def setUp(self) -> None: is_staff=True, ) + +@ddt.ddt +class TestTaxonomyViewSet(TestTaxonomyViewMixin): + """ + Test taxonomy view set + """ + @ddt.data( (None, status.HTTP_200_OK, 4), (1, status.HTTP_200_OK, 3), @@ -238,7 +247,7 @@ def test_create_taxonomy_system_defined(self, create_data): self.client.force_authenticate(user=self.staff) response = self.client.post(url, create_data, format="json") assert response.status_code == status.HTTP_201_CREATED - assert response.data["system_defined"] is False + assert not response.data["system_defined"] @ddt.data( (None, status.HTTP_403_FORBIDDEN), @@ -391,7 +400,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 + assert response.status_code == status.HTTP_404_NOT_FOUND @ddt.ddt @@ -782,3 +791,319 @@ 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): + """ + Tests the list tags of taxonomy view + """ + + fixtures = ["tests/openedx_tagging/core/fixtures/tagging.yaml"] + + def setUp(self): + self.small_taxonomy = Taxonomy.objects.get(name="Life on Earth") + self.large_taxonomy = Taxonomy(name="Large Taxonomy") + self.large_taxonomy.save() + + self.small_taxonomy_url = TAXONOMY_TAGS_URL.format(pk=self.small_taxonomy.pk) + self.large_taxonomy_url = TAXONOMY_TAGS_URL.format(pk=self.large_taxonomy.pk) + + self.root_tags_count = 51 + self.children_tags_count = [12, 12] # 51 * 12 * 12 = 7344 tags + + self.page_size = TagsPagination().page_size + + return super().setUp() + + def _create_tag(self, depth: int, parent: Tag | None = None): + """ + Creates tags and children in a recursive way. + """ + tag_count = self.large_taxonomy.tag_set.count() + tag = Tag( + taxonomy=self.large_taxonomy, + parent=parent, + value=f"Tag {tag_count}", + ) + tag.save() + if depth < len(self.children_tags_count): + for _ in range(self.children_tags_count[depth]): + self._create_tag(depth + 1, parent=tag) + return tag + + def _build_large_taxonomy(self): + # Pupulates the large taxonomy with tags + for _ in range(self.root_tags_count): + self._create_tag(0) + + def test_invalid_taxonomy(self): + url = TAXONOMY_TAGS_URL.format(pk=212121) + + self.client.force_authenticate(user=self.staff) + response = self.client.get(url) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_not_authorized_user(self): + # Not authenticated user + response = self.client.get(self.small_taxonomy_url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + self.small_taxonomy.enabled = False + self.small_taxonomy.save() + self.client.force_authenticate(user=self.user) + response = self.client.get(self.small_taxonomy_url) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_small_taxonomy(self): + self.client.force_authenticate(user=self.staff) + response = self.client.get(self.small_taxonomy_url) + assert response.status_code == status.HTTP_200_OK + + data = response.data + results = data.get("results", []) + + # Count of root tags + root_count = self.small_taxonomy.tag_set.filter(parent=None).count() + assert len(results) == root_count + + # Checking tag fields + root_tag = self.small_taxonomy.tag_set.get(id=results[0].get("id")) + root_children_count = root_tag.children.count() + assert results[0].get("value") == root_tag.value + assert results[0].get("taxonomy_id") == self.small_taxonomy.id + assert results[0].get("children_count") == root_children_count + assert len(results[0].get("sub_tags")) == root_children_count + + # Checking pagination values + assert data.get("next") is None + assert data.get("previous") is None + assert data.get("count") == root_count + assert data.get("num_pages") == 1 + assert data.get("current_page") == 1 + + def test_small_search(self): + search_term = 'eU' + url = f"{self.small_taxonomy_url}?search_term={search_term}" + self.client.force_authenticate(user=self.staff) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + + data = response.data + results = data.get("results", []) + + assert len(results) == 3 + + # Checking pagination values + assert data.get("next") is None + assert data.get("previous") is None + assert data.get("count") == 3 + assert data.get("num_pages") == 1 + assert data.get("current_page") == 1 + + def test_large_taxonomy(self): + self._build_large_taxonomy() + self.client.force_authenticate(user=self.staff) + response = self.client.get(self.large_taxonomy_url) + assert response.status_code == status.HTTP_200_OK + + data = response.data + results = data.get("results", []) + + # Count of paginated root tags + assert len(results) == self.page_size + + # Checking tag fields + root_tag = self.large_taxonomy.tag_set.get(id=results[0].get("id")) + assert results[0].get("value") == root_tag.value + assert results[0].get("taxonomy_id") == self.large_taxonomy.id + assert results[0].get("parent_id") == root_tag.parent_id + assert results[0].get("children_count") == root_tag.children.count() + assert results[0].get("sub_tags_link") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}" + f"/tags/?parent_tag_id={root_tag.id}" + ) + + # Checking pagination values + assert data.get("next") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}/tags/?page=2" + ) + assert data.get("previous") is None + assert data.get("count") == self.root_tags_count + assert data.get("num_pages") == 6 + assert data.get("current_page") == 1 + + def test_next_page_large_taxonomy(self): + self._build_large_taxonomy() + self.client.force_authenticate(user=self.staff) + + # Gets the root tags to obtain the next links + response = self.client.get(self.large_taxonomy_url) + + # Gets the next root tags + response = self.client.get(response.data.get("next")) + assert response.status_code == status.HTTP_200_OK + + data = response.data + + # Checking pagination values + assert data.get("next") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}/tags/?page=3" + ) + assert data.get("previous") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}/tags/" + ) + assert data.get("count") == self.root_tags_count + assert data.get("num_pages") == 6 + assert data.get("current_page") == 2 + + def test_large_search(self): + self._build_large_taxonomy() + search_term = '1' + url = f"{self.large_taxonomy_url}?search_term={search_term}" + self.client.force_authenticate(user=self.staff) + response = self.client.get(url) + assert response.status_code == status.HTTP_200_OK + + data = response.data + results = data.get("results", []) + + # Count of paginated root tags + assert len(results) == self.page_size + + # Checking pagination values + assert data.get("next") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}" + f"/tags/?page=2&search_term={search_term}" + ) + assert data.get("previous") is None + assert data.get("count") == 51 + assert data.get("num_pages") == 6 + assert data.get("current_page") == 1 + + def test_next_large_search(self): + self._build_large_taxonomy() + search_term = '1' + url = f"{self.large_taxonomy_url}?search_term={search_term}" + + # Get first page of the search + self.client.force_authenticate(user=self.staff) + response = self.client.get(url) + + # Get next page + response = self.client.get(response.data.get("next")) + + data = response.data + results = data.get("results", []) + + # Count of paginated root tags + assert len(results) == self.page_size + + # Checking pagination values + assert data.get("next") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}" + f"/tags/?page=3&search_term={search_term}" + ) + assert data.get("previous") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}" + f"/tags/?search_term={search_term}" + ) + assert data.get("count") == 51 + assert data.get("num_pages") == 6 + assert data.get("current_page") == 2 + + def test_get_children(self): + self._build_large_taxonomy() + self.client.force_authenticate(user=self.staff) + + # Get root tags to obtain the children link of a tag. + response = self.client.get(self.large_taxonomy_url) + results = response.data.get("results", []) + + # Get children tags + response = self.client.get(results[0].get("sub_tags_link")) + assert response.status_code == status.HTTP_200_OK + + data = response.data + results = data.get("results", []) + + # Count of paginated children tags + assert len(results) == self.page_size + + # Checking tag fields + tag = self.large_taxonomy.tag_set.get(id=results[0].get("id")) + assert results[0].get("value") == tag.value + assert results[0].get("taxonomy_id") == self.large_taxonomy.id + assert results[0].get("parent_id") == tag.parent_id + assert results[0].get("children_count") == tag.children.count() + assert results[0].get("sub_tags_link") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}" + f"/tags/?parent_tag_id={tag.id}" + ) + + # Checking pagination values + assert data.get("next") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}" + f"/tags/?page=2&parent_tag_id={tag.parent_id}" + ) + assert data.get("previous") is None + assert data.get("count") == self.children_tags_count[0] + assert data.get("num_pages") == 2 + assert data.get("current_page") == 1 + + def test_get_leaves(self): + # Get tags depth=2 + self.client.force_authenticate(user=self.staff) + parent_tag = Tag.objects.get(value="Animalia") + + # Build url to get tags depth=2 + url = f"{self.small_taxonomy_url}?parent_tag_id={parent_tag.id}" + response = self.client.get(url) + results = response.data.get("results", []) + + # Checking tag fields + tag = self.small_taxonomy.tag_set.get(id=results[0].get("id")) + assert results[0].get("value") == tag.value + assert results[0].get("taxonomy_id") == self.small_taxonomy.id + assert results[0].get("parent_id") == tag.parent_id + assert results[0].get("children_count") == tag.children.count() + assert results[0].get("sub_tags_link") is None + + def test_next_children(self): + self._build_large_taxonomy() + self.client.force_authenticate(user=self.staff) + + # Get roots to obtain children link of a tag + response = self.client.get(self.large_taxonomy_url) + results = response.data.get("results", []) + + # Get children to obtain next link + response = self.client.get(results[0].get("sub_tags_link")) + + # Get next children + response = self.client.get(response.data.get("next")) + assert response.status_code == status.HTTP_200_OK + + data = response.data + results = data.get("results", []) + tag = self.large_taxonomy.tag_set.get(id=results[0].get("id")) + + # Checking pagination values + assert data.get("next") is None + assert data.get("previous") == ( + "http://testserver/tagging/" + f"rest_api/v1/taxonomies/{self.large_taxonomy.id}/tags/?parent_tag_id={tag.parent_id}" + ) + assert data.get("count") == self.children_tags_count[0] + assert data.get("num_pages") == 2 + assert data.get("current_page") == 2