Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 116 additions & 32 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#pylint: disable=E1101

import json
import shutil
import mock

Expand All @@ -15,6 +14,7 @@
import copy
from json import loads
from datetime import timedelta
from django.test import TestCase

from django.contrib.auth.models import User
from django.dispatch import Signal
Expand Down Expand Up @@ -53,6 +53,7 @@
from uuid import uuid4
from pymongo import MongoClient
from student.models import CourseEnrollment
import re

from contentstore.utils import delete_course_and_groups
from xmodule.modulestore.django import loc_mapper
Expand Down Expand Up @@ -135,6 +136,8 @@ def check_components_on_page(self, component_types, expected_types):

resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment after edit_unit no longer using locations.
# _test_no_locations(self, resp)

for expected in expected_types:
self.assertIn(expected, resp.content)
Expand All @@ -160,15 +163,21 @@ def test_malformed_edit_unit_request(self):

resp = self.client.get_html(reverse('edit_unit', kwargs={'location': location.url()}))
self.assertEqual(resp.status_code, 400)
_test_no_locations(self, resp, status_code=400)

def check_edit_unit(self, test_course_name):
import_from_xml(modulestore('direct'), 'common/test/data/', [test_course_name])

for descriptor in modulestore().get_items(Location(None, None, 'vertical', None, None)):
items = modulestore().get_items(Location('i4x', 'edX', test_course_name, 'vertical', None, None))
# Assert is here to make sure that the course being tested actually has verticals.
self.assertGreater(len(items), 0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be worthwhile to have a comment here saying that the check is to ensure the test is meaningful or something like that. (Just in case someone says, "there's no verticals in the source for ____" they should then say, "oh, that makes this test a noop")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

for descriptor in items:
print "Checking ", descriptor.location.url()
print descriptor.__class__, descriptor.location
resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment after edit_unit not using locations.
# _test_no_locations(self, resp)

def lockAnAsset(self, content_store, course_location):
"""
Expand Down Expand Up @@ -483,6 +492,8 @@ def _test_preview(self, location):
)
resp = self.client.get_html(locator.url_reverse('xblock'))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when preview no longer has locations being returned.
# _test_no_locations(self, resp)
return resp

def test_delete(self):
Expand Down Expand Up @@ -841,6 +852,7 @@ def test_illegal_draft_crud_ops(self):
def test_bad_contentstore_request(self):
resp = self.client.get_html('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png')
self.assertEqual(resp.status_code, 400)
_test_no_locations(self, resp, 400)

def test_rewrite_nonportable_links_on_import(self):
module_store = modulestore('direct')
Expand Down Expand Up @@ -1026,12 +1038,11 @@ def check_import(self, module_store, root_dir, draft_store, content_store, stub_
items = module_store.get_items(stub_location.replace(category='vertical', name=None))
self.assertGreater(len(items), 0)
for descriptor in items:
# don't try to look at private verticals. Right now we're running
# the service in non-draft aware
if getattr(descriptor, 'is_draft', False):
print "Checking {0}....".format(descriptor.location.url())
resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
self.assertEqual(resp.status_code, 200)
print "Checking {0}....".format(descriptor.location.url())
resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when edit_unit no longer has locations.
# _test_no_locations(self, resp)

# verify that we have the content in the draft store as well
vertical = draft_store.get_item(
Expand Down Expand Up @@ -1508,6 +1519,7 @@ def test_course_index_view_with_no_courses(self):
status_code=200,
html=True
)
_test_no_locations(self, resp)

def test_course_factory(self):
"""Test that the course factory works correctly."""
Expand All @@ -1530,6 +1542,8 @@ def test_course_index_view_with_course(self):
status_code=200,
html=True
)
# TODO: uncomment when course index no longer has locations being returned.
# _test_no_locations(self, resp)

def test_course_overview_view_with_course(self):
"""Test viewing the course overview page with an existing course"""
Expand Down Expand Up @@ -1589,6 +1603,13 @@ def test_cms_imported_course_walkthrough(self):
Import and walk through some common URL endpoints. This just verifies non-500 and no other
correct behavior, so it is not a deep test
"""
def test_get_html(page):
# Helper function for getting HTML for a page in Studio and
# checking that it does not error.
resp = self.client.get_html(new_location.url_reverse(page))
self.assertEqual(resp.status_code, 200)
_test_no_locations(self, resp)

import_from_xml(modulestore('direct'), 'common/test/data/', ['simple'])
loc = Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None])
new_location = loc_mapper().translate_location(loc.course_id, loc, False, True)
Expand All @@ -1598,62 +1619,78 @@ def test_cms_imported_course_walkthrough(self):
self.assertContains(resp, 'Chapter 2')

# go to various pages

# import page
resp = self.client.get_html(new_location.url_reverse('import/', ''))
self.assertEqual(resp.status_code, 200)

# export page
resp = self.client.get_html(new_location.url_reverse('export/', ''))
self.assertEqual(resp.status_code, 200)

# course team
url = new_location.url_reverse('course_team/', '')
resp = self.client.get_html(url)
self.assertEqual(resp.status_code, 200)

# course info
resp = self.client.get(new_location.url_reverse('course_info'))
self.assertEqual(resp.status_code, 200)
test_get_html('import')
test_get_html('export')
test_get_html('course_team')
test_get_html('course_info')
test_get_html('checklists')
test_get_html('assets')

# settings_details
resp = self.client.get(reverse('settings_details',
resp = self.client.get_html(reverse('settings_details',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge conflict w/ my PR

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. But if you merge will this change, it will make sure you got rid of all locations, and it will handle some of the "must remember to un-comment later" issues.

kwargs={'org': loc.org,
'course': loc.course,
'name': loc.name}))
self.assertEqual(resp.status_code, 200)
_test_no_locations(self, resp)

# settings_details
resp = self.client.get(reverse('settings_grading',
resp = self.client.get_html(reverse('settings_grading',
kwargs={'org': loc.org,
'course': loc.course,
'name': loc.name}))
self.assertEqual(resp.status_code, 200)

# assets_handler (HTML for full page content)
url = new_location.url_reverse('assets/', '')
resp = self.client.get_html(url)
# TODO: uncomment when grading is not using old locations.
# _test_no_locations(self, resp)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be uncommented after my PR lands


# advanced settings
resp = self.client.get_html(reverse('course_advanced_settings',
kwargs={'org': loc.org,
'course': loc.course,
'name': loc.name}))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when advanced settings not using old locations.
# _test_no_locations(self, resp)

# textbook index
resp = self.client.get_html(reverse('textbook_index',
kwargs={'org': loc.org,
'course': loc.course,
'name': loc.name}))
self.assertEqual(resp.status_code, 200)
_test_no_locations(self, resp)

# go look at a subsection page
subsection_location = loc.replace(category='sequential', name='test_sequence')
resp = self.client.get_html(
reverse('edit_subsection', kwargs={'location': subsection_location.url()})
)
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when grading and outline not using old locations.
# _test_no_locations(self, resp)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another race condition w/ my PR


# go look at the Edit page
unit_location = loc.replace(category='vertical', name='test_vertical')
resp = self.client.get_html(
reverse('edit_unit', kwargs={'location': unit_location.url()}))
self.assertEqual(resp.status_code, 200)
# TODO: uncomment when edit_unit not using old locations.
# _test_no_locations(self, resp)

resp = self.client.get_html(reverse('edit_tabs',
kwargs={'org': loc.org,
'course': loc.course,
'coursename': loc.name}))
self.assertEqual(resp.status_code, 200)
_test_no_locations(self, resp)

def delete_item(category, name):
""" Helper method for testing the deletion of an xblock item. """
del_loc = loc.replace(category=category, name=name)
del_location = loc_mapper().translate_location(loc.course_id, del_loc, False, True)
resp = self.client.delete(del_location.url_reverse('xblock'))
self.assertEqual(resp.status_code, 204)
_test_no_locations(self, resp, status_code=204, html=False)

# delete a component
delete_item(category='html', name='test_html')
Expand Down Expand Up @@ -1853,7 +1890,10 @@ def _show_course_overview(self, location):
Show the course overview page.
"""
new_location = loc_mapper().translate_location(location.course_id, location, False, True)
return self.client.get_html(new_location.url_reverse('course/', ''))
resp = self.client.get_html(new_location.url_reverse('course/', ''))
# TODO: uncomment when i4x no longer in overview.
# _test_no_locations(self, resp)
return resp


@override_settings(MODULESTORE=TEST_MODULESTORE)
Expand Down Expand Up @@ -1920,6 +1960,32 @@ def test_metadata_persistence(self):
pass


class EntryPageTestCase(TestCase):
"""
Tests entry pages that aren't specific to a course.
"""
def setUp(self):
self.client = AjaxEnabledTestClient()

def _test_page(self, page, status_code=200):
resp = self.client.get_html(reverse(page))
self.assertEqual(resp.status_code, status_code)
_test_no_locations(self, resp, status_code)

def test_how_it_works(self):
self._test_page("howitworks")

def test_signup(self):
self._test_page("signup")

def test_login(self):
self._test_page("login")

def test_logout(self):
# Logout redirects.
self._test_page("logout", 302)


def _create_course(test, course_data):
"""
Creates a course via an AJAX request and verifies the URL returned in the response.
Expand All @@ -1945,3 +2011,21 @@ def _course_factory_create_course():
def _get_course_id(test_course_data):
"""Returns the course ID (org/number/run)."""
return "{org}/{number}/{run}".format(**test_course_data)


def _test_no_locations(test, resp, status_code=200, html=True):
"""
Verifies that "i4x", which appears in old locations, but not
new locators, does not appear in the HTML response output.
Used to verify that database refactoring is complete.
"""
test.assertNotContains(resp, 'i4x', status_code=status_code, html=html)
if html:
# For HTML pages, it is nice to call the method with html=True because
# it checks that the HTML properly parses. However, it won't find i4x usages
# in JavaScript blocks.
content = resp.content
num_jump_to = len(re.findall(r"8000(\S)*jump_to/i4x", content))
total_i4x = len(re.findall(r"i4x", content))

test.assertEqual(total_i4x - num_jump_to, 0, "i4x found outside of LMS jump-to links")
5 changes: 1 addition & 4 deletions cms/static/js/models/course_info.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@ define(["backbone"], function(Backbone) {
url: '',

defaults: {
"courseId": "", // the location url
"updates" : null, // UpdateCollection
"handouts": null // HandoutCollection
},

idAttribute : "courseId"
}
});
return CourseInfo;
});
2 changes: 1 addition & 1 deletion cms/templates/asset_index.html
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ <h3 class="title-3">${_("What can I do on this page?")}</h3>
<a href="#" class="close-button"><i class="icon-remove-sign"></i> <span class="sr">${_('close')}</span></a>
<div class="modal-body">
<h1 class="title">${_("Upload New File")}</h1>
<p class="file-name"></a>
<p class="file-name">
<div class="progress-bar">
<div class="progress-fill"></div>
</div>
Expand Down
1 change: 0 additions & 1 deletion cms/templates/course_info.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
var editor = new CourseInfoEditView({
el: $('.main-wrapper'),
model : new CourseInfoModel({
courseId : '${context_course.location}',
updates : course_updates,
base_asset_url : '${base_asset_url}',
handouts : course_handouts
Expand Down
2 changes: 1 addition & 1 deletion cms/templates/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ <h2 class="title-2">${_("Basic Information")}</h2>
<div class="note note-promotion note-promotion-courseURL has-actions">
<h3 class="title">${_("Course Summary Page")} <span class="tip">${_("(for student enrollment and access)")}</span></h3>
<div class="copy">
<p><a class="link-courseURL" rel="external" href="https:${utils.get_lms_link_for_about_page(course_location)}" />https:${utils.get_lms_link_for_about_page(course_location)}</a></p>
<p><a class="link-courseURL" rel="external" href="https:${utils.get_lms_link_for_about_page(course_location)}" >https:${utils.get_lms_link_for_about_page(course_location)}</a></p>
</div>

<ul class="list-actions">
Expand Down
15 changes: 13 additions & 2 deletions cms/templates/widgets/segment-io.html
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
<%!
from xmodule.modulestore.django import loc_mapper
%>

% if context_course:
<%
ctx_loc = context_course.location
locator = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)
%>
% endif

% if settings.MITX_FEATURES.get('SEGMENT_IO'):
<!-- begin Segment.io -->
<script type="text/javascript">
// if inside course, inject the course location into the JS namespace
%if context_course:
var course_location_analytics = "${context_course.location}";
var course_location_analytics = "${locator}";
%endif

var analytics=analytics||[];analytics.load=function(e){var t=document.createElement("script");t.type="text/javascript",t.async=!0,t.src=("https:"===document.location.protocol?"https://":"http://")+"d2dq2ahtl5zl1z.cloudfront.net/analytics.js/v1/"+e+"/analytics.min.js";var n=document.getElementsByTagName("script")[0];n.parentNode.insertBefore(t,n);var r=function(e){return function(){analytics.push([e].concat(Array.prototype.slice.call(arguments,0)))}},i=["identify","track","trackLink","trackForm","trackClick","trackSubmit","pageview","ab","alias","ready"];for(var s=0;s<i.length;s++)analytics[i[s]]=r(i[s])};
Expand All @@ -22,7 +33,7 @@
<!-- dummy segment.io -->
<script type="text/javascript">
%if context_course:
var course_location_analytics = "${context_course.location}";
var course_location_analytics = "${locator}";
%endif
var analytics = {
"track": function() {}
Expand Down