From 71a1562e21a1448a21a0cd224a02a8627b67fb98 Mon Sep 17 00:00:00 2001 From: Kristin Stephens Date: Thu, 25 Jul 2013 14:25:58 -0700 Subject: [PATCH 001/206] Fix offline_gradecalc Bug: Throws error when can't find a user on the request object for grading or can't find a session attribute. Fix: Added a user attribute with a value of the current student and a session attribute with a value of an empty dictionary. --- lms/djangoapps/instructor/offline_gradecalc.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/offline_gradecalc.py b/lms/djangoapps/instructor/offline_gradecalc.py index fe5b95c3b9..97f252e3a9 100644 --- a/lms/djangoapps/instructor/offline_gradecalc.py +++ b/lms/djangoapps/instructor/offline_gradecalc.py @@ -44,12 +44,14 @@ def offline_grade_calculation(course_id): def is_secure(self): return False - request = DummyRequest() - print "%d enrolled students" % len(enrolled_students) course = get_course_by_id(course_id) for student in enrolled_students: + request = DummyRequest() + request.user = student + request.session = {} + gradeset = grades.grade(student, request, course, keep_raw_scores=True) gs = enc.encode(gradeset) ocg, created = models.OfflineComputedGrade.objects.get_or_create(user=student, course_id=course_id) From 02463398195b2cc99adc13ef867df49c2e7e8e0e Mon Sep 17 00:00:00 2001 From: Sef Kloninger Date: Wed, 31 Jul 2013 13:20:29 -0700 Subject: [PATCH 002/206] fix crash in peer eval xmodule As documented in Jira LMS-806: Steps to reproduce: * as one user, for a peer evaluated problem, enter something that needs peer grading * as another user, log in and fire up the peer grading panel. You should see something that needs attention, a yellow exclamation badge * clicking on the "do grading" panel will fail with a 500, stacktrace attached. * it's possible that the question where this is being done must first have professor calibration done in order for peers to be able to actually get work to grade. Don Mitchell debugged this with me and saw some problems in the ways that the peer_evaluation xmodule code was accessing dates. Instead of using the normal lms accessors, by going straight into the _module_data it would be bypassing some important code (caching?). Fixing this to be more standard did the trick. --- .../xmodule/xmodule/peer_grading_module.py | 19 +++++++++---------- common/lib/xmodule/xmodule/timeinfo.py | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index 09cac9a6b4..05883ce8c0 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -12,7 +12,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError from .timeinfo import TimeInfo from xblock.core import Dict, String, Scope, Boolean, Integer, Float -from xmodule.fields import Date +from xmodule.fields import Date, Timedelta from xmodule.open_ended_grading_classes.peer_grading_service import PeerGradingService, GradingServiceError, MockPeerGradingService from open_ended_grading_classes import combined_open_ended_rubric @@ -47,9 +47,8 @@ class PeerGradingFields(object): help="Due date that should be displayed.", default=None, scope=Scope.settings) - grace_period_string = String( + graceperiod = Timedelta( help="Amount of grace to give on the due date.", - default=None, scope=Scope.settings ) student_data_for_location = Dict( @@ -105,12 +104,12 @@ class PeerGradingModule(PeerGradingFields, XModule): log.error("Linked location {0} for peer grading module {1} does not exist".format( self.link_to_location, self.location)) raise - due_date = self.linked_problem._model_data.get('due', None) + due_date = self.linked_problem.lms.due if due_date: - self._model_data['due'] = due_date + self.lms.due = due_date try: - self.timeinfo = TimeInfo(self.due, self.grace_period_string) + self.timeinfo = TimeInfo(self.due, self.graceperiod) except Exception: log.error("Error parsing due date information in location {0}".format(self.location)) raise @@ -533,10 +532,10 @@ class PeerGradingModule(PeerGradingFields, XModule): problem_location = problem['location'] descriptor = _find_corresponding_module_for_location(problem_location) if descriptor: - problem['due'] = descriptor._model_data.get('due', None) - grace_period_string = descriptor._model_data.get('graceperiod', None) + problem['due'] = descriptor.lms.due + grace_period = descriptor.lms.graceperiod try: - problem_timeinfo = TimeInfo(problem['due'], grace_period_string) + problem_timeinfo = TimeInfo(problem['due'], grace_period) except: log.error("Malformed due date or grace period string for location {0}".format(problem_location)) raise @@ -629,5 +628,5 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor): @property def non_editable_metadata_fields(self): non_editable_fields = super(PeerGradingDescriptor, self).non_editable_metadata_fields - non_editable_fields.extend([PeerGradingFields.due, PeerGradingFields.grace_period_string]) + non_editable_fields.extend([PeerGradingFields.due, PeerGradingFields.graceperiod]) return non_editable_fields diff --git a/common/lib/xmodule/xmodule/timeinfo.py b/common/lib/xmodule/xmodule/timeinfo.py index 8f4d99506a..76f24a0b23 100644 --- a/common/lib/xmodule/xmodule/timeinfo.py +++ b/common/lib/xmodule/xmodule/timeinfo.py @@ -14,20 +14,23 @@ class TimeInfo(object): """ _delta_standin = Timedelta() - def __init__(self, due_date, grace_period_string): + def __init__(self, due_date, grace_period_string_or_timedelta): if due_date is not None: self.display_due_date = due_date else: self.display_due_date = None - if grace_period_string is not None and self.display_due_date: - try: - self.grace_period = TimeInfo._delta_standin.from_json(grace_period_string) - self.close_date = self.display_due_date + self.grace_period - except: - log.error("Error parsing the grace period {0}".format(grace_period_string)) - raise + if grace_period_string_or_timedelta is not None and self.display_due_date: + if isinstance(grace_period_string_or_timedelta, basestring): + try: + self.grace_period = TimeInfo._delta_standin.from_json(grace_period_string_or_timedelta) + except: + log.error("Error parsing the grace period {0}".format(grace_period_string_or_timedelta)) + raise + else: + self.grace_period = grace_period_string_or_timedelta + self.close_date = self.display_due_date + self.grace_period else: self.grace_period = None self.close_date = self.display_due_date From 93df9c4c303c10591db046b4ecb0036677a9deea Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 09:41:02 -0400 Subject: [PATCH 003/206] resolve merge conflicts --- cms/templates/widgets/header.html | 2 +- common/lib/xmodule/xmodule/course_module.py | 25 +++++++++++++++++++ lms/djangoapps/course_wiki/views.py | 2 +- lms/djangoapps/courseware/courses.py | 4 +-- lms/templates/course.html | 4 +-- lms/templates/courseware/course_about.html | 19 +++++++++++++- lms/templates/courseware/courseware.html | 2 +- lms/templates/courseware/info.html | 2 +- .../courseware/mktg_course_about.html | 4 +-- lms/templates/courseware/progress.html | 2 +- lms/templates/courseware/static_tab.html | 2 +- lms/templates/courseware/syllabus.html | 2 +- lms/templates/dashboard.html | 16 ++++++++++-- lms/templates/discussion/index.html | 2 +- lms/templates/discussion/single_thread.html | 2 +- lms/templates/discussion/user_profile.html | 2 +- lms/templates/instructor/staff_grading.html | 2 +- lms/templates/navigation.html | 2 +- .../combined_notifications.html | 2 +- .../open_ended_flagged_problems.html | 2 +- .../open_ended_problems.html | 2 +- lms/templates/static_htmlbook.html | 3 ++- lms/templates/static_pdfbook.html | 3 ++- lms/templates/staticbook.html | 2 +- lms/templates/test_center_register.html | 4 +-- 25 files changed, 85 insertions(+), 29 deletions(-) diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 6178058689..9e39bae749 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -13,7 +13,7 @@

${_("Current Course:")} - ${ctx_loc.org}${ctx_loc.course} + ${context_course.display_org_with_default}${context_course.display_number_with_default} ${context_course.display_name_with_default}

diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 3c9f5c0683..b6ac48b7f5 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -362,6 +362,11 @@ class CourseFields(object): # Explicit comparison to True because we always want to return a bool. hide_progress_tab = Boolean(help="DO NOT USE THIS", scope=Scope.settings) + display_organization = String(help="An optional display string for the course organization that will get rendered in the LMS", + scope=Scope.settings) + + display_coursenumber = String(help="An optional display string for the course number that will get rendered in the LMS", + scope=Scope.settings) class CourseDescriptor(CourseFields, SequenceDescriptor): module_class = SequenceModule @@ -933,6 +938,26 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): def number(self): return self.location.course + @property + def display_number_with_default(self): + """ + Return a display course number if it has been specified, otherwise return the 'course' that is in the location + """ + if self.display_coursenumber: + return self.display_coursenumber + + return self.location.course + @property def org(self): return self.location.org + + @property + def display_org_with_default(self): + """ + Return a display organization if it has been specified, otherwise return the 'org' that is in the location + """ + if self.display_organization: + return self.display_organization + + return self.location.org diff --git a/lms/djangoapps/course_wiki/views.py b/lms/djangoapps/course_wiki/views.py index 74ef7d4a74..e92598a9a5 100644 --- a/lms/djangoapps/course_wiki/views.py +++ b/lms/djangoapps/course_wiki/views.py @@ -95,7 +95,7 @@ def course_wiki_redirect(request, course_id): root, course_slug, title=course_slug, - content="This is the wiki for **{0}**'s _{1}_.".format(course.org, course.display_name_with_default), + content="This is the wiki for **{0}**'s _{1}_.".format(course.display_org_with_default, course.display_name_with_default), user_message="Course page automatically created.", user=None, ip_address=None, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 91ec14011e..086f92a123 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -174,9 +174,9 @@ def get_course_about_section(course, section_key): elif section_key == "title": return course.display_name_with_default elif section_key == "university": - return course.location.org + return course.display_org_with_default elif section_key == "number": - return course.number + return course.display_number_with_default raise KeyError("Invalid about key " + str(section_key)) diff --git a/lms/templates/course.html b/lms/templates/course.html index ddbae2c715..01581ed764 100644 --- a/lms/templates/course.html +++ b/lms/templates/course.html @@ -14,13 +14,13 @@ from courseware.courses import course_image_url, get_course_about_section
-

${course.number} ${get_course_about_section(course, 'title')}

+

${course.display_number_with_default} ${get_course_about_section(course, 'title')}

➔
- ${course.number} ${get_course_about_section(course, 'title')} Cover Image + ${course.display_number_with_default} ${get_course_about_section(course, 'title')} Cover Image

${get_course_about_section(course, 'short_description')}

diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index b1bc715e9a..36cb7054d0 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -66,7 +66,11 @@ +<<<<<<< HEAD <%block name="title">${_("About {course.number}").format(course=course)} +======= +<%block name="title">About ${course.display_number_with_default} +>>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite.
@@ -75,7 +79,7 @@

- ${course.number}: ${get_course_about_section(course, "title")} + ${course.display_number_with_default}: ${get_course_about_section(course, "title")} % if not self.theme_enabled(): ${get_course_about_section(course, "university")} % endif @@ -87,13 +91,21 @@ %if show_courseware_link: %endif +<<<<<<< HEAD ${_("You are registered for this course {course.number}").format(course=course)} +======= + You are registered for this course (${course.display_number_with_default}) +>>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. %if show_courseware_link: ${_("View Courseware")} %endif %else: +<<<<<<< HEAD ${_("Register for {course.number}").format(course=course)} +======= + Register for ${course.display_number_with_default} +>>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite.
%endif

@@ -162,8 +174,13 @@
    +<<<<<<< HEAD

  1. ${_("Course Number")}

    ${course.number}

  2. ${_("Classes Start")}

    ${course.start_date_text}
    3. +======= +

  3. Course Number

    ${course.display_number_with_default}
    4. +

  4. Classes Start

    ${course.start_date_text}
    5. +>>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. ## We plan to ditch end_date (which is not stored in course metadata), ## but for backwards compatibility, show about/end_date blob if it exists. diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 8d033434f0..cb7a2f96fc 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -2,7 +2,7 @@ <%inherit file="/main.html" /> <%namespace name='static' file='/static_content.html'/> <%block name="bodyclass">courseware ${course.css_class} -<%block name="title">${_("{course_number} Courseware").format(course_number=course.number)} +<%block name="title">${_("{course_number} Courseware").format(course_number=course.display_number_with_default)} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index 4bb961428e..4d889b35f7 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course.number} Course Info").format(course=course)} +<%block name="title">${_("{course.number} Course Info").format(course=course.display_number_with_default)} <%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%! diff --git a/lms/templates/courseware/mktg_course_about.html b/lms/templates/courseware/mktg_course_about.html index a2c1bd7d0a..46f2060bbc 100644 --- a/lms/templates/courseware/mktg_course_about.html +++ b/lms/templates/courseware/mktg_course_about.html @@ -8,7 +8,7 @@ <%inherit file="../mktg_iframe.html" /> -<%block name="title">${_("About {course_number}").format(course_number=course.number)} +<%block name="title">${_("About {course_number}").format(course_number=course.display_number_with_default)} <%block name="bodyclass">view-partial-mktgregister @@ -52,7 +52,7 @@
    ${_("You Are Registered")}
    %endif %elif allow_registration: - ${_("Register for")} ${course.number} + ${_("Register for")} ${course.display_number_with_default} %else:
    ${_("Registration Is Closed")}
    %endif diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 51feed23ab..105ccebe86 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -8,7 +8,7 @@ <%namespace name="progress_graph" file="/courseware/progress_graph.js"/> -<%block name="title">${_("{course_number} Progress").format(course_number=course.number)} +<%block name="title">${_("{course_number} Progress").format(course_number=course.display_number_with_default)} <%! from django.core.urlresolvers import reverse diff --git a/lms/templates/courseware/static_tab.html b/lms/templates/courseware/static_tab.html index 5b6e1480dd..884c161496 100644 --- a/lms/templates/courseware/static_tab.html +++ b/lms/templates/courseware/static_tab.html @@ -6,7 +6,7 @@ <%static:css group='course'/> -<%block name="title">${course.number} ${tab['name']} +<%block name="title">${course.display_number_with_default} ${tab['name']} <%include file="/courseware/course_navigation.html" args="active_page='static_tab_{0}'.format(tab['url_slug'])" /> diff --git a/lms/templates/courseware/syllabus.html b/lms/templates/courseware/syllabus.html index f9b5455273..3499193377 100644 --- a/lms/templates/courseware/syllabus.html +++ b/lms/templates/courseware/syllabus.html @@ -6,7 +6,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course.number} Course Info").format(course=course)} +<%block name="title">${_("{course.display_number_with_default} Course Info").format(course=course)} <%include file="/courseware/course_navigation.html" args="active_page='syllabus'" /> <%! diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 6356b64209..2c565cfd9c 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -140,11 +140,19 @@ % if course.id in show_courseware_links_for: +<<<<<<< HEAD ${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}')} % else:
    ${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}')} +======= + ${course.display_number_with_default} ${course.display_name_with_default} Cover Image + + % else: +
    + ${course.display_number_with_default} ${course.display_name_with_default} Cover Image +>>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite.
    % endif @@ -162,9 +170,9 @@

    ${get_course_about_section(course, 'university')}

    % if course.id in show_courseware_links_for: - ${course.number} ${course.display_name_with_default} + ${course.display_number_with_default} ${course.display_name_with_default} % else: - ${course.number} ${course.display_name_with_default} + ${course.display_number_with_default} ${course.display_name_with_default} % endif

    @@ -197,6 +205,7 @@ % endif % if registration.is_rejected:
    +<<<<<<< HEAD

    ${_("Your registration for the Pearson exam has been rejected. Please {link_start}see your registration status details{link_end}.").format( link_start=''.format(url=testcenter_register_target), @@ -206,6 +215,9 @@ link_end='', email="exam-help@edx.org", )} +======= +

    Your registration for the Pearson exam has been rejected. Please see your registration status details. Otherwise contact edX at exam-help@edx.org for further help.

    +>>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite.
    % endif % if not registration.is_accepted and not registration.is_rejected: diff --git a/lms/templates/discussion/index.html b/lms/templates/discussion/index.html index 3b6937b7d5..de53433790 100644 --- a/lms/templates/discussion/index.html +++ b/lms/templates/discussion/index.html @@ -6,7 +6,7 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion -<%block name="title">${_("Discussion - {course_number}").format(course_number=course.number) | h} +<%block name="title">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default) | h} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/discussion/single_thread.html b/lms/templates/discussion/single_thread.html index 6aae08a7a7..5bd6c6ca03 100644 --- a/lms/templates/discussion/single_thread.html +++ b/lms/templates/discussion/single_thread.html @@ -7,7 +7,7 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion -<%block name="title">${_("Discussion - {course_number}").format(course_number=course.number) | h} +<%block name="title">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default) | h} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/discussion/user_profile.html b/lms/templates/discussion/user_profile.html index 1c1901c10b..dbfa79d4e7 100644 --- a/lms/templates/discussion/user_profile.html +++ b/lms/templates/discussion/user_profile.html @@ -4,7 +4,7 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion -<%block name="title">${_("Discussion - {course_number}").format(course_number=course.number) | h} +<%block name="title">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default) | h} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index fb6766fe15..fee2275927 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course_number} Staff Grading").format(course_number=course.number)} +<%block name="title">${_("{course_number} Staff Grading").format(course_number=course.display_number_with_default)} <%include file="/courseware/course_navigation.html" args="active_page='staff_grading'" /> diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index 589d12666d..3ca3ae2d1d 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -50,7 +50,7 @@ site_status_msg = get_site_status_msg(course_id) % if course: -

    ${course.org}: ${course.number} ${course.display_name_with_default}

    +

    ${course.display_org_with_default}: ${course.display_number_with_default} ${course.display_name_with_default}

    % endif % if user.is_authenticated(): diff --git a/lms/templates/open_ended_problems/combined_notifications.html b/lms/templates/open_ended_problems/combined_notifications.html index 86eb4083dd..b1149194d9 100644 --- a/lms/templates/open_ended_problems/combined_notifications.html +++ b/lms/templates/open_ended_problems/combined_notifications.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course_number} Combined Notifications").format(course_number=course.number)} +<%block name="title">${_("{course_number} Combined Notifications").format(course_number=course.display_number_with_default)} <%include file="/courseware/course_navigation.html" args="active_page='open_ended'" /> diff --git a/lms/templates/open_ended_problems/open_ended_flagged_problems.html b/lms/templates/open_ended_problems/open_ended_flagged_problems.html index ab60e54300..61ec49584c 100644 --- a/lms/templates/open_ended_problems/open_ended_flagged_problems.html +++ b/lms/templates/open_ended_problems/open_ended_flagged_problems.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course_number} Flagged Open Ended Problems").format(course_number=course.number)} +<%block name="title">${_("{course_number} Flagged Open Ended Problems").format(course_number=course.display_number_with_default)} <%include file="/courseware/course_navigation.html" args="active_page='open_ended_flagged_problems'" /> diff --git a/lms/templates/open_ended_problems/open_ended_problems.html b/lms/templates/open_ended_problems/open_ended_problems.html index 56b269d8b7..e8496b6647 100644 --- a/lms/templates/open_ended_problems/open_ended_problems.html +++ b/lms/templates/open_ended_problems/open_ended_problems.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course_number} Open Ended Problems").format(course_number=course.number)} +<%block name="title">${_("{course_number} Open Ended Problems").format(course_number=course.display_number_with_default)} <%include file="/courseware/course_navigation.html" args="active_page='open_ended_problems'" /> diff --git a/lms/templates/static_htmlbook.html b/lms/templates/static_htmlbook.html index 67274e30c9..9f8f4bb3d5 100644 --- a/lms/templates/static_htmlbook.html +++ b/lms/templates/static_htmlbook.html @@ -2,7 +2,8 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> -<%block name="title">${_('{course_number} Textbook').format(course_number=course.number)} +<%block name="title">${_('{course_number} Textbook').format(course_number=course.display_number_with_default)} + <%block name="headextra"> diff --git a/lms/templates/static_pdfbook.html b/lms/templates/static_pdfbook.html index dfa7a47157..a38e9ff541 100644 --- a/lms/templates/static_pdfbook.html +++ b/lms/templates/static_pdfbook.html @@ -5,7 +5,8 @@ <%block name="title"> -${_('{course_number} Textbook').format(course_number=course.number)} +${_('{course_number} Textbook').format(course_number=course.display_number_with_default)} + <%block name="headextra"> diff --git a/lms/templates/staticbook.html b/lms/templates/staticbook.html index 980448b482..157af3a061 100644 --- a/lms/templates/staticbook.html +++ b/lms/templates/staticbook.html @@ -2,7 +2,7 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> -<%block name="title">${_("{course_number} Textbook").format(course_number=course.number)} +<%block name="title">${_("{course_number} Textbook").format(course_number=course.display_number_with_default)} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/test_center_register.html b/lms/templates/test_center_register.html index df6845e577..ba88cfd6dd 100644 --- a/lms/templates/test_center_register.html +++ b/lms/templates/test_center_register.html @@ -95,7 +95,7 @@
    -

    ${get_course_about_section(course, 'university')} ${course.number} ${course.display_name_with_default}

    +

    ${get_course_about_section(course, 'university')} ${course.display_number_with_default} ${course.display_name_with_default}

    % if registration:

    ${_('Your Pearson VUE Proctored Exam Registration')}

    @@ -442,7 +442,7 @@ % endif
    -

    ${_("About {university} {course_number}").format(university=get_course_about_section(course, 'university'), course_number=course.number)}

    +

    ${_("About {university} {course_number}").format(university=get_course_about_section(course, 'university'), course_number=course.display_number_with_default)}

    % if course.has_ended(): ${_('Course Completed:')} ${course.end_date_text} From 3a2bb7ba0156dd3f7ba1ec4e637d1cde30a8b866 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Jul 2013 15:07:19 -0400 Subject: [PATCH 004/206] add escaping --- cms/templates/widgets/header.html | 2 +- common/lib/xmodule/xmodule/course_module.py | 9 +++++---- lms/templates/course.html | 4 ++-- lms/templates/courseware/course_about.html | 14 +++++++++++++- lms/templates/courseware/courseware.html | 4 ++++ lms/templates/courseware/info.html | 4 ++++ lms/templates/courseware/mktg_course_about.html | 8 ++++++++ lms/templates/courseware/progress.html | 4 ++++ lms/templates/courseware/static_tab.html | 2 +- lms/templates/courseware/syllabus.html | 4 ++++ lms/templates/dashboard.html | 16 ++++++++++++++-- lms/templates/discussion/single_thread.html | 4 ++++ lms/templates/instructor/staff_grading.html | 4 ++++ lms/templates/navigation.html | 2 +- .../combined_notifications.html | 4 ++++ .../open_ended_flagged_problems.html | 4 ++++ .../open_ended_problems/open_ended_problems.html | 4 ++++ lms/templates/static_htmlbook.html | 4 ++++ lms/templates/static_pdfbook.html | 6 ++++++ lms/templates/staticbook.html | 4 ++++ lms/templates/test_center_register.html | 4 ++-- 21 files changed, 97 insertions(+), 14 deletions(-) diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 9e39bae749..a0d4d0fd41 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -13,7 +13,7 @@

    ${_("Current Course:")} - ${context_course.display_org_with_default}${context_course.display_number_with_default} + ${context_course.display_org_with_default | h}${context_course.display_number_with_default | h} ${context_course.display_name_with_default}

    diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index b6ac48b7f5..bf902e99bf 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -6,6 +6,7 @@ from path import path # NOTE (THK): Only used for detecting presence of syllabu import requests from datetime import datetime import dateutil.parser +import cgi from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor, SequenceModule @@ -944,9 +945,9 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): Return a display course number if it has been specified, otherwise return the 'course' that is in the location """ if self.display_coursenumber: - return self.display_coursenumber + return cgi.escape(self.display_coursenumber) - return self.location.course + return self.number @property def org(self): @@ -958,6 +959,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): Return a display organization if it has been specified, otherwise return the 'org' that is in the location """ if self.display_organization: - return self.display_organization + return cgi.escape(self.display_organization) - return self.location.org + return self.org diff --git a/lms/templates/course.html b/lms/templates/course.html index 01581ed764..4f2a78984f 100644 --- a/lms/templates/course.html +++ b/lms/templates/course.html @@ -14,13 +14,13 @@ from courseware.courses import course_image_url, get_course_about_section
    -

    ${course.display_number_with_default} ${get_course_about_section(course, 'title')}

    +

    ${course.display_number_with_default | h} ${get_course_about_section(course, 'title')}

    ➔
    - ${course.display_number_with_default} ${get_course_about_section(course, 'title')} Cover Image + ${course.display_number_with_default | h} ${get_course_about_section(course, 'title')} Cover Image

    ${get_course_about_section(course, 'short_description')}

    diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 36cb7054d0..f0b39adc0c 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -66,11 +66,15 @@ +<<<<<<< HEAD <<<<<<< HEAD <%block name="title">${_("About {course.number}").format(course=course)} ======= <%block name="title">About ${course.display_number_with_default} >>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. +======= +<%block name="title">About ${course.display_number_with_default | h} +>>>>>>> add escaping
    @@ -79,7 +83,7 @@

    - ${course.display_number_with_default}: ${get_course_about_section(course, "title")} + ${course.display_number_with_default | h}: ${get_course_about_section(course, "title")} % if not self.theme_enabled(): ${get_course_about_section(course, "university")} % endif @@ -101,11 +105,15 @@ %endif %else: +<<<<<<< HEAD <<<<<<< HEAD ${_("Register for {course.number}").format(course=course)} ======= Register for ${course.display_number_with_default} >>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. +======= + Register for ${course.display_number_with_default | h} +>>>>>>> add escaping
    %endif

    @@ -174,11 +182,15 @@
      +<<<<<<< HEAD <<<<<<< HEAD

    1. ${_("Course Number")}

      ${course.number}

    2. ${_("Classes Start")}

      ${course.start_date_text}
      3. =======

    3. Course Number

      ${course.display_number_with_default}
      4. +======= +

    4. Course Number

      ${course.display_number_with_default | h}
      5. +>>>>>>> add escaping

    5. Classes Start

      ${course.start_date_text}
      6. >>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index cb7a2f96fc..cc5646b3a4 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -2,7 +2,11 @@ <%inherit file="/main.html" /> <%namespace name='static' file='/static_content.html'/> <%block name="bodyclass">courseware ${course.css_class} +<<<<<<< HEAD <%block name="title">${_("{course_number} Courseware").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Courseware +>>>>>>> add escaping <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index 4d889b35f7..67bef227fa 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -7,7 +7,11 @@ <%static:css group='course'/> +<<<<<<< HEAD <%block name="title">${_("{course.number} Course Info").format(course=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Course Info +>>>>>>> add escaping <%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%! diff --git a/lms/templates/courseware/mktg_course_about.html b/lms/templates/courseware/mktg_course_about.html index 46f2060bbc..71be39bc20 100644 --- a/lms/templates/courseware/mktg_course_about.html +++ b/lms/templates/courseware/mktg_course_about.html @@ -8,7 +8,11 @@ <%inherit file="../mktg_iframe.html" /> +<<<<<<< HEAD <%block name="title">${_("About {course_number}").format(course_number=course.display_number_with_default)} +======= +<%block name="title">About ${course.display_number_with_default | h} +>>>>>>> add escaping <%block name="bodyclass">view-partial-mktgregister @@ -52,7 +56,11 @@
      ${_("You Are Registered")}
      %endif %elif allow_registration: +<<<<<<< HEAD ${_("Register for")} ${course.display_number_with_default} +======= + Register for ${course.display_number_with_default | h} +>>>>>>> add escaping %else:
      ${_("Registration Is Closed")}
      %endif diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 105ccebe86..85f96bfbd5 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -8,7 +8,11 @@ <%namespace name="progress_graph" file="/courseware/progress_graph.js"/> +<<<<<<< HEAD <%block name="title">${_("{course_number} Progress").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Progress +>>>>>>> add escaping <%! from django.core.urlresolvers import reverse diff --git a/lms/templates/courseware/static_tab.html b/lms/templates/courseware/static_tab.html index 884c161496..f03dcacb80 100644 --- a/lms/templates/courseware/static_tab.html +++ b/lms/templates/courseware/static_tab.html @@ -6,7 +6,7 @@ <%static:css group='course'/> -<%block name="title">${course.display_number_with_default} ${tab['name']} +<%block name="title">${course.display_number_with_default | h} ${tab['name']} <%include file="/courseware/course_navigation.html" args="active_page='static_tab_{0}'.format(tab['url_slug'])" /> diff --git a/lms/templates/courseware/syllabus.html b/lms/templates/courseware/syllabus.html index 3499193377..49c76f13ac 100644 --- a/lms/templates/courseware/syllabus.html +++ b/lms/templates/courseware/syllabus.html @@ -6,7 +6,11 @@ <%static:css group='course'/> +<<<<<<< HEAD <%block name="title">${_("{course.display_number_with_default} Course Info").format(course=course)} +======= +<%block name="title">${course.display_number_with_default | h} Course Info +>>>>>>> add escaping <%include file="/courseware/course_navigation.html" args="active_page='syllabus'" /> <%! diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 2c565cfd9c..01d23e7e5d 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -140,6 +140,7 @@ % if course.id in show_courseware_links_for: +<<<<<<< HEAD <<<<<<< HEAD ${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}')} @@ -153,6 +154,13 @@
      ${course.display_number_with_default} ${course.display_name_with_default} Cover Image >>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. +======= + ${course.display_number_with_default | h} ${course.display_name_with_default} Cover Image + + % else: +
      + ${course.display_number_with_default | h} ${course.display_name_with_default} Cover Image +>>>>>>> add escaping
      % endif @@ -170,9 +178,9 @@

      ${get_course_about_section(course, 'university')}

      % if course.id in show_courseware_links_for: - ${course.display_number_with_default} ${course.display_name_with_default} + ${course.display_number_with_default | h} ${course.display_name_with_default} % else: - ${course.display_number_with_default} ${course.display_name_with_default} + ${course.display_number_with_default | h} ${course.display_name_with_default} % endif

      @@ -205,6 +213,7 @@ % endif % if registration.is_rejected:
      +<<<<<<< HEAD <<<<<<< HEAD

      ${_("Your registration for the Pearson exam has been rejected. Please {link_start}see your registration status details{link_end}.").format( @@ -218,6 +227,9 @@ =======

      Your registration for the Pearson exam has been rejected. Please see your registration status details. Otherwise contact edX at exam-help@edx.org for further help.

      >>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. +======= +

      Your registration for the Pearson exam has been rejected. Please see your registration status details. Otherwise contact edX at exam-help@edx.org for further help.

      +>>>>>>> add escaping
      % endif % if not registration.is_accepted and not registration.is_rejected: diff --git a/lms/templates/discussion/single_thread.html b/lms/templates/discussion/single_thread.html index 5bd6c6ca03..9dade09a9a 100644 --- a/lms/templates/discussion/single_thread.html +++ b/lms/templates/discussion/single_thread.html @@ -7,7 +7,11 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion +<<<<<<< HEAD <%block name="title">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default) | h} +======= +<%block name="title">Discussion – ${course.display_number_with_default | h} +>>>>>>> add escaping <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index fee2275927..e506ef02cc 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -7,7 +7,11 @@ <%static:css group='course'/> +<<<<<<< HEAD <%block name="title">${_("{course_number} Staff Grading").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Staff Grading +>>>>>>> add escaping <%include file="/courseware/course_navigation.html" args="active_page='staff_grading'" /> diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index 3ca3ae2d1d..ee9b400251 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -50,7 +50,7 @@ site_status_msg = get_site_status_msg(course_id) % if course: -

      ${course.display_org_with_default}: ${course.display_number_with_default} ${course.display_name_with_default}

      +

      ${course.display_org_with_default | h}: ${course.display_number_with_default | h} ${course.display_name_with_default}

      % endif % if user.is_authenticated(): diff --git a/lms/templates/open_ended_problems/combined_notifications.html b/lms/templates/open_ended_problems/combined_notifications.html index b1149194d9..1b8898fedc 100644 --- a/lms/templates/open_ended_problems/combined_notifications.html +++ b/lms/templates/open_ended_problems/combined_notifications.html @@ -7,7 +7,11 @@ <%static:css group='course'/> +<<<<<<< HEAD <%block name="title">${_("{course_number} Combined Notifications").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Combined Notifications +>>>>>>> add escaping <%include file="/courseware/course_navigation.html" args="active_page='open_ended'" /> diff --git a/lms/templates/open_ended_problems/open_ended_flagged_problems.html b/lms/templates/open_ended_problems/open_ended_flagged_problems.html index 61ec49584c..f366684c02 100644 --- a/lms/templates/open_ended_problems/open_ended_flagged_problems.html +++ b/lms/templates/open_ended_problems/open_ended_flagged_problems.html @@ -7,7 +7,11 @@ <%static:css group='course'/> +<<<<<<< HEAD <%block name="title">${_("{course_number} Flagged Open Ended Problems").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Flagged Open Ended Problems +>>>>>>> add escaping <%include file="/courseware/course_navigation.html" args="active_page='open_ended_flagged_problems'" /> diff --git a/lms/templates/open_ended_problems/open_ended_problems.html b/lms/templates/open_ended_problems/open_ended_problems.html index e8496b6647..67c5ba8da3 100644 --- a/lms/templates/open_ended_problems/open_ended_problems.html +++ b/lms/templates/open_ended_problems/open_ended_problems.html @@ -7,7 +7,11 @@ <%static:css group='course'/> +<<<<<<< HEAD <%block name="title">${_("{course_number} Open Ended Problems").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Open Ended Problems +>>>>>>> add escaping <%include file="/courseware/course_navigation.html" args="active_page='open_ended_problems'" /> diff --git a/lms/templates/static_htmlbook.html b/lms/templates/static_htmlbook.html index 9f8f4bb3d5..2b7702f5a4 100644 --- a/lms/templates/static_htmlbook.html +++ b/lms/templates/static_htmlbook.html @@ -2,8 +2,12 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> +<<<<<<< HEAD <%block name="title">${_('{course_number} Textbook').format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Textbook +>>>>>>> add escaping <%block name="headextra"> diff --git a/lms/templates/static_pdfbook.html b/lms/templates/static_pdfbook.html index a38e9ff541..e6cde45fd2 100644 --- a/lms/templates/static_pdfbook.html +++ b/lms/templates/static_pdfbook.html @@ -3,10 +3,16 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> <%block name="title"> +<<<<<<< HEAD ${_('{course_number} Textbook').format(course_number=course.display_number_with_default)} +======= + + + ${course.display_number_with_default | h} Textbook +>>>>>>> add escaping <%block name="headextra"> diff --git a/lms/templates/staticbook.html b/lms/templates/staticbook.html index 157af3a061..443918ac75 100644 --- a/lms/templates/staticbook.html +++ b/lms/templates/staticbook.html @@ -2,7 +2,11 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> +<<<<<<< HEAD <%block name="title">${_("{course_number} Textbook").format(course_number=course.display_number_with_default)} +======= +<%block name="title">${course.display_number_with_default | h} Textbook +>>>>>>> add escaping <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/test_center_register.html b/lms/templates/test_center_register.html index ba88cfd6dd..de7ecd23df 100644 --- a/lms/templates/test_center_register.html +++ b/lms/templates/test_center_register.html @@ -95,7 +95,7 @@
      -

      ${get_course_about_section(course, 'university')} ${course.display_number_with_default} ${course.display_name_with_default}

      +

      ${get_course_about_section(course, 'university')} ${course.display_number_with_default | h} ${course.display_name_with_default}

      % if registration:

      ${_('Your Pearson VUE Proctored Exam Registration')}

      @@ -442,7 +442,7 @@ % endif
      -

      ${_("About {university} {course_number}").format(university=get_course_about_section(course, 'university'), course_number=course.display_number_with_default)}

      +

      ${_("About {university} {course_number}").format(university=get_course_about_section(course, 'university'), course_number=course.course.display_number_with_default)}

      % if course.has_ended(): ${_('Course Completed:')} ${course.end_date_text} From 0937cdcbe6f19398b724174d60a1660ca6d0fe40 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Jul 2013 15:25:19 -0400 Subject: [PATCH 005/206] one more escaping --- lms/templates/courseware/course_about.html | 39 ++++------------------ 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index f0b39adc0c..a45f6a93cb 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -66,15 +66,7 @@ -<<<<<<< HEAD -<<<<<<< HEAD -<%block name="title">${_("About {course.number}").format(course=course)} -======= -<%block name="title">About ${course.display_number_with_default} ->>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. -======= -<%block name="title">About ${course.display_number_with_default | h} ->>>>>>> add escaping +<%block name="title">${_("About {course.display_number_with_default}").format(course=course) | h}

      @@ -95,25 +87,15 @@ %if show_courseware_link: %endif -<<<<<<< HEAD - ${_("You are registered for this course {course.number}").format(course=course)} -======= - You are registered for this course (${course.display_number_with_default}) ->>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. + + ${_("You are registered for this course {course.display_number_with_default}"). %if show_courseware_link: ${_("View Courseware")} %endif %else: -<<<<<<< HEAD -<<<<<<< HEAD - ${_("Register for {course.number}").format(course=course)} -======= - Register for ${course.display_number_with_default} ->>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. -======= - Register for ${course.display_number_with_default | h} ->>>>>>> add escaping + ${_("Register for {course.display_number_with_default}").format(course=course)} +
      %endif
      @@ -182,17 +164,8 @@
        -<<<<<<< HEAD -<<<<<<< HEAD -

      1. ${_("Course Number")}

        ${course.number}
        2. +

      2. ${_("Course Number")}

        ${course.display_number_with_default | h}

      3. ${_("Classes Start")}

        ${course.start_date_text}
        4. -======= -

      4. Course Number

        ${course.display_number_with_default}
        5. -======= -

      5. Course Number

        ${course.display_number_with_default | h}
        6. ->>>>>>> add escaping -

      6. Classes Start

        ${course.start_date_text}
        7. ->>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. ## We plan to ditch end_date (which is not stored in course metadata), ## but for backwards compatibility, show about/end_date blob if it exists. From ebbeeb16a228d9001c3db6285529f7cfd42cabe6 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Jul 2013 15:53:04 -0400 Subject: [PATCH 006/206] add some unit tests for the new fields --- .../lib/xmodule/xmodule/tests/test_course_module.py | 12 +++++++++++- common/test/data/toy/roots/2012_Fall.xml | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index 53181b5a28..48690a8b25 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -53,7 +53,7 @@ def get_dummy_course(start, announcement=None, is_new=None, advertised_start=Non end = to_attrb('end', end) start_xml = ''' - \ No newline at end of file + \ No newline at end of file From e41554b24ce75b093e699dfcaa448d23624e3e23 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 1 Aug 2013 11:58:37 -0400 Subject: [PATCH 007/206] be sure to encode the display strings --- lms/djangoapps/course_wiki/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/course_wiki/views.py b/lms/djangoapps/course_wiki/views.py index e92598a9a5..1884ab7f11 100644 --- a/lms/djangoapps/course_wiki/views.py +++ b/lms/djangoapps/course_wiki/views.py @@ -1,5 +1,6 @@ import logging import re +import cgi from django.conf import settings from django.contrib.sites.models import Site @@ -95,7 +96,7 @@ def course_wiki_redirect(request, course_id): root, course_slug, title=course_slug, - content="This is the wiki for **{0}**'s _{1}_.".format(course.display_org_with_default, course.display_name_with_default), + content=cgi.escape("This is the wiki for **{0}**'s _{1}_.".format(course.display_org_with_default, course.display_name_with_default)), user_message="Course page automatically created.", user=None, ip_address=None, From f041ab1ccc544ea25c9668aff87885fa0252f60a Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 11:22:25 -0400 Subject: [PATCH 008/206] fix rebase conflicts --- lms/templates/courseware/course_about.html | 2 +- lms/templates/courseware/courseware.html | 6 +--- lms/templates/courseware/info.html | 7 +---- .../courseware/mktg_course_about.html | 12 ++------ lms/templates/courseware/progress.html | 6 +--- lms/templates/courseware/syllabus.html | 6 +--- lms/templates/dashboard.html | 28 ++----------------- .../combined_notifications.html | 6 +--- .../open_ended_problems.html | 6 +--- lms/templates/static_htmlbook.html | 7 +---- lms/templates/static_pdfbook.html | 9 +----- lms/templates/staticbook.html | 6 +--- 12 files changed, 14 insertions(+), 87 deletions(-) diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index a45f6a93cb..17c3357861 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -88,7 +88,7 @@ %endif - ${_("You are registered for this course {course.display_number_with_default}"). + ${_("You are registered for this course {course.display_number_with_default}").format(course=course) | h} %if show_courseware_link: ${_("View Courseware")} diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index cc5646b3a4..2a52b50b09 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -2,11 +2,7 @@ <%inherit file="/main.html" /> <%namespace name='static' file='/static_content.html'/> <%block name="bodyclass">courseware ${course.css_class} -<<<<<<< HEAD -<%block name="title">${_("{course_number} Courseware").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Courseware ->>>>>>> add escaping +<%block name="title">${_("{course_number} Courseware").format(course_number=course.display_number_with_default) | h} <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index 67bef227fa..0a520e0a1e 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -7,12 +7,7 @@ <%static:css group='course'/> -<<<<<<< HEAD -<%block name="title">${_("{course.number} Course Info").format(course=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Course Info ->>>>>>> add escaping - +<%block name="title">${_("{course.number} Course Info").format(course=course.display_number_with_default) | h} <%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%! from courseware.courses import get_course_info_section diff --git a/lms/templates/courseware/mktg_course_about.html b/lms/templates/courseware/mktg_course_about.html index 71be39bc20..be623b5baa 100644 --- a/lms/templates/courseware/mktg_course_about.html +++ b/lms/templates/courseware/mktg_course_about.html @@ -8,11 +8,7 @@ <%inherit file="../mktg_iframe.html" /> -<<<<<<< HEAD -<%block name="title">${_("About {course_number}").format(course_number=course.display_number_with_default)} -======= -<%block name="title">About ${course.display_number_with_default | h} ->>>>>>> add escaping +<%block name="title">${_("About {course_number}").format(course_number=course.display_number_with_default) | h} <%block name="bodyclass">view-partial-mktgregister @@ -56,11 +52,7 @@
        ${_("You Are Registered")}
        %endif %elif allow_registration: -<<<<<<< HEAD - ${_("Register for")} ${course.display_number_with_default} -======= - Register for ${course.display_number_with_default | h} ->>>>>>> add escaping + ${_("Register for")} ${course.display_number_with_default | h} %else:
        ${_("Registration Is Closed")}
        %endif diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 85f96bfbd5..324eadf560 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -8,11 +8,7 @@ <%namespace name="progress_graph" file="/courseware/progress_graph.js"/> -<<<<<<< HEAD -<%block name="title">${_("{course_number} Progress").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Progress ->>>>>>> add escaping +<%block name="title">${_("{course_number} Progress").format(course_number=course.display_number_with_default) | h} <%! from django.core.urlresolvers import reverse diff --git a/lms/templates/courseware/syllabus.html b/lms/templates/courseware/syllabus.html index 49c76f13ac..e2f3c53739 100644 --- a/lms/templates/courseware/syllabus.html +++ b/lms/templates/courseware/syllabus.html @@ -6,11 +6,7 @@ <%static:css group='course'/> -<<<<<<< HEAD -<%block name="title">${_("{course.display_number_with_default} Course Info").format(course=course)} -======= -<%block name="title">${course.display_number_with_default | h} Course Info ->>>>>>> add escaping +<%block name="title">${_("{course.display_number_with_default} Course Info").format(course=course) | h} <%include file="/courseware/course_navigation.html" args="active_page='syllabus'" /> <%! diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 01d23e7e5d..1dfa66461c 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -140,27 +140,11 @@ % if course.id in show_courseware_links_for: -<<<<<<< HEAD -<<<<<<< HEAD - ${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}')} + ${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}') |h} % else:
        ${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}')} -======= - ${course.display_number_with_default} ${course.display_name_with_default} Cover Image - - % else: -
        - ${course.display_number_with_default} ${course.display_name_with_default} Cover Image ->>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. -======= - ${course.display_number_with_default | h} ${course.display_name_with_default} Cover Image - - % else: -
        - ${course.display_number_with_default | h} ${course.display_name_with_default} Cover Image ->>>>>>> add escaping
        % endif @@ -213,23 +197,15 @@ % endif % if registration.is_rejected:
        -<<<<<<< HEAD -<<<<<<< HEAD

        ${_("Your registration for the Pearson exam has been rejected. Please {link_start}see your registration status details{link_end}.").format( link_start=''.format(url=testcenter_register_target), link_end='')} ${_("Otherwise {link_start}contact edX at {email}{link_end} for further help.").format( - link_start=''.format(email="exam-help@edx.org", about=get_course_about_section(course, 'university'), number=course.number), + link_start=''.format(email="exam-help@edx.org", about=get_course_about_section(course, 'university'), number=course.display_number_with_default), link_end='', email="exam-help@edx.org", )} -======= -

        Your registration for the Pearson exam has been rejected. Please see your registration status details. Otherwise contact edX at exam-help@edx.org for further help.

        ->>>>>>> add display_coursenumber and display_organization fields on the CourseModule, with some property accessors. Update LMS/CMS pages to use those display strings as appropraite. -======= -

        Your registration for the Pearson exam has been rejected. Please see your registration status details. Otherwise contact edX at exam-help@edx.org for further help.

        ->>>>>>> add escaping
        % endif % if not registration.is_accepted and not registration.is_rejected: diff --git a/lms/templates/open_ended_problems/combined_notifications.html b/lms/templates/open_ended_problems/combined_notifications.html index 1b8898fedc..ce22a5c580 100644 --- a/lms/templates/open_ended_problems/combined_notifications.html +++ b/lms/templates/open_ended_problems/combined_notifications.html @@ -7,11 +7,7 @@ <%static:css group='course'/> -<<<<<<< HEAD -<%block name="title">${_("{course_number} Combined Notifications").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Combined Notifications ->>>>>>> add escaping +<%block name="title">${_("{course_number} Combined Notifications").format(course_number=course.display_number_with_default) | h} <%include file="/courseware/course_navigation.html" args="active_page='open_ended'" /> diff --git a/lms/templates/open_ended_problems/open_ended_problems.html b/lms/templates/open_ended_problems/open_ended_problems.html index 67c5ba8da3..68882984f5 100644 --- a/lms/templates/open_ended_problems/open_ended_problems.html +++ b/lms/templates/open_ended_problems/open_ended_problems.html @@ -7,11 +7,7 @@ <%static:css group='course'/> -<<<<<<< HEAD -<%block name="title">${_("{course_number} Open Ended Problems").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Open Ended Problems ->>>>>>> add escaping +<%block name="title">${_("{course_number} Open Ended Problems").format(course_number=course.display_number_with_default) | h} <%include file="/courseware/course_navigation.html" args="active_page='open_ended_problems'" /> diff --git a/lms/templates/static_htmlbook.html b/lms/templates/static_htmlbook.html index 2b7702f5a4..60421502e6 100644 --- a/lms/templates/static_htmlbook.html +++ b/lms/templates/static_htmlbook.html @@ -2,12 +2,7 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> -<<<<<<< HEAD -<%block name="title">${_('{course_number} Textbook').format(course_number=course.display_number_with_default)} - -======= -<%block name="title">${course.display_number_with_default | h} Textbook ->>>>>>> add escaping +<%block name="title">${_('{course_number} Textbook').format(course_number=course.display_number_with_default) | h} <%block name="headextra"> diff --git a/lms/templates/static_pdfbook.html b/lms/templates/static_pdfbook.html index e6cde45fd2..4e9f604a0c 100644 --- a/lms/templates/static_pdfbook.html +++ b/lms/templates/static_pdfbook.html @@ -3,16 +3,9 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> <%block name="title"> -<<<<<<< HEAD -${_('{course_number} Textbook').format(course_number=course.display_number_with_default)} - -======= - - - ${course.display_number_with_default | h} Textbook ->>>>>>> add escaping +${_('{course_number} Textbook').format(course_number=course.display_number_with_default) | h} <%block name="headextra"> diff --git a/lms/templates/staticbook.html b/lms/templates/staticbook.html index 443918ac75..09741e0a9e 100644 --- a/lms/templates/staticbook.html +++ b/lms/templates/staticbook.html @@ -2,11 +2,7 @@ <%inherit file="main.html" /> <%namespace name='static' file='static_content.html'/> -<<<<<<< HEAD -<%block name="title">${_("{course_number} Textbook").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Textbook ->>>>>>> add escaping +<%block name="title">${_("{course_number} Textbook").format(course_number=course.display_number_with_default) | h} <%block name="headextra"> <%static:css group='course'/> From c867be7961d5d17c0074126aa348a9f92d06d1f5 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Wed, 31 Jul 2013 16:39:10 -0400 Subject: [PATCH 009/206] Limit the rate of logins. --- CHANGELOG.rst | 2 + cms/djangoapps/contentstore/tests/tests.py | 15 ++++ cms/djangoapps/course_creators/admin.py | 2 +- .../course_creators/tests/test_admin.py | 24 +++++- cms/envs/common.py | 14 +++- cms/envs/test.py | 4 + cms/urls.py | 2 +- common/djangoapps/external_auth/admin.py | 2 +- .../tests/test_openid_provider.py | 48 ++++++++++- common/djangoapps/external_auth/views.py | 10 ++- common/djangoapps/student/admin.py | 2 +- common/djangoapps/student/tests/test_login.py | 23 ++++++ common/djangoapps/student/views.py | 24 ++++-- common/djangoapps/track/admin.py | 2 +- lms/djangoapps/courseware/admin.py | 2 +- lms/envs/common.py | 15 +++- lms/envs/dev_edx4edx.py | 79 ------------------- lms/envs/test.py | 4 + lms/urls.py | 2 +- requirements/edx/base.txt | 1 + 20 files changed, 171 insertions(+), 106 deletions(-) delete mode 100644 lms/envs/dev_edx4edx.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index edba051451..d988b66f34 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Common: Added ratelimiting to our authentication backend. + Common: Add additional logging to cover login attempts and logouts. Studio: Send e-mails to new Studio users (on edge only) when their course creator diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 1f2a4185a3..0cbc82cbf1 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -1,4 +1,5 @@ from django.test.client import Client +from django.core.cache import cache from django.core.urlresolvers import reverse from .utils import parse_json, user, registration @@ -79,6 +80,8 @@ class AuthTestCase(ContentStoreTestCase): self.pw = 'xyz' self.username = 'testuser' self.client = Client() + # clear the cache so ratelimiting won't affect these tests + cache.clear() def check_page_get(self, url, expected): resp = self.client.get(url) @@ -119,6 +122,18 @@ class AuthTestCase(ContentStoreTestCase): # Now login should work self.login(self.email, self.pw) + def test_login_ratelimited(self): + # try logging in 30 times, the default limit in the number of failed + # login attempts in one 5 minute period before the rate gets limited + for i in xrange(30): + resp = self._login(self.email, 'wrong_password{0}'.format(i)) + self.assertEqual(resp.status_code, 200) + resp = self._login(self.email, 'wrong_password') + self.assertEqual(resp.status_code, 200) + data = parse_json(resp) + self.assertFalse(data['success']) + self.assertIn('Too many failed login attempts.', data['value']) + def test_login_link_on_activation_age(self): self.create_account(self.username, self.email, self.pw) # we want to test the rendering of the activation page when the user isn't logged in diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 65473d8bde..0f9a189bb7 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -5,7 +5,7 @@ django admin page for the course creators table from course_creators.models import CourseCreator, update_creator_state from course_creators.views import update_course_creator_group -from django.contrib import admin +from ratelimitbackend import admin from django.conf import settings from django.dispatch import receiver from mitxmako.shortcuts import render_to_string diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 91a28d77ae..f28fec5a44 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -43,14 +43,14 @@ class CourseCreatorAdminTest(TestCase): """ Tests that updates to state impact the creator group maintained in authz.py and that e-mails are sent. """ - STUDIO_REQUEST_EMAIL = 'mark@marky.mark' - + STUDIO_REQUEST_EMAIL = 'mark@marky.mark' + def change_state(state, is_creator): """ Helper method for changing state """ self.table_entry.state = state self.creator_admin.save_model(self.request, self.table_entry, None, True) self.assertEqual(is_creator, is_user_in_creator_group(self.user)) - + context = {'studio_request_email': STUDIO_REQUEST_EMAIL} if state == CourseCreator.GRANTED: template = 'emails/course_creator_granted.txt' @@ -69,7 +69,8 @@ class CourseCreatorAdminTest(TestCase): { "ENABLE_CREATOR_GROUP": True, "STUDIO_REQUEST_EMAIL": STUDIO_REQUEST_EMAIL - }): + } + ): # User is initially unrequested. self.assertFalse(is_user_in_creator_group(self.user)) @@ -106,3 +107,18 @@ class CourseCreatorAdminTest(TestCase): self.request.user = self.user self.assertFalse(self.creator_admin.has_change_permission(self.request)) + + def test_rate_limit_login(self): + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_CREATOR_GROUP': True}): + post_params = {'username': self.user.username, 'password': 'wrong_password'} + # try logging in 30 times, the default limit in the number of failed + # login attempts in one 5 minute period before the rate gets limited + for _ in xrange(30): + response = self.client.post('/admin/', post_params) + self.assertEquals(response.status_code, 200) + + response = self.client.post('/admin/', post_params) + # Since we are using the default rate limit behavior, we are + # expecting this to return a 403 error to indicate that there have + # been too many attempts + self.assertEquals(response.status_code, 403) diff --git a/cms/envs/common.py b/cms/envs/common.py index 155c4d46d8..40084c20ae 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -108,6 +108,11 @@ TEMPLATE_CONTEXT_PROCESSORS = ( 'django.core.context_processors.csrf' ) +# use the ratelimit backend to prevent brute force attacks +AUTHENTICATION_BACKENDS = ( + 'ratelimitbackend.backends.RateLimitModelBackend', +) + LMS_BASE = None #################### CAPA External Code Evaluation ############################# @@ -152,7 +157,10 @@ MIDDLEWARE_CLASSES = ( # Detects user-requested locale from 'accept-language' header in http request 'django.middleware.locale.LocaleMiddleware', - 'django.middleware.transaction.TransactionMiddleware' + 'django.middleware.transaction.TransactionMiddleware', + + # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500 + 'ratelimitbackend.middleware.RateLimitMiddleware', ) ############################ SIGNAL HANDLERS ################################ @@ -188,8 +196,8 @@ STATICFILES_DIRS = [ COMMON_ROOT / "static", PROJECT_ROOT / "static", -# This is how you would use the textbook images locally -# ("book", ENV_ROOT / "book_images") + # This is how you would use the textbook images locally + # ("book", ENV_ROOT / "book_images") ] # Locale/Internationalization diff --git a/cms/envs/test.py b/cms/envs/test.py index efc7c5a7ef..4f3b0caee0 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -15,6 +15,7 @@ sessions. Assumes structure: from .common import * import os from path import path +from warnings import filterwarnings # Nose Test Runner INSTALLED_APPS += ('django_nose',) @@ -124,6 +125,9 @@ CACHES = { } } +# hide ratelimit warnings while running tests +filterwarnings('ignore', message='No request passed to the backend, unable to rate-limit') + ################################# CELERY ###################################### CELERY_ALWAYS_EAGER = True diff --git a/cms/urls.py b/cms/urls.py index 5945394f55..0beb21362e 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -6,7 +6,7 @@ from django.conf.urls import patterns, include, url from . import one_time_startup # There is a course creators admin table. -from django.contrib import admin +from ratelimitbackend import admin admin.autodiscover() urlpatterns = ('', # nopep8 diff --git a/common/djangoapps/external_auth/admin.py b/common/djangoapps/external_auth/admin.py index 1ee18dadc1..fad0917604 100644 --- a/common/djangoapps/external_auth/admin.py +++ b/common/djangoapps/external_auth/admin.py @@ -3,7 +3,7 @@ django admin pages for courseware model ''' from external_auth.models import * -from django.contrib import admin +from ratelimitbackend import admin class ExternalAuthMapAdmin(admin.ModelAdmin): diff --git a/common/djangoapps/external_auth/tests/test_openid_provider.py b/common/djangoapps/external_auth/tests/test_openid_provider.py index 1f7f201087..2024da469f 100644 --- a/common/djangoapps/external_auth/tests/test_openid_provider.py +++ b/common/djangoapps/external_auth/tests/test_openid_provider.py @@ -9,12 +9,15 @@ from urlparse import parse_qs from django.conf import settings from django.test import TestCase, LiveServerTestCase +from django.core.cache import cache from django.test.utils import override_settings -# from django.contrib.auth.models import User from django.core.urlresolvers import reverse from django.test.client import RequestFactory from unittest import skipUnless +from student.tests.factories import UserFactory +from external_auth.views import provider_login + class MyFetcher(HTTPFetcher): """A fetcher that uses server-internal calls for performing HTTP @@ -199,6 +202,49 @@ class OpenIdProviderTest(TestCase): """ Test for 403 error code when the url""" self.attempt_login(403, return_to="http://apps.cs50.edx.or") + def _send_bad_redirection_login(self): + """ + Attempt to log in to the provider with setup parameters + + Intentionally fail the login to force a redirect + """ + user = UserFactory() + + factory = RequestFactory() + post_params = {'email': user.email, 'password': 'password'} + fake_url = 'fake url' + request = factory.post(reverse('openid-provider-login'), post_params) + openid_setup = { + 'request': factory.request(), + 'url': fake_url + } + request.session = { + 'openid_setup': openid_setup + } + response = provider_login(request) + return response + + @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or + settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True) + def test_login_openid_handle_redirection(self): + """ Test to see that we can handle login redirection properly""" + response = self._send_bad_redirection_login() + self.assertEquals(response.status_code, 302) + + @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or + settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True) + def test_login_openid_handle_redirection_ratelimited(self): + # try logging in 30 times, the default limit in the number of failed + # log in attempts before the rate gets limited + for _ in xrange(30): + self._send_bad_redirection_login() + + response = self._send_bad_redirection_login() + # verify that we are not returning the default 403 + self.assertEquals(response.status_code, 302) + # clear the ratelimit cache so that we don't fail other logins + cache.clear() + class OpenIdProviderLiveServerTest(LiveServerTestCase): """ diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 9065ea78d6..37dcd5313a 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -39,6 +39,7 @@ from openid.consumer.consumer import SUCCESS from openid.server.server import Server, ProtocolError, UntrustedReturnURL from openid.server.trustroot import TrustRoot from openid.extensions import ax, sreg +from ratelimitbackend.exceptions import RateLimitException import student.views as student_views # Required for Pearson @@ -191,7 +192,7 @@ def _external_login_or_signup(request, user.backend = auth_backend AUDIT_LOG.info('Linked user "%s" logged in via Shibboleth', user.email) else: - user = authenticate(username=uname, password=eamap.internal_password) + user = authenticate(username=uname, password=eamap.internal_password, request=request) if user is None: # we want to log the failure, but don't want to log the password attempted: AUDIT_LOG.warning('External Auth Login failed for "%s"', uname) @@ -718,7 +719,12 @@ def provider_login(request): # Failure is again redirected to the login dialog. username = user.username password = request.POST.get('password', None) - user = authenticate(username=username, password=password) + try: + user = authenticate(username=username, password=password, request=request) + except RateLimitException: + AUDIT_LOG.warning('OpenID - Too many failed login attempts.') + return HttpResponseRedirect(openid_request_url) + if user is None: request.session['openid_error'] = True msg = "OpenID login failed - password for %s is invalid" diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 4d6976d7d4..209f7cf6c0 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -4,7 +4,7 @@ django admin pages for courseware model from student.models import UserProfile, UserTestGroup, CourseEnrollmentAllowed from student.models import CourseEnrollment, Registration, PendingNameChange -from django.contrib import admin +from ratelimitbackend import admin admin.site.register(UserProfile) diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 5a6cd043ae..e8def470c0 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -6,6 +6,7 @@ from mock import patch from django.test import TestCase from django.test.client import Client +from django.core.cache import cache from django.core.urlresolvers import reverse, NoReverseMatch from student.tests.factories import UserFactory, RegistrationFactory, UserProfileFactory @@ -29,6 +30,7 @@ class LoginTest(TestCase): # Create the test client self.client = Client() + cache.clear() # Store the login url try: @@ -95,6 +97,27 @@ class LoginTest(TestCase): self.assertEqual(response.status_code, 302) self._assert_audit_log(mock_audit_log, 'info', [u'Logout', u'test']) + def test_login_ratelimited_success(self): + # Try (and fail) logging in with fewer attempts than the limit of 30 + # and verify that you can still successfully log in afterwards. + for i in xrange(20): + password = u'test_password{0}'.format(i) + response, _audit_log = self._login_response('test@edx.org', password) + self._assert_response(response, success=False) + # now try logging in with a valid password + response, _audit_log = self._login_response('test@edx.org', 'test_password') + self._assert_response(response, success=True) + + def test_login_ratelimited(self): + # try logging in 30 times, the default limit in the number of failed + # login attempts in one 5 minute period before the rate gets limited + for i in xrange(30): + password = u'test_password{0}'.format(i) + self._login_response('test@edx.org', password) + # check to see if this response indicates that this was ratelimited + response, _audit_log = self._login_response('test@edx.org', 'wrong_password') + self._assert_response(response, success=False, value='Too many failed login attempts') + def _login_response(self, email, password, patched_audit_log='student.views.AUDIT_LOG'): ''' Post the login info ''' post_params = {'email': email, 'password': password} diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 40bd4a3034..223c124e46 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -28,6 +28,8 @@ from django.utils.http import cookie_date from django.utils.http import base36_to_int from django.utils.translation import ugettext as _ +from ratelimitbackend.exceptions import RateLimitException + from mitxmako.shortcuts import render_to_response, render_to_string from bs4 import BeautifulSoup @@ -421,13 +423,23 @@ def login_user(request, error=""): user = User.objects.get(email=email) except User.DoesNotExist: AUDIT_LOG.warning(u"Login failed - Unknown user email: {0}".format(email)) - return HttpResponse(json.dumps({'success': False, - 'value': _('Email or password is incorrect.')})) # TODO: User error message + user = None - username = user.username - user = authenticate(username=username, password=password) + # if the user doesn't exist, we want to set the username to an invalid + # username so that authentication is guaranteed to fail and we can take + # advantage of the ratelimited backend + username = user.username if user else "" + try: + user = authenticate(username=username, password=password, request=request) + # this occurs when there are too many attempts from the same IP address + except RateLimitException: + return HttpResponse(json.dumps({'success': False, + 'value': _('Too many failed login attempts. Try again later.')})) if user is None: - AUDIT_LOG.warning(u"Login failed - password for {0} is invalid".format(email)) + # if we didn't find this username earlier, the account for this email + # doesn't exist, and doesn't have a corresponding password + if username != "": + AUDIT_LOG.warning(u"Login failed - password for {0} is invalid".format(email)) return HttpResponse(json.dumps({'success': False, 'value': _('Email or password is incorrect.')})) @@ -942,7 +954,7 @@ def auto_auth(request): # if they already are a user, log in try: user = User.objects.get(username=username) - user = authenticate(username=username, password=password) + user = authenticate(username=username, password=password, request=request) login(request, user) # else create and activate account info diff --git a/common/djangoapps/track/admin.py b/common/djangoapps/track/admin.py index d75f206846..e0835f5a8a 100644 --- a/common/djangoapps/track/admin.py +++ b/common/djangoapps/track/admin.py @@ -3,6 +3,6 @@ django admin pages for courseware model ''' from track.models import TrackingLog -from django.contrib import admin +from ratelimitbackend import admin admin.site.register(TrackingLog) diff --git a/lms/djangoapps/courseware/admin.py b/lms/djangoapps/courseware/admin.py index 743d1fed52..147630dbc5 100644 --- a/lms/djangoapps/courseware/admin.py +++ b/lms/djangoapps/courseware/admin.py @@ -3,7 +3,7 @@ django admin pages for courseware model ''' from courseware.models import StudentModule, OfflineComputedGrade, OfflineComputedGradeLog -from django.contrib import admin +from ratelimitbackend import admin from django.contrib.auth.models import User admin.site.register(StudentModule) diff --git a/lms/envs/common.py b/lms/envs/common.py index 324069b6d6..9d597835b4 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -236,6 +236,10 @@ TEMPLATE_CONTEXT_PROCESSORS = ( 'mitxmako.shortcuts.marketing_link_context_processor', ) +# use the ratelimit backend to prevent brute force attacks +AUTHENTICATION_BACKENDS = ( + 'ratelimitbackend.backends.RateLimitModelBackend', +) STUDENT_FILEUPLOAD_MAX_SIZE = 4 * 1000 * 1000 # 4 MB MAX_FILEUPLOADS_PER_INPUT = 20 @@ -434,10 +438,10 @@ OPEN_ENDED_GRADING_INTERFACE = { 'url': 'http://sandbox-grader-001.m.edx.org/peer_grading', 'username': 'incorrect_user', 'password': 'incorrect_pass', - 'staff_grading' : 'staff_grading', - 'peer_grading' : 'peer_grading', - 'grading_controller' : 'grading_controller' - } + 'staff_grading': 'staff_grading', + 'peer_grading': 'peer_grading', + 'grading_controller': 'grading_controller' +} # Used for testing, debugging peer grading MOCK_PEER_GRADING = False @@ -492,6 +496,9 @@ MIDDLEWARE_CLASSES = ( 'django_comment_client.utils.ViewNameMiddleware', 'codejail.django_integration.ConfigureCodeJailMiddleware', + + # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500 + 'ratelimitbackend.middleware.RateLimitMiddleware', ) ############################### Pipeline ####################################### diff --git a/lms/envs/dev_edx4edx.py b/lms/envs/dev_edx4edx.py deleted file mode 100644 index 13a66ed1e8..0000000000 --- a/lms/envs/dev_edx4edx.py +++ /dev/null @@ -1,79 +0,0 @@ -""" -This config file runs the simplest dev environment using sqlite, and db-based -sessions. Assumes structure: - -/envroot/ - /db # This is where it'll write the database file - /mitx # The location of this repo - /log # Where we're going to write log files -""" - -# We intentionally define lots of variables that aren't used, and -# want to import all variables from base settings files -# pylint: disable=W0401, W0614 - -import socket - -if 'eecs1' in socket.gethostname(): - MITX_ROOT_URL = '/mitx2' - -from .common import * -from .dev import * - -if 'eecs1' in socket.gethostname(): - MITX_ROOT_URL = '/mitx2' - -#----------------------------------------------------------------------------- -# edx4edx content server - -EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend' -MITX_FEATURES['REROUTE_ACTIVATION_EMAIL'] = 'ichuang@mitx.mit.edu' -EDX4EDX_ROOT = ENV_ROOT / "data/edx4edx" - -#EMAIL_BACKEND = 'django_ses.SESBackend' - -#----------------------------------------------------------------------------- -# ichuang - -DEBUG = True -ENABLE_MULTICOURSE = True # set to False to disable multicourse display (see lib.util.views.mitxhome) - -MAKO_TEMPLATES['course'] = [DATA_DIR, EDX4EDX_ROOT] - -#MITX_FEATURES['USE_DJANGO_PIPELINE'] = False -MITX_FEATURES['DISPLAY_HISTOGRAMS_TO_STAFF'] = False -MITX_FEATURES['DISPLAY_EDIT_LINK'] = True - -COURSE_DEFAULT = "edx4edx" -COURSE_NAME = "edx4edx" -COURSE_NUMBER = "edX.01" -COURSE_TITLE = "edx4edx: edX Author Course" -SITE_NAME = "ichuang.mitx.mit.edu" - -COURSE_SETTINGS = {'edx4edx': {'number' : 'edX.01', - 'title': 'edx4edx: edX Author Course', - 'xmlpath': '/edx4edx/', - 'github_url': 'https://github.com/MITx/edx4edx', - 'active': True, - 'default_chapter': 'Introduction', - 'default_section': 'edx4edx_Course', - }, - } - -#----------------------------------------------------------------------------- - -MIDDLEWARE_CLASSES = MIDDLEWARE_CLASSES + ( - 'ssl_auth.ssl_auth.NginxProxyHeaderMiddleware', # ssl authentication behind nginx proxy - ) - -AUTHENTICATION_BACKENDS = ( - 'ssl_auth.ssl_auth.SSLLoginBackend', - 'django.contrib.auth.backends.ModelBackend', - ) - -INSTALLED_APPS = INSTALLED_APPS + ( - 'ssl_auth', - ) - -LOGIN_REDIRECT_URL = MITX_ROOT_URL + '/' -LOGIN_URL = MITX_ROOT_URL + '/' diff --git a/lms/envs/test.py b/lms/envs/test.py index 81505ab0b3..3b5f7c11d5 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -15,6 +15,7 @@ sessions. Assumes structure: from .common import * import os from path import path +from warnings import filterwarnings # can't test start dates with this True, but on the other hand, # can test everything else :) @@ -135,6 +136,9 @@ CACHES = { # Dummy secret key for dev SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd' +# hide ratelimit warnings while running tests +filterwarnings('ignore', message='No request passed to the backend, unable to rate-limit') + ################################## OPENID ##################################### MITX_FEATURES['AUTH_USE_OPENID'] = True MITX_FEATURES['AUTH_USE_OPENID_PROVIDER'] = True diff --git a/lms/urls.py b/lms/urls.py index 59cadb8a5b..5fe37e77ea 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1,6 +1,6 @@ from django.conf import settings from django.conf.urls import patterns, include, url -from django.contrib import admin +from ratelimitbackend import admin from django.conf.urls.static import static # Not used, the work is done in the imported module. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 32827d052a..ffa2977dbd 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -52,6 +52,7 @@ sorl-thumbnail==11.12 South==0.7.6 sympy==0.7.1 xmltodict==0.4.1 +django-ratelimit-backend==0.6 # Used for debugging ipython==0.13.1 From c20ca2bf5e3d12399fe0ea8a9cf9bcac4e7a017e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 11:25:05 -0400 Subject: [PATCH 010/206] fix more rebase conflicts --- lms/templates/discussion/single_thread.html | 4 ---- lms/templates/instructor/staff_grading.html | 6 +----- .../open_ended_problems/open_ended_flagged_problems.html | 6 +----- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/lms/templates/discussion/single_thread.html b/lms/templates/discussion/single_thread.html index 9dade09a9a..5bd6c6ca03 100644 --- a/lms/templates/discussion/single_thread.html +++ b/lms/templates/discussion/single_thread.html @@ -7,11 +7,7 @@ <%inherit file="../main.html" /> <%namespace name='static' file='../static_content.html'/> <%block name="bodyclass">discussion -<<<<<<< HEAD <%block name="title">${_("Discussion - {course_number}").format(course_number=course.display_number_with_default) | h} -======= -<%block name="title">Discussion – ${course.display_number_with_default | h} ->>>>>>> add escaping <%block name="headextra"> <%static:css group='course'/> diff --git a/lms/templates/instructor/staff_grading.html b/lms/templates/instructor/staff_grading.html index e506ef02cc..40e80de11e 100644 --- a/lms/templates/instructor/staff_grading.html +++ b/lms/templates/instructor/staff_grading.html @@ -7,11 +7,7 @@ <%static:css group='course'/> -<<<<<<< HEAD -<%block name="title">${_("{course_number} Staff Grading").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Staff Grading ->>>>>>> add escaping +<%block name="title">${_("{course_number} Staff Grading").format(course_number=course.display_number_with_default) | h} <%include file="/courseware/course_navigation.html" args="active_page='staff_grading'" /> diff --git a/lms/templates/open_ended_problems/open_ended_flagged_problems.html b/lms/templates/open_ended_problems/open_ended_flagged_problems.html index f366684c02..8e746f585f 100644 --- a/lms/templates/open_ended_problems/open_ended_flagged_problems.html +++ b/lms/templates/open_ended_problems/open_ended_flagged_problems.html @@ -7,11 +7,7 @@ <%static:css group='course'/> -<<<<<<< HEAD -<%block name="title">${_("{course_number} Flagged Open Ended Problems").format(course_number=course.display_number_with_default)} -======= -<%block name="title">${course.display_number_with_default | h} Flagged Open Ended Problems ->>>>>>> add escaping +<%block name="title">${_("{course_number} Flagged Open Ended Problems").format(course_number=course.display_number_with_default) | h} <%include file="/courseware/course_navigation.html" args="active_page='open_ended_flagged_problems'" /> From d4a526906ed786a92f3d84dca1cbab4613deb96c Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Fri, 2 Aug 2013 12:54:46 -0400 Subject: [PATCH 011/206] template errors --- lms/templates/help_modal.html | 2 +- lms/templates/static_templates/help.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/templates/help_modal.html b/lms/templates/help_modal.html index 6692d02d09..41a751aae4 100644 --- a/lms/templates/help_modal.html +++ b/lms/templates/help_modal.html @@ -31,7 +31,7 @@ discussion_link = get_discussion_link(course) if course else None

        % endif -

        ${_('Have general questions about {platform_name}? You can find lots of helpful information in the {platform_name} {link_start}FAQ{link_end}.').format(link_start='', link_end='').format(platform_name=settings.PLATFORM_NAME)}

        +

        ${_('Have general questions about {platform_name}? You can find lots of helpful information in the {platform_name} {link_start}FAQ{link_end}.').format(link_start='', link_end='', platform_name=settings.PLATFORM_NAME)}

        ${_('Have a question about something specific? You can contact the {platform_name} general support team directly:').format(platform_name=settings.PLATFORM_NAME)}

        diff --git a/lms/templates/static_templates/help.html b/lms/templates/static_templates/help.html index b2743f892f..f473495f9b 100644 --- a/lms/templates/static_templates/help.html +++ b/lms/templates/static_templates/help.html @@ -189,7 +189,7 @@

        ${_("How are {edX} certificates delivered?").format(edX="edX")}

        -

        ${_("{EdX} certificates are delivered online through edx.org. So be sure to check your email in the weeks following the final grading - you will be able to download and print your certificate.").format(edX="edX")}

        +

        ${_("{EdX} certificates are delivered online through edx.org. So be sure to check your email in the weeks following the final grading - you will be able to download and print your certificate.").format(EdX="EdX")}

        From 7401c389173663050b36b9e6c09aedd1959fc531 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Fri, 2 Aug 2013 13:34:26 -0400 Subject: [PATCH 012/206] adjusting fix for studio drop-down nav in IE9 --- cms/static/sass/elements/_navigation.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cms/static/sass/elements/_navigation.scss b/cms/static/sass/elements/_navigation.scss index 739d091b8e..a9e2f45533 100644 --- a/cms/static/sass/elements/_navigation.scss +++ b/cms/static/sass/elements/_navigation.scss @@ -65,6 +65,7 @@ nav { pointer-events: none; width: ($baseline*8); overflow: hidden; + height: 0; // dropped down state @@ -72,6 +73,7 @@ nav { opacity: 1.0; pointer-events: auto; overflow: visible; + height: auto; } } From cca94c62e9f0abc7ced9694f40836514b9c1682c Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 2 Aug 2013 15:19:35 -0400 Subject: [PATCH 013/206] Update Babel and Transifex libraries --- docs/shared/requirements.txt | 6 +++--- requirements/edx/base.txt | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/shared/requirements.txt b/docs/shared/requirements.txt index b1fb899050..be4f8cb503 100644 --- a/docs/shared/requirements.txt +++ b/docs/shared/requirements.txt @@ -3,7 +3,7 @@ # Installing modules with C dependencies on RTD can be tricky, and pip doesn't # have an 'ignore-errors' option, so all requirements fail. Here we keep the # maximal list of modules that still works. -# +# beautifulsoup4==4.1.3 beautifulsoup==3.2.1 boto==2.6.0 @@ -61,8 +61,8 @@ dogstatsd-python==0.2.1 newrelic==1.8.0.13 # Used for Internationalization and localization -Babel==0.9.6 -transifex-client==0.8 +Babel==1.3 +transifex-client==0.9.1 -e common/lib/calc diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ffa2977dbd..8a5fe6633d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -69,8 +69,8 @@ newrelic==1.8.0.13 sphinx==1.1.3 # Used for Internationalization and localization -Babel==0.9.6 -transifex-client==0.8 +Babel==1.3 +transifex-client==0.9.1 # Used for testing coverage==3.6 From 0f18ea4bf7c8176c83e87c2f683c26767f035654 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Fri, 2 Aug 2013 17:18:49 -0400 Subject: [PATCH 014/206] redirects edge landing page to /dashboard --- cms/djangoapps/contentstore/views/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/requests.py b/cms/djangoapps/contentstore/views/requests.py index abbf84755e..dc1b7871ab 100644 --- a/cms/djangoapps/contentstore/views/requests.py +++ b/cms/djangoapps/contentstore/views/requests.py @@ -12,7 +12,7 @@ def landing(request, org, course, coursename): # points to the temporary edge page def edge(request): - return redirect('/') + return redirect('/dashboard') def event(request): From 192d51bf7a8644ddde7636ebb5a313646aad6264 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 21:37:27 -0400 Subject: [PATCH 015/206] touch-ups per review feedback --- common/lib/xmodule/xmodule/course_module.py | 5 ++--- lms/templates/courseware/course_about.html | 2 +- lms/templates/test_center_register.html | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index bf902e99bf..57b13c10b3 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -6,7 +6,6 @@ from path import path # NOTE (THK): Only used for detecting presence of syllabu import requests from datetime import datetime import dateutil.parser -import cgi from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor, SequenceModule @@ -945,7 +944,7 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): Return a display course number if it has been specified, otherwise return the 'course' that is in the location """ if self.display_coursenumber: - return cgi.escape(self.display_coursenumber) + return self.display_coursenumber return self.number @@ -959,6 +958,6 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): Return a display organization if it has been specified, otherwise return the 'org' that is in the location """ if self.display_organization: - return cgi.escape(self.display_organization) + return self.display_organization return self.org diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 17c3357861..0c95a3cf8e 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -94,7 +94,7 @@ %endif %else: - ${_("Register for {course.display_number_with_default}").format(course=course)} + ${_("Register for {course.display_number_with_default}").format(course=course) | h}
        %endif diff --git a/lms/templates/test_center_register.html b/lms/templates/test_center_register.html index de7ecd23df..c35acf9914 100644 --- a/lms/templates/test_center_register.html +++ b/lms/templates/test_center_register.html @@ -95,7 +95,7 @@
        -

        ${get_course_about_section(course, 'university')} ${course.display_number_with_default | h} ${course.display_name_with_default}

        +

        ${get_course_about_section(course, 'university')} ${course.display_number_with_default | h} ${course.display_name_with_default | h}

        % if registration:

        ${_('Your Pearson VUE Proctored Exam Registration')}

        @@ -442,7 +442,7 @@ % endif
        -

        ${_("About {university} {course_number}").format(university=get_course_about_section(course, 'university'), course_number=course.course.display_number_with_default)}

        +

        ${_("About {university} {course_number}").format(university=get_course_about_section(course, 'university'), course_number=course.course.display_number_with_default) | h}

        % if course.has_ended(): ${_('Course Completed:')} ${course.end_date_text} From eaeaf3a520bd9b5af359dc7810c3aa38be5518b7 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 21:44:48 -0400 Subject: [PATCH 016/206] fix formatting error --- lms/templates/courseware/info.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index 0a520e0a1e..ed48f69363 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course.number} Course Info").format(course=course.display_number_with_default) | h} +<%block name="title">${_("{course.number} Course Info").format(course=course) | h} <%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%! from courseware.courses import get_course_info_section From 18458bd94b9cd984a6919c831d9b70c94d62adea Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 21:45:51 -0400 Subject: [PATCH 017/206] wrong accessor --- lms/templates/courseware/info.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index ed48f69363..a1aad363a9 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -7,7 +7,7 @@ <%static:css group='course'/> -<%block name="title">${_("{course.number} Course Info").format(course=course) | h} +<%block name="title">${_("{course.display_number_with_default} Course Info").format(course=course) | h} <%include file="/courseware/course_navigation.html" args="active_page='info'" /> <%! from courseware.courses import get_course_info_section From f57b02c19948a0220e65976ddae92f0d719fc918 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 22:19:59 -0400 Subject: [PATCH 018/206] add a metadata cache and re-compute ignore when deleting a course to speed up the whole thing as well as squelching warnings in the console output --- .../management/commands/delete_course.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 5aafe9f8a6..4d8c4eda55 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -9,12 +9,14 @@ from xmodule.course_module import CourseDescriptor from .prompt import query_yes_no from auth.authz import _delete_course_group +from request_cache.middleware import RequestCache +from django.core.cache import get_cache # # To run from command line: rake cms:delete_course LOC=MITx/111/Foo1 # - +CACHE = get_cache('mongo_metadata_inheritance') class Command(BaseCommand): help = '''Delete a MongoDB backed course''' @@ -22,7 +24,7 @@ class Command(BaseCommand): if len(args) != 1 and len(args) != 2: raise CommandError("delete_course requires one or more arguments: |commit|") - loc_str = args[0] + course_id = args[0] commit = False if len(args) == 2: @@ -34,9 +36,14 @@ class Command(BaseCommand): ms = modulestore('direct') cs = contentstore() - if query_yes_no("Deleting course {0}. Confirm?".format(loc_str), default="no"): + ms.metadata_inheritance_cache_subsystem = CACHE + ms.request_cache = RequestCache.get_request_cache() + org, course_num, run = course_id.split("/") + ms.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) + + if query_yes_no("Deleting course {0}. Confirm?".format(course_id), default="no"): if query_yes_no("Are you sure. This action cannot be undone!", default="no"): - loc = CourseDescriptor.id_to_location(loc_str) + loc = CourseDescriptor.id_to_location(course_id) if delete_course(ms, cs, loc, commit): print 'removing User permissions from course....' # in the django layer, we need to remove all the user permissions groups associated with this course From 9f07507396ad37f1ab85d7ca115c753716c26631 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 22:28:53 -0400 Subject: [PATCH 019/206] add same optimizations to the clone course command --- .../contentstore/management/commands/clone.py | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/clone.py b/cms/djangoapps/contentstore/management/commands/clone.py index f20625d7f2..5fffe29543 100644 --- a/cms/djangoapps/contentstore/management/commands/clone.py +++ b/cms/djangoapps/contentstore/management/commands/clone.py @@ -12,7 +12,14 @@ from auth.authz import _copy_course_group # # To run from command line: rake cms:clone SOURCE_LOC=MITx/111/Foo1 DEST_LOC=MITx/135/Foo3 # +from request_cache.middleware import RequestCache +from django.core.cache import get_cache +# +# To run from command line: rake cms:delete_course LOC=MITx/111/Foo1 +# + +CACHE = get_cache('mongo_metadata_inheritance') class Command(BaseCommand): """Clone a MongoDB-backed course to another location""" @@ -21,19 +28,27 @@ class Command(BaseCommand): def handle(self, *args, **options): "Execute the command" if len(args) != 2: - raise CommandError("clone requires two arguments: ") + raise CommandError("clone requires two arguments: ") - source_location_str = args[0] - dest_location_str = args[1] + source_course_id = args[0] + dest_course_id = args[1] mstore = modulestore('direct') cstore = contentstore() - print("Cloning course {0} to {1}".format(source_location_str, dest_location_str)) + mstore.metadata_inheritance_cache_subsystem = CACHE + mstore.request_cache = RequestCache.get_request_cache() + org, course_num, run = dest_course_id.split("/") + mstore.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) - source_location = CourseDescriptor.id_to_location(source_location_str) - dest_location = CourseDescriptor.id_to_location(dest_location_str) + print("Cloning course {0} to {1}".format(source_course_id, dest_course_id)) + + source_location = CourseDescriptor.id_to_location(source_course_id) + dest_location = CourseDescriptor.id_to_location(dest_course_id) if clone_course(mstore, cstore, source_location, dest_location): + # be sure to recompute metadata inheritance after all those updates + mstore.refresh_cached_metadata_inheritance_tree(dest_location) + print("copying User permissions...") _copy_course_group(source_location, dest_location) From 9da0aa2e8564401b9b72fc80c6898de98cac69e2 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 2 Aug 2013 22:30:31 -0400 Subject: [PATCH 020/206] rename clone.py to a more descriptive clone_course.py --- .../management/commands/{clone.py => clone_course.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename cms/djangoapps/contentstore/management/commands/{clone.py => clone_course.py} (100%) diff --git a/cms/djangoapps/contentstore/management/commands/clone.py b/cms/djangoapps/contentstore/management/commands/clone_course.py similarity index 100% rename from cms/djangoapps/contentstore/management/commands/clone.py rename to cms/djangoapps/contentstore/management/commands/clone_course.py From da4f295d006e49b0d10dc90e306f6cba1640dd68 Mon Sep 17 00:00:00 2001 From: Alexander Kryklia Date: Fri, 12 Jul 2013 18:20:54 +0300 Subject: [PATCH 021/206] Add TabsEditingDescriptor for VideoAlpha Updates comment Make Model descriptor class property. Fix tests --- cms/static/coffee/fixtures/tabs-edit.html | 33 +++++ cms/static/coffee/spec/tabs/edit.coffee | 95 ++++++++++++ cms/templates/widgets/tabs-aggregator.html | 21 +++ .../widgets/tabs/metadata-edit-tab.html | 24 +++ .../widgets/videoalpha/codemirror-edit.html | 33 +++++ .../widgets/videoalpha/subtitles.html | 0 .../test_files/test_tabseditingdescriptor.css | 4 + .../test_tabseditingdescriptor.scss | 4 + .../xmodule/xmodule/css/tabs/codemirror.scss | 19 +++ common/lib/xmodule/xmodule/css/tabs/tabs.scss | 137 ++++++++++++++++++ .../css/videoalpha/common_tabs_edit.scss | 0 common/lib/xmodule/xmodule/editing_module.py | 43 ++++++ .../js/src/tabs/tabs-aggregator.coffee | 125 ++++++++++++++++ .../xmodule/tests/test_editing_module.py | 64 ++++++++ .../xmodule/xmodule/tests/test_videoalpha.py | 34 ++++- .../lib/xmodule/xmodule/videoalpha_module.py | 22 ++- 16 files changed, 654 insertions(+), 4 deletions(-) create mode 100644 cms/static/coffee/fixtures/tabs-edit.html create mode 100644 cms/static/coffee/spec/tabs/edit.coffee create mode 100644 cms/templates/widgets/tabs-aggregator.html create mode 100644 cms/templates/widgets/tabs/metadata-edit-tab.html create mode 100644 cms/templates/widgets/videoalpha/codemirror-edit.html create mode 100644 cms/templates/widgets/videoalpha/subtitles.html create mode 100644 common/lib/xmodule/test_files/test_tabseditingdescriptor.css create mode 100644 common/lib/xmodule/test_files/test_tabseditingdescriptor.scss create mode 100644 common/lib/xmodule/xmodule/css/tabs/codemirror.scss create mode 100644 common/lib/xmodule/xmodule/css/tabs/tabs.scss create mode 100644 common/lib/xmodule/xmodule/css/videoalpha/common_tabs_edit.scss create mode 100644 common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee create mode 100644 common/lib/xmodule/xmodule/tests/test_editing_module.py diff --git a/cms/static/coffee/fixtures/tabs-edit.html b/cms/static/coffee/fixtures/tabs-edit.html new file mode 100644 index 0000000000..c83a145622 --- /dev/null +++ b/cms/static/coffee/fixtures/tabs-edit.html @@ -0,0 +1,33 @@ +

        +
        +
        +
        + +
        +
        +
        + +
        +
        + Transcripts +
        +
        + Subtitles +
        +
        +
        + +
        +
        +
        + +
        +
        + diff --git a/cms/static/coffee/spec/tabs/edit.coffee b/cms/static/coffee/spec/tabs/edit.coffee new file mode 100644 index 0000000000..734e398c74 --- /dev/null +++ b/cms/static/coffee/spec/tabs/edit.coffee @@ -0,0 +1,95 @@ +describe "TabsEditingDescriptor", -> + beforeEach -> + @isInactiveClass = "is-inactive" + @isCurrent = "current" + loadFixtures 'tabs-edit.html' + @descriptor = new TabsEditingDescriptor($('.base_wrapper')) + @html_id = 'test_id' + @tab_0_switch = jasmine.createSpy('tab_0_switch'); + @tab_0_modelUpdate = jasmine.createSpy('tab_0_modelUpdate'); + @tab_1_switch = jasmine.createSpy('tab_1_switch'); + @tab_1_modelUpdate = jasmine.createSpy('tab_1_modelUpdate'); + TabsEditingDescriptor.Model.addModelUpdate(@html_id, 'Tab 0 Editor', @tab_0_modelUpdate) + TabsEditingDescriptor.Model.addOnSwitch(@html_id, 'Tab 0 Editor', @tab_0_switch) + TabsEditingDescriptor.Model.addModelUpdate(@html_id, 'Tab 1 Transcripts', @tab_1_modelUpdate) + TabsEditingDescriptor.Model.addOnSwitch(@html_id, 'Tab 1 Transcripts', @tab_1_switch) + + spyOn($.fn, 'hide').andCallThrough() + spyOn($.fn, 'show').andCallThrough() + spyOn(TabsEditingDescriptor.Model, 'initialize') + spyOn(TabsEditingDescriptor.Model, 'updateValue') + + afterEach -> + TabsEditingDescriptor.Model.modules= {} + + describe "constructor", -> + it "first tab should be visible", -> + expect(@descriptor.$tabs.first()).toHaveClass(@isCurrent) + expect(@descriptor.$content.first()).not.toHaveClass(@isInactiveClass) + + describe "onSwitchEditor", -> + it "switching tabs changes styles", -> + @descriptor.$tabs.eq(1).trigger("click") + expect(@descriptor.$tabs.eq(0)).not.toHaveClass(@isCurrent) + expect(@descriptor.$content.eq(0)).toHaveClass(@isInactiveClass) + expect(@descriptor.$tabs.eq(1)).toHaveClass(@isCurrent) + expect(@descriptor.$content.eq(1)).not.toHaveClass(@isInactiveClass) + expect(@tab_1_switch).toHaveBeenCalled() + + it "if click on current tab, nothing should happen", -> + spyOn($.fn, 'trigger').andCallThrough() + currentTab = @descriptor.$tabs.filter('.' + @isCurrent) + @descriptor.$tabs.eq(0).trigger("click") + expect(@descriptor.$tabs.filter('.' + @isCurrent)).toEqual(currentTab) + expect($.fn.trigger.calls.length).toEqual(1) + + it "onSwitch function call", -> + @descriptor.$tabs.eq(1).trigger("click") + expect(TabsEditingDescriptor.Model.updateValue).toHaveBeenCalled() + expect(@tab_1_switch).toHaveBeenCalled() + + describe "save", -> + it "function for current tab should be called", -> + @descriptor.$tabs.eq(1).trigger("click") + data = @descriptor.save().data + expect(@tab_1_modelUpdate).toHaveBeenCalled() + + it "detach click event", -> + spyOn($.fn, "off") + @descriptor.save() + expect($.fn.off).toHaveBeenCalledWith( + 'click', + '.editor-tabs .tab', + @descriptor.onSwitchEditor + ) + + describe "editor/settings header", -> + it "is hidden", -> + expect(@descriptor.element.find(".component-edit-header").css('display')).toEqual('none') + +describe "TabsEditingDescriptor special save cases", -> + beforeEach -> + @isInactiveClass = "is-inactive" + @isCurrent = "current" + loadFixtures 'tabs-edit.html' + @descriptor = new window.TabsEditingDescriptor($('.base_wrapper')) + @html_id = 'test_id' + + describe "save", -> + it "case: no init", -> + data = @descriptor.save().data + expect(data).toEqual(null) + + it "case: no function in model update", -> + TabsEditingDescriptor.Model.initialize(@html_id) + data = @descriptor.save().data + expect(data).toEqual(null) + + it "case: no function in model update, but value presented", -> + @tab_0_modelUpdate = jasmine.createSpy('tab_0_modelUpdate').andReturn(1) + TabsEditingDescriptor.Model.addModelUpdate(@html_id, 'Tab 0 Editor', @tab_0_modelUpdate) + @descriptor.$tabs.eq(1).trigger("click") + expect(@tab_0_modelUpdate).toHaveBeenCalled() + data = @descriptor.save().data + expect(data).toEqual(1) + diff --git a/cms/templates/widgets/tabs-aggregator.html b/cms/templates/widgets/tabs-aggregator.html new file mode 100644 index 0000000000..d5953005b5 --- /dev/null +++ b/cms/templates/widgets/tabs-aggregator.html @@ -0,0 +1,21 @@ +<%! from django.utils.translation import ugettext as _ %> +
        +
        +
        + + +
        +
        + % for tab in tabs: +
        + <%include file="${tab['template']}" args="tabName=tab['name']"/> +
        + % endfor +
        +
        +
        + diff --git a/cms/templates/widgets/tabs/metadata-edit-tab.html b/cms/templates/widgets/tabs/metadata-edit-tab.html new file mode 100644 index 0000000000..4fc8314afd --- /dev/null +++ b/cms/templates/widgets/tabs/metadata-edit-tab.html @@ -0,0 +1,24 @@ +<%namespace name='static' file='../../static_content.html'/> +<% + import json +%> + +## js templates + + + + + + + + +
        + diff --git a/cms/templates/widgets/videoalpha/codemirror-edit.html b/cms/templates/widgets/videoalpha/codemirror-edit.html new file mode 100644 index 0000000000..10c28d829c --- /dev/null +++ b/cms/templates/widgets/videoalpha/codemirror-edit.html @@ -0,0 +1,33 @@ +<%! from django.utils.translation import ugettext as _ %> +<%page args="tabName"/> +
        + +
        + + + diff --git a/cms/templates/widgets/videoalpha/subtitles.html b/cms/templates/widgets/videoalpha/subtitles.html new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/lib/xmodule/test_files/test_tabseditingdescriptor.css b/common/lib/xmodule/test_files/test_tabseditingdescriptor.css new file mode 100644 index 0000000000..eba23dedfd --- /dev/null +++ b/common/lib/xmodule/test_files/test_tabseditingdescriptor.css @@ -0,0 +1,4 @@ +.supertestclass{ + color: red; +} + diff --git a/common/lib/xmodule/test_files/test_tabseditingdescriptor.scss b/common/lib/xmodule/test_files/test_tabseditingdescriptor.scss new file mode 100644 index 0000000000..eba23dedfd --- /dev/null +++ b/common/lib/xmodule/test_files/test_tabseditingdescriptor.scss @@ -0,0 +1,4 @@ +.supertestclass{ + color: red; +} + diff --git a/common/lib/xmodule/xmodule/css/tabs/codemirror.scss b/common/lib/xmodule/xmodule/css/tabs/codemirror.scss new file mode 100644 index 0000000000..0b10a1a09d --- /dev/null +++ b/common/lib/xmodule/xmodule/css/tabs/codemirror.scss @@ -0,0 +1,19 @@ +.editor{ + @include clearfix(); + + .CodeMirror { + @include box-sizing(border-box); + width: 100%; + position: relative; + height: 379px; + border: 1px solid #3c3c3c; + border-top: 1px solid #8891a1; + background: $white; + color: #3c3c3c; + } + + .CodeMirror-scroll { + height: 100%; + } +} + diff --git a/common/lib/xmodule/xmodule/css/tabs/tabs.scss b/common/lib/xmodule/xmodule/css/tabs/tabs.scss new file mode 100644 index 0000000000..3991bbad23 --- /dev/null +++ b/common/lib/xmodule/xmodule/css/tabs/tabs.scss @@ -0,0 +1,137 @@ +// styles duped from _unit.scss - Edit Header (Component Name, Mode-Editor, Mode-Settings) + + +.tabs-wrapper{ + padding-top: 0; + position: relative; + + .wrapper-comp-settings { + // set visibility to metadata editor + display: block; + } +} + +.editor-single-tab-name { + display: none; +} + + +.editor-with-tabs { + @include clearfix(); + position: relative; + + + .edit-header { + @include box-sizing(border-box); + padding: 18px 0 18px $baseline; + top: 0 !important; // ugly override for second level tab override + right: 0; + background-color: $blue; + border-bottom: 1px solid $blue-d2; + color: $white; + + //Component Name + .component-name { + @extend .t-copy-sub1; + position: relative; + top: 0; + left: 0; + width: 50%; + color: $white; + font-weight: 600; + + + + em { + display: inline-block; + margin-right: ($baseline/4); + font-weight: 400; + color: $white; + } + } + + //Nav-Edit Modes + .editor-tabs { + list-style: none; + right: 0; + top: ($baseline/4); + position: absolute; + padding: 12px ($baseline*0.75); + + .inner_tab_wrap { + display: inline-block; + margin-left: 8px; + + a.tab { + @include font-size(14); + @include linear-gradient(top, rgba(255, 255, 255, .3), rgba(255, 255, 255, 0)); + border: 1px solid $blue-d1; + border-radius: 3px; + padding: ($baseline/4) ($baseline); + background-color: $blue; + font-weight: bold; + color: $white; + + &.current { + @include linear-gradient($blue, $blue); + color: $blue-d1; + box-shadow: inset 0 1px 2px 1px $shadow-l1; + background-color: $blue-d4; + cursor: default; + } + + &:hover { + box-shadow: inset 0 1px 2px 1px $shadow; + background-image: linear-gradient(#009FE6, #009FE6) !important; + } + } + } + } + } + + .is-inactive { + display: none; + } + + .comp-subtitles-entry { + text-align: center; + + .file-upload { + display: none; + } + + .comp-subtitles-import-list { + > li { + display: block; + margin: $baseline/2 0px $baseline/2 0; + } + + .blue-button { + font-size: 1em; + display: block; + width: 70%; + margin: 0 auto; + text-align: center; + } + } + } + + +} + +.component-tab { + background: $white; + position: relative; + border-top: 1px solid #8891a1; + + &#advanced { + padding: 0; + border: none; + } + + .blue-button { + @include blue-button; + } +} + + diff --git a/common/lib/xmodule/xmodule/css/videoalpha/common_tabs_edit.scss b/common/lib/xmodule/xmodule/css/videoalpha/common_tabs_edit.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index df4ebc5646..eed8fb310f 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/common/lib/xmodule/xmodule/editing_module.py @@ -1,3 +1,5 @@ +"""Descriptors for XBlocks/Xmodules, that provide editing of atrributes""" + from pkg_resources import resource_string from xmodule.mako_module import MakoModuleDescriptor from xblock.core import Scope, String @@ -7,6 +9,7 @@ log = logging.getLogger(__name__) class EditingFields(object): + """Contains specific template information (the raw data body)""" data = String(scope=Scope.content, default='') @@ -29,6 +32,46 @@ class EditingDescriptor(EditingFields, MakoModuleDescriptor): return _context +class TabsEditingDescriptor(EditingFields, MakoModuleDescriptor): + """ + Module that provides a raw editing view of its data and children. It does not + perform any validation on its definition---just passes it along to the browser. + + This class is intended to be used as a mixin. + + Engine (module_edit.js) wants for metadata editor + template to be always loaded, so don't forget to include + settings tab in your module descriptor. + """ + mako_template = "widgets/tabs-aggregator.html" + css = {'scss': [resource_string(__name__, 'css/tabs/tabs.scss')]} + js = {'coffee': [resource_string( + __name__, 'js/src/tabs/tabs-aggregator.coffee')]} + js_module_name = "TabsEditingDescriptor" + tabs = [] + + def get_context(self): + _context = super(TabsEditingDescriptor, self).get_context() + _context.update({ + 'tabs': self.tabs, + 'html_id': self.location.html_id(), # element_id + 'data': self.data, + }) + return _context + + @classmethod + def get_css(cls): + # load every tab's css + for tab in cls.tabs: + tab_styles = tab.get('css', {}) + for css_type, css_content in tab_styles.items(): + if css_type in cls.css: + cls.css[css_type].extend(css_content) + else: + cls.css[css_type] = css_content + return cls.css + + class XMLEditingDescriptor(EditingDescriptor): """ Module that provides a raw editing view of its data as XML. It does not perform diff --git a/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee b/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee new file mode 100644 index 0000000000..7c6e85e34b --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee @@ -0,0 +1,125 @@ +class @TabsEditingDescriptor + @isInactiveClass : "is-inactive" + + constructor: (element) -> + @element = element; + ### + Not tested on syncing of multiple editors of same type in tabs + (Like many CodeMirrors). + ### + + # hide editor/settings bar + $('.component-edit-header').hide() + + @$tabs = $(".tab", @element) + @$content = $(".component-tab", @element) + + @element.find('.editor-tabs .tab').each (index, value) => + $(value).on('click', @onSwitchEditor) + + # If default visible tab is not setted or if were marked as current + # more than 1 tab just first tab will be shown + currentTab = @$tabs.filter('.current') + currentTab = @$tabs.first() if currentTab.length isnt 1 + @html_id = @$tabs.closest('.wrapper-comp-editor').data('html_id') + currentTab.trigger("click", [true, @html_id]) + + onSwitchEditor: (e, firstTime, html_id) => + e.preventDefault(); + + isInactiveClass = TabsEditingDescriptor.isInactiveClass + $currentTarget = $(e.currentTarget) + + if not $currentTarget.hasClass('current') or firstTime is true + previousTab = null + + @$tabs.each( (index, value) -> + if $(value).hasClass('current') + previousTab = $(value).html() + ) + + # init and save data from previous tab + TabsEditingDescriptor.Model.updateValue(@html_id, previousTab) + + # Save data from editor in previous tab to editor in current tab here. + # (to be implemented when there is a use case for this functionality) + + # call onswitch + onSwitchFunction = TabsEditingDescriptor.Model.modules[@html_id].tabSwitch[$currentTarget.text()] + onSwitchFunction() if $.isFunction(onSwitchFunction) + + @$tabs.removeClass('current') + $currentTarget.addClass('current') + + # Tabs are implemeted like anchors. Therefore we can use hash to find + # corresponding content + content_id = $currentTarget.attr('href') + + @$content + .addClass(isInactiveClass) + .filter(content_id) + .removeClass(isInactiveClass) + + save: -> + @element.off('click', '.editor-tabs .tab', @onSwitchEditor) + current_tab = @$tabs.filter('.current').html() + data: TabsEditingDescriptor.Model.getValue(@html_id, current_tab) + + @Model : + addModelUpdate : (id, tabName, modelUpdateFunction) -> + ### + Function that registers 'modelUpdate' functions of every tab. + These functions are used to update value, which will be returned + by calling save on component. + ### + @initialize(id) + @modules[id].modelUpdate[tabName] = modelUpdateFunction + + addOnSwitch : (id, tabName, onSwitchFunction) -> + ### + Function that registers functions invoked when switching + to particular tab. + ### + @initialize(id) + @modules[id].tabSwitch[tabName] = onSwitchFunction + + updateValue : (id, tabName) -> + ### + Function that invokes when switching tabs. + It ensures that data from previous tab is stored. + If new tab need this data, it should retrieve it from + stored value. + ### + @initialize(id) + modelUpdateFunction = @modules[id]['modelUpdate'][tabName] + @modules[id]['value'] = modelUpdateFunction() if $.isFunction(modelUpdateFunction) + + getValue : (id, tabName) -> + ### + Retrieves stored data on component save. + 1. When we switching tabs - previous tab data is always saved to @[id].value + 2. If current tab have registered 'modelUpdate' method, it should be invoked 1st. + (If we have edited in 1st tab, then switched to 2nd, 2nd tab should + care about getting data from @[id].value in onSwitch.) + ### + if not @modules[id] + return null + if $.isFunction(@modules[id]['modelUpdate'][tabName]) + return @modules[id]['modelUpdate'][tabName]() + else + if typeof @modules[id]['value'] is 'undefined' + return null + else + return @modules[id]['value'] + + # html_id's of descriptors will be stored in modules variable as + # containers for callbacks. + modules: {} + + initialize : (id) -> + ### + Initialize objects per id. Id is html_id of descriptor. + ### + @modules[id] = @modules[id] or {} + @modules[id].tabSwitch = @modules[id]['tabSwitch'] or {} + @modules[id].modelUpdate = @modules[id]['modelUpdate'] or {} diff --git a/common/lib/xmodule/xmodule/tests/test_editing_module.py b/common/lib/xmodule/xmodule/tests/test_editing_module.py new file mode 100644 index 0000000000..b734c44637 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_editing_module.py @@ -0,0 +1,64 @@ +# -*- coding: utf-8 -*- + +import unittest +import os +import logging + +from mock import Mock +from pkg_resources import resource_string +from xmodule.editing_module import TabsEditingDescriptor + +from .import get_test_system + +log = logging.getLogger(__name__) + + +class TabsEditingDescriptorTestCase(unittest.TestCase): + """ Testing TabsEditingDescriptor""" + + def setUp(self): + super(TabsEditingDescriptorTestCase, self).setUp() + system = get_test_system() + system.render_template = Mock(return_value="
        Test Template HTML
        ") + self.tabs = [ + { + 'name': "Test_css", + 'template': "tabs/codemirror-edit.html", + 'current': True, + 'css': { + 'scss': [resource_string(__name__, + '../../test_files/test_tabseditingdescriptor.scss')], + 'css': [resource_string(__name__, + '../../test_files/test_tabseditingdescriptor.css')] + } + }, + { + 'name': "Subtitles", + 'template': "videoalpha/subtitles.html", + }, + { + 'name': "Settings", + 'template': "tabs/video-metadata-edit-tab.html" + } + ] + + TabsEditingDescriptor.tabs = self.tabs + self.descriptor = TabsEditingDescriptor( + runtime=system, + model_data={}) + + def test_get_css(self): + """test get_css""" + css = self.descriptor.get_css() + test_files_dir = os.path.dirname(__file__).replace('xmodule/tests', 'test_files') + test_css_file = os.path.join(test_files_dir, 'test_tabseditingdescriptor.scss') + with open(test_css_file) as new_css: + added_css = new_css.read() + self.assertEqual(css['scss'].pop(), added_css) + self.assertEqual(css['css'].pop(), added_css) + + def test_get_context(self): + """"test get_context""" + rendered_context = self.descriptor.get_context() + self.assertListEqual(rendered_context['tabs'], self.tabs) + diff --git a/common/lib/xmodule/xmodule/tests/test_videoalpha.py b/common/lib/xmodule/xmodule/tests/test_videoalpha.py index f2258d1b55..659bfb2fa0 100644 --- a/common/lib/xmodule/xmodule/tests/test_videoalpha.py +++ b/common/lib/xmodule/xmodule/tests/test_videoalpha.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- """Test for Video Alpha Xmodule functional logic. -These tests data readed from xml, not from mongo. +These tests data readed from xml or from mongo. we have a ModuleStoreTestCase class defined in common/lib/xmodule/xmodule/modulestore/tests/django_utils.py. You can @@ -12,10 +12,12 @@ in common/lib/xmodule/xmodule/modulestore/tests/factories.py to create the course, section, subsection, unit, etc. """ +import unittest from xmodule.videoalpha_module import VideoAlphaDescriptor from . import LogicTest from lxml import etree - +from pkg_resources import resource_string +from .import get_test_system class VideoAlphaModuleTest(LogicTest): """Logic tests for VideoAlpha Xmodule.""" @@ -49,3 +51,31 @@ class VideoAlphaModuleTest(LogicTest): ) output = self.xmodule.get_timeframe(xmltree) self.assertEqual(output, (247, 47079)) + + +class VideoAlphaDescriptorTest(unittest.TestCase): + """Test for VideoAlphaDescriptor""" + + def setUp(self): + system = get_test_system() + self.descriptor = VideoAlphaDescriptor( + runtime=system, + model_data={}) + + def test_get_context(self): + """"test get_context""" + correct_tabs = [ + { + 'name': "XML", + 'template': "videoalpha/codemirror-edit.html", + 'css': {'scss': [resource_string(__name__, + '../css/tabs/codemirror.scss')]}, + 'current': True, + }, + { + 'name': "Settings", + 'template': "tabs/metadata-edit-tab.html" + } + ] + rendered_context = self.descriptor.get_context() + self.assertListEqual(rendered_context['tabs'], correct_tabs) diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 4ca337dcb0..89018e64c6 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -20,7 +20,7 @@ from django.http import Http404 from django.conf import settings from xmodule.x_module import XModule -from xmodule.raw_module import RawDescriptor +from xmodule.editing_module import TabsEditingDescriptor from xmodule.modulestore.mongo import MongoModuleStore from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent @@ -187,6 +187,24 @@ class VideoAlphaModule(VideoAlphaFields, XModule): }) -class VideoAlphaDescriptor(VideoAlphaFields, RawDescriptor): +class VideoAlphaDescriptor(VideoAlphaFields, TabsEditingDescriptor): """Descriptor for `VideoAlphaModule`.""" module_class = VideoAlphaModule + template_dir_name = "videoalpha" + + tabs = [ + { + 'name': "XML", + 'template': "videoalpha/codemirror-edit.html", + 'css': {'scss': [resource_string(__name__, 'css/tabs/codemirror.scss')]}, + 'current': True, + }, + # { + # 'name': "Subtitles", + # 'template': "videoalpha/subtitles.html", + # }, + { + 'name': "Settings", + 'template': "tabs/metadata-edit-tab.html" + } + ] From 6225c2a532820416d2acb1d36ee08694c4d61f52 Mon Sep 17 00:00:00 2001 From: Alexander Kryklia Date: Mon, 5 Aug 2013 12:37:04 +0300 Subject: [PATCH 022/206] Fixes --- common/lib/xmodule/xmodule/tests/test_editing_module.py | 3 +-- common/lib/xmodule/xmodule/videoalpha_module.py | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_editing_module.py b/common/lib/xmodule/xmodule/tests/test_editing_module.py index b734c44637..03e257940f 100644 --- a/common/lib/xmodule/xmodule/tests/test_editing_module.py +++ b/common/lib/xmodule/xmodule/tests/test_editing_module.py @@ -1,5 +1,4 @@ -# -*- coding: utf-8 -*- - +""" Tests for editing descriptors""" import unittest import os import logging diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 89018e64c6..084389411a 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -190,7 +190,6 @@ class VideoAlphaModule(VideoAlphaFields, XModule): class VideoAlphaDescriptor(VideoAlphaFields, TabsEditingDescriptor): """Descriptor for `VideoAlphaModule`.""" module_class = VideoAlphaModule - template_dir_name = "videoalpha" tabs = [ { From a0aa652df41abd5e420fdab70e7cbd1bfe52fff9 Mon Sep 17 00:00:00 2001 From: Valera Rozuvan Date: Mon, 5 Aug 2013 15:00:18 +0300 Subject: [PATCH 023/206] Fixed typo of function call. Added comment why it is necessary. --- .../xmodule/xmodule/js/src/videoalpha/03_video_player.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/03_video_player.js b/common/lib/xmodule/xmodule/js/src/videoalpha/03_video_player.js index 6f2e232844..be2287be6e 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/03_video_player.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/03_video_player.js @@ -318,7 +318,14 @@ function (HTML5Video) { availablePlaybackRates = this.videoPlayer.player.getAvailablePlaybackRates(); if ((this.currentPlayerMode === 'html5') && (this.videoType === 'youtube')) { if (availablePlaybackRates.length === 1) { - restartUsingFlash(this); + // This condition is needed in cases when Firefox version is less than 20. In those versions + // HTML5 playback could only happen at 1 speed (no speed changing). Therefore, in this case, + // we need to switch back to Flash. + // + // This might also happen in other browsers, therefore when we have 1 speed available, we fall + // back to Flash. + + _restartUsingFlash(this); return; } else if (availablePlaybackRates.length > 1) { From a52964a78664a4ba2d32a8c3686214dfbfb3cbf7 Mon Sep 17 00:00:00 2001 From: Vasyl Nakvasiuk Date: Mon, 5 Aug 2013 17:14:11 +0300 Subject: [PATCH 024/206] Hotfix world cloud functionality --- lms/templates/word_cloud.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lms/templates/word_cloud.html b/lms/templates/word_cloud.html index 5ec83afcc7..dc66b04a9b 100644 --- a/lms/templates/word_cloud.html +++ b/lms/templates/word_cloud.html @@ -1,3 +1,5 @@ +<%! from django.utils.translation import ugettext as _ %> +
        Date: Fri, 2 Aug 2013 14:19:37 -0400 Subject: [PATCH 025/206] more template fixes for faq, press, and help --- lms/templates/static_templates/about.html | 2 +- lms/templates/static_templates/faq.html | 16 ++++++++-------- lms/templates/static_templates/help.html | 4 ++-- lms/templates/static_templates/press.html | 4 +++- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lms/templates/static_templates/about.html b/lms/templates/static_templates/about.html index fbaaf4e816..fa6188746a 100644 --- a/lms/templates/static_templates/about.html +++ b/lms/templates/static_templates/about.html @@ -52,7 +52,7 @@

        ${_("{massachusetts_institute_of_technology}").format(massachusetts_institute_of_technology="Massachusetts Institute of Technology")}

        ${_("The {massachusetts_institute_of_technology} — a coeducational, privately endowed research university founded in 1861 — is dedicated to advancing knowledge and educating students in science, technology, and other areas of scholarship that will best serve the nation and the world in the 21st century. The Institute has close to 1,000 faculty and 10,000 undergraduate and graduate students. It is organized into five Schools: Architecture and Urban Planning; Engineering; Humanities, Arts, and Social Sciences; {Sloan} School of Management; and Science.").format(massachusetts_institute_of_technology="Massachusetts Institute of Technology", Sloan="Sloan")}

        -

        ${_("{MIT}'s commitment to innovation has led to a host of scientific breakthroughs and technological advances. Achievements of the Institute's faculty and graduates have included the first chemical synthesis of penicillin and vitamin A, the development of inertial guidance systems, modern technologies for artificial limbs, and the magnetic core memory that made possible the development of digital computers. 78 alumni, faculty, researchers and staff have won Nobel Prizes.")}

        +

        ${_("{MIT}'s commitment to innovation has led to a host of scientific breakthroughs and technological advances. Achievements of the Institute's faculty and graduates have included the first chemical synthesis of penicillin and vitamin A, the development of inertial guidance systems, modern technologies for artificial limbs, and the magnetic core memory that made possible the development of digital computers. 78 alumni, faculty, researchers and staff have won Nobel Prizes.").format(MIT="MIT")}

        ${_("Current areas of research and education include neuroscience and the study of the brain and mind, bioengineering, cancer, energy, the environment and sustainable development, information sciences and technology, new media, financial technology, and entrepreneurship.")}

        diff --git a/lms/templates/static_templates/faq.html b/lms/templates/static_templates/faq.html index ea3e0e4725..9b90fd5c6c 100644 --- a/lms/templates/static_templates/faq.html +++ b/lms/templates/static_templates/faq.html @@ -21,7 +21,7 @@

        ${_("Organization")}

        ${_("What is {edX}?").format(edX="edX")}

        -

        ${_('{edX} is a not-for-profit enterprise of its founding partners, the {MIT_long} ({MIT}) and {harvard_u} that offers online learning to on-campus students and to millions of people around the world. To do so, {edX} is building an open-source online learning platform and hosts an online web portal at www.edx.org for online education.').format(edX="edX", MIT_long="Massachusetts Institute of Technology", MIT="MIT", harvard_u="Harvard University")}

        +

        ${_('{EdX} is a not-for-profit enterprise of its founding partners, the {MIT_long} ({MIT}) and {harvard_u} that offers online learning to on-campus students and to millions of people around the world. To do so, {edX} is building an open-source online learning platform and hosts an online web portal at www.edx.org for online education.').format(EdX="EdX", edX="edX", MIT_long="Massachusetts Institute of Technology", MIT="MIT", harvard_u="Harvard University")}

        ${_("{EdX} currently offers {HarvardX}, {MITx} and {BerkeleyX} classes online for free. Beginning in fall 2013, {edX} will offer {WellesleyX} , {GeorgetownX} and the {UTexas} classes online for free. The {UT} System includes nine universities and six health institutions. In 2014, {edX} will further expand its consortium, including several international schools, when it begins offering courses from {EPFL}, {McGill}, {Toronto}, {ANU}, {Delft}, and {Rice}. The {edX} institutions aim to extend their collective reach to build a global community of online students. Along with offering online courses, the three universities undertake research on how students learn and how technology can transform learning both on-campus and online throughout the world.").format( EdX="EdX", edX="edX", @@ -32,7 +32,7 @@ GeorgetownX="GeorgetownX", UTexas="University of Texas System", UT="UT", - EPFL="École Polytechnique Fédérale de Lausanne", + EPFL=u"École Polytechnique Fédérale de Lausanne", McGill="McGill University", Toronto="University of Toronto", ANU="Australian National University", @@ -44,12 +44,12 @@

        ${_("Will {edX} be adding additional X Universities?").format(edX="edX")}

        -

        ${_("More than 200 institutions from around the world have expressed interest in collaborating with {edX} since {Harvard} and {MIT} announced its creation in May. {EdX} is focused above all on quality and developing the best not-for-profit model for online education. In addition to providing online courses on the {edX} platform, the {x_consortium} will be a forum in which members can share experiences around online learning. {Harvard}, {MIT}, {Berkeley}, the {UTexas} and the other {consortium} members will work collaboratively to establish the {\"X_University\" Consortium}, whose membership will expand to include additional \"{X_Universities}.\" As noted above, {edX}'s newest {consortium} members include {Wellesley}, {Georgetown}, {EPFL}, {McGill}, {Toronto}, {ANU}, {Delft}, and {Rice}. Each member of the {consortium} will offer courses on the {edX} platform as an \"{X_University}\". The gathering of many universities' educational content together on one site will enable learners worldwide to access the offered course content of any participating university from a single website, and to use a set of online educational tools shared by all participating universities.").format( +

        ${_("More than 200 institutions from around the world have expressed interest in collaborating with {edX} since {Harvard} and {MIT} announced its creation in May. {EdX} is focused above all on quality and developing the best not-for-profit model for online education. In addition to providing online courses on the {edX} platform, the {x_consortium} will be a forum in which members can share experiences around online learning. {Harvard}, {MIT}, {Berkeley}, the {UTexas} and the other {consortium} members will work collaboratively to establish the {x_consortium}, whose membership will expand to include additional \"{X_Universities}.\" As noted above, {edX}'s newest {consortium} members include {Wellesley}, {Georgetown}, {EPFL}, {McGill}, {Toronto}, {ANU}, {Delft}, and {Rice}. Each member of the {consortium} will offer courses on the {edX} platform as an \"{X_University}\". The gathering of many universities' educational content together on one site will enable learners worldwide to access the offered course content of any participating university from a single website, and to use a set of online educational tools shared by all participating universities.").format( EdX="EdX", edX="edX", Harvard="Harvard", MIT="MIT", - x_consortium="\"X_University\" Consortium", + x_consortium="\"X University\" Consortium", consortium="consortium", X_Universities="X Universities", X_University="X University", @@ -57,14 +57,14 @@ Wellesley="Wellesley", Georgetown="Georgetown", UTexas="University of Texas System", - EPFL="École Polytechnique Fédérale de Lausanne", + EPFL=u"École Polytechnique Fédérale de Lausanne", McGill="McGill University", Toronto="University of Toronto", ANU="Australian National University", Delft="Delft University of Technology", Rice="Rice University", )}

        -

        ${_("{edX} will actively explore the addition of other institutions from around the world to the {edX} platform, and looks forward to adding more \"{X Universities}\".").format(edX="edX", X_Universities="X Universities")} +

        ${_("{EdX} will actively explore the addition of other institutions from around the world to the {edX} platform, and looks forward to adding more \"{X_Universities}\".").format(EdX="EdX", edX="edX", X_Universities="X Universities")}

        @@ -79,7 +79,7 @@

        ${_("Will certificates be awarded?")}

        ${_("Yes. Online learners who demonstrate mastery of subjects can earn a certificate " "of mastery. Certificates will be issued at the discretion of {edX} and the underlying " - "{X_University} that offered the course under the name of the underlying \"{X_University}\" from where the course originated, i.e. {HarvardX}, {MITx} or {BerkeleyX}. " + "\"{X_University}\" that offered the course under the name of the underlying \"{X_University}\" from where the course originated, i.e. {HarvardX}, {MITx} or {BerkeleyX}. " "For the courses in Fall 2012, those certificates will be free. There is a plan to " "charge a modest fee for certificates in the future. Note: At this time, {edX} is " "holding certificates for learners connected with Cuba, Iran, Syria and Sudan " @@ -102,7 +102,7 @@

        ${_("{EdX} institutions have assembled faculty members who will collect and analyze data to assess results and the impact {edX} is having on learning.").format(EdX="EdX", edX="edX")}

        -

        ${_("How may I apply to study with {edX}?")}

        +

        ${_("How may I apply to study with {edX}?").format(edX="edX")}

        ${_('Simply complete the online {link_start}signup form{link_end}. Enrolling will create your unique student record in the {edX} database, allow you to register for classes, and to receive a certificate on successful completion.').format(link_start='', link_end='', edX="edX")}

        diff --git a/lms/templates/static_templates/help.html b/lms/templates/static_templates/help.html index f473495f9b..f4f65be053 100644 --- a/lms/templates/static_templates/help.html +++ b/lms/templates/static_templates/help.html @@ -139,7 +139,7 @@

        ${_("How can I talk to professors, fellows and teaching assistants?")}

        -

        ${_("The Discussion Forums are the best place to reach out to the {edX} teaching team for your class, and you don\'t have to wait in line or rearrange your schedule to fit your professor'\s - just post your questions. The response isn\'t always immediate, but it\'s usually pretty darned quick.").format(edX="edX")}

        +

        ${_("The Discussion Forums are the best place to reach out to the {edX} teaching team for your class, and you don\'t have to wait in line or rearrange your schedule to fit your professor\'s - just post your questions. The response isn\'t always immediate, but it\'s usually pretty darned quick.").format(edX="edX")}

        @@ -319,7 +319,7 @@ ${_("The Classes")} ${_("Certificates and Credits")} ${_("{edX} & Open source").format(edX="edX")} - $${_("Other Help Questions - Account Questions")} + ${_("Other Help Questions - Account Questions")}
      diff --git a/lms/templates/static_templates/press.html b/lms/templates/static_templates/press.html index 8abead7175..58acbca332 100644 --- a/lms/templates/static_templates/press.html +++ b/lms/templates/static_templates/press.html @@ -18,7 +18,9 @@ % for article in articles:
      - ${"" % static.url('images/press/' + article.image)} + + +
      From 09a3bc01be920a0464e21f8a63af74b859959af0 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 5 Aug 2013 12:51:42 -0400 Subject: [PATCH 026/206] show the organization, course number, course run in the course listing page in CMS --- cms/djangoapps/contentstore/views/user.py | 3 +++ cms/static/sass/views/_dashboard.scss | 7 ++++++- cms/templates/index.html | 5 ++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index e1c75bad0f..24df0c64c2 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -54,6 +54,9 @@ def index(request): course.location, course_id=course.location.course_id, ), + course.display_org_with_default, + course.display_number_with_default, + course.location.name ) return render_to_response('index.html', { diff --git a/cms/static/sass/views/_dashboard.scss b/cms/static/sass/views/_dashboard.scss index ab3ad6f810..6206891542 100644 --- a/cms/static/sass/views/_dashboard.scss +++ b/cms/static/sass/views/_dashboard.scss @@ -330,10 +330,15 @@ body.dashboard { .class-name { display: block; - font-size: 19px; + font-size: 22px; font-weight: 300; } + .class-listing-info { + color: #989898; + margin-right: 15px; + } + .detail { font-size: 14px; font-weight: 400; diff --git a/cms/templates/index.html b/cms/templates/index.html index 9c845ccb5a..150b0c070f 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -133,10 +133,13 @@ %if len(courses) > 0:
        - %for course, url, lms_link in sorted(courses, key=lambda s: s[0].lower() if s[0] is not None else ''): + %for course, url, lms_link, org, num, run in sorted(courses, key=lambda s: s[0].lower() if s[0] is not None else ''):
      • ${course} + + ${num} + ${run} ${_("View Live")}
        • From ef53e8df6c845bed881e1be9059123dd5ea197aa Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 5 Aug 2013 12:56:06 -0400 Subject: [PATCH 027/206] one field had wrong label nesting --- cms/templates/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/index.html b/cms/templates/index.html index 150b0c070f..207740a02f 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -137,7 +137,7 @@
      • ${course} - + ${org} ${num} ${run} From 3893ac665601c3f5eaf7853a92674304d79800b4 Mon Sep 17 00:00:00 2001 From: Julian Arni Date: Mon, 5 Aug 2013 12:59:16 -0400 Subject: [PATCH 028/206] Bump up pip requirement to latest. Solves an issue with using git > 1.8.1 for github based pip requirements. --- requirements/edx/base.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index ffa2977dbd..688d32d5fe 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -35,7 +35,7 @@ nltk==2.0.4 paramiko==1.9.0 path.py==3.0.1 Pillow==1.7.8 -pip>=1.3 +pip>=1.4 polib==1.0.3 pycrypto>=2.6 pygments==1.5 From 8807d9fe51c14d035827c71542f4fe691789b4df Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 5 Aug 2013 11:25:02 -0400 Subject: [PATCH 029/206] Integrate JsonResponse into lms/djangoapps/instructor/views/api.py --- lms/djangoapps/instructor/views/api.py | 90 +++++++++----------------- 1 file changed, 31 insertions(+), 59 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 34698225c5..ecfb7f46c6 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -7,12 +7,13 @@ Many of these GETs may become PUTs in the future. """ import re -import json import logging from django_future.csrf import ensure_csrf_cookie from django.views.decorators.cache import cache_control from django.core.urlresolvers import reverse -from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden +from django.utils.translation import ugettext as _ +from django.http import HttpResponseBadRequest, HttpResponseForbidden +from util.json_request import JsonResponse from courseware.access import has_access from courseware.courses import get_course_with_access, get_course_by_id @@ -41,13 +42,23 @@ def common_exceptions_400(func): Catches common exceptions and renders matching 400 errors. (decorator without arguments) """ - def wrapped(*args, **kwargs): # pylint: disable=C0111 + def wrapped(request, *args, **kwargs): # pylint: disable=C0111 + use_json = (request.is_ajax() or + request.META.get("HTTP_ACCEPT", "").startswith("application/json")) try: - return func(*args, **kwargs) + return func(request, *args, **kwargs) except User.DoesNotExist: - return HttpResponseBadRequest("User does not exist.") + message = "User does not exist." + if use_json: + return JsonResponse({"error": _(message)}, 400) + else: + return HttpResponseBadRequest(_(message)) except AlreadyRunningError: - return HttpResponseBadRequest("Task already running.") + message = "Task is already running." + if use_json: + return JsonResponse({"error": _(message)}, 400) + else: + return HttpResponseBadRequest(_(message)) return wrapped @@ -82,10 +93,7 @@ def require_query_params(*args, **kwargs): error_response_data['info'][param] = extra if len(error_response_data['parameters']) > 0: - return HttpResponseBadRequest( - json.dumps(error_response_data), - mimetype="application/json", - ) + return JsonResponse(error_response_data, status=400) else: return func(*args, **kwargs) return wrapped @@ -194,10 +202,7 @@ def students_update_enrollment(request, course_id): 'results': results, 'auto_enroll': auto_enroll, } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -255,10 +260,7 @@ def modify_access(request, course_id): 'action': action, 'success': 'yes', } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -308,10 +310,7 @@ def list_course_role_members(request, course_id): course, rolename )), } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -330,10 +329,7 @@ def get_grading_config(request, course_id): 'course_id': course_id, 'grading_config_summary': grading_config_summary, } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -362,10 +358,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=W06 'queried_features': query_features, 'available_features': available_features, } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) else: header, datarows = analytics.csvs.format_dictlist(student_data, query_features) return analytics.csvs.create_csv_response("enrolled_profiles.csv", header, datarows) @@ -417,10 +410,7 @@ def get_distribution(request, course_id): if p_dist.type == 'EASY_CHOICE': response_payload['feature_results']['choices_display_names'] = p_dist.choices_display_names - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -449,10 +439,7 @@ def get_student_progress_url(request, course_id): 'course_id': course_id, 'progress_url': progress_url, } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -521,10 +508,7 @@ def reset_student_attempts(request, course_id): else: return HttpResponseBadRequest() - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -572,10 +556,7 @@ def rescore_problem(request, course_id): else: return HttpResponseBadRequest() - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -618,10 +599,7 @@ def list_instructor_tasks(request, course_id): response_payload = { 'tasks': map(extract_task_features, tasks), } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -680,10 +658,7 @@ def list_forum_members(request, course_id): 'course_id': course_id, rolename: map(extract_user_info, users), } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) @ensure_csrf_cookie @@ -747,10 +722,7 @@ def update_forum_role_membership(request, course_id): 'course_id': course_id, 'action': action, } - response = HttpResponse( - json.dumps(response_payload), content_type="application/json" - ) - return response + return JsonResponse(response_payload) def _split_input_list(str_list): @@ -782,6 +754,6 @@ def _msk_from_problem_urlname(course_id, urlname): urlname = "problem/" + urlname - (org, course_name, _) = course_id.split("/") + (org, course_name, __) = course_id.split("/") module_state_key = "i4x://" + org + "/" + course_name + "/" + urlname return module_state_key From 7b01147c1a92b34503bebc40a82e6669859fad48 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 5 Aug 2013 13:51:47 -0400 Subject: [PATCH 030/206] Translate messages early --- lms/djangoapps/instructor/views/api.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ecfb7f46c6..64d70232f8 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -48,17 +48,17 @@ def common_exceptions_400(func): try: return func(request, *args, **kwargs) except User.DoesNotExist: - message = "User does not exist." + message = _("User does not exist.") if use_json: - return JsonResponse({"error": _(message)}, 400) + return JsonResponse({"error": message}, 400) else: - return HttpResponseBadRequest(_(message)) + return HttpResponseBadRequest(message) except AlreadyRunningError: - message = "Task is already running." + message = _("Task is already running.") if use_json: - return JsonResponse({"error": _(message)}, 400) + return JsonResponse({"error": message}, 400) else: - return HttpResponseBadRequest(_(message)) + return HttpResponseBadRequest(message) return wrapped From 94976df8d72f19c9052d365e4c4a40db48d9910a Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 5 Aug 2013 14:16:26 -0400 Subject: [PATCH 031/206] Check that content-type starts with application/json When Chrome sends the AJAX request to add a user to the course team, it sets the Content-type to "application/json". However, when Firefox sends the same request, it sets the Content-type to "application/json; charset=UTF-8". This commit only checks that the Content-type begins with "application/json", not is identical to it; that way, Firefox can play, too. --- cms/djangoapps/contentstore/views/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index e1c75bad0f..5b38d47452 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -179,7 +179,7 @@ def course_team_user(request, org, course, name, email): return JsonResponse() # all other operations require the requesting user to specify a role - if request.META.get("CONTENT_TYPE", "") == "application/json" and request.body: + if request.META.get("CONTENT_TYPE", "").startswith("application/json") and request.body: try: payload = json.loads(request.body) except: From a227b14fdd879156c63599f957f5b53f3503a200 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 5 Aug 2013 13:58:43 -0400 Subject: [PATCH 032/206] Auto-enroll course staff to fix "View Live". STUD-554 Code review feedback. --- CHANGELOG.rst | 3 + .../contentstore/tests/test_contentstore.py | 13 +++- .../contentstore/tests/test_users.py | 66 ++++++++++++++++++- cms/djangoapps/contentstore/tests/utils.py | 10 +++ cms/djangoapps/contentstore/views/course.py | 5 ++ cms/djangoapps/contentstore/views/user.py | 7 ++ 6 files changed, 101 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cba3d2dbf8..9100e05c68 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Studio: Studio course authors (both instructors and staff) will be auto-enrolled +for their courses so that "View Live" works. + Common: Added ratelimiting to our authentication backend. Common: Add additional logging to cover login attempts and logouts. diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 4c9fcf7f81..3b2cb7dcc9 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -49,7 +49,7 @@ import datetime from pytz import UTC from uuid import uuid4 from pymongo import MongoClient - +from .utils import get_enrollment_count TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex @@ -1041,12 +1041,18 @@ class ContentStoreTest(ModuleStoreTestCase): data = parse_json(resp) self.assertNotIn('ErrMsg', data) self.assertEqual(data['id'], 'i4x://MITx/{0}/course/2013_Spring'.format(test_course_data['number'])) + # Verify that the creator is now registered in the course. + self.assertEqual(1, get_enrollment_count(self.user, self._get_course_id(test_course_data))) return test_course_data def test_create_course_check_forum_seeding(self): """Test new course creation and verify forum seeding """ test_course_data = self.assert_created_course(number_suffix=uuid4().hex) - self.assertTrue(are_permissions_roles_seeded('MITx/{0}/2013_Spring'.format(test_course_data['number']))) + self.assertTrue(are_permissions_roles_seeded(self._get_course_id(test_course_data))) + + def _get_course_id(self, test_course_data): + """Returns the course ID (org/number/run).""" + return "{org}/{number}/{run}".format(**test_course_data) def test_create_course_duplicate_course(self): """Test new course creation - error path""" @@ -1057,10 +1063,13 @@ class ContentStoreTest(ModuleStoreTestCase): """ Checks that the course did not get created """ + course_id = self._get_course_id(self.course_data) + initially_enrolled_count = get_enrollment_count(self.user, course_id) resp = self.client.post(reverse('create_new_course'), self.course_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) self.assertEqual(data['ErrMsg'], error_message) + self.assertEqual(initially_enrolled_count, get_enrollment_count(self.user, course_id)) def test_create_course_duplicate_number(self): """Test new course creation - error path""" diff --git a/cms/djangoapps/contentstore/tests/test_users.py b/cms/djangoapps/contentstore/tests/test_users.py index 4b9dcf487f..54981b3c49 100644 --- a/cms/djangoapps/contentstore/tests/test_users.py +++ b/cms/djangoapps/contentstore/tests/test_users.py @@ -1,5 +1,8 @@ +""" +Tests for contentstore/views/user.py. +""" import json -from .utils import CourseTestCase +from .utils import CourseTestCase, get_enrollment_count from django.contrib.auth.models import User, Group from django.core.urlresolvers import reverse from auth.authz import get_course_groupname_for_role @@ -90,6 +93,7 @@ class UsersTestCase(CourseTestCase): # no content: should not be in any roles self.assertNotIn(self.staff_groupname, groups) self.assertNotIn(self.inst_groupname, groups) + self.assert_not_enrolled() def test_detail_post_staff(self): resp = self.client.post( @@ -104,6 +108,7 @@ class UsersTestCase(CourseTestCase): groups = [g.name for g in ext_user.groups.all()] self.assertIn(self.staff_groupname, groups) self.assertNotIn(self.inst_groupname, groups) + self.assert_enrolled() def test_detail_post_staff_other_inst(self): inst_group, _ = Group.objects.get_or_create(name=self.inst_groupname) @@ -122,6 +127,7 @@ class UsersTestCase(CourseTestCase): groups = [g.name for g in ext_user.groups.all()] self.assertIn(self.staff_groupname, groups) self.assertNotIn(self.inst_groupname, groups) + self.assert_enrolled() # check that other user is unchanged user = User.objects.get(email=self.user.email) groups = [g.name for g in user.groups.all()] @@ -141,6 +147,7 @@ class UsersTestCase(CourseTestCase): groups = [g.name for g in ext_user.groups.all()] self.assertNotIn(self.staff_groupname, groups) self.assertIn(self.inst_groupname, groups) + self.assert_enrolled() def test_detail_post_missing_role(self): resp = self.client.post( @@ -152,6 +159,7 @@ class UsersTestCase(CourseTestCase): self.assert4XX(resp.status_code) result = json.loads(resp.content) self.assertIn("error", result) + self.assert_not_enrolled() def test_detail_post_bad_json(self): resp = self.client.post( @@ -163,6 +171,7 @@ class UsersTestCase(CourseTestCase): self.assert4XX(resp.status_code) result = json.loads(resp.content) self.assertIn("error", result) + self.assert_not_enrolled() def test_detail_post_no_json(self): resp = self.client.post( @@ -176,6 +185,7 @@ class UsersTestCase(CourseTestCase): groups = [g.name for g in ext_user.groups.all()] self.assertIn(self.staff_groupname, groups) self.assertNotIn(self.inst_groupname, groups) + self.assert_enrolled() def test_detail_delete_staff(self): group, _ = Group.objects.get_or_create(name=self.staff_groupname) @@ -317,3 +327,57 @@ class UsersTestCase(CourseTestCase): ext_user = User.objects.get(email=self.ext_user.email) groups = [g.name for g in ext_user.groups.all()] self.assertIn(self.staff_groupname, groups) + + def test_user_not_initially_enrolled(self): + # Verify that ext_user is not enrolled in the new course before being added as a staff member. + self.assert_not_enrolled() + + def test_remove_staff_does_not_enroll(self): + # Add user with staff permissions. + self.client.post( + self.detail_url, + data=json.dumps({"role": "staff"}), + content_type="application/json", + HTTP_ACCEPT="application/json", + ) + self.assert_enrolled() + # Remove user from staff on course. Will not un-enroll them from the course. + resp = self.client.delete( + self.detail_url, + HTTP_ACCEPT="application/json", + ) + self.assert2XX(resp.status_code) + self.assert_enrolled() + + def test_staff_to_instructor_still_enrolled(self): + # Add user with staff permission. + self.client.post( + self.detail_url, + data=json.dumps({"role": "staff"}), + content_type="application/json", + HTTP_ACCEPT="application/json", + ) + self.assert_enrolled() + # Now add with instructor permission. Verify still enrolled. + resp = self.client.post( + self.detail_url, + data=json.dumps({"role": "instructor"}), + content_type="application/json", + HTTP_ACCEPT="application/json", + ) + self.assert2XX(resp.status_code) + self.assert_enrolled() + + def assert_not_enrolled(self): + """ Asserts that self.ext_user is not enrolled in self.course. """ + self.assertEqual( + 0, get_enrollment_count(self.ext_user, self.course.location.course_id), + 'Did not expect ext_user to be enrolled in course' + ) + + def assert_enrolled(self): + """ Asserts that self.ext_user is enrolled in self.course. """ + self.assertEqual( + 1, get_enrollment_count(self.ext_user, self.course.location.course_id), + 'User ext_user should have been enrolled in the course' + ) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index a3f211a703..88ff08c3bd 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -5,6 +5,7 @@ Utilities for contentstore tests import json from student.models import Registration +from student.models import CourseEnrollment from django.contrib.auth.models import User from django.test.client import Client @@ -27,6 +28,15 @@ def registration(email): return Registration.objects.get(user__email=email) +def get_enrollment_count(user, course_id): + """ + Gets the number of enrolled registrants for given course and user=self.user. + + This should always be 0 or 1. + """ + return CourseEnrollment.objects.filter(user=user, course_id=course_id).count() + + class CourseTestCase(ModuleStoreTestCase): def setUp(self): """ diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e68210dea4..b35f9bcd56 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -44,6 +44,8 @@ from .component import ( from django_comment_common.utils import seed_permissions_roles +from student.models import CourseEnrollment + from xmodule.html_module import AboutDescriptor __all__ = ['course_index', 'create_new_course', 'course_info', 'course_info_updates', 'get_course_settings', @@ -162,6 +164,9 @@ def create_new_course(request): # seed the forums seed_permissions_roles(new_course.location.course_id) + # auto-enroll the course creator in the course so that "View Live" will work. + CourseEnrollment.objects.get_or_create(user=request.user, course_id=new_course.location.course_id) + return JsonResponse({'id': new_course.location.url()}) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index e1c75bad0f..22376b36db 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -23,6 +23,8 @@ from course_creators.views import ( from .access import has_access +from student.models import CourseEnrollment + @login_required @ensure_csrf_cookie @@ -201,6 +203,8 @@ def course_team_user(request, org, course, name, email): return JsonResponse(msg, 400) user.groups.add(groups["instructor"]) user.save() + # auto-enroll the course creator in the course so that "View Live" will work. + CourseEnrollment.objects.get_or_create(user=user, course_id=location.course_id) elif role == "staff": # if we're trying to downgrade a user from "instructor" to "staff", # make sure we have at least one other instructor in the course team. @@ -214,6 +218,9 @@ def course_team_user(request, org, course, name, email): user.groups.remove(groups["instructor"]) user.groups.add(groups["staff"]) user.save() + # auto-enroll the course creator in the course so that "View Live" will work. + CourseEnrollment.objects.get_or_create(user=user, course_id=location.course_id) + return JsonResponse() From 9e96b9525b61f6aa44d0de09836fb2b5d3f1560e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 5 Aug 2013 15:24:19 -0400 Subject: [PATCH 033/206] tie in the normal courseware link url rewriting in capa module since that presents HTML via an Ajax callbacks and not via module_render.py --- common/lib/xmodule/xmodule/capa_module.py | 12 ++++++++++-- common/lib/xmodule/xmodule/x_module.py | 4 ++++ lms/djangoapps/courseware/module_render.py | 9 +++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 8489b5f986..dbd535a471 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -534,8 +534,16 @@ class CapaModule(CapaFields, XModule): id=self.location.html_id(), ajax_url=self.system.ajax_url ) + html + "
      " - # now do the substitutions which are filesystem based, e.g. '/static/' prefixes - return self.system.replace_urls(html) + # now do all the substitutions which the LMS module_render normally does, but + # we need to do here explicitly since we can get called for our HTML via AJAX + html = self.system.replace_urls(html) + if self.system.replace_course_urls: + html = self.system.replace_course_urls(html) + + if self.system.replace_jump_to_id_urls: + html = self.system.replace_jump_to_id_urls(html) + + return html def handle_ajax(self, dispatch, data): """ diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index d399001a6a..b991858dca 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -902,6 +902,8 @@ class ModuleSystem(Runtime): s3_interface=None, cache=None, can_execute_unsafe_code=None, + replace_course_urls=None, + replace_jump_to_id_urls=None ): ''' Create a closure around the system environment. @@ -978,6 +980,8 @@ class ModuleSystem(Runtime): self.cache = cache or DoNothingCache() self.can_execute_unsafe_code = can_execute_unsafe_code or (lambda: False) + self.replace_course_urls = replace_course_urls + self.replace_jump_to_id_urls = replace_jump_to_id_urls def get(self, attr): ''' provide uniform access to attributes (like etree).''' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index caa8967476..0a48c56f87 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -349,6 +349,15 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours data_directory=getattr(descriptor, 'data_dir', None), course_namespace=descriptor.location._replace(category=None, name=None), ), + replace_course_urls=partial( + static_replace.replace_course_urls, + course_id=course_id + ), + replace_jump_to_id_urls=partial( + static_replace.replace_jump_to_id_urls, + course_id=course_id, + jump_to_id_base_url=reverse('jump_to_id', kwargs={'course_id': course_id, 'module_id': ''}) + ), node_path=settings.NODE_PATH, xblock_model_data=xblock_model_data, publish=publish, From 1abea0b406a08f808f1850871a62fcb6504dcccc Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Mon, 5 Aug 2013 14:16:26 -0400 Subject: [PATCH 034/206] Check that content-type starts with application/json When Chrome sends the AJAX request to add a user to the course team, it sets the Content-type to "application/json". However, when Firefox sends the same request, it sets the Content-type to "application/json; charset=UTF-8". This commit only checks that the Content-type begins with "application/json", not is identical to it; that way, Firefox can play, too. --- cms/djangoapps/contentstore/views/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index e1c75bad0f..5b38d47452 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -179,7 +179,7 @@ def course_team_user(request, org, course, name, email): return JsonResponse() # all other operations require the requesting user to specify a role - if request.META.get("CONTENT_TYPE", "") == "application/json" and request.body: + if request.META.get("CONTENT_TYPE", "").startswith("application/json") and request.body: try: payload = json.loads(request.body) except: From f85cfc11fa9d9adaaf0757b4fc75f47913c7cba8 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Mon, 5 Aug 2013 13:23:45 -0400 Subject: [PATCH 035/206] redirects lms landing page to student.views.index if there is no marketing site resets the cms edge redirect to '/' --- cms/djangoapps/contentstore/views/requests.py | 2 +- lms/djangoapps/branding/views.py | 13 ++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/cms/djangoapps/contentstore/views/requests.py b/cms/djangoapps/contentstore/views/requests.py index dc1b7871ab..abbf84755e 100644 --- a/cms/djangoapps/contentstore/views/requests.py +++ b/cms/djangoapps/contentstore/views/requests.py @@ -12,7 +12,7 @@ def landing(request, org, course, coursename): # points to the temporary edge page def edge(request): - return redirect('/dashboard') + return redirect('/') def event(request): diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index 985dfa52d0..531b61f3ff 100644 --- a/lms/djangoapps/branding/views.py +++ b/lms/djangoapps/branding/views.py @@ -4,7 +4,6 @@ from django.shortcuts import redirect from django_future.csrf import ensure_csrf_cookie import student.views -import branding import courseware.views from mitxmako.shortcuts import marketing_link from util.cache import cache_if_anonymous @@ -26,11 +25,7 @@ def index(request): if settings.MITX_FEATURES.get('ENABLE_MKTG_SITE'): return redirect(settings.MKTG_URLS.get('ROOT')) - university = branding.get_university(request.META.get('HTTP_HOST')) - if university is None: - return student.views.index(request, user=request.user) - - return redirect('/') + return student.views.index(request, user=request.user) @ensure_csrf_cookie @@ -44,8 +39,4 @@ def courses(request): if settings.MITX_FEATURES.get('ENABLE_MKTG_SITE', False): return redirect(marketing_link('COURSES'), permanent=True) - university = branding.get_university(request.META.get('HTTP_HOST')) - if university is None: - return courseware.views.courses(request) - - return redirect('/') + return courseware.views.courses(request) From 2c75caca20d3f850567e4db046e4b446ca2c94b7 Mon Sep 17 00:00:00 2001 From: Frances Botsford Date: Mon, 5 Aug 2013 16:10:08 -0400 Subject: [PATCH 036/206] adjustment to sidebar help text on PDF textbook page in studio --- cms/templates/textbooks.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/textbooks.html b/cms/templates/textbooks.html index 3ee1bc5b74..28349b5436 100644 --- a/cms/templates/textbooks.html +++ b/cms/templates/textbooks.html @@ -76,7 +76,7 @@ $(function() {

      ${_("What if my book isn't divided into chapters?")}

      -

      ${_("If you haven't broken your textbook into chapters, you can upload the entire text as Chapter 1.")}

      +

      ${_("If you haven't broken your text into chapters, you can upload the entire text as a single chapter and enter a name of your choice in the Chapter Name field.")}

    From 6200b2903fadc0c73bb1cd2aba8aca7f3ca2ac4b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Jul 2013 14:58:27 -0400 Subject: [PATCH 037/206] have the Files and Upload pages surface a 'portable_url' which uses the /static/ shorthand which is more portable across course runs --- cms/djangoapps/contentstore/views/assets.py | 4 +++- cms/templates/asset_index.html | 4 ++-- common/lib/xmodule/xmodule/contentstore/content.py | 7 +++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index e4201cddd7..11bea55833 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -105,6 +105,7 @@ def asset_index(request, org, course, name): asset_location = StaticContent.compute_location(asset_id['org'], asset_id['course'], asset_id['name']) display_info['url'] = StaticContent.get_url_path_from_location(asset_location) + display_info['portable_url'] = StaticContent.get_static_path_from_location(asset_location) # note, due to the schema change we may not have a 'thumbnail_location' in the result set _thumbnail_location = asset.get('thumbnail_location', None) @@ -187,12 +188,13 @@ def upload_asset(request, org, course, coursename): response_payload = {'displayname': content.name, 'uploadDate': get_default_time_display(readback.last_modified_at), 'url': StaticContent.get_url_path_from_location(content.location), + 'portable_url': StaticContent.get_static_path_from_location(content.location), 'thumb_url': StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None, 'msg': 'Upload completed' } response = JsonResponse(response_payload) - response['asset_url'] = StaticContent.get_url_path_from_location(content.location) + response['asset_url'] = StaticContent.get_static_path_from_location(content.location) return response diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index 6c92994a6f..c681cf5058 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -29,7 +29,7 @@ {{uploadDate}} - + @@ -89,7 +89,7 @@ ${asset['uploadDate']} - + diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 28a78ea8c1..b270d1dbcb 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -58,6 +58,13 @@ class StaticContent(object): else: return None + @staticmethod + def get_static_path_from_location(location): + if location is not None: + return "/static/{name}".format(**location.dict()) + else: + return None + @staticmethod def get_base_url_path_for_course_assets(loc): if loc is not None: From d329e3c833de9071da0c9251b79c8e28f48f28a1 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Jul 2013 17:23:09 -0400 Subject: [PATCH 038/206] don't attempt to do URL rewriting in the import step, let the LMS/CMS runtimes do it on the fly on reference --- .../xmodule/modulestore/xml_importer.py | 89 ------------------- 1 file changed, 89 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 8df768f442..698310da87 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -1,7 +1,6 @@ import logging import os import mimetypes -from lxml.html import rewrite_links as lxml_rewrite_links from path import path from xblock.core import Scope @@ -61,45 +60,6 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ return remap_dict -def verify_content_links(module, base_dir, static_content_store, link, remap_dict=None): - if link.startswith('/static/'): - # yes, then parse out the name - path = link[len('/static/'):] - - static_pathname = base_dir / path - - if os.path.exists(static_pathname): - try: - content_loc = StaticContent.compute_location(module.location.org, module.location.course, path) - filename = os.path.basename(path) - mime_type = mimetypes.guess_type(filename)[0] - - with open(static_pathname, 'rb') as f: - data = f.read() - - content = StaticContent(content_loc, filename, mime_type, data, import_path=path) - - # first let's save a thumbnail so we can get back a thumbnail location - (thumbnail_content, thumbnail_location) = static_content_store.generate_thumbnail(content) - - if thumbnail_content is not None: - content.thumbnail_location = thumbnail_location - - #then commit the content - static_content_store.save(content) - - new_link = StaticContent.get_url_path_from_location(content_loc) - - if remap_dict is not None: - remap_dict[link] = new_link - - return new_link - except Exception, e: - logging.exception('Skipping failed content load from {0}. Exception: {1}'.format(path, e)) - - return link - - def import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None, verbose=False): # remap module to the new namespace if target_location_namespace is not None: @@ -126,27 +86,6 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path, module.children = new_locs if hasattr(module, 'data'): - # cdodge: now go through any link references to '/static/' and make sure we've imported - # it as a StaticContent asset - try: - remap_dict = {} - - # use the rewrite_links as a utility means to enumerate through all links - # in the module data. We use that to load that reference into our asset store - # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to - # do the rewrites natively in that code. - # For example, what I'm seeing is -> - # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's - # no good, so we have to do this kludge - if isinstance(module.data, str) or isinstance(module.data, unicode): # some module 'data' fields are non strings which blows up the link traversal code - lxml_rewrite_links(module.data, lambda link: verify_content_links(module, course_data_path, static_content_store, link, remap_dict)) - - for key in remap_dict.keys(): - module.data = module.data.replace(key, remap_dict[key]) - - except Exception: - logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location)) - modulestore.update_item(module.location, module.data) if module.has_children: @@ -166,9 +105,6 @@ def import_course_from_xml(modulestore, static_content_store, course_data_path, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}] # note, add 'progress' when we can support it on Edge - # a bit of a hack, but typically the "course image" which is shown on marketing pages is hard coded to /images/course_image.jpg - # so let's make sure we import in case there are no other references to it in the modules - verify_content_links(module, course_data_path, static_content_store, '/static/images/course_image.jpg') import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace, verbose=verbose) @@ -241,10 +177,6 @@ def import_from_xml(store, data_dir, course_dirs=None, import_module(module, store, course_data_path, static_content_store) - # a bit of a hack, but typically the "course image" which is shown on marketing pages is hard coded to /images/course_image.jpg - # so let's make sure we import in case there are no other references to it in the modules - verify_content_links(module, course_data_path, static_content_store, '/static/images/course_image.jpg') - course_items.append(module) # then import all the static content @@ -302,27 +234,6 @@ def import_module(module, store, course_data_path, static_content_store, allow_n module_data = {} if 'data' in content: module_data = content['data'] - - # cdodge: now go through any link references to '/static/' and make sure we've imported - # it as a StaticContent asset - try: - remap_dict = {} - - # use the rewrite_links as a utility means to enumerate through all links - # in the module data. We use that to load that reference into our asset store - # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to - # do the rewrites natively in that code. - # For example, what I'm seeing is -> - # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's - # no good, so we have to do this kludge - if isinstance(module_data, str) or isinstance(module_data, unicode): # some module 'data' fields are non strings which blows up the link traversal code - lxml_rewrite_links(module_data, lambda link: verify_content_links(module, course_data_path, static_content_store, link, remap_dict)) - - for key in remap_dict.keys(): - module_data = module_data.replace(key, remap_dict[key]) - - except Exception: - logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location)) else: module_data = content From 3dbe4c189f714a9e4930ef975109ce12da3b89c1 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Jul 2013 17:47:05 -0400 Subject: [PATCH 039/206] add unit tests around asserting that /static/ links are not re-written on import --- cms/djangoapps/contentstore/tests/test_contentstore.py | 10 ++++++++++ common/test/data/toy/course/2012_Fall.xml | 1 + common/test/data/toy/html/toyhtml.html | 1 + common/test/data/toy/html/toyhtml.xml | 1 + 4 files changed, 13 insertions(+) create mode 100644 common/test/data/toy/html/toyhtml.html create mode 100644 common/test/data/toy/html/toyhtml.xml diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 4c9fcf7f81..d01cd64a34 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -303,6 +303,16 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): num_drafts = self._get_draft_counts(course) self.assertEqual(num_drafts, 1) + def test_no_static_link_rewrites_on_import(self): + module_store = modulestore('direct') + import_from_xml(module_store, 'common/test/data/', ['toy']) + + handouts = module_store.get_item(Location(['i4x', 'edX', 'toy', 'course_info', 'handouts', None])) + self.assertIn('/static/', handouts.data) + + handouts = module_store.get_item(Location(['i4x', 'edX', 'toy', 'html', 'toyhtml', None])) + self.assertIn('/static/', handouts.data) + def test_import_textbook_as_content_element(self): module_store = modulestore('direct') import_from_xml(module_store, 'common/test/data/', ['toy']) diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index 679f7bbfdb..2fd5401c24 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -4,6 +4,7 @@ +