Merge pull request #29136 from edx/mroytman/MST-1130-remove-verified-name-waffle-flag
Remove Use of VERIFIED_NAME_FLAG Waffle Flag and is_verified_enabled Utility
This commit is contained in:
@@ -19,7 +19,6 @@ 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
|
||||
@@ -437,7 +436,7 @@ class GeneratedCertificate(models.Model):
|
||||
"""
|
||||
name_to_use = student_api.get_name(user.id)
|
||||
|
||||
if is_verified_name_enabled() and should_use_verified_name_for_certs(user):
|
||||
if 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
|
||||
|
||||
@@ -7,8 +7,6 @@ from unittest import mock
|
||||
|
||||
from edx_name_affirmation.api import create_verified_name, create_verified_name_config
|
||||
from edx_name_affirmation.statuses import VerifiedNameStatus
|
||||
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
|
||||
@@ -195,14 +193,13 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase):
|
||||
assert cert.grade == self.grade
|
||||
assert cert.name == ''
|
||||
|
||||
@override_waffle_flag(VERIFIED_NAME_FLAG, active=True)
|
||||
@ddt.data((True, VerifiedNameStatus.APPROVED),
|
||||
(True, VerifiedNameStatus.DENIED),
|
||||
(False, VerifiedNameStatus.PENDING))
|
||||
@ddt.unpack
|
||||
def test_generation_verified_name(self, should_use_verified_name_for_certs, status):
|
||||
"""
|
||||
Test that if verified name functionality is enabled and the user has their preference set to use
|
||||
Test that if 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.
|
||||
"""
|
||||
|
||||
@@ -14,8 +14,6 @@ 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.statuses import VerifiedNameStatus
|
||||
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 openedx_events.tests.utils import OpenEdxEventsTestMixin
|
||||
from path import Path as path
|
||||
@@ -626,7 +624,6 @@ class GeneratedCertificateTest(SharedModuleStoreTestCase, OpenEdxEventsTestMixin
|
||||
|
||||
self._assert_event_data(mock_emit_certificate_event, expected_event_data)
|
||||
|
||||
@override_waffle_flag(VERIFIED_NAME_FLAG, active=True)
|
||||
@ddt.data((True, VerifiedNameStatus.APPROVED),
|
||||
(True, VerifiedNameStatus.DENIED),
|
||||
(False, VerifiedNameStatus.PENDING))
|
||||
|
||||
@@ -13,8 +13,7 @@ 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.statuses import VerifiedNameStatus
|
||||
from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG
|
||||
from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch
|
||||
from edx_toggles.toggles.testutils import override_waffle_switch
|
||||
from organizations import api as organizations_api
|
||||
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
@@ -1582,16 +1581,14 @@ class CertificatesViewsTests(CommonCertificatesTestCase, CacheIsolationTestCase)
|
||||
)
|
||||
|
||||
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
|
||||
@override_waffle_flag(VERIFIED_NAME_FLAG, active=True)
|
||||
@ddt.data((True, VerifiedNameStatus.APPROVED),
|
||||
(True, VerifiedNameStatus.DENIED),
|
||||
(False, VerifiedNameStatus.PENDING))
|
||||
@ddt.unpack
|
||||
def test_certificate_view_verified_name(self, should_use_verified_name_for_certs, status):
|
||||
"""
|
||||
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.
|
||||
Test that if 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, status=status)
|
||||
|
||||
@@ -5,7 +5,6 @@ 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
|
||||
@@ -242,7 +241,7 @@ def get_preferred_certificate_name(user):
|
||||
"""
|
||||
name_to_use = student_api.get_name(user.id)
|
||||
|
||||
if is_verified_name_enabled() and should_use_verified_name_for_certs(user):
|
||||
if 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
|
||||
|
||||
@@ -21,9 +21,8 @@ from django.test.client import Client, RequestFactory
|
||||
from django.test.utils import override_settings
|
||||
from django.urls import reverse
|
||||
from django.utils.timezone import now
|
||||
from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG
|
||||
from opaque_keys.edx.locator import CourseLocator
|
||||
from waffle.testutils import override_flag, override_switch
|
||||
from waffle.testutils import override_switch
|
||||
|
||||
from common.djangoapps.course_modes.models import CourseMode
|
||||
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
|
||||
@@ -1259,20 +1258,17 @@ class TestSubmitPhotosForVerification(MockS3BotoMixin, TestVerificationBase):
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_submit_photos_and_change_name(self, flag_on):
|
||||
with override_flag(VERIFIED_NAME_FLAG.name, flag_on):
|
||||
# Submit the photos, along with a name change
|
||||
self._submit_photos(
|
||||
face_image=self.IMAGE_DATA,
|
||||
photo_id_image=self.IMAGE_DATA,
|
||||
full_name=self.FULL_NAME
|
||||
)
|
||||
# Submit the photos, along with a name change
|
||||
self._submit_photos(
|
||||
face_image=self.IMAGE_DATA,
|
||||
photo_id_image=self.IMAGE_DATA,
|
||||
full_name=self.FULL_NAME
|
||||
)
|
||||
|
||||
# Check that the user's name was changed in the database if verified_name is off
|
||||
self._assert_user_name(self.FULL_NAME, equality=not flag_on)
|
||||
# Since we are giving a full name, it should be written into the attempt
|
||||
# whether or not the user name was updated
|
||||
attempt = SoftwareSecurePhotoVerification.objects.get(user=self.user)
|
||||
self.assertEqual(attempt.name, self.FULL_NAME)
|
||||
# Since we are giving a full name, it should be written into the attempt
|
||||
# whether or not the user name was updated
|
||||
attempt = SoftwareSecurePhotoVerification.objects.get(user=self.user)
|
||||
self.assertEqual(attempt.name, self.FULL_NAME)
|
||||
|
||||
def test_submit_photos_sends_confirmation_email(self):
|
||||
self._submit_photos(
|
||||
@@ -1363,15 +1359,6 @@ class TestSubmitPhotosForVerification(MockS3BotoMixin, TestVerificationBase):
|
||||
}
|
||||
self._submit_photos(expected_status_code=status_code, **params)
|
||||
|
||||
def test_invalid_name(self):
|
||||
response = self._submit_photos(
|
||||
face_image=self.IMAGE_DATA,
|
||||
photo_id_image=self.IMAGE_DATA,
|
||||
full_name="",
|
||||
expected_status_code=400
|
||||
)
|
||||
assert response.content.decode('utf-8') == 'Name must be at least 1 character long.'
|
||||
|
||||
def test_missing_required_param(self):
|
||||
# Missing face image parameter
|
||||
params = {
|
||||
|
||||
@@ -21,7 +21,6 @@ from django.utils.translation import ugettext_lazy
|
||||
from django.views.decorators.csrf import csrf_exempt
|
||||
from django.views.decorators.http import require_POST
|
||||
from django.views.generic.base import View
|
||||
from edx_name_affirmation.toggles import is_verified_name_enabled
|
||||
from edx_rest_api_client.exceptions import SlumberBaseException
|
||||
from ipware.ip import get_client_ip
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
@@ -44,9 +43,6 @@ from lms.djangoapps.verify_student.utils import can_verify_now
|
||||
from openedx.core.djangoapps.commerce.utils import ecommerce_api_client
|
||||
from openedx.core.djangoapps.embargo import api as embargo_api
|
||||
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
|
||||
from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH
|
||||
from openedx.core.djangoapps.user_api.accounts.api import update_account_settings
|
||||
from openedx.core.djangoapps.user_api.errors import AccountValidationError, UserNotFound
|
||||
from openedx.core.lib.log_utils import audit_log
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
@@ -850,12 +846,6 @@ class SubmitPhotosView(View):
|
||||
if "full_name" in params:
|
||||
full_name = params["full_name"]
|
||||
|
||||
# If necessary, update the user's full name
|
||||
if full_name is not None and not is_verified_name_enabled():
|
||||
response = self._update_full_name(request, full_name)
|
||||
if response is not None:
|
||||
return response
|
||||
|
||||
# Retrieve the image data
|
||||
# Validation ensures that we'll have a face image, but we may not have
|
||||
# a photo ID image if this is a re-verification.
|
||||
@@ -944,36 +934,6 @@ class SubmitPhotosView(View):
|
||||
|
||||
return params, None
|
||||
|
||||
def _update_full_name(self, request, full_name):
|
||||
"""
|
||||
Update the user's full name.
|
||||
|
||||
Arguments:
|
||||
user (User): The user to update.
|
||||
full_name (unicode): The user's updated full name.
|
||||
|
||||
Returns:
|
||||
error encoded as an HttpResponse or None indicating success
|
||||
|
||||
"""
|
||||
try:
|
||||
update_account_settings(request.user, {"name": full_name})
|
||||
except UserNotFound:
|
||||
log.error(("No profile found for user {user_id}").format(user_id=request.user.id))
|
||||
return HttpResponseBadRequest(_("No profile found for user"))
|
||||
except AccountValidationError:
|
||||
msg = _(
|
||||
"Name must be at least {min_length} character long."
|
||||
).format(min_length=NAME_MIN_LENGTH)
|
||||
log.error(
|
||||
(
|
||||
"User {user_id} suffered ID verification error while submitting a full_name change"
|
||||
).format(
|
||||
user_id=request.user.id
|
||||
)
|
||||
)
|
||||
return HttpResponseBadRequest(msg)
|
||||
|
||||
def _validate_and_decode_image_data(self, request, face_data, photo_id_data=None):
|
||||
"""
|
||||
Validate and decode image data sent with the request.
|
||||
|
||||
@@ -4036,7 +4036,6 @@ ACCOUNT_VISIBILITY_CONFIGURATION["admin_fields"] = (
|
||||
"phone_number",
|
||||
"activation_key",
|
||||
"pending_name_change",
|
||||
"is_verified_name_enabled",
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@ from django.core.validators import ValidationError, validate_email
|
||||
from django.utils.translation import override as override_language
|
||||
from django.utils.translation import gettext as _
|
||||
from edx_name_affirmation.name_change_validator import NameChangeValidator
|
||||
from edx_name_affirmation.toggles import is_verified_name_enabled
|
||||
from pytz import UTC
|
||||
from common.djangoapps.student import views as student_views
|
||||
from common.djangoapps.student.models import (
|
||||
@@ -281,10 +280,7 @@ def _does_name_change_require_verification(user_profile, old_name, new_name):
|
||||
|
||||
validator = NameChangeValidator(old_names_list, num_certs, old_name, new_name)
|
||||
|
||||
return (
|
||||
is_verified_name_enabled()
|
||||
and not validator.validate()
|
||||
)
|
||||
return not validator.validate()
|
||||
|
||||
|
||||
def _get_old_language_proficiencies_if_updating(user_profile, data):
|
||||
|
||||
@@ -13,7 +13,6 @@ from django.core.exceptions import ObjectDoesNotExist
|
||||
from django.urls import reverse
|
||||
from rest_framework import serializers
|
||||
|
||||
from edx_name_affirmation.toggles import is_verified_name_enabled
|
||||
|
||||
from common.djangoapps.student.models import (
|
||||
LanguageProficiency,
|
||||
@@ -170,7 +169,6 @@ class UserReadOnlySerializer(serializers.Serializer): # lint-amnesty, pylint: d
|
||||
"extended_profile_fields": None,
|
||||
"phone_number": None,
|
||||
"pending_name_change": None,
|
||||
"is_verified_name_enabled": is_verified_name_enabled(),
|
||||
}
|
||||
|
||||
if user_profile:
|
||||
|
||||
@@ -17,8 +17,6 @@ from django.http import HttpResponse
|
||||
from django.test import TestCase
|
||||
from django.test.client import RequestFactory
|
||||
from django.urls import reverse
|
||||
from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG
|
||||
from edx_toggles.toggles.testutils import override_waffle_flag
|
||||
from pytz import UTC
|
||||
from social_django.models import UserSocialAuth
|
||||
from common.djangoapps.student.models import (
|
||||
@@ -365,7 +363,6 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAc
|
||||
assert 'Valid e-mail address required.' in field_errors['email']['developer_message']
|
||||
assert 'Full Name cannot contain the following characters: < >' in field_errors['name']['user_message']
|
||||
|
||||
@override_waffle_flag(VERIFIED_NAME_FLAG, active=True)
|
||||
def test_validate_name_change_same_name(self):
|
||||
"""
|
||||
Test that saving the user's profile name without changing it should not raise an error.
|
||||
@@ -396,7 +393,6 @@ 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=False))
|
||||
@override_waffle_flag(VERIFIED_NAME_FLAG, active=True)
|
||||
def test_name_update_requires_idv(self):
|
||||
"""
|
||||
Test that a name change is blocked through this API if it requires ID verification.
|
||||
@@ -413,7 +409,6 @@ 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))
|
||||
@override_waffle_flag(VERIFIED_NAME_FLAG, active=True)
|
||||
def test_name_update_does_not_require_idv(self):
|
||||
"""
|
||||
Test that the user can change their name if change does not require IDV.
|
||||
@@ -618,7 +613,6 @@ class AccountSettingsOnCreationTest(CreateAccountMixin, TestCase):
|
||||
'course_certificates': None,
|
||||
'phone_number': None,
|
||||
'pending_name_change': None,
|
||||
'is_verified_name_enabled': False,
|
||||
}
|
||||
|
||||
def test_normalize_password(self):
|
||||
|
||||
@@ -286,7 +286,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
Verify that all account fields are returned (even those that are not shareable).
|
||||
"""
|
||||
data = response.data
|
||||
assert 30 == len(data)
|
||||
assert 29 == len(data)
|
||||
|
||||
# public fields (3)
|
||||
expected_account_privacy = (
|
||||
@@ -427,7 +427,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
"""
|
||||
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
|
||||
self.create_mock_profile(self.user)
|
||||
with self.assertNumQueries(27):
|
||||
with self.assertNumQueries(26):
|
||||
response = self.send_get(self.different_client)
|
||||
self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY)
|
||||
|
||||
@@ -442,7 +442,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
"""
|
||||
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
|
||||
self.create_mock_profile(self.user)
|
||||
with self.assertNumQueries(27):
|
||||
with self.assertNumQueries(26):
|
||||
response = self.send_get(self.different_client)
|
||||
self._verify_private_account_response(response)
|
||||
|
||||
@@ -566,7 +566,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
with self.assertNumQueries(queries):
|
||||
response = self.send_get(self.client)
|
||||
data = response.data
|
||||
assert 30 == len(data)
|
||||
assert 29 == len(data)
|
||||
assert self.user.username == data['username']
|
||||
assert ((self.user.first_name + ' ') + self.user.last_name) == data['name']
|
||||
for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"):
|
||||
@@ -589,7 +589,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
assert data['accomplishments_shared'] is False
|
||||
|
||||
self.client.login(username=self.user.username, password=TEST_PASSWORD)
|
||||
verify_get_own_information(25)
|
||||
verify_get_own_information(24)
|
||||
|
||||
# Now make sure that the user can get the same information, even if not active
|
||||
self.user.is_active = False
|
||||
@@ -609,7 +609,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
legacy_profile.save()
|
||||
|
||||
self.client.login(username=self.user.username, password=TEST_PASSWORD)
|
||||
with self.assertNumQueries(25):
|
||||
with self.assertNumQueries(24):
|
||||
response = self.send_get(self.client)
|
||||
for empty_field in ("level_of_education", "gender", "country", "state", "bio",):
|
||||
assert response.data[empty_field] is None
|
||||
@@ -965,7 +965,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
response = self.send_get(client)
|
||||
if has_full_access:
|
||||
data = response.data
|
||||
assert 30 == len(data)
|
||||
assert 29 == len(data)
|
||||
assert self.user.username == data['username']
|
||||
assert ((self.user.first_name + ' ') + self.user.last_name) == data['name']
|
||||
assert self.user.email == data['email']
|
||||
|
||||
@@ -251,9 +251,6 @@ class AccountViewSet(ViewSet):
|
||||
* pending_name_change: If the user has an active name change request, returns the
|
||||
requested name.
|
||||
|
||||
* is_verified_name_enabled: Temporary flag to control verified name field - see
|
||||
https://github.com/edx/edx-name-affirmation/blob/main/edx_name_affirmation/toggles.py
|
||||
|
||||
For all text fields, plain text instead of HTML is supported. The
|
||||
data is stored exactly as specified. Clients must HTML escape
|
||||
rendered values to avoid script injections.
|
||||
|
||||
@@ -441,7 +441,7 @@ edx-i18n-tools==0.8.1
|
||||
# via ora2
|
||||
edx-milestones==0.3.3
|
||||
# via -r requirements/edx/base.in
|
||||
edx-name-affirmation==1.0.3
|
||||
edx-name-affirmation==2.0.0
|
||||
# via -r requirements/edx/base.in
|
||||
edx-opaque-keys[django]==2.2.2
|
||||
# via
|
||||
@@ -462,7 +462,7 @@ edx-opaque-keys[django]==2.2.2
|
||||
# xmodule
|
||||
edx-organizations==6.10.0
|
||||
# via -r requirements/edx/base.in
|
||||
edx-proctoring==4.4.1
|
||||
edx-proctoring==4.5.0
|
||||
# via
|
||||
# -r requirements/edx/base.in
|
||||
# edx-proctoring-proctortrack
|
||||
|
||||
@@ -550,7 +550,7 @@ edx-lint==5.2.0
|
||||
# via -r requirements/edx/testing.txt
|
||||
edx-milestones==0.3.3
|
||||
# via -r requirements/edx/testing.txt
|
||||
edx-name-affirmation==1.0.3
|
||||
edx-name-affirmation==2.0.0
|
||||
# via -r requirements/edx/testing.txt
|
||||
edx-opaque-keys[django]==2.2.2
|
||||
# via
|
||||
@@ -571,7 +571,7 @@ edx-opaque-keys[django]==2.2.2
|
||||
# xmodule
|
||||
edx-organizations==6.10.0
|
||||
# via -r requirements/edx/testing.txt
|
||||
edx-proctoring==4.4.1
|
||||
edx-proctoring==4.5.0
|
||||
# via
|
||||
# -r requirements/edx/testing.txt
|
||||
# edx-proctoring-proctortrack
|
||||
|
||||
@@ -532,7 +532,7 @@ edx-lint==5.2.0
|
||||
# via -r requirements/edx/testing.in
|
||||
edx-milestones==0.3.3
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-name-affirmation==1.0.3
|
||||
edx-name-affirmation==2.0.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-opaque-keys[django]==2.2.2
|
||||
# via
|
||||
@@ -553,7 +553,7 @@ edx-opaque-keys[django]==2.2.2
|
||||
# xmodule
|
||||
edx-organizations==6.10.0
|
||||
# via -r requirements/edx/base.txt
|
||||
edx-proctoring==4.4.1
|
||||
edx-proctoring==4.5.0
|
||||
# via
|
||||
# -r requirements/edx/base.txt
|
||||
# edx-proctoring-proctortrack
|
||||
|
||||
Reference in New Issue
Block a user