Introduce id verification step for proctored exams #309

andy-armstrong · 2016-09-23T01:52:12Z

TNL-5083

Description

This change introduces a new id verification step when rendering a proctored exam that is shown when the user has not yet passed verification. It also updates the subsequent instruction step to remove the unnecessary recommendation to ensure that the learner is verified.

Sandbox

https://id-verification.sandbox.edx.org/courses/course-v1:AndyA+AAPT+1/courseware/9b7aeac146d54fc7a552f2feb2b00f51/be3702e5245a4d6f96194449303bdbf3/

You can use the four test users which are already in the various states:

verified_id_none@example.com
verified_id_failed@example.com
verified_id_pending@example.com
verified@example.com

Testing

~~- [x] i18n~~

Unit, integration, acceptance tests as appropriate

Reviewers

If you've been tagged for review, please check your corresponding box once you've given the 👍.

FYI: @douglashall

Post-review

Rebase and squash commits

coveralls · 2016-09-23T01:55:26Z

Coverage remained the same at 100.0% when pulling 3c21e38 on andya/proctoring-verification-step-2 into 1faad3a on andya/clean-up-tests.

coveralls · 2016-09-23T01:57:05Z

Coverage remained the same at 100.0% when pulling 3c21e38 on andya/proctoring-verification-step-2 into 1faad3a on andya/clean-up-tests.

andy-armstrong · 2016-09-23T02:00:02Z

@dianakhuang @robrap here's a new version of this branch that is rebased against the test cleanup work, so it is clearer what has changed. Tomorrow hopefully we can get this working on a sandbox.

andy-armstrong · 2016-09-23T02:02:19Z

@catong this work for proctoring isn't available on a sandbox yet, but it would be great if you could review the documentation strings. They are based on the designs found in the ticket:

https://openedx.atlassian.net/browse/TNL-5083

robrap · 2016-09-23T11:43:41Z

I'll review in more detail later. I was thinking last night that we'll need a way to disable this for load testing.

robrap · 2016-09-23T13:24:05Z

@andy-armstrong @dianakhuang: It's possible that we can ignore my earlier comment about verification and load testing, because I think auto_auth was updated to allow for verification. We'll need to confirm.

robrap · 2016-09-23T13:27:53Z

@andy-armstrong: It looks like it turned out verification_status was already available?

Thanks. All looks good to me. 👍

dianakhuang · 2016-09-23T15:06:50Z

Sandbox is now available! https://dianakhuang.sandbox.edx.org/

coveralls · 2016-09-23T15:38:13Z

Coverage remained the same at 100.0% when pulling ab79cd7 on andya/proctoring-verification-step-2 into f2c320f on master.

dianakhuang · 2016-09-23T17:01:04Z

On the sandbox, there are 4 students in various states:

verified_id_none@example.com
verified_id_failed@example.com
verified_id_pending@example.com
verified@example.com

verified@example.com has an approved verification.

catong · 2016-09-23T17:52:35Z

edx_proctoring/templates/proctored_exam/id_verification.html

+    <div class="">
+        <h3>
+            {% blocktrans %}
+                Complete verification prior to starting your proctored exam.


"Complete your verification before starting the proctored exam."

catong · 2016-09-23T17:53:12Z

edx_proctoring/templates/proctored_exam/id_verification.html

+        </h3>
+        <p>
+            {% blocktrans %}
+                In order to start your proctored exam, you must successfully complete identity verification.


"You must successfully complete identity verification before you can start the proctored exam."

catong · 2016-09-23T17:54:20Z

edx_proctoring/templates/proctored_exam/id_verification.html

+        {% if verification_status == 'pending' %}
+            <p>
+                {% blocktrans %}
+                    Your verification is currently pending. You should expect to hear back in 2-3 days


"Your verification is pending. Results should be available 2-3 days after you submit your verification."

catong · 2016-09-23T17:55:34Z

edx_proctoring/templates/proctored_exam/id_verification.html

+        {% elif verification_status == 'must_reverify' %}
+            <p>
+                {% blocktrans %}
+                    Your verification attempt failed. Please retry verification after reading


"Your verification attempt failed. Please read our guidelines to make sure you understand the requirements for successfully completing verification, then try again."

catong · 2016-09-23T17:58:06Z

edx_proctoring/templates/proctored_exam/id_verification.html

+            <p>
+                {% blocktrans %}
+                    Your verification has expired. Please retry verification after reading
+                    our guidelines to ensure you successfully complete verification.


@andy-armstrong The second sentence for the "expired" scenario is identical to that for the "failed" scenario, but I think we might be able to provide more helpful info for this specific case. What does an "expired" verification attempt mean? Previously verified and a certain amount of time has elapsed?

@catong Yes, this scenario is for a user who verified too long ago. I'm not entirely sure what the expiration rules are (a year?).

Thanks @andy-armstrong. I don't think we should put a specific time frame in our messaging, but instead of "retry verification", the message should be something like: "Your verification has expired. You must successfully complete a new identity verification before you can start the proctored exam."

catong · 2016-09-23T17:58:41Z

edx_proctoring/tests/test_student_view.py

@@ -149,6 +151,42 @@ def test_get_honor_view(self):
        self.assertIsNone(rendered_response)

    @ddt.data(
+        (None, 'Make sure you have valid photo identification'),
+        ('pending', 'Your verification is currently pending'),


I think we can strike the word "currently": "Your verification is pending"

catong · 2016-09-23T18:04:30Z

edx_proctoring/templates/proctored_exam/id_verification.html

+        {% else %}
+            <p>
+                {% blocktrans %}
+                    Make sure you have valid photo identification, such as a driver's license


On the page that results after I select "Continue to Verification" there's a prominent banner that proclaims, "You need a computer that has a webcam." Should we add the webcam requirement to the previous message too?
"Make sure you are on a computer with a webcam, and that you have valid photo identification such as a driver's license or passport, before you continue."

coveralls · 2016-09-27T01:35:52Z

Coverage remained the same at 100.0% when pulling 8146226 on andya/proctoring-verification-step-2 into f2c320f on master.

andy-armstrong · 2016-09-28T14:11:02Z

@robrap @dianakhuang @catong @marcotuts this PR is ready for review.

dianakhuang

Overall, it looks good! I just had one question/comment. 👍

dianakhuang · 2016-09-27T15:06:30Z

edx_proctoring/templates/proctored_exam/instructions.html

@@ -34,11 +34,6 @@
      {% endblocktrans %}
    </p>
    <p>
-      {% blocktrans %}
-        You will be asked to verify your identity before you begin the exam. Make sure you have valid photo identification, such as a driver's license or passport, before you continue.


I'm not sure why you removed this line. I thought this was referring to the fact that the proctoring software asks for photo id again.

@dianakhuang Good catch! @andy-armstrong Perhaps we can prevent confusion by changing the wording a bit?
"You will be asked to verify your identity as part of the proctoring exam set up."?

Interesting. Removing this line was in the acceptance criteria:

Once this verification step is added, we can remove the line of text in the Proctoring Exam Instructions that reads: "You will be asked to verify your identity before you begin the exam. Make sure you have valid photo identification, such as a driver's license or passport, before you continue."
This existing step in the process will no longer need to happen within the exam workflow as it will be completed prior to start of the proctoring instructions step.

I'll add it back with the revised wording.

@catong How's this?

You will be asked to verify your identity as part of the proctoring exam set up. Make sure you are on a computer with a webcam, and that you have valid photo identification such as a driver's license or passport, before you continue.

That sounds good to me @andy-armstrong

catong · 2016-09-28T14:23:08Z

edx_proctoring/templates/proctored_exam/id_verification.html

+            </p>
+            <div>
+                <a href="{{ reverify_url }}" class="reverify-url btn-pl-primary">
+                    {% trans "Retry Verification" %}


@andy-armstrong The link for expired verifications should probably be "Continue to Verification" rather than "Retry".

catong

Just one comment about the link for expired verifications (should not be "retry"), otherwise this looks great! 👍

coveralls · 2016-09-28T16:26:36Z

Coverage remained the same at 100.0% when pulling 16e2f80 on andya/proctoring-verification-step-2 into 113ba49 on master.

andy-armstrong · 2016-09-28T16:53:25Z

@catong @dianakhuang I've addressed your feedback and squashed so I can hopefully merge this shortly.

@robrap @chris-mike @marcotuts please review today if possible. Thanks.

TNL-5083

coveralls · 2016-09-28T17:33:42Z

Coverage remained the same at 100.0% when pulling 6ef5eec on andya/proctoring-verification-step-2 into 113ba49 on master.

robrap · 2016-09-28T20:48:34Z

edx_proctoring/tests/test_views.py

@@ -414,6 +414,7 @@ def setUp(self):
        self.student_taking_exam = User()
        self.student_taking_exam.save()

+        set_runtime_service('credit', MockCreditService())


@andy-armstrong: Where is this used?

I guess this is to deal with credit_state. It seems like this would have been required already for some existing tests, but maybe I'm missing something. Anyway, I don't think this is an issue, I'm just trying to understand.

Good question. These tests call proctoring code that assumes that there's a credit service installed. They work when run as a complete suite because other tests have installed the service and it doesn't get uninstalled. I put this in so that I can run individual suites and/or test methods.

robrap · 2016-09-28T20:58:40Z

👍 Looks good. I had a question, but I don't think you have anything to do. I'm just trying to understand the test code. Thanks.

coveralls · 2016-09-29T23:14:53Z

Coverage remained the same at 100.0% when pulling a5681bc on andya/proctoring-verification-step-2 into 113ba49 on master.

andy-armstrong · 2016-09-29T23:17:22Z

@marcotuts discovered that I'd put the id verification step in the wrong place in the workflow. It was showing before the user chose to enter a proctored exam, rather than being shown immediately after they make that choice. I've fixed the code and the tests, and I've updated the sandbox. Great catch, @marcotuts!

FYI @dianakhuang @robrap @catong @chris-mike.

andy-armstrong · 2016-10-04T22:59:28Z

I got a verbal 👍 from Lauren, so I'm merging this now.

andy-armstrong force-pushed the andya/proctoring-verification-step-2 branch from 4022e67 to 3c21e38 Compare September 23, 2016 01:53

andy-armstrong force-pushed the andya/proctoring-verification-step-2 branch from 3c21e38 to ab79cd7 Compare September 23, 2016 15:34

andy-armstrong changed the base branch from andya/clean-up-tests to master September 23, 2016 15:34

catong reviewed Sep 23, 2016

View reviewed changes

dianakhuang approved these changes Sep 28, 2016

View reviewed changes

catong reviewed Sep 28, 2016

View reviewed changes

catong approved these changes Sep 28, 2016

View reviewed changes

andy-armstrong force-pushed the andya/proctoring-verification-step-2 branch from 8146226 to 16e2f80 Compare September 28, 2016 16:22

Introduce id verification step for proctored exams

6ef5eec

TNL-5083

andy-armstrong force-pushed the andya/proctoring-verification-step-2 branch from 16e2f80 to 6ef5eec Compare September 28, 2016 17:29

robrap reviewed Sep 28, 2016

View reviewed changes

andy-armstrong mentioned this pull request Sep 29, 2016

Introduce id verification step for proctored exams openedx/edx-platform#13518

Merged

8 tasks

Move id verification step after step to choose proctoring

a5681bc

andy-armstrong merged commit bd5f634 into master Oct 4, 2016

andy-armstrong deleted the andya/proctoring-verification-step-2 branch October 4, 2016 22:59

Introduce id verification step for proctored exams #309

Introduce id verification step for proctored exams #309

Conversation

andy-armstrong commented Sep 23, 2016 • edited

Description

Sandbox

Testing

Reviewers

Post-review

coveralls commented Sep 23, 2016

coveralls commented Sep 23, 2016

andy-armstrong commented Sep 23, 2016

andy-armstrong commented Sep 23, 2016

robrap commented Sep 23, 2016 • edited

robrap commented Sep 23, 2016

robrap commented Sep 23, 2016

dianakhuang commented Sep 23, 2016

coveralls commented Sep 23, 2016

dianakhuang commented Sep 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 27, 2016

andy-armstrong commented Sep 28, 2016

dianakhuang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catong left a comment

Choose a reason for hiding this comment

coveralls commented Sep 28, 2016

andy-armstrong commented Sep 28, 2016

coveralls commented Sep 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robrap commented Sep 28, 2016

coveralls commented Sep 29, 2016

andy-armstrong commented Sep 29, 2016

andy-armstrong commented Oct 4, 2016

andy-armstrong commented Sep 23, 2016 •

edited

robrap commented Sep 23, 2016 •

edited