Merge pull request #1055 from MITx/feature/victor/per-user-survey-urls

Feature/victor/per user survey urls

common/djangoapps/student/models.py

@@ -36,10 +36,12 @@ file and check it in at the same time as your model changes. To do that,
 3. Add the migration file created in mitx/common/djangoapps/student/migrations/
 """
 from datetime import datetime
+from hashlib import sha1
 import json
 import logging
 import uuid
 
+
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import models
@@ -191,6 +193,20 @@ class TestCenterUser(models.Model):
     def email(self):
         return self.user.email
 
+def unique_id_for_user(user):
+    """
+    Return a unique id for a user, suitable for inserting into
+    e.g. personalized survey links.
+
+    Currently happens to be implemented as a sha1 hash of the username
+    (and thus assumes that usernames don't change).
+    """
+    # Using the user id as the salt because it's sort of random, and is already
+    # in the db.
+    salt = str(user.id)
+    return sha1(salt + user.username).hexdigest()
+
+
 ## TODO: Should be renamed to generic UserGroup, and possibly
 # Given an optional field for type of group
 class UserTestGroup(models.Model):

common/djangoapps/student/tests.py

@@ -6,11 +6,16 @@ Replace this with more appropriate tests for your application.
 """
 import logging
 from datetime import datetime
+from hashlib import sha1
 
 from django.test import TestCase
+from mock import patch, Mock
 from nose.plugins.skip import SkipTest
 
-from .models import User, UserProfile, CourseEnrollment, replicate_user, USER_FIELDS_TO_COPY
+from .models import (User, UserProfile, CourseEnrollment,
+                     replicate_user, USER_FIELDS_TO_COPY,
+                     unique_id_for_user)
+from .views import process_survey_link, _cert_info
 
 COURSE_1 = 'edX/toy/2012_Fall'
 COURSE_2 = 'edx/full/6.002_Spring_2012'
@@ -196,8 +201,90 @@ class ReplicationTest(TestCase):
                           id=portal_user_profile.id)
 
 
-
-
-
-
-
+class CourseEndingTest(TestCase):
+    """Test things related to course endings: certificates, surveys, etc"""
+
+    def test_process_survey_link(self):
+        username = "fred"
+        user = Mock(username=username)
+        id = unique_id_for_user(user)
+        link1 = "http://www.mysurvey.com"
+        self.assertEqual(process_survey_link(link1, user), link1)
+
+        link2 = "http://www.mysurvey.com?unique={UNIQUE_ID}"
+        link2_expected = "http://www.mysurvey.com?unique={UNIQUE_ID}".format(UNIQUE_ID=id)
+        self.assertEqual(process_survey_link(link2, user), link2_expected)
+
+    def test_cert_info(self):
+        user = Mock(username="fred")
+        survey_url = "http://a_survey.com"
+        course = Mock(end_of_course_survey_url=survey_url)
+
+        self.assertEqual(_cert_info(user, course, None),
+                         {'status': 'processing',
+                          'show_disabled_download_button': False,
+                          'show_download_url': False,
+                          'show_survey_button': False,})
+
+        cert_status = {'status': 'unavailable'}
+        self.assertEqual(_cert_info(user, course, cert_status),
+                         {'status': 'processing',
+                          'show_disabled_download_button': False,
+                          'show_download_url': False,
+                          'show_survey_button': False})
+
+        cert_status = {'status': 'generating', 'grade': '67'}
+        self.assertEqual(_cert_info(user, course, cert_status),
+                         {'status': 'generating',
+                          'show_disabled_download_button': True,
+                          'show_download_url': False,
+                          'show_survey_button': True,
+                          'survey_url': survey_url,
+                          'grade': '67'
+                          })
+
+        cert_status = {'status': 'regenerating', 'grade': '67'}
+        self.assertEqual(_cert_info(user, course, cert_status),
+                         {'status': 'generating',
+                          'show_disabled_download_button': True,
+                          'show_download_url': False,
+                          'show_survey_button': True,
+                          'survey_url': survey_url,
+                          'grade': '67'
+                          })
+
+        download_url = 'http://s3.edx/cert'
+        cert_status = {'status': 'downloadable', 'grade': '67',
+                       'download_url': download_url}
+        self.assertEqual(_cert_info(user, course, cert_status),
+                         {'status': 'ready',
+                          'show_disabled_download_button': False,
+                          'show_download_url': True,
+                          'download_url': download_url,
+                          'show_survey_button': True,
+                          'survey_url': survey_url,
+                          'grade': '67'
+                          })
+
+        cert_status = {'status': 'notpassing', 'grade': '67',
+                       'download_url': download_url}
+        self.assertEqual(_cert_info(user, course, cert_status),
+                         {'status': 'notpassing',
+                          'show_disabled_download_button': False,
+                          'show_download_url': False,
+                          'show_survey_button': True,
+                          'survey_url': survey_url,
+                          'grade': '67'
+                          })
+
+        # Test a course that doesn't have a survey specified
+        course2 = Mock(end_of_course_survey_url=None)
+        cert_status = {'status': 'notpassing', 'grade': '67',
+                       'download_url': download_url}
+        self.assertEqual(_cert_info(user, course2, cert_status),
+                         {'status': 'notpassing',
+                          'show_disabled_download_button': False,
+                          'show_download_url': False,
+                          'show_survey_button': False,
+                          'grade': '67'
+                          })

common/djangoapps/student/views.py

@@ -28,7 +28,7 @@ from django.core.cache import cache
 from django_future.csrf import ensure_csrf_cookie, csrf_exempt
 from student.models import (Registration, UserProfile,
                             PendingNameChange, PendingEmailChange,
-                            CourseEnrollment)
+                            CourseEnrollment, unique_id_for_user)
 
 from certificates.models import CertificateStatuses, certificate_status_for_student
 
@@ -39,6 +39,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
 
 from datetime import date
 from collections import namedtuple
+
 from courseware.courses import get_courses_by_university
 from courseware.access import has_access
 
@@ -127,6 +128,73 @@ def press(request):
     return render_to_response('static_templates/press.html', {'articles': articles})
 
 
+def process_survey_link(survey_link, user):
+    """
+    If {UNIQUE_ID} appears in the link, replace it with a unique id for the user.
+    Currently, this is sha1(user.username).  Otherwise, return survey_link.
+    """
+    return survey_link.format(UNIQUE_ID=unique_id_for_user(user))
+
+
+def cert_info(user, course):
+    """
+    Get the certificate info needed to render the dashboard section for the given
+    student and course.  Returns a dictionary with keys:
+
+    'status': one of 'generating', 'ready', 'notpassing', 'processing'
+    'show_download_url': bool
+    'download_url': url, only present if show_download_url is True
+    'show_disabled_download_button': bool -- true if state is 'generating'
+    'show_survey_button': bool
+    'survey_url': url, only if show_survey_button is True
+    'grade': if status is not 'processing'
+    """
+    if not course.has_ended():
+        return {}
+
+    return _cert_info(user, course, certificate_status_for_student(user, course.id))
+
+def _cert_info(user, course, cert_status):
+    """
+    Implements the logic for cert_info -- split out for testing.
+    """
+    default_status = 'processing'
+    if cert_status is None:
+        return {'status': default_status,
+                'show_disabled_download_button': False,
+                'show_download_url': False,
+                'show_survey_button': False}
+
+    # simplify the status for the template using this lookup table
+    template_state = {
+        CertificateStatuses.generating: 'generating',
+        CertificateStatuses.regenerating: 'generating',
+        CertificateStatuses.downloadable: 'ready',
+        CertificateStatuses.notpassing: 'notpassing',
+        }
+
+    status = template_state.get(cert_status['status'], default_status)
+
+    d = {'status': status,
+         'show_download_url': status == 'ready',
+         'show_disabled_download_button': status == 'generating',}
+
+    if (status in ('generating', 'ready', 'notpassing') and
+        course.end_of_course_survey_url is not None):
+        d.update({
+         'show_survey_button': True,
+         'survey_url': process_survey_link(course.end_of_course_survey_url, user)})
+    else:
+        d['show_survey_button'] = False
+
+    if status == 'ready':
+        d['download_url'] = cert_status['download_url']
+
+    if status in ('generating', 'ready', 'notpassing'):
+        d['grade'] = cert_status['grade']
+
+    return d
+
 @login_required
 @ensure_csrf_cookie
 def dashboard(request):
@@ -160,12 +228,7 @@ def dashboard(request):
     show_courseware_links_for = frozenset(course.id for course in courses
                                           if has_access(request.user, course, 'load'))
 
-    # TODO: workaround to not have to zip courses and certificates in the template
-    # since before there is a migration to certificates
-    if settings.MITX_FEATURES.get('CERTIFICATES_ENABLED'):
-        cert_statuses = { course.id: certificate_status_for_student(request.user, course.id) for course in courses}
-    else:
-        cert_statuses = {}
+    cert_statuses = { course.id: cert_info(request.user, course) for course in courses}
 
     context = {'courses': courses,
                'message': message,

lms/djangoapps/certificates/models.py

@@ -75,7 +75,9 @@ def certificate_status_for_student(student, course_id):
     This returns a dictionary with a key for status, and other information.
     The status is one of the following:
 
-    unavailable  - A student is not eligible for a certificate.
+    unavailable  - No entry for this student--if they are actually in
+                   the course, they probably have not been graded for
+                   certificate generation yet.
     generating   - A request has been made to generate a certificate,
                    but it has not been generated yet.
     regenerating - A request has been made to regenerate a certificate,
@@ -90,7 +92,7 @@ def certificate_status_for_student(student, course_id):
     "download_url".
 
     If the student has been graded, the dictionary also contains their
-    grade for the course.
+    grade for the course with the key "grade".
     '''
 
     try:

lms/templates/dashboard.html

@@ -159,54 +159,43 @@
         %>
         % if course.has_ended() and cert_status:
             <%
-            passing_grade = False
-            cert_button = False
-            survey_button = False
-            if cert_status['status'] in [CertificateStatuses.generating, CertificateStatuses.regenerating]:
+            if cert_status['status'] == 'generating':
                 status_css_class = 'course-status-certrendering'
-                cert_button = True
-                survey_button = True
-                passing_grade = True
-            elif cert_status['status'] == CertificateStatuses.downloadable:
+            elif cert_status['status'] == 'ready':
                 status_css_class = 'course-status-certavailable'
-                cert_button = True
-                survey_button = True
-                passing_grade = True
-            elif cert_status['status'] == CertificateStatuses.notpassing:
+            elif cert_status['status'] == 'notpassing':
                 status_css_class = 'course-status-certnotavailable'
-                survey_button = True
             else:
-                # This is primarily the 'unavailable' state, but also 'error', 'deleted', etc.
                 status_css_class = 'course-status-processing'
-
-            if survey_button and not course.end_of_course_survey_url:
-                survey_button = False
             %>
             <div class="message message-status ${status_css_class} is-shown">
 
-            % if cert_status['status'] == CertificateStatuses.unavailable:
-              <p class="message-copy">Final course details are being wrapped up at this time.
-              Your final standing will be available shortly.</p>
-            % elif passing_grade:
+            % if cert_status['status'] == 'processing':
+                  <p class="message-copy">Final course details are being wrapped up at
+                  this time. Your final standing will be available shortly.</p>
+            % elif cert_status['status'] in ('generating', 'ready'):
                   <p class="message-copy">You have received a grade of
                             <span class="grade-value">${cert_status['grade']}</span>
                             in this course.</p>
-            % elif cert_status['status'] == CertificateStatuses.notpassing:
-              <p class="message-copy">You did not complete the necessary requirements for completion of this course. 
-              </p>
+            % elif cert_status['status'] == 'notpassing':
+                  <p class="message-copy">You did not complete the necessary requirements for
+                     completion of this course.</p>
             % endif
-            % if cert_button or survey_button:
+              
+            % if cert_status['show_disabled_download_button'] or cert_status['show_download_url'] or cert_status['show_survey_button']:
               <ul class="actions">
-                % if cert_button and cert_status['status'] in [CertificateStatuses.generating, CertificateStatuses.regenerating]:
-                  <li class="action"><span class="btn disabled" href="">Your Certificate is Generating</span></li>
-                % elif cert_button and cert_status['status'] == CertificateStatuses.downloadable:
+                % if cert_status['show_disabled_download_button']:
+                  <li class="action"><span class="btn disabled" href="">
+                      Your Certificate is Generating</span></li>
+                % elif cert_status['show_download_url']:
                   <li class="action">
                   <a class="btn" href="${cert_status['download_url']}"
                      title="This link will open/download a PDF document">
                      Download Your PDF Certificate</a></li>
                 % endif
-                % if survey_button:
-                  <li class="action"><a class="cta" href="${course.end_of_course_survey_url}">
+                     
+                % if cert_status['show_survey_button']:
+                  <li class="action"><a class="cta" href="${cert_status['survey_url']}">
                          Complete our course feedback survey</a></li>
                 % endif
               </ul>

rakefile

@@ -54,7 +54,6 @@ default_options = {
 task :predjango do
     sh("find . -type f -name *.pyc -delete")
     sh('pip install -q --upgrade -r local-requirements.txt')
-    sh('git submodule update --init')
 end
 
 task :clean_test_files do