Skip to content

Commit

Permalink
fix: remove all references to exam attempt student name
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ashultz0 committed Jul 21, 2021
1 parent 5490500 commit f496c1c
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 87 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion edx_proctoring/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
17 changes: 3 additions & 14 deletions edx_proctoring/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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
Expand All @@ -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):
"""
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
22 changes: 9 additions & 13 deletions edx_proctoring/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand All @@ -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):
"""
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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):
Expand All @@ -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)
)

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -422,7 +418,7 @@ def test_exam_attempt_is_resumable(self):
# Create a user and their attempt
user = User.objects.create(username='testerresumable', email='[email protected]')
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'
)

Expand Down
43 changes: 3 additions & 40 deletions edx_proctoring/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
)

Expand Down Expand Up @@ -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()
Expand Down
17 changes: 1 addition & 16 deletions edx_proctoring/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@
ProctoredExamSoftwareSecureReview,
ProctoredExamStudentAllowance,
ProctoredExamStudentAllowanceHistory,
ProctoredExamStudentAttempt,
ProctoredExamStudentAttemptHistory
ProctoredExamStudentAttempt
)
from edx_proctoring.runtime import get_runtime_service
from edx_proctoring.serializers import (
Expand Down Expand Up @@ -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 """
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down

0 comments on commit f496c1c

Please sign in to comment.