From 5001741566c920442eb7b90752c02360dbfbd044 Mon Sep 17 00:00:00 2001 From: Maria Grimaldi Date: Thu, 30 Sep 2021 12:07:35 -0400 Subject: [PATCH] feat: add support for certificates --- CHANGELOG.rst | 4 ++ docs/source/conf.py | 4 +- eox_tagging/api/v1/filters.py | 56 ++++++++++++++++++------- eox_tagging/api/v1/serializers.py | 25 ++++++----- eox_tagging/api/v1/test/test_filters.py | 36 ++++++++++++++++ eox_tagging/api/v1/viewset.py | 25 +++++++++-- eox_tagging/edxapp_accessors.py | 35 +++++++++++----- eox_tagging/models.py | 8 ++-- eox_tagging/settings/test.py | 1 + eox_tagging/test/test_models.py | 1 - eox_tagging/validators.py | 17 ++++++++ 11 files changed, 165 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 842c4a25..f9f2c6f8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,10 @@ Change Log Unreleased ---------- +Added +~~~~~ +* Support for tagging Certificate objects. + [2.2.0] - 2021-05-13 -------------------- diff --git a/docs/source/conf.py b/docs/source/conf.py index 460a7732..1fdbea53 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -14,9 +14,11 @@ # import os import sys + +import eox_tagging + sys.path.insert(0, os.path.abspath('../..')) oath = sys.path -import eox_tagging # -- Project information ----------------------------------------------------- diff --git a/eox_tagging/api/v1/filters.py b/eox_tagging/api/v1/filters.py index b9739a2b..3d477c12 100644 --- a/eox_tagging/api/v1/filters.py +++ b/eox_tagging/api/v1/filters.py @@ -7,7 +7,18 @@ from eox_tagging.constants import AccessLevel from eox_tagging.models import Tag -PROXY_MODEL_NAME = "opaquekeyproxymodel" +FILTER_TARGET_MAPPING = { + "course_id": { + "object": "courseoverview", + }, + "username": { + "object": "user", + }, + "certificate_verify_uuid": { + "object": "generatedcertificate", + "target_id": "verify_uuid", + } +} class TagFilter(filters.FilterSet): @@ -15,6 +26,7 @@ class TagFilter(filters.FilterSet): course_id = filters.CharFilter(method="filter_by_target_object") username = filters.CharFilter(method="filter_by_target_object") + certificate_verify_uuid = filters.CharFilter(method="filter_by_target_object") target_type = filters.CharFilter(method="filter_target_types") created_at = filters.DateTimeFromToRangeFilter() activation_date = filters.DateTimeFromToRangeFilter() @@ -28,20 +40,23 @@ class Meta: def filter_by_target_object(self, queryset, name, value): """Filter that returns the tags associated with target.""" - TARGET_TYPES = { - "course_id": "courseoverview", - "username": "user", - } if value: - DEFAULT = { - name: value, + + filter_target = FILTER_TARGET_MAPPING.get(name) + target_id_name = filter_target.get("target_id") + name = target_id_name if target_id_name else name + + filter_params = { + "target_type": filter_target.get("object"), + "target_id": { + name: value, + }, } + try: - filter_params = { - "target_type": TARGET_TYPES.get(name), - "target_id": DEFAULT, - } - queryset = queryset.find_all_tags_for(**filter_params) + queryset = queryset.find_all_tags_for( + **filter_params + ) except Exception: # pylint: disable=broad-except return queryset.none() @@ -51,22 +66,33 @@ def filter_target_types(self, queryset, name, value): # pylint: disable=unused- """ Filter that returns targets using their type. - **SPECIAL CASE**: course enrollments. + **SPECIAL CASE**: course enrollments/generated certificate. - If the user wants to filter by target_type courseenrollment and wants to add filters on + If the user wants to filter by target_type courseenrollment/generatedcertificate and wants to add filters on user or course, it must pass the following: - target_type: if the other arguments are passed this is used to differentiate between course_id from courseoverview and username from user object. + Case Course Enrollment - enrollment_course_id (optional) - enrollment_username (optional) + Case Generated Certificate + - certificate_course_id (optional) + - certificate_username (optional) """ target_id = {} + username = None + course_id = None if value == "courseenrollment": course_id = self.request.query_params.get("enrollment_course_id") username = self.request.query_params.get("enrollment_username") - target_id.update({"username": username, "course_id": course_id}) + + elif value == "generatedcertificate": + course_id = self.request.query_params.get("certificate_course_id") + username = self.request.query_params.get("certificate_username") + + target_id.update({"username": username, "course_id": course_id}) try: if any(object_id for object_id in target_id.values()): diff --git a/eox_tagging/api/v1/serializers.py b/eox_tagging/api/v1/serializers.py index a13723c6..d9c1fd28 100644 --- a/eox_tagging/api/v1/serializers.py +++ b/eox_tagging/api/v1/serializers.py @@ -1,20 +1,18 @@ """ Serializers for tags and related objects. """ -import re - from django.core.exceptions import ValidationError from django.utils.translation import ugettext as _ from rest_framework import serializers from eox_tagging.api.v1 import fields from eox_tagging.constants import AccessLevel, Status -from eox_tagging.edxapp_accessors import get_object, get_site +from eox_tagging.edxapp_accessors import get_object_from_edxapp, get_site from eox_tagging.models import Tag -PROXY_MODEL_NAME = "opaquekeyproxymodel" MODELS_WITH_COMPOUND_KEYS = { - "courseenrollment": ["username", "course_id"], # Compound keys + "courseenrollment": ["username", "course_id"], + "generatedcertificate": ["username", "course_id"], } @@ -46,11 +44,8 @@ def get_meta(self, instance): "inactivated_at": instance.inactivated_at, } - # Validation and creation of tags def create(self, validated_data): """Function that creates a Tag instance.""" - - # Finding target and owner objects target_object = None owner_object = None target_type = validated_data.pop("target_object_type") @@ -58,14 +53,14 @@ def create(self, validated_data): target = validated_data.pop("target_object", None) if target_type and target_type.lower() in MODELS_WITH_COMPOUND_KEYS: - data = self.__convert_compound_keys(target, target_type) + data = self._convert_compound_keys(target, target_type.lower()) else: data = { "target_id": target, } try: - target_object = get_object(target_type, **data) + target_object = get_object_from_edxapp(target_type, **data) except Exception: raise serializers.ValidationError({"Target": _("Error getting {} object." .format(target_type))}) @@ -75,7 +70,6 @@ def create(self, validated_data): else: owner_object = get_site() - # Set objects tag_object = { "target_object": target_object, "owner_object": owner_object, @@ -87,11 +81,16 @@ def create(self, validated_data): except ValidationError as e: raise serializers.ValidationError({"Tag": _("{}".format(e.message))}) - def __convert_compound_keys(self, ids, object_type): + @staticmethod + def _convert_compound_keys(ids, object_type): """ Function that converts strings with format: `key1: key2` into a dictionary. """ - target_id = re.split(r':\s', ids) + target_id = ids.replace(" ", "").split(":", 1) + if len(target_id) == 1: + return { + "target_id": ids, + } target_labels = MODELS_WITH_COMPOUND_KEYS.get(object_type) target_pairs = zip(target_labels, target_id) return dict(target_pairs) diff --git a/eox_tagging/api/v1/test/test_filters.py b/eox_tagging/api/v1/test/test_filters.py index 369671c3..fc62ee81 100644 --- a/eox_tagging/api/v1/test/test_filters.py +++ b/eox_tagging/api/v1/test/test_filters.py @@ -29,6 +29,20 @@ def test_filter_by_target_object(self, objects_mock): target_id={name: value}, ) + @patch.object(Tag, 'objects') + def test_filter_target_certificate_by_uuid(self, objects_mock): + """Used to test filtering tags by target object.""" + objects_mock.find_all_tags_for = Mock() + value = "abdcefg12345" + name = "certificate_verify_uuid" + + self.filterset.filter_by_target_object(objects_mock, name, value) + + objects_mock.find_all_tags_for.assert_called_with( + target_type="generatedcertificate", + target_id={"verify_uuid": value}, + ) + @patch.object(Tag, 'objects') def test_filter_target_enrollment(self, objects_mock): """ @@ -51,6 +65,28 @@ def test_filter_target_enrollment(self, objects_mock): target_id={"username": "username", "course_id": "course_id"}, ) + @patch.object(Tag, 'objects') + def test_filter_target_certificate(self, objects_mock): + """ + Used to test filtering tags depending on the target type. This also filters generatedcertificate + given that we must specify the target type besides the other filter parameters, in this case + course_id and username. + """ + objects_mock.find_all_tags_for = Mock() + value = "generatedcertificate" + name = "target_type" + self.filterset.request.query_params = { + "certificate_course_id": "course_id", + "certificate_username": "username", + } + + self.filterset.filter_target_types(objects_mock, name, value) + + objects_mock.find_all_tags_for.assert_called_with( + target_type=value, + target_id={"username": "username", "course_id": "course_id"}, + ) + @patch.object(Tag, 'objects') def test_filter_target_enroll_by_user(self, objects_mock): """ diff --git a/eox_tagging/api/v1/viewset.py b/eox_tagging/api/v1/viewset.py index 9364c54d..6efe0f72 100644 --- a/eox_tagging/api/v1/viewset.py +++ b/eox_tagging/api/v1/viewset.py @@ -47,12 +47,12 @@ site configuration it can take any string. - `target_type` (**required**, string, _body_): - One of courseoverview, user, courseenrollment + One of courseoverview, user, courseenrollment, generatedcertificate - `target_id` (**required**, string, _body_): Identifier of the target\ object. For users, username; for courseoverview, course_id and for\ - courseenrollments a string with the following format: "`username:\ - course_id`" + courseenrollments/generatedcertificates a string with the following format: "`username:\ + course_id`. For generatedcertificates it can also be its verify_uuid" - `activation_date` (**optional**, string, _body_): DateTime format `YYYY-MM-DD HH:MM:SS`. @@ -123,6 +123,11 @@ str, "Shortcut to filter objects of target_type `user` with id `username`.", ), + query_parameter( + "certificate_verify_uuid", + str, + "Shortcut to filter objects of target_type `generatedcertificate` with id `verify_uuid`.", + ), query_parameter( "owner_type", str, @@ -132,7 +137,7 @@ "target_type", str, "The type of the object that was tagged, one of: `course (use opaquekeyproxymodel for`" - "course overview types), `courseenrollment`, `user`", + "course overview types), `courseenrollment`, `user`, `generatedcertificate`", ), query_parameter( "enrollment_username", @@ -146,6 +151,18 @@ "Course identifier to be used when target_type=courseenrollment." "Can be omitted and is ignored for a different target_type", ), + query_parameter( + "certificate_username", + str, + "User identifier (username) to be used when target_type=generatedcertificate. " + "Can be omitted and is ignored for a different target_type", + ), + query_parameter( + "certificate_course_id", + str, + "Course identifier to be used when target_type=generatedcertificate." + "Can be omitted and is ignored for a different target_type", + ), query_parameter( "created_at_before", str, diff --git a/eox_tagging/edxapp_accessors.py b/eox_tagging/edxapp_accessors.py index 22208bdf..f06180a4 100644 --- a/eox_tagging/edxapp_accessors.py +++ b/eox_tagging/edxapp_accessors.py @@ -3,13 +3,15 @@ from django.conf import settings from django.contrib.auth.models import User from django.contrib.sites.models import Site -from django.core.exceptions import ValidationError +from eox_core.edxapp_wrapper.certificates import get_generated_certificate from eox_core.edxapp_wrapper.users import get_edxapp_user from opaque_keys.edx.keys import CourseKey from eox_tagging.edxapp_wrappers.course_overview import CourseOverview from eox_tagging.edxapp_wrappers.enrollments import CourseEnrollment +GeneratedCertificate = get_generated_certificate() + def get_user(**kwargs): """Function used to get users.""" @@ -55,17 +57,30 @@ def get_course_enrollment(**kwargs): return CourseEnrollment.objects.get(user_id=user.id, course_id=course_id) -def get_object(related_object_type, **kwargs): - """Helper function to get objects using RELATED_FIELDS dictionary.""" - RELATED_OBJECTS = { +def get_certificate(**kwargs): + """ + Get GeneratedCertificate for specified id, key download URL or the course_id + and username associated with it. + """ + target_id = {} + verify_uuid = kwargs.get("target_id") + if verify_uuid: + target_id["verify_uuid"] = kwargs.get("target_id") + else: + target_id["user__username"] = kwargs.get("username") + target_id["course_id"] = CourseKey.from_string(kwargs.get("course_id")) + + return GeneratedCertificate.objects.get(**target_id) + + +def get_object_from_edxapp(object_type, **kwargs): + """Helper function to get objects from edx-platfrom given its identifiers.""" + related_objects = { "user": get_user, "courseoverview": get_course, "site": get_site, "courseenrollment": get_course_enrollment, + "generatedcertificate": get_certificate, } - try: - related_object = RELATED_OBJECTS.get(related_object_type.lower())(**kwargs) - except Exception: - raise ValidationError("This field is required.") - - return related_object + related_object = related_objects.get(object_type.lower()) + return related_object(**kwargs) diff --git a/eox_tagging/models.py b/eox_tagging/models.py index 36ec7218..70f6bab3 100644 --- a/eox_tagging/models.py +++ b/eox_tagging/models.py @@ -28,8 +28,6 @@ PROXY_MODEL_NAME = "opaquekeyproxymodel" -COURSE_ENROLLMENT_MODEL_NAME = "courseenrollment" - class TagQuerySet(QuerySet): """ Tag queryset used as manager.""" @@ -102,10 +100,11 @@ def _get_object_for_this_type(self, object_type, object_id): "opaque_key": CourseKey.from_string(object_id), } - if object_type.lower() == COURSE_ENROLLMENT_MODEL_NAME: + if object_type.lower() in ["courseenrollment", "generatedcertificate"]: course_id = object_id.get("course_id") username = object_id.get("username") + verify_uuid = object_id.get("verify_uuid") object_id = {} if course_id: @@ -114,6 +113,9 @@ def _get_object_for_this_type(self, object_type, object_id): if username: object_id["user__username"] = username + if verify_uuid: + object_id["verify_uuid"] = verify_uuid + object_instances = ctype.get_all_objects_for_this_type(**object_id) return object_instances, ctype diff --git a/eox_tagging/settings/test.py b/eox_tagging/settings/test.py index 883b92ca..48b52049 100644 --- a/eox_tagging/settings/test.py +++ b/eox_tagging/settings/test.py @@ -54,6 +54,7 @@ def plugin_settings(settings): # pylint: disable=function-redefined settings.EOX_TAGGING_BEARER_AUTHENTICATION = 'eox_tagging.edxapp_wrappers.backends.bearer_authentication_i_v1_test' settings.DATA_API_DEF_PAGE_SIZE = 1000 settings.DATA_API_MAX_PAGE_SIZE = 5000 + settings.EOX_CORE_CERTIFICATES_BACKEND = "eox_core.edxapp_wrapper.backends.certificates_h_v1_test" settings.TEST_SITE = 1 diff --git a/eox_tagging/test/test_models.py b/eox_tagging/test/test_models.py index 240de4f9..2f554d78 100644 --- a/eox_tagging/test/test_models.py +++ b/eox_tagging/test/test_models.py @@ -534,7 +534,6 @@ def test_create_tag_with_course(self, opaque_objects_mock): self.tagQueryset.create.called_once_with(target_object=opaque_mock) - # pylint: disable=protected-access @patch.object(TagQuerySet, '_get_object_for_this_type') def test_find_all_tags_for_course(self, _get_object_for_this_type): """Test getting tags with course as target.""" diff --git a/eox_tagging/validators.py b/eox_tagging/validators.py index e6c37486..3fba11c6 100644 --- a/eox_tagging/validators.py +++ b/eox_tagging/validators.py @@ -10,6 +10,7 @@ from django.conf import settings from django.contrib.sites.models import Site from django.core.exceptions import ObjectDoesNotExist, ValidationError +from eox_core.edxapp_wrapper.certificates import get_generated_certificate from eox_core.edxapp_wrapper.enrollments import get_enrollment from eox_core.edxapp_wrapper.users import get_edxapp_user from opaque_keys import InvalidKeyError # pylint: disable=ungrouped-imports, useless-suppression @@ -19,6 +20,7 @@ log = logging.getLogger(__name__) DATETIME_FORMAT_VALIDATION = "%Y-%m-%d %H:%M:%S" +GeneratedCertificate = get_generated_certificate() class TagValidators: @@ -36,6 +38,7 @@ def __init__(self, instance): 'OpaqueKeyProxyModel': self.__validate_proxy_model, 'CourseEnrollment': self.__validate_enrollment_integrity, 'Site': self.__validate_site_integrity, + 'GeneratedCertificate': self.__validate_certificate_integrity, } self.__configuration_types = { "in": list, @@ -251,6 +254,20 @@ def __validate_site_integrity(self, object_name): except ObjectDoesNotExist: raise ValidationError("EOX_TAGGING | Site '{}' does not exist".format(site_id)) + def __validate_certificate_integrity(self, object_name): + """ + Function that validates existence of the certificate object. + + Arguments: + - object_name: name of the object to validate. + """ + certificate_id = self.instance.get_attribute(object_name).id + + try: + GeneratedCertificate.objects.get(id=certificate_id) + except ObjectDoesNotExist: + raise ValidationError("EOX_TAGGING | Certificate '{}' does not exist".format(certificate_id)) + # Other validations def validate_no_updating(self): """Function that validates that the save is not an update."""