From d2fa7f7239842a5c8fa8782e1cfcc6fa7a901282 Mon Sep 17 00:00:00 2001 From: Bianca Severino <biancasev@gmail.com> Date: Tue, 3 Aug 2021 10:13:02 -0400 Subject: [PATCH] feat: use verified name for certs if enabled If the verified name feature is enabled and the user has their preference set to use verified name for certificates, create and display certificates with their verified name rather than their profile name. --- lms/djangoapps/certificates/generation.py | 9 ++--- lms/djangoapps/certificates/models.py | 25 +++++++++++-- .../certificates/tests/test_generation.py | 37 +++++++++++++++++++ .../certificates/tests/test_models.py | 33 +++++++++++++++++ .../certificates/tests/test_webview_views.py | 33 ++++++++++++++++- lms/djangoapps/certificates/utils.py | 23 ++++++++++++ lms/djangoapps/certificates/views/webview.py | 9 ++++- 7 files changed, 156 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index 9a0b944f8a4..a6485c57ab9 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -10,10 +10,9 @@ These methods should be called from tasks. import logging from uuid import uuid4 -from common.djangoapps.student import models_api as student_api from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import GeneratedCertificate -from lms.djangoapps.certificates.utils import emit_certificate_event +from lms.djangoapps.certificates.utils import emit_certificate_event, get_preferred_certificate_name log = logging.getLogger(__name__) @@ -66,9 +65,7 @@ def _generate_certificate(user, course_key, status, enrollment_mode, course_grad # Retrieve the existing certificate for the learner if it exists existing_certificate = GeneratedCertificate.certificate_for_student(user, course_key) - profile_name = student_api.get_name(user.id) - if not profile_name: - profile_name = '' + preferred_name = get_preferred_certificate_name(user) # Retain the `verify_uuid` from an existing certificate if possible, this will make it possible for the learner to # keep the existing URL to their certificate @@ -84,7 +81,7 @@ def _generate_certificate(user, course_key, status, enrollment_mode, course_grad 'user': user, 'course_id': course_key, 'mode': enrollment_mode, - 'name': profile_name, + 'name': preferred_name, 'status': status, 'grade': course_grade, 'download_url': '', diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index 005f4c66e39..e76849f35a3 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -18,6 +18,8 @@ from django.db.models import Count from django.dispatch import receiver from django.utils.translation import ugettext_lazy as _ +from edx_name_affirmation.api import get_verified_name, should_use_verified_name_for_certs +from edx_name_affirmation.toggles import is_verified_name_enabled from model_utils import Choices from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField @@ -370,9 +372,7 @@ class GeneratedCertificate(models.Model): if not mode: mode = self.mode - profile_name = student_api.get_name(self.user.id) - if not profile_name: - profile_name = '' + preferred_name = self._get_preferred_certificate_name(self.user) self.error_reason = '' self.download_uuid = '' @@ -380,7 +380,7 @@ class GeneratedCertificate(models.Model): self.grade = grade self.status = status self.mode = mode - self.name = profile_name + self.name = preferred_name self.save() COURSE_CERT_REVOKED.send_robust( @@ -404,6 +404,23 @@ class GeneratedCertificate(models.Model): } emit_certificate_event('revoked', self.user, str(self.course_id), event_data=event_data) + def _get_preferred_certificate_name(self, user): + """ + Copy of `get_preferred_certificate_name` from utils.py - importing it here would introduce + a circular dependency. + """ + name_to_use = student_api.get_name(user.id) + + if is_verified_name_enabled() and should_use_verified_name_for_certs(user): + verified_name_obj = get_verified_name(user, is_verified=True) + if verified_name_obj: + name_to_use = verified_name_obj.verified_name + + if not name_to_use: + name_to_use = '' + + return name_to_use + def is_valid(self): """ Return True if certificate is valid else return False. diff --git a/lms/djangoapps/certificates/tests/test_generation.py b/lms/djangoapps/certificates/tests/test_generation.py index e7e752fd502..add17cdae77 100644 --- a/lms/djangoapps/certificates/tests/test_generation.py +++ b/lms/djangoapps/certificates/tests/test_generation.py @@ -1,9 +1,14 @@ """ Tests for certificate generation """ +import ddt import logging from unittest import mock +from edx_name_affirmation.api import create_verified_name, create_verified_name_config +from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG +from edx_toggles.toggles.testutils import override_waffle_flag + from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import UserProfile from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory @@ -20,6 +25,7 @@ log = logging.getLogger(__name__) PROFILE_NAME_METHOD = 'common.djangoapps.student.models_api.get_name' +@ddt.ddt class CertificateTests(EventTestMixin, ModuleStoreTestCase): """ Tests for certificate generation @@ -187,3 +193,34 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase): assert cert.mode == self.enrollment_mode assert cert.grade == self.grade assert cert.name == '' + + @override_waffle_flag(VERIFIED_NAME_FLAG, active=True) + @ddt.data((True, True), (True, False), (False, False)) + @ddt.unpack + def test_generation_verified_name(self, should_use_verified_name_for_certs, is_verified): + """ + Test that if verified name functionality is enabled and the user has their preference set to use + verified name for certificates, their verified name will appear on the certificate rather than + their profile name. + """ + verified_name = 'Jonathan Doe' + create_verified_name(self.u, verified_name, self.name, is_verified=is_verified) + create_verified_name_config(self.u, use_verified_name_for_certs=should_use_verified_name_for_certs) + + GeneratedCertificateFactory( + user=self.u, + course_id=self.key, + mode=CourseMode.AUDIT, + status=CertificateStatuses.unverified + ) + + generate_course_certificate( + self.u, self.key, CertificateStatuses.downloadable, self.enrollment_mode, self.grade, self.gen_mode, + ) + + cert = GeneratedCertificate.objects.get(user=self.u, course_id=self.key) + + if should_use_verified_name_for_certs and is_verified: + assert cert.name == verified_name + else: + assert cert.name == self.name diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index 456c471cb53..2025c27289d 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -12,6 +12,9 @@ from django.core.exceptions import ValidationError from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase from django.test.utils import override_settings +from edx_name_affirmation.api import create_verified_name, create_verified_name_config +from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG +from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.locator import CourseKey, CourseLocator from path import Path as path @@ -360,6 +363,7 @@ class CertificateInvalidationTest(SharedModuleStoreTestCase): assert mock_revoke_task.call_args[0] == (self.user.username, str(self.course_id)) +@ddt.ddt class GeneratedCertificateTest(SharedModuleStoreTestCase): """ Test GeneratedCertificates @@ -540,6 +544,35 @@ class GeneratedCertificateTest(SharedModuleStoreTestCase): self._assert_event_data(mock_emit_certificate_event, expected_event_data) + @override_waffle_flag(VERIFIED_NAME_FLAG, active=True) + @ddt.data((True, True), (True, False), (False, False)) + @ddt.unpack + def test_invalidate_with_verified_name(self, should_use_verified_name_for_certs, is_verified): + """ + Test the invalidate method with verified name turned on for the user's certificates + """ + verified_name = 'Jonathan Doe' + profile = UserProfile.objects.get(user=self.user) + create_verified_name(self.user, verified_name, profile.name, is_verified=is_verified) + create_verified_name_config(self.user, use_verified_name_for_certs=should_use_verified_name_for_certs) + + cert = GeneratedCertificateFactory.create( + status=CertificateStatuses.downloadable, + user=self.user, + course_id=self.course_key, + mode=CourseMode.AUDIT, + name='Fuzzy Hippo' + ) + mode = CourseMode.VERIFIED + source = 'invalidated_test' + cert.invalidate(mode=mode, source=source) + + cert = GeneratedCertificate.objects.get(user=self.user, course_id=self.course_key) + if should_use_verified_name_for_certs and is_verified: + assert cert.name == verified_name + else: + assert cert.name == profile.name + @patch('lms.djangoapps.certificates.utils.emit_certificate_event') def test_unverified(self, mock_emit_certificate_event): """ diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index a389e297a99..188e4b737be 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -12,8 +12,10 @@ from django.conf import settings from django.test.client import Client, RequestFactory from django.test.utils import override_settings from django.urls import reverse +from edx_name_affirmation.api import create_verified_name, create_verified_name_config +from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG from edx_toggles.toggles import LegacyWaffleSwitch -from edx_toggles.toggles.testutils import override_waffle_switch +from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch from organizations import api as organizations_api from common.djangoapps.course_modes.models import CourseMode @@ -1524,6 +1526,35 @@ class CertificatesViewsTests(CommonCertificatesTestCase, CacheIsolationTestCase) ) ) + @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED) + @override_waffle_flag(VERIFIED_NAME_FLAG, active=True) + @ddt.data((True, True), (True, False), (False, False)) + @ddt.unpack + def test_certificate_view_verified_name(self, should_use_verified_name_for_certs, is_verified): + """ + Test that if verified name functionality is enabled and the user has their preference set to use + verified name for certificates, their verified name will appear on the certificate rather than + their profile name. + """ + verified_name = 'Jonathan Doe' + create_verified_name(self.user, verified_name, self.user.profile.name, is_verified=is_verified) + create_verified_name_config(self.user, use_verified_name_for_certs=should_use_verified_name_for_certs) + + self._add_course_certificates(count=1, signatory_count=1) + test_url = get_certificate_url( + user_id=self.user.id, + course_id=str(self.course.id), + uuid=self.cert.verify_uuid + ) + + response = self.client.get(test_url, HTTP_HOST='test.localhost') + if should_use_verified_name_for_certs and is_verified: + self.assertContains(response, verified_name) + self.assertNotContains(response, self.user.profile.name) + else: + self.assertContains(response, self.user.profile.name) + self.assertNotContains(response, verified_name) + class CertificateEventTests(CommonCertificatesTestCase, EventTrackingTestCase): """ diff --git a/lms/djangoapps/certificates/utils.py b/lms/djangoapps/certificates/utils.py index da3fd0db450..d4111436589 100644 --- a/lms/djangoapps/certificates/utils.py +++ b/lms/djangoapps/certificates/utils.py @@ -4,12 +4,16 @@ Certificates utilities from datetime import datetime import logging +from edx_name_affirmation.api import get_verified_name, should_use_verified_name_for_certs +from edx_name_affirmation.toggles import is_verified_name_enabled + from django.conf import settings from django.urls import reverse from eventtracking import tracker from opaque_keys.edx.keys import CourseKey from pytz import utc +from common.djangoapps.student import models_api as student_api from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import GeneratedCertificate from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none @@ -228,3 +232,22 @@ def certificate_status_for_student(student, course_id): except GeneratedCertificate.DoesNotExist: generated_certificate = None return certificate_status(generated_certificate) + + +def get_preferred_certificate_name(user): + """ + If the verified name feature is enabled and the user has their preference set to use their + verified name for certificates, return their verified name. Else, return the user's profile + name, or an empty string if it doesn't exist. + """ + name_to_use = student_api.get_name(user.id) + + if is_verified_name_enabled() and should_use_verified_name_for_certs(user): + verified_name_obj = get_verified_name(user, is_verified=True) + if verified_name_obj: + name_to_use = verified_name_obj.verified_name + + if not name_to_use: + name_to_use = '' + + return name_to_use diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index f37b3c64e45..e18137eacde 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -43,7 +43,11 @@ from lms.djangoapps.certificates.models import ( GeneratedCertificate ) from lms.djangoapps.certificates.permissions import PREVIEW_CERTIFICATES -from lms.djangoapps.certificates.utils import emit_certificate_event, get_certificate_url +from lms.djangoapps.certificates.utils import ( + emit_certificate_event, + get_certificate_url, + get_preferred_certificate_name +) from openedx.core.djangoapps.catalog.api import get_course_run_details from openedx.core.djangoapps.certificates.api import display_date_for_certificate from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none @@ -305,7 +309,8 @@ def _update_context_with_user_info(context, user, user_certificate): """ Updates context dictionary with user related info. """ - user_fullname = user.profile.name + user_fullname = get_preferred_certificate_name(user) + context['username'] = user.username context['course_mode'] = user_certificate.mode context['accomplishment_user_id'] = user.id -- GitLab