From 4d136f8d3bc1d247941b68f0af8b2caac4e947b0 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Thu, 7 Mar 2013 11:51:45 -0500 Subject: [PATCH 01/16] fixed annotation tooltip styling issue in studio --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index 308b379ec1..c462d4806e 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -127,6 +127,7 @@ $body-font-size: em(14); font-weight: 400; padding: 0 10px 10px 10px; background-color: transparent; + border-color: transparent; } p { color: inherit; From f5c3775b5dcbb8b16e6a0fcd27fd8b835516a56e Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Thu, 7 Mar 2013 12:21:47 -0500 Subject: [PATCH 02/16] fixed the annotation tooltip line height so it is the same in studio and the lms. --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index c462d4806e..b5739b28fc 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -144,6 +144,7 @@ $body-font-size: em(14); margin: 0px 0px 10px 0; max-height: 225px; overflow: auto; + line-height: normal; } .annotatable-reply { display: block; From 60b060263c15bb90fc658349224c50158609e31d Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Thu, 7 Mar 2013 16:02:22 -0500 Subject: [PATCH 03/16] refactor highlight css to prevent issues with cascade --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index b5739b28fc..f8ae779b8c 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -55,6 +55,7 @@ $body-font-size: em(14); display: inline; cursor: pointer; + $highlight_index: 0; @each $highlight in ( (yellow rgba(255,255,10,0.3) rgba(255,255,10,0.9)), (red rgba(178,19,16,0.3) rgba(178,19,16,0.9)), @@ -62,12 +63,13 @@ $body-font-size: em(14); (green rgba(25,255,132,0.3) rgba(25,255,132,0.9)), (blue rgba(35,163,255,0.3) rgba(35,163,255,0.9)), (purple rgba(115,9,178,0.3) rgba(115,9,178,0.9))) { - + + $highlight_index: $highlight_index + 1; $marker: nth($highlight,1); $color: nth($highlight,2); $selected_color: nth($highlight,3); - @if $marker == yellow { + @if $highlight_index == 1 { &.highlight { background-color: $color; &.selected { background-color: $selected_color; } @@ -167,5 +169,3 @@ $body-font-size: em(14); border-top-color: rgba(0, 0, 0, .85); } } - - From 094458dd6f0e4437a71dcbcd990d31286725dc16 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Mon, 11 Mar 2013 16:19:36 -0400 Subject: [PATCH 04/16] Modified tooltip positioning on non-overlapping annotation spans. --- .../xmodule/js/src/annotatable/display.coffee | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee index 2ad49ae6d7..523b0e99cf 100644 --- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee @@ -75,6 +75,7 @@ class @Annotatable classes: 'ui-tooltip-annotatable' events: show: @onShowTip + move: @onMoveTip onClickToggleAnnotations: (e) => @toggleAnnotations() @@ -87,6 +88,40 @@ class @Annotatable onShowTip: (event, api) => event.preventDefault() if @annotationsHidden + onMoveTip: (event, api, position) => + ### + This method handles an edge case in which a tooltip is displayed above + a non-overlapping span like this: + + (( TOOLTIP )) + \/ + text text text ... text text text ...... + + + The problem is that the tooltip looks disconnected from both spans, so + we should re-position the tooltip to appear above the span. + ### + + tip = api.elements.tooltip + adjust_y = api.options.position?.adjust?.y || 0 + target = api.elements.target + rects = $(target).get(0).getClientRects() + is_non_overlapping = (rects?.length == 2 and rects[0].left > rects[1].right) + + if is_non_overlapping + focus_rect = rects[0] + rect_center = focus_rect.left + (focus_rect.width / 2) + rect_top = focus_rect.top + tip_width = $(tip).width() + tip_height = $(tip).height() + tip_left = rect_center - (tip_width / 2) + tip_top = window.pageYOffset + rect_top - tip_height + adjust_y + win_width = $(window).width() + if tip_left + tip_width > win_width + tip_left = win_width - tip_width + position.left = tip_left + position.top = tip_top + getSpanForProblemReturn: (el) -> problem_id = $(@problemReturnSelector).index(el) @$(@spanSelector).filter("[data-problem-id='#{problem_id}']") From fcf82ba2bc44cb701c020d0494e2537139635f27 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Mon, 11 Mar 2013 18:02:22 -0400 Subject: [PATCH 05/16] fixed pep8 violations for annotation module --- common/lib/xmodule/xmodule/annotatable_module.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index f093b76f52..1385296ddf 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -11,13 +11,13 @@ from xmodule.contentstore.content import StaticContent log = logging.getLogger(__name__) + class AnnotatableModule(XModule): js = {'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee'), resource_string(__name__, 'js/src/collapsible.coffee'), resource_string(__name__, 'js/src/html/display.coffee'), resource_string(__name__, 'js/src/annotatable/display.coffee')], - 'js': [] - } + 'js': []} js_module_name = "Annotatable" css = {'scss': [resource_string(__name__, 'css/annotatable/display.scss')]} icon_class = 'annotatable' @@ -34,11 +34,11 @@ class AnnotatableModule(XModule): if color is not None: if color in self.highlight_colors: - cls.append('highlight-'+color) + cls.append('highlight-' + color) attr['_delete'] = highlight_key attr['value'] = ' '.join(cls) - return { 'class' : attr } + return {'class': attr} def _get_annotation_data_attr(self, index, el): """ Returns a dict in which the keys are the HTML data attributes @@ -58,7 +58,7 @@ class AnnotatableModule(XModule): if xml_key in el.attrib: value = el.get(xml_key, '') html_key = attrs_map[xml_key] - data_attrs[html_key] = { 'value': value, '_delete': xml_key } + data_attrs[html_key] = {'value': value, '_delete': xml_key} return data_attrs @@ -76,7 +76,6 @@ class AnnotatableModule(XModule): delete_key = attr[key]['_delete'] del el.attrib[delete_key] - def _render_content(self): """ Renders annotatable content with annotation spans and returns HTML. """ xmltree = etree.fromstring(self.content) @@ -123,9 +122,9 @@ class AnnotatableModule(XModule): self.element_id = self.location.html_id() self.highlight_colors = ['yellow', 'orange', 'purple', 'blue', 'green'] + class AnnotatableDescriptor(RawDescriptor): module_class = AnnotatableModule stores_state = True template_dir_name = "annotatable" mako_template = "widgets/raw-edit.html" - From d860b167d6838443d05b5b67805047a7e032f6a3 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Tue, 12 Mar 2013 14:09:56 -0400 Subject: [PATCH 06/16] fixed tooltip positioning for non-overlapping spans in studio --- .../xmodule/css/annotatable/display.scss | 4 +++ .../xmodule/js/src/annotatable/display.coffee | 34 ++++++++++++++----- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index f8ae779b8c..6e1a38ee31 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -1,6 +1,10 @@ $border-color: #C8C8C8; $body-font-size: em(14); +.annotatable-wrapper { + position: relative; +} + .annotatable-header { margin-bottom: .5em; .annotatable-title { diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee index 523b0e99cf..e38e48eeda 100644 --- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee @@ -1,7 +1,8 @@ class @Annotatable _debug: false - # selectors for the annotatable xmodule + # selectors for the annotatable xmodule + wrapperSelector: '.annotatable-wrapper' toggleAnnotationsSelector: '.annotatable-toggle-annotations' toggleInstructionsSelector: '.annotatable-toggle-instructions' instructionsSelector: '.annotatable-instructions' @@ -61,7 +62,7 @@ class @Annotatable my: 'bottom center' # of tooltip at: 'top center' # of target target: $(el) # where the tooltip was triggered (i.e. the annotation span) - container: @$el + container: @$(@wrapperSelector) adjust: y: -5 show: @@ -104,23 +105,38 @@ class @Annotatable tip = api.elements.tooltip adjust_y = api.options.position?.adjust?.y || 0 + container = api.options.position?.container || $('body') target = api.elements.target + rects = $(target).get(0).getClientRects() is_non_overlapping = (rects?.length == 2 and rects[0].left > rects[1].right) if is_non_overlapping - focus_rect = rects[0] + # we want to choose the largest of the two non-overlapping spans and display + # the tooltip above the center of it (see api.options.position settings) + focus_rect = (if rects[0].width > rects[1].width then rects[0] else rects[1]) rect_center = focus_rect.left + (focus_rect.width / 2) rect_top = focus_rect.top tip_width = $(tip).width() tip_height = $(tip).height() - tip_left = rect_center - (tip_width / 2) - tip_top = window.pageYOffset + rect_top - tip_height + adjust_y + + # tooltip is positioned relative to its container, so we need to factor in offsets + container_offset = $(container).offset() + offset_left = -container_offset.left + offset_top = $('body').scrollTop() - container_offset.top + + tip_left = offset_left + rect_center - (tip_width / 2) + tip_top = offset_top + rect_top - tip_height + adjust_y + + # make sure the new tip position doesn't clip the edges of the screen win_width = $(window).width() - if tip_left + tip_width > win_width - tip_left = win_width - tip_width - position.left = tip_left - position.top = tip_top + if tip_left < offset_left + tip_left = offset_left + else if tip_left + tip_width > win_width + offset_left + tip_left = win_width + offset_left - tip_width + + # final step: update the position object (used by qtip2 to show the tip after the move event) + $.extend position, 'left': tip_left, 'top': tip_top getSpanForProblemReturn: (el) -> problem_id = $(@problemReturnSelector).index(el) From bf6ca1b0e759252795ca89ad905828d30ceada28 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Tue, 12 Mar 2013 17:32:00 -0400 Subject: [PATCH 07/16] use document to get scrollTop for firefox --- common/lib/xmodule/xmodule/js/src/annotatable/display.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee index e38e48eeda..8a32c8f51e 100644 --- a/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/annotatable/display.coffee @@ -123,7 +123,7 @@ class @Annotatable # tooltip is positioned relative to its container, so we need to factor in offsets container_offset = $(container).offset() offset_left = -container_offset.left - offset_top = $('body').scrollTop() - container_offset.top + offset_top = $(document).scrollTop() - container_offset.top tip_left = offset_left + rect_center - (tip_width / 2) tip_top = offset_top + rect_top - tip_height + adjust_y From 7507f91d2274a64c2993717bcbecb078346461a3 Mon Sep 17 00:00:00 2001 From: Arthur Barrett Date: Mon, 18 Mar 2013 15:43:43 -0400 Subject: [PATCH 08/16] Added TODO and explanation for the top-level scss variables. --- common/lib/xmodule/xmodule/css/annotatable/display.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/lib/xmodule/xmodule/css/annotatable/display.scss b/common/lib/xmodule/xmodule/css/annotatable/display.scss index 6e1a38ee31..e2c095de2d 100644 --- a/common/lib/xmodule/xmodule/css/annotatable/display.scss +++ b/common/lib/xmodule/xmodule/css/annotatable/display.scss @@ -1,3 +1,9 @@ +/* TODO: move top-level variables to a common _variables.scss. + * NOTE: These variables were only added here because when this was integrated with the CMS, + * SASS compilation errors were triggered because the CMS didn't have the same variables defined + * that the LMS did, so the quick fix was to localize the LMS variables not shared by the CMS. + * -Abarrett and Vshnayder + */ $border-color: #C8C8C8; $body-font-size: em(14); From cf5197129824afad9fa2da8843af92ae5eb6f288 Mon Sep 17 00:00:00 2001 From: "Mark L. Chang" Date: Wed, 27 Mar 2013 22:36:18 -0400 Subject: [PATCH 09/16] added net promoter score tracking javascript shim from Qualaroo --- cms/templates/base.html | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cms/templates/base.html b/cms/templates/base.html index e852b5d7fe..30c7045440 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -23,6 +23,18 @@ + + + + + + + <%block name="header_extras"> From 29731ec40b1fe843502640c3472d5d5f342aad16 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Fri, 29 Mar 2013 15:20:34 -0400 Subject: [PATCH 10/16] management command to remove excess input_state entries --- .../management/commands/remove_input_state.py | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 lms/djangoapps/courseware/management/commands/remove_input_state.py diff --git a/lms/djangoapps/courseware/management/commands/remove_input_state.py b/lms/djangoapps/courseware/management/commands/remove_input_state.py new file mode 100644 index 0000000000..722fa90fdd --- /dev/null +++ b/lms/djangoapps/courseware/management/commands/remove_input_state.py @@ -0,0 +1,129 @@ +''' +This is a one-off command aimed at fixing a temporary problem encountered where input_state was added to +the same dict object in capa problems, so was accumulating. The fix is simply to remove input_state entry +from state for all problems in the affected date range. +''' + +import json +import logging +from optparse import make_option + +from django.core.management.base import BaseCommand +from django.db import transaction + +from courseware.models import StudentModule, StudentModuleHistory + +LOG = logging.getLogger(__name__) + + +class Command(BaseCommand): + ''' + The fix here is to remove the "input_state" entry in the StudentModule objects of any problems that + contain them. No problem is yet making use of this, and the code should do the right thing if it's + missing (by recreating an empty dict for its value). + + To narrow down the set of problems that might need fixing, the StudentModule + objects to be checked is filtered down to those: + + created < '2013-03-29 16:30:00' (the problem must have been answered before the buggy code was reverted, + on Prod and Edge) + modified > '2013-03-28 22:00:00' (the problem must have been visited after the bug was introduced + on Prod and Edge) + state like '%input_state%' (the problem must have "input_state" set). + ''' + + num_visited = 0 + num_changed = 0 + num_hist_visited = 0 + num_hist_changed = 0 + + option_list = BaseCommand.option_list + ( + make_option('--save', + action='store_true', + dest='save_changes', + default=False, + help='Persist the changes that were encountered. If not set, no changes are saved.'), ) + + def fix_studentmodules(self, save_changes): + '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' + modules = StudentModule.objects.filter(modified__gt='2013-03-28 22:00:00', + created__lt='2013-03-29 16:30:00', + state__contains='input_state') + + for module in modules: + self.remove_studentmodule_input_state(module, save_changes) + + LOG.info("Finished student modules: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) + + hist_modules = StudentModuleHistory.objects.filter(created__gt='2013-03-28 22:00:00', + created__lt='2013-03-29 16:30:00', + state__contains='input_state') + + for hist_module in hist_modules: + self.remove_studentmodulehistory_input_state(hist_module, save_changes) + + LOG.info("Finished student history modules: updating {0} of {1} modules".format(self.num_hist_changed, self.num_hist_visited)) + + @transaction.autocommit + def remove_studentmodule_input_state(self, module, save_changes): + ''' Fix the grade assigned to a StudentModule''' + module_state = module.state + if module_state is None: + # not likely, since we filter on it. But in general... + LOG.info("No state found for {type} module {id} for student {student} in course {course_id}" + .format(type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) + return + + state_dict = json.loads(module_state) + self.num_visited += 1 + + if 'input_state' not in state_dict: + pass + elif save_changes: + # make the change and persist + del state_dict['input_state'] + module.state = json.dumps(state_dict) + module.save() + self.num_changed += 1 + else: + # don't make the change, but increment the count indicating the change would be made + self.num_changed += 1 + + @transaction.autocommit + def remove_studentmodulehistory_input_state(self, module, save_changes): + ''' Fix the grade assigned to a StudentModule''' + module_state = module.state + if module_state is None: + # not likely, since we filter on it. But in general... + LOG.info("No state found for {type} module {id} for student {student} in course {course_id}" + .format(type=module.module_type, id=module.module_state_key, + student=module.student.username, course_id=module.course_id)) + return + + state_dict = json.loads(module_state) + self.num_hist_visited += 1 + + if 'input_state' not in state_dict: + pass + elif save_changes: + # make the change and persist + del state_dict['input_state'] + module.state = json.dumps(state_dict) + module.save() + self.num_hist_changed += 1 + else: + # don't make the change, but increment the count indicating the change would be made + self.num_hist_changed += 1 + + def handle(self, **options): + '''Handle management command request''' + + save_changes = options['save_changes'] + + LOG.info("Starting run: save_changes = {0}".format(save_changes)) + + self.fix_studentmodules(save_changes) + + LOG.info("Finished run: updating {0} of {1} student modules".format(self.num_changed, self.num_visited)) + LOG.info("Finished run: updating {0} of {1} student history modules".format(self.num_hist_changed, self.num_hist_visited)) From b6f95c71ce30f74f3b62ecbf39c26f14cc825143 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Tue, 2 Apr 2013 16:20:46 -0400 Subject: [PATCH 11/16] change command to handle file input instead --- .../management/commands/remove_input_state.py | 84 ++++++++++++++----- 1 file changed, 62 insertions(+), 22 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/remove_input_state.py b/lms/djangoapps/courseware/management/commands/remove_input_state.py index 722fa90fdd..772758a33b 100644 --- a/lms/djangoapps/courseware/management/commands/remove_input_state.py +++ b/lms/djangoapps/courseware/management/commands/remove_input_state.py @@ -8,7 +8,7 @@ import json import logging from optparse import make_option -from django.core.management.base import BaseCommand +from django.core.management.base import BaseCommand, CommandError from django.db import transaction from courseware.models import StudentModule, StudentModuleHistory @@ -30,6 +30,17 @@ class Command(BaseCommand): modified > '2013-03-28 22:00:00' (the problem must have been visited after the bug was introduced on Prod and Edge) state like '%input_state%' (the problem must have "input_state" set). + + This filtering is done on the production database replica, so that the larger select queries don't lock + the real production database. The list of id values for Student Modules is written to a file, and the + file is passed into this command. The sql file passed to mysql contains: + + select sm.id from courseware_studentmodule sm + where sm.modified > "2013-03-28 22:00:00" + and sm.created < "2013-03-29 16:30:00" + and sm.state like "%input_state%" + and sm.module_type = 'problem'; + ''' num_visited = 0 @@ -42,26 +53,54 @@ class Command(BaseCommand): action='store_true', dest='save_changes', default=False, - help='Persist the changes that were encountered. If not set, no changes are saved.'), ) + help='Persist the changes that were encountered. If not set, no changes are saved.'), + make_option('--idlist', + action='store', + dest='idlist_path', + help='Path containing id values of StudentModule objects to clean (one per line).'), + ) - def fix_studentmodules(self, save_changes): - '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' - modules = StudentModule.objects.filter(modified__gt='2013-03-28 22:00:00', - created__lt='2013-03-29 16:30:00', - state__contains='input_state') +# DON'T DO THIS: +# def fix_studentmodules(self, save_changes): +# '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' +# modules = StudentModule.objects.filter(modified__gt='2013-03-28 22:00:00', +# created__lt='2013-03-29 16:30:00', +# state__contains='input_state') +# +# for module in modules: +# self.remove_studentmodule_input_state(module, save_changes) +# +# LOG.info("Finished student modules: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) +# +# hist_modules = StudentModuleHistory.objects.filter(created__gt='2013-03-28 22:00:00', +# created__lt='2013-03-29 16:30:00', +# state__contains='input_state') +# +# for hist_module in hist_modules: +# self.remove_studentmodulehistory_input_state(hist_module, save_changes) +# +# LOG.info("Finished student history modules: updating {0} of {1} modules".format(self.num_hist_changed, self.num_hist_visited)) - for module in modules: + def fix_studentmodules_in_list(self, save_changes, idlist_path): + '''Read in the list of StudentModule objects that might need fixing, and then fix each one''' + + # open file and read id values from it: + for line in open(idlist_path, 'r'): + student_module_id = line.strip() + if student_module_id == 'id': + continue + try: + module = StudentModule.objects.get(id=student_module_id) + except: + LOG.error("Unable to find student module with id = {0}: skipping... ".format(student_module_id)) + continue self.remove_studentmodule_input_state(module, save_changes) + hist_modules = StudentModuleHistory.objects.filter(student_module_id = student_module_id) + for hist_module in hist_modules: + self.remove_studentmodulehistory_input_state(hist_module, save_changes) + LOG.info("Finished student modules: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) - - hist_modules = StudentModuleHistory.objects.filter(created__gt='2013-03-28 22:00:00', - created__lt='2013-03-29 16:30:00', - state__contains='input_state') - - for hist_module in hist_modules: - self.remove_studentmodulehistory_input_state(hist_module, save_changes) - LOG.info("Finished student history modules: updating {0} of {1} modules".format(self.num_hist_changed, self.num_hist_visited)) @transaction.autocommit @@ -116,14 +155,15 @@ class Command(BaseCommand): # don't make the change, but increment the count indicating the change would be made self.num_hist_changed += 1 - def handle(self, **options): + def handle(self, *args, **options): '''Handle management command request''' - + if len(args) != 1: + raise CommandError("missing idlist file") + idlist_path = args[0] save_changes = options['save_changes'] + LOG.info("Starting run: reading from idlist file {0}; save_changes = {1}".format(idlist_path, save_changes)) - LOG.info("Starting run: save_changes = {0}".format(save_changes)) - - self.fix_studentmodules(save_changes) - + self.fix_studentmodules_in_list(save_changes, idlist_path) + LOG.info("Finished run: updating {0} of {1} student modules".format(self.num_changed, self.num_visited)) LOG.info("Finished run: updating {0} of {1} student history modules".format(self.num_hist_changed, self.num_hist_visited)) From 7a57e369d1f18baa3a9e172a2bef70144076588e Mon Sep 17 00:00:00 2001 From: "Mark L. Chang" Date: Wed, 3 Apr 2013 00:26:12 -0400 Subject: [PATCH 12/16] put survey behind MITX_FEATURES flag --- cms/envs/common.py | 1 + cms/envs/dev.py | 3 +++ cms/templates/base.html | 14 ++------------ cms/templates/widgets/qualaroo.html | 13 +++++++++++++ 4 files changed, 19 insertions(+), 12 deletions(-) create mode 100644 cms/templates/widgets/qualaroo.html diff --git a/cms/envs/common.py b/cms/envs/common.py index 12fa09947a..5ad9068636 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -34,6 +34,7 @@ MITX_FEATURES = { 'ENABLE_DISCUSSION_SERVICE': False, 'AUTH_USE_MIT_CERTIFICATES': False, 'STUB_VIDEO_FOR_TESTING': False, # do not display video when running automated acceptance tests + 'STUDIO_NPS_SURVEY': True, } ENABLE_JASMINE = False diff --git a/cms/envs/dev.py b/cms/envs/dev.py index 5612db1396..524983ed91 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -143,3 +143,6 @@ DEBUG_TOOLBAR_CONFIG = { # To see stacktraces for MongoDB queries, set this to True. # Stacktraces slow down page loads drastically (for pages with lots of queries). DEBUG_TOOLBAR_MONGO_STACKTRACES = False + +# disable NPS survey in dev mode +MITX_FEATURES['STUDIO_NPS_SURVEY'] = False diff --git a/cms/templates/base.html b/cms/templates/base.html index 30c7045440..d8f0747569 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -23,18 +23,6 @@ - - - - - - - <%block name="header_extras"> @@ -70,6 +58,8 @@ <%block name="jsextra"> + + <%include file="widgets/qualaroo.html" /> diff --git a/cms/templates/widgets/qualaroo.html b/cms/templates/widgets/qualaroo.html new file mode 100644 index 0000000000..04d10e08d1 --- /dev/null +++ b/cms/templates/widgets/qualaroo.html @@ -0,0 +1,13 @@ +% if settings.MITX_FEATURES.get('STUDIO_NPS_SURVEY'): + + + + + + +% endif From 5df7f41c8f3aa698d80d086cf2fd996f57b6607f Mon Sep 17 00:00:00 2001 From: "Mark L. Chang" Date: Wed, 27 Mar 2013 22:36:18 -0400 Subject: [PATCH 13/16] added net promoter score tracking javascript shim from Qualaroo --- cms/templates/base.html | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cms/templates/base.html b/cms/templates/base.html index e852b5d7fe..30c7045440 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -23,6 +23,18 @@ + + + + + + + <%block name="header_extras"> From 43f6f0ce8f9d704d9a4c970778ad8f02bec05c85 Mon Sep 17 00:00:00 2001 From: "Mark L. Chang" Date: Wed, 3 Apr 2013 00:26:12 -0400 Subject: [PATCH 14/16] put survey behind MITX_FEATURES flag --- cms/envs/common.py | 1 + cms/envs/dev.py | 3 +++ cms/templates/base.html | 14 ++------------ cms/templates/widgets/qualaroo.html | 13 +++++++++++++ 4 files changed, 19 insertions(+), 12 deletions(-) create mode 100644 cms/templates/widgets/qualaroo.html diff --git a/cms/envs/common.py b/cms/envs/common.py index 12fa09947a..5ad9068636 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -34,6 +34,7 @@ MITX_FEATURES = { 'ENABLE_DISCUSSION_SERVICE': False, 'AUTH_USE_MIT_CERTIFICATES': False, 'STUB_VIDEO_FOR_TESTING': False, # do not display video when running automated acceptance tests + 'STUDIO_NPS_SURVEY': True, } ENABLE_JASMINE = False diff --git a/cms/envs/dev.py b/cms/envs/dev.py index 5612db1396..524983ed91 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -143,3 +143,6 @@ DEBUG_TOOLBAR_CONFIG = { # To see stacktraces for MongoDB queries, set this to True. # Stacktraces slow down page loads drastically (for pages with lots of queries). DEBUG_TOOLBAR_MONGO_STACKTRACES = False + +# disable NPS survey in dev mode +MITX_FEATURES['STUDIO_NPS_SURVEY'] = False diff --git a/cms/templates/base.html b/cms/templates/base.html index 30c7045440..d8f0747569 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -23,18 +23,6 @@ - - - - - - - <%block name="header_extras"> @@ -70,6 +58,8 @@ <%block name="jsextra"> + + <%include file="widgets/qualaroo.html" /> diff --git a/cms/templates/widgets/qualaroo.html b/cms/templates/widgets/qualaroo.html new file mode 100644 index 0000000000..04d10e08d1 --- /dev/null +++ b/cms/templates/widgets/qualaroo.html @@ -0,0 +1,13 @@ +% if settings.MITX_FEATURES.get('STUDIO_NPS_SURVEY'): + + + + + + +% endif From c66eabb3083eb960923205e27af1213666d2b60a Mon Sep 17 00:00:00 2001 From: Vasyl Nakvasiuk Date: Tue, 2 Apr 2013 10:28:14 +0300 Subject: [PATCH 15/16] fix caption_asset_path for videoalpha --- common/lib/xmodule/xmodule/videoalpha_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 5ae8d890e6..a88c906b9c 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -131,7 +131,7 @@ class VideoAlphaModule(VideoAlphaFields, XModule): else: # VS[compat] # cdodge: filesystem static content support. - caption_asset_path = "/static/{0}/subs/".format(getattr(self, 'data_dir', None)) + caption_asset_path = "/static/subs/" return self.system.render_template('videoalpha.html', { 'youtube_streams': self.youtube_streams, From b11f0e17f74a94411c00f169b89f70a2a8029aa1 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Wed, 3 Apr 2013 10:22:43 -0400 Subject: [PATCH 16/16] respond to feedback --- .../management/commands/remove_input_state.py | 58 +++++-------------- 1 file changed, 16 insertions(+), 42 deletions(-) diff --git a/lms/djangoapps/courseware/management/commands/remove_input_state.py b/lms/djangoapps/courseware/management/commands/remove_input_state.py index 772758a33b..9adabeafc9 100644 --- a/lms/djangoapps/courseware/management/commands/remove_input_state.py +++ b/lms/djangoapps/courseware/management/commands/remove_input_state.py @@ -21,7 +21,7 @@ class Command(BaseCommand): The fix here is to remove the "input_state" entry in the StudentModule objects of any problems that contain them. No problem is yet making use of this, and the code should do the right thing if it's missing (by recreating an empty dict for its value). - + To narrow down the set of problems that might need fixing, the StudentModule objects to be checked is filtered down to those: @@ -30,15 +30,15 @@ class Command(BaseCommand): modified > '2013-03-28 22:00:00' (the problem must have been visited after the bug was introduced on Prod and Edge) state like '%input_state%' (the problem must have "input_state" set). - - This filtering is done on the production database replica, so that the larger select queries don't lock + + This filtering is done on the production database replica, so that the larger select queries don't lock the real production database. The list of id values for Student Modules is written to a file, and the file is passed into this command. The sql file passed to mysql contains: - - select sm.id from courseware_studentmodule sm - where sm.modified > "2013-03-28 22:00:00" - and sm.created < "2013-03-29 16:30:00" - and sm.state like "%input_state%" + + select sm.id from courseware_studentmodule sm + where sm.modified > "2013-03-28 22:00:00" + and sm.created < "2013-03-29 16:30:00" + and sm.state like "%input_state%" and sm.module_type = 'problem'; ''' @@ -53,56 +53,29 @@ class Command(BaseCommand): action='store_true', dest='save_changes', default=False, - help='Persist the changes that were encountered. If not set, no changes are saved.'), - make_option('--idlist', - action='store', - dest='idlist_path', - help='Path containing id values of StudentModule objects to clean (one per line).'), + help='Persist the changes that were encountered. If not set, no changes are saved.'), ) -# DON'T DO THIS: -# def fix_studentmodules(self, save_changes): -# '''Identify the list of StudentModule objects that might need fixing, and then fix each one''' -# modules = StudentModule.objects.filter(modified__gt='2013-03-28 22:00:00', -# created__lt='2013-03-29 16:30:00', -# state__contains='input_state') -# -# for module in modules: -# self.remove_studentmodule_input_state(module, save_changes) -# -# LOG.info("Finished student modules: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) -# -# hist_modules = StudentModuleHistory.objects.filter(created__gt='2013-03-28 22:00:00', -# created__lt='2013-03-29 16:30:00', -# state__contains='input_state') -# -# for hist_module in hist_modules: -# self.remove_studentmodulehistory_input_state(hist_module, save_changes) -# -# LOG.info("Finished student history modules: updating {0} of {1} modules".format(self.num_hist_changed, self.num_hist_visited)) - def fix_studentmodules_in_list(self, save_changes, idlist_path): '''Read in the list of StudentModule objects that might need fixing, and then fix each one''' - + # open file and read id values from it: for line in open(idlist_path, 'r'): student_module_id = line.strip() + # skip the header, if present: if student_module_id == 'id': continue try: module = StudentModule.objects.get(id=student_module_id) - except: + except StudentModule.DoesNotExist: LOG.error("Unable to find student module with id = {0}: skipping... ".format(student_module_id)) continue self.remove_studentmodule_input_state(module, save_changes) - hist_modules = StudentModuleHistory.objects.filter(student_module_id = student_module_id) + hist_modules = StudentModuleHistory.objects.filter(student_module_id=student_module_id) for hist_module in hist_modules: self.remove_studentmodulehistory_input_state(hist_module, save_changes) - LOG.info("Finished student modules: updating {0} of {1} modules".format(self.num_changed, self.num_visited)) - LOG.info("Finished student history modules: updating {0} of {1} modules".format(self.num_hist_changed, self.num_hist_visited)) - @transaction.autocommit def remove_studentmodule_input_state(self, module, save_changes): ''' Fix the grade assigned to a StudentModule''' @@ -164,6 +137,7 @@ class Command(BaseCommand): LOG.info("Starting run: reading from idlist file {0}; save_changes = {1}".format(idlist_path, save_changes)) self.fix_studentmodules_in_list(save_changes, idlist_path) - + LOG.info("Finished run: updating {0} of {1} student modules".format(self.num_changed, self.num_visited)) - LOG.info("Finished run: updating {0} of {1} student history modules".format(self.num_hist_changed, self.num_hist_visited)) + LOG.info("Finished run: updating {0} of {1} student history modules".format(self.num_hist_changed, + self.num_hist_visited))