From d532b0c24402513a2154d916ee6a8e22372ef338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 24 Jul 2023 16:21:03 -0300 Subject: [PATCH 1/6] feat: add taxonomy view/management apis --- openedx_tagging/core/tagging/rest_api/urls.py | 9 + .../core/tagging/rest_api/v1/__init__.py | 0 .../core/tagging/rest_api/v1/permissions.py | 18 + .../core/tagging/rest_api/v1/serializers.py | 31 ++ .../core/tagging/rest_api/v1/urls.py | 16 + .../core/tagging/rest_api/v1/views.py | 147 +++++++ openedx_tagging/core/tagging/rules.py | 6 +- openedx_tagging/core/tagging/urls.py | 10 + projects/urls.py | 1 + requirements/dev.txt | 4 +- requirements/doc.txt | 4 + requirements/quality.txt | 4 + requirements/test.in | 1 + requirements/test.txt | 4 + test_settings.py | 6 +- .../core/tagging/test_rules.py | 2 +- .../core/tagging/test_views.py | 365 ++++++++++++++++++ tox.ini | 2 +- 18 files changed, 622 insertions(+), 8 deletions(-) create mode 100644 openedx_tagging/core/tagging/rest_api/urls.py create mode 100644 openedx_tagging/core/tagging/rest_api/v1/__init__.py create mode 100644 openedx_tagging/core/tagging/rest_api/v1/permissions.py create mode 100644 openedx_tagging/core/tagging/rest_api/v1/serializers.py create mode 100644 openedx_tagging/core/tagging/rest_api/v1/urls.py create mode 100644 openedx_tagging/core/tagging/rest_api/v1/views.py create mode 100644 openedx_tagging/core/tagging/urls.py create mode 100644 tests/openedx_tagging/core/tagging/test_views.py diff --git a/openedx_tagging/core/tagging/rest_api/urls.py b/openedx_tagging/core/tagging/rest_api/urls.py new file mode 100644 index 000000000..d7f012bb7 --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/urls.py @@ -0,0 +1,9 @@ +""" +Taxonomies API URLs. +""" + +from django.urls import path, include + +from .v1 import urls as v1_urls + +urlpatterns = [path("v1/", include(v1_urls))] diff --git a/openedx_tagging/core/tagging/rest_api/v1/__init__.py b/openedx_tagging/core/tagging/rest_api/v1/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/openedx_tagging/core/tagging/rest_api/v1/permissions.py b/openedx_tagging/core/tagging/rest_api/v1/permissions.py new file mode 100644 index 000000000..b75c5ff79 --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/v1/permissions.py @@ -0,0 +1,18 @@ +""" +Taxonomy permissions +""" + +from rest_framework.permissions import DjangoObjectPermissions + + +class TaxonomyObjectPermissions(DjangoObjectPermissions): + perms_map = { + "GET": ["%(app_label)s.view_%(model_name)s"], + "OPTIONS": [], + "HEAD": ["%(app_label)s.view_%(model_name)s"], + "POST": ["%(app_label)s.add_%(model_name)s"], + "PUT": ["%(app_label)s.change_%(model_name)s"], + "PATCH": ["%(app_label)s.change_%(model_name)s"], + "DELETE": ["%(app_label)s.delete_%(model_name)s"], + } + diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py new file mode 100644 index 000000000..9b173c795 --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -0,0 +1,31 @@ +""" +API Serializers for taxonomies +""" + +from django.utils.module_loading import import_string +from rest_framework import serializers + +from openedx_tagging.core.tagging.models import Taxonomy + +class TaxonomyListQueryParamsSerializer(serializers.Serializer): + """ + Serializer for the query params for the GET view + """ + + enabled = serializers.BooleanField(required=False) + +class TaxonomySerializer(serializers.ModelSerializer): + class Meta: + model = Taxonomy + fields = [ + "id", + "name", + "description", + "enabled", + "required", + "allow_multiple", + "allow_free_text", + "system_defined", + "visible_to_authors", + ] + diff --git a/openedx_tagging/core/tagging/rest_api/v1/urls.py b/openedx_tagging/core/tagging/rest_api/v1/urls.py new file mode 100644 index 000000000..97d653a28 --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/v1/urls.py @@ -0,0 +1,16 @@ +""" +Taxonomies API v1 URLs. +""" + +from rest_framework.routers import DefaultRouter + +from django.urls.conf import path, include + +from . import views + +router = DefaultRouter() +router.register("taxonomies", views.TaxonomyView, basename="taxonomy") + +urlpatterns = [ + path('', include(router.urls)) +] diff --git a/openedx_tagging/core/tagging/rest_api/v1/views.py b/openedx_tagging/core/tagging/rest_api/v1/views.py new file mode 100644 index 000000000..1ea02aca9 --- /dev/null +++ b/openedx_tagging/core/tagging/rest_api/v1/views.py @@ -0,0 +1,147 @@ +""" +Tagging API Views +""" +from django.http import Http404 +from rest_framework.viewsets import ModelViewSet + +from ...api import ( + create_taxonomy, + get_taxonomy, + get_taxonomies, +) +from .serializers import TaxonomyListQueryParamsSerializer, TaxonomySerializer +from .permissions import TaxonomyObjectPermissions + + +class TaxonomyView(ModelViewSet): + """ + View to list, create, retrieve, update, or delete Taxonomies. + + **List Query Parameters** + * enabled (optional) - Filter by enabled status. Valid values: true, false, 1, 0, "true", "false", "1" + + **List Example Requests** + GET api/tagging/v1/taxonomy - Get all taxonomies + GET api/tagging/v1/taxonomy?enabled=true - Get all enabled taxonomies + GET api/tagging/v1/taxonomy?enabled=false - Get all disabled taxonomies + + **List Query Returns** + * 200 - Success + * 400 - Invalid query parameter + * 403 - Permission denied + + **Retrieve Parameters** + * pk (required): - The pk of the taxonomy to retrieve + + **Retrieve Example Requests** + GET api/tagging/v1/taxonomy/:pk - Get a specific taxonomy + + **Retrieve Query Returns** + * 200 - Success + * 404 - Taxonomy not found or User does not have permission to access the taxonomy + + **Create Parameters** + * name (required): User-facing label used when applying tags from this taxonomy to Open edX objects. + * description (optional): Provides extra information for the user when applying tags from this taxonomy to an object. + * enabled (optional): Only enabled taxonomies will be shown to authors (default: true). + * required (optional): Indicates that one or more tags from this taxonomy must be added to an object (default: False). + * allow_multiple (optional): Indicates that multiple tags from this taxonomy may be added to an object (default: False). + * allow_free_text (optional): Indicates that tags in this taxonomy need not be predefined; authors may enter their own tag values (default: False). + + **Create Example Requests** + POST api/tagging/v1/taxonomy - Create a taxonomy + { + "name": "Taxonomy Name", - User-facing label used when applying tags from this taxonomy to Open edX objects." + "description": "This is a description", + "enabled": True, + "required": True, + "allow_multiple": True, + "allow_free_text": True, + } + + + **Create Query Returns** + * 201 - Success + * 403 - Permission denied + + **Update Parameters** + * pk (required): - The pk of the taxonomy to update + + **Update Request Body** + * name (optional): User-facing label used when applying tags from this taxonomy to Open edX objects. + * description (optional): Provides extra information for the user when applying tags from this taxonomy to an object. + * enabled (optional): Only enabled taxonomies will be shown to authors. + * required (optional): Indicates that one or more tags from this taxonomy must be added to an object. + * allow_multiple (optional): Indicates that multiple tags from this taxonomy may be added to an object. + * allow_free_text (optional): Indicates that tags in this taxonomy need not be predefined; authors may enter their own tag values. + + **Update Example Requests** + PUT api/tagging/v1/taxonomy/:pk - Update a taxonomy + { + "name": "Taxonomy New Name", + "description": "This is a new description", + "enabled": False, + "required": False, + "allow_multiple": False, + "allow_free_text": True, + } + PATCH api/tagging/v1/taxonomy/:pk - Partially update a taxonomy + { + "name": "Taxonomy New Name", + } + + **Update Query Returns** + * 200 - Success + * 403 - Permission denied + + **Delete Parameters** + * pk (required): - The pk of the taxonomy to delete + + **Delete Example Requests** + DELETE api/tagging/v1/taxonomy/:pk - Delete a taxonomy + + **Delete Query Returns** + * 200 - Success + * 404 - Taxonomy not found + * 403 - Permission denied + + """ + + + serializer_class = TaxonomySerializer + permission_classes = [TaxonomyObjectPermissions] + + def get_object(self): + """ + Return the requested taxonomy object, if the user has appropriate + permissions. + """ + pk = self.kwargs.get("pk") + taxonomy = get_taxonomy(pk) + if not taxonomy: + raise Http404("Taxonomy not found") + self.check_object_permissions(self.request, taxonomy) + + return taxonomy + + def get_queryset(self): + """ + Return a list of taxonomies. + + Returns all taxonomies by default. + If you want the disabled taxonomies, pass enabled=False. + If you want the enabled taxonomies, pass enabled=True. + """ + query_params = TaxonomyListQueryParamsSerializer( + data=self.request.query_params.dict() + ) + query_params.is_valid(raise_exception=True) + enabled = query_params.data.get("enabled", None) + + return get_taxonomies(enabled) + + def perform_create(self, serializer): + """ + Create a new taxonomy. + """ + serializer.instance = create_taxonomy(**serializer.validated_data) diff --git a/openedx_tagging/core/tagging/rules.py b/openedx_tagging/core/tagging/rules.py index 72b9fa6b4..2cc94d0dc 100644 --- a/openedx_tagging/core/tagging/rules.py +++ b/openedx_tagging/core/tagging/rules.py @@ -16,10 +16,10 @@ @rules.predicate def can_view_taxonomy(user: User, taxonomy: Taxonomy = None) -> bool: """ - Anyone can view an enabled taxonomy, + Anyone can view an enabled taxonomy or list all taxonomies, but only taxonomy admins can view a disabled taxonomy. """ - return (taxonomy and taxonomy.enabled) or is_taxonomy_admin(user) + return not taxonomy or taxonomy.enabled or is_taxonomy_admin(user) @rules.predicate @@ -28,7 +28,7 @@ def can_change_taxonomy(user: User, taxonomy: Taxonomy = None) -> bool: Even taxonomy admins cannot change system taxonomies. """ return is_taxonomy_admin(user) and ( - not taxonomy or not taxonomy or (taxonomy and not taxonomy.system_defined) + not taxonomy or (taxonomy and not taxonomy.system_defined) ) diff --git a/openedx_tagging/core/tagging/urls.py b/openedx_tagging/core/tagging/urls.py new file mode 100644 index 000000000..da2c52081 --- /dev/null +++ b/openedx_tagging/core/tagging/urls.py @@ -0,0 +1,10 @@ +""" +Tagging API URLs. +""" + +from django.urls import path, include + +from .rest_api import urls + +app_name = "oel_tagging" +urlpatterns = [path("", include(urls))] diff --git a/projects/urls.py b/projects/urls.py index e27da7532..cf95abb13 100644 --- a/projects/urls.py +++ b/projects/urls.py @@ -9,5 +9,6 @@ path("admin/", admin.site.urls), path("media_server/", include("openedx_learning.contrib.media_server.urls")), path("rest_api/", include("openedx_learning.rest_api.urls")), + path("tagging/rest_api/", include("openedx_tagging.core.tagging.urls")), path('__debug__/', include('debug_toolbar.urls')), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) diff --git a/requirements/dev.txt b/requirements/dev.txt index add9cf1de..48cf4fb40 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -81,7 +81,9 @@ django==3.2.19 # djangorestframework # edx-i18n-tools django-debug-toolbar==4.1.0 - # via -r requirements/dev.in + # via + # -r requirements/dev.in + # -r requirements/quality.txt djangorestframework==3.14.0 # via -r requirements/quality.txt docutils==0.20.1 diff --git a/requirements/doc.txt b/requirements/doc.txt index d9a4c3c2a..f63eb9113 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -41,8 +41,11 @@ django==3.2.19 # via # -c requirements/constraints.txt # -r requirements/test.txt + # django-debug-toolbar # djangorestframework # sphinxcontrib-django +django-debug-toolbar==4.1.0 + # via -r requirements/test.txt djangorestframework==3.14.0 # via -r requirements/test.txt doc8==1.1.1 @@ -175,6 +178,7 @@ sqlparse==0.4.4 # via # -r requirements/test.txt # django + # django-debug-toolbar stevedore==5.1.0 # via # -r requirements/test.txt diff --git a/requirements/quality.txt b/requirements/quality.txt index c580bd7e1..425cfa514 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -47,7 +47,10 @@ django==3.2.19 # via # -c requirements/constraints.txt # -r requirements/test.txt + # django-debug-toolbar # djangorestframework +django-debug-toolbar==4.1.0 + # via -r requirements/test.txt djangorestframework==3.14.0 # via -r requirements/test.txt docutils==0.20.1 @@ -198,6 +201,7 @@ sqlparse==0.4.4 # via # -r requirements/test.txt # django + # django-debug-toolbar stevedore==5.1.0 # via # -r requirements/test.txt diff --git a/requirements/test.in b/requirements/test.in index 1d521351d..c050dd241 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -14,3 +14,4 @@ pytest-django # pytest extension for better Django support code-annotations # provides commands used by the pii_check make target. ddt # supports data driven tests mock # supports overriding classes and methods in tests +django-debug-toolbar # provides a debug toolbar for Django diff --git a/requirements/test.txt b/requirements/test.txt index e64d8190b..22c43aede 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -23,7 +23,10 @@ ddt==1.6.0 # via # -c requirements/constraints.txt # -r requirements/base.txt + # django-debug-toolbar # djangorestframework +django-debug-toolbar==4.1.0 + # via -r requirements/test.in djangorestframework==3.14.0 # via -r requirements/base.txt exceptiongroup==1.1.1 @@ -72,6 +75,7 @@ sqlparse==0.4.4 # via # -r requirements/base.txt # django + # django-debug-toolbar stevedore==5.1.0 # via code-annotations text-unidecode==1.3 diff --git a/test_settings.py b/test_settings.py index 2e41694e4..63290bc66 100644 --- a/test_settings.py +++ b/test_settings.py @@ -33,8 +33,10 @@ def root(*args): "django.contrib.sessions", "django.contrib.staticfiles", # Admin - # 'django.contrib.admin', - # 'django.contrib.admindocs', + 'django.contrib.admin', + 'django.contrib.admindocs', + # Debugging + "debug_toolbar", # django-rules based authorization 'rules.apps.AutodiscoverRulesConfig', # Our own apps diff --git a/tests/openedx_tagging/core/tagging/test_rules.py b/tests/openedx_tagging/core/tagging/test_rules.py index 577c594f3..415ae193a 100644 --- a/tests/openedx_tagging/core/tagging/test_rules.py +++ b/tests/openedx_tagging/core/tagging/test_rules.py @@ -90,7 +90,7 @@ def test_view_taxonomy_enabled(self, enabled): assert self.superuser.has_perm("oel_tagging.view_taxonomy", self.taxonomy) assert self.staff.has_perm("oel_tagging.view_taxonomy") assert self.staff.has_perm("oel_tagging.view_taxonomy", self.taxonomy) - assert not self.learner.has_perm("oel_tagging.view_taxonomy") + assert self.learner.has_perm("oel_tagging.view_taxonomy") assert ( self.learner.has_perm("oel_tagging.view_taxonomy", self.taxonomy) == enabled ) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py new file mode 100644 index 000000000..222763adf --- /dev/null +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -0,0 +1,365 @@ +""" +Tests tagging rest api views +""" + +import ddt +from django.contrib.auth import get_user_model +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from openedx_tagging.core.tagging.models import Taxonomy + +User = get_user_model() + + +def check_taxonomy( + data, + id, + name, + description=None, + enabled=True, + required=False, + allow_multiple=False, + allow_free_text=False, + system_defined=False, + visible_to_authors=True, +): + assert data["id"] == id + assert data["name"] == name + assert data["description"] == description + assert data["enabled"] == enabled + assert data["required"] == required + assert data["allow_multiple"] == allow_multiple + assert data["allow_free_text"] == allow_free_text + assert data["system_defined"] == system_defined + assert data["visible_to_authors"] == visible_to_authors + + +@ddt.ddt +class TestTaxonomyViewSet(APITestCase): + def setUp(self): + super().setUp() + + self.user = User.objects.create( + username="user", + email="user@example.com", + ) + + self.staff = User.objects.create( + username="staff", + email="staff@example.com", + is_staff=True, + ) + + @ddt.data( + (None, status.HTTP_200_OK, 3), + (1, status.HTTP_200_OK, 2), + (0, status.HTTP_200_OK, 1), + (True, status.HTTP_200_OK, 2), + (False, status.HTTP_200_OK, 1), + ("True", status.HTTP_200_OK, 2), + ("False", status.HTTP_200_OK, 1), + ("1", status.HTTP_200_OK, 2), + ("0", status.HTTP_200_OK, 1), + (2, status.HTTP_400_BAD_REQUEST, None), + ("invalid", status.HTTP_400_BAD_REQUEST, None), + ) + @ddt.unpack + def test_list_taxonomy_queryparams(self, enabled, expected_status, expected_count): + Taxonomy.objects.create(name="Taxonomy enabled 1", enabled=True).save() + Taxonomy.objects.create(name="Taxonomy enabled 2", enabled=True).save() + Taxonomy.objects.create(name="Taxonomy disabled", enabled=False).save() + + url = reverse("oel_tagging:taxonomy-list") + + self.client.force_authenticate(user=self.staff) + if enabled is not None: + response = self.client.get(url, {"enabled": enabled}) + else: + response = self.client.get(url) + assert response.status_code == expected_status + + # If we were able to list the taxonomies, check that we got the expected number back + if status.is_success(expected_status): + assert len(response.data) == expected_count + + @ddt.data( + (None, status.HTTP_403_FORBIDDEN), + ("user", status.HTTP_200_OK), + ("staff", status.HTTP_200_OK), + ) + @ddt.unpack + def test_list_taxonomy(self, user_attr, expected_status): + url = reverse("oel_tagging:taxonomy-list") + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.get(url) + assert response.status_code == expected_status + + @ddt.data( + (None, {"enabled": True}, status.HTTP_403_FORBIDDEN), + (None, {"enabled": False}, status.HTTP_403_FORBIDDEN), + ( + "user", + {"enabled": True}, + status.HTTP_200_OK, + ), + ("user", {"enabled": False}, status.HTTP_404_NOT_FOUND), + ("staff", {"enabled": True}, status.HTTP_200_OK), + ("staff", {"enabled": False}, status.HTTP_200_OK), + ) + @ddt.unpack + def test_detail_taxonomy(self, user_attr, taxonomy_data, expected_status): + create_data = {**{"name": "taxonomy detail test"}, **taxonomy_data} + taxonomy = Taxonomy.objects.create(**create_data) + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.get(url) + assert response.status_code == expected_status + + if status.is_success(expected_status): + check_taxonomy(response.data, taxonomy.pk, **create_data) + self.assertGreaterEqual(response.data.items(), create_data.items()) + + def test_detail_taxonomy_404(self): + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + + self.client.force_authenticate(user=self.staff) + response = self.client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @ddt.data( + (None, status.HTTP_403_FORBIDDEN), + ("user", status.HTTP_403_FORBIDDEN), + ("staff", status.HTTP_201_CREATED), + ) + @ddt.unpack + def test_create_taxonomy(self, user_attr, expected_status): + url = reverse("oel_tagging:taxonomy-list") + + create_data = { + "name": "taxonomy_data_2", + "description": "This is a description", + "enabled": False, + "required": True, + "allow_multiple": True, + } + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.post(url, create_data, format="json") + assert response.status_code == expected_status + + # If we were able to create the taxonomy, check if it was created + if status.is_success(expected_status): + check_taxonomy(response.data, response.data["id"], **create_data) + url = reverse( + "oel_tagging:taxonomy-detail", kwargs={"pk": response.data["id"]} + ) + response = self.client.get(url) + check_taxonomy(response.data, response.data["id"], **create_data) + + @ddt.data( + {}, + {"name": "Error taxonomy 2", "required": "Invalid value"}, + {"name": "Error taxonomy 3", "enabled": "Invalid value"}, + ) + def test_create_taxonomy_error(self, create_data): + url = reverse("oel_tagging:taxonomy-list") + + self.client.force_authenticate(user=self.staff) + response = self.client.post(url, create_data, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @ddt.data({"name": "System defined taxonomy", "system_defined": True}) + def test_create_taxonomy_system_defined(self, create_data): + """ + Cannont create a taxonomy with system_defined=true + """ + url = reverse("oel_tagging:taxonomy-list") + + 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"] == False + + @ddt.data( + (None, status.HTTP_403_FORBIDDEN), + ("user", status.HTTP_403_FORBIDDEN), + ("staff", status.HTTP_200_OK), + ) + @ddt.unpack + def test_update_taxonomy(self, user_attr, expected_status): + taxonomy = Taxonomy.objects.create( + name="test update taxonomy", + description="taxonomy description", + enabled=True, + required=False, + ) + taxonomy.save() + + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.put(url, {"name": "new name"}, format="json") + assert response.status_code == expected_status + + # If we were able to update the taxonomy, check if the name changed + if status.is_success(expected_status): + response = self.client.get(url) + check_taxonomy( + response.data, + response.data["id"], + **{ + "name": "new name", + "description": "taxonomy description", + "enabled": True, + "required": False, + }, + ) + + @ddt.data( + (False, False, status.HTTP_200_OK), + (False, True, status.HTTP_200_OK), + (True, False, status.HTTP_403_FORBIDDEN), + (True, True, status.HTTP_403_FORBIDDEN), + ) + @ddt.unpack + def test_update_taxonomy_system_defined( + self, create_value, update_value, expected_status + ): + taxonomy = Taxonomy.objects.create( + name="test system taxonomy", system_defined=create_value + ) + taxonomy.save() + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + + self.client.force_authenticate(user=self.staff) + response = self.client.put( + url, {"name": "new name", "system_defined": update_value}, format="json" + ) + assert response.status_code == expected_status + + if status.is_success(expected_status): + response = self.client.get(url) + assert response.data["system_defined"] == False + + def test_update_taxonomy_404(self): + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + + self.client.force_authenticate(user=self.staff) + response = self.client.put(url, {"name": "new name"}, format="json") + assert response.status_code == status.HTTP_404_NOT_FOUND + + @ddt.data( + (None, status.HTTP_403_FORBIDDEN), + ("user", status.HTTP_403_FORBIDDEN), + ("staff", status.HTTP_200_OK), + ) + @ddt.unpack + def test_patch_taxonomy(self, user_attr, expected_status): + taxonomy = Taxonomy.objects.create( + name="test patch taxonomy", enabled=False, required=True + ) + taxonomy.save() + + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.patch( + url, {"name": "new name", "required": False}, format="json" + ) + assert response.status_code == expected_status + + # If we were able to update the taxonomy, check if the name changed + if status.is_success(expected_status): + response = self.client.get(url) + check_taxonomy( + response.data, + response.data["id"], + **{ + "name": "new name", + "enabled": False, + "required": False, + }, + ) + + @ddt.data( + (False, False, status.HTTP_200_OK), + (False, True, status.HTTP_200_OK), + (True, False, status.HTTP_403_FORBIDDEN), + (True, True, status.HTTP_403_FORBIDDEN), + ) + @ddt.unpack + def test_patch_taxonomy_system_defined( + self, create_value, update_value, expected_status + ): + taxonomy = Taxonomy.objects.create( + name="test system taxonomy", system_defined=create_value + ) + taxonomy.save() + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + + self.client.force_authenticate(user=self.staff) + response = self.client.patch( + url, {"system_defined": update_value}, format="json" + ) + assert response.status_code == expected_status + + if status.is_success(expected_status): + response = self.client.get(url) + assert response.data["system_defined"] == False + + def test_patch_taxonomy_404(self): + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + + self.client.force_authenticate(user=self.staff) + response = self.client.patch(url, {"name": "new name"}, format="json") + assert response.status_code == status.HTTP_404_NOT_FOUND + + @ddt.data( + (None, status.HTTP_403_FORBIDDEN), + ("user", status.HTTP_403_FORBIDDEN), + ("staff", status.HTTP_204_NO_CONTENT), + ) + @ddt.unpack + def test_delete_taxonomy(self, user_attr, expected_status): + taxonomy = Taxonomy.objects.create(name="test delete taxonomy") + taxonomy.save() + + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.delete(url) + assert response.status_code == expected_status + + # If we were able to delete the taxonomy, check that it's really gone + if status.is_success(expected_status): + response = self.client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_delete_taxonomy_404(self): + url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + + self.client.force_authenticate(user=self.staff) + response = self.client.delete(url) + assert response.status_code, status.HTTP_404_NOT_FOUND diff --git a/tox.ini b/tox.ini index 4a0100cb2..15f4bb8bb 100644 --- a/tox.ini +++ b/tox.ini @@ -31,7 +31,7 @@ match-dir = (?!migrations) [pytest] DJANGO_SETTINGS_MODULE = test_settings -addopts = --cov openedx_learning --cov openedx_tagging --cov-report term-missing --cov-report xml +addopts = --cov openedx_learning --cov openedx_tagging --cov tests --cov-report term-missing --cov-report xml norecursedirs = .* docs requirements site-packages [testenv] From a7bc37af850cbdc56537d7c8455a050423049708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 25 Jul 2023 16:42:49 -0300 Subject: [PATCH 2/6] fix: removing unused import --- openedx_tagging/core/tagging/rest_api/v1/serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx_tagging/core/tagging/rest_api/v1/serializers.py b/openedx_tagging/core/tagging/rest_api/v1/serializers.py index 9b173c795..a46350a9a 100644 --- a/openedx_tagging/core/tagging/rest_api/v1/serializers.py +++ b/openedx_tagging/core/tagging/rest_api/v1/serializers.py @@ -2,7 +2,6 @@ API Serializers for taxonomies """ -from django.utils.module_loading import import_string from rest_framework import serializers from openedx_tagging.core.tagging.models import Taxonomy From 2ccae75d75227729a3beca0b010d0995e4430697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 27 Jul 2023 08:59:38 -0300 Subject: [PATCH 3/6] test: document system_defined behavior --- .../openedx_tagging/core/tagging/test_views.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index 222763adf..f6a4e6dfa 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -241,6 +241,9 @@ def test_update_taxonomy(self, user_attr, expected_status): def test_update_taxonomy_system_defined( self, create_value, update_value, expected_status ): + ''' + Test that we can't update system_defined field + ''' taxonomy = Taxonomy.objects.create( name="test system taxonomy", system_defined=create_value ) @@ -253,9 +256,9 @@ def test_update_taxonomy_system_defined( ) assert response.status_code == expected_status - if status.is_success(expected_status): - response = self.client.get(url) - assert response.data["system_defined"] == False + # Verify that system_defined has not changed + response = self.client.get(url) + assert response.data["system_defined"] == create_value def test_update_taxonomy_404(self): url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) @@ -310,6 +313,9 @@ def test_patch_taxonomy(self, user_attr, expected_status): def test_patch_taxonomy_system_defined( self, create_value, update_value, expected_status ): + ''' + Test that we can't patch system_defined field + ''' taxonomy = Taxonomy.objects.create( name="test system taxonomy", system_defined=create_value ) @@ -322,9 +328,9 @@ def test_patch_taxonomy_system_defined( ) assert response.status_code == expected_status - if status.is_success(expected_status): - response = self.client.get(url) - assert response.data["system_defined"] == False + # Verify that system_defined has not changed + response = self.client.get(url) + assert response.data["system_defined"] == create_value def test_patch_taxonomy_404(self): url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) From cec362d2cfc5342949e743cd6097c1ca750d8dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 27 Jul 2023 09:01:09 -0300 Subject: [PATCH 4/6] test: use hardcoded urls instead of reverse() --- .../core/tagging/test_views.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index f6a4e6dfa..cab08f896 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -12,6 +12,8 @@ User = get_user_model() +TAXONOMY_LIST_URL = '/tagging/rest_api/v1/taxonomies/' +TAXONOMY_DETAIL_URL = '/tagging/rest_api/v1/taxonomies/{pk}/' def check_taxonomy( data, @@ -71,7 +73,7 @@ def test_list_taxonomy_queryparams(self, enabled, expected_status, expected_coun Taxonomy.objects.create(name="Taxonomy enabled 2", enabled=True).save() Taxonomy.objects.create(name="Taxonomy disabled", enabled=False).save() - url = reverse("oel_tagging:taxonomy-list") + url = TAXONOMY_LIST_URL self.client.force_authenticate(user=self.staff) if enabled is not None: @@ -91,7 +93,7 @@ def test_list_taxonomy_queryparams(self, enabled, expected_status, expected_coun ) @ddt.unpack def test_list_taxonomy(self, user_attr, expected_status): - url = reverse("oel_tagging:taxonomy-list") + url = TAXONOMY_LIST_URL if user_attr: user = getattr(self, user_attr) @@ -116,7 +118,7 @@ def test_list_taxonomy(self, user_attr, expected_status): def test_detail_taxonomy(self, user_attr, taxonomy_data, expected_status): create_data = {**{"name": "taxonomy detail test"}, **taxonomy_data} taxonomy = Taxonomy.objects.create(**create_data) - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + url = TAXONOMY_DETAIL_URL.format(pk=taxonomy.pk) if user_attr: user = getattr(self, user_attr) @@ -130,7 +132,7 @@ def test_detail_taxonomy(self, user_attr, taxonomy_data, expected_status): self.assertGreaterEqual(response.data.items(), create_data.items()) def test_detail_taxonomy_404(self): - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + url = TAXONOMY_DETAIL_URL.format(pk=123123) self.client.force_authenticate(user=self.staff) response = self.client.get(url) @@ -143,7 +145,7 @@ def test_detail_taxonomy_404(self): ) @ddt.unpack def test_create_taxonomy(self, user_attr, expected_status): - url = reverse("oel_tagging:taxonomy-list") + url = TAXONOMY_LIST_URL create_data = { "name": "taxonomy_data_2", @@ -175,7 +177,7 @@ def test_create_taxonomy(self, user_attr, expected_status): {"name": "Error taxonomy 3", "enabled": "Invalid value"}, ) def test_create_taxonomy_error(self, create_data): - url = reverse("oel_tagging:taxonomy-list") + url = TAXONOMY_LIST_URL self.client.force_authenticate(user=self.staff) response = self.client.post(url, create_data, format="json") @@ -186,7 +188,7 @@ def test_create_taxonomy_system_defined(self, create_data): """ Cannont create a taxonomy with system_defined=true """ - url = reverse("oel_tagging:taxonomy-list") + url = TAXONOMY_LIST_URL self.client.force_authenticate(user=self.staff) response = self.client.post(url, create_data, format="json") @@ -208,7 +210,7 @@ def test_update_taxonomy(self, user_attr, expected_status): ) taxonomy.save() - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + url = TAXONOMY_DETAIL_URL.format(pk=taxonomy.pk) if user_attr: user = getattr(self, user_attr) @@ -248,7 +250,7 @@ def test_update_taxonomy_system_defined( name="test system taxonomy", system_defined=create_value ) taxonomy.save() - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + url = TAXONOMY_DETAIL_URL.format(pk=taxonomy.pk) self.client.force_authenticate(user=self.staff) response = self.client.put( @@ -261,7 +263,7 @@ def test_update_taxonomy_system_defined( assert response.data["system_defined"] == create_value def test_update_taxonomy_404(self): - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + url = TAXONOMY_DETAIL_URL.format(pk=123123) self.client.force_authenticate(user=self.staff) response = self.client.put(url, {"name": "new name"}, format="json") @@ -279,7 +281,7 @@ def test_patch_taxonomy(self, user_attr, expected_status): ) taxonomy.save() - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + url = TAXONOMY_DETAIL_URL.format(pk=taxonomy.pk) if user_attr: user = getattr(self, user_attr) @@ -320,7 +322,7 @@ def test_patch_taxonomy_system_defined( name="test system taxonomy", system_defined=create_value ) taxonomy.save() - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + url = TAXONOMY_DETAIL_URL.format(pk=taxonomy.pk) self.client.force_authenticate(user=self.staff) response = self.client.patch( @@ -333,7 +335,7 @@ def test_patch_taxonomy_system_defined( assert response.data["system_defined"] == create_value def test_patch_taxonomy_404(self): - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + url = TAXONOMY_DETAIL_URL.format(pk=123123) self.client.force_authenticate(user=self.staff) response = self.client.patch(url, {"name": "new name"}, format="json") @@ -349,7 +351,7 @@ def test_delete_taxonomy(self, user_attr, expected_status): taxonomy = Taxonomy.objects.create(name="test delete taxonomy") taxonomy.save() - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": taxonomy.pk}) + url = TAXONOMY_DETAIL_URL.format(pk=taxonomy.pk) if user_attr: user = getattr(self, user_attr) @@ -364,7 +366,7 @@ def test_delete_taxonomy(self, user_attr, expected_status): assert response.status_code == status.HTTP_404_NOT_FOUND def test_delete_taxonomy_404(self): - url = reverse("oel_tagging:taxonomy-detail", kwargs={"pk": 123123}) + url = TAXONOMY_DETAIL_URL.format(pk=123123) self.client.force_authenticate(user=self.staff) response = self.client.delete(url) From db71ede31da94935367ad166974f262a1f545497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 28 Jul 2023 16:16:13 -0300 Subject: [PATCH 5/6] test: removing duplicated check --- tests/openedx_tagging/core/tagging/test_views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index cab08f896..b3c40bbee 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -129,7 +129,6 @@ def test_detail_taxonomy(self, user_attr, taxonomy_data, expected_status): if status.is_success(expected_status): check_taxonomy(response.data, taxonomy.pk, **create_data) - self.assertGreaterEqual(response.data.items(), create_data.items()) def test_detail_taxonomy_404(self): url = TAXONOMY_DETAIL_URL.format(pk=123123) From 354815178e1a53348961fa50e8e6049baa8259cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 28 Jul 2023 16:43:57 -0300 Subject: [PATCH 6/6] test: remove 'reverse' --- tests/openedx_tagging/core/tagging/test_views.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/openedx_tagging/core/tagging/test_views.py b/tests/openedx_tagging/core/tagging/test_views.py index b3c40bbee..0fd0d6840 100644 --- a/tests/openedx_tagging/core/tagging/test_views.py +++ b/tests/openedx_tagging/core/tagging/test_views.py @@ -4,7 +4,6 @@ import ddt from django.contrib.auth import get_user_model -from django.urls import reverse from rest_framework import status from rest_framework.test import APITestCase @@ -164,9 +163,8 @@ def test_create_taxonomy(self, user_attr, expected_status): # If we were able to create the taxonomy, check if it was created if status.is_success(expected_status): check_taxonomy(response.data, response.data["id"], **create_data) - url = reverse( - "oel_tagging:taxonomy-detail", kwargs={"pk": response.data["id"]} - ) + url = TAXONOMY_DETAIL_URL.format(pk=response.data["id"]) + response = self.client.get(url) check_taxonomy(response.data, response.data["id"], **create_data)