Merge pull request #9370 from edx/cdodge/add-course-id-to-survey-answers

add the ability to persist the course_key where the user answered the…

lms/djangoapps/courseware/tests/test_course_survey.py

@@ -4,10 +4,12 @@ Python tests for the Survey workflows
 
 from collections import OrderedDict
 from nose.plugins.attrib import attr
+from copy import deepcopy
 
 from django.core.urlresolvers import reverse
+from django.contrib.auth.models import User
 
-from survey.models import SurveyForm
+from survey.models import SurveyForm, SurveyAnswer
 
 from common.test.utils import XssTestMixin
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -65,6 +67,8 @@ class SurveyViewsTests(LoginEnrollmentTestCase, ModuleStoreTestCase, XssTestMixi
         self.enroll(self.course_without_survey, True)
         self.enroll(self.course_with_bogus_survey, True)
 
+        self.user = User.objects.get(email=email)
+
         self.view_url = reverse('view_survey', args=[self.test_survey_name])
         self.postback_url = reverse('submit_answers', args=[self.test_survey_name])
 
@@ -137,6 +141,52 @@ class SurveyViewsTests(LoginEnrollmentTestCase, ModuleStoreTestCase, XssTestMixi
 
         self._assert_no_redirect(self.course)
 
+    def test_course_id_field(self):
+        """
+        Assert that the course_id will be in the form fields, if available
+        """
+
+        resp = self.client.get(
+            reverse(
+                'course_survey',
+                kwargs={'course_id': unicode(self.course.id)}
+            )
+        )
+
+        self.assertEqual(resp.status_code, 200)
+        expected = '<input type="hidden" name="course_id" value="{course_id}" />'.format(
+            course_id=unicode(self.course.id)
+        )
+
+        self.assertContains(resp, expected)
+
+    def test_course_id_persists(self):
+        """
+        Assert that a posted back course_id is stored in the database
+        """
+
+        answers = deepcopy(self.student_answers)
+        answers.update({
+            'course_id': unicode(self.course.id)
+        })
+
+        resp = self.client.post(
+            self.postback_url,
+            answers
+        )
+        self.assertEquals(resp.status_code, 200)
+
+        self._assert_no_redirect(self.course)
+
+        # however we want to make sure we persist the course_id
+        answer_objs = SurveyAnswer.objects.filter(
+            user=self.user,
+            form=self.survey
+        )
+
+        for answer_obj in answer_objs:
+            self.assertEquals(answer_obj.course_key, self.course.id)
+
     def test_visiting_course_with_bogus_survey(self):
         """
         Verifies that going to the courseware with a required, but non-existing survey, does not redirect

lms/djangoapps/survey/migrations/0002_auto__add_field_surveyanswer_course_key.py

+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'SurveyAnswer.course_key'
+        db.add_column('survey_surveyanswer', 'course_key',
+                      self.gf('xmodule_django.models.CourseKeyField')(max_length=255, null=True, db_index=True),
+                      keep_default=False)
+
+
+    def backwards(self, orm):
+        # Deleting field 'SurveyAnswer.course_key'
+        db.delete_column('survey_surveyanswer', 'course_key')
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'survey.surveyanswer': {
+            'Meta': {'object_name': 'SurveyAnswer'},
+            'course_key': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'null': 'True', 'db_index': 'True'}),
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'field_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'field_value': ('django.db.models.fields.CharField', [], {'max_length': '1024'}),
+            'form': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['survey.SurveyForm']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'survey.surveyform': {
+            'Meta': {'object_name': 'SurveyForm'},
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'form': ('django.db.models.fields.TextField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255', 'db_index': 'True'})
+        }
+    }
+
+    complete_apps = ['survey']
\ No newline at end of file

lms/djangoapps/survey/models.py

@@ -13,6 +13,8 @@ from model_utils.models import TimeStampedModel
 
 from survey.exceptions import SurveyFormNameAlreadyExists, SurveyFormNotFound
 
+from xmodule_django.models import CourseKeyField
+
 log = logging.getLogger("edx.survey")
 
 
@@ -104,7 +106,7 @@ class SurveyForm(TimeStampedModel):
         """
         return SurveyAnswer.do_survey_answers_exist(self, user)
 
-    def save_user_answers(self, user, answers):
+    def save_user_answers(self, user, answers, course_key):
         """
         Store answers to the form for a given user. Answers is a dict of simple
         name/value pairs
@@ -112,7 +114,16 @@ class SurveyForm(TimeStampedModel):
         IMPORTANT: There is no validaton of form answers at this point. All data
         supplied to this method is presumed to be previously validated
         """
-        SurveyAnswer.save_answers(self, user, answers)
+
+        # first remove any answer the user might have done before
+        self.clear_user_answers(user)
+        SurveyAnswer.save_answers(self, user, answers, course_key)
+
+    def clear_user_answers(self, user):
+        """
+        Removes all answers that a user has submitted
+        """
+        SurveyAnswer.objects.filter(form=self, user=user).delete()
 
     def get_field_names(self):
         """
@@ -135,7 +146,10 @@ class SurveyForm(TimeStampedModel):
         # NOTE: This wrapping doesn't change the ability to query it
         tree = etree.fromstring(u'<div>{}</div>'.format(html))
 
-        input_fields = tree.findall('.//input') + tree.findall('.//select')
+        input_fields = (
+            tree.findall('.//input') + tree.findall('.//select') +
+            tree.findall('.//textarea')
+        )
 
         for input_field in input_fields:
             if 'name' in input_field.keys() and input_field.attrib['name'] not in names:
@@ -153,6 +167,10 @@ class SurveyAnswer(TimeStampedModel):
     field_name = models.CharField(max_length=255, db_index=True)
     field_value = models.CharField(max_length=1024)
 
+    # adding the course_id where the end-user answered the survey question
+    # since it didn't exist in the beginning, it is nullable
+    course_key = CourseKeyField(max_length=255, db_index=True, null=True)
+
     @classmethod
     def do_survey_answers_exist(cls, form, user):
         """
@@ -205,7 +223,7 @@ class SurveyAnswer(TimeStampedModel):
         return results
 
     @classmethod
-    def save_answers(cls, form, user, answers):
+    def save_answers(cls, form, user, answers, course_key):
         """
         Store answers to the form for a given user. Answers is a dict of simple
         name/value pairs
@@ -219,6 +237,20 @@ class SurveyAnswer(TimeStampedModel):
             # See if there is an answer stored for this user, form, field_name pair or not
             # this will allow for update cases. This does include an additional lookup,
             # but write operations will be relatively infrequent
-            answer, __ = SurveyAnswer.objects.get_or_create(user=user, form=form, field_name=name)
-            answer.field_value = value
-            answer.save()
+            value = answers[name]
+            defaults = {"field_value": value}
+            if course_key:
+                defaults['course_key'] = course_key
+
+            answer, created = SurveyAnswer.objects.get_or_create(
+                user=user,
+                form=form,
+                field_name=name,
+                defaults=defaults
+            )
+
+            if not created:
+                # Allow for update cases.
+                answer.field_value = value
+                answer.course_key = course_key
+                answer.save()

lms/djangoapps/survey/tests/test_models.py

@@ -2,6 +2,7 @@
 Python tests for the Survey models
 """
 
+import ddt
 from collections import OrderedDict
 
 from django.test import TestCase
@@ -10,9 +11,10 @@ from django.contrib.auth.models import User
 
 from survey.exceptions import SurveyFormNotFound, SurveyFormNameAlreadyExists
 from django.core.exceptions import ValidationError
-from survey.models import SurveyForm
+from survey.models import SurveyForm, SurveyAnswer
 
 
+@ddt.ddt
 class SurveyModelsTests(TestCase):
     """
     All tests for the Survey models.py file
@@ -32,12 +34,22 @@ class SurveyModelsTests(TestCase):
         self.test_survey_name = 'TestForm'
         self.test_form = '<li><input name="field1" /></li><li><input name="field2" /></li><li><select name="ddl"><option>1</option></select></li>'
         self.test_form_update = '<input name="field1" />'
+        self.course_id = 'foo/bar/baz'
 
         self.student_answers = OrderedDict({
             'field1': 'value1',
             'field2': 'value2',
         })
 
+        self.student_answers_update = OrderedDict({
+            'field1': 'value1-updated',
+            'field2': 'value2-updated',
+        })
+
+        self.student_answers_update2 = OrderedDict({
+            'field1': 'value1-updated2',
+        })
+
         self.student2_answers = OrderedDict({
             'field1': 'value3'
         })
@@ -142,7 +154,8 @@ class SurveyModelsTests(TestCase):
         self.assertFalse(survey.has_user_answered_survey(self.student))
         self.assertEquals(len(survey.get_answers()), 0)
 
-    def test_single_user_answers(self):
+    @ddt.data(None, 'foo/bar/baz')
+    def test_single_user_answers(self, course_id):
         """
         Create a new survey and add answers to it
         """
@@ -150,7 +163,7 @@ class SurveyModelsTests(TestCase):
         survey = self._create_test_survey()
         self.assertIsNotNone(survey)
 
-        survey.save_user_answers(self.student, self.student_answers)
+        survey.save_user_answers(self.student, self.student_answers, course_id)
 
         self.assertTrue(survey.has_user_answered_survey(self.student))
 
@@ -164,6 +177,19 @@ class SurveyModelsTests(TestCase):
         self.assertTrue(self.student.id in answers)
         self.assertEquals(all_answers[self.student.id], self.student_answers)
 
+        # check that the course_id was set
+
+        answer_objs = SurveyAnswer.objects.filter(
+            user=self.student,
+            form=survey
+        )
+
+        for answer_obj in answer_objs:
+            if course_id:
+                self.assertEquals(unicode(answer_obj.course_key), course_id)
+            else:
+                self.assertIsNone(answer_obj.course_key)
+
     def test_multiple_user_answers(self):
         """
         Create a new survey and add answers to it
@@ -172,8 +198,8 @@ class SurveyModelsTests(TestCase):
         survey = self._create_test_survey()
         self.assertIsNotNone(survey)
 
-        survey.save_user_answers(self.student, self.student_answers)
-        survey.save_user_answers(self.student2, self.student2_answers)
+        survey.save_user_answers(self.student, self.student_answers, self.course_id)
+        survey.save_user_answers(self.student2, self.student2_answers, self.course_id)
 
         self.assertTrue(survey.has_user_answered_survey(self.student))
 
@@ -187,12 +213,43 @@ class SurveyModelsTests(TestCase):
         answers = survey.get_answers(self.student)
         self.assertEquals(len(answers.keys()), 1)
         self.assertTrue(self.student.id in answers)
-        self.assertEquals(all_answers[self.student.id], self.student_answers)
+        self.assertEquals(answers[self.student.id], self.student_answers)
 
         answers = survey.get_answers(self.student2)
         self.assertEquals(len(answers.keys()), 1)
         self.assertTrue(self.student2.id in answers)
-        self.assertEquals(all_answers[self.student2.id], self.student2_answers)
+        self.assertEquals(answers[self.student2.id], self.student2_answers)
+
+    def test_update_answers(self):
+        """
+        Make sure the update case works
+        """
+
+        survey = self._create_test_survey()
+        self.assertIsNotNone(survey)
+
+        survey.save_user_answers(self.student, self.student_answers, self.course_id)
+
+        answers = survey.get_answers(self.student)
+        self.assertEquals(len(answers.keys()), 1)
+        self.assertTrue(self.student.id in answers)
+        self.assertEquals(answers[self.student.id], self.student_answers)
+
+        # update
+        survey.save_user_answers(self.student, self.student_answers_update, self.course_id)
+
+        answers = survey.get_answers(self.student)
+        self.assertEquals(len(answers.keys()), 1)
+        self.assertTrue(self.student.id in answers)
+        self.assertEquals(answers[self.student.id], self.student_answers_update)
+
+        # update with just a subset of the origin dataset
+        survey.save_user_answers(self.student, self.student_answers_update2, self.course_id)
+
+        answers = survey.get_answers(self.student)
+        self.assertEquals(len(answers.keys()), 1)
+        self.assertTrue(self.student.id in answers)
+        self.assertEquals(answers[self.student.id], self.student_answers_update2)
 
     def test_limit_num_users(self):
         """
@@ -201,8 +258,8 @@ class SurveyModelsTests(TestCase):
         """
         survey = self._create_test_survey()
 
-        survey.save_user_answers(self.student, self.student_answers)
-        survey.save_user_answers(self.student2, self.student2_answers)
+        survey.save_user_answers(self.student, self.student_answers, self.course_id)
+        survey.save_user_answers(self.student2, self.student2_answers, self.course_id)
 
         # even though we have 2 users submitted answers
         # limit the result set to just 1
@@ -217,8 +274,8 @@ class SurveyModelsTests(TestCase):
         survey = self._create_test_survey()
         self.assertIsNotNone(survey)
 
-        survey.save_user_answers(self.student, self.student_answers)
-        survey.save_user_answers(self.student2, self.student2_answers)
+        survey.save_user_answers(self.student, self.student_answers, self.course_id)
+        survey.save_user_answers(self.student2, self.student2_answers, self.course_id)
 
         names = survey.get_field_names()
 

lms/djangoapps/survey/tests/test_utils.py

@@ -108,7 +108,7 @@ class SurveyModelsTests(ModuleStoreTestCase):
         """
         Assert that a new course which has a required survey and user has answers for it
         """
-        self.survey.save_user_answers(self.student, self.student_answers)
+        self.survey.save_user_answers(self.student, self.student_answers, None)
         self.assertFalse(must_answer_survey(self.course, self.student))
 
     def test_staff_must_answer_survey(self):

lms/djangoapps/survey/tests/test_views.py

@@ -9,7 +9,7 @@ from django.test.client import Client
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 
-from survey.models import SurveyForm
+from survey.models import SurveyForm, SurveyAnswer
 
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -32,15 +32,20 @@ class SurveyViewsTests(ModuleStoreTestCase):
         self.student = User.objects.create_user('student', 'student@test.com', self.password)
 
         self.test_survey_name = 'TestSurvey'
-        self.test_form = '<input name="field1" /><input name="field2" /><select name="ddl"><option>1</option></select>'
+        self.test_form = '''
+            <input name="field1" /><input name="field2" /><select name="ddl"><option>1</option></select>
+            <textarea name="textarea" />
+        '''
 
         self.student_answers = OrderedDict({
             u'field1': u'value1',
             u'field2': u'value2',
             u'ddl': u'1',
+            u'textarea': u'textarea'
         })
 
         self.course = CourseFactory.create(
+            display_name='Test Course',
             course_survey_required=True,
             course_survey_name=self.test_survey_name
         )
@@ -124,6 +129,7 @@ class SurveyViewsTests(ModuleStoreTestCase):
 
         data['csrfmiddlewaretoken'] = 'foo'
         data['_redirect_url'] = 'bar'
+        data['course_id'] = unicode(self.course.id)
 
         resp = self.client.post(
             self.postback_url,
@@ -133,6 +139,16 @@ class SurveyViewsTests(ModuleStoreTestCase):
         answers = self.survey.get_answers(self.student)
         self.assertNotIn('csrfmiddlewaretoken', answers[self.student.id])
         self.assertNotIn('_redirect_url', answers[self.student.id])
+        self.assertNotIn('course_id', answers[self.student.id])
+
+        # however we want to make sure we persist the course_id
+        answer_objs = SurveyAnswer.objects.filter(
+            user=self.student,
+            form=self.survey
+        )
+
+        for answer_obj in answer_objs:
+            self.assertEquals(unicode(answer_obj.course_key), data['course_id'])
 
     def test_encoding_answers(self):
         """

lms/djangoapps/survey/views.py

@@ -14,6 +14,8 @@ from django.views.decorators.http import require_POST
 from django.conf import settings
 from django.utils.html import escape
 
+from opaque_keys.edx.keys import CourseKey
+
 from edxmako.shortcuts import render_to_response
 from survey.models import SurveyForm
 from microsite_configuration import microsite
@@ -92,6 +94,8 @@ def submit_answers(request, survey_name):
     # in a hidden form field
     redirect_url = answers['_redirect_url'] if '_redirect_url' in answers else reverse('dashboard')
 
+    course_key = CourseKey.from_string(answers['course_id']) if 'course_id' in answers else None
+
     allowed_field_names = survey.get_field_names()
 
     # scrub the answers to make sure nothing malicious from the user gets stored in
@@ -102,7 +106,7 @@ def submit_answers(request, survey_name):
         if answer_key in allowed_field_names:
             filtered_answers[answer_key] = escape(answers[answer_key])
 
-    survey.save_user_answers(request.user, filtered_answers)
+    survey.save_user_answers(request.user, filtered_answers, course_key)
 
     response_params = json.dumps({
         # The HTTP end-point for the payment processor.

lms/templates/survey/survey.html

@@ -21,6 +21,8 @@ from django.utils import html
       <input type="hidden" name="_redirect_url" value="${redirect_url}" />
 
       % if course:
+        <input type="hidden" name="course_id" value="${unicode(course.id)}" />
+
         <div class="header-survey">
           <h4 class="course-info">
             <span class="course-org">${course.display_org_with_default}</span><span class="course-number"> ${course.display_number_with_default}</span>