openedx / edx-proctoring

GNU Affero General Public License v3.0
58 stars 85 forks source link

Fill in `pii_retirement: to_be_implemented` #1241

Open kdmccormick opened 2 weeks ago

kdmccormick commented 2 weeks ago

Problem

Since the annotations were added way back in 2019, three ProctoredExamSoftwareSecure* models in this package have been annotated with pii_retirement: to_be_implemented, with a comment saying "retirement to be implemented in https://openedx.atlassian.net/browse/EDUCATOR-4776". Links:

to_be_implemented needs to be filled in with one of: ['retained', 'local_api', 'consumer_api', 'third_party']. Docs: https://docs.openedx.org/projects/openedx-proposals/en/latest/architectural-decisions/oep-0030-arch-pii-markup-and-auditing.html#docstring-annotations

Background

For a yet-to-be-determined reason, running pii_check on edx-platform master was not catching this. We are rewriting the quality checks to not use Paver and have noticed that when run locally, pii_check does indeed flag this issue. In order to get pii_check fully passing again, we'll need to resolve this violation.

code_annotations django_find_annotations --config_file .pii_annotations.yml --report_path /openedx/edx-platform/reports/pii --app_name lms --lint --coverage
Configured for report path: /openedx/edx-platform/reports/pii
Configured for source path: ./
Found safelist at .annotation_safe_list.yml. Reading.

Performing linting checks...

Search failed due to linting errors!
4 errors:
---------------------------------
celery_utils.FailedTask is annotated, but also in the safelist.
/openedx/venv/lib/python3.11/site-packages/edx_proctoring/models.py::edx_proctoring.ProctoredExamSoftwareSecureComment: "to_be_implemented" is not a valid choice for ".. pii_retirement:". Expected one of ['retained', 'local_api', 'consumer_api', 'third_party'].
/openedx/venv/lib/python3.11/site-packages/edx_proctoring/models.py::edx_proctoring.ProctoredExamSoftwareSecureReview: "to_be_implemented" is not a valid choice for ".. pii_retirement:". Expected one of ['retained', 'local_api', 'consumer_api', 'third_party'].
/openedx/venv/lib/python3.11/site-packages/edx_proctoring/models.py::edx_proctoring.ProctoredExamSoftwareSecureReviewHistory: "to_be_implemented" is not a valid choice for ".. pii_retirement:". Expected one of ['retained', 'local_api', 'consumer_api', 'third_party'].
kdmccormick commented 4 days ago

@nsprenkle , we need this fix to unblock edx-platform maintenance work. I am going to set the retirement values to retained, under the pessimistic assumption that this PII is not being cleaned up when an account is retired. I'll link back to to this ticket, which I'll keep open in case you'd like to change the annotation value later.