fix: Do Not Prevent Learner From Changing Name With Non-Verified Enrollment/Non-Passable Certificate

The _does_name_change_require_verification(user_profile, old_name, new_name) method of the accounts user_api determines whether a learner can change their name from old_name to new_name. Originally, it delegated solely to the NameChangeValidator class of the edx-name-affirmation API, which ran a set of checks against the name change. One of said checks was asserting that learners with one or more certificates could not change their name without completing IDV.

This pull request changes this behavior.

Learners may have certificates that are not in a passable status (e.g. "unverified"). We only want to require IDV for name changes for learners that have passing statuses. The existing code prevented learners from changing their name if they had any certificates at all, irrespective of the certificate status. This change only considers certificates in a passable status.

Additionally, learners may have certificates and also not be enrolled in any "verified" seats. For example, despite edX no longer offering "honor" seats, learners may have enrollments in "honor" modes, which grant certificates but are not considered "verified" enrollment modes. IDV requires that a learner be enrolled in a "verified" seat in order to complete IDV. Prior to this change, learners that were navigated to IDV to validate a name change were unable to complete IDV. This change introduce a check that a learner is in a "verified" mode in addition to using the NameChangeValidator. This prevents the account MFE from navigating an IDV-ineligible learner to IDV.

MST-1254: https://openedx.atlassian.net/browse/MST-1254

openedx/core/djangoapps/enrollments/api.py

@@ -21,6 +21,59 @@ log = logging.getLogger(__name__)
 DEFAULT_DATA_API = 'openedx.core.djangoapps.enrollments.data'
 
 
+def get_verified_enrollments(username, include_inactive=False):
+    """Retrieves all the courses in which user is enrolled in a verified mode.
+
+    Takes a user and retrieves all relative enrollments in which the learner is enrolled in a verified mode.
+    Includes information regarding how the user is enrolled
+    in the the course.
+
+    Args:
+        username: The username of the user we want to retrieve course enrollment information for.
+        include_inactive (bool): Determines whether inactive enrollments will be included
+
+    Returns:
+        A list of enrollment information for the given user.
+
+    Examples:
+        >>> get_verified_enrollments("Bob")
+        [
+            {
+                "created": "2014-10-25T20:18:00Z",
+                "mode": "verified",
+                "is_active": True,
+                "user": "Bob",
+                "course_details": {
+                    "course_id": "edX/edX-Insider/2014T2",
+                    "course_name": "edX Insider Course",
+                    "enrollment_end": "2014-12-20T20:18:00Z",
+                    "enrollment_start": "2014-10-15T20:18:00Z",
+                    "course_start": "2015-02-03T00:00:00Z",
+                    "course_end": "2015-05-06T00:00:00Z",
+                    "course_modes": [
+                        {
+                            "slug": "honor",
+                            "name": "Honor Code Certificate",
+                            "min_price": 0,
+                            "suggested_prices": "",
+                            "currency": "usd",
+                            "expiration_datetime": null,
+                            "description": null,
+                            "sku": null,
+                            "bulk_sku": null
+                        }
+                    ],
+                    "invite_only": True
+                }
+            }
+        ]
+
+    """
+    enrollments = get_enrollments(username, include_inactive)
+    enrollments = filter(lambda enrollment: CourseMode.is_verified_slug(enrollment['mode']), enrollments)
+    return list(enrollments)
+
+
 def get_enrollments(username, include_inactive=False):
     """Retrieves all the courses a user is enrolled in.
 

openedx/core/djangoapps/enrollments/tests/test_api.py

@@ -165,6 +165,31 @@ class EnrollmentTest(CacheIsolationTestCase):
         for result_enrollment in result:
             assert result_enrollment['course']['course_id'] in [enrollment['course_id'] for enrollment in enrollments]
 
+    @ddt.data(
+        # Simple test of honor and verified.
+        ([
+            {'course_id': 'the/first/course', 'course_modes': [], 'mode': 'honor'},
+            {'course_id': 'the/second/course', 'course_modes': ['honor', 'verified'], 'mode': 'verified'}
+        ], 1),
+
+        # No enrollments
+        ([], 0),
+
+        # One Enrollment
+        ([
+            {'course_id': 'the/third/course', 'course_modes': ['honor', 'verified', 'audit'], 'mode': 'audit'}
+        ], 0),
+    )
+    @ddt.unpack
+    def test_get_verified_enrollments(self, enrollments, num_verified_enrollments):
+        for enrollment in enrollments:
+            fake_data_api.add_course(enrollment['course_id'], course_modes=enrollment['course_modes'])
+            api.add_enrollment(self.USERNAME, enrollment['course_id'], enrollment['mode'])
+        result = api.get_verified_enrollments(self.USERNAME)
+        assert num_verified_enrollments == len(result)
+        for result_enrollment in result:
+            assert result_enrollment['course']['course_id'] in [enrollment['course_id'] for enrollment in enrollments]
+
     def test_update_enrollment(self):
         # Add fake course enrollment information to the fake data API
         fake_data_api.add_course(self.COURSE_ID, course_modes=['honor', 'verified', 'audit'])

openedx/core/djangoapps/user_api/accounts/api.py

@@ -24,7 +24,9 @@ from common.djangoapps.student.models import (
 from common.djangoapps.util.model_utils import emit_settings_changed_event
 from common.djangoapps.util.password_policy_validators import validate_password
 from lms.djangoapps.certificates.api import get_certificates_for_user
+from lms.djangoapps.certificates.data import CertificateStatuses
 
+from openedx.core.djangoapps.enrollments.api import get_verified_enrollments
 from openedx.core.djangoapps.user_api import accounts, errors, helpers
 from openedx.core.djangoapps.user_api.errors import (
     AccountUpdateError,
@@ -270,18 +272,31 @@ def _validate_name_change(user_profile, data, field_errors):
 
 def _does_name_change_require_verification(user_profile, old_name, new_name):
     """
-    If name change requires verification, do not update it through this API.
+    If name change requires ID verification, do not update it through this API.
     """
-
     profile_meta = user_profile.get_meta()
     old_names_list = profile_meta['old_names'] if 'old_names' in profile_meta else []
 
     user = user_profile.user
-    num_certs = len(get_certificates_for_user(user.username))
 
-    validator = NameChangeValidator(old_names_list, num_certs, old_name, new_name)
+    # We only want to validate on a list of passing certificates for the learner. A learner may have
+    # a certificate in a non-passing status, and we do not have to require ID verification based on certificates
+    # that are not passing.
+    passing_certs = filter(
+        lambda cert: CertificateStatuses.is_passing_status(cert["status"]),
+        get_certificates_for_user(user.username)
+    )
+    num_passing_certs = len(list(passing_certs))
 
-    return not validator.validate()
+    # We check whether the learner has active verified enrollments because we do not want to
+    # require the learner to perform ID verification if the learner is not enrolled in a verified mode
+    # in any courses. The learner will not be able to complete ID verification without being enrolled in
+    # at least one seat.
+    has_verified_enrollments = len(get_verified_enrollments(user.username)) > 0
+
+    validator = NameChangeValidator(old_names_list, num_passing_certs, old_name, new_name)
+
+    return not validator.validate() and has_verified_enrollments
 
 
 def _get_old_language_proficiencies_if_updating(user_profile, data):

openedx/core/djangoapps/user_api/accounts/tests/test_api.py

@@ -29,6 +29,7 @@ from common.djangoapps.student.tests.factories import UserFactory
 from common.djangoapps.student.tests.tests import UserSettingsEventTestMixin
 from common.djangoapps.student.views.management import activate_secondary_email
 
+from lms.djangoapps.certificates.data import CertificateStatuses
 from openedx.core.djangoapps.ace_common.tests.mixins import EmailTemplateTagMixin
 from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY
 from openedx.core.djangoapps.user_api.accounts.api import (
@@ -384,15 +385,18 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAc
 
         with patch(
             'openedx.core.djangoapps.user_api.accounts.api.get_certificates_for_user',
-            return_value=['mock_certificate']
+            return_value=[{'status': CertificateStatuses.downloadable}]
         ):
             update_account_settings(self.user, {'name': account_settings['name']})
             # The name should not be added to profile metadata
             updated_meta = user_profile.get_meta()
             self.assertEqual(meta, updated_meta)
 
-    @patch('edx_name_affirmation.name_change_validator.NameChangeValidator', Mock())
     @patch('edx_name_affirmation.name_change_validator.NameChangeValidator.validate', Mock(return_value=False))
+    @patch('openedx.core.djangoapps.user_api.accounts.api.get_certificates_for_user',
+           Mock(return_value=[{'status': CertificateStatuses.downloadable}]))
+    @patch('openedx.core.djangoapps.user_api.accounts.api.get_verified_enrollments',
+           Mock(return_value=[{'name': 'Bob'}]))
     def test_name_update_requires_idv(self):
         """
         Test that a name change is blocked through this API if it requires ID verification.
@@ -409,11 +413,26 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAc
 
     @patch('edx_name_affirmation.name_change_validator.NameChangeValidator', Mock())
     @patch('edx_name_affirmation.name_change_validator.NameChangeValidator.validate', Mock(return_value=True))
-    def test_name_update_does_not_require_idv(self):
+    @ddt.data(
+        (True, False),
+        (False, True),
+        (False, False)
+    )
+    @ddt.unpack
+    def test_name_update_does_not_require_idv(self, has_passable_cert, enrolled_in_verified_mode):
         """
         Test that the user can change their name if change does not require IDV.
         """
-        update_account_settings(self.user, {'name': 'New Name'})
+        with patch('openedx.core.djangoapps.user_api.accounts.api.get_certificates_for_user') as mock_get_certs,\
+             patch('openedx.core.djangoapps.user_api.accounts.api.get_verified_enrollments') as \
+                mock_get_verified_enrollments:
+            mock_get_certs.return_value = (
+                [{'status': CertificateStatuses.downloadable}] if
+                has_passable_cert else
+                [{'status': CertificateStatuses.unverified}]
+            )
+            mock_get_verified_enrollments.return_value = [{'name': 'Bob'}] if enrolled_in_verified_mode else []
+            update_account_settings(self.user, {'name': 'New Name'})
 
         account_settings = get_account_settings(self.default_request)[0]
         assert account_settings['name'] == 'New Name'