Merge pull request #54 from edx/cdodge/credit-requirements-fulfillment

update the credit fulfillment table based on the callback from softwa…

edx_proctoring/api.py

@@ -389,41 +389,57 @@ def stop_exam_attempt(exam_id, user_id):
     """
     Marks the exam attempt as completed (sets the completed_at field and updates the record)
     """
-    exam_attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt(exam_id, user_id)
-    if exam_attempt_obj is None:
-        raise StudentExamAttemptDoesNotExistsException('Error. Trying to stop an exam that does not exist.')
-    else:
-        exam_attempt_obj.completed_at = datetime.now(pytz.UTC)
-        exam_attempt_obj.status = ProctoredExamStudentAttemptStatus.completed
-        exam_attempt_obj.save()
-        return exam_attempt_obj.id
+    return update_attempt_status(exam_id, user_id, ProctoredExamStudentAttemptStatus.completed)
 
 
 def mark_exam_attempt_timeout(exam_id, user_id):
     """
     Marks the exam attempt as timed_out
     """
-    exam_attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt(exam_id, user_id)
-    if exam_attempt_obj is None:
-        raise StudentExamAttemptDoesNotExistsException('Error. Trying to time out an exam that does not exist.')
-    else:
-        exam_attempt_obj.status = ProctoredExamStudentAttemptStatus.timed_out
-        exam_attempt_obj.save()
-        return exam_attempt_obj.id
+    return update_attempt_status(exam_id, user_id, ProctoredExamStudentAttemptStatus.timed_out)
 
 
 def mark_exam_attempt_as_ready(exam_id, user_id):
     """
     Marks the exam attemp as ready to start
     """
+    return update_attempt_status(exam_id, user_id, ProctoredExamStudentAttemptStatus.ready_to_start)
+
+
+def update_attempt_status(exam_id, user_id, to_status):
+    """
+    Internal helper to handle state transitions of attempt status
+    """
 
     exam_attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt(exam_id, user_id)
     if exam_attempt_obj is None:
-        raise StudentExamAttemptDoesNotExistsException('Error. Trying to time out an exam that does not exist.')
-    else:
-        exam_attempt_obj.status = ProctoredExamStudentAttemptStatus.ready_to_start
-        exam_attempt_obj.save()
-        return exam_attempt_obj.id
+        raise StudentExamAttemptDoesNotExistsException('Error. Trying to look up an exam that does not exist.')
+
+    exam_attempt_obj.status = to_status
+    exam_attempt_obj.save()
+
+    # trigger workflow, as needed
+    credit_service = get_runtime_service('credit')
+
+    # see if the status transition this changes credit requirement status
+    update_credit = to_status in [
+        ProctoredExamStudentAttemptStatus.verified, ProctoredExamStudentAttemptStatus.rejected,
+        ProctoredExamStudentAttemptStatus.declined, ProctoredExamStudentAttemptStatus.not_reviewed
+    ]
+
+    if update_credit:
+        exam = get_exam_by_id(exam_id)
+        verification = 'satisfied' if to_status == ProctoredExamStudentAttemptStatus.verified \
+            else 'failed'
+        credit_service.set_credit_requirement_status(
+            user_id=exam_attempt_obj.user_id,
+            course_key_or_id=exam['course_id'],
+            req_namespace='proctored_exam',
+            req_name='proctored_exam_id:{exam_id}'.format(exam_id=exam_id),
+            status=verification
+        )
+
+    return exam_attempt_obj.id
 
 
 def remove_exam_attempt(attempt_id):

edx_proctoring/backends/software_secure.py

@@ -18,13 +18,15 @@ from edx_proctoring.exceptions import (
     StudentExamAttemptDoesNotExistsException,
     ProctoredExamSuspiciousLookup,
     ProctoredExamReviewAlreadyExists,
+    ProctoredExamBadReviewStatus,
 )
 
 from edx_proctoring. models import (
     ProctoredExamSoftwareSecureReview,
     ProctoredExamSoftwareSecureComment,
     ProctoredExamStudentAttempt,
-    ProctoredExamStudentAttemptHistory
+    ProctoredExamStudentAttemptHistory,
+    ProctoredExamStudentAttemptStatus,
 )
 
 log = logging.getLogger(__name__)
@@ -130,6 +132,20 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         # what we consider the attempt_code is SoftwareSecure's 'examCode'
         attempt_code = payload['examMetaData']['examCode']
 
+        # get the SoftwareSecure status on this attempt
+        review_status = payload['reviewStatus']
+
+        bad_status = review_status not in [
+            'Not Reviewed', 'Suspicious', 'Rules Violation', 'Clean'
+        ]
+
+        if bad_status:
+            err_msg = (
+                'Received unexpected reviewStatus field calue from payload. '
+                'Was {review_status}.'.format(review_status=review_status)
+            )
+            raise ProctoredExamBadReviewStatus(err_msg)
+
         # do a lookup on the attempt by examCode, and compare the
         # passed in ssiRecordLocator and make sure it matches
         # what we recorded as the external_id. We need to look in both
@@ -137,9 +153,11 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
 
         attempt_obj = ProctoredExamStudentAttempt.objects.get_exam_attempt_by_code(attempt_code)
 
+        is_archived_attempt = False
         if not attempt_obj:
             # try archive table
             attempt_obj = ProctoredExamStudentAttemptHistory.get_exam_attempt_by_code(attempt_code)
+            is_archived_attempt = True
 
             if not attempt_obj:
                 # still can't find, error out
@@ -196,6 +214,28 @@ class SoftwareSecureBackendProvider(ProctoringBackendProvider):
         for comment in payload.get('desktopComments', []):
             self._save_review_comment(review, comment)
 
+        # we could have gottent a review for an archived attempt
+        # this should *not* cause an update in our credit
+        # eligibility table
+        if not is_archived_attempt:
+            # update our attempt status, note we have to import api.py here because
+            # api.py imports software_secure.py, so we'll get an import circular reference
+
+            from edx_proctoring.api import update_attempt_status
+
+            # only 'Clean' and 'Rules Violation' could as passing
+            status = (
+                ProctoredExamStudentAttemptStatus.verified
+                if review_status in ['Clean', 'Suspicious']
+                else ProctoredExamStudentAttemptStatus.rejected
+            )
+
+            update_attempt_status(
+                attempt_obj.proctored_exam_id,
+                attempt_obj.user_id,
+                status
+            )
+
     def _save_review_comment(self, review, comment):
         """
         Helper method to save a review comment

edx_proctoring/backends/tests/test_software_secure.py

@@ -3,13 +3,14 @@ Tests for the software_secure module
 """
 
 import json
+import ddt
 from string import Template  # pylint: disable=deprecated-module
 from mock import patch
 from httmock import all_requests, HTTMock
 
 from django.test import TestCase
 from django.contrib.auth.models import User
-from edx_proctoring.runtime import set_runtime_service
+from edx_proctoring.runtime import set_runtime_service, get_runtime_service
 
 from edx_proctoring.backends import get_backend_provider
 from edx_proctoring.exceptions import BackendProvideCannotRegisterAttempt
@@ -18,13 +19,13 @@ from edx_proctoring.api import (
     get_exam_attempt_by_id,
     create_exam,
     create_exam_attempt,
-    get_exam_attempt_by_id,
     remove_exam_attempt,
 )
 from edx_proctoring.exceptions import (
     StudentExamAttemptDoesNotExistsException,
     ProctoredExamSuspiciousLookup,
     ProctoredExamReviewAlreadyExists,
+    ProctoredExamBadReviewStatus
 )
 from edx_proctoring. models import (
     ProctoredExamSoftwareSecureReview,
@@ -32,6 +33,8 @@ from edx_proctoring. models import (
 )
 from edx_proctoring.backends.tests.test_review_payload import TEST_REVIEW_PAYLOAD
 
+from edx_proctoring.tests.test_services import MockCreditService
+
 
 @all_requests
 def mock_response_content(url, request):  # pylint: disable=unused-argument
@@ -57,30 +60,6 @@ def mock_response_error(url, request):  # pylint: disable=unused-argument
     }
 
 
-class MockCreditService(object):
-    """
-    Simple mock of the Credit Service
-    """
-
-    def get_credit_state(self, user_id, course_key):  # pylint: disable=unused-argument
-        """
-        Mock implementation
-        """
-
-        return {
-            'enrollment_mode': 'verified',
-            'profile_fullname': 'Wolfgang von Strucker',
-            'credit_requirement_status': []
-        }
-
-    def set_credit_requirement_status(self, user_id, course_key, req_namespace,
-                                      req_name, status="satisfied", reason=None):  # pylint: disable=unused-argument
-        """
-        Mock implementation
-        """
-        pass
-
-
 @patch(
     'django.conf.settings.PROCTORING_BACKEND_PROVIDER',
     {
@@ -96,6 +75,7 @@ class MockCreditService(object):
         }
     }
 )
+@ddt.ddt
 class SoftwareSecureTests(TestCase):
     """
     All tests for the SoftwareSecureBackendProvider
@@ -105,6 +85,7 @@ class SoftwareSecureTests(TestCase):
         """
         Initialize
         """
+        super(SoftwareSecureTests, self).setUp()
         self.user = User(username='foo', email='foo@bar.com')
         self.user.save()
 
@@ -234,9 +215,16 @@ class SoftwareSecureTests(TestCase):
         provider = get_backend_provider()
         self.assertIsNone(provider.stop_exam_attempt(None, None))
 
-    def test_review_callback(self):
+    @ddt.data(
+        ('Clean', 'satisfied'),
+        ('Suspicious', 'satisfied'),
+        ('Rules Violation', 'failed'),
+        ('Not Reviewed', 'failed'),
+    )
+    @ddt.unpack
+    def test_review_callback(self, review_status, credit_requirement_status):
         """
-        Simulates a happy path when SoftwareSecure calls us back with a payload
+        Simulates callbacks from SoftwareSecure with various statuses
         """
 
         provider = get_backend_provider()
@@ -264,6 +252,7 @@ class SoftwareSecureTests(TestCase):
             attempt_code=attempt['attempt_code'],
             external_id=attempt['external_id']
         )
+        test_payload = test_payload.replace('Clean', review_status)
 
         provider.on_review_callback(json.loads(test_payload))
 
@@ -271,7 +260,7 @@ class SoftwareSecureTests(TestCase):
         review = ProctoredExamSoftwareSecureReview.get_review_by_attempt_code(attempt['attempt_code'])
 
         self.assertIsNotNone(review)
-        self.assertEqual(review.review_status, 'Clean')
+        self.assertEqual(review.review_status, review_status)
         self.assertEqual(
             review.video_url,
             'http://www.remoteproctor.com/AdminSite/Account/Reviewer/DirectLink-Generic.aspx?ID=foo'
@@ -283,6 +272,16 @@ class SoftwareSecureTests(TestCase):
 
         self.assertEqual(len(comments), 6)
 
+        # check that we got credit requirement set appropriately
+
+        credit_service = get_runtime_service('credit')
+        credit_status = credit_service.get_credit_state(self.user.id, 'foo/bar/baz')
+
+        self.assertEqual(
+            credit_status['credit_requirement_status'][0]['status'],
+            credit_requirement_status
+        )
+
     def test_review_bad_code(self):
         """
         Asserts raising of an exception if we get a report for
@@ -298,6 +297,22 @@ class SoftwareSecureTests(TestCase):
         with self.assertRaises(StudentExamAttemptDoesNotExistsException):
             provider.on_review_callback(json.loads(test_payload))
 
+    def test_review_status_code(self):
+        """
+        Asserts raising of an exception if we get a report
+        with a reviewStatus which is unexpected
+        """
+
+        provider = get_backend_provider()
+        test_payload = Template(TEST_REVIEW_PAYLOAD).substitute(
+            attempt_code='not-here',
+            external_id='also-not-here'
+        )
+        test_payload = test_payload.replace('Clean', 'Unexpected')
+
+        with self.assertRaises(ProctoredExamBadReviewStatus):
+            provider.on_review_callback(json.loads(test_payload))
+
     def test_review_mistmatched_tokens(self):
         """
         Asserts raising of an exception if we get a report for

edx_proctoring/exceptions.py

@@ -69,3 +69,9 @@ class ProctoredExamReviewAlreadyExists(ProctoredBaseException):
     Raised when a lookup on the student attempt table does not fully match
     all expected security keys
     """
+
+
+class ProctoredExamBadReviewStatus(ProctoredBaseException):
+    """
+    Raised if we get an unexpected status back from the Proctoring attempt review status
+    """

edx_proctoring/models.py

@@ -156,6 +156,9 @@ class ProctoredExamStudentAttemptStatus(object):
     # the student is eligible to decide if he/she wants to persue credit
     eligible = 'eligible'
 
+    # the student declined to take the exam as a proctored exam
+    declined = 'declined'
+
     # the attempt record has been created, but the exam has not yet
     # been started
     created = 'created'
@@ -183,6 +186,9 @@ class ProctoredExamStudentAttemptStatus(object):
     # the exam has been rejected
     rejected = 'rejected'
 
+    # the exam was not reviewed
+    not_reviewed = 'not_reviewed'
+
     # the exam is believed to be in error
     error = 'error'
 

edx_proctoring/tests/test_services.py

+# pylint: disable=unused-argument
+
 """
 Test for the xBlock service
 """
@@ -10,6 +12,54 @@ from edx_proctoring import api as edx_proctoring_api
 import types
 
 
+class MockCreditService(object):
+    """
+    Simple mock of the Credit Service
+    """
+
+    def __init__(self):
+        """
+        Initializer
+        """
+        self.status = {
+            'enrollment_mode': 'verified',
+            'profile_fullname': 'Wolfgang von Strucker',
+            'credit_requirement_status': []
+        }
+
+    def get_credit_state(self, user_id, course_key):  # pylint: disable=unused-argument
+        """
+        Mock implementation
+        """
+
+        return self.status
+
+    def set_credit_requirement_status(self, user_id, course_key_or_id, req_namespace,
+                                      req_name, status="satisfied", reason=None):
+        """
+        Mock implementation
+        """
+
+        found = [
+            requirement
+            for requirement in self.status['credit_requirement_status']
+            if requirement['name'] == req_name and
+            requirement['namespace'] == req_namespace and
+            requirement['course_id'] == unicode(course_key_or_id)
+        ]
+
+        if not found:
+            self.status['credit_requirement_status'].append({
+                'course_id': unicode(course_key_or_id),
+                'req_namespace': req_namespace,
+                'namespace': req_namespace,
+                'name': req_name,
+                'status': status
+            })
+        else:
+            found[0]['status'] = status
+
+
 class TestProctoringService(unittest.TestCase):
     """
     Tests for ProctoringService

edx_proctoring/tests/test_views.py

@@ -28,6 +28,8 @@ from .utils import (
 from edx_proctoring.urls import urlpatterns
 from edx_proctoring.backends.tests.test_review_payload import TEST_REVIEW_PAYLOAD
 from edx_proctoring.backends.tests.test_software_secure import mock_response_content
+from edx_proctoring.tests.test_services import MockCreditService
+from edx_proctoring.runtime import set_runtime_service
 
 
 class ProctoredExamsApiTests(LoggedInTestCase):
@@ -73,6 +75,7 @@ class ProctoredExamViewTests(LoggedInTestCase):
         self.user.is_staff = True
         self.user.save()
         self.client.login_user(self.user)
+        set_runtime_service('credit', MockCreditService())
 
     def test_create_exam(self):
         """