From 87f4d83df1d4181ff9c319408a6f214fef1548e9 Mon Sep 17 00:00:00 2001 From: Johan Castiblanco Date: Mon, 11 Sep 2023 16:13:33 -0500 Subject: [PATCH] feat: add certificates metrics style: isort and pylint pass chore: set tests checkpoint with cert backend style: pylint and isort chore: rename correct cert_status chore: add new dosctrings feat: add test for course certificates metric fx test: add test for view including total certs chore: rename `not_passing` refactor: pr recommendations for total count refactor: use GeneratedCertificates main fields PR recomendations: This ensure I create a django test model that only use the fields that test or the implementation use.\ --- .../test_backends/certificates_m_v1.py | 39 +++++++++ eox_nelp/migrations/0001_initial.py | 11 +++ eox_nelp/settings/test.py | 2 +- eox_nelp/stats/api/v1/views.py | 46 +++++++++- eox_nelp/stats/metrics.py | 57 ++++++++++++- eox_nelp/stats/tests/api/v1/tests_views.py | 57 ++++++++++++- eox_nelp/stats/tests/tests_metrics.py | 84 +++++++++++++++++++ 7 files changed, 290 insertions(+), 6 deletions(-) diff --git a/eox_nelp/edxapp_wrapper/test_backends/certificates_m_v1.py b/eox_nelp/edxapp_wrapper/test_backends/certificates_m_v1.py index 43885c0c..bccd1fec 100644 --- a/eox_nelp/edxapp_wrapper/test_backends/certificates_m_v1.py +++ b/eox_nelp/edxapp_wrapper/test_backends/certificates_m_v1.py @@ -1,5 +1,11 @@ """Test backend for certificates module.""" +from django.contrib.auth.models import User +from django.db import models from mock import Mock +from model_utils import Choices +from opaque_keys.edx.django.models import CourseKeyField + +from eox_nelp.edxapp_wrapper.test_backends import create_test_model def get_generated_certificates_admin(): @@ -8,3 +14,36 @@ def get_generated_certificates_admin(): Mock class. """ return Mock() + + +MODES = Choices( + 'verified', + 'honor', + 'audit', + 'professional', + 'no-id-professional', + 'masters', + 'executive-education', + 'paid-executive-education', + 'paid-bootcamp', +) + + +def get_generated_certificate(): + """Return test model. + Returns: + Generated Certificates dummy model. + """ + generated_certificate_fields = { + "user": models.ForeignKey(User, on_delete=models.CASCADE), + "course_id": CourseKeyField(max_length=255, blank=True, default=None), + "grade": models.CharField(max_length=5, blank=True, default=''), + "status": models.CharField(max_length=32, default='unavailable'), + "mode": models.CharField(max_length=32, choices=MODES, default=MODES.honor), + # not model fields : + "MODES": MODES, + } + + return create_test_model( + "GeneratedCertificate", "eox_nelp", __package__, generated_certificate_fields + ) diff --git a/eox_nelp/migrations/0001_initial.py b/eox_nelp/migrations/0001_initial.py index ee0aec94..c9639daa 100644 --- a/eox_nelp/migrations/0001_initial.py +++ b/eox_nelp/migrations/0001_initial.py @@ -95,6 +95,17 @@ class Migration(migrations.Migration): ('org', models.CharField(blank=True, max_length=500, null=True)) ], ), + migrations.CreateModel( + name='GeneratedCertificate', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL, on_delete=models.CASCADE)), + ('course_id', opaque_keys.edx.django.models.CourseKeyField(default=None, max_length=255, blank=True)), + ('grade', models.CharField(default='', max_length=5, blank=True)), + ('status', models.CharField(default='unavailable', max_length=32)), + ('mode', models.CharField(default='honor', max_length=32, choices=[('verified', 'verified'), ('honor', 'honor'), ('audit', 'audit'), ('professional', 'professional'), ('no-id-professional', 'no-id-professional')])), + ], + ), ] if getattr(settings, 'TESTING_MIGRATIONS', False): diff --git a/eox_nelp/settings/test.py b/eox_nelp/settings/test.py index c0f03a68..187e9b8b 100644 --- a/eox_nelp/settings/test.py +++ b/eox_nelp/settings/test.py @@ -99,7 +99,7 @@ def plugin_settings(settings): # pylint: disable=function-redefined EOX_CORE_COURSEWARE_BACKEND = "eox_nelp.edxapp_wrapper.test_backends.courseware_m_v1" EOX_CORE_GRADES_BACKEND = "eox_nelp.edxapp_wrapper.test_backends.grades_m_v1" -EOX_CORE_CERTIFICATES_BACKEND = "eox_core.edxapp_wrapper.backends.certificates_h_v1_test" +EOX_CORE_CERTIFICATES_BACKEND = "eox_nelp.edxapp_wrapper.test_backends.certificates_m_v1" GET_SITE_CONFIGURATION_MODULE = 'eox_tenant.edxapp_wrapper.backends.site_configuration_module_test_v1' GET_THEMING_HELPERS = 'eox_tenant.edxapp_wrapper.backends.theming_helpers_test_v1' diff --git a/eox_nelp/stats/api/v1/views.py b/eox_nelp/stats/api/v1/views.py index 650cee39..de240967 100644 --- a/eox_nelp/stats/api/v1/views.py +++ b/eox_nelp/stats/api/v1/views.py @@ -40,6 +40,10 @@ class GeneralTenantStatsView(APIView): "openassessment": 0, "problem": 49, "video": 0 + }, + "certiticates": { + "downloadable": 5, + "notpassing": 4 } } ``` @@ -50,18 +54,22 @@ def get(self, request): tenant = request.site.domain courses = metrics.get_courses_metrics(tenant) components = {} - + certificates = {} for metric in courses.get("metrics", []): course_components = metric.get("components", {}) + certificates_components = metric.get("certificates", {}).get("total", {}) for key, value in course_components.items(): components[key] = components.get(key, 0) + value + for key, value in certificates_components.items(): + certificates[key] = certificates.get(key, 0) + value return Response({ "learners": metrics.get_learners_metric(tenant), "courses": courses.get("total_courses", 0), "instructors": metrics.get_instructors_metric(tenant), - "components": components + "components": components, + "certificates": certificates, }) @@ -98,6 +106,22 @@ class GeneralCourseStatsView(APIView): "openassessment": 0, "problem": 49, "video": 0 + }, + "certificates" : { + "verified": {}, + "honor": {}, + "audit": {}, + "professional": {}, + "no-id-professional": { + "downloadable": 5, + "notpassing": 4, + }, + "masters": {}, + "executive-education": {}, + "total": { + "downloadable": 5, + "notpassing": 4 + } } }, ... @@ -124,6 +148,24 @@ class GeneralCourseStatsView(APIView): "openassessment": 0, "problem": 49, "video": 0 + }, + "certificates" : { + "verified": {...}, + "honor": {...}, + "audit": {...}, + "professional": {}, + "no-id-professional": { + "downloadable": 5, + "notpassing": 4... + }, + "masters": {...}, + "executive-education": {...}, + "paid-executive-education": {...}, + "paid-bootcamp": {...}, + "total": { + "downloadable": 5, + "notpassing": 4 + } } } ``` diff --git a/eox_nelp/stats/metrics.py b/eox_nelp/stats/metrics.py index abec5361..3a1d3907 100644 --- a/eox_nelp/stats/metrics.py +++ b/eox_nelp/stats/metrics.py @@ -6,8 +6,10 @@ get_learners_metric: Return number of learners, for the visible courses. get_instructors_metric: Return number of instructors, for the visible courses. get_courses_metrics: Return metrics for the visible courses. + get_course_certificates_metric: Return a dict metric representin the certificates of a course. """ from django.conf import settings +from eox_core.edxapp_wrapper.certificates import get_generated_certificate from eox_nelp.edxapp_wrapper.branding import get_visible_courses from eox_nelp.edxapp_wrapper.modulestore import modulestore @@ -15,6 +17,8 @@ from eox_nelp.edxapp_wrapper.student import CourseAccessRole, CourseEnrollment from eox_nelp.stats.decorators import cache_method +GeneratedCertificate = get_generated_certificate() + @cache_method def get_cached_courses(tenant): # pylint: disable=unused-argument @@ -70,6 +74,7 @@ def get_course_metrics(course_key): user__is_staff=False, user__is_superuser=False ).values('user').distinct().count() + certificates = get_course_certificates_metric(course_key) return { "id": str(course_key), @@ -79,7 +84,8 @@ def get_course_metrics(course_key): "sections": len(chapters), "sub_sections": len(sequentials), "units": len(verticals), - "components": components + "components": components, + "certificates": certificates, } @@ -134,3 +140,52 @@ def get_courses_metrics(tenant): metrics = [get_course_metrics(course.id) for course in courses] return {"total_courses": courses.count(), "metrics": metrics} + + +def get_course_certificates_metric(course_key): + """ + Returns the total of certificates in a course. + + Args: + course_key: Course identifier. + + Return: + : Contains certificates of course metric. + { + "verified": {...}, + "honor": {...}, + "audit": {}, + "professional": {...}, + "no-id-professional": { + "downloadable": 5, + "notpassing": 4, + }, + "masters": {...}, + "executive-education": {...} + "paid-executive-education":{...}, + "paid-bootcamp": {...}, + "total": 0 + } + """ + certificates = {} + course_certificates_qs = GeneratedCertificate.objects.filter( + course_id=course_key, + ) + cert_statuses = [cert["status"] for cert in course_certificates_qs.values("status").distinct()] + + for mode in GeneratedCertificate.MODES: + db_mode = mode[0] + human_mode = mode[1] + certificates[human_mode] = { + cert_status: course_certificates_qs.filter( + mode=db_mode, + status=cert_status, + ).values("user").distinct().count() + for cert_status in cert_statuses + } + certificates["total"] = { + cert_status: course_certificates_qs.filter(status=cert_status).values("user").distinct().count() + for cert_status in cert_statuses + } + + return certificates diff --git a/eox_nelp/stats/tests/api/v1/tests_views.py b/eox_nelp/stats/tests/api/v1/tests_views.py index 718fc1f5..6b2f0262 100644 --- a/eox_nelp/stats/tests/api/v1/tests_views.py +++ b/eox_nelp/stats/tests/api/v1/tests_views.py @@ -47,7 +47,9 @@ def test_default(self, mock_metrics): response = self.client.get(url_endpoint) self.assertEqual(status.HTTP_200_OK, response.status_code) - self.assertTrue(["learners", "courses", "instructors", "components"] == list(response.data.keys())) + self.assertTrue( + ["learners", "courses", "instructors", "components", "certificates"] == list(response.data.keys()) + ) @override_settings( MIDDLEWARE=["eox_tenant.middleware.CurrentSiteMiddleware"], @@ -86,6 +88,56 @@ def test_total_components(self, mock_metrics): self.assertEqual(expected_components, response.data["components"]) mock_metrics.get_courses_metrics.assert_called_once_with("testserver") + @override_settings( + MIDDLEWARE=["eox_tenant.middleware.CurrentSiteMiddleware"], + ) + @patch("eox_nelp.stats.api.v1.views.metrics") + def test_total_certificates(self, mock_metrics): + """ + Test that the view will calculate the total of certificates based on the metrics values + + Expected behavior: + - Status code 200. + - Components total values are the expected. + - get_courses_metrics is called once. + """ + total_courses = 4 + fake_metric = { + "certificates": { + "verified": {"downloadable": 0, "notpassing": 0}, + "honor": {"downloadable": 0, "notpassing": 0}, + "audit": {"downloadable": 0, "notpassing": 0}, + "professional": {"downloadable": 0, "notpassing": 0}, + "no-id-professional": {"downloadable": 5, "notpassing": 4}, + "masters": {"downloadable": 10, "notpassing": 0}, + "executive-education": {"downloadable": 0, "notpassing": 1}, + "paid-executive-education": {"downloadable": 0, "notpassing": 0}, + "paid-bootcamp": {"downloadable": 0, "notpassing": 0}, + "total": { + "downloadable": 15, + "notpassing": 5, + } + }, + } + + mock_metrics.get_learners_metric.return_value = 5 + mock_metrics.get_instructors_metric.return_value = 4875 + mock_metrics.get_courses_metrics.return_value = { + "total_courses": total_courses, + "metrics": [fake_metric for c in range(total_courses)], + } + expected_certificates = { + "downloadable": fake_metric["certificates"]["total"]["downloadable"] * total_courses, + "notpassing": fake_metric["certificates"]["total"]["notpassing"] * total_courses, + } + url_endpoint = reverse("stats-api:v1:general-stats") + + response = self.client.get(url_endpoint) + + self.assertEqual(status.HTTP_200_OK, response.status_code) + self.assertEqual(expected_certificates, response.data["certificates"]) + mock_metrics.get_courses_metrics.assert_called_once_with("testserver") + @override_settings(MIDDLEWARE=["eox_tenant.middleware.CurrentSiteMiddleware"]) @data("post", "put", "patch", "delete") def test_invalid_method(self, method): @@ -174,7 +226,8 @@ def test_get_detail(self, mock_metrics): "openassessment": 0, "problem": 49, "video": 0 - } + }, + "certificates": 12 } url_endpoint = reverse("stats-api:v1:course-stats", args=[course_id]) diff --git a/eox_nelp/stats/tests/tests_metrics.py b/eox_nelp/stats/tests/tests_metrics.py index 0ad65a9f..e4e5b27d 100644 --- a/eox_nelp/stats/tests/tests_metrics.py +++ b/eox_nelp/stats/tests/tests_metrics.py @@ -9,8 +9,10 @@ """ import unittest +from django.contrib.auth import get_user_model from django.core.cache import cache from django.test import override_settings +from eox_core.edxapp_wrapper.certificates import get_generated_certificate from mock import MagicMock, Mock, patch from opaque_keys.edx.keys import CourseKey @@ -27,6 +29,9 @@ ) from eox_nelp.tests.utils import generate_list_mock_data +User = get_user_model() +GeneratedCertificate = get_generated_certificate() + class TestGetCachedCourses(unittest.TestCase): """Tests cases for get_cached_courses function.""" @@ -261,6 +266,33 @@ def setUp(self): # pylint: disable=invalid-name distinct_result = values_result.distinct.return_value distinct_result.count.return_value = self.expected_returned_roles + # this block use the GeneratedCertificates django test model defined + user, _ = User.objects.get_or_create(username="vader") + user2, _ = User.objects.get_or_create(username="vader2") + GeneratedCertificate.objects.get_or_create(**{ + 'user': user, + 'course_id': CourseKey.from_string("course-v1:test+Cx105+2022_T4"), + 'grade': '71.0', + 'status': 'downloadable', + 'mode': 'no-id-professional', + + }) + GeneratedCertificate.objects.get_or_create(**{ + 'user': user2, + 'course_id': CourseKey.from_string("course-v1:test+Cx105+2022_T4"), + 'grade': '59.0', + 'status': 'notpassing', + 'mode': 'honor', + + }) + GeneratedCertificate.objects.get_or_create(**{ + 'user': user, + 'course_id': CourseKey.from_string("course-v1:test2+Cx105+2022_T4"), + 'grade': '90.0', + 'status': 'downloadable', + 'mode': 'honor', + }) + def tearDown(self): """Clean cache and restarts mocks""" # This line just verifies that de get_course modulestore method cwas called with the right parameter @@ -353,6 +385,58 @@ def test_set_empty_allowed_components(self): self.assertFalse(course["components"]) + def test_right_certificates_metric(self): + """Based on the initial conditions, this check that the course metrics has the expected certificates value. + + Expected behavior: + - Certificates dict is the expected. + """ + cert_dict = { + "verified": { + "downloadable": 0, + "notpassing": 0 + }, + "honor": { + "downloadable": 0, + "notpassing": 1, + }, + "audit": { + "downloadable": 0, + "notpassing": 0 + }, + "professional": { + "downloadable": 0, + "notpassing": 0 + }, + "no-id-professional": { + "downloadable": 1, + "notpassing": 0 + }, + "masters": { + "downloadable": 0, + "notpassing": 0 + }, + "executive-education": { + "downloadable": 0, + "notpassing": 0 + }, + "paid-executive-education": { + "downloadable": 0, + "notpassing": 0 + }, + "paid-bootcamp": { + "downloadable": 0, + "notpassing": 0 + }, + "total": { + "downloadable": 1, + "notpassing": 1 + }, + } + course = get_course_metrics(self.course_key) + + self.assertDictEqual(course["certificates"], cert_dict) + @override_settings(STATS_SETTINGS={"API_XBLOCK_TYPES": ["html", "problem", "video"]}) def test_set_allowed_components(self): """This modifies the STATS_SETTINGS value in order to test that