Skip to content

Commit

Permalink
Merge pull request #912 from edx/ashultz0/rm-proctor-attempt-name
Browse files Browse the repository at this point in the history
fix: remove all references to exam attempt student name
  • Loading branch information
ashultz0 committed Jul 22, 2021
2 parents 5490500 + f496c1c commit 68827c0
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 68827c0

Please sign in to comment.