From 757b0ca06df5dc00ecf1b8a494e5e9b851a72fbe Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Thu, 26 Sep 2024 10:20:16 +0100 Subject: [PATCH 1/5] feat: add cohort certificate overrides filter Add an Open edX Filter that configure cohort certificate overrides filter. fccn/nau-technical#257 --- .../certificates/context_overrides.py | 59 +++++++ .../edxapp_wrapper/backends/cohort_v1.py | 13 ++ .../backends/cohort_v1_tests.py | 10 ++ .../edxapp_wrapper/cohort.py | 16 ++ nau_openedx_extensions/settings/common.py | 3 + nau_openedx_extensions/settings/test.py | 4 + .../test_certificates_context_overrides.py | 151 ++++++++++++++++++ 7 files changed, 256 insertions(+) create mode 100644 nau_openedx_extensions/certificates/context_overrides.py create mode 100644 nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py create mode 100644 nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py create mode 100644 nau_openedx_extensions/edxapp_wrapper/cohort.py create mode 100644 nau_openedx_extensions/tests/test_certificates_context_overrides.py diff --git a/nau_openedx_extensions/certificates/context_overrides.py b/nau_openedx_extensions/certificates/context_overrides.py new file mode 100644 index 0000000..dd8c5fd --- /dev/null +++ b/nau_openedx_extensions/certificates/context_overrides.py @@ -0,0 +1,59 @@ +""" +This file defines overrides of the context render of course certificates using an Open edX Filters pipeline step. +""" +import logging + +from openedx_filters import PipelineStep + +from nau_openedx_extensions.edxapp_wrapper.cohort import get_cohort + +log = logging.getLogger(__name__) + +class CertificatesContextCohortOverride(PipelineStep): + """ + Override the certificates render template context with information from the student cohort. + If user has a cohort and that cohort has custom certificate overrides, then override the root context variables + with the cohorted ones. + + Example usage: + Add the following configurations to your configuration file: + "OPEN_EDX_FILTERS_CONFIG": { + "org.openedx.learning.certificate.render.started.v1": { + "fail_silently": false, + "pipeline": [ + "nau_openedx_extensions.certificates.context_overrides.CertificatesContextCohortOverride" + ] + } + } + + Configure course on field "Certificate Web/HTML View Overrides" with: + { + "footer_additional_logo": "https://lms.example.com/some_logo.png", + "cohort_overrides": { + "test": { + "footer_additional_logo": "https://lms.example.com/override_logo.png" + } + } + } + """ + + def run_filter(self, context, custom_template): # pylint: disable=unused-argument, arguments-differ + """ + The filter logic. + """ + username = context['username'] + course_key = context['course_id'] + if 'cohort_overrides' in context: + cohort = get_cohort(username, course_key) + if cohort: + if cohort.name in context['cohort_overrides']: + cohort_override_dict = context['cohort_overrides'][cohort.name] + context.update(cohort_override_dict) + else: + log.info("The user '%s' enrollment on course '%s' doesn't have a cohort certificate context overrides configured for the cohort '%s'.", username, course_key, cohort.name) + else: + log.info("User '%s' not in a cohort on course '%s'", username, course_key) + else: + log.info("No Certificates context cohort_overrides defined on course '%s'", course_key) + + return {"context": context, "custom_template": custom_template} diff --git a/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py new file mode 100644 index 0000000..8f74fd2 --- /dev/null +++ b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py @@ -0,0 +1,13 @@ +""" +Cohort abstraction backend +""" +from common.djangoapps.student.models import get_user_by_username_or_email +from openedx.core.djangoapps.course_groups.cohorts import get_cohort as edxapp_get_cohort # pylint: disable=import-error + + +def get_cohort(username, course_key): + """ + Get the Course Cohort for the User that belongs the username if available other case return None. + """ + user = get_user_by_username_or_email(username) + return edxapp_get_cohort(user, course_key, assign=False, use_cached=False) diff --git a/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py new file mode 100644 index 0000000..f68f356 --- /dev/null +++ b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py @@ -0,0 +1,10 @@ +""" +Cohort abstraction backend +""" + + +def get_cohort(username, course_key): + """ + For tests. + """ + return None diff --git a/nau_openedx_extensions/edxapp_wrapper/cohort.py b/nau_openedx_extensions/edxapp_wrapper/cohort.py new file mode 100644 index 0000000..5fba674 --- /dev/null +++ b/nau_openedx_extensions/edxapp_wrapper/cohort.py @@ -0,0 +1,16 @@ +""" CourseMetadata backend abstraction """ + +from importlib import import_module + +from django.conf import settings + + +def get_cohort(*args, **kwargs): + """ + Get the Course Cohort for the User that belongs the username if available other case return None. + """ + backend_module = settings.NAU_COHORT_MODULE + backend = import_module(backend_module) + + return backend.get_cohort(*args, **kwargs) + diff --git a/nau_openedx_extensions/settings/common.py b/nau_openedx_extensions/settings/common.py index 260fee4..7ad3126 100644 --- a/nau_openedx_extensions/settings/common.py +++ b/nau_openedx_extensions/settings/common.py @@ -81,6 +81,9 @@ def plugin_settings(settings): settings.NAU_STUDENT_MODULE = ( "nau_openedx_extensions.edxapp_wrapper.backends.student_l_v1" ) + settings.NAU_COHORT_MODULE = ( + "nau_openedx_extensions.edxapp_wrapper.backends.cohort_v1" + ) # Overwrite the default certificate name settings.CERT_NAME_SHORT = _("Certificate") diff --git a/nau_openedx_extensions/settings/test.py b/nau_openedx_extensions/settings/test.py index bc2145c..5be662d 100644 --- a/nau_openedx_extensions/settings/test.py +++ b/nau_openedx_extensions/settings/test.py @@ -46,3 +46,7 @@ class SettingsClass: NAU_STUDENT_MODULE = ( "nau_openedx_extensions.edxapp_wrapper.backends.student_l_v1_tests" ) + +NAU_COHORT_MODULE = ( + "nau_openedx_extensions.edxapp_wrapper.backends.cohort_v1_tests" +) diff --git a/nau_openedx_extensions/tests/test_certificates_context_overrides.py b/nau_openedx_extensions/tests/test_certificates_context_overrides.py new file mode 100644 index 0000000..2fa3259 --- /dev/null +++ b/nau_openedx_extensions/tests/test_certificates_context_overrides.py @@ -0,0 +1,151 @@ +""" +Tests for certificates context overrides. +""" +from unittest.mock import MagicMock, Mock, PropertyMock, patch + +from django.test import TestCase +from nau_openedx_extensions.certificates.context_overrides import CertificatesContextCohortOverride + + +class CertificatesContextOverridesTest(TestCase): + """ + Test certificates context overrides for cohorts. + The override configuration is managed on the /settings/advanced/ + on field name `cert_html_view_overrides`. + """ + + @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') + def test_certificates_context_overrides_no_cohort_and_no_override(self, get_cohort_mock): + """ + Check if no cohort and no override. + """ + get_cohort_mock.return_value = None + + context = { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + } + result = CertificatesContextCohortOverride.run_filter(None, context, "some_template") + get_cohort_mock.assert_not_called() + + self.assertDictEqual(result['context'], { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + }) + + @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') + def test_certificates_context_overrides_with_cohort_no_override(self, get_cohort_mock): + """ + Check that no override is applied when the learner has a cohort but no override is configured. + """ + mocked_cohort = MagicMock() + cohort_name_property = PropertyMock(return_value="SomeGroup") + type(mocked_cohort).name = cohort_name_property + get_cohort_mock.return_value = mocked_cohort + + context = { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + } + CertificatesContextCohortOverride.run_filter(None, context, "some_template") + get_cohort_mock.assert_not_called() + + @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') + def test_certificates_context_overrides_with_no_cohort_and_with_override(self, get_cohort_mock): + """ + Check that no override is applied when the learner hasn't a cohort and the course has an override. + """ + get_cohort_mock.return_value = None + + context = { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + "footer_additional_logo": "http://lms.example.com/base_logo.png", + "cohort_overrides" : { + "SomeGroup": { + "footer_additional_logo": "http://lms.example.com/override_logo.png", + }, + }, + } + result = CertificatesContextCohortOverride.run_filter(None, context, "some_template") + get_cohort_mock.assert_called_once_with("nau@example.com", "course-v1:Demo+DemoX+Demo_Course") + + self.assertDictEqual(result['context'], { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + "footer_additional_logo": "http://lms.example.com/base_logo.png", + "cohort_overrides" : { + "SomeGroup": { + "footer_additional_logo": "http://lms.example.com/override_logo.png", + }, + }, + }) + + + @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') + def test_certificates_context_overrides_with_cohort_and_override_dont_match(self, get_cohort_mock): + """ + Check that the override isn't being applied when the learner belongs to a cohort that isn't configured on the override. + """ + mocked_cohort = MagicMock() + cohort_name_property = PropertyMock(return_value="some_group_not_configured_on_override") + type(mocked_cohort).name = cohort_name_property + get_cohort_mock.return_value = mocked_cohort + + context = { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + "footer_additional_logo": "http://lms.example.com/base_logo.png", + "cohort_overrides" : { + "SomeGroup": { + "footer_additional_logo": "http://lms.example.com/override_logo.png", + }, + }, + } + result = CertificatesContextCohortOverride.run_filter(None, context, "some_template") + get_cohort_mock.assert_called_once_with("nau@example.com", "course-v1:Demo+DemoX+Demo_Course") + + self.assertDictEqual(result['context'], { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + "footer_additional_logo": "http://lms.example.com/base_logo.png", + "cohort_overrides" : { + "SomeGroup": { + "footer_additional_logo": "http://lms.example.com/override_logo.png", + }, + }, + }) + + @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') + def test_certificates_context_overrides_with_cohort_and_override(self, get_cohort_mock): + """ + Check that the override is being applied if the learner belongs to a cohort and that cohort is configured has override. + """ + mocked_cohort = MagicMock() + cohort_name_property = PropertyMock(return_value="SomeGroup") + type(mocked_cohort).name = cohort_name_property + get_cohort_mock.return_value = mocked_cohort + + context = { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + "footer_additional_logo": "http://lms.example.com/base_logo.png", + "cohort_overrides" : { + "SomeGroup": { + "footer_additional_logo": "http://lms.example.com/override_logo.png", + }, + }, + } + result = CertificatesContextCohortOverride.run_filter(None, context, "some_template") + get_cohort_mock.assert_called_once_with("nau@example.com", "course-v1:Demo+DemoX+Demo_Course") + + self.assertDictEqual(result['context'], { + "username": "nau@example.com", + "course_id": "course-v1:Demo+DemoX+Demo_Course", + "footer_additional_logo": "http://lms.example.com/override_logo.png", + "cohort_overrides" : { + "SomeGroup": { + "footer_additional_logo": "http://lms.example.com/override_logo.png", + }, + }, + }) From f75de40e02241b25c35c825f988018514f0625ea Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Thu, 26 Sep 2024 10:20:38 +0100 Subject: [PATCH 2/5] build: `make clean` no longer deletes venv --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a808b77..ccabf2f 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,8 @@ clean: ## delete most git-ignored files find . -name '*.pyc' -exec rm -f {} + find . -name '*.pyo' -exec rm -f {} + find . -name '*~' -exec rm -f {} + - rm -rf venv + + echo "cleaned" +# rm -rf venv + virtual_environment: ## create virtual environment test -d venv || virtualenv venv --python=python3 From 7ca74f00a5f26af35b02cfab1fecade12d22df23 Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Thu, 26 Sep 2024 10:21:05 +0100 Subject: [PATCH 3/5] build: print log messages during test run --- .../certificates/context_overrides.py | 31 +++++++++++++------ .../edxapp_wrapper/backends/cohort_v1.py | 5 +-- .../backends/cohort_v1_tests.py | 2 +- .../edxapp_wrapper/cohort.py | 1 - .../locale/en/LC_MESSAGES/django.po | 6 ++-- .../locale/pt_PT/LC_MESSAGES/django.po | 6 ++-- .../test_certificates_context_overrides.py | 24 +++++++------- pytest.ini | 6 ++++ 8 files changed, 51 insertions(+), 30 deletions(-) create mode 100644 pytest.ini diff --git a/nau_openedx_extensions/certificates/context_overrides.py b/nau_openedx_extensions/certificates/context_overrides.py index dd8c5fd..0af6410 100644 --- a/nau_openedx_extensions/certificates/context_overrides.py +++ b/nau_openedx_extensions/certificates/context_overrides.py @@ -1,6 +1,7 @@ """ This file defines overrides of the context render of course certificates using an Open edX Filters pipeline step. """ + import logging from openedx_filters import PipelineStep @@ -9,6 +10,7 @@ log = logging.getLogger(__name__) + class CertificatesContextCohortOverride(PipelineStep): """ Override the certificates render template context with information from the student cohort. @@ -37,23 +39,34 @@ class CertificatesContextCohortOverride(PipelineStep): } """ - def run_filter(self, context, custom_template): # pylint: disable=unused-argument, arguments-differ + def run_filter(self, context, custom_template): # pylint: disable=arguments-differ """ The filter logic. """ - username = context['username'] - course_key = context['course_id'] - if 'cohort_overrides' in context: + username = context["username"] + course_key = context["course_id"] + if "cohort_overrides" in context: cohort = get_cohort(username, course_key) if cohort: - if cohort.name in context['cohort_overrides']: - cohort_override_dict = context['cohort_overrides'][cohort.name] + if cohort.name in context["cohort_overrides"]: + cohort_override_dict = context["cohort_overrides"][cohort.name] context.update(cohort_override_dict) else: - log.info("The user '%s' enrollment on course '%s' doesn't have a cohort certificate context overrides configured for the cohort '%s'.", username, course_key, cohort.name) + log.info( + "The user '%s' enrollment on course '%s' doesn't have a cohort " + "certificate context overrides configured for the cohort '%s'.", + username, + course_key, + cohort.name, + ) else: - log.info("User '%s' not in a cohort on course '%s'", username, course_key) + log.info( + "User '%s' not in a cohort on course '%s'", username, course_key + ) else: - log.info("No Certificates context cohort_overrides defined on course '%s'", course_key) + log.info( + "No Certificates context cohort_overrides defined on course '%s'", + course_key, + ) return {"context": context, "custom_template": custom_template} diff --git a/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py index 8f74fd2..020761c 100644 --- a/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py +++ b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1.py @@ -1,8 +1,9 @@ """ Cohort abstraction backend """ -from common.djangoapps.student.models import get_user_by_username_or_email -from openedx.core.djangoapps.course_groups.cohorts import get_cohort as edxapp_get_cohort # pylint: disable=import-error +from common.djangoapps.student.models import get_user_by_username_or_email # pylint: disable=import-error +from openedx.core.djangoapps.course_groups.cohorts import \ + get_cohort as edxapp_get_cohort # pylint: disable=import-error def get_cohort(username, course_key): diff --git a/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py index f68f356..5562a9c 100644 --- a/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py +++ b/nau_openedx_extensions/edxapp_wrapper/backends/cohort_v1_tests.py @@ -3,7 +3,7 @@ """ -def get_cohort(username, course_key): +def get_cohort(username, course_key): # pylint: disable=unused-argument """ For tests. """ diff --git a/nau_openedx_extensions/edxapp_wrapper/cohort.py b/nau_openedx_extensions/edxapp_wrapper/cohort.py index 5fba674..c024e55 100644 --- a/nau_openedx_extensions/edxapp_wrapper/cohort.py +++ b/nau_openedx_extensions/edxapp_wrapper/cohort.py @@ -13,4 +13,3 @@ def get_cohort(*args, **kwargs): backend = import_module(backend_module) return backend.get_cohort(*args, **kwargs) - diff --git a/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po b/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po index 6d26231..6f4e142 100644 --- a/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po +++ b/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: equipa@nau.edu.pt\n" -"POT-Creation-Date: 2024-05-10 14:45+0100\n" +"POT-Creation-Date: 2024-09-26 11:25+0100\n" "PO-Revision-Date: 2021-02-15 15:56+0000\n" "Last-Translator: FULL NAME \n" "Language: en\n" @@ -133,11 +133,11 @@ msgid "" "out in order to obtain a certificate." msgstr "" -#: nau_openedx_extensions/settings/common.py:86 +#: nau_openedx_extensions/settings/common.py:89 msgid "Certificate" msgstr "" -#: nau_openedx_extensions/settings/common.py:87 +#: nau_openedx_extensions/settings/common.py:90 msgid "Certificate of Achievement" msgstr "" diff --git a/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po b/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po index 057c35e..67e2d3d 100644 --- a/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po +++ b/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: equipa@nau.edu.pt\n" -"POT-Creation-Date: 2024-05-10 14:45+0100\n" +"POT-Creation-Date: 2024-09-26 11:25+0100\n" "PO-Revision-Date: 2021-02-15 15:56+0000\n" "Last-Translator: Ivo Branco \n" "Language: pt_PT\n" @@ -142,11 +142,11 @@ msgstr "" "Este curso encontra-se arquivado e já não permite a realização de " "atividades para obtenção de certificado." -#: nau_openedx_extensions/settings/common.py:86 +#: nau_openedx_extensions/settings/common.py:89 msgid "Certificate" msgstr "Certificado" -#: nau_openedx_extensions/settings/common.py:87 +#: nau_openedx_extensions/settings/common.py:90 msgid "Certificate of Achievement" msgstr "Certificado de Conclusão" diff --git a/nau_openedx_extensions/tests/test_certificates_context_overrides.py b/nau_openedx_extensions/tests/test_certificates_context_overrides.py index 2fa3259..833be4d 100644 --- a/nau_openedx_extensions/tests/test_certificates_context_overrides.py +++ b/nau_openedx_extensions/tests/test_certificates_context_overrides.py @@ -1,9 +1,10 @@ """ Tests for certificates context overrides. """ -from unittest.mock import MagicMock, Mock, PropertyMock, patch +from unittest.mock import MagicMock, PropertyMock, patch from django.test import TestCase + from nau_openedx_extensions.certificates.context_overrides import CertificatesContextCohortOverride @@ -27,7 +28,7 @@ def test_certificates_context_overrides_no_cohort_and_no_override(self, get_coho } result = CertificatesContextCohortOverride.run_filter(None, context, "some_template") get_cohort_mock.assert_not_called() - + self.assertDictEqual(result['context'], { "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", @@ -61,7 +62,7 @@ def test_certificates_context_overrides_with_no_cohort_and_with_override(self, g "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", "footer_additional_logo": "http://lms.example.com/base_logo.png", - "cohort_overrides" : { + "cohort_overrides": { "SomeGroup": { "footer_additional_logo": "http://lms.example.com/override_logo.png", }, @@ -74,18 +75,18 @@ def test_certificates_context_overrides_with_no_cohort_and_with_override(self, g "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", "footer_additional_logo": "http://lms.example.com/base_logo.png", - "cohort_overrides" : { + "cohort_overrides": { "SomeGroup": { "footer_additional_logo": "http://lms.example.com/override_logo.png", }, }, }) - @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') def test_certificates_context_overrides_with_cohort_and_override_dont_match(self, get_cohort_mock): """ - Check that the override isn't being applied when the learner belongs to a cohort that isn't configured on the override. + Check that the override isn't being applied when the learner belongs to a cohort that isn't configured on the + override. """ mocked_cohort = MagicMock() cohort_name_property = PropertyMock(return_value="some_group_not_configured_on_override") @@ -96,7 +97,7 @@ def test_certificates_context_overrides_with_cohort_and_override_dont_match(self "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", "footer_additional_logo": "http://lms.example.com/base_logo.png", - "cohort_overrides" : { + "cohort_overrides": { "SomeGroup": { "footer_additional_logo": "http://lms.example.com/override_logo.png", }, @@ -109,7 +110,7 @@ def test_certificates_context_overrides_with_cohort_and_override_dont_match(self "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", "footer_additional_logo": "http://lms.example.com/base_logo.png", - "cohort_overrides" : { + "cohort_overrides": { "SomeGroup": { "footer_additional_logo": "http://lms.example.com/override_logo.png", }, @@ -119,7 +120,8 @@ def test_certificates_context_overrides_with_cohort_and_override_dont_match(self @patch('nau_openedx_extensions.certificates.context_overrides.get_cohort') def test_certificates_context_overrides_with_cohort_and_override(self, get_cohort_mock): """ - Check that the override is being applied if the learner belongs to a cohort and that cohort is configured has override. + Check that the override is being applied if the learner belongs to a cohort and that cohort + is configured has override. """ mocked_cohort = MagicMock() cohort_name_property = PropertyMock(return_value="SomeGroup") @@ -130,7 +132,7 @@ def test_certificates_context_overrides_with_cohort_and_override(self, get_cohor "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", "footer_additional_logo": "http://lms.example.com/base_logo.png", - "cohort_overrides" : { + "cohort_overrides": { "SomeGroup": { "footer_additional_logo": "http://lms.example.com/override_logo.png", }, @@ -143,7 +145,7 @@ def test_certificates_context_overrides_with_cohort_and_override(self, get_cohor "username": "nau@example.com", "course_id": "course-v1:Demo+DemoX+Demo_Course", "footer_additional_logo": "http://lms.example.com/override_logo.png", - "cohort_overrides" : { + "cohort_overrides": { "SomeGroup": { "footer_additional_logo": "http://lms.example.com/override_logo.png", }, diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..55750fe --- /dev/null +++ b/pytest.ini @@ -0,0 +1,6 @@ +[tool:pytest] +DJANGO_SETTINGS_MODULE = nau_openedx_extensions.settings.test + +[pytest] +log_cli = 1 +log_cli_level = INFO From 6031aa0128974aed6fa14da7f264c66c235924fa Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Wed, 2 Oct 2024 09:52:55 +0100 Subject: [PATCH 4/5] build: print log messages during test run Print log message only if the test fails. --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 55750fe..b637301 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,5 +2,5 @@ DJANGO_SETTINGS_MODULE = nau_openedx_extensions.settings.test [pytest] -log_cli = 1 +; log_cli = 1 log_cli_level = INFO From 84415579bea1799c4647ccd203dc4ee1e9973773 Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Wed, 2 Oct 2024 14:01:32 +0100 Subject: [PATCH 5/5] feat: create an ID Verification for verified mode enrollments Add a receiver of `COURSE_ENROLLMENT_CHANGED` Open edX Event, that will create a new Manual ID Verification if the enrollment mode of the enrollment that was changed is `verified`. Added openedx-events as dependency. Update dependencies. Fix lint format code because of the test update dependencies. fccn/nau-technical#200 --- nau_openedx_extensions/apps.py | 3 +- .../backends/verify_student_v1.py | 33 ++++ .../backends/verify_student_v1_tests.py | 28 +++ .../edxapp_wrapper/verify_student.py | 29 +++ .../locale/en/LC_MESSAGES/django.mo | Bin 784 -> 784 bytes .../locale/en/LC_MESSAGES/django.po | 6 +- .../locale/pt_PT/LC_MESSAGES/django.mo | Bin 2456 -> 2456 bytes .../locale/pt_PT/LC_MESSAGES/django.po | 6 +- nau_openedx_extensions/settings/common.py | 11 +- nau_openedx_extensions/settings/test.py | 3 + nau_openedx_extensions/signals.py | 7 + .../tests/test_verify_student.py | 183 ++++++++++++++++++ .../verify_student/id_verification.py | 50 +++++ requirements/base.in | 1 + requirements/base.txt | 1 + requirements/constraints.txt | 1 + requirements/test.txt | 1 + 17 files changed, 351 insertions(+), 12 deletions(-) create mode 100644 nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1.py create mode 100644 nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1_tests.py create mode 100644 nau_openedx_extensions/edxapp_wrapper/verify_student.py create mode 100644 nau_openedx_extensions/signals.py create mode 100644 nau_openedx_extensions/tests/test_verify_student.py create mode 100644 nau_openedx_extensions/verify_student/id_verification.py diff --git a/nau_openedx_extensions/apps.py b/nau_openedx_extensions/apps.py index 5506d3c..af54792 100644 --- a/nau_openedx_extensions/apps.py +++ b/nau_openedx_extensions/apps.py @@ -42,7 +42,6 @@ def ready(self): """ Method to perform actions after apps registry is ended """ + from nau_openedx_extensions import signals # pylint: disable=import-outside-toplevel,unused-import # noqa from nau_openedx_extensions.permissions import \ load_permissions # pylint: disable=import-outside-toplevel,unused-import # noqa - - # load_permissions() diff --git a/nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1.py b/nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1.py new file mode 100644 index 0000000..9b46d9f --- /dev/null +++ b/nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1.py @@ -0,0 +1,33 @@ +""" +Real implementation of user id verifications service. +""" +from django.contrib.auth import get_user_model +from lms.djangoapps.verify_student.models import ManualVerification # pylint: disable=import-error + + +def get_user_id_verifications(user_id, *args, **kwargs): + """ + Read the user's `ManualVerification` from the edx-platform. + + Args: + user: The user id to read the Id Verifications. + + Returns: + An enumeration of those Id Verifications + """ + user = get_user_model().objects.get(id=user_id) + return ManualVerification.objects.filter(user=user).order_by('-created_at') + + +def create_user_id_verification(user_id, *args, **kwargs): + """ + Create a new `ManualVerification` on the edx-platform. + + Args: + user: The user id that this Id verification should be created. + + Returns: + The object created + """ + user = get_user_model().objects.get(id=user_id) + ManualVerification(user=user, name=user.profile.name, *args, **kwargs).save() diff --git a/nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1_tests.py b/nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1_tests.py new file mode 100644 index 0000000..a786719 --- /dev/null +++ b/nau_openedx_extensions/edxapp_wrapper/backends/verify_student_v1_tests.py @@ -0,0 +1,28 @@ +""" +Real implementation of user id verifications service. +""" + + +def get_user_id_verifications(user_id, *args, **kwargs): # pylint: disable=unused-argument + """ + Read the user's `ManualVerification` from the edx-platform. + + Args: + user_id: The user id to read the Id Verifications. + + Returns: + An enumeration of those Id Verifications + """ + return [] + +def create_user_id_verification(user_id, *args, **kwargs): # pylint: disable=unused-argument + """ + Create a new `ManualVerification` on the edx-platform. + + Args: + user_id: The user id that this Id verification should be created. + + Returns: + The object created + """ + return None diff --git a/nau_openedx_extensions/edxapp_wrapper/verify_student.py b/nau_openedx_extensions/edxapp_wrapper/verify_student.py new file mode 100644 index 0000000..9658d2f --- /dev/null +++ b/nau_openedx_extensions/edxapp_wrapper/verify_student.py @@ -0,0 +1,29 @@ +""" +Student backend abstraction +""" +from __future__ import absolute_import, unicode_literals + +from importlib import import_module + +from django.conf import settings + + +def get_user_id_verifications(user_id, *args, **kwargs): + """ + Read the user's `ManualVerification` from the edx-platform. + """ + + backend_module = settings.NAU_VERIFY_STUDENT_MODULE + backend = import_module(backend_module) + + return backend.get_user_id_verifications(user_id, *args, **kwargs) + + +def create_user_id_verification(user_id, *args, **kwargs): + """ + Create an user Id Verification `ManualVerification` instance on the edx-platform. + """ + backend_module = settings.NAU_VERIFY_STUDENT_MODULE + backend = import_module(backend_module) + + return backend.create_user_id_verification(user_id, *args, **kwargs) diff --git a/nau_openedx_extensions/locale/en/LC_MESSAGES/django.mo b/nau_openedx_extensions/locale/en/LC_MESSAGES/django.mo index 8abd246a15b7ff873bdd4788b8de4e1127e8801d..46d442a7c17dbdcc98303f4a4266c635027d8f33 100644 GIT binary patch delta 22 dcmbQhHi2!zJ8nY*T>~QpLt`sbvyDH+nE*}129f{( delta 22 dcmbQhHi2!zJ8lD0T|)x}LlY|#(~UpHnE*}h29y8* diff --git a/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po b/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po index 6f4e142..007c63a 100644 --- a/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po +++ b/nau_openedx_extensions/locale/en/LC_MESSAGES/django.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: equipa@nau.edu.pt\n" -"POT-Creation-Date: 2024-09-26 11:25+0100\n" +"POT-Creation-Date: 2024-10-02 13:56+0100\n" "PO-Revision-Date: 2021-02-15 15:56+0000\n" "Last-Translator: FULL NAME \n" "Language: en\n" @@ -133,11 +133,11 @@ msgid "" "out in order to obtain a certificate." msgstr "" -#: nau_openedx_extensions/settings/common.py:89 +#: nau_openedx_extensions/settings/common.py:35 msgid "Certificate" msgstr "" -#: nau_openedx_extensions/settings/common.py:90 +#: nau_openedx_extensions/settings/common.py:36 msgid "Certificate of Achievement" msgstr "" diff --git a/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.mo b/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.mo index 4311d06ca9408ad836664b06e48871dd0f8c4d8a..b2f3646d20ef8678444b41a15eb6c6fe73a45735 100644 GIT binary patch delta 24 fcmbOsJVSUxI}5j=fv$m(f}ydMsoCa9ENrX*R1yXX delta 24 fcmbOsJVSUxI}5jgsji`cf}x3(iRtD^ENrX*R6Yg_ diff --git a/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po b/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po index 67e2d3d..415ac13 100644 --- a/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po +++ b/nau_openedx_extensions/locale/pt_PT/LC_MESSAGES/django.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: equipa@nau.edu.pt\n" -"POT-Creation-Date: 2024-09-26 11:25+0100\n" +"POT-Creation-Date: 2024-10-02 13:56+0100\n" "PO-Revision-Date: 2021-02-15 15:56+0000\n" "Last-Translator: Ivo Branco \n" "Language: pt_PT\n" @@ -142,11 +142,11 @@ msgstr "" "Este curso encontra-se arquivado e já não permite a realização de " "atividades para obtenção de certificado." -#: nau_openedx_extensions/settings/common.py:89 +#: nau_openedx_extensions/settings/common.py:35 msgid "Certificate" msgstr "Certificado" -#: nau_openedx_extensions/settings/common.py:90 +#: nau_openedx_extensions/settings/common.py:36 msgid "Certificate of Achievement" msgstr "Certificado de Conclusão" diff --git a/nau_openedx_extensions/settings/common.py b/nau_openedx_extensions/settings/common.py index 7ad3126..36e4995 100644 --- a/nau_openedx_extensions/settings/common.py +++ b/nau_openedx_extensions/settings/common.py @@ -31,6 +31,10 @@ def plugin_settings(settings): See: https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/plugins/README.rst """ + # Overwrite the default certificate name + settings.CERT_NAME_SHORT = _("Certificate") + settings.CERT_NAME_LONG = _("Certificate of Achievement") + settings.NAU_CUSTOM_SAML_IDENTITY_PROVIDERS = [ { "provider_key": "nau_custom_saml_provider", @@ -84,7 +88,6 @@ def plugin_settings(settings): settings.NAU_COHORT_MODULE = ( "nau_openedx_extensions.edxapp_wrapper.backends.cohort_v1" ) - - # Overwrite the default certificate name - settings.CERT_NAME_SHORT = _("Certificate") - settings.CERT_NAME_LONG = _("Certificate of Achievement") + settings.NAU_VERIFY_STUDENT_MODULE = ( + "nau_openedx_extensions.edxapp_wrapper.backends.verify_student_v1" + ) diff --git a/nau_openedx_extensions/settings/test.py b/nau_openedx_extensions/settings/test.py index 5be662d..4edd08c 100644 --- a/nau_openedx_extensions/settings/test.py +++ b/nau_openedx_extensions/settings/test.py @@ -50,3 +50,6 @@ class SettingsClass: NAU_COHORT_MODULE = ( "nau_openedx_extensions.edxapp_wrapper.backends.cohort_v1_tests" ) +NAU_VERIFY_STUDENT_MODULE = ( + "nau_openedx_extensions.edxapp_wrapper.backends.verify_student_v1_tests" +) diff --git a/nau_openedx_extensions/signals.py b/nau_openedx_extensions/signals.py new file mode 100644 index 0000000..44e828a --- /dev/null +++ b/nau_openedx_extensions/signals.py @@ -0,0 +1,7 @@ +""" +File that contains the definition of all signals and its receivers. +""" + +from nau_openedx_extensions.verify_student.id_verification import ( # pylint: disable=unused-import + event_receiver_no_id_verify_for_enrollment_modes, +) diff --git a/nau_openedx_extensions/tests/test_verify_student.py b/nau_openedx_extensions/tests/test_verify_student.py new file mode 100644 index 0000000..81f5147 --- /dev/null +++ b/nau_openedx_extensions/tests/test_verify_student.py @@ -0,0 +1,183 @@ +""" +Tests for Id Verification of studentes. +""" + +from datetime import datetime +from unittest.mock import Mock, patch + +from django.test import TestCase, override_settings +from openedx_events.learning.data import CourseEnrollmentData, UserData +from openedx_events.learning.signals import COURSE_ENROLLMENT_CHANGED + +from nau_openedx_extensions.verify_student.id_verification import event_receiver_no_id_verify_for_enrollment_modes + + +class VerifyStudentTest(TestCase): + """ + Tests for Id Verification patch. + """ + + @patch( + "nau_openedx_extensions.verify_student.id_verification.get_user_id_verifications" + ) + @patch( + "nau_openedx_extensions.verify_student.id_verification.create_user_id_verification" + ) + def test_verify_student_create_new_verification( + self, create_user_id_verification_mock, get_user_id_verifications_mock + ): + """ + Test an enrollment mode that requires a ID Verification. + """ + user_id = 10 + get_user_id_verifications_mock.return_value = [] + COURSE_ENROLLMENT_CHANGED.connect(event_receiver_no_id_verify_for_enrollment_modes) + COURSE_ENROLLMENT_CHANGED.send_event( + enrollment=CourseEnrollmentData( + user=UserData(id=user_id, is_active=True, pii=None), + mode="verified", + course=None, + is_active=None, + creation_date=None, + ) + ) + create_user_id_verification_mock.assert_called_once() + self.assertEqual(create_user_id_verification_mock.call_args.args[0], user_id) + create_user_id_verification_mock_kwargs = ( + create_user_id_verification_mock.call_args.kwargs + ) + self.assertEqual( + create_user_id_verification_mock_kwargs, + { + **create_user_id_verification_mock_kwargs, + **{ + "status": "approved", + }, + }, + ) + self.assertEqual( + create_user_id_verification_mock_kwargs, + { + **create_user_id_verification_mock_kwargs, + **{ + "reason": "Skip id verification from nau_openedx_extensions", + }, + }, + ) + self.assertEqual( + create_user_id_verification_mock.call_args.kwargs.get( + "expiration_date" + ).year, + datetime.today().year + 100, + ) + + @patch( + "nau_openedx_extensions.verify_student.id_verification.get_user_id_verifications" + ) + @patch( + "nau_openedx_extensions.verify_student.id_verification.create_user_id_verification" + ) + def test_verify_student_enrollment_mode_not_need_id_verification_patch( + self, create_user_id_verification_mock, get_user_id_verifications_mock + ): + """ + Test a case enrollment mode that doesn't requires a ID Verification. + """ + user_id = 10 + get_user_id_verifications_mock.return_value = [] + COURSE_ENROLLMENT_CHANGED.connect(event_receiver_no_id_verify_for_enrollment_modes) + COURSE_ENROLLMENT_CHANGED.send_event( + enrollment=CourseEnrollmentData( + user=UserData(id=user_id, is_active=True, pii=None), + mode="honor", + course=None, + is_active=None, + creation_date=None, + ) + ) + create_user_id_verification_mock.assert_not_called() + + @patch( + "nau_openedx_extensions.verify_student.id_verification.get_user_id_verifications" + ) + @patch( + "nau_openedx_extensions.verify_student.id_verification.create_user_id_verification" + ) + @override_settings(NAU_NO_ID_VERIFY_FOR_ENROLLMENT_MODES="verified, somemode") + def test_verify_student_change_enrollment_modes_require_id_verification( + self, create_user_id_verification_mock, get_user_id_verifications_mock + ): + """ + Test that changing the setting `NAU_NO_ID_VERIFY_FOR_ENROLLMENT_MODES` to include a custom enrollment mode, + and test with that new custom enrollment mode, it still creates an id verification. + """ + user_id = 10 + get_user_id_verifications_mock.return_value = [] + COURSE_ENROLLMENT_CHANGED.connect(event_receiver_no_id_verify_for_enrollment_modes) + COURSE_ENROLLMENT_CHANGED.send_event( + enrollment=CourseEnrollmentData( + user=UserData(id=user_id, is_active=True, pii=None), + mode="somemode", + course=None, + is_active=None, + creation_date=None, + ) + ) + create_user_id_verification_mock.assert_called_once() + self.assertEqual(create_user_id_verification_mock.call_args.args[0], user_id) + create_user_id_verification_mock_kwargs = ( + create_user_id_verification_mock.call_args.kwargs + ) + self.assertEqual( + create_user_id_verification_mock_kwargs, + { + **create_user_id_verification_mock_kwargs, + **{ + "status": "approved", + }, + }, + ) + self.assertEqual( + create_user_id_verification_mock_kwargs, + { + **create_user_id_verification_mock_kwargs, + **{ + "reason": "Skip id verification from nau_openedx_extensions", + }, + }, + ) + self.assertEqual( + create_user_id_verification_mock.call_args.kwargs.get( + "expiration_date" + ).year, + datetime.today().year + 100, + ) + + @patch( + "nau_openedx_extensions.verify_student.id_verification.get_user_id_verifications" + ) + @patch( + "nau_openedx_extensions.verify_student.id_verification.create_user_id_verification" + ) + def test_verify_student_with_existing_id_verification( + self, create_user_id_verification_mock, get_user_id_verifications_mock + ): + """ + Test that if the user already has an id verification, it won't try to create a new one. + """ + active_at_datetime_mock = Mock() + active_at_datetime_mock.return_value = True + + user_id = 10 + get_user_id_verifications_mock.return_value = active_at_datetime_mock + COURSE_ENROLLMENT_CHANGED.connect(event_receiver_no_id_verify_for_enrollment_modes) + COURSE_ENROLLMENT_CHANGED.send_event( + enrollment=CourseEnrollmentData( + user=UserData(id=user_id, is_active=True, pii=None), + mode="verify", + course=None, + is_active=None, + creation_date=None, + ) + ) + create_user_id_verification_mock.assert_not_called() diff --git a/nau_openedx_extensions/verify_student/id_verification.py b/nau_openedx_extensions/verify_student/id_verification.py new file mode 100644 index 0000000..20ea909 --- /dev/null +++ b/nau_openedx_extensions/verify_student/id_verification.py @@ -0,0 +1,50 @@ +""" +NAU Custom code to skip Open edX ID Verification module. +""" +import logging +from datetime import datetime, timedelta + +from django.conf import settings +from django.dispatch import receiver +from openedx_events.learning.data import CourseEnrollmentData +from openedx_events.learning.signals import COURSE_ENROLLMENT_CHANGED +from pytz import UTC + +from nau_openedx_extensions.edxapp_wrapper.verify_student import create_user_id_verification, get_user_id_verifications + +log = logging.getLogger(__name__) + + +@receiver(COURSE_ENROLLMENT_CHANGED) +def event_receiver_no_id_verify_for_enrollment_modes(enrollment: CourseEnrollmentData, **kwargs): + """ + This receiver will ignore / skip the id verification of the Open edX platform. + Meaning that will create `ManualVerification` object if `enrollment_mode` is defined in the + `NAU_NO_ID_VERIFY_FOR_ENROLLMENT_MODES` setting, defaults to just the `verified` enrollment mode. + It should be configured using the Open edX signal: + `openedx_events.learning.signals.COURSE_ENROLLMENT_CHANGED` + """ + log.info("On event receiver that makes removes the need of ID Verify for some enrollment modes") + enrollment_mode = enrollment.mode + enrollment_modes_to_skip_as_str = getattr(settings, 'NAU_NO_ID_VERIFY_FOR_ENROLLMENT_MODES', 'verified') + enrollment_modes_to_skip = list(map(str.strip, enrollment_modes_to_skip_as_str.split(','))) + if enrollment_mode in enrollment_modes_to_skip: + user_id = enrollment.user.id + now = datetime.now(UTC) + + def verification_active_predicate(verification): + return verification.active_at_datetime(now) + + user_manual_verifications = get_user_id_verifications(user_id) + verification_active = next(filter(verification_active_predicate, user_manual_verifications), None) + if verification_active: + log.info("User %d already has an ID verification", user_id) + else: + expiration_date = now + timedelta(days=36500) # 100 years + log.info("Create user ID Verification for %d", user_id) + create_user_id_verification( + user_id, + status='approved', + expiration_date=expiration_date, + reason="Skip id verification from nau_openedx_extensions" + ) diff --git a/requirements/base.in b/requirements/base.in index 5e3ac27..65ca28c 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -5,3 +5,4 @@ six future; python_version < "3.0" web-fragments openedx-filters==0.7.0 +openedx-events==0.8.1 diff --git a/requirements/base.txt b/requirements/base.txt index 23d4952..45bd75b 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -11,6 +11,7 @@ django==2.2.25 # via -c requirements/constraints.txt, edx-opaque-keys edx-opaque-keys[django]==2.2.0 # via -c requirements/constraints.txt, -r requirements/base.in kombu==4.6.11 # via celery openedx-filters==0.7.0 # via -c requirements/constraints.txt, -r requirements/base.in +openedx-events==0.8.1 pbr==5.10.0 # via stevedore pymongo==4.2.0 # via edx-opaque-keys pytz==2022.2.1 # via celery, django diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 9ac3412..5e1cee2 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -3,5 +3,6 @@ celery<5.0 Django==2.2.25 edx-opaque-keys[django]==2.2.0 openedx-filters==0.7.0 +openedx-events==0.8.1 pip-tools<5.4 click==7.1.2 diff --git a/requirements/test.txt b/requirements/test.txt index c77c2cd..8f303f5 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -23,6 +23,7 @@ lazy-object-proxy==1.7.1 # via astroid markupsafe==2.1.1 # via jinja2 mccabe==0.7.0 # via pylint openedx-filters==0.7.0 # via -c requirements/constraints.txt, -r requirements/base.txt +openedx-events==0.8.1 packaging==21.3 # via pytest pbr==5.10.0 # via -r requirements/base.txt, stevedore platformdirs==2.5.2 # via pylint