diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index 14617a9..20f7a7e 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -6,6 +6,8 @@ waffle.Sample: ".. no_pii:": "This model has no PII" waffle.Switch: ".. no_pii:": "This model has no PII" +admin.LogEntry: + ".. no_pii:": "This model has no PII" contenttypes.ContentType: ".. no_pii:": "This model has no PII" auth.User: diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 66565bc..aeb0e0d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,12 @@ Change Log Unreleased ~~~~~~~~~~ +[0.3.0] - 2021-08-02 +~~~~~~~~~~~~~~~~~~~~ +* Add `use_verified_name_for_certs` field to the VerifiedNameView + response, and create a new endpoint to update the user's verified + name config. +* Admin page configuration for VerifiedName and VerifiedNameConfig. [0.2.0] - 2021-07-22 ~~~~~~~~~~~~~~~~~~~~ diff --git a/edx_name_affirmation/__init__.py b/edx_name_affirmation/__init__.py index 47acaf3..2a3fd40 100644 --- a/edx_name_affirmation/__init__.py +++ b/edx_name_affirmation/__init__.py @@ -2,6 +2,6 @@ Django app housing name affirmation logic. """ -__version__ = '0.2.0' +__version__ = '0.3.0' default_app_config = 'edx_name_affirmation.apps.EdxNameAffirmationConfig' # pylint: disable=invalid-name diff --git a/edx_name_affirmation/admin.py b/edx_name_affirmation/admin.py new file mode 100644 index 0000000..901aefc --- /dev/null +++ b/edx_name_affirmation/admin.py @@ -0,0 +1,34 @@ +""" +Custom Django admin pages for Name Affirmation +""" + +from django.contrib import admin + +from edx_name_affirmation.models import VerifiedName, VerifiedNameConfig + + +class VerifiedNameAdmin(admin.ModelAdmin): + """ + Admin for the VerifiedName Model + """ + list_display = ( + 'id', 'user', 'verified_name', 'verification_attempt_id', 'proctored_exam_attempt_id', + 'is_verified', 'created', 'modified', + ) + readonly_fields = ('id', 'user', 'created', 'modified') + search_fields = ('user__username', 'verification_attempt_id', 'proctored_exam_attempt_id',) + + +class VerifiedNameConfigAdmin(admin.ModelAdmin): + """ + Admin for the VerifiedNameConfig Model + """ + list_display = ( + 'id', 'user', 'use_verified_name_for_certs', 'change_date', + ) + readonly_fields = ('change_date',) + search_fields = ('user__username',) + + +admin.site.register(VerifiedName, VerifiedNameAdmin) +admin.site.register(VerifiedNameConfig, VerifiedNameConfigAdmin) diff --git a/edx_name_affirmation/api.py b/edx_name_affirmation/api.py index e8551c8..61c4f81 100644 --- a/edx_name_affirmation/api.py +++ b/edx_name_affirmation/api.py @@ -223,6 +223,12 @@ def create_verified_name_config(user, use_verified_name_for_certs=None): the user's verified name over their profile name. """ fields = {'user': user} + + # Default to the values from the most recent config + existing_config = VerifiedNameConfig.objects.filter(user=user).order_by('-change_date').first() + if existing_config: + fields['use_verified_name_for_certs'] = existing_config.use_verified_name_for_certs + if use_verified_name_for_certs is not None: fields['use_verified_name_for_certs'] = use_verified_name_for_certs diff --git a/edx_name_affirmation/serializers.py b/edx_name_affirmation/serializers.py index 8b30284..083de7e 100644 --- a/edx_name_affirmation/serializers.py +++ b/edx_name_affirmation/serializers.py @@ -3,7 +3,7 @@ from django.contrib.auth import get_user_model -from edx_name_affirmation.models import VerifiedName +from edx_name_affirmation.models import VerifiedName, VerifiedNameConfig User = get_user_model() @@ -29,3 +29,19 @@ class Meta: "created", "username", "verified_name", "profile_name", "verification_attempt_id", "proctored_exam_attempt_id", "is_verified" ) + + +class VerifiedNameConfigSerializer(serializers.ModelSerializer): + """ + Serializer for the VerifiedNameConfig Model. + """ + username = serializers.CharField(source="user.username") + use_verified_name_for_certs = serializers.BooleanField(required=False, allow_null=True) + + class Meta: + """ + Meta Class + """ + model = VerifiedNameConfig + + fields = ("change_date", "username", "use_verified_name_for_certs") diff --git a/edx_name_affirmation/tests/test_admin.py b/edx_name_affirmation/tests/test_admin.py new file mode 100644 index 0000000..1f7aba3 --- /dev/null +++ b/edx_name_affirmation/tests/test_admin.py @@ -0,0 +1,74 @@ +""" +Unit tests for the NameAffirmation admin classes +""" + + +from unittest import mock + +from django.contrib.admin.sites import AdminSite +from django.test import TestCase + +from edx_name_affirmation.admin import VerifiedNameAdmin, VerifiedNameConfigAdmin +from edx_name_affirmation.models import VerifiedName, VerifiedNameConfig + + +class NameAffirmationAdminTests(TestCase): + """ + Unit tests for the NameAffirmation admin classes + """ + + def setUp(self): + super().setUp() + self.verified_name_admin = VerifiedNameAdmin(VerifiedName, AdminSite()) + self.verified_name_config_admin = VerifiedNameConfigAdmin( + VerifiedNameConfig, AdminSite() + ) + + def test_verified_name_admin(self): + request = mock.Mock() + + expected_list_display = ( + 'id', 'user', 'verified_name', 'verification_attempt_id', 'proctored_exam_attempt_id', + 'is_verified', 'created', 'modified', + ) + self.assertEqual( + expected_list_display, + self.verified_name_admin.get_list_display(request) + ) + + expected_readonly_fields = ('id', 'user', 'created', 'modified') + self.assertEqual( + expected_readonly_fields, + self.verified_name_admin.get_readonly_fields(request) + ) + + expected_search_fields = ( + 'user__username', 'verification_attempt_id', 'proctored_exam_attempt_id', + ) + self.assertEqual( + expected_search_fields, + self.verified_name_admin.get_search_fields(request) + ) + + def test_verified_name_config_admin(self): + request = mock.Mock() + + expected_list_display = ( + 'id', 'user', 'use_verified_name_for_certs', 'change_date', + ) + self.assertEqual( + expected_list_display, + self.verified_name_config_admin.get_list_display(request) + ) + + expected_readonly_fields = ('change_date',) + self.assertEqual( + expected_readonly_fields, + self.verified_name_config_admin.get_readonly_fields(request) + ) + + expected_search_fields = ('user__username',) + self.assertEqual( + expected_search_fields, + self.verified_name_config_admin.get_search_fields(request) + ) diff --git a/edx_name_affirmation/tests/test_api.py b/edx_name_affirmation/tests/test_api.py index f5e0148..82f760e 100644 --- a/edx_name_affirmation/tests/test_api.py +++ b/edx_name_affirmation/tests/test_api.py @@ -5,6 +5,7 @@ import ddt from django.contrib.auth import get_user_model +from django.core.cache import cache from django.test import TestCase from edx_name_affirmation.api import ( @@ -40,6 +41,12 @@ def setUp(self): super().setUp() self.user = User(username='jondoe', email='jondoe@test.com') self.user.save() + # Create a fresh config with default values + VerifiedNameConfig.objects.create(user=self.user) + + def tearDown(self): + super().tearDown() + cache.clear() def test_create_verified_name_defaults(self): """ @@ -275,20 +282,18 @@ def test_create_verified_name_config(self): """ Test that verified name config is created and updated successfully """ - create_verified_name_config(self.user) - - # check that one record exists - configs = VerifiedNameConfig.objects.filter(user=self.user) - self.assertEqual(len(configs), 1) - config_obj = configs[0] - self.assertFalse(config_obj.use_verified_name_for_certs) - self.assertEqual(config_obj.user, self.user) - create_verified_name_config(self.user, use_verified_name_for_certs=True) # check that new record was created - configs = VerifiedNameConfig.objects.filter(user=self.user).order_by('change_date') - self.assertEqual(len(configs), 2) - config_obj = configs[1] + config_obj = VerifiedNameConfig.current(self.user) self.assertTrue(config_obj.use_verified_name_for_certs) self.assertEqual(config_obj.user, self.user) + + def test_create_verified_name_config_no_overwrite(self): + """ + Test that if a field is set to True, it will not be overridden by False + if not specified when the config is updated + """ + create_verified_name_config(self.user, use_verified_name_for_certs=True) + create_verified_name_config(self.user) + self.assertTrue(should_use_verified_name_for_certs(self.user)) diff --git a/edx_name_affirmation/tests/test_views.py b/edx_name_affirmation/tests/test_views.py index 25ddb87..0e61104 100644 --- a/edx_name_affirmation/tests/test_views.py +++ b/edx_name_affirmation/tests/test_views.py @@ -6,9 +6,16 @@ from edx_toggles.toggles.testutils import override_waffle_flag from django.contrib.auth import get_user_model +from django.core.cache import cache from django.urls import reverse -from edx_name_affirmation.api import create_verified_name, get_verified_name +from edx_name_affirmation.api import ( + create_verified_name, + create_verified_name_config, + get_verified_name, + should_use_verified_name_for_certs +) +from edx_name_affirmation.models import VerifiedNameConfig from edx_name_affirmation.toggles import VERIFIED_NAME_FLAG from .utils import LoggedInTestCase @@ -29,20 +36,19 @@ class VerifiedNameViewTests(LoggedInTestCase): ATTEMPT_ID = 11111 + def setUp(self): + super().setUp() + # Create a fresh config with default values + VerifiedNameConfig.objects.create(user=self.user) + + def tearDown(self): + super().tearDown() + cache.clear() + def test_verified_name(self): - create_verified_name(self.user, self.VERIFIED_NAME, self.PROFILE_NAME, is_verified=True) - verified_name = get_verified_name(self.user, is_verified=True) + verified_name = self._create_verified_name(is_verified=True) - expected_data = { - 'created': verified_name.created.isoformat(), - 'username': self.user.username, - 'verified_name': verified_name.verified_name, - 'profile_name': verified_name.profile_name, - 'verification_attempt_id': verified_name.verification_attempt_id, - 'proctored_exam_attempt_id': verified_name.proctored_exam_attempt_id, - 'is_verified': verified_name.is_verified, - 'verified_name_enabled': False, - } + expected_data = self._get_expected_data(self.user, verified_name) response = self.client.get(reverse('edx_name_affirmation:verified_name')) self.assertEqual(response.status_code, 200) @@ -51,25 +57,24 @@ def test_verified_name(self): @override_waffle_flag(VERIFIED_NAME_FLAG, active=True) def test_verified_name_feature_enabled(self): - create_verified_name(self.user, self.VERIFIED_NAME, self.PROFILE_NAME, is_verified=True) - verified_name = get_verified_name(self.user, is_verified=True) + verified_name = self._create_verified_name(is_verified=True) - expected_data = { - 'created': verified_name.created.isoformat(), - 'username': self.user.username, - 'verified_name': verified_name.verified_name, - 'profile_name': verified_name.profile_name, - 'verification_attempt_id': verified_name.verification_attempt_id, - 'proctored_exam_attempt_id': verified_name.proctored_exam_attempt_id, - 'is_verified': verified_name.is_verified, - 'verified_name_enabled': True, - } + expected_data = self._get_expected_data(self.user, verified_name, verified_name_enabled=True) response = self.client.get(reverse('edx_name_affirmation:verified_name')) self.assertEqual(response.status_code, 200) data = json.loads(response.content.decode('utf-8')) self.assertEqual(data, expected_data) + def test_verified_name_existing_config(self): + verified_name = self._create_verified_name(is_verified=True) + create_verified_name_config(self.user, use_verified_name_for_certs=True) + expected_data = self._get_expected_data(self.user, verified_name, use_verified_name_for_certs=True) + + response = self.client.get(reverse('edx_name_affirmation:verified_name')) + data = json.loads(response.content.decode('utf-8')) + self.assertTrue(data, expected_data) + def test_staff_access_verified_name(self): other_user = User(username='other_tester', email='other@test.com') other_user.save() @@ -83,20 +88,11 @@ def test_staff_access_verified_name(self): self.user.save() # create verified name - create_verified_name(self.user, self.VERIFIED_NAME, self.PROFILE_NAME, is_verified=False) + self._create_verified_name() other_user_verified_name = get_verified_name(other_user, is_verified=True) # expected data should match the verifiedname from the other user - expected_data = { - 'created': other_user_verified_name.created.isoformat(), - 'username': other_user.username, - 'verified_name': other_user_verified_name.verified_name, - 'profile_name': other_user_verified_name.profile_name, - 'verification_attempt_id': other_user_verified_name.verification_attempt_id, - 'proctored_exam_attempt_id': other_user_verified_name.proctored_exam_attempt_id, - 'is_verified': other_user_verified_name.is_verified, - 'verified_name_enabled': False, - } + expected_data = self._get_expected_data(other_user, other_user_verified_name) response = self.client.get(reverse('edx_name_affirmation:verified_name'), {'username': other_user.username}) self.assertEqual(response.status_code, 200) @@ -196,3 +192,135 @@ def test_post_400_two_attempt_ids(self): verified_name_data ) self.assertEqual(response.status_code, 400) + + def _create_verified_name( + self, verification_attempt_id=None, proctored_exam_attempt_id=None, is_verified=False, + ): + """ + Create and return a verified name object. + """ + create_verified_name( + self.user, + self.VERIFIED_NAME, + self.PROFILE_NAME, + verification_attempt_id, + proctored_exam_attempt_id, + is_verified + ) + return get_verified_name(self.user) + + def _get_expected_data( + self, user, verified_name_obj, + use_verified_name_for_certs=False, verified_name_enabled=False, + ): + """ + Create a dictionary of expected data. + """ + return { + 'created': verified_name_obj.created.isoformat(), + 'username': user.username, + 'verified_name': verified_name_obj.verified_name, + 'profile_name': verified_name_obj.profile_name, + 'verification_attempt_id': verified_name_obj.verification_attempt_id, + 'proctored_exam_attempt_id': verified_name_obj.proctored_exam_attempt_id, + 'is_verified': verified_name_obj.is_verified, + 'use_verified_name_for_certs': use_verified_name_for_certs, + 'verified_name_enabled': verified_name_enabled + } + + +class VerifiedNameConfigViewTests(LoggedInTestCase): + """ + Tests for the VerifiedNameConfigView + """ + + def setUp(self): + super().setUp() + # Create a fresh config with default values + VerifiedNameConfig.objects.create(user=self.user) + + def tearDown(self): + super().tearDown() + cache.clear() + + def test_post_201(self): + config_data = { + 'username': self.user.username, + 'use_verified_name_for_certs': True + } + response = self.client.post( + reverse('edx_name_affirmation:verified_name_config'), + config_data + ) + self.assertEqual(response.status_code, 201) + + use_verified_name_for_certs = should_use_verified_name_for_certs(self.user) + self.assertTrue(use_verified_name_for_certs) + + def test_post_201_missing_field(self): + initial_config_data = { + 'username': self.user.username, + 'use_verified_name_for_certs': True + } + config_data_missing_field = {'username': self.user.username} + + first_response = self.client.post( + reverse('edx_name_affirmation:verified_name_config'), + initial_config_data + ) + second_response = self.client.post( + reverse('edx_name_affirmation:verified_name_config'), + config_data_missing_field + ) + + self.assertEqual(first_response.status_code, 201) + self.assertEqual(second_response.status_code, 201) + + # `use_verified_name_for_certs` should not be overriden with False due to a missing field + use_verified_name_for_certs = should_use_verified_name_for_certs(self.user) + self.assertTrue(use_verified_name_for_certs) + + def test_post_201_if_staff(self): + self.user.is_staff = True + self.user.save() + + other_user = User(username='other_user', email='other@test.com') + other_user.save() + + config_data = { + 'username': other_user.username, + 'use_verified_name_for_certs': True + } + response = self.client.post( + reverse('edx_name_affirmation:verified_name_config'), + config_data + ) + self.assertEqual(response.status_code, 201) + + use_verified_name_for_certs = should_use_verified_name_for_certs(other_user) + self.assertTrue(use_verified_name_for_certs) + + def test_post_403_non_staff(self): + other_user = User(username='other_tester', email='other@test.com') + other_user.save() + + config_data = { + 'username': other_user.username, + 'use_verified_name_for_certs': True + } + response = self.client.post( + reverse('edx_name_affirmation:verified_name_config'), + config_data + ) + self.assertEqual(response.status_code, 403) + + def test_post_400_invalid_serializer(self): + config_data = { + 'username': self.user.username, + 'use_verified_name_for_certs': 'not a boolean' + } + response = self.client.post( + reverse('edx_name_affirmation:verified_name_config'), + config_data + ) + self.assertEqual(response.status_code, 400) diff --git a/edx_name_affirmation/urls.py b/edx_name_affirmation/urls.py index a31f3f6..e7b5172 100644 --- a/edx_name_affirmation/urls.py +++ b/edx_name_affirmation/urls.py @@ -13,5 +13,12 @@ views.VerifiedNameView.as_view(), name='verified_name' ), + + url( + r'edx_name_affirmation/v1/verified_name/config$', + views.VerifiedNameConfigView.as_view(), + name='verified_name_config' + ), + url(r'^', include('rest_framework.urls', namespace='rest_framework')), ] diff --git a/edx_name_affirmation/views.py b/edx_name_affirmation/views.py index e1dcee8..7459e05 100644 --- a/edx_name_affirmation/views.py +++ b/edx_name_affirmation/views.py @@ -11,9 +11,14 @@ from django.contrib.auth import get_user_model -from edx_name_affirmation.api import create_verified_name, get_verified_name +from edx_name_affirmation.api import ( + create_verified_name, + create_verified_name_config, + get_verified_name, + should_use_verified_name_for_certs +) from edx_name_affirmation.exceptions import VerifiedNameMultipleAttemptIds -from edx_name_affirmation.serializers import VerifiedNameSerializer +from edx_name_affirmation.serializers import VerifiedNameConfigSerializer, VerifiedNameSerializer from edx_name_affirmation.toggles import is_verified_name_enabled @@ -49,6 +54,16 @@ class VerifiedNameView(AuthenticatedAPIView): ** Scenarios ** ?username=jdoe returns an existing verified name object matching the username + Example response: { + "username": "jdoe", + "verified_name": "Jonathan Doe", + "profile_name": "Jon Doe", + "verification_attempt_id": 123, + "proctored_exam_attempt_id": None, + "is_verified": True, + "use_verified_name_for_certs": False, + "verified_name_enabled": True + } """ def get(self, request): """ @@ -69,9 +84,11 @@ def get(self, request): data={'detail': 'There is no verified name related to this user.'} ) + use_verified_name_for_certs = should_use_verified_name_for_certs(user) serialized_data = VerifiedNameSerializer(verified_name).data serialized_data['verified_name_enabled'] = is_verified_name_enabled() + serialized_data['use_verified_name_for_certs'] = use_verified_name_for_certs return Response(serialized_data) def post(self, request): @@ -106,3 +123,45 @@ def post(self, request): response_status = status.HTTP_400_BAD_REQUEST data = serializer.errors return Response(status=response_status, data=data) + + +class VerifiedNameConfigView(AuthenticatedAPIView): + """ + Endpoint for VerifiedNameConfig. + /edx_name_affirmation/v1/verified_name/config + + Supports: + HTTP POST: Creates a new VerifiedNameConfig. + + HTTP POST + Creates a new VerifiedName. + Example POST data: { + "username": "jdoe", + "use_verified_name_for_certs": True + } + """ + def post(self, request): + """ + Create VerifiedNameConfig + """ + username = request.data.get('username') + if username != request.user.username and not request.user.is_staff: + msg = 'Must be a staff user to override the requested user’s VerifiedNameConfig value' + return Response(status=status.HTTP_403_FORBIDDEN, data={'detail': msg}) + + serializer = VerifiedNameConfigSerializer(data=request.data) + + if serializer.is_valid(): + user = get_user_model().objects.get(username=username) if username else request.user + create_verified_name_config( + user, + use_verified_name_for_certs=request.data.get('use_verified_name_for_certs'), + ) + response_status = status.HTTP_201_CREATED + data = {} + + else: + response_status = status.HTTP_400_BAD_REQUEST + data = serializer.errors + + return Response(status=response_status, data=data) diff --git a/test_settings.py b/test_settings.py index d81602e..6d1ee7f 100644 --- a/test_settings.py +++ b/test_settings.py @@ -28,6 +28,7 @@ def root(*args): INSTALLED_APPS = ( 'config_models', + 'django.contrib.admin', 'django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions',