diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py index d7ae927d..5bc795ab 100644 --- a/problem_builder/instructor_tool.py +++ b/problem_builder/instructor_tool.py @@ -23,14 +23,17 @@ All processing is done offline. """ import json +from django.core.paginator import Paginator from xblock.core import XBlock from xblock.exceptions import JsonHandlerError -from xblock.fields import Scope, String, Dict +from xblock.fields import Scope, String, Dict, List from xblock.fragment import Fragment from xblockutils.resources import ResourceLoader loader = ResourceLoader(__name__) +PAGE_SIZE = 15 + # Make '_' a no-op so we can scrape strings def _(text): @@ -63,6 +66,12 @@ class InstructorToolBlock(XBlock): default=None, scope=Scope.user_state, ) + display_data = List( + # The list of results associated with the most recent successful export. + # Stored separately to avoid the overhead of sending it to the client. + default=None, + scope=Scope.user_state, + ) has_author_view = True @property @@ -75,6 +84,11 @@ def author_view(self, context=None): # different celery queues; our task listener is waiting for tasks on the LMS queue) return Fragment(u'
Instructor Tool Block
This block only works from the LMS.
') + def studio_view(self, context=None): + """ View for editing Instructor Tool block in Studio. """ + # Display friendly message explaining that the block is not editable. + return Fragment(u'This is a preconfigured block. It is not editable.
') + def check_pending_export(self): """ If we're waiting for an export, see if it has finished, and if so, get the result. @@ -90,11 +104,26 @@ def _save_result(self, task_result): self.active_export_task_id = '' if task_result.successful(): if isinstance(task_result.result, dict) and not task_result.result.get('error'): + self.display_data = task_result.result['display_data'] + del task_result.result['display_data'] self.last_export_result = task_result.result else: self.last_export_result = {'error': u'Unexpected result: {}'.format(repr(task_result.result))} + self.display_data = None else: self.last_export_result = {'error': unicode(task_result.result)} + self.display_data = None + + @XBlock.json_handler + def get_result_page(self, data, suffix=''): + """ Return requested page of `last_export_result`. """ + paginator = Paginator(self.display_data, PAGE_SIZE) + page = data.get('page', None) + return { + 'display_data': paginator.page(page).object_list, + 'num_results': len(self.display_data), + 'page_size': PAGE_SIZE + } def student_view(self, context=None): """ Normal View """ @@ -105,9 +134,87 @@ def student_view(self, context=None): _('Rating Question'): 'RatingBlock', _('Long Answer'): 'AnswerBlock', } + eligible_block_types = ('pb-mcq', 'pb-rating', 'pb-answer') + flat_block_tree = [] + + def get_block_id(block): + """ + Return ID of `block`, taking into account needs of both LMS/CMS and workbench runtimes. + """ + usage_id = block.scope_ids.usage_id + # Try accessing block ID. If usage_id does not have it, return usage_id itself + return unicode(getattr(usage_id, 'block_id', usage_id)) + + def get_block_name(block): + """ + Return name of `block`. + + Try attributes in the following order: + - block.question + - block.name (fallback for old courses) + - block.display_name + - block ID + """ + for attribute in ('question', 'name', 'display_name'): + if getattr(block, attribute, None): + return getattr(block, attribute, None) + return get_block_id(block) + + def get_block_type(block): + """ + Return type of `block`, taking into account different key styles that might be in use. + """ + try: + block_type = block.runtime.id_reader.get_block_type(block.scope_ids.def_id) + except AttributeError: + block_type = block.runtime.id_reader.get_block_type(block.scope_ids.usage_id) + return block_type + + def build_tree(block, ancestors): + """ + Build up a tree of information about the XBlocks descending from root_block + """ + block_id = get_block_id(block) + block_name = get_block_name(block) + block_type = get_block_type(block) + if block_type != 'pb-choice': + eligible = block_type in eligible_block_types + if eligible: + # If this block is a question whose answers we can export, + # we mark all of its ancestors as exportable too + if ancestors and not ancestors[-1]["eligible"]: + for ancestor in ancestors: + ancestor["eligible"] = True + + new_entry = { + "depth": len(ancestors), + "id": block_id, + "name": block_name, + "eligible": eligible, + } + flat_block_tree.append(new_entry) + if block.has_children and not getattr(block, "has_dynamic_children", lambda: False)(): + for child_id in block.children: + build_tree(block.runtime.get_block(child_id), ancestors=(ancestors + [new_entry])) + + root_block = self + while root_block.parent: + root_block = root_block.get_parent() + root_block_id = get_block_id(root_block) + root_entry = { + "depth": 0, + "id": root_block_id, + "name": "All", + } + flat_block_tree.append(root_entry) + + for child_id in root_block.children: + child_block = root_block.runtime.get_block(child_id) + build_tree(child_block, [root_entry]) + html = loader.render_template( 'templates/html/instructor_tool.html', - {'block_choices': block_choices} + {'block_choices': block_choices, 'block_tree': flat_block_tree} ) fragment = Fragment(html) fragment.add_css_url(self.runtime.local_resource_url(self, 'public/css/instructor_tool.css')) @@ -144,6 +251,7 @@ def raise_error(self, code, message): self.last_export_result = { 'error': message, } + self.display_data = None raise JsonHandlerError(code, message) @XBlock.json_handler @@ -157,6 +265,7 @@ def delete_export(self, data, suffix=''): def _delete_export(self): self.last_export_result = None + self.display_data = None self.active_export_task_id = '' @XBlock.json_handler @@ -187,9 +296,6 @@ def start_export(self, data, suffix=''): root_block_id = self.scope_ids.usage_id # Block ID not in workbench runtime. root_block_id = unicode(getattr(root_block_id, 'block_id', root_block_id)) - get_root = True - else: - get_root = False # Launch task from .tasks import export_data as export_data_task # Import here since this is edX LMS specific @@ -203,7 +309,6 @@ def start_export(self, data, suffix=''): block_types, user_id, match_string, - get_root=get_root ) if async_result.ready(): # In development mode, the task may have executed synchronously. diff --git a/problem_builder/public/css/instructor_tool.css b/problem_builder/public/css/instructor_tool.css index 007e900d..60d576e4 100644 --- a/problem_builder/public/css/instructor_tool.css +++ b/problem_builder/public/css/instructor_tool.css @@ -25,6 +25,12 @@ display: table-cell; padding-left: 1em; } +.data-export-field-container { + width: 43%; +} +.data-export-options .data-export-actions { + max-width: 10%; +} .data-export-field { margin-top: .5em; margin-bottom: .5em; @@ -34,7 +40,7 @@ vertical-align: middle; } .data-export-field input, .data-export-field select { - max-width: 60%; + width: 55%; float: right; } .data-export-results, .data-export-download, .data-export-cancel, .data-export-delete { diff --git a/problem_builder/public/js/instructor_tool.js b/problem_builder/public/js/instructor_tool.js index 7d4ebe5e..c1e9c8a7 100644 --- a/problem_builder/public/js/instructor_tool.js +++ b/problem_builder/public/js/instructor_tool.js @@ -4,6 +4,12 @@ function InstructorToolBlock(runtime, element) { // Pagination + $(document).ajaxSend(function(event, jqxhr, options) { + if (options.url.indexOf('get_result_page') !== -1) { + options.data = JSON.stringify(options.data); + } + }); + var Result = Backbone.Model.extend({ initialize: function(attrs, options) { @@ -18,12 +24,54 @@ function InstructorToolBlock(runtime, element) { model: Result, - getCurrentPage: function(returnObject) { - var currentPage = this.state.currentPage; - if (returnObject) { - return this.getPage(currentPage); - } - return currentPage; + state: { + order: 0 + }, + + url: runtime.handlerUrl(element, 'get_result_page'), + + parseState: function(response) { + return { + totalRecords: response.num_results, + pageSize: response.page_size + }; + }, + + parseRecords: function(response) { + return _.map(response.display_data, function(row) { + return new Result(null, { values: row }); + }); + }, + + fetchOptions: { + reset: true, + type: 'POST', + contentType: 'application/json', + processData: false + }, + + getFirstPage: function() { + Backbone.PageableCollection.prototype + .getFirstPage.call(this, this.fetchOptions); + }, + + getPreviousPage: function() { + Backbone.PageableCollection.prototype + .getPreviousPage.call(this, this.fetchOptions); + }, + + getNextPage: function() { + Backbone.PageableCollection.prototype + .getNextPage.call(this, this.fetchOptions); + }, + + getLastPage: function() { + Backbone.PageableCollection.prototype + .getLastPage.call(this, this.fetchOptions); + }, + + getCurrentPage: function() { + return this.state.currentPage; }, getTotalPages: function() { @@ -34,17 +82,26 @@ function InstructorToolBlock(runtime, element) { var ResultsView = Backbone.View.extend({ + initialize: function() { + this.listenTo(this.collection, 'reset', this.render); + this.listenTo(this, 'rendered', this._show); + this.listenTo(this, 'processing', this._hide); + this.listenTo(this, 'error', this._hide); + this.listenTo(this, 'update', this._updateInfo); + }, + render: function() { - this._insertRecords(this.collection.getCurrentPage(true)); + this._insertRecords(); this._updateControls(); this.$('#total-pages').text(this.collection.getTotalPages() || 0); + this.trigger('rendered'); return this; }, - _insertRecords: function(records) { + _insertRecords: function() { var tbody = this.$('tbody'); tbody.empty(); - records.each(function(result, index) { + this.collection.each(function(result, index) { var row = $('').text(info)); + }, + events: { 'click #first-page': '_firstPage', 'click #prev-page': '_prevPage', @@ -66,26 +137,26 @@ function InstructorToolBlock(runtime, element) { }, _firstPage: function() { - this._insertRecords(this.collection.getFirstPage()); + this.collection.getFirstPage(); this._updateControls(); }, _prevPage: function() { if (this.collection.hasPreviousPage()) { - this._insertRecords(this.collection.getPreviousPage()); + this.collection.getPreviousPage(); } this._updateControls(); }, _nextPage: function() { if (this.collection.hasNextPage()) { - this._insertRecords(this.collection.getNextPage()); + this.collection.getNextPage(); } this._updateControls(); }, _lastPage: function() { - this._insertRecords(this.collection.getLastPage()); + this.collection.getLastPage(); this._updateControls(); }, @@ -107,10 +178,42 @@ function InstructorToolBlock(runtime, element) { }); var resultsView = new ResultsView({ - collection: new Results([], { mode: "client", state: { pageSize: 15 } }), + collection: new Results([]), el: $element.find('#results') }); + // Status area + + var StatusView = Backbone.View.extend({ + + initialize: function() { + this.listenTo(this, 'processing', this._showSpinner); + this.listenTo(this, 'notify', this._displayMessage); + this.listenTo(this, 'stopped', this._empty); + this.listenTo(resultsView, 'rendered', this._empty); + }, + + _showSpinner: function() { + this.$el.empty(); + this.$el.append( + $('').addClass('icon fa fa-spinner fa-spin') + ).css('text-align', 'center'); + }, + + _displayMessage: function(message) { + this.$el.append($('
').text(message)); + }, + + _empty: function() { + this.$el.empty(); + } + + }); + + var statusView = new StatusView({ + el: $element.find('.data-export-status') + }); + // Set up gettext in case it isn't available in the client runtime: if (typeof gettext == "undefined") { window.gettext = function gettext_stub(string) { return string; }; @@ -121,7 +224,7 @@ function InstructorToolBlock(runtime, element) { var $downloadButton = $element.find('.data-export-download'); var $deleteButton = $element.find('.data-export-delete'); var $blockTypes = $element.find("select[name='block_types']"); - var $rootBlockId = $element.find("input[name='root_block_id']"); + var $rootBlockId = $element.find("select[name='root_block_id']"); var $username = $element.find("input[name='username']"); var $matchString = $element.find("input[name='match_string']"); var $resultTable = $element.find('.data-export-results'); @@ -147,18 +250,15 @@ function InstructorToolBlock(runtime, element) { if (statusChanged) updateView(); } - function showSpinner() { + function disableActions() { $startButton.prop('disabled', true); $cancelButton.prop('disabled', true); $downloadButton.prop('disabled', true); $deleteButton.prop('disabled', true); - $('.data-export-status', $element).empty().append( - $('').addClass('icon fa fa-spinner fa-spin') - ).css("text-align", "center"); } - function hideResults() { - $resultTable.hide(); + function showInfo(info) { + resultsView.trigger('update', info); } function showResults() { @@ -167,6 +267,22 @@ function InstructorToolBlock(runtime, element) { } } + function hideResults() { + resultsView.trigger('processing'); + } + + function showSpinner() { + statusView.trigger('processing'); + } + + function hideSpinner() { + statusView.trigger('stopped'); + } + + function showStatusMessage(message) { + statusView.trigger('notify', message); + } + function handleError(data) { // Shim to make the XBlock JsonHandlerError response work with our format. status = {'last_export_result': JSON.parse(data.responseText), 'export_pending': false}; @@ -174,26 +290,22 @@ function InstructorToolBlock(runtime, element) { } function updateView() { - var $exportInfo = $('.data-export-info', $element), - $statusArea = $('.data-export-status', $element), startTime; - $statusArea.empty(); - $exportInfo.empty(); + var startTime; $startButton.toggle(!status.export_pending).prop('disabled', false); $cancelButton.toggle(status.export_pending).prop('disabled', false); $downloadButton.toggle(Boolean(status.download_url)).prop('disabled', false); $deleteButton.toggle(Boolean(status.last_export_result)).prop('disabled', false); if (status.last_export_result) { if (status.last_export_result.error) { - $statusArea.append($('
').text( - _.template( - gettext('Data export failed. Reason: <%= error %>'), - {'error': status.last_export_result.error} - ) - )); hideResults(); + hideSpinner(); + showStatusMessage(_.template( + gettext('Data export failed. Reason: <%= error %>'), + {'error': status.last_export_result.error} + )); } else { startTime = new Date(status.last_export_result.start_timestamp * 1000); - $exportInfo.append($('
').text( + showInfo( _.template( ngettext( 'Results retrieved on <%= creation_time %> (<%= seconds %> second).', @@ -204,24 +316,14 @@ function InstructorToolBlock(runtime, element) { 'creation_time': startTime.toString(), 'seconds': status.last_export_result.generation_time_s.toFixed(1) } - ) - )); - - // Display results - var results = _.map(status.last_export_result.display_data, function(row) { - return new Result(null, { values: row }); - }); - - resultsView.collection.fullCollection.reset(results); - resultsView.render(); - - showResults(); + )); + resultsView.collection.getFirstPage(); } } else { if (status.export_pending) { - $statusArea.append($('
').text( - gettext('The report is currently being generated…') - )); + showStatusMessage(gettext('The report is currently being generated…')); + } else { + hideSpinner(); } } } @@ -249,6 +351,7 @@ function InstructorToolBlock(runtime, element) { dataType: 'json' }); showSpinner(); + disableActions(); }); } @@ -265,6 +368,7 @@ function InstructorToolBlock(runtime, element) { }); showSpinner(); + disableActions(); getStatus(); } diff --git a/problem_builder/tasks.py b/problem_builder/tasks.py index 75a056de..5a3380d8 100644 --- a/problem_builder/tasks.py +++ b/problem_builder/tasks.py @@ -6,7 +6,6 @@ from celery.task import task from celery.utils.log import get_task_logger from instructor_task.models import ReportStore -from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from student.models import user_by_anonymous_id from xmodule.modulestore.django import modulestore @@ -21,7 +20,7 @@ @task() -def export_data(course_id, source_block_id_str, block_types, user_id, match_string, get_root=True): +def export_data(course_id, source_block_id_str, block_types, user_id, match_string): """ Exports student answers to all MCQ questions to a CSV file. """ @@ -31,18 +30,10 @@ def export_data(course_id, source_block_id_str, block_types, user_id, match_stri try: course_key = CourseKey.from_string(course_id) src_block = modulestore().get_items(course_key, qualifiers={'name': source_block_id_str}, depth=0)[0] - if src_block is None: - raise InvalidKeyError - except InvalidKeyError: + except IndexError: raise ValueError("Could not find the specified Block ID.") course_key_str = unicode(course_key) - root = src_block - if get_root: - # Get the root block for the course. - while root.parent: - root = root.get_parent() - type_map = {cls.__name__: cls for cls in [MCQBlock, RatingBlock, AnswerBlock]} if not block_types: @@ -65,7 +56,7 @@ def scan_for_blocks(block): # Blocks may refer to missing children. Don't break in this case. pass - scan_for_blocks(root) + scan_for_blocks(src_block) # Define the header row of our CSV: rows = [] @@ -106,7 +97,7 @@ def _extract_data(course_key_str, block, user_id, match_string): block_type = _get_type(block) # Extract info for "Question" column - block_question = block.question + block_question = _get_question(block) # Extract info for "Answer" and "Username" columns # - Get all of the most recent student submissions for this block: @@ -150,6 +141,13 @@ def _get_type(block): return block.scope_ids.block_type +def _get_question(block): + """ + Return question for `block`; default to question ID if `question` is not set. + """ + return block.question or block.name + + def _get_submissions(course_key_str, block, user_id): """ Return submissions for `block`. @@ -158,6 +156,8 @@ def _get_submissions(course_key_str, block, user_id): # Note this requires one giant query that retrieves all student submissions for `block` at once. block_id = unicode(block.scope_ids.usage_id.replace(branch=None, version_guid=None)) block_type = _get_type(block) + if block_type == 'pb-answer': + block_id = block.name # item_id of Long Answer submission matches question ID and not block ID if not user_id: return sub_api.get_all_submissions(course_key_str, block_id, block_type) else: diff --git a/problem_builder/templates/html/instructor_tool.html b/problem_builder/templates/html/instructor_tool.html index 95bb4178..8b579dc5 100644 --- a/problem_builder/templates/html/instructor_tool.html +++ b/problem_builder/templates/html/instructor_tool.html @@ -27,8 +27,16 @@