From e8e09afa0a3b00fe88ea4978d7647cf14dc9b20b Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 10:45:14 -0400 Subject: [PATCH 01/10] Make sure that we properly parse and save section release times Firefox wasn't saving section release times, due to issues with JS Date() parsing. I've modified the code to make it more explicit around what it should do and how it should work, which also makes it work better with both Firefox and Chrome. --- cms/static/js/base.js | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index de0fd955dc..7f82e67431 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -253,6 +253,12 @@ function syncReleaseDate(e) { $("#start_time").val(""); } +function pad2(number) { + // pad a number to two places: useful for formatting months, days, hours, etc + // when displaying a date/time + return (number < 10 ? '0' : '') + number; +} + function getEdxTimeFromDateTimeVals(date_val, time_val) { if (date_val != '') { if (time_val == '') time_val = '00:00'; @@ -772,21 +778,23 @@ function cancelSetSectionScheduleDate(e) { function saveSetSectionScheduleDate(e) { e.preventDefault(); - var input_date = $('.edit-subsection-publish-settings .start-date').val(); - var input_time = $('.edit-subsection-publish-settings .start-time').val(); - - var start = getEdxTimeFromDateTimeVals(input_date, input_time); + var date = $('.edit-subsection-publish-settings .start-date').datepicker("getDate"); + var time = $('.edit-subsection-publish-settings .start-time').timepicker("getTime"); + var datetime = new Date(Date.UTC( + date.getFullYear(), date.getMonth(), date.getDate(), + time.getHours(), time.getMinutes() + )); var id = $modal.attr('data-id'); analytics.track('Edited Section Release Date', { 'course': course_location_analytics, 'id': id, - 'start': start + 'start': datetime }); var saving = new CMS.Views.Notification.Mini({ - title: gettext("Saving") + "…", + title: gettext("Saving") + "…" }); saving.show(); // call into server to commit the new order @@ -798,7 +806,7 @@ function saveSetSectionScheduleDate(e) { data: JSON.stringify({ 'id': id, 'metadata': { - 'start': start + 'start': datetime } }) }).success(function() { @@ -806,12 +814,15 @@ function saveSetSectionScheduleDate(e) { var html = _.template( '' + '' + gettext("Will Release:") + ' ' + - gettext("<%= date %> at <%= time %> UTC") + + gettext("{month}/{day}/{year} at {hour}:{minute} UTC") + '' + - '' + + '' + gettext("Edit") + '', - {date: input_date, time: input_time, id: id}); + {year: datetime.getUTCFullYear(), month: pad2(datetime.getUTCMonth() + 1), day: pad2(datetime.getUTCDate()), + hour: pad2(datetime.getUTCHours()), minute: pad2(datetime.getUTCMinutes()), + id: id}, + {interpolate: /\{(.+?)\}/g}); $thisSection.find('.section-published-date').html(html); hideModal(); saving.hide(); From 87ce16c70e19c22c36177f8021a56e7b584e08ca Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 11:28:35 -0400 Subject: [PATCH 02/10] Cleanup/reformatting --- cms/static/js/views/overview.js | 2 +- cms/templates/edit_subsection.html | 9 ++++----- cms/templates/overview.html | 8 ++++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cms/static/js/views/overview.js b/cms/static/js/views/overview.js index e41a6971a6..4ce0d3688e 100644 --- a/cms/static/js/views/overview.js +++ b/cms/static/js/views/overview.js @@ -148,7 +148,7 @@ function generateCheckHoverState(selectorsToOpen, selectorsToShove) { } }); - } + }; } function removeHesitate(event, ui) { diff --git a/cms/templates/edit_subsection.html b/cms/templates/edit_subsection.html index bdcaf18015..5b03643f3b 100644 --- a/cms/templates/edit_subsection.html +++ b/cms/templates/edit_subsection.html @@ -1,11 +1,10 @@ -<%! from django.utils.translation import ugettext as _ %> <%inherit file="base.html" /> <%! - import logging - from xmodule.util.date_utils import get_default_time_display, almost_same_datetime + import logging + from xmodule.util.date_utils import get_default_time_display, almost_same_datetime + from django.utils.translation import ugettext as _ + from django.core.urlresolvers import reverse %> - -<%! from django.core.urlresolvers import reverse %> <%block name="title">${_("CMS Subsection")} <%block name="bodyclass">is-signedin course subsection diff --git a/cms/templates/overview.html b/cms/templates/overview.html index 3795e9d09b..2c42df187a 100644 --- a/cms/templates/overview.html +++ b/cms/templates/overview.html @@ -1,10 +1,10 @@ -<%! from django.utils.translation import ugettext as _ %> <%inherit file="base.html" /> <%! - import logging - from xmodule.util import date_utils + import logging + from xmodule.util import date_utils + from django.utils.translation import ugettext as _ + from django.core.urlresolvers import reverse %> -<%! from django.core.urlresolvers import reverse %> <%block name="title">${_("Course Outline")} <%block name="bodyclass">is-signedin course outline From 9c09323b0d1bd6215dd8b880f534aae245b64993 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 11:28:57 -0400 Subject: [PATCH 03/10] Properly parse and save section release times --- cms/static/js/base.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 7f82e67431..b2fcec84ae 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -313,9 +313,18 @@ function saveSubsection() { metadata[$(el).data("metadata-name")] = el.value; } - // Piece back together the date/time UI elements into one date/time string - metadata['start'] = getEdxTimeFromDateTimeInputs('start_date', 'start_time'); - metadata['due'] = getEdxTimeFromDateTimeInputs('due_date', 'due_time'); + // get datetimes for start and due, stick into metadata + _(["start", "due"]).each(function(name) { + var date, time; + date = $("#"+name+"_date").datepicker("getDate"); + time = $("#"+name+"_time").timepicker("getTime"); + if (date && time) { + metadata[name] = new Date(Date.UTC( + date.getFullYear(), date.getMonth(), date.getDate(), + time.getHours(), time.getMinutes() + )); + } + }); $.ajax({ url: "/save_item", From d2c4ac2597d6c97d9f29629111699f5ec4fc27d2 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 14:30:19 -0400 Subject: [PATCH 04/10] Fixed Jasmine tests --- .../coffee/spec/views/overview_spec.coffee | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/cms/static/coffee/spec/views/overview_spec.coffee b/cms/static/coffee/spec/views/overview_spec.coffee index d900e4bfb1..8bda96da66 100644 --- a/cms/static/coffee/spec/views/overview_spec.coffee +++ b/cms/static/coffee/spec/views/overview_spec.coffee @@ -1,22 +1,19 @@ describe "Course Overview", -> beforeEach -> - appendSetFixtures """ - - """ - - appendSetFixtures """ - - """ + _.each ["/static/js/vendor/date.js", "/static/js/vendor/timepicker/jquery.timepicker.js", "/jsi18n/"], (path) -> + appendSetFixtures """ + + """ appendSetFixtures """
Will Release: 06/12/2013 at 04:00 UTC - Edit + Edit
- """#" + """ appendSetFixtures """
@@ -38,7 +35,7 @@ describe "Course Overview", -> SaveCancel
- """#" + """ appendSetFixtures """
@@ -46,12 +43,13 @@ describe "Course Overview", ->
- """#" + """ spyOn(window, 'saveSetSectionScheduleDate').andCallThrough() # Have to do this here, as it normally gets bound in document.ready() $('a.save-button').click(saveSetSectionScheduleDate) $('a.delete-section-button').click(deleteSection) + $(".edit-subsection-publish-settings .start-date").datepicker() @notificationSpy = spyOn(CMS.Views.Notification.Mini.prototype, 'show').andCallThrough() window.analytics = jasmine.createSpyObj('analytics', ['track']) From c0bd7db2936c89f3125e10ac562a6bcfcbf00645 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 16:47:23 -0400 Subject: [PATCH 05/10] Removed unused JS functions --- cms/static/js/base.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index b2fcec84ae..7db66a525d 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -259,23 +259,6 @@ function pad2(number) { return (number < 10 ? '0' : '') + number; } -function getEdxTimeFromDateTimeVals(date_val, time_val) { - if (date_val != '') { - if (time_val == '') time_val = '00:00'; - - return new Date(date_val + " " + time_val + "Z"); - } - - else return null; -} - -function getEdxTimeFromDateTimeInputs(date_id, time_id) { - var input_date = $('#' + date_id).val(); - var input_time = $('#' + time_id).val(); - - return getEdxTimeFromDateTimeVals(input_date, input_time); -} - function autosaveInput(e) { var self = this; if (this.saveTimer) { From aab9661fe962012f0d7f983e94d19302162be6b6 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 16:48:09 -0400 Subject: [PATCH 06/10] Scoped pad2 function to the one place that it's called --- cms/static/js/base.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 7db66a525d..4a5fc2b182 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -253,12 +253,6 @@ function syncReleaseDate(e) { $("#start_time").val(""); } -function pad2(number) { - // pad a number to two places: useful for formatting months, days, hours, etc - // when displaying a date/time - return (number < 10 ? '0' : '') + number; -} - function autosaveInput(e) { var self = this; if (this.saveTimer) { @@ -802,6 +796,12 @@ function saveSetSectionScheduleDate(e) { } }) }).success(function() { + var pad2 = function(number) { + // pad a number to two places: useful for formatting months, days, hours, etc + // when displaying a date/time + return (number < 10 ? '0' : '') + number; + }; + var $thisSection = $('.courseware-section[data-id="' + id + '"]'); var html = _.template( '' + From ae019ef8c9f4d0ea67574ee3edf4a2b5c7f3d393 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 17:09:12 -0400 Subject: [PATCH 07/10] Abstracted functionality to get datetime into separate JS function --- cms/static/js/base.js | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index 4a5fc2b182..bb772da02b 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -253,6 +253,22 @@ function syncReleaseDate(e) { $("#start_time").val(""); } +function getDatetime(datepickerInput, timepickerInput) { + // given a pair of inputs (datepicker and timepicker), return a JS Date + // object that corresponds to the datetime that they represent. Assume + // UTC timezone, NOT the timezone of the user's browser. + var date = $(datepickerInput).datepicker("getDate"); + var time = $(timepickerInput).timepicker("getTime"); + if(date && time) { + return new Date(Date.UTC( + date.getFullYear(), date.getMonth(), date.getDate(), + time.getHours(), time.getMinutes() + )); + } else { + return null; + } +} + function autosaveInput(e) { var self = this; if (this.saveTimer) { @@ -292,14 +308,13 @@ function saveSubsection() { // get datetimes for start and due, stick into metadata _(["start", "due"]).each(function(name) { - var date, time; - date = $("#"+name+"_date").datepicker("getDate"); - time = $("#"+name+"_time").timepicker("getTime"); - if (date && time) { - metadata[name] = new Date(Date.UTC( - date.getFullYear(), date.getMonth(), date.getDate(), - time.getHours(), time.getMinutes() - )); + + var datetime = getDatetime( + document.getElementById(name+"_date"), + document.getElementById(name+"_time") + ); + if (datetime) { + metadata[name] = datetime; } }); @@ -764,12 +779,10 @@ function cancelSetSectionScheduleDate(e) { function saveSetSectionScheduleDate(e) { e.preventDefault(); - var date = $('.edit-subsection-publish-settings .start-date').datepicker("getDate"); - var time = $('.edit-subsection-publish-settings .start-time').timepicker("getTime"); - var datetime = new Date(Date.UTC( - date.getFullYear(), date.getMonth(), date.getDate(), - time.getHours(), time.getMinutes() - )); + var datetime = getDatetime( + $('.edit-subsection-publish-settings .start-date'), + $('.edit-subsection-publish-settings .start-time') + ); var id = $modal.attr('data-id'); From ee3ce7b6c20861509b6ae72287ccd03eb20fb2e1 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 7 Aug 2013 09:45:44 -0400 Subject: [PATCH 08/10] Don't ignore null datetimes on subsection settings Clicking "Sync to " should send an AJAX request with the datetimes set to null, so that the server resets them. --- cms/static/js/base.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cms/static/js/base.js b/cms/static/js/base.js index bb772da02b..80b24776da 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -313,9 +313,9 @@ function saveSubsection() { document.getElementById(name+"_date"), document.getElementById(name+"_time") ); - if (datetime) { - metadata[name] = datetime; - } + // if datetime is null, we want to set that in metadata anyway; + // its an indication to the server to clear the datetime in the DB + metadata[name] = datetime; }); $.ajax({ From e30a9a6fe33877a79a6047e3ff5a6cf9e858133d Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Wed, 7 Aug 2013 11:03:19 -0400 Subject: [PATCH 09/10] Created a new lettuce test to catch bug, updated other lettuce tests --- .../contentstore/features/section.feature | 2 +- .../contentstore/features/section.py | 13 ++-- .../contentstore/features/subsection.feature | 24 ++++++-- .../contentstore/features/subsection.py | 60 ++++++++++++++----- 4 files changed, 75 insertions(+), 24 deletions(-) diff --git a/cms/djangoapps/contentstore/features/section.feature b/cms/djangoapps/contentstore/features/section.feature index a08b490c6d..d9dd6f9398 100644 --- a/cms/djangoapps/contentstore/features/section.feature +++ b/cms/djangoapps/contentstore/features/section.feature @@ -24,7 +24,7 @@ Feature: Create Section Given I have opened a new course in Studio And I have added a new section When I click the Edit link for the release date - And I save a new section release date + And I set the section release date to 12/25/2013 Then the section release date is updated And I see a "saving" notification diff --git a/cms/djangoapps/contentstore/features/section.py b/cms/djangoapps/contentstore/features/section.py index 955c6a8f4e..3ca8e1676d 100644 --- a/cms/djangoapps/contentstore/features/section.py +++ b/cms/djangoapps/contentstore/features/section.py @@ -35,10 +35,15 @@ def i_click_the_edit_link_for_the_release_date(_step): world.css_click(button_css) -@step('I save a new section release date$') -def i_save_a_new_section_release_date(_step): - set_date_and_time('input.start-date.date.hasDatepicker', '12/25/2013', - 'input.start-time.time.ui-timepicker-input', '00:00') +@step('I set the section release date to ([0-9/-]+)( [0-9:]+)?') +def set_section_release_date(_step, datestring, timestring): + if hasattr(timestring, "strip"): + timestring = timestring.strip() + if not timestring: + timestring = "00:00" + set_date_and_time( + 'input.start-date.date.hasDatepicker', datestring, + 'input.start-time.time.ui-timepicker-input', timestring) world.browser.click_link_by_text('Save') diff --git a/cms/djangoapps/contentstore/features/subsection.feature b/cms/djangoapps/contentstore/features/subsection.feature index 9f5793dbe7..84755b3644 100644 --- a/cms/djangoapps/contentstore/features/subsection.feature +++ b/cms/djangoapps/contentstore/features/subsection.feature @@ -14,7 +14,7 @@ Feature: Create Subsection When I click the New Subsection link And I enter a subsection name with a quote and click save Then I see my subsection name with a quote on the Courseware page - And I click to edit the subsection name + And I click on the subsection Then I see the complete subsection name with a quote in the editor Scenario: Assign grading type to a subsection and verify it is still shown after refresh (bug #258) @@ -27,10 +27,13 @@ Feature: Create Subsection Scenario: Set a due date in a different year (bug #256) Given I have opened a new subsection in Studio - And I have set a release date and due date in different years - Then I see the correct dates + And I set the subsection release date to 12/25/2011 03:00 + And I set the subsection due date to 01/02/2012 04:00 + Then I see the subsection release date is 12/25/2011 03:00 + And I see the subsection due date is 01/02/2012 04:00 And I reload the page - Then I see the correct dates + Then I see the subsection release date is 12/25/2011 03:00 + And I see the subsection due date is 01/02/2012 04:00 Scenario: Delete a subsection Given I have opened a new course section in Studio @@ -40,3 +43,16 @@ Feature: Create Subsection And I press the "subsection" delete icon And I confirm the prompt Then the subsection does not exist + + Scenario: Sync to Section + Given I have opened a new course section in Studio + And I click the Edit link for the release date + And I set the section release date to 01/02/2103 + And I have added a new subsection + And I click on the subsection + And I set the subsection release date to 01/20/2103 + And I reload the page + And I click the link to sync release date to section + And I wait for "1" second + And I reload the page + Then I see the subsection release date is 01/02/2103 diff --git a/cms/djangoapps/contentstore/features/subsection.py b/cms/djangoapps/contentstore/features/subsection.py index e280ec615d..60a325f550 100644 --- a/cms/djangoapps/contentstore/features/subsection.py +++ b/cms/djangoapps/contentstore/features/subsection.py @@ -41,8 +41,8 @@ def i_save_subsection_name_with_quote(step): save_subsection_name('Subsection With "Quote"') -@step('I click to edit the subsection name$') -def i_click_to_edit_subsection_name(step): +@step('I click on the subsection$') +def click_on_subsection(step): world.css_click('span.subsection-name-value') @@ -53,12 +53,28 @@ def i_see_complete_subsection_name_with_quote_in_editor(step): assert_equal(world.css_value(css), 'Subsection With "Quote"') -@step('I have set a release date and due date in different years$') -def test_have_set_dates_in_different_years(step): - set_date_and_time('input#start_date', '12/25/2011', 'input#start_time', '03:00') - world.css_click('.set-date') - # Use a year in the past so that current year will always be different. - set_date_and_time('input#due_date', '01/02/2012', 'input#due_time', '04:00') +@step('I set the subsection release date to ([0-9/-]+)( [0-9:]+)?') +def set_subsection_release_date(_step, datestring, timestring): + if hasattr(timestring, "strip"): + timestring = timestring.strip() + if not timestring: + timestring = "00:00" + set_date_and_time( + 'input#start_date', datestring, + 'input#start_time', timestring) + + +@step('I set the subsection due date to ([0-9/-]+)( [0-9:]+)?') +def set_subsection_due_date(_step, datestring, timestring): + if hasattr(timestring, "strip"): + timestring = timestring.strip() + if not timestring: + timestring = "00:00" + if not world.css_visible('input#due_date'): + world.css_click('.due-date-input .set-date') + set_date_and_time( + 'input#due_date', datestring, + 'input#due_time', timestring) @step('I mark it as Homework$') @@ -72,6 +88,11 @@ def i_see_it_marked__as_homework(step): assert_equal(world.css_value(".status-label"), 'Homework') +@step('I click the link to sync release date to section') +def click_sync_release_date(step): + world.css_click('.sync-date') + + ############ ASSERTIONS ################### @@ -91,16 +112,25 @@ def the_subsection_does_not_exist(step): assert world.browser.is_element_not_present_by_css(css) -@step('I see the correct dates$') -def i_see_the_correct_dates(step): - assert_equal('12/25/2011', get_date('input#start_date')) - assert_equal('03:00', get_date('input#start_time')) - assert_equal('01/02/2012', get_date('input#due_date')) - assert_equal('04:00', get_date('input#due_time')) +@step('I see the subsection release date is ([0-9/-]+)( [0-9:]+)?') +def i_see_subsection_release(_step, datestring, timestring): + if hasattr(timestring, "strip"): + timestring = timestring.strip() + assert_equal(datestring, get_date('input#start_date')) + if timestring: + assert_equal(timestring, get_date('input#start_time')) + + +@step('I see the subsection due date is ([0-9/-]+)( [0-9:]+)?') +def i_see_subsection_due(_step, datestring, timestring): + if hasattr(timestring, "strip"): + timestring = timestring.strip() + assert_equal(datestring, get_date('input#due_date')) + if timestring: + assert_equal(timestring, get_date('input#due_time')) ############ HELPER METHODS ################### - def get_date(css): return world.css_find(css).first.value.strip() From df25770fa714d7349ff1d87e32bcaffb837071ba Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Tue, 6 Aug 2013 16:39:02 -0400 Subject: [PATCH 10/10] Assign isExternal JS function to window object When JS functions are defined with names, they are local variables, and inaccessible if defined inside a closure. Django-Pipeline concatenates all of our JS into one big closure. This function explicitly assings the function to a property of the `window` object, so that it is accessible to other JS functions. --- common/static/js/utility.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/common/static/js/utility.js b/common/static/js/utility.js index 6407faad48..a983535b46 100644 --- a/common/static/js/utility.js +++ b/common/static/js/utility.js @@ -1,20 +1,20 @@ // checks whether or not the url is external to the local site. // generously provided by StackOverflow: http://stackoverflow.com/questions/6238351/fastest-way-to-detect-external-urls -function isExternal(url) { +window.isExternal = function (url) { // parse the url into protocol, host, path, query, and fragment. More information can be found here: http://tools.ietf.org/html/rfc3986#appendix-B var match = url.match(/^([^:\/?#]+:)?(?:\/\/([^\/?#]*))?([^?#]+)?(\?[^#]*)?(#.*)?/); // match[1] matches a protocol if one exists in the url // if the protocol in the url does not match the protocol in the window's location, this url is considered external - if (typeof match[1] === "string" && - match[1].length > 0 - && match[1].toLowerCase() !== location.protocol) + if (typeof match[1] === "string" && + match[1].length > 0 && + match[1].toLowerCase() !== location.protocol) return true; // match[2] matches the host if one exists in the url // if the host in the url does not match the host of the window location, this url is considered external - if (typeof match[2] === "string" && - match[2].length > 0 && + if (typeof match[2] === "string" && + match[2].length > 0 && // this regex removes the port number if it patches the current location's protocol - match[2].replace(new RegExp(":("+{"http:":80,"https:":443}[location.protocol]+")?$"), "") !== location.host) + match[2].replace(new RegExp(":("+{"http:":80,"https:":443}[location.protocol]+")?$"), "") !== location.host) return true; return false; -} +};