Merge pull request #1755 from MITx/fix/brian/fix500v1

Fix/brian/fix500v1

lms/djangoapps/course_wiki/tests/tests.py

@@ -3,13 +3,11 @@ from django.test.utils import override_settings
 
 import xmodule.modulestore.django
 
-from courseware.tests.tests import PageLoader, TEST_DATA_XML_MODULESTORE
+from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.xml_importer import import_from_xml
-
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class WikiRedirectTestCase(PageLoader):
+class WikiRedirectTestCase(LoginEnrollmentTestCase):
     def setUp(self):
         xmodule.modulestore.django._MODULESTORES = {}
         courses = modulestore().get_courses()
@@ -30,8 +28,6 @@ class WikiRedirectTestCase(PageLoader):
         self.activate_user(self.student)
         self.activate_user(self.instructor)
 
-
-
     def test_wiki_redirect(self):
         """
         Test that requesting wiki URLs redirect properly to or out of classes.
@@ -69,7 +65,6 @@ class WikiRedirectTestCase(PageLoader):
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(resp['Location'], 'http://testserver' + destination)
 
-
     def create_course_page(self, course):
         """
         Test that loading the course wiki page creates the wiki page.
@@ -98,7 +93,6 @@ class WikiRedirectTestCase(PageLoader):
         self.assertTrue("course info" in resp.content.lower())
         self.assertTrue("courseware" in resp.content.lower())
 
-
     def test_course_navigator(self):
         """"
         Test that going from a course page to a wiki page contains the course navigator.
@@ -108,7 +102,6 @@ class WikiRedirectTestCase(PageLoader):
         self.enroll(self.toy)
         self.create_course_page(self.toy)
 
-
         course_wiki_page = reverse('wiki:get', kwargs={'path': self.toy.wiki_slug + '/'})
         referer = reverse("courseware", kwargs={'course_id': self.toy.id})
 

lms/djangoapps/courseware/tests/test_module_render.py

@@ -2,16 +2,16 @@ from mock import MagicMock
 import json
 
 from django.http import Http404, HttpResponse
+from django.core.urlresolvers import reverse
 from django.conf import settings
 from django.test import TestCase
 from django.test.client import RequestFactory
-from django.core.urlresolvers import reverse
 from django.test.utils import override_settings
 
 from xmodule.modulestore.exceptions import ItemNotFoundError
-import courseware.module_render as render
 from xmodule.modulestore.django import modulestore
-from courseware.tests.tests import PageLoader
+import courseware.module_render as render
+from courseware.tests.tests import LoginEnrollmentTestCase
 from courseware.model_data import ModelDataCache
 
 from .factories import UserFactory
@@ -38,7 +38,7 @@ TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR)
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class ModuleRenderTestCase(PageLoader):
+class ModuleRenderTestCase(LoginEnrollmentTestCase):
     def setUp(self):
         self.location = ['i4x', 'edX', 'toy', 'chapter', 'Overview']
         self.course_id = 'edX/toy/2012_Fall'
@@ -54,8 +54,7 @@ class ModuleRenderTestCase(PageLoader):
         mock_request = MagicMock()
         mock_request.FILES.keys.return_value = ['file_id']
         mock_request.FILES.getlist.return_value = ['file'] * (settings.MAX_FILEUPLOADS_PER_INPUT + 1)
-        self.assertEquals(render.modx_dispatch(mock_request, 'dummy', self.location,
-                                          'dummy').content,
+        self.assertEquals(render.modx_dispatch(mock_request, 'dummy', self.location, 'dummy').content,
                           json.dumps({'success': 'Submission aborted! Maximum %d files may be submitted at once' %
                                       settings.MAX_FILEUPLOADS_PER_INPUT}))
         mock_request_2 = MagicMock()

lms/djangoapps/courseware/tests/tests.py

 import logging
-log = logging.getLogger("mitx." + __name__)
-
 import json
 import time
-
 from urlparse import urlsplit, urlunsplit
 
 from django.contrib.auth.models import User, Group
@@ -29,29 +26,30 @@ from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import Location
 from xmodule.modulestore.xml_importer import import_from_xml
 from xmodule.modulestore.xml import XMLModuleStore
-from xmodule.timeparse import stringify_time
 
+log = logging.getLogger("mitx." + __name__)
 
 def parse_json(response):
     """Parse response, which is assumed to be json"""
     return json.loads(response.content)
 
 
-def user(email):
+def get_user(email):
     '''look up a user by email'''
     return User.objects.get(email=email)
 
 
-def registration(email):
+def get_registration(email):
     '''look up registration object by email'''
     return Registration.objects.get(user__email=email)
 
-# A bit of a hack--want mongo modulestore for these tests, until
-# jump_to works with the xmlmodulestore or we have an even better solution
-# NOTE: this means this test requires mongo to be running.
-
 
 def mongo_store_config(data_dir):
+    '''
+    Defines default module store using MongoModuleStore
+    
+    Use of this config requires mongo to be running
+    '''
     return {
         'default': {
             'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
@@ -68,6 +66,7 @@ def mongo_store_config(data_dir):
 
 
 def draft_mongo_store_config(data_dir):
+    '''Defines default module store using DraftMongoModuleStore'''
     return {
         'default': {
             'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
@@ -84,6 +83,7 @@ def draft_mongo_store_config(data_dir):
 
 
 def xml_store_config(data_dir):
+    '''Defines default module store using XMLModuleStore'''
     return {
         'default': {
             'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
@@ -100,8 +100,8 @@ TEST_DATA_MONGO_MODULESTORE = mongo_store_config(TEST_DATA_DIR)
 TEST_DATA_DRAFT_MONGO_MODULESTORE = draft_mongo_store_config(TEST_DATA_DIR)
 
 
-class ActivateLoginTestCase(TestCase):
-    '''Check that we can activate and log in'''
+class LoginEnrollmentTestCase(TestCase):
+    '''Base TestCase providing support for user creation, activation, login, and course enrollment'''
 
     def assertRedirectsNoFollow(self, response, expected_url):
         """
@@ -117,32 +117,33 @@ class ActivateLoginTestCase(TestCase):
 
         e_scheme, e_netloc, e_path, e_query, e_fragment = urlsplit(expected_url)
         if not (e_scheme or e_netloc):
-            expected_url = urlunsplit(('http', 'testserver', e_path,
-                e_query, e_fragment))
+            expected_url = urlunsplit(('http', 'testserver', e_path, e_query, e_fragment))
 
         self.assertEqual(url, expected_url, "Response redirected to '{0}', expected '{1}'".format(
             url, expected_url))
 
-    def setUp(self):
-        email = 'view@test.com'
-        password = 'foo'
-        self.create_account('viewtest', email, password)
-        self.activate_user(email)
-        self.login(email, password)
+    def setup_viewtest_user(self):
+        '''create a user account, activate, and log in'''
+        self.viewtest_email = 'view@test.com'
+        self.viewtest_password = 'foo'
+        self.viewtest_username = 'viewtest'
+        self.create_account(self.viewtest_username, self.viewtest_email, self.viewtest_password)
+        self.activate_user(self.viewtest_email)
+        self.login(self.viewtest_email, self.viewtest_password)
 
     # ============ User creation and login ==============
 
-    def _login(self, email, pw):
+    def _login(self, email, password):
         '''Login.  View should always return 200.  The success/fail is in the
         returned json'''
         resp = self.client.post(reverse('login'),
-                                {'email': email, 'password': pw})
+                                {'email': email, 'password': password})
         self.assertEqual(resp.status_code, 200)
         return resp
 
-    def login(self, email, pw):
+    def login(self, email, password):
         '''Login, check that it worked.'''
-        resp = self._login(email, pw)
+        resp = self._login(email, password)
         data = parse_json(resp)
         self.assertTrue(data['success'])
         return resp
@@ -154,34 +155,34 @@ class ActivateLoginTestCase(TestCase):
         self.assertEqual(resp.status_code, 302)
         return resp
 
-    def _create_account(self, username, email, pw):
+    def _create_account(self, username, email, password):
         '''Try to create an account.  No error checking'''
         resp = self.client.post('/create_account', {
             'username': username,
             'email': email,
-            'password': pw,
+            'password': password,
             'name': 'Fred Weasley',
             'terms_of_service': 'true',
             'honor_code': 'true',
         })
         return resp
 
-    def create_account(self, username, email, pw):
+    def create_account(self, username, email, password):
         '''Create the account and check that it worked'''
-        resp = self._create_account(username, email, pw)
+        resp = self._create_account(username, email, password)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
         self.assertEqual(data['success'], True)
 
         # Check both that the user is created, and inactive
-        self.assertFalse(user(email).is_active)
+        self.assertFalse(get_user(email).is_active)
 
         return resp
 
     def _activate_user(self, email):
         '''Look up the activation key for the user, then hit the activate view.
         No error checking'''
-        activation_key = registration(email).activation_key
+        activation_key = get_registration(email).activation_key
 
         # and now we try to activate
         resp = self.client.get(reverse('activate', kwargs={'key': activation_key}))
@@ -191,19 +192,7 @@ class ActivateLoginTestCase(TestCase):
         resp = self._activate_user(email)
         self.assertEqual(resp.status_code, 200)
         # Now make sure that the user is now actually activated
-        self.assertTrue(user(email).is_active)
-
-    def test_activate_login(self):
-        '''The setup function does all the work'''
-        pass
-
-    def test_logout(self):
-        '''Setup function does login'''
-        self.logout()
-
-
-class PageLoader(ActivateLoginTestCase):
-    ''' Base class that adds a function to load all pages in a modulestore '''
+        self.assertTrue(get_user(email).is_active)
 
     def _enroll(self, course):
         """Post to the enrollment view, and return the parsed json response"""
@@ -240,8 +229,7 @@ class PageLoader(ActivateLoginTestCase):
         """
         resp = self.client.get(url)
         self.assertEqual(resp.status_code, code,
-                         "got code {0} for url '{1}'. Expected code {2}"
-                         .format(resp.status_code, url, code))
+                         "got code {0} for url '{1}'. Expected code {2}".format(resp.status_code, url, code))
         return resp
 
     def check_for_post_code(self, code, url, data={}):
@@ -251,10 +239,27 @@ class PageLoader(ActivateLoginTestCase):
         """
         resp = self.client.post(url, data)
         self.assertEqual(resp.status_code, code,
-                         "got code {0} for url '{1}'. Expected code {2}"
-                         .format(resp.status_code, url, code))
+                         "got code {0} for url '{1}'. Expected code {2}".format(resp.status_code, url, code))
         return resp
 
+
+class ActivateLoginTest(LoginEnrollmentTestCase):
+    '''Test logging in and logging out'''
+    def setUp(self):
+        self.setup_viewtest_user()
+
+    def test_activate_login(self):
+        '''Test login -- the setup function does all the work'''
+        pass
+
+    def test_logout(self):
+        '''Test logout -- setup function does login'''
+        self.logout()
+
+
+class PageLoaderTestCase(LoginEnrollmentTestCase):
+    ''' Base class that adds a function to load all pages in a modulestore '''
+
     def check_pages_load(self, module_store):
         """Make all locations in course load"""
        # enroll in the course before trying to access pages
@@ -264,14 +269,14 @@ class PageLoader(ActivateLoginTestCase):
         self.enroll(course)
         course_id = course.id
 
-        n = 0
+        num = 0
         num_bad = 0
         all_ok = True
 
         for descriptor in module_store.get_items(
                 Location(None, None, None, None, None)):
 
-            n += 1
+            num += 1
             print "Checking ", descriptor.location.url()
 
             # We have ancillary course information now as modules and we can't simply use 'jump_to' to view them
@@ -332,22 +337,21 @@ class PageLoader(ActivateLoginTestCase):
             print msg
             self.assertTrue(all_ok)  # fail fast
 
-        print "{0}/{1} good".format(n - num_bad, n)
-        log.info("{0}/{1} good".format(n - num_bad, n))
+        print "{0}/{1} good".format(num - num_bad, num)
+        log.info("{0}/{1} good".format(num - num_bad, num))
         self.assertTrue(all_ok)
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class TestCoursesLoadTestCase_XmlModulestore(PageLoader):
-    '''Check that all pages in test courses load properly'''
+class TestCoursesLoadTestCase_XmlModulestore(PageLoaderTestCase):
+    '''Check that all pages in test courses load properly from XML'''
 
     def setUp(self):
-        ActivateLoginTestCase.setUp(self)
+        self.setup_viewtest_user()
         xmodule.modulestore.django._MODULESTORES = {}
 
     def test_toy_course_loads(self):
-        module_store = XMLModuleStore(
-                                        TEST_DATA_DIR,
+        module_store = XMLModuleStore(TEST_DATA_DIR,
                                       default_class='xmodule.hidden_module.HiddenDescriptor',
                                       course_dirs=['toy'],
                                       load_error_modules=True,
@@ -356,8 +360,7 @@ class TestCoursesLoadTestCase_XmlModulestore(PageLoader):
         self.check_pages_load(module_store)
 
     def test_full_course_loads(self):
-        module_store = XMLModuleStore(
-                                        TEST_DATA_DIR,
+        module_store = XMLModuleStore(TEST_DATA_DIR,
                                       default_class='xmodule.hidden_module.HiddenDescriptor',
                                       course_dirs=['full'],
                                       load_error_modules=True,
@@ -366,11 +369,11 @@ class TestCoursesLoadTestCase_XmlModulestore(PageLoader):
 
 
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
-class TestCoursesLoadTestCase_MongoModulestore(PageLoader):
-    '''Check that all pages in test courses load properly'''
+class TestCoursesLoadTestCase_MongoModulestore(PageLoaderTestCase):
+    '''Check that all pages in test courses load properly from Mongo'''
 
     def setUp(self):
-        ActivateLoginTestCase.setUp(self)
+        self.setup_viewtest_user()
         xmodule.modulestore.django._MODULESTORES = {}
         modulestore().collection.drop()
 
@@ -386,7 +389,7 @@ class TestCoursesLoadTestCase_MongoModulestore(PageLoader):
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class TestNavigation(PageLoader):
+class TestNavigation(LoginEnrollmentTestCase):
     """Check that navigation state is saved properly"""
 
     def setUp(self):
@@ -447,7 +450,7 @@ class TestDraftModuleStore(TestCase):
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class TestViewAuth(PageLoader):
+class TestViewAuth(LoginEnrollmentTestCase):
     """Check that view authentication works properly"""
 
     # NOTE: setUpClass() runs before override_settings takes effect, so
@@ -492,7 +495,7 @@ class TestViewAuth(PageLoader):
                 'gradebook',
                 'grade_summary',)]
             urls.append(reverse('student_progress', kwargs={'course_id': course.id,
-                                                     'student_id': user(self.student).id}))
+                                                            'student_id': get_user(self.student).id}))
             return urls
 
         # shouldn't be able to get to the instructor pages
@@ -502,8 +505,8 @@ class TestViewAuth(PageLoader):
 
         # Make the instructor staff in the toy course
         group_name = _course_staff_group_name(self.toy.location)
-        g = Group.objects.create(name=group_name)
-        g.user_set.add(user(self.instructor))
+        group = Group.objects.create(name=group_name)
+        group.user_set.add(get_user(self.instructor))
 
         self.logout()
         self.login(self.instructor, self.password)
@@ -518,9 +521,9 @@ class TestViewAuth(PageLoader):
             self.check_for_get_code(404, url)
 
         # now also make the instructor staff
-        u = user(self.instructor)
-        u.is_staff = True
-        u.save()
+        instructor = get_user(self.instructor)
+        instructor.is_staff = True
+        instructor.save()
 
         # and now should be able to load both
         for url in instructor_urls(self.toy) + instructor_urls(self.full):
@@ -627,7 +630,7 @@ class TestViewAuth(PageLoader):
             # to make access checking smarter and understand both the effective
             # user (the student), and the requesting user (the prof)
             url = reverse('student_progress', kwargs={'course_id': course.id,
-                                                     'student_id': user(self.student).id})
+                                                      'student_id': get_user(self.student).id})
             print 'checking for 404 on view-as-student: {0}'.format(url)
             self.check_for_get_code(404, url)
 
@@ -648,8 +651,8 @@ class TestViewAuth(PageLoader):
         print '=== Testing course instructor access....'
         # Make the instructor staff in the toy course
         group_name = _course_staff_group_name(self.toy.location)
-        g = Group.objects.create(name=group_name)
-        g.user_set.add(user(self.instructor))
+        group = Group.objects.create(name=group_name)
+        group.user_set.add(get_user(self.instructor))
 
         self.logout()
         self.login(self.instructor, self.password)
@@ -663,9 +666,9 @@ class TestViewAuth(PageLoader):
 
         print '=== Testing staff access....'
         # now also make the instructor staff
-        u = user(self.instructor)
-        u.is_staff = True
-        u.save()
+        instructor = get_user(self.instructor)
+        instructor.is_staff = True
+        instructor.save()
 
         # and now should be able to load both
         check_staff(self.toy)
@@ -698,8 +701,8 @@ class TestViewAuth(PageLoader):
         print '=== Testing course instructor access....'
         # Make the instructor staff in the toy course
         group_name = _course_staff_group_name(self.toy.location)
-        g = Group.objects.create(name=group_name)
-        g.user_set.add(user(self.instructor))
+        group = Group.objects.create(name=group_name)
+        group.user_set.add(get_user(self.instructor))
 
         print "logout/login"
         self.logout()
@@ -709,10 +712,10 @@ class TestViewAuth(PageLoader):
 
         print '=== Testing staff access....'
         # now make the instructor global staff, but not in the instructor group
-        g.user_set.remove(user(self.instructor))
-        u = user(self.instructor)
-        u.is_staff = True
-        u.save()
+        group.user_set.remove(get_user(self.instructor))
+        instructor = get_user(self.instructor)
+        instructor.is_staff = True
+        instructor.save()
 
         # unenroll and try again
         self.unenroll(self.toy)
@@ -726,8 +729,8 @@ class TestViewAuth(PageLoader):
 
         # Make courses start in the future
         tomorrow = time.time() + 24 * 3600
-        nextday = tomorrow + 24 * 3600
-        yesterday = time.time() - 24 * 3600
+        # nextday = tomorrow + 24 * 3600
+        # yesterday = time.time() - 24 * 3600
 
         # toy course's hasn't started
         self.toy.lms.start = time.gmtime(tomorrow)
@@ -737,20 +740,20 @@ class TestViewAuth(PageLoader):
         self.toy.lms.days_early_for_beta = 2
 
         # student user shouldn't see it
-        student_user = user(self.student)
+        student_user = get_user(self.student)
         self.assertFalse(has_access(student_user, self.toy, 'load'))
 
         # now add the student to the beta test group
         group_name = course_beta_test_group_name(self.toy.location)
-        g = Group.objects.create(name=group_name)
-        g.user_set.add(student_user)
+        group = Group.objects.create(name=group_name)
+        group.user_set.add(student_user)
 
         # now the student should see it
         self.assertTrue(has_access(student_user, self.toy, 'load'))
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class TestCourseGrader(PageLoader):
+class TestCourseGrader(LoginEnrollmentTestCase):
     """Check that a course gets graded properly"""
 
     # NOTE: setUpClass() runs before override_settings takes effect, so
@@ -773,11 +776,12 @@ class TestCourseGrader(PageLoader):
         self.activate_user(self.student)
         self.enroll(self.graded_course)
 
-        self.student_user = user(self.student)
+        self.student_user = get_user(self.student)
 
         self.factory = RequestFactory()
 
     def get_grade_summary(self):
+        '''calls grades.grade for current user and course'''
         model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
             self.graded_course.id, self.student_user, self.graded_course)
 
@@ -788,9 +792,11 @@ class TestCourseGrader(PageLoader):
                             self.graded_course, model_data_cache)
 
     def get_homework_scores(self):
+        '''get scores for homeworks'''
         return self.get_grade_summary()['totaled_scores']['Homework']
 
     def get_progress_summary(self):
+        '''return progress summary structure for current user and course'''
         model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
             self.graded_course.id, self.student_user, self.graded_course)
 
@@ -802,6 +808,7 @@ class TestCourseGrader(PageLoader):
         return progress_summary
 
     def check_grade_percent(self, percent):
+        '''assert that percent grade is as expected'''
         grade_summary = self.get_grade_summary()
         self.assertEqual(grade_summary['percent'], percent)
 
@@ -816,8 +823,7 @@ class TestCourseGrader(PageLoader):
         problem_location = "i4x://edX/graded/problem/{0}".format(problem_url_name)
 
         modx_url = reverse('modx_dispatch',
-                            kwargs={
-                                'course_id': self.graded_course.id,
+                            kwargs={'course_id': self.graded_course.id,
                                     'location': problem_location,
                                     'dispatch': 'problem_check', })
 
@@ -831,14 +837,15 @@ class TestCourseGrader(PageLoader):
         return resp
 
     def problem_location(self, problem_url_name):
+        '''Get location string for problem, assuming hardcoded course_id'''
         return "i4x://edX/graded/problem/{0}".format(problem_url_name)
 
     def reset_question_answer(self, problem_url_name):
+        '''resets specified problem for current user'''
         problem_location = self.problem_location(problem_url_name)
 
         modx_url = reverse('modx_dispatch',
-                            kwargs={
-                                'course_id': self.graded_course.id,
+                            kwargs={'course_id': self.graded_course.id,
                                     'location': problem_location,
                                     'dispatch': 'problem_reset', })
 
@@ -855,6 +862,7 @@ class TestCourseGrader(PageLoader):
             return [s.earned for s in self.get_homework_scores()]
 
         def score_for_hw(hw_url_name):
+            """returns list of scores for a given url"""
             hw_section = [section for section
                           in self.get_progress_summary()[0]['sections']
                           if section.get('url_name') == hw_url_name][0]

lms/djangoapps/django_comment_client/forum/views.py

 import json
 import logging
+import xml.sax.saxutils as saxutils
 
 from django.contrib.auth.decorators import login_required
-from django.views.decorators.http import require_POST
-from django.http import HttpResponse, Http404
-from django.utils import simplejson
+from django.http import Http404
 from django.core.context_processors import csrf
-from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
 
 from mitxmako.shortcuts import render_to_response, render_to_string
 from courseware.courses import get_course_with_access
-from course_groups.cohorts import is_course_cohorted, get_cohort_id, is_commentable_cohorted, get_cohorted_commentables, get_cohort, get_course_cohorts, get_cohort_by_id
+from course_groups.cohorts import (is_course_cohorted, get_cohort_id, is_commentable_cohorted, 
+                                   get_cohorted_commentables, get_course_cohorts, get_cohort_by_id)
 from courseware.access import has_access
 
-from urllib import urlencode
-from operator import methodcaller
-from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
-from django_comment_client.utils import (merge_dict, extract, strip_none,
-                                         strip_blank, get_courseware_context)
-
+from django_comment_client.permissions import cached_has_permission
+from django_comment_client.utils import (merge_dict, extract, strip_none, get_courseware_context)
 import django_comment_client.utils as utils
 import comment_client as cc
-import xml.sax.saxutils as saxutils
 
 THREADS_PER_PAGE = 20
 INLINE_THREADS_PER_PAGE = 20
@@ -31,6 +25,7 @@ escapedict = {'"': '"'}
 log = logging.getLogger("edx.discussions")
 
 
+@login_required
 def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAGE):
     """
     This may raise cc.utils.CommentClientError or
@@ -60,7 +55,6 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
         cc_user.default_sort_key = request.GET.get('sort_key')
         cc_user.save()
 
-
     #there are 2 dimensions to consider when executing a search with respect to group id
     #is user a moderator
     #did the user request a group
@@ -103,13 +97,13 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
         if not 'pinned' in thread:
             thread['pinned'] = False
 
-
     query_params['page'] = page
     query_params['num_pages'] = num_pages
 
     return threads, query_params
 
 
+@login_required
 def inline_discussion(request, course_id, discussion_id):
     """
     Renders JSON for DiscussionModules
@@ -142,14 +136,14 @@ def inline_discussion(request, course_id, discussion_id):
     cohorts_list = list()
 
     if is_cohorted:
-        cohorts_list.append({'name':'All Groups','id':None})
+        cohorts_list.append({'name': 'All Groups', 'id': None})
 
         #if you're a mod, send all cohorts and let you pick
 
         if is_moderator:
             cohorts = get_course_cohorts(course_id)
             for c in cohorts:
-                cohorts_list.append({'name':c.name, 'id':c.id})
+                cohorts_list.append({'name': c.name, 'id': c.id})
 
         else:
             #students don't get to choose
@@ -216,9 +210,6 @@ def forum_form_discussion(request, course_id):
 
         user_cohort_id = get_cohort_id(request.user, course_id)
 
-    
-        
-
         context = {
             'csrf': csrf(request)['csrf_token'],
             'course': course,
@@ -242,6 +233,7 @@ def forum_form_discussion(request, course_id):
 
         return render_to_response('discussion/index.html', context)
 
+
 @login_required
 def single_thread(request, course_id, discussion_id, thread_id):
     course = get_course_with_access(request.user, course_id, 'load')
@@ -369,8 +361,6 @@ def user_profile(request, course_id, user_id):
                 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict),
             })
         else:
-
-
             context = {
                 'course': course,
                 'user': request.user,
@@ -426,5 +416,5 @@ def followed_threads(request, course_id, user_id):
             }
 
             return render_to_response('discussion/user_profile.html', context)
-    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
+    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
         raise Http404

lms/djangoapps/django_comment_client/tests.py

-from django.contrib.auth.models import User, Group
-from django.core.urlresolvers import reverse
-from django.test import TestCase
-from django.test.client import RequestFactory
-from django.conf import settings
-
-from mock import Mock
-
-from django.test.utils import override_settings
-
-import xmodule.modulestore.django
-
-from student.models import CourseEnrollment
-
-from django.db.models.signals import m2m_changed, pre_delete, pre_save, post_delete, post_save
-from django.dispatch.dispatcher import _make_id
 import string
 import random
-from .permissions import has_permission
-from .models import Role, Permission
-
-from xmodule.modulestore.django import modulestore
-from xmodule.modulestore import Location
-from xmodule.modulestore.xml_importer import import_from_xml
-from xmodule.modulestore.xml import XMLModuleStore
-
-import comment_client
 
-from courseware.tests.tests import PageLoader, TEST_DATA_XML_MODULESTORE
-
-#@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-#class TestCohorting(PageLoader):
-#    """Check that cohorting works properly"""
-#
-#    def setUp(self):
-#        xmodule.modulestore.django._MODULESTORES = {}
-#
-#        # Assume courses are there
-#        self.toy = modulestore().get_course("edX/toy/2012_Fall")
-#
-#        # Create two accounts
-#        self.student = 'view@test.com'
-#        self.student2 = 'view2@test.com'
-#        self.password = 'foo'
-#        self.create_account('u1', self.student, self.password)
-#        self.create_account('u2', self.student2, self.password)
-#        self.activate_user(self.student)
-#        self.activate_user(self.student2)
-#
-#    def test_create_thread(self):
-#        my_save = Mock()
-#        comment_client.perform_request = my_save
-#
-#        resp = self.client.post(
-#            reverse('django_comment_client.base.views.create_thread',
-#                    kwargs={'course_id': 'edX/toy/2012_Fall',
-#                            'commentable_id': 'General'}),
-#                                        {'some': "some",
-#                                         'data': 'data'})
-#        self.assertTrue(my_save.called)
-#
-#        #self.assertEqual(resp.status_code, 200)
-#        #self.assertEqual(my_save.something, "expected", "complaint if not true")
-#
-#        self.toy.cohort_config = {"cohorted": True}
-#
-#        # call the view again ...
-#
-#       # assert that different things happened
+from django.contrib.auth.models import User
+from django.test import TestCase
 
+from student.models import CourseEnrollment
+from django_comment_client.permissions import has_permission
+from django_comment_client.models import Role
 
 
 class PermissionsTestCase(TestCase):

lms/djangoapps/instructor/tests.py

@@ -8,13 +8,6 @@ Notes for running by hand:
 django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/instructor
 """
 
-import courseware.tests.tests as ct
-
-import json
-
-from nose import SkipTest
-from mock import patch, Mock
-
 from django.test.utils import override_settings
 
 # Need access to internal func to put users in the right group
@@ -26,13 +19,13 @@ from django_comment_client.models import Role, FORUM_ROLE_ADMINISTRATOR, \
 from django_comment_client.utils import has_forum_access
 
 from courseware.access import _course_staff_group_name
-import courseware.tests.tests as ct
+from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE, get_user
 from xmodule.modulestore.django import modulestore
 import xmodule.modulestore.django
 
 
-@override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE)
-class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
+@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+class TestInstructorDashboardGradeDownloadCSV(LoginEnrollmentTestCase):
     '''
     Check for download of csv
     '''
@@ -55,7 +48,7 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
         def make_instructor(course):
             group_name = _course_staff_group_name(course.location)
             g = Group.objects.create(name=group_name)
-            g.user_set.add(ct.user(self.instructor))
+            g.user_set.add(get_user(self.instructor))
 
         make_instructor(self.toy)
 
@@ -63,7 +56,6 @@ class TestInstructorDashboardGradeDownloadCSV(ct.PageLoader):
         self.login(self.instructor, self.password)
         self.enroll(self.toy)
 
-
     def test_download_grades_csv(self):
         course = self.toy
         url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
@@ -101,9 +93,8 @@ def action_name(operation, rolename):
         return '{0} forum {1}'.format(operation, FORUM_ADMIN_ACTION_SUFFIX[rolename])
 
 
-
-@override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE)
-class TestInstructorDashboardForumAdmin(ct.PageLoader):
+@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+class TestInstructorDashboardForumAdmin(LoginEnrollmentTestCase):
     '''
     Check for change in forum admin role memberships
     '''
@@ -112,7 +103,6 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader):
         xmodule.modulestore.django._MODULESTORES = {}
         courses = modulestore().get_courses()
 
-
         self.course_id = "edX/toy/2012_Fall"
         self.toy = modulestore().get_course(self.course_id)
 
@@ -127,14 +117,12 @@ class TestInstructorDashboardForumAdmin(ct.PageLoader):
 
         group_name = _course_staff_group_name(self.toy.location)
         g = Group.objects.create(name=group_name)
-        g.user_set.add(ct.user(self.instructor))
+        g.user_set.add(get_user(self.instructor))
 
         self.logout()
         self.login(self.instructor, self.password)
         self.enroll(self.toy)
 
-
-
     def initialize_roles(self, course_id):
         self.admin_role = Role.objects.get_or_create(name=FORUM_ROLE_ADMINISTRATOR, course_id=course_id)[0]
         self.moderator_role = Role.objects.get_or_create(name=FORUM_ROLE_MODERATOR, course_id=course_id)[0]

lms/djangoapps/licenses/tests.py

+"""Tests for License package"""
 import logging
+import json
+
 from uuid import uuid4
 from random import shuffle
 from tempfile import NamedTemporaryFile
+from factory import Factory, SubFactory
 
 from django.test import TestCase
 from django.core.management import call_command
-
-from .models import CourseSoftware, UserLicense
+from django.core.urlresolvers import reverse
+from licenses.models import CourseSoftware, UserLicense
+from courseware.tests.tests import LoginEnrollmentTestCase, get_user
 
 COURSE_1 = 'edX/toy/2012_Fall'
 
 SOFTWARE_1 = 'matlab'
 SOFTWARE_2 = 'stata'
 
+SERIAL_1 = '123456abcde'
+
 log = logging.getLogger(__name__)
 
 
+class CourseSoftwareFactory(Factory):
+    '''Factory for generating CourseSoftware objects in database'''
+    FACTORY_FOR = CourseSoftware
+
+    name = SOFTWARE_1
+    full_name = SOFTWARE_1
+    url = SOFTWARE_1
+    course_id = COURSE_1
+
+
+class UserLicenseFactory(Factory):
+    '''
+    Factory for generating UserLicense objects in database
+
+    By default, the user assigned is null, indicating that the
+    serial number has not yet been assigned.
+    '''
+    FACTORY_FOR = UserLicense
+
+    software = SubFactory(CourseSoftwareFactory)
+    serial = SERIAL_1
+
+
+class LicenseTestCase(LoginEnrollmentTestCase):
+    '''Tests for licenses.views'''
+    def setUp(self):
+        '''creates a user and logs in'''
+        self.setup_viewtest_user()
+        self.software = CourseSoftwareFactory()
+
+    def test_get_license(self):
+        UserLicenseFactory(user=get_user(self.viewtest_email), software=self.software)
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'false'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        self.assertEqual(200, response.status_code)
+        json_returned = json.loads(response.content)
+        self.assertFalse('error' in json_returned)
+        self.assertTrue('serial' in json_returned)
+        self.assertEquals(json_returned['serial'], SERIAL_1)
+
+    def test_get_nonexistent_license(self):
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'false'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        self.assertEqual(200, response.status_code)
+        json_returned = json.loads(response.content)
+        self.assertFalse('serial' in json_returned)
+        self.assertTrue('error' in json_returned)
+
+    def test_create_nonexistent_license(self):
+        '''Should not assign a license to an unlicensed user when none are available'''
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'true'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        self.assertEqual(200, response.status_code)
+        json_returned = json.loads(response.content)
+        self.assertFalse('serial' in json_returned)
+        self.assertTrue('error' in json_returned)
+
+    def test_create_license(self):
+        '''Should assign a license to an unlicensed user if one is unassigned'''
+        # create an unassigned license
+        UserLicenseFactory(software=self.software)
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'true'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        self.assertEqual(200, response.status_code)
+        json_returned = json.loads(response.content)
+        self.assertFalse('error' in json_returned)
+        self.assertTrue('serial' in json_returned)
+        self.assertEquals(json_returned['serial'], SERIAL_1)
+
+    def test_get_license_from_wrong_course(self):
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'false'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format('some/other/course'))
+        self.assertEqual(404, response.status_code)
+
+    def test_get_license_from_non_ajax(self):
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'false'},
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        self.assertEqual(404, response.status_code)
+
+    def test_get_license_without_software(self):
+        response = self.client.post(reverse('user_software_license'),
+                                    {'generate': 'false'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        self.assertEqual(404, response.status_code)
+
+    def test_get_license_without_login(self):
+        self.logout()
+        response = self.client.post(reverse('user_software_license'),
+                                    {'software': SOFTWARE_1, 'generate': 'false'},
+                                    HTTP_X_REQUESTED_WITH='XMLHttpRequest',
+                                    HTTP_REFERER='/courses/{0}/some_page'.format(COURSE_1))
+        # if we're not logged in, we should be referred to the login page
+        self.assertEqual(302, response.status_code)
+
+
 class CommandTest(TestCase):
+    '''Test management command for importing serial numbers'''
+
     def test_import_serial_numbers(self):
         size = 20
 
@@ -51,31 +167,33 @@ class CommandTest(TestCase):
         licenses_count = UserLicense.objects.all().count()
         self.assertEqual(3 * size, licenses_count)
 
-        cs = CourseSoftware.objects.get(pk=1)
+        software = CourseSoftware.objects.get(pk=1)
 
-        lics = UserLicense.objects.filter(software=cs)[:size]
+        lics = UserLicense.objects.filter(software=software)[:size]
         known_serials = list(l.serial for l in lics)
         known_serials.extend(generate_serials(10))
 
         shuffle(known_serials)
 
         log.debug('Adding some new and old serials to {0}'.format(SOFTWARE_1))
-        with NamedTemporaryFile() as f:
-            f.write('\n'.join(known_serials))
-            f.flush()
-            args = [COURSE_1, SOFTWARE_1, f.name]
+        with NamedTemporaryFile() as tmpfile:
+            tmpfile.write('\n'.join(known_serials))
+            tmpfile.flush()
+            args = [COURSE_1, SOFTWARE_1, tmpfile.name]
             call_command('import_serial_numbers', *args)
 
         log.debug('Check if we added only the new ones')
-        licenses_count = UserLicense.objects.filter(software=cs).count()
+        licenses_count = UserLicense.objects.filter(software=software).count()
         self.assertEqual((2 * size) + 10, licenses_count)
 
 
 def generate_serials(size=20):
+    '''generate a list of serial numbers'''
     return [str(uuid4()) for _ in range(size)]
 
 
 def generate_serials_file(size=20):
+    '''output list of generated serial numbers to a temp file'''
     serials = generate_serials(size)
 
     temp_file = NamedTemporaryFile()

lms/djangoapps/licenses/views.py

@@ -7,12 +7,13 @@ from collections import namedtuple, defaultdict
 
 from mitxmako.shortcuts import render_to_string
 
+from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
 from django.http import HttpResponse, Http404
-from django.views.decorators.csrf import requires_csrf_token, csrf_protect
+from django.views.decorators.csrf import requires_csrf_token
 
-from .models import CourseSoftware
-from .models import get_courses_licenses, get_or_create_license, get_license
+from licenses.models import CourseSoftware
+from licenses.models import get_courses_licenses, get_or_create_license, get_license
 
 
 log = logging.getLogger("mitx.licenses")
@@ -44,6 +45,7 @@ def get_licenses_by_course(user, courses):
     return data_by_course
 
 
+@login_required
 @requires_csrf_token
 def user_software_license(request):
     if request.method != 'POST' or not request.is_ajax():
@@ -65,19 +67,21 @@ def user_software_license(request):
     try:
         software = CourseSoftware.objects.get(name=software_name,
                                               course_id=course_id)
-        print software
     except CourseSoftware.DoesNotExist:
         raise Http404
 
+    try:
         user = User.objects.get(id=user_id)
+    except User.DoesNotExist:
+        raise Http404
 
     if generate:
-        license = get_or_create_license(user, software)
+        software_license = get_or_create_license(user, software)
     else:
-        license = get_license(user, software)
+        software_license = get_license(user, software)
 
-    if license:
-        response = {'serial': license.serial}
+    if software_license:
+        response = {'serial': software_license.serial}
     else:
         response = {'error': 'No serial number found'}
 

lms/djangoapps/open_ended_grading/tests.py

@@ -4,22 +4,22 @@ Tests for open ended grading interfaces
 django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/open_ended_grading
 """
 
-from django.test import TestCase
-from open_ended_grading import staff_grading_service
-from xmodule.open_ended_grading_classes import peer_grading_service
-from xmodule import peer_grading_module
+import json
+from mock import MagicMock
+
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import Group
+from mitxmako.shortcuts import render_to_string
 
-from courseware.access import _course_staff_group_name
-import courseware.tests.tests as ct
+from xmodule.open_ended_grading_classes import peer_grading_service
+from xmodule import peer_grading_module
 from xmodule.modulestore.django import modulestore
 import xmodule.modulestore.django
-from nose import SkipTest
-from mock import patch, Mock, MagicMock
-import json
 from xmodule.x_module import ModuleSystem
-from mitxmako.shortcuts import render_to_string
+
+from open_ended_grading import staff_grading_service
+from courseware.access import _course_staff_group_name
+from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE, get_user
 
 import logging
 
@@ -30,8 +30,8 @@ from django.http import QueryDict
 from xmodule.tests import test_util_open_ended
 
 
-@override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE)
-class TestStaffGradingService(ct.PageLoader):
+@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+class TestStaffGradingService(LoginEnrollmentTestCase):
     '''
     Check that staff grading service proxy works.  Basically just checking the
     access control and error handling logic -- all the actual work is on the
@@ -56,7 +56,7 @@ class TestStaffGradingService(ct.PageLoader):
         def make_instructor(course):
             group_name = _course_staff_group_name(course.location)
             g = Group.objects.create(name=group_name)
-            g.user_set.add(ct.user(self.instructor))
+            g.user_set.add(get_user(self.instructor))
 
         make_instructor(self.toy)
 
@@ -126,8 +126,8 @@ class TestStaffGradingService(ct.PageLoader):
         self.assertIsNotNone(d['problem_list'])
 
 
-@override_settings(MODULESTORE=ct.TEST_DATA_XML_MODULESTORE)
-class TestPeerGradingService(ct.PageLoader):
+@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
+class TestPeerGradingService(LoginEnrollmentTestCase):
     '''
     Check that staff grading service proxy works.  Basically just checking the
     access control and error handling logic -- all the actual work is on the