diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 8b8f5f9eb1f..7b1eb30ccb6 100644 --- a/edx_proctoring/__init__.py +++ b/edx_proctoring/__init__.py @@ -5,6 +5,6 @@ from __future__ import absolute_import # Be sure to update the version number in edx_proctoring/package.json -__version__ = '1.5.0' +__version__ = '1.5.1' default_app_config = 'edx_proctoring.apps.EdxProctoringConfig' # pylint: disable=invalid-name diff --git a/edx_proctoring/admin.py b/edx_proctoring/admin.py index 60bb3403055..8a3cbe85678 100644 --- a/edx_proctoring/admin.py +++ b/edx_proctoring/admin.py @@ -21,7 +21,6 @@ ProctoredExamStudentAttempt, ) from edx_proctoring.api import update_attempt_status -from edx_proctoring.backends import get_backend_provider from edx_proctoring.utils import locate_attempt_by_attempt_code from edx_proctoring.exceptions import ( ProctoredExamIllegalStatusTransition, @@ -292,9 +291,6 @@ def save_model(self, request, review, form, change): # pylint: disable=argument """ review.reviewed_by = request.user review.save() - # call the review saved and since it's coming from - # the Django admin will we accept failures - get_backend_provider().on_review_saved(review, allow_rejects=True) def get_form(self, request, obj=None, **kwargs): form = super(ProctoredExamSoftwareSecureReviewAdmin, self).get_form(request, obj, **kwargs) diff --git a/edx_proctoring/signals.py b/edx_proctoring/signals.py index 764940e9ad6..024d6d65435 100644 --- a/edx_proctoring/signals.py +++ b/edx_proctoring/signals.py @@ -1,7 +1,15 @@ "edx-proctoring signals" +from crum import get_current_request + from django.db.models.signals import pre_save, post_save, pre_delete from django.dispatch import receiver +from django.urls import reverse + +from edx_proctoring import api +from edx_proctoring import constants from edx_proctoring import models +from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus, SoftwareSecureReviewStatus +from edx_proctoring.utils import emit_event, locate_attempt_by_attempt_code from edx_proctoring.backends import get_backend_provider @@ -120,3 +128,65 @@ def on_review_changed(sender, instance, signal, **kwargs): # pylint: disable=un # don't archive on create return models.archive_model(models.ProctoredExamSoftwareSecureReviewHistory, instance, id='review_id') + + +@receiver(post_save, sender=models.ProctoredExamSoftwareSecureReview) +def finish_review_workflow(sender, instance, signal, **kwargs): # pylint: disable=unused-argument + """ + Updates the attempt status based on the review status + Also notifies support about suspicious reviews. + """ + review = instance + attempt_obj, is_archived = locate_attempt_by_attempt_code(review.attempt_code) + attempt = api.ProctoredExamStudentAttemptSerializer(attempt_obj).data + + # we could have gotten a review for an archived attempt + # this should *not* cause an update in our credit + # eligibility table + if review.review_status in SoftwareSecureReviewStatus.passing_statuses: + attempt_status = ProctoredExamStudentAttemptStatus.verified + elif review.reviewed_by or not constants.REQUIRE_FAILURE_SECOND_REVIEWS: + # reviews from the django admin have a reviewer set. They should be allowed to + # reject an attempt + attempt_status = ProctoredExamStudentAttemptStatus.rejected + else: + # if we are not allowed to store 'rejected' on this + # code path, then put status into 'second_review_required' + attempt_status = ProctoredExamStudentAttemptStatus.second_review_required + + if review.review_status in SoftwareSecureReviewStatus.notify_support_for_status: + instructor_service = api.get_runtime_service('instructor') + request = get_current_request() + if instructor_service and request: + course_id = attempt['proctored_exam']['course_id'] + exam_id = attempt['proctored_exam']['id'] + review_url = request.build_absolute_uri( + u'{}?attempt={}'.format( + reverse('edx_proctoring:instructor_dashboard_exam', args=[course_id, exam_id]), + attempt['external_id'] + )) + instructor_service.send_support_notification( + course_id=attempt['proctored_exam']['course_id'], + exam_name=attempt['proctored_exam']['exam_name'], + student_username=attempt['user']['username'], + review_status=review.review_status, + review_url=review_url, + ) + + if not is_archived: + # updating attempt status will trigger workflow + # (i.e. updating credit eligibility table) + # archived attempts should not trigger the workflow + api.update_attempt_status( + attempt['proctored_exam']['id'], + attempt['user']['id'], + attempt_status, + raise_if_not_found=False + ) + + # emit an event for 'review_received' + data = { + 'review_attempt_code': review.attempt_code, + 'review_status': review.review_status, + } + emit_event(attempt['proctored_exam'], 'review_received', attempt=attempt, override_data=data) diff --git a/edx_proctoring/statuses.py b/edx_proctoring/statuses.py index 7385e1c239a..0289ad2ac1d 100644 --- a/edx_proctoring/statuses.py +++ b/edx_proctoring/statuses.py @@ -135,7 +135,7 @@ def validate(cls, status): """ Validate review status """ - if status not in [cls.passed, cls.violation, cls.suspicious, cls.not_reviewed]: + if status not in (cls.passed, cls.violation, cls.suspicious, cls.not_reviewed): raise ProctoredExamBadReviewStatus(status) return True diff --git a/edx_proctoring/tests/test_reviews.py b/edx_proctoring/tests/test_reviews.py index 72b44e019e9..eeb31c86bd3 100644 --- a/edx_proctoring/tests/test_reviews.py +++ b/edx_proctoring/tests/test_reviews.py @@ -5,6 +5,7 @@ import json +from crum import set_current_request import ddt from mock import patch @@ -55,6 +56,7 @@ def setUp(self): set_runtime_service('instructor', MockInstructorService()) set_runtime_service('grades', MockGradesService()) set_runtime_service('certificates', MockCertificateService()) + set_current_request(self.dummy_request) def tearDown(self): super(ReviewTests, self).tearDown() @@ -119,7 +121,7 @@ def test_psi_review_callback(self, psi_review_status, review_status, credit_requ with self.assertRaises(ProctoredExamBadReviewStatus): ProctoredExamReviewCallback().make_review(self.attempt, test_payload) else: - ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload) # make sure that what we have in the Database matches what we expect review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(self.attempt['attempt_code']) @@ -245,7 +247,7 @@ def test_allow_review_resubmission(self): # now call again, this will not throw exception test_payload['status'] = ReviewStatus.suspicious - ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload) # make sure that what we have in the Database matches what we expect review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(self.attempt['attempt_code']) @@ -280,7 +282,7 @@ def test_failure_submission(self): allow_rejects = not constants.REQUIRE_FAILURE_SECOND_REVIEWS # submit a Suspicious review payload - ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload) # now look at the attempt and make sure it did not # transition to failure on the callback, @@ -310,7 +312,7 @@ def test_failure_submission_rejected(self): test_payload = self.get_review_payload(ReviewStatus.suspicious) allow_rejects = not constants.REQUIRE_FAILURE_SECOND_REVIEWS # submit a Suspicious review payload - ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload) # now look at the attempt and make sure it did not # transition to failure on the callback, @@ -363,7 +365,7 @@ def test_update_archived_attempt(self): # now we'll make another review for the archived attempt. It should NOT update the status test_payload = self.get_review_payload(ReviewStatus.suspicious) self.attempt['is_archived'] = True - ProctoredExamReviewCallback().make_review(self.attempt, test_payload, request=self.dummy_request) + ProctoredExamReviewCallback().make_review(self.attempt, test_payload) attempt, is_archived = locate_attempt_by_attempt_code(self.attempt['attempt_code']) self.assertTrue(is_archived) self.assertEqual(attempt.status, 'verified') diff --git a/edx_proctoring/views.py b/edx_proctoring/views.py index a4a9554ce16..bb74b7873fa 100644 --- a/edx_proctoring/views.py +++ b/edx_proctoring/views.py @@ -71,7 +71,6 @@ get_time_remaining_for_attempt, locate_attempt_by_attempt_code, humanized_time, - emit_event, ) ATTEMPTS_PER_PAGE = 25 @@ -794,7 +793,7 @@ class BaseReviewCallback(object): Base class for review callbacks. make_review handles saving reviews and review comments. """ - def make_review(self, attempt, data, request=None, backend=None): + def make_review(self, attempt, data, backend=None): """ Save the review and review comments """ @@ -861,54 +860,6 @@ def make_review(self, attempt, data, request=None, backend=None): ) comment.save() - # we could have gotten a review for an archived attempt - # this should *not* cause an update in our credit - # eligibility table - if review.review_status in SoftwareSecureReviewStatus.passing_statuses: - attempt_status = ProctoredExamStudentAttemptStatus.verified - elif not constants.REQUIRE_FAILURE_SECOND_REVIEWS: - attempt_status = ProctoredExamStudentAttemptStatus.rejected - else: - # if we are not allowed to store 'rejected' on this - # code path, then put status into 'second_review_required' - attempt_status = ProctoredExamStudentAttemptStatus.second_review_required - - if review.review_status in SoftwareSecureReviewStatus.notify_support_for_status: - instructor_service = get_runtime_service('instructor') - if instructor_service: - course_id = attempt['proctored_exam']['course_id'] - exam_id = attempt['proctored_exam']['id'] - review_url = request.build_absolute_uri( - u'{}?attempt={}'.format( - reverse('edx_proctoring:instructor_dashboard_exam', args=[course_id, exam_id]), - attempt['external_id'] - )) - instructor_service.send_support_notification( - course_id=attempt['proctored_exam']['course_id'], - exam_name=attempt['proctored_exam']['exam_name'], - student_username=attempt['user']['username'], - review_status=review.review_status, - review_url=review_url, - ) - - if not attempt.get('is_archived', False): - # updating attempt status will trigger workflow - # (i.e. updating credit eligibility table) - # archived attempts should not trigger the workflow - update_attempt_status( - attempt['proctored_exam']['id'], - attempt['user']['id'], - attempt_status, - raise_if_not_found=False - ) - - # emit an event for 'review_received' - data = { - 'review_attempt_code': review.attempt_code, - 'review_status': review.review_status, - } - emit_event(attempt['proctored_exam'], 'review_received', attempt=attempt, override_data=data) - class ProctoredExamReviewCallback(ProctoredAPIView, BaseReviewCallback): """ @@ -922,7 +873,7 @@ def post(self, request, external_id): attempt = get_exam_attempt_by_external_id(external_id) if attempt is None: raise StudentExamAttemptDoesNotExistsException('not found') - self.make_review(attempt, request.data, request) + self.make_review(attempt, request.data) return Response(data='OK') @@ -983,7 +934,6 @@ def post(self, request): serialized['is_archived'] = is_archived self.make_review(serialized, request.data, - request, backend=provider) return Response('OK') diff --git a/package.json b/package.json index d657cd14f8a..ced351f0498 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "@edx/edx-proctoring", "//": "Be sure to update the version number in edx_proctoring/__init__.py", "//": "Note that the version format is slightly different than that of the Python version when using prereleases.", - "version": "1.5.0", + "version": "1.5.1", "main": "edx_proctoring/static/index.js", "repository": { "type": "git", diff --git a/requirements/base.in b/requirements/base.in index be01c1e6c9a..faf8e7d2b26 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -8,6 +8,7 @@ django-ipware>=1.1.0 jsonfield>=2.0.2 pytz>=2018 pycryptodomex>=3.4.7 +django-crum python-dateutil>=2.1 django-webpack-loader>=0.6.0 django-waffle diff --git a/requirements/base.txt b/requirements/base.txt index 8e6bce62584..a711c054852 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,11 +6,12 @@ # certifi==2018.11.29 # via requests chardet==3.0.4 # via requests +django-crum==0.7.3 django-ipware==2.1.0 django-model-utils==3.1.2 django-waffle==0.15.0 django-webpack-loader==0.6.0 -django==1.11.17 +django==1.11.18 djangorestframework-jwt==1.11.0 # via edx-drf-extensions djangorestframework==3.6.4 edx-django-utils==1.0.3 # via edx-drf-extensions @@ -29,7 +30,7 @@ pyjwkest==1.3.2 # via edx-drf-extensions pyjwt==1.7.1 # via djangorestframework-jwt, edx-rest-api-client pymongo==3.7.2 # via edx-opaque-keys, event-tracking python-dateutil==2.7.5 -pytz==2018.7 +pytz==2018.9 requests==2.21.0 # via edx-drf-extensions, edx-rest-api-client, pyjwkest, slumber rest-condition==1.0.3 # via edx-drf-extensions semantic-version==2.6.0 # via edx-drf-extensions diff --git a/requirements/dev.txt b/requirements/dev.txt index b83a99ba6ef..46e48cc2236 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -13,11 +13,11 @@ certifi==2018.11.29 # via requests chardet==3.0.4 # via requests click-log==0.1.8 # via edx-lint click==7.0 # via click-log, edx-lint, pip-tools -configparser==3.5.0 # via pydocstyle, pylint +configparser==3.5.0 # via importlib-metadata, pydocstyle, pylint contextlib2==0.5.5 # via importlib-metadata diff-cover==1.0.6 distlib==0.2.8 # via caniusepython3 -django==1.11.17 +django==1.11.18 docutils==0.14 # via readme-renderer edx-i18n-tools==0.4.8 edx_lint==0.5.5 @@ -25,7 +25,7 @@ enum34==1.1.6 # via astroid filelock==3.0.10 # via tox futures==3.2.0 # via caniusepython3, isort idna==2.8 # via requests -importlib-metadata==0.7 # via path.py +importlib-metadata==0.8 # via path.py inflect==2.1.0 # via jinja2-pluralize isort==4.3.4 jinja2-pluralize==0.3.0 # via diff-cover @@ -49,7 +49,7 @@ pylint-django==0.7.2 # via edx-lint pylint-plugin-utils==0.4 # via pylint-celery, pylint-django pylint==1.7.1 # via edx-lint, pylint-celery, pylint-django, pylint-plugin-utils pyparsing==2.3.0 # via packaging -pytz==2018.7 # via django +pytz==2018.9 # via django pyyaml==3.13 # via edx-i18n-tools readme-renderer==24.0 # via twine requests-toolbelt==0.8.0 # via twine @@ -61,10 +61,11 @@ snowballstemmer==1.2.1 # via pydocstyle toml==0.10.0 # via tox tox-battery==0.5.1 tox==3.6.1 -tqdm==4.28.1 # via twine +tqdm==4.29.0 # via twine twine==1.12.1 urllib3==1.24.1 # via requests -virtualenv==16.1.0 # via tox +virtualenv==16.2.0 # via tox webencodings==0.5.1 # via bleach wheel==0.32.3 wrapt==1.10.11 # via astroid +zipp==0.3.3 # via importlib-metadata diff --git a/requirements/doc.txt b/requirements/doc.txt index 24c3078fcf2..ccef18509b7 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -9,11 +9,12 @@ babel==2.6.0 # via sphinx bleach==3.0.2 # via readme-renderer certifi==2018.11.29 # via requests chardet==3.0.4 # via doc8, requests +django-crum==0.7.3 django-ipware==2.1.0 django-model-utils==3.1.2 django-waffle==0.15.0 django-webpack-loader==0.6.0 -django==1.11.17 +django==1.11.18 djangorestframework-jwt==1.11.0 # via edx-drf-extensions djangorestframework==3.6.4 doc8==0.8.0 @@ -42,11 +43,11 @@ pyjwt==1.7.1 # via djangorestframework-jwt, edx-rest-api-client pymongo==3.7.2 # via edx-opaque-keys, event-tracking pyparsing==2.3.0 # via packaging python-dateutil==2.7.5 -pytz==2018.7 +pytz==2018.9 readme-renderer==24.0 requests==2.21.0 # via edx-drf-extensions, edx-rest-api-client, pyjwkest, slumber, sphinx rest-condition==1.0.3 # via edx-drf-extensions -restructuredtext-lint==1.2.1 # via doc8 +restructuredtext-lint==1.2.2 # via doc8 semantic-version==2.6.0 # via edx-drf-extensions six==1.12.0 # via bleach, doc8, edx-drf-extensions, edx-opaque-keys, edx-sphinx-theme, packaging, pockets, pyjwkest, python-dateutil, readme-renderer, sphinx, sphinxcontrib-napoleon, stevedore slumber==0.7.1 # via edx-rest-api-client diff --git a/requirements/quality.txt b/requirements/quality.txt index ad06c84e375..681cc9d2512 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -14,7 +14,7 @@ click-log==0.1.8 # via edx-lint click==7.0 # via click-log, edx-lint configparser==3.5.0 # via pydocstyle, pylint distlib==0.2.8 # via caniusepython3 -django==1.11.17 +django==1.11.18 edx_lint==0.5.5 enum34==1.1.6 # via astroid futures==3.2.0 # via caniusepython3, isort @@ -30,7 +30,7 @@ pylint-django==0.7.2 # via edx-lint pylint-plugin-utils==0.4 # via pylint-celery, pylint-django pylint==1.7.1 # via edx-lint, pylint-celery, pylint-django, pylint-plugin-utils pyparsing==2.3.0 # via packaging -pytz==2018.7 # via django +pytz==2018.9 # via django requests==2.21.0 # via caniusepython3 singledispatch==3.4.0.3 # via astroid, pylint six==1.12.0 # via astroid, edx-lint, packaging, pydocstyle, pylint, singledispatch diff --git a/requirements/test.txt b/requirements/test.txt index 95f4ec621f8..b9ffe122f0b 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -13,6 +13,7 @@ chardet==3.0.4 # via requests cookies==2.2.1 # via responses coverage==4.5.2 # via pytest-cov ddt==1.2.0 +django-crum==0.7.3 django-ipware==2.1.0 django-model-utils==3.1.2 django-waffle==0.15.0 @@ -35,7 +36,7 @@ jsonfield==2.0.2 lazy==1.3 # via bok-choy logilab-common==1.4.2 mock==2.0.0 -more-itertools==4.3.0 # via pytest +more-itertools==5.0.0 # via pytest newrelic==4.8.0.110 # via edx-django-utils pathlib2==2.3.3 # via pytest, pytest-django pbr==5.1.1 # via mock, stevedore @@ -46,13 +47,13 @@ pycryptodomex==3.7.2 pyjwkest==1.3.2 # via edx-drf-extensions pyjwt==1.7.1 # via djangorestframework-jwt, edx-rest-api-client pymongo==3.7.2 # via edx-opaque-keys, event-tracking -pytest-cov==2.6.0 +pytest-cov==2.6.1 pytest-django==3.4.4 pytest-forked==0.2 # via pytest-xdist pytest-xdist==1.25.0 -pytest==4.0.2 # via pytest-cov, pytest-django, pytest-forked, pytest-xdist +pytest==4.1.0 # via pytest-cov, pytest-django, pytest-forked, pytest-xdist python-dateutil==2.7.5 -pytz==2018.7 +pytz==2018.9 requests==2.21.0 # via edx-drf-extensions, edx-rest-api-client, httmock, pyjwkest, responses, slumber responses==0.10.5 rest-condition==1.0.3 # via edx-drf-extensions