From f496c1ce4490614336d743096b6d51b5e1a7feeb Mon Sep 17 00:00:00 2001 From: Andy Shultz Date: Tue, 20 Jul 2021 14:48:19 -0400 Subject: [PATCH] fix: remove all references to exam attempt student name This is the second removal step after making it nullable. The last will migrate it out of existence. Name has never actually been used and has no live data. Some tests used the student name to check that data had been updated, those were switched over to use external ID instead. MST-872 --- CHANGELOG.rst | 4 +++ edx_proctoring/__init__.py | 2 +- edx_proctoring/api.py | 1 - edx_proctoring/models.py | 17 ++---------- edx_proctoring/serializers.py | 2 +- edx_proctoring/tests/test_models.py | 22 ++++++--------- edx_proctoring/tests/test_views.py | 43 ++--------------------------- edx_proctoring/views.py | 17 +----------- package.json | 2 +- 9 files changed, 23 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d451d07425d..865a179b79f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,10 @@ Change Log Unreleased ~~~~~~~~~~ +[3.20.3] - 2021-07-21 +~~~~~~~~~~~~~~~~~~~~~ +* Removed use of name field in proctored exam attempt. + [3.20.2] - 2021-07-21 ~~~~~~~~~~~~~~~~~~~~~ * Removed IP fields in proctored exam attempt from the DB diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 9dba8e72bfe..2d66dd56100 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -3,6 +3,6 @@ """ # Be sure to update the version number in edx_proctoring/package.json -__version__ = '3.20.2' +__version__ = '3.20.3' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 25131aee4d2..0f691e423f8 100644 --- a/edx_proctoring/api.py +++ b/edx_proctoring/api.py @@ -1060,7 +1060,6 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False): attempt = ProctoredExamStudentAttempt.create_exam_attempt( exam_id, user_id, - '', # student name is TBD attempt_code, taking_as_proctored, exam['is_practice_exam'], diff --git a/edx_proctoring/models.py b/edx_proctoring/models.py index c6cd7f1f632..f63c27d26a9 100644 --- a/edx_proctoring/models.py +++ b/edx_proctoring/models.py @@ -364,9 +364,7 @@ class ProctoredExamStudentAttempt(TimeStampedModel): Information about the Student Attempt on a Proctored Exam. - .. pii: new attempts log the student's name - .. pii_types: name - .. pii_retirement: local_api + .. no_pii: """ objects = ProctoredExamStudentAttemptManager() @@ -401,9 +399,6 @@ class ProctoredExamStudentAttempt(TimeStampedModel): # the proctoring software is_sample_attempt = models.BooleanField(default=False, verbose_name=ugettext_noop("Is Sample Attempt")) - # Note - this is currently unset - student_name = models.CharField(max_length=255, null=True) - # what review policy was this exam submitted under # Note that this is not a foreign key because # this ID might point to a record that is in the History table @@ -428,7 +423,7 @@ class Meta: verbose_name = 'proctored exam attempt' @classmethod - def create_exam_attempt(cls, exam_id, user_id, student_name, attempt_code, + def create_exam_attempt(cls, exam_id, user_id, attempt_code, taking_as_proctored, is_sample_attempt, external_id, review_policy_id=None, status=None, time_remaining_seconds=None): """ @@ -439,7 +434,6 @@ def create_exam_attempt(cls, exam_id, user_id, student_name, attempt_code, return cls.objects.create( proctored_exam_id=exam_id, user_id=user_id, - student_name=student_name, attempt_code=attempt_code, taking_as_proctored=taking_as_proctored, is_sample_attempt=is_sample_attempt, @@ -461,9 +455,7 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel): This should be the same schema as ProctoredExamStudentAttempt but will record (for audit history) all entries that have been updated. - .. pii: new attempts log the student's name - .. pii_types: name - .. pii_retirement: local_api + .. no_pii: """ user = models.ForeignKey(USER_MODEL, db_index=True, on_delete=models.CASCADE) @@ -498,9 +490,6 @@ class ProctoredExamStudentAttemptHistory(TimeStampedModel): # the proctoring software is_sample_attempt = models.BooleanField(default=False) - # Note - this is currently unset - student_name = models.CharField(max_length=255, null=True) - # what review policy was this exam submitted under # Note that this is not a foreign key because # this ID might point to a record that is in the History table diff --git a/edx_proctoring/serializers.py b/edx_proctoring/serializers.py index 322f33eaf73..40e433ab944 100644 --- a/edx_proctoring/serializers.py +++ b/edx_proctoring/serializers.py @@ -95,7 +95,7 @@ class Meta: "id", "created", "modified", "user", "started_at", "completed_at", "external_id", "status", "proctored_exam", "allowed_time_limit_mins", "attempt_code", "is_sample_attempt", "taking_as_proctored", - "review_policy_id", "student_name", "is_status_acknowledged", + "review_policy_id", "is_status_acknowledged", "time_remaining_seconds", "is_resumable" ) diff --git a/edx_proctoring/tests/test_models.py b/edx_proctoring/tests/test_models.py index 568c55bf848..1e50179f7ae 100644 --- a/edx_proctoring/tests/test_models.py +++ b/edx_proctoring/tests/test_models.py @@ -220,12 +220,11 @@ def test_delete_proctored_exam_attempt(self): # pylint: disable=invalid-name attempt = ProctoredExamStudentAttempt.objects.create( proctored_exam_id=proctored_exam.id, user_id=1, - student_name="John. D", allowed_time_limit_mins=10, attempt_code="123456", taking_as_proctored=True, is_sample_attempt=True, - external_id=1 + external_id="external" ) # No entry in the History table on creation of the Allowance entry. @@ -239,18 +238,17 @@ def test_delete_proctored_exam_attempt(self): # pylint: disable=invalid-name # make sure we can ready it back with helper class method deleted_item = ProctoredExamStudentAttemptHistory.get_exam_attempt_by_code("123456") - self.assertEqual(deleted_item.student_name, "John. D") + self.assertEqual(deleted_item.external_id, "external") - # re-create and delete again using same attempt_cde + # re-create and delete again using same attempt_code attempt = ProctoredExamStudentAttempt.objects.create( proctored_exam_id=proctored_exam.id, user_id=1, - student_name="John. D Updated", allowed_time_limit_mins=10, attempt_code="123456", taking_as_proctored=True, is_sample_attempt=True, - external_id=1 + external_id="updated" ) attempt.delete_exam_attempt() @@ -259,7 +257,7 @@ def test_delete_proctored_exam_attempt(self): # pylint: disable=invalid-name self.assertEqual(len(attempt_history), 2) deleted_item = ProctoredExamStudentAttemptHistory.get_exam_attempt_by_code("123456") - self.assertEqual(deleted_item.student_name, "John. D Updated") + self.assertEqual(deleted_item.external_id, "updated") def test_update_proctored_exam_attempt(self): """ @@ -276,7 +274,6 @@ def test_update_proctored_exam_attempt(self): proctored_exam_id=proctored_exam.id, user_id=1, status=ProctoredExamStudentAttemptStatus.created, - student_name="John. D", allowed_time_limit_mins=10, attempt_code="123456", taking_as_proctored=True, @@ -289,7 +286,7 @@ def test_update_proctored_exam_attempt(self): self.assertEqual(len(attempt_history), 0) # re-saving, but not changing status should not make an archive copy - attempt.student_name = 'John. D Updated' + attempt.external_id = "changed" attempt.save() attempt_history = ProctoredExamStudentAttemptHistory.objects.filter(user_id=1) @@ -304,7 +301,7 @@ def test_update_proctored_exam_attempt(self): # make sure we can ready it back with helper class method updated_item = ProctoredExamStudentAttemptHistory.get_exam_attempt_by_code("123456") - self.assertEqual(updated_item.student_name, "John. D Updated") + self.assertEqual(updated_item.external_id, "changed") self.assertEqual(updated_item.status, ProctoredExamStudentAttemptStatus.created) def test_get_exam_attempts(self): @@ -324,7 +321,7 @@ def test_get_exam_attempts(self): for i in range(90): user = User.objects.create(username='tester{0}'.format(i), email='tester{0}@test.com'.format(i)) ProctoredExamStudentAttempt.create_exam_attempt( - proctored_exam.id, user.id, 'test_name{0}'.format(i), + proctored_exam.id, user.id, 'test_attempt_code{0}'.format(i), True, False, 'test_external_id{0}'.format(i) ) @@ -355,7 +352,6 @@ def test_exam_review_policy(self): attempt = ProctoredExamStudentAttempt.create_exam_attempt( proctored_exam.id, self.user.id, - 'test_name{0}'.format(self.user.id), 'test_attempt_code{0}'.format(self.user.id), True, False, @@ -422,7 +418,7 @@ def test_exam_attempt_is_resumable(self): # Create a user and their attempt user = User.objects.create(username='testerresumable', email='testerresumable@test.com') ProctoredExamStudentAttempt.create_exam_attempt( - proctored_exam.id, user.id, 'test_name_resumable', + proctored_exam.id, user.id, 'test_attempt_code_resumable', True, False, 'test_external_id_resumable' ) diff --git a/edx_proctoring/tests/test_views.py b/edx_proctoring/tests/test_views.py index 2f39ce5c6ba..b23c1113402 100644 --- a/edx_proctoring/tests/test_views.py +++ b/edx_proctoring/tests/test_views.py @@ -43,8 +43,7 @@ ProctoredExam, ProctoredExamStudentAllowance, ProctoredExamStudentAllowanceHistory, - ProctoredExamStudentAttempt, - ProctoredExamStudentAttemptHistory + ProctoredExamStudentAttempt ) from edx_proctoring.runtime import get_runtime_service, set_runtime_service from edx_proctoring.serializers import ProctoredExamSerializer, ProctoredExamStudentAllowanceSerializer @@ -2585,7 +2584,7 @@ def test_attempt_ready_to_start(self): time_limit_mins=90 ) attempt = ProctoredExamStudentAttempt.create_exam_attempt( - proctored_exam.id, self.user.id, 'test_user', + proctored_exam.id, self.user.id, 'test_attempt_code', True, False, 'test_external_id' ) attempt.status = ProctoredExamStudentAttemptStatus.ready_to_start @@ -3362,7 +3361,7 @@ def test_paginated_exam_attempts(self): for i in range(90): user = User.objects.create(username='student{0}'.format(i), email='student{0}@test.com'.format(i)) ProctoredExamStudentAttempt.create_exam_attempt( - proctored_exam.id, user.id, 'test_name{0}'.format(i), + proctored_exam.id, user.id, 'test_attempt_code{0}'.format(i), True, False, 'test_external_id{0}'.format(i) ) @@ -5768,42 +5767,6 @@ def test_retire_user_no_data(self): assert response.status_code == 204 - def test_retire_user_exam_attempt(self): - """ Retiring a user should obfuscate PII for exam attempts and return a 204 status """ - # Create an exam attempt - proctored_exam = self._create_proctored_exam() - ProctoredExamStudentAttempt.objects.create( - proctored_exam=proctored_exam, - user=self.user_to_retire, - student_name='me', - ) - - # Run the retirement command - deletion_url = reverse('edx_proctoring:user_retirement_api', kwargs={'user_id': self.user_to_retire.id}) - response = self.client.post(deletion_url) - assert response.status_code == 204 - - retired_attempt = ProctoredExamStudentAttempt.objects.filter(user_id=self.user_to_retire.id).first() - assert retired_attempt.student_name == '' - - def test_retire_user_exam_attempt_history(self): - """ Retiring a user should obfuscate PII for exam attempt history and return a 204 status """ - # Create and archive an exam attempt so it appears in the history table - proctored_exam = self._create_proctored_exam() - ProctoredExamStudentAttemptHistory.objects.create( - proctored_exam=proctored_exam, - user=self.user_to_retire, - student_name='me', - ) - - # Run the retirement command - response = self.client.post(self.deletion_url) - assert response.status_code == 204 - - retired_attempt_history = ProctoredExamStudentAttemptHistory \ - .objects.filter(user_id=self.user_to_retire.id).first() - assert retired_attempt_history.student_name == '' - def test_retire_user_allowances(self): """ Retiring a user should delete their allowances and return a 204 """ proctored_exam = self._create_proctored_exam() diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index 28d4db7cc3b..997c8d90184 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -81,8 +81,7 @@ ProctoredExamSoftwareSecureReview, ProctoredExamStudentAllowance, ProctoredExamStudentAllowanceHistory, - ProctoredExamStudentAttempt, - ProctoredExamStudentAttemptHistory + ProctoredExamStudentAttempt ) from edx_proctoring.runtime import get_runtime_service from edx_proctoring.serializers import ( @@ -2025,19 +2024,6 @@ class UserRetirement(AuthenticatedAPIView): """ Retire user personally-identifiable information (PII) for a user """ - def _retire_exam_attempts_user_info(self, user_id): - """ Remove PII for exam attempts and exam history """ - attempts = ProctoredExamStudentAttempt.objects.filter(user_id=user_id) - if attempts: - for attempt in attempts: - attempt.student_name = '' - attempt.save() - - attempts_history = ProctoredExamStudentAttemptHistory.objects.filter(user_id=user_id) - if attempts_history: - for attempt_history in attempts_history: - attempt_history.student_name = '' - attempt_history.save() def _retire_user_allowances(self, user_id): """ Clear user allowance values """ @@ -2057,7 +2043,6 @@ def post(self, request, user_id): # pylint: disable=unused-argument return Response(status=403) code = 204 - self._retire_exam_attempts_user_info(user_id) self._retire_user_allowances(user_id) return Response(status=code) diff --git a/package.json b/package.json index 5e6de9179c5..43d83626352 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@edx/edx-proctoring", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "3.20.2", + "version": "3.20.3", "main": "edx_proctoring/static/index.js", "scripts": { "test": "gulp test"