diff --git a/eox_nelp/api_clients/certificates.py b/eox_nelp/api_clients/certificates.py index 79bff579..b4fd3148 100644 --- a/eox_nelp/api_clients/certificates.py +++ b/eox_nelp/api_clients/certificates.py @@ -59,7 +59,7 @@ def create_external_certificate_request(certificate_data): path = "Certificates" user = certificate_data["user"] payload = { - "reference_id": f"nelc-openedx-lms-cert-{certificate_data['id']}", + "reference_id": certificate_data["reference_id"], "date": { "issuance": certificate_data["created_at"], "expiration": None, diff --git a/eox_nelp/api_clients/tests/tests_certificates.py b/eox_nelp/api_clients/tests/tests_certificates.py index 585204b1..4d077887 100644 --- a/eox_nelp/api_clients/tests/tests_certificates.py +++ b/eox_nelp/api_clients/tests/tests_certificates.py @@ -11,6 +11,7 @@ from eox_nelp.api_clients.certificates import ExternalCertificatesApiClient from eox_nelp.api_clients.tests import TestBasicAuthApiClientMixin +from eox_nelp.signals.utils import generate_reference_id class TestExternalCertificatesApiClient(TestBasicAuthApiClientMixin, unittest.TestCase): @@ -33,12 +34,14 @@ def test_create_certificate(self, post_mock): } post_mock.return_value = expected_value user = { - "national_id": "10224587", + "national_id": "1222555888", "english_name": " Testing", "arabic_name": "اختبارات", } + course_id = "course-v1:FutureX+guide+2023" data = { "id": "124ABC", + "reference_id": generate_reference_id(user["national_id"], course_id), "created_at": timezone.now(), "expiration_date": timezone.now() + timezone.timedelta(days=365), "grade": 10, diff --git a/eox_nelp/signals/tests/test_utils.py b/eox_nelp/signals/tests/test_utils.py index b335b3c1..863a7a7a 100644 --- a/eox_nelp/signals/tests/test_utils.py +++ b/eox_nelp/signals/tests/test_utils.py @@ -5,6 +5,7 @@ """ import unittest +from ddt import data, ddt from django.conf import settings from django.contrib.auth import get_user_model from django.test import override_settings @@ -17,6 +18,9 @@ User = get_user_model() +WRONG_NATIONAL_IDS = [0, "", "324234", "VADER", "3666888999", "166688899", "فيدر"] +SAML_EXTRA_ASSOCIATIONS_LIST = ["1666888998ASDF", "1222666444a6ca", "12226664443242344334534543", "1222666444#@$%"] + class UserHasPassingGradeTestCase(unittest.TestCase): """Test class for function `_user_has_passing_grade`""" @@ -37,13 +41,14 @@ def test_call_user_has_passing_grade(self, course_grade_factory_mock): course_grade_factory_mock().read.assert_called_with(user, course_key=CourseKey.from_string(course_id)) +@ddt class GenerateExternalCertificateDataTestCase(unittest.TestCase): """Test class for function `_generate_external_certificate_data`""" def setUp(self): """ Set common conditions for test cases.""" self.user, _ = User.objects.get_or_create( - username="10024578", + username="1333666888", ) self.certificate_data = CertificateData( user=UserData( @@ -84,6 +89,7 @@ def test_generate_certificate_data(self, generate_certificate_mock, passing_mock expected_value = { "id": certificate.id, + 'reference_id': '1333666888~course-v1:test+Cx105+2022_T4', "created_at": str(time.date()), "expiration_date": None, "grade": self.certificate_data.grade * 100, @@ -120,9 +126,131 @@ def test_invalid_group_codes(self, generate_certificate_mock, passing_mock): certificate.id = 85 generate_certificate_mock.objects.get.return_value = certificate passing_mock.return_value = True - data = { + external_certificate_data = { "time": timezone.now(), "certificate_data": self.certificate_data, } - self.assertRaises(KeyError, _generate_external_certificate_data, **data) + self.assertRaises(KeyError, _generate_external_certificate_data, **external_certificate_data) + + @override_settings(EXTERNAL_CERTIFICATES_GROUP_CODES={"course-v1:test+Cx105+2022_T4": "ABC123"}) + @patch("eox_nelp.signals.utils._user_has_passing_grade") + @patch("eox_nelp.signals.utils.GeneratedCertificate") + @data(*WRONG_NATIONAL_IDS) + def test_invalid_mational_id(self, wrong_national_id, generate_certificate_mock, passing_mock): + """This tests when the user has an invalid NationalId. + + Expected behavior: + - Raise ValueError + """ + certificate = Mock() + certificate.id = 85 + generate_certificate_mock.objects.get.return_value = certificate + passing_mock.return_value = True + wrong_user, _ = User.objects.get_or_create( + username=wrong_national_id, + ) + certificate_data = CertificateData( + user=UserData( + pii=UserPersonalData( + username=wrong_user.username, + email="harry@potter.com", + name="Harry Potter", + ), + id=wrong_user.id, + is_active=True, + ), + course=CourseData( + course_key=CourseKey.from_string("course-v1:test+Cx105+2022_T4"), + ), + mode="audit", + grade=0.85, + current_status="non-passing", + download_url="", + name="", + ) + external_certificate_data = { + "time": timezone.now(), + "certificate_data": certificate_data, + } + + self.assertRaisesRegex( + ValueError, + f"The username or national_id: {wrong_user.username} doesnt match national ID regex", + _generate_external_certificate_data, + **external_certificate_data, + ) + + @override_settings(EXTERNAL_CERTIFICATES_GROUP_CODES={"course-v1:test+Cx105+2022_T4": "ABC123"}) + @patch("eox_nelp.signals.utils._user_has_passing_grade") + @patch("eox_nelp.signals.utils.GeneratedCertificate") + @data(*SAML_EXTRA_ASSOCIATIONS_LIST) + def test_generate_certificate_data_saml_extra_association( + self, + saml_extra_association, + generate_certificate_mock, + passing_mock + ): + """This tests the normal behavior ofa user with saml_extra_association + the method `_generate_external_certificate_data` + + Expected behavior: + - Result is as the expected value + - GeneratedCertificate mock is called with the right parameters. + - _user_has_passing_grade is called with the right parameters. + """ + time = timezone.now() + certificate = Mock() + certificate.id = 99 + generate_certificate_mock.objects.get.return_value = certificate + passing_mock.return_value = True + saml_association_user, _ = User.objects.get_or_create( + username=saml_extra_association, + ) + certificate_data = CertificateData( + user=UserData( + pii=UserPersonalData( + username=saml_association_user.username, + email="harry@potter.com", + name="Harry Potter", + ), + id=saml_association_user.id, + is_active=True, + ), + course=CourseData( + course_key=CourseKey.from_string("course-v1:test+Cx105+2022_T4"), + ), + mode="audit", + grade=0.88, + current_status="non-passing", + download_url="", + name="", + ) + national_id = saml_association_user.username[:10] + + expected_value = { + "id": certificate.id, + 'reference_id': f'{national_id}~course-v1:test+Cx105+2022_T4', + "created_at": str(time.date()), + "expiration_date": None, + "grade": certificate_data.grade * 100, + "is_passing": True, + "group_code": settings.EXTERNAL_CERTIFICATES_GROUP_CODES[str(self.certificate_data.course.course_key)], + "user": { + "national_id": national_id, + "english_name": certificate_data.user.pii.name, + "arabic_name": "", + } + } + + result = _generate_external_certificate_data(time, certificate_data) + + self.assertEqual(result, expected_value) + generate_certificate_mock.objects.get.assert_called_once_with( + user=saml_association_user, + course_id=certificate_data.course.course_key, + ) + passing_mock.assert_called_once_with( + saml_association_user, + str(certificate_data.course.course_key) + ) diff --git a/eox_nelp/signals/utils.py b/eox_nelp/signals/utils.py index 0d030912..6f115ec1 100644 --- a/eox_nelp/signals/utils.py +++ b/eox_nelp/signals/utils.py @@ -10,6 +10,8 @@ from eox_core.edxapp_wrapper.grades import get_course_grade_factory from opaque_keys.edx.keys import CourseKey +from eox_nelp.utils import is_valid_national_id + CourseGradeFactory = get_course_grade_factory() GeneratedCertificate = get_generated_certificate() User = get_user_model() @@ -39,16 +41,18 @@ def _generate_external_certificate_data(time, certificate_data): group_codes = getattr(settings, "EXTERNAL_CERTIFICATES_GROUP_CODES", {}) course_id = str(certificate_data.course.course_key) extra_info = getattr(user, "extrainfo", None) + national_id = user.username[:10] # saml association extra filter return { "id": certificate.id, + "reference_id": generate_reference_id(national_id, course_id), "created_at": str(time.date()), "expiration_date": None, "grade": float(certificate_data.grade) * 100, "is_passing": _user_has_passing_grade(user, course_id), "group_code": group_codes[course_id], "user": { - "national_id": user.username, + "national_id": national_id, "english_name": certificate_data.user.pii.name, "arabic_name": extra_info.arabic_name if extra_info else "", } @@ -67,3 +71,20 @@ def _user_has_passing_grade(user, course_id): course_grade = CourseGradeFactory().read(user, course_key=CourseKey.from_string(course_id)) return course_grade.passed + + +def generate_reference_id(national_id, course_id): + """Generate string of reference_id shape. + Args: + national_id: Username of the user(national_id). + course_id: Unique course identifier. + + Returns: + reference_id: The string of reference_id representation. + + Raise: + ValueError: National Id is not valid. + """ + is_valid_national_id(national_id, raise_exception=True) + + return f"{national_id}~{course_id}" diff --git a/eox_nelp/utils.py b/eox_nelp/utils.py index a0d12927..fca7e8c4 100644 --- a/eox_nelp/utils.py +++ b/eox_nelp/utils.py @@ -1,6 +1,9 @@ """Utils that can be used for the plugin project""" +import re from copy import copy +NATIONAL_ID_REGEX = r"^[1-2]\d{9}$" + def map_instance_attributes_to_dict(instance, attributes_mapping): """Create a dictionary that represents some fields or attributes of a instance based on @@ -38,3 +41,40 @@ def map_instance_attributes_to_dict(instance, attributes_mapping): instance_dict[extra_field] = extra_value return instance_dict + + +def check_regex(string, regex): + """Checks if the string matches the regex. + + Args: + string: The string to check. + regex: The regex to match against. + + Returns: + True if the string matches the regex, False otherwise. + """ + pattern = re.compile(regex) + + return pattern.match(string) is not None + + +def is_valid_national_id(national_id, raise_exception=False): + """Validate if a national_id has the shape of a national_id + + Args: + national_id: The string of national_id to check. + + Returns: + True if the national_id is ok, False otherwise. + + Raise: + ValueError: This will be raised when the username are excluded dont match national Id regex. + """ + check_national_id = check_regex(national_id, NATIONAL_ID_REGEX) + + if raise_exception and not check_national_id: + raise ValueError( + f"The username or national_id: {national_id} doesnt match national ID regex ({NATIONAL_ID_REGEX})", + ) + + return check_national_id