From 2ad934f1c2eab0dc789b71299429b1a8c1f0555c Mon Sep 17 00:00:00 2001 From: JonahStanley Date: Mon, 10 Jun 2013 11:41:17 -0400 Subject: [PATCH 01/30] Added testing for grading settings Please note that 2 tests will not reliably work: Deleting a grade- Was able to circumvent this with javascript Moving the grade slider- Could not circumvent, is skipped but the framework is in place --- .../contentstore/features/grading.feature | 55 ++++++++++ .../contentstore/features/grading.py | 101 ++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 cms/djangoapps/contentstore/features/grading.feature create mode 100644 cms/djangoapps/contentstore/features/grading.py diff --git a/cms/djangoapps/contentstore/features/grading.feature b/cms/djangoapps/contentstore/features/grading.feature new file mode 100644 index 0000000000..40694d6434 --- /dev/null +++ b/cms/djangoapps/contentstore/features/grading.feature @@ -0,0 +1,55 @@ +Feature: Course Grading + As a course author, I want to be able to configure how my course is graded + + Scenario: Users can add grading ranges + Given I have opened a new course in Studio + And I am viewing the grading settings + When I add "1" new grade + Then I see I now have "3" grades + + Scenario: Users can only have up to 5 grading ranges + Given I have opened a new course in Studio + And I am viewing the grading settings + When I add "6" new grades + Then I see I now have "5" grades + + #Cannot reliably make the delete button appear so using javascript instead + Scenario: Users can delete grading ranges + Given I have opened a new course in Studio + And I am viewing the grading settings + When I add "1" new grade + And I delete a grade + Then I see I now have "2" grades + + #Cannot reliably move the slider + @skip + Scenario: Users can move grading ranges + Given I have opened a new course in Studio + And I am viewing the grading settings + When I move a grading section + Then I see that the grade range has changed + + Scenario: Users can modify Assignment types + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I change assignment type "Homework" to "New Type" + And I go back to the main course page + Then I do see the assignment name "New Type" + And I do not see the assignment name "Homework" + + Scenario: Users can delete Assignment types + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I delete the assignment type "Homework" + And I go back to the main course page + Then I do not see the assignment name "Homework" + + Scenario: Users can add Assignment types + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I add a new assignment type "New Type" + And I go back to the main course page + Then I do see the assignment name "New Type" diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py new file mode 100644 index 0000000000..32fa5d6349 --- /dev/null +++ b/cms/djangoapps/contentstore/features/grading.py @@ -0,0 +1,101 @@ +#pylint: disable=C0111 +#pylint: disable=W0621 + +from lettuce import world, step +from common import * + + +@step(u'I am viewing the grading settings') +def view_grading_settings(step): + world.click_course_settings() + link_css = 'li.nav-course-settings-grading a' + world.css_click(link_css) + + +@step(u'I add "([^"]*)" new grade') +def add_grade(step, many): + grade_css = '.new-grade-button' + for i in range(int(many)): + world.css_click(grade_css) + + +@step(u'I delete a grade') +def delete_grade(step): + #grade_css = 'li.grade-specific-bar > a.remove-button' + #range_css = '.grade-specific-bar' + #world.css_find(range_css)[1].mouseover() + #world.css_click(grade_css) + world.browser.execute_script('document.getElementsByClassName("remove-button")[0].click()') + + +@step(u'I see I now have "([^"]*)" grades$') +def view_grade_slider(step, how_many): + grade_slider_css = '.grade-specific-bar' + all_grades = world.css_find(grade_slider_css) + assert len(all_grades) == int(how_many) + + +@step(u'I move a grading section') +def move_grade_slider(step): + moveable_css = '.ui-resizable-e' + f = world.css_find(moveable_css).first + f.action_chains.click_and_hold().move_by_offset(100, 100).release().perform() + + +@step(u'I change assignment type "([^"]*)" to "([^"]*)"$') +def change_assignment_name(step, old_name, new_name): + name_id = '#course-grading-assignment-name' + index = get_type_index(old_name) + f = world.css_find(name_id)[index] + assert index != -1 + for count in range(len(old_name)): + f._element.send_keys(Keys.END, Keys.BACK_SPACE) + f._element.send_keys(new_name) + + +@step(u'I go back to the main course page') +def main_course_page(step): + main_page_link_css = 'a[href="/MITx/999/course/Robot_Super_Course"]' + world.css_click(main_page_link_css) + + +@step(u'I do( not)? see the assignment name "([^"]*)"$') +def see_assignment_name(step, do_not, name): + assignment_menu_css = 'ul.menu > li > a' + assignment_menu = world.css_find(assignment_menu_css) + allnames = [item.html for item in assignment_menu] + if do_not: + assert not name in allnames + else: + assert name in allnames + + +@step(u'I delete the assignment type "([^"]*)"$') +def delete_assignment_type(step, to_delete): + delete_css = '.remove-grading-data' + delete = world.css_find(delete_css)[get_type_index(to_delete)] + delete.click() + + +@step(u'I add a new assignment type "([^"]*)"$') +def add_assignment_type(step, new_name): + add_button_css = '.add-grading-data' + world.css_click(add_button_css) + name_id = '#course-grading-assignment-name' + f = world.css_find(name_id)[4] + f._element.send_keys(new_name) + + +@step(u'I have populated the course') +def populate_course(step): + step.given('I have added a new section') + step.given('I have added a new subsection') + + +def get_type_index(name): + name_id = '#course-grading-assignment-name' + f = world.css_find(name_id) + for i in range(len(f)): + if f[i].value == name: + return i + return -1 From 29275c48d1fcbf1b05b26175983b5a06d786192e Mon Sep 17 00:00:00 2001 From: JonahStanley Date: Tue, 11 Jun 2013 11:29:54 -0400 Subject: [PATCH 02/30] Added in the drag and drop support --- cms/djangoapps/contentstore/features/grading.feature | 2 -- cms/djangoapps/contentstore/features/grading.py | 10 +++++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/features/grading.feature b/cms/djangoapps/contentstore/features/grading.feature index 40694d6434..78634cb964 100644 --- a/cms/djangoapps/contentstore/features/grading.feature +++ b/cms/djangoapps/contentstore/features/grading.feature @@ -21,8 +21,6 @@ Feature: Course Grading And I delete a grade Then I see I now have "2" grades - #Cannot reliably move the slider - @skip Scenario: Users can move grading ranges Given I have opened a new course in Studio And I am viewing the grading settings diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py index 32fa5d6349..c2c2206e02 100644 --- a/cms/djangoapps/contentstore/features/grading.py +++ b/cms/djangoapps/contentstore/features/grading.py @@ -39,7 +39,15 @@ def view_grade_slider(step, how_many): def move_grade_slider(step): moveable_css = '.ui-resizable-e' f = world.css_find(moveable_css).first - f.action_chains.click_and_hold().move_by_offset(100, 100).release().perform() + f.action_chains.drag_and_drop_by_offset(f._element, 100, 0).perform() + + +@step(u'I see that the grade range has changed') +def confirm_change(step): + range_css = '.range' + all_ranges = world.css_find(range_css) + for i in range(len(all_ranges)): + assert all_ranges[i].html != '0-50' @step(u'I change assignment type "([^"]*)" to "([^"]*)"$') From 85e3062225eb7af345f04aa713e13757fdd8157f Mon Sep 17 00:00:00 2001 From: JonahStanley Date: Tue, 18 Jun 2013 12:51:18 -0400 Subject: [PATCH 03/30] Fixed click to comply to new css_click --- cms/djangoapps/contentstore/features/grading.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py index c2c2206e02..4e59897c1c 100644 --- a/cms/djangoapps/contentstore/features/grading.py +++ b/cms/djangoapps/contentstore/features/grading.py @@ -81,8 +81,7 @@ def see_assignment_name(step, do_not, name): @step(u'I delete the assignment type "([^"]*)"$') def delete_assignment_type(step, to_delete): delete_css = '.remove-grading-data' - delete = world.css_find(delete_css)[get_type_index(to_delete)] - delete.click() + world.css_click(delete_css, index=get_type_index(to_delete)) @step(u'I add a new assignment type "([^"]*)"$') From 6e96b885186b8ea44b76a38aa788538b9a0d68c2 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 19 Jun 2013 11:28:16 -0400 Subject: [PATCH 04/30] On marketing site, disable course settings options that do not work. When on the marketing site (edx.org) disable portions of the course settings page in Studio that do not actually work in that environment. --- .../tests/test_course_settings.py | 45 +++++ cms/djangoapps/contentstore/views/course.py | 3 +- cms/templates/settings.html | 172 ++++++++++-------- 3 files changed, 139 insertions(+), 81 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 8c15b1ae95..3d676390ea 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -1,11 +1,13 @@ import datetime import json import copy +import mock from django.contrib.auth.models import User from django.test.client import Client from django.core.urlresolvers import reverse from django.utils.timezone import UTC +from django.test.utils import override_settings from xmodule.modulestore import Location from models.settings.course_details import (CourseDetails, CourseSettingsEncoder) @@ -118,6 +120,49 @@ class CourseDetailsTestCase(CourseTestCase): jsondetails.effort, "After set effort" ) + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) + def test_marketing_site_fetch(self): + settings_details_url = reverse('settings_details', + kwargs= {'org': self.course_location.org, + 'name': self.course_location.name, + 'course': self.course_location.course + }) + + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): + response = self.client.get(settings_details_url) + self.assertContains(response, "Course Summary Page") + self.assertContains(response, "your course summary page will not be viewable") + + self.assertContains(response, "Course Start Date") + self.assertContains(response, "Course End Date") + self.assertNotContains(response, "Enrollment Start Date") + self.assertNotContains(response, "Enrollment End Date") + self.assertContains(response, "not the dates shown on your course summary page") + + self.assertNotContains(response, "Introducing Your Course") + self.assertNotContains(response, "Requirements") + + def test_regular_site_fetch(self): + settings_details_url = reverse('settings_details', + kwargs= {'org': self.course_location.org, + 'name': self.course_location.name, + 'course': self.course_location.course + }) + + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': False}): + response = self.client.get(settings_details_url) + self.assertContains(response, "Course Summary Page") + self.assertNotContains(response, "your course summary page will not be viewable") + + self.assertContains(response, "Course Start Date") + self.assertContains(response, "Course End Date") + self.assertContains(response, "Enrollment Start Date") + self.assertContains(response, "Enrollment End Date") + self.assertNotContains(response, "not the dates shown on your course summary page") + + self.assertContains(response, "Introducing Your Course") + self.assertContains(response, "Requirements") + class CourseDetailsViewTest(CourseTestCase): def alter_field(self, url, details, field, val): diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 8762eb3a2a..62df50d5f4 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -230,7 +230,8 @@ def get_course_settings(request, org, course, name): kwargs={"org": org, "course": course, "name": name, - "section": "details"}) + "section": "details"}), + 'about_page_editable': not settings.MITX_FEATURES.get('ENABLE_MKTG_SITE', False) }) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 2adc0cd980..55dd2b67b2 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -1,3 +1,5 @@ +<%! from django.utils.translation import ugettext as _ %> + <%inherit file="base.html" /> <%block name="title">Schedule & Details Settings <%block name="bodyclass">is-signedin course schedule settings @@ -50,8 +52,8 @@ from contentstore import utils

- Settings - > Schedule & Details + ${_("Settings")} + > ${_("Schedule & Details")}

@@ -62,36 +64,40 @@ from contentstore import utils
-

Basic Information

- The nuts and bolts of your course +

${_("Basic Information")}

+ ${_("The nuts and bolts of your course")}
  1. - - + +
  2. - - + +
  3. - - + +
-

Course Summary Page (for student enrollment and access)

+

${_("Course Summary Page")} ${_("(for student enrollment and access)")}

https:${utils.get_lms_link_for_about_page(course_location)}

- + % if not about_page_editable: +
+

${_("Note: your course summary page will not be viewable until your course has been announced. To provide content for the page and preview it, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).")}

+
+ % endif
@@ -101,20 +107,26 @@ from contentstore import utils
-

Course Schedule

- Important steps and segments of your course +

${_('Course Schedule')}

+ ${_('Dates that control when your course can be viewed.')}
+ % if not about_page_editable: +
+

${_("Note: these dates impact when your courseware can be viewed, but they are not the dates shown on your course summary page. To provide the course start and registration dates as shown on your course summary page, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).")}

+
+ % endif +
  1. - + - First day the course begins + ${_("First day the course begins")}
    - +
    @@ -122,29 +134,29 @@ from contentstore import utils
  2. - + - Last day your course is active + ${_("Last day your course is active")}
    - +
- + % if about_page_editable:
  1. - + - First day students can enroll + ${_("First day students can enroll")}
    - +
    @@ -152,91 +164,91 @@ from contentstore import utils
  2. - + - Last day students can enroll + ${_("Last day students can enroll")}
    - +
+ % endif
-
+ % if about_page_editable: +
+
+

${_("Introducing Your Course")}

+ ${_("Information for prospective students")} +
-
-
-

Introducing Your Course

- Information for prospective students -
+
    +
  1. + + + ${_("Introductions, prerequisites, FAQs that are used on ")}${_("your course summary page")}${_(" (formatted in HTML)")} +
    2. -
      -
    1. - - - Introductions, prerequisites, FAQs that are used on your course summary page (formatted in HTML) -
      2. +
    2. + +
      +
      + +
      +
      + ${_("Delete Current Video")} +
      +
      -
    3. - -
      -
      - -
      -
      - Delete Current Video -
      -
      +
      + + ${_("Enter your YouTube video's ID (along with any restriction parameters)")} +
      +
      4. +
    +
-
- - Enter your YouTube video's ID (along with any restriction parameters) -
- - -
+
-
+
+
+

${_("Requirements")}

+ ${_("Expectations of the students taking this course")} +
-
-
-

Requirements

- Expectations of the students taking this course -
- -
    -
  1. - - - Time spent on all course work -
    2. -
-
+
    +
  1. + + + ${_("Time spent on all course work")} +
    2. +
+
+ % endif -
@@ -111,12 +116,6 @@ from contentstore import utils ${_('Dates that control when your course can be viewed.')} - % if not about_page_editable: -
-

${_("Note: these dates impact when your courseware can be viewed, but they are not the dates shown on your course summary page. To provide the course start and registration dates as shown on your course summary page, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).")}

-
- % endif -
  1. @@ -146,6 +145,7 @@ from contentstore import utils
+ % if about_page_editable:
  1. @@ -177,6 +177,15 @@ from contentstore import utils
% endif + + % if not about_page_editable: +
+

${_("Note: These Dates Are Not Used When Promoting Your Course")}

+
+

${_('These dates impact when your courseware can be viewed, but they are not the dates shown on your course summary page. To provide the course start and registration dates as shown on your course summary page, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).')}

+
+
+ % endif
% if about_page_editable: From d2f0d85085f5b4ca00c3b1076b8adfbd208c0853 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 19 Jun 2013 17:42:05 -0400 Subject: [PATCH 08/30] studio - adds in basic rules for notices UI --- cms/static/sass/elements/_system-help.scss | 39 ++++++++++++++++++++++ cms/static/sass/views/_settings.scss | 8 ++++- common/static/sass/_mixins.scss | 7 ++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/cms/static/sass/elements/_system-help.scss b/cms/static/sass/elements/_system-help.scss index 7fcb218282..017faab54d 100644 --- a/cms/static/sass/elements/_system-help.scss +++ b/cms/static/sass/elements/_system-help.scss @@ -1,2 +1,41 @@ // studio - elements - system help // ==================== + +// notices - in-context: to be used as notices to users within the context of a form/action +.notice-incontext { + @extend .ui-well; + + .title { + @extend .t-title7; + margin-bottom: ($baseline/4); + font-weight: 600; + + [class^="icon-"] { + @extend .t-icon5; + display: inline-block; + vertical-align: middle; + margin-right: ($baseline/4); + } + } + + .copy { + @extend .t-copy-sub2; + } + + strong { + font-weight: 600; + } +} + +// particular warnings around a workflow for something +.notice-workflow { + background: $yellow-l5; + + .copy { + color: $gray-d1; + } + + .icon-warning-sign { + color: $yellow-s3; + } +} diff --git a/cms/static/sass/views/_settings.scss b/cms/static/sass/views/_settings.scss index 735774511f..cbb1034626 100644 --- a/cms/static/sass/views/_settings.scss +++ b/cms/static/sass/views/_settings.scss @@ -21,7 +21,7 @@ body.course.settings { font-size: 14px; } - .message-status { + .message-status { display: none; @include border-top-radius(2px); @include box-sizing(border-box); @@ -52,6 +52,12 @@ body.course.settings { } } + // notices - used currently for edx mktg + .notice-workflow { + margin-top: ($baseline); + } + + // in form - elements .group-settings { margin: 0 0 ($baseline*2) 0; diff --git a/common/static/sass/_mixins.scss b/common/static/sass/_mixins.scss index c3a548bbf7..c26738a1b7 100644 --- a/common/static/sass/_mixins.scss +++ b/common/static/sass/_mixins.scss @@ -189,3 +189,10 @@ } } + +// UI archetypes - well +.ui-well { + @include box-shadow(inset 0 1px 2px 1px $shadow-l1); + padding: ($baseline*0.75); +} + From 7904464975f3d20ebb33a65600c0c589352434a7 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 19 Jun 2013 17:42:36 -0400 Subject: [PATCH 09/30] studio - revises notice headings to use icons --- cms/templates/settings.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 2ee4ad1c07..e0a99acff9 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -100,7 +100,7 @@ from contentstore import utils % if not about_page_editable:
-

${_("Note: About Your Course's Promotion")}

+

${_("Note: About Your Course's Promotion")}

${_('Your course summary page will not be viewable until your course has been announced. To provide content for the page and preview it, follow the instructions provided by PM or Conrad Warre (conrad@edx.org).')}

@@ -180,7 +180,7 @@ from contentstore import utils % if not about_page_editable:
-

${_("Note: These Dates Are Not Used When Promoting Your Course")}

+

${_("Note: These Dates Are Not Used When Promoting Your Course")}

${_('These dates impact when your courseware can be viewed, but they are not the dates shown on your course summary page. To provide the course start and registration dates as shown on your course summary page, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).')}

From 4ddca36bbecfd8df3f4f4d53ccb48d31ea6b4c9a Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 19 Jun 2013 18:13:54 -0400 Subject: [PATCH 10/30] Studio - adds minor UI/content changes to settings notices --- cms/static/sass/elements/_system-help.scss | 1 + cms/templates/settings.html | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cms/static/sass/elements/_system-help.scss b/cms/static/sass/elements/_system-help.scss index 017faab54d..5f4cec26d7 100644 --- a/cms/static/sass/elements/_system-help.scss +++ b/cms/static/sass/elements/_system-help.scss @@ -4,6 +4,7 @@ // notices - in-context: to be used as notices to users within the context of a form/action .notice-incontext { @extend .ui-well; + @include border-radius(($baseline/10)); .title { @extend .t-title7; diff --git a/cms/templates/settings.html b/cms/templates/settings.html index e0a99acff9..6f2ef7653d 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -100,9 +100,9 @@ from contentstore import utils % if not about_page_editable:
-

${_("Note: About Your Course's Promotion")}

+

${_("Promoting Your Course with edX")}

-

${_('Your course summary page will not be viewable until your course has been announced. To provide content for the page and preview it, follow the instructions provided by PM or Conrad Warre (conrad@edx.org).')}

+

${_('Your course summary page will not be viewable until your course has been announced. To provide content for the page and preview it, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).')}

% endif @@ -180,7 +180,7 @@ from contentstore import utils % if not about_page_editable:
-

${_("Note: These Dates Are Not Used When Promoting Your Course")}

+

${_("These Dates Are Not Used When Promoting Your Course")}

${_('These dates impact when your courseware can be viewed, but they are not the dates shown on your course summary page. To provide the course start and registration dates as shown on your course summary page, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).')}

From e76ef3aa31aaaa1f02a4f920084ce040777ec532 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 18 Jun 2013 08:06:42 -0400 Subject: [PATCH 11/30] Combined video and videoalpha acceptance tests. Resolved conflict between two steps with the same name. --- .../courseware/features/video.feature | 10 ++++-- lms/djangoapps/courseware/features/video.py | 22 ++++++++++++ .../courseware/features/videoalpha.py | 36 ------------------- 3 files changed, 29 insertions(+), 39 deletions(-) delete mode 100644 lms/djangoapps/courseware/features/videoalpha.py diff --git a/lms/djangoapps/courseware/features/video.feature b/lms/djangoapps/courseware/features/video.feature index c4d96f93f7..2b8d0f013a 100644 --- a/lms/djangoapps/courseware/features/video.feature +++ b/lms/djangoapps/courseware/features/video.feature @@ -1,6 +1,10 @@ Feature: Video component As a student, I want to view course videos in LMS. - Scenario: Autoplay is enabled in LMS - Given the course has a Video component - Then when I view the video it has autoplay enabled + Scenario: Autoplay is enabled in LMS for a Video component + Given the course has a Video component + Then when I view the video it has autoplay enabled + + Scenario: Autoplay is enabled in the LMS for a VideoAlpha component + Given the course has a VideoAlpha component + Then when I view the video it has autoplay enabled diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 8cef5564f3..745f0ae99a 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -27,8 +27,30 @@ def view_video(_step): world.browser.visit(url) +@step('the course has a VideoAlpha component') +def view_videoalpha(step): + coursename = TEST_COURSE_NAME.replace(' ', '_') + i_am_registered_for_the_course(step, coursename) + + # Make sure we have a videoalpha + add_videoalpha_to_course(coursename) + chapter_name = TEST_SECTION_NAME.replace(" ", "_") + section_name = chapter_name + url = django_url('/courses/edx/Test_Course/Test_Course/courseware/%s/%s' % + (chapter_name, section_name)) + + world.browser.visit(url) + + def add_video_to_course(course): template_name = 'i4x://edx/templates/video/default' world.ItemFactory.create(parent_location=section_location(course), template=template_name, display_name='Video') + + +def add_videoalpha_to_course(course): + template_name = 'i4x://edx/templates/videoalpha/default' + world.ItemFactory.create(parent_location=section_location(course), + template=template_name, + display_name='Video Alpha 1') diff --git a/lms/djangoapps/courseware/features/videoalpha.py b/lms/djangoapps/courseware/features/videoalpha.py deleted file mode 100644 index cabf8c681f..0000000000 --- a/lms/djangoapps/courseware/features/videoalpha.py +++ /dev/null @@ -1,36 +0,0 @@ -#pylint: disable=C0111 -#pylint: disable=W0613 -#pylint: disable=W0621 - -from lettuce import world, step -from lettuce.django import django_url -from common import TEST_COURSE_NAME, TEST_SECTION_NAME, i_am_registered_for_the_course, section_location - -############### ACTIONS #################### - - -@step('when I view the video it has autoplay enabled') -def does_autoplay(step): - assert(world.css_find('.videoalpha')[0]['data-autoplay'] == 'True') - - -@step('the course has a Video component') -def view_videoalpha(step): - coursename = TEST_COURSE_NAME.replace(' ', '_') - i_am_registered_for_the_course(step, coursename) - - # Make sure we have a videoalpha - add_videoalpha_to_course(coursename) - chapter_name = TEST_SECTION_NAME.replace(" ", "_") - section_name = chapter_name - url = django_url('/courses/edx/Test_Course/Test_Course/courseware/%s/%s' % - (chapter_name, section_name)) - - world.browser.visit(url) - - -def add_videoalpha_to_course(course): - template_name = 'i4x://edx/templates/videoalpha/default' - world.ItemFactory.create(parent_location=section_location(course), - template=template_name, - display_name='Video Alpha 1') From 3b37e0c19f13ab5dfacbdd197e6a50f7e1dc4bb0 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 18 Jun 2013 08:35:56 -0400 Subject: [PATCH 12/30] Fixed incorrect videoalpha template name --- lms/djangoapps/courseware/features/video.py | 2 +- lms/djangoapps/courseware/features/videoalpha.feature | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) delete mode 100644 lms/djangoapps/courseware/features/videoalpha.feature diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 745f0ae99a..90f68c1daf 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -50,7 +50,7 @@ def add_video_to_course(course): def add_videoalpha_to_course(course): - template_name = 'i4x://edx/templates/videoalpha/default' + template_name = 'i4x://edx/templates/videoalpha/Video_Alpha_1' world.ItemFactory.create(parent_location=section_location(course), template=template_name, display_name='Video Alpha 1') diff --git a/lms/djangoapps/courseware/features/videoalpha.feature b/lms/djangoapps/courseware/features/videoalpha.feature deleted file mode 100644 index 2a0acb0f9b..0000000000 --- a/lms/djangoapps/courseware/features/videoalpha.feature +++ /dev/null @@ -1,6 +0,0 @@ -Feature: Video Alpha component - As a student, I want to view course videos in LMS. - - Scenario: Autoplay is enabled in LMS - Given the course has a Video component - Then when I view the video it has autoplay enabled From 6ab5bb2f205953070c9c9099352761acf5239419 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 18 Jun 2013 08:47:00 -0400 Subject: [PATCH 13/30] Changed videoalpha stub to exclude video sources, but include the rest of the player. --- lms/templates/videoalpha.html | 56 +++++++++++++++++------------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/lms/templates/videoalpha.html b/lms/templates/videoalpha.html index 07c7dbee27..4e136bd170 100644 --- a/lms/templates/videoalpha.html +++ b/lms/templates/videoalpha.html @@ -2,34 +2,34 @@

${display_name}

% endif -%if settings.MITX_FEATURES['STUB_VIDEO_FOR_TESTING']: -
-%else: -
-
-
-
-
-
-
-
-
-
-%endif +
+
+
+
+
+
+
+
+
+
% if sources.get('main'):
From b1c963ab5e35c7999ee43e949876000010421d39 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 18 Jun 2013 09:02:21 -0400 Subject: [PATCH 14/30] Changed stubbing behavior in video templates to exclude only the sources (not the player) --- lms/templates/video.html | 48 ++++++++++++++++------------------- lms/templates/videoalpha.html | 4 +++ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/lms/templates/video.html b/lms/templates/video.html index 267372176a..91b5f63b81 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -2,37 +2,33 @@

${display_name}

% endif -%if settings.MITX_FEATURES['STUB_VIDEO_FOR_TESTING']: -
-
-
-
-
-
    -
    • -
  • -
    0:00 / 0:00
    -
    • -
-
- Fill Browser -
-
-
-
-
-%elif settings.MITX_FEATURES.get('USE_YOUTUBE_OBJECT_API') and normal_speed_video_id: +%if settings.MITX_FEATURES.get('USE_YOUTUBE_OBJECT_API') and normal_speed_video_id: + % if not settings.MITX_FEATURES['STUB_VIDEO_FOR_TESTING']: + value="https://www.youtube.com/v/${normal_speed_video_id}?version=3&autoplay=1&rel=0"> + % endif + - + %else: -
+
diff --git a/lms/templates/videoalpha.html b/lms/templates/videoalpha.html index 4e136bd170..2bb5d817a8 100644 --- a/lms/templates/videoalpha.html +++ b/lms/templates/videoalpha.html @@ -5,7 +5,11 @@
Date: Thu, 20 Jun 2013 07:44:33 -0400 Subject: [PATCH 15/30] Updated video alpha template name to reflect recent changes in master. --- lms/djangoapps/courseware/features/video.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 90f68c1daf..cd1bdcf60f 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -50,7 +50,7 @@ def add_video_to_course(course): def add_videoalpha_to_course(course): - template_name = 'i4x://edx/templates/videoalpha/Video_Alpha_1' + template_name = 'i4x://edx/templates/videoalpha/Video_Alpha' world.ItemFactory.create(parent_location=section_location(course), template=template_name, - display_name='Video Alpha 1') + display_name='Video Alpha') From 83af3e594f561c4f1c3c35b59f34089b2843a887 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 17 Jun 2013 13:49:40 -0400 Subject: [PATCH 16/30] Use built-in rakelibdir feature Rake by default imports all .rake files in the rakelib dir, so we can use that rather than doing our own import loop. --- rakefile | 6 +----- {rakefiles => rakelib}/assets.rake | 0 {rakefiles => rakelib}/deploy.rake | 0 {rakefiles => rakelib}/deprecated.rake | 0 {rakefiles => rakelib}/django.rake | 0 {rakefiles => rakelib}/docs.rake | 0 {rakefiles => rakelib}/helpers.rb | 0 {rakefiles => rakelib}/i18n.rake | 0 {rakefiles => rakelib}/jasmine.rake | 0 {rakefiles => rakelib}/prereqs.rake | 2 -- {rakefiles => rakelib}/quality.rake | 0 {rakefiles => rakelib}/tests.rake | 0 {rakefiles => rakelib}/workspace.rake | 0 13 files changed, 1 insertion(+), 7 deletions(-) rename {rakefiles => rakelib}/assets.rake (100%) rename {rakefiles => rakelib}/deploy.rake (100%) rename {rakefiles => rakelib}/deprecated.rake (100%) rename {rakefiles => rakelib}/django.rake (100%) rename {rakefiles => rakelib}/docs.rake (100%) rename {rakefiles => rakelib}/helpers.rb (100%) rename {rakefiles => rakelib}/i18n.rake (100%) rename {rakefiles => rakelib}/jasmine.rake (100%) rename {rakefiles => rakelib}/prereqs.rake (98%) rename {rakefiles => rakelib}/quality.rake (100%) rename {rakefiles => rakelib}/tests.rake (100%) rename {rakefiles => rakelib}/workspace.rake (100%) diff --git a/rakefile b/rakefile index 20101a14db..3fcd16f995 100644 --- a/rakefile +++ b/rakefile @@ -1,10 +1,6 @@ require 'json' require 'rake/clean' -require './rakefiles/helpers.rb' - -Dir['rakefiles/*.rake'].each do |rakefile| - import rakefile -end +require './rakelib/helpers.rb' # Build Constants REPO_ROOT = File.dirname(__FILE__) diff --git a/rakefiles/assets.rake b/rakelib/assets.rake similarity index 100% rename from rakefiles/assets.rake rename to rakelib/assets.rake diff --git a/rakefiles/deploy.rake b/rakelib/deploy.rake similarity index 100% rename from rakefiles/deploy.rake rename to rakelib/deploy.rake diff --git a/rakefiles/deprecated.rake b/rakelib/deprecated.rake similarity index 100% rename from rakefiles/deprecated.rake rename to rakelib/deprecated.rake diff --git a/rakefiles/django.rake b/rakelib/django.rake similarity index 100% rename from rakefiles/django.rake rename to rakelib/django.rake diff --git a/rakefiles/docs.rake b/rakelib/docs.rake similarity index 100% rename from rakefiles/docs.rake rename to rakelib/docs.rake diff --git a/rakefiles/helpers.rb b/rakelib/helpers.rb similarity index 100% rename from rakefiles/helpers.rb rename to rakelib/helpers.rb diff --git a/rakefiles/i18n.rake b/rakelib/i18n.rake similarity index 100% rename from rakefiles/i18n.rake rename to rakelib/i18n.rake diff --git a/rakefiles/jasmine.rake b/rakelib/jasmine.rake similarity index 100% rename from rakefiles/jasmine.rake rename to rakelib/jasmine.rake diff --git a/rakefiles/prereqs.rake b/rakelib/prereqs.rake similarity index 98% rename from rakefiles/prereqs.rake rename to rakelib/prereqs.rake index ff8b4b8784..e06d411435 100644 --- a/rakefiles/prereqs.rake +++ b/rakelib/prereqs.rake @@ -1,5 +1,3 @@ -require './rakefiles/helpers.rb' - PREREQS_MD5_DIR = ENV["PREREQ_CACHE_DIR"] || File.join(REPO_ROOT, '.prereqs_cache') CLOBBER.include(PREREQS_MD5_DIR) diff --git a/rakefiles/quality.rake b/rakelib/quality.rake similarity index 100% rename from rakefiles/quality.rake rename to rakelib/quality.rake diff --git a/rakefiles/tests.rake b/rakelib/tests.rake similarity index 100% rename from rakefiles/tests.rake rename to rakelib/tests.rake diff --git a/rakefiles/workspace.rake b/rakelib/workspace.rake similarity index 100% rename from rakefiles/workspace.rake rename to rakelib/workspace.rake From d9268acd6bb9b19175315998de623818e0a799f1 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 17 Jun 2013 13:52:00 -0400 Subject: [PATCH 17/30] Provide instructions of ruby imports fail in rake `rake install_prereqs` requires a minimal level of ruby and rake already installed. If it doesn't exist, then print out a helpful message indicating next steps. --- CHANGELOG.rst | 2 ++ rakefile | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7fc07a3f19..9c7af1520b 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: Make rake provide better error messages if packages are missing. + Common: Repairs development documentation generation by sphinx. LMS: Problem rescoring. Added options on the Grades tab of the diff --git a/rakefile b/rakefile index 3fcd16f995..96bd4c2e96 100644 --- a/rakefile +++ b/rakefile @@ -1,6 +1,12 @@ -require 'json' -require 'rake/clean' -require './rakelib/helpers.rb' +begin + require 'json' + require 'rake/clean' + require './rakelib/helpers.rb' +rescue LoadError => error + puts "Import faild (#{error})" + puts "Please run `bundle install` to bootstrap ruby dependencies" + exit 1 +end # Build Constants REPO_ROOT = File.dirname(__FILE__) From d99ad53ae90bebc0fb2c099b42a2ffa1c64ab4b8 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 19 Jun 2013 15:30:47 -0400 Subject: [PATCH 18/30] Add system and env arguments to asset tasks The preprocess task requires system and env arguments in order to correctly load up the django environment to preprocess sass files to inject themes. In order for that task to recieve the arguments, all tasks that depend on it also have to accept that same set of arguments. This will all go away once the next evolution of themes arrives, which will remove the preprocessing needed to inject theme names. --- rakelib/assets.rake | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/rakelib/assets.rake b/rakelib/assets.rake index 764d049a68..0c58047bc2 100644 --- a/rakelib/assets.rake +++ b/rakelib/assets.rake @@ -55,8 +55,9 @@ def sass_cmd(watch=false, debug=false) "#{watch ? '--watch' : '--update'} -E utf-8 #{sass_watch_paths.join(' ')}" end +# This task takes arguments purely to pass them via dependencies to the preprocess task desc "Compile all assets" -multitask :assets => 'assets:all' +task :assets, [:system, :env] => 'assets:all' namespace :assets do @@ -80,8 +81,9 @@ namespace :assets do {:xmodule => [:install_python_prereqs], :coffee => [:install_node_prereqs, :'assets:coffee:clobber'], :sass => [:install_ruby_prereqs, :preprocess]}.each_pair do |asset_type, prereq_tasks| + # This task takes arguments purely to pass them via dependencies to the preprocess task desc "Compile all #{asset_type} assets" - task asset_type => prereq_tasks do + task asset_type, [:system, :env] => prereq_tasks do |t, args| cmd = send(asset_type.to_s + "_cmd", watch=false, debug=false) if cmd.kind_of?(Array) cmd.each {|c| sh(c)} @@ -90,7 +92,8 @@ namespace :assets do end end - multitask :all => asset_type + # This task takes arguments purely to pass them via dependencies to the preprocess task + multitask :all, [:system, :env] => asset_type multitask :debug => "assets:#{asset_type}:debug" multitask :_watch => "assets:#{asset_type}:_watch" From 2e480f6404b2ae640623266a4cbd9f91dfa067c6 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 17 Jun 2013 15:00:55 -0400 Subject: [PATCH 19/30] Switch to standard coffee watcher Using `ulimit -n` to set the limit much higher than the default of 256 in Darwin seems to avoid the `EMFILE` error that was plaguing our Mac developers. --- CHANGELOG.rst | 2 ++ doc/development.md | 19 +++++++++++++++++++ rakelib/assets.rake | 26 +++++++++----------------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9c7af1520b..99accb83cb 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: Use coffee directly when watching for coffeescript file changes. + Common: Make rake provide better error messages if packages are missing. Common: Repairs development documentation generation by sphinx. diff --git a/doc/development.md b/doc/development.md index c99e99f906..e6ab650002 100644 --- a/doc/development.md +++ b/doc/development.md @@ -63,6 +63,25 @@ To get a full list of available rake tasks, use: rake -T +### Troubleshooting + +#### Reference Error: XModule is not defined (javascript) +This means that the javascript defining an xmodule hasn't loaded correctly. There are a number +of different things that could be causing this: + +1. See `Error: watch EMFILE` + +#### Error: watch EMFILE (coffee) +When running a development server, we also start a watcher process alongside to recompile coffeescript +and sass as changes are made. On Mac OSX systems, the coffee watcher process takes more file handles +than are allowed by default. This will result in `EMFILE` errors when coffeescript is running, and +will prevent javascript from compiling, leading to the error 'XModule is not defined' + +To work around this issue, we use `Process::setrlimit` to set the number of allowed open files. +Coffee watches both directories and files, so you will need to set this fairly high (anecdotally, +8000 seems to do the trick on OSX 10.7.5, 10.8.3, and 10.8.4) + + ## Running Tests See `testing.md` for instructions on running the test suite. diff --git a/rakelib/assets.rake b/rakelib/assets.rake index 0c58047bc2..10dfb14f18 100644 --- a/rakelib/assets.rake +++ b/rakelib/assets.rake @@ -6,6 +6,8 @@ if USE_CUSTOM_THEME THEME_SASS = File.join(THEME_ROOT, "static", "sass") end +MINIMAL_DARWIN_NOFILE_LIMIT = 8000 + def xmodule_cmd(watch=false, debug=false) xmodule_cmd = 'xmodule_assets common/static/xmodule' if watch @@ -21,24 +23,14 @@ def xmodule_cmd(watch=false, debug=false) end def coffee_cmd(watch=false, debug=false) - if watch - # On OSx, coffee fails with EMFILE when - # trying to watch all of our coffee files at the same - # time. - # - # Ref: https://github.com/joyent/node/issues/2479 - # - # So, instead, we use watchmedo, which works around the problem - "watchmedo shell-command " + - "--command 'node_modules/.bin/coffee -c ${watch_src_path}' " + - "--recursive " + - "--patterns '*.coffee' " + - "--ignore-directories " + - "--wait " + - "." - else - 'node_modules/.bin/coffee --compile .' + if watch && Launchy::Application.new.host_os_family.darwin? + available_files = Process::getrlimit(:NOFILE)[0] + if available_files < MINIMAL_DARWIN_NOFILE_LIMIT + Process.setrlimit(:NOFILE, MINIMAL_DARWIN_NOFILE_LIMIT) + + end end + "node_modules/.bin/coffee --compile #{watch ? '--watch' : ''} ." end def sass_cmd(watch=false, debug=false) From f4200127bcc91fc30eca2f727a21c6a71139fbfd Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 14 Jun 2013 21:46:27 -0400 Subject: [PATCH 20/30] Make coffee watch message more informative When running under watchmedo, coffee doesn't display any useful information when it recompiles a changed file, so we make watchmedo echo that information instead. --- rakelib/assets.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rakelib/assets.rake b/rakelib/assets.rake index 0c58047bc2..c80b275c27 100644 --- a/rakelib/assets.rake +++ b/rakelib/assets.rake @@ -30,7 +30,7 @@ def coffee_cmd(watch=false, debug=false) # # So, instead, we use watchmedo, which works around the problem "watchmedo shell-command " + - "--command 'node_modules/.bin/coffee -c ${watch_src_path}' " + + "--command 'echo \">>> Change detected to ${watch_src_path}\" && node_modules/.bin/coffee -c ${watch_src_path}' " + "--recursive " + "--patterns '*.coffee' " + "--ignore-directories " + From eb3e94660bf166b75a34601549b7093507c46bf7 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 14 Jun 2013 21:47:22 -0400 Subject: [PATCH 21/30] Don't delete generated files from xmodule-assets xmodule-assets creates coffeescript files in the output directories. On its next run, it used to delete the javascript files compiled from those coffee files. Now it doesn't which should make coffee have to do less work. Fixes LMS-451 --- CHANGELOG.rst | 4 ++++ common/lib/xmodule/xmodule/static_content.py | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9c7af1520b..fbc007949c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,10 @@ 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. +XModule: Don't delete generated xmodule asset files when compiling (for +instance, when XModule provides a coffeescript file, don't delete +the associated javascript) + Common: Make rake provide better error messages if packages are missing. Common: Repairs development documentation generation by sphinx. diff --git a/common/lib/xmodule/xmodule/static_content.py b/common/lib/xmodule/xmodule/static_content.py index 4c4827e0aa..42fef65b11 100755 --- a/common/lib/xmodule/xmodule/static_content.py +++ b/common/lib/xmodule/xmodule/static_content.py @@ -121,15 +121,23 @@ def _write_js(output_root, classes): type=filetype) contents[filename] = fragment - _write_files(output_root, contents) + _write_files(output_root, contents, {'.coffee': '.js'}) return [output_root / filename for filename in contents.keys()] -def _write_files(output_root, contents): +def _write_files(output_root, contents, generated_suffix_map=None): _ensure_dir(output_root) - for extra_file in set(output_root.files()) - set(contents.keys()): - extra_file.remove_p() + to_delete = set(file.basename() for file in output_root.files()) - set(contents.keys()) + + if generated_suffix_map: + for output_file in contents.keys(): + for suffix, generated_suffix in generated_suffix_map.items(): + if output_file.endswith(suffix): + to_delete.discard(output_file.replace(suffix, generated_suffix)) + + for extra_file in to_delete: + (output_root / extra_file).remove_p() for filename, file_content in contents.iteritems(): (output_root / filename).write_bytes(file_content) From cd57e281f5eab2d7616e47d4d9b95adf46efda1e Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 14 Jun 2013 22:27:24 -0400 Subject: [PATCH 22/30] Only write to xmodule asset files that have changed Use md5 checksumming to verify that we only write out xmodule asset files whose contents differ from what we are about to write. This minimizes thrashing of the other watchers. Fixes LMS-452 --- CHANGELOG.rst | 2 ++ common/lib/xmodule/xmodule/static_content.py | 12 +++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fbc007949c..d74816990b 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. +XModule: Only write out assets files if the contents have changed. + XModule: Don't delete generated xmodule asset files when compiling (for instance, when XModule provides a coffeescript file, don't delete the associated javascript) diff --git a/common/lib/xmodule/xmodule/static_content.py b/common/lib/xmodule/xmodule/static_content.py index 42fef65b11..7662499c16 100755 --- a/common/lib/xmodule/xmodule/static_content.py +++ b/common/lib/xmodule/xmodule/static_content.py @@ -4,6 +4,7 @@ This module has utility functions for gathering up the static content that is defined by XModules and XModuleDescriptors (javascript and css) """ +import logging import hashlib import os import errno @@ -15,6 +16,9 @@ from path import path from xmodule.x_module import XModuleDescriptor +LOG = logging.getLogger(__name__) + + def write_module_styles(output_root): return _write_styles('.xmodule_display', output_root, _list_modules()) @@ -140,7 +144,13 @@ def _write_files(output_root, contents, generated_suffix_map=None): (output_root / extra_file).remove_p() for filename, file_content in contents.iteritems(): - (output_root / filename).write_bytes(file_content) + output_file = output_root / filename + + if not output_file.isfile() or output_file.read_md5() != hashlib.md5(file_content).digest(): + LOG.debug("Writing %s", output_file) + output_file.write_bytes(file_content) + else: + LOG.debug("%s unchanged, skipping", output_file) def main(): From 64912a774199e40a68cab3020d3e98ad9bb8c6e5 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 14 Jun 2013 21:17:08 -0400 Subject: [PATCH 23/30] Make assets watchers run as singletons Previously, multiple copies of the watchers started from the different shells would run simultaneously, which left the possiblity of zombie watchers, increased resource consumption, and incorrect results. This fixes that problem by only starting a watcher if that same command isn't already in the process list. Fixes LMS-499 --- CHANGELOG.rst | 3 +++ Gemfile | 1 + rakelib/assets.rake | 4 ++-- rakelib/helpers.rb | 12 ++++++++++++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9c7af1520b..18855e82ae 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. +Common: Make asset watchers run as singletons (so they won't start if the +watcher is already running in another shell). + Common: Make rake provide better error messages if packages are missing. Common: Repairs development documentation generation by sphinx. diff --git a/Gemfile b/Gemfile index 7f7b146978..1ad685c34d 100644 --- a/Gemfile +++ b/Gemfile @@ -4,3 +4,4 @@ gem 'sass', '3.1.15' gem 'bourbon', '~> 1.3.6' gem 'colorize', '~> 0.5.8' gem 'launchy', '~> 2.1.2' +gem 'sys-proctable', '~> 0.9.3' diff --git a/rakelib/assets.rake b/rakelib/assets.rake index 0c58047bc2..c935b0d53b 100644 --- a/rakelib/assets.rake +++ b/rakelib/assets.rake @@ -114,9 +114,9 @@ namespace :assets do task :_watch => (prereq_tasks + ["assets:#{asset_type}:debug"]) do cmd = send(asset_type.to_s + "_cmd", watch=true, debug=true) if cmd.kind_of?(Array) - cmd.each {|c| background_process(c)} + cmd.each {|c| singleton_process(c)} else - background_process(cmd) + singleton_process(cmd) end end end diff --git a/rakelib/helpers.rb b/rakelib/helpers.rb index 4b10bef709..3373214a19 100644 --- a/rakelib/helpers.rb +++ b/rakelib/helpers.rb @@ -1,4 +1,6 @@ require 'digest/md5' +require 'sys/proctable' +require 'colorize' def find_executable(exec) path = %x(which #{exec}).strip @@ -84,6 +86,16 @@ def background_process(*command) end end +# Runs a command as a background process, as long as no other processes +# tagged with the same tag are running +def singleton_process(*command) + if Sys::ProcTable.ps.select {|proc| proc.cmdline.include?(command.join(' '))}.empty? + background_process(*command) + else + puts "Process '#{command.join(' ')} already running, skipping".blue + end +end + def environments(system) Dir["#{system}/envs/**/*.py"].select{|file| ! (/__init__.py$/ =~ file)}.map do |env_file| env_file.gsub("#{system}/envs/", '').gsub(/\.py/, '').gsub('/', '.') From 419c5e7a5ce513d64291759658d6d8930e5fe2c5 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Thu, 20 Jun 2013 09:23:16 -0400 Subject: [PATCH 24/30] studio - removes course settings promo link and notice icons --- cms/templates/settings.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 6f2ef7653d..72df12cdd5 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -85,6 +85,7 @@ from contentstore import utils + % if about_page_editable:

${_("Course Summary Page")} ${_("(for student enrollment and access)")}

@@ -97,10 +98,11 @@ from contentstore import utils
+ % endif % if not about_page_editable:
-

${_("Promoting Your Course with edX")}

+

${_("Promoting Your Course with edX")}

${_('Your course summary page will not be viewable until your course has been announced. To provide content for the page and preview it, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).')}

@@ -180,7 +182,7 @@ from contentstore import utils % if not about_page_editable:
-

${_("These Dates Are Not Used When Promoting Your Course")}

+

${_("These Dates Are Not Used When Promoting Your Course")}

${_('These dates impact when your courseware can be viewed, but they are not the dates shown on your course summary page. To provide the course start and registration dates as shown on your course summary page, follow the instructions provided by your PM or Conrad Warre (conrad@edx.org).')}

From 604e820211755d80a9aac6759c75930ec58c9c4a Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Thu, 20 Jun 2013 09:23:56 -0400 Subject: [PATCH 25/30] studio - revises styling of in-context notices UI --- cms/static/sass/elements/_system-help.scss | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/cms/static/sass/elements/_system-help.scss b/cms/static/sass/elements/_system-help.scss index 5f4cec26d7..a9a3e16128 100644 --- a/cms/static/sass/elements/_system-help.scss +++ b/cms/static/sass/elements/_system-help.scss @@ -10,22 +10,24 @@ @extend .t-title7; margin-bottom: ($baseline/4); font-weight: 600; - - [class^="icon-"] { - @extend .t-icon5; - display: inline-block; - vertical-align: middle; - margin-right: ($baseline/4); - } } .copy { - @extend .t-copy-sub2; + @extend .t-copy-sub1; + @include transition(opacity 0.25s ease-in-out 0); + opacity: 0.75; } strong { font-weight: 600; } + + &:hover { + + .copy { + opacity: 1.0; + } + } } // particular warnings around a workflow for something @@ -35,8 +37,4 @@ .copy { color: $gray-d1; } - - .icon-warning-sign { - color: $yellow-s3; - } } From c00721bbe6dd341590bea992e93803b98291c3e1 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Tue, 18 Jun 2013 13:10:47 -0400 Subject: [PATCH 26/30] Fixed the preferences scope of xblock. Added self to authors. Conflicts: AUTHORS CHANGELOG.rst --- AUTHORS | 1 + CHANGELOG.rst | 2 ++ lms/djangoapps/courseware/model_data.py | 2 +- lms/djangoapps/courseware/tests/factories.py | 2 +- lms/djangoapps/courseware/tests/test_model_data.py | 7 ++++++- 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index 1af7349491..9bb4ede121 100644 --- a/AUTHORS +++ b/AUTHORS @@ -77,3 +77,4 @@ Slater Victoroff Peter Fogg Bethany LaPenta Renzo Lucioni +Felix Sun diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9c405ed365..206be44c87 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -27,6 +27,8 @@ students' number of attempts to zero. Provides a list of background tasks that are currently running for the course, and an option to see a history of background tasks for a given problem. +LMS: Fixed the preferences scope for storing data in xmodules. + LMS: Forums. Added handling for case where discussion module can get `None` as value of lms.start in `lms/djangoapps/django_comment_client/utils.py` diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index f363546af0..790f1fd721 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -163,7 +163,7 @@ class ModelDataCache(object): return self._chunked_query( XModuleStudentPrefsField, 'module_type__in', - set(descriptor.location.category for descriptor in self.descriptors), + set(descriptor.module_class.__name__ for descriptor in self.descriptors), student=self.user.pk, field_name__in=set(field.name for field in fields), ) diff --git a/lms/djangoapps/courseware/tests/factories.py b/lms/djangoapps/courseware/tests/factories.py index 26df68ca7e..69f8f54eec 100644 --- a/lms/djangoapps/courseware/tests/factories.py +++ b/lms/djangoapps/courseware/tests/factories.py @@ -75,7 +75,7 @@ class StudentPrefsFactory(DjangoModelFactory): field_name = 'existing_field' value = json.dumps('old_value') student = SubFactory(UserFactory) - module_type = 'problem' + module_type = 'MockProblemModule' class StudentInfoFactory(DjangoModelFactory): diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index 9f225f73bd..e961f80939 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -29,6 +29,7 @@ def mock_descriptor(fields=[], lms_fields=[]): descriptor.location = location('def_id') descriptor.module_class.fields = fields descriptor.module_class.lms.fields = lms_fields + descriptor.module_class.__name__ = 'MockProblemModule' return descriptor location = partial(Location, 'i4x', 'edX', 'test_course', 'problem') @@ -37,7 +38,7 @@ course_id = 'edX/test_course/test' content_key = partial(LmsKeyValueStore.Key, Scope.content, None, location('def_id')) settings_key = partial(LmsKeyValueStore.Key, Scope.settings, None, location('def_id')) user_state_key = partial(LmsKeyValueStore.Key, Scope.user_state, 'user', location('def_id')) -prefs_key = partial(LmsKeyValueStore.Key, Scope.preferences, 'user', 'problem') +prefs_key = partial(LmsKeyValueStore.Key, Scope.preferences, 'user', 'MockProblemModule') user_info_key = partial(LmsKeyValueStore.Key, Scope.user_info, 'user', None) @@ -190,6 +191,10 @@ class StorageTestBase(object): self.mdc = ModelDataCache([mock_descriptor([mock_field(self.scope, 'existing_field')])], course_id, self.user) self.kvs = LmsKeyValueStore(self.desc_md, self.mdc) + def test_set_and_get_existing_field(self): + self.kvs.set(self.key_factory('existing_field'), 'test_value') + self.assertEquals('test_value', self.kvs.get(self.key_factory('existing_field'))) + def test_get_existing_field(self): "Test that getting an existing field in an existing Storage Field works" self.assertEquals('old_value', self.kvs.get(self.key_factory('existing_field'))) From 9e69586bb360db757c1d3c43bbe862cb65a6b670 Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 20 Jun 2013 10:31:44 -0400 Subject: [PATCH 27/30] pep8 fixes. --- .../features/advanced-settings.py | 7 +++-- common/djangoapps/terrain/ui_helpers.py | 26 +++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index 3113603467..473fc20a68 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -31,11 +31,10 @@ def press_the_notification_button(step, name): # Save was clicked if either the save notification bar is gone, or we have a error notification # overlaying it (expected in the case of typing Object into display_name). - save_clicked = lambda : world.is_css_not_present('.is-shown.wrapper-notification-warning') or \ - world.is_css_present('.is-shown.wrapper-notification-error') + save_clicked = lambda: world.is_css_not_present('.is-shown.wrapper-notification-warning') or\ + world.is_css_present('.is-shown.wrapper-notification-error') - assert_true(world.css_click(css, success_condition=save_clicked), - 'The save button was not clicked after 5 attempts.') + assert_true(world.css_click(css, success_condition=save_clicked), 'Save button not clicked after 5 attempts.') @step(u'I edit the value of a policy key$') diff --git a/common/djangoapps/terrain/ui_helpers.py b/common/djangoapps/terrain/ui_helpers.py index 8e4330d940..77667f2d63 100644 --- a/common/djangoapps/terrain/ui_helpers.py +++ b/common/djangoapps/terrain/ui_helpers.py @@ -49,7 +49,7 @@ def css_has_text(css_selector, text): @world.absorb def css_find(css, wait_time=5): - def is_visible(driver): + def is_visible(_driver): return EC.visibility_of_element_located((By.CSS_SELECTOR, css,)) world.browser.is_element_present_by_css(css, wait_time=wait_time) @@ -58,7 +58,7 @@ def css_find(css, wait_time=5): @world.absorb -def css_click(css_selector, index=0, attempts=5, success_condition=lambda:True): +def css_click(css_selector, index=0, attempts=5, success_condition=lambda: True): """ Perform a click on a CSS selector, retrying if it initially fails. @@ -90,15 +90,15 @@ def css_click(css_selector, index=0, attempts=5, success_condition=lambda:True): @world.absorb -def css_click_at(css, x=10, y=10): +def css_click_at(css, x_cord=10, y_cord=10): ''' A method to click at x,y coordinates of the element rather than in the center of the element ''' - e = css_find(css).first - e.action_chains.move_to_element_with_offset(e._element, x, y) - e.action_chains.click() - e.action_chains.perform() + element = css_find(css).first + element.action_chains.move_to_element_with_offset(element._element, x_cord, y_cord) + element.action_chains.click() + element.action_chains.perform() @world.absorb @@ -143,7 +143,7 @@ def css_visible(css_selector): @world.absorb def dialogs_closed(): - def are_dialogs_closed(driver): + def are_dialogs_closed(_driver): ''' Return True when no modal dialogs are visible ''' @@ -154,12 +154,12 @@ def dialogs_closed(): @world.absorb def save_the_html(path='/tmp'): - u = world.browser.url + url = world.browser.url html = world.browser.html.encode('ascii', 'ignore') - filename = '%s.html' % quote_plus(u) - f = open('%s/%s' % (path, filename), 'w') - f.write(html) - f.close() + filename = '%s.html' % quote_plus(url) + file = open('%s/%s' % (path, filename), 'w') + file.write(html) + file.close() @world.absorb From 485b5b2d6dada79a31a0f7b5276d5140f3faab69 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Thu, 20 Jun 2013 10:50:15 -0400 Subject: [PATCH 28/30] studio - unconditionalizes course summary info in settings --- cms/templates/settings.html | 2 -- 1 file changed, 2 deletions(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 72df12cdd5..14c79e586a 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -85,7 +85,6 @@ from contentstore import utils - % if about_page_editable:

${_("Course Summary Page")} ${_("(for student enrollment and access)")}

@@ -98,7 +97,6 @@ from contentstore import utils
- % endif % if not about_page_editable:
From a498fa52bae85ed9ae6d02714efbaaeadea9593a Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 20 Jun 2013 11:29:31 -0400 Subject: [PATCH 29/30] Modify after UX text changes. --- cms/djangoapps/contentstore/tests/test_course_settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index d038b9f1e2..6b8622f992 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -138,7 +138,7 @@ class CourseDetailsTestCase(CourseTestCase): with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): response = self.client.get(settings_details_url) self.assertContains(response, "Course Summary Page") - self.assertContains(response, "your course summary page will not be viewable") + self.assertContains(response, "course summary page will not be viewable") self.assertContains(response, "Course Start Date") self.assertContains(response, "Course End Date") @@ -157,7 +157,7 @@ class CourseDetailsTestCase(CourseTestCase): with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': False}): response = self.client.get(settings_details_url) self.assertContains(response, "Course Summary Page") - self.assertNotContains(response, "your course summary page will not be viewable") + self.assertNotContains(response, "course summary page will not be viewable") self.assertContains(response, "Course Start Date") self.assertContains(response, "Course End Date") From 9a753f1cc529e3a1fe0b9a14c48342958daca8ca Mon Sep 17 00:00:00 2001 From: cahrens Date: Thu, 20 Jun 2013 12:03:59 -0400 Subject: [PATCH 30/30] Studio feature to disable course settings on Drupal site. --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9c405ed365..3bc3b6b9bf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,9 @@ XModule: Don't delete generated xmodule asset files when compiling (for instance, when XModule provides a coffeescript file, don't delete the associated javascript) +Studio: For courses running on edx.org (marketing site), disable fields in +Course Settings that do not apply. + Common: Make asset watchers run as singletons (so they won't start if the watcher is already running in another shell).