From 5be4fa305cd20094db142c3aab6b5bf50f07c757 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Tue, 14 Jul 2015 14:58:12 +0200
Subject: [PATCH 01/21] Catch IndexError that gets thrown when user provides
 invalid root block ID.

---
 problem_builder/tasks.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/problem_builder/tasks.py b/problem_builder/tasks.py
index 75a056de..d6c3b74b 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
@@ -31,9 +30,7 @@ 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)
 

From 3fc5716301883e0f2f64a1c1d0e92faa8c682fc5 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Tue, 14 Jul 2015 22:16:20 +0200
Subject: [PATCH 02/21] Implement client-server communication for pagination.

---
 problem_builder/instructor_tool.py            | 27 +++++-
 problem_builder/public/js/instructor_tool.js  | 83 ++++++++++++++-----
 .../tests/integration/test_instructor_tool.py |  6 +-
 3 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index d7ae927d..9dcdfe54 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -23,14 +23,16 @@
 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 +65,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
@@ -90,11 +98,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 """
@@ -144,6 +167,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 +181,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
diff --git a/problem_builder/public/js/instructor_tool.js b/problem_builder/public/js/instructor_tool.js
index 7d4ebe5e..ed93ed8e 100644
--- a/problem_builder/public/js/instructor_tool.js
+++ b/problem_builder/public/js/instructor_tool.js
@@ -18,12 +18,57 @@ function InstructorToolBlock(runtime, element) {
 
         model: Result,
 
-        getCurrentPage: function(returnObject) {
-            var currentPage = this.state.currentPage;
-            if (returnObject) {
-                return this.getPage(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,
+            beforeSend: function(jqXHR, options) {
+                options.data = JSON.stringify(options.data);
             }
-            return currentPage;
+        },
+
+        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 +79,21 @@ function InstructorToolBlock(runtime, element) {
 
     var ResultsView = Backbone.View.extend({
 
+        initialize: function() {
+            this.listenTo(this.collection, 'reset', this.render);
+        },
+
         render: function() {
-            this._insertRecords(this.collection.getCurrentPage(true));
+            this._insertRecords();
             this._updateControls();
             this.$('#total-pages').text(this.collection.getTotalPages() || 0);
             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 = $('<tr>');
                 _.each(Result.properties, function(name) {
                     row.append($('<td>').text(result.get(name)));
@@ -66,26 +115,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,7 +156,7 @@ function InstructorToolBlock(runtime, element) {
     });
 
     var resultsView = new ResultsView({
-        collection: new Results([], { mode: "client", state: { pageSize: 15 } }),
+        collection: new Results([]),
         el: $element.find('#results')
     });
 
@@ -207,13 +256,7 @@ function InstructorToolBlock(runtime, element) {
                     )
                 ));
 
-                // 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();
+                resultsView.collection.getFirstPage();
 
                 showResults();
             }
diff --git a/problem_builder/tests/integration/test_instructor_tool.py b/problem_builder/tests/integration/test_instructor_tool.py
index 7431946b..a21f58ea 100644
--- a/problem_builder/tests/integration/test_instructor_tool.py
+++ b/problem_builder/tests/integration/test_instructor_tool.py
@@ -7,7 +7,7 @@
 from selenium.common.exceptions import NoSuchElementException
 from xblockutils.base_test import SeleniumXBlockTest
 
-from problem_builder.instructor_tool import InstructorToolBlock
+from problem_builder.instructor_tool import PAGE_SIZE, InstructorToolBlock
 
 
 class MockTasksModule(object):
@@ -199,7 +199,7 @@ def test_pagination_single_result(self):
             successful=True, display_data=[[
                 'Test section', 'Test subsection', 'Test unit',
                 'Test type', 'Test question', 'Test answer', 'Test username'
-            ] for _ in range(45)]),
+            ] for _ in range(PAGE_SIZE*3)]),
         'instructor_task': True,
         'instructor_task.models': MockInstructorTaskModelsModule(),
     })
@@ -224,7 +224,7 @@ def test_pagination_multiple_results(self):
                 'Test type', 'Test question', 'Test answer', 'Test username'
         ]:
             occurrences = re.findall(contents, result_block.text)
-            self.assertEqual(len(occurrences), 15)
+            self.assertEqual(len(occurrences), PAGE_SIZE)
 
         self.assertFalse(first_page_button.is_enabled())
         self.assertFalse(prev_page_button.is_enabled())

From 45bb18a626a0aef65b4d918ed4e8d1ea69c24ec8 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Wed, 15 Jul 2015 21:48:27 +0200
Subject: [PATCH 03/21] Turn "Root block ID" field into user-friendly dropdown.

---
 problem_builder/instructor_tool.py            | 51 +++++++++++++++++--
 .../public/css/instructor_tool.css            |  8 ++-
 problem_builder/public/js/instructor_tool.js  |  2 +-
 problem_builder/tasks.py                      | 10 +---
 .../templates/html/instructor_tool.html       | 12 ++++-
 5 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 9dcdfe54..9d06e2e8 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -34,6 +34,7 @@
 
 PAGE_SIZE = 15
 
+
 # Make '_' a no-op so we can scrape strings
 def _(text):
     return text
@@ -128,9 +129,53 @@ def student_view(self, context=None):
             _('Rating Question'): 'RatingBlock',
             _('Long Answer'): 'AnswerBlock',
         }
+        block_types = ('pb-mcq', 'pb-rating', 'pb-answer')
+        flat_block_tree = []
+
+        def build_tree(block, ancestors):
+            """
+            Build up a tree of information about the XBlocks descending from root_block
+            """
+            block_id = block.scope_ids.usage_id.block_id
+            block_name = getattr(block, "display_name", None)
+            block_type = block.runtime.id_reader.get_block_type(block.scope_ids.def_id)
+            if not block_name and block_type in block_types:
+                block_name = block.question
+            eligible = block_type in 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 block_type == 'pb-mcq' 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_entry = {
+            "depth": 0,
+            "id": root_block.scope_ids.usage_id.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'))
@@ -212,9 +257,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
@@ -228,7 +270,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..3db5c1d5 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 {
+    min-width: 45%;
+}
+.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%;
+    max-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 ed93ed8e..ba2fbbea 100644
--- a/problem_builder/public/js/instructor_tool.js
+++ b/problem_builder/public/js/instructor_tool.js
@@ -170,7 +170,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');
diff --git a/problem_builder/tasks.py b/problem_builder/tasks.py
index d6c3b74b..cfc91bba 100644
--- a/problem_builder/tasks.py
+++ b/problem_builder/tasks.py
@@ -20,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.
     """
@@ -34,12 +34,6 @@ def export_data(course_id, source_block_id_str, block_types, user_id, match_stri
         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:
@@ -62,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 = []
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 @@ <h3>{% trans "Filters" %}</h3>
     <div class="data-export-field-container">
       <div class="data-export-field">
         <label>
-          <span>{% trans "Root block ID:" %}</span>
-          <input type="text" name="root_block_id" />
+          <span>{% trans "Section/Question:" %}</span>
+          <select name="root_block_id">
+            {% for block in block_tree %}
+            <option value="{{ block.id }}"
+                    {% if not block.eligible %} disabled="disabled" {% endif %}>
+              {% for _ in ""|ljust:block.depth %}&nbsp;&nbsp;{% endfor %}
+              {{ block.name }}
+            </option>
+            {% endfor %}
+          </select>
         </label>
       </div>
     </div>

From 9b2a4a17abcd2dc7f34ec76a484f4f6a43cdc273 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Thu, 16 Jul 2015 17:09:32 +0200
Subject: [PATCH 04/21] Make sure spinner stays visible and result table stays
 hidden until results have been inserted into the DOM.

---
 problem_builder/public/js/instructor_tool.js | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/problem_builder/public/js/instructor_tool.js b/problem_builder/public/js/instructor_tool.js
index ba2fbbea..c9cfdc68 100644
--- a/problem_builder/public/js/instructor_tool.js
+++ b/problem_builder/public/js/instructor_tool.js
@@ -87,6 +87,8 @@ function InstructorToolBlock(runtime, element) {
             this._insertRecords();
             this._updateControls();
             this.$('#total-pages').text(this.collection.getTotalPages() || 0);
+            $('.data-export-status', $element).empty();
+            this.$el.show(700);
             return this;
         },
 
@@ -225,7 +227,6 @@ function InstructorToolBlock(runtime, element) {
     function updateView() {
         var $exportInfo = $('.data-export-info', $element),
             $statusArea = $('.data-export-status', $element), startTime;
-        $statusArea.empty();
         $exportInfo.empty();
         $startButton.toggle(!status.export_pending).prop('disabled', false);
         $cancelButton.toggle(status.export_pending).prop('disabled', false);
@@ -255,10 +256,7 @@ function InstructorToolBlock(runtime, element) {
                         }
                     )
                 ));
-
                 resultsView.collection.getFirstPage();
-
-                showResults();
             }
         } else {
             if (status.export_pending) {

From 6fcbba2603c7b93f703cf592a79c28729d62bb1d Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Thu, 16 Jul 2015 18:47:00 +0200
Subject: [PATCH 05/21] Make dropdown work in workbench.

---
 problem_builder/instructor_tool.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 9d06e2e8..84d42463 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -136,7 +136,9 @@ def build_tree(block, ancestors):
             """
             Build up a tree of information about the XBlocks descending from root_block
             """
-            block_id = block.scope_ids.usage_id.block_id
+            block_id = block.scope_ids.usage_id
+            # Block ID not in workbench runtime.
+            block_id = unicode(getattr(block_id, 'block_id', block_id))
             block_name = getattr(block, "display_name", None)
             block_type = block.runtime.id_reader.get_block_type(block.scope_ids.def_id)
             if not block_name and block_type in block_types:
@@ -163,12 +165,16 @@ def build_tree(block, ancestors):
         root_block = self
         while root_block.parent:
             root_block = root_block.get_parent()
+        root_block_id = root_block.scope_ids.usage_id
+        # Block ID not in workbench runtime.
+        root_block_id = unicode(getattr(root_block_id, 'block_id', root_block_id))
         root_entry = {
             "depth": 0,
-            "id": root_block.scope_ids.usage_id.block_id,
+            "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])

From 75994ad0c455369b04ddd2813b3e136e6b59b98a Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Thu, 16 Jul 2015 19:28:08 +0200
Subject: [PATCH 06/21] Fix failing tests.

---
 problem_builder/tests/integration/test_instructor_tool.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/problem_builder/tests/integration/test_instructor_tool.py b/problem_builder/tests/integration/test_instructor_tool.py
index a21f58ea..75363fc7 100644
--- a/problem_builder/tests/integration/test_instructor_tool.py
+++ b/problem_builder/tests/integration/test_instructor_tool.py
@@ -146,6 +146,7 @@ def test_pagination_no_results(self):
         start_button.click()
 
         self.wait_until_visible(result_block)
+        time.sleep(1)  # Allow some time for result block to fully fade in
 
         self.assertFalse(first_page_button.is_enabled())
         self.assertFalse(prev_page_button.is_enabled())
@@ -179,6 +180,7 @@ def test_pagination_single_result(self):
         start_button.click()
 
         self.wait_until_visible(result_block)
+        time.sleep(1)  # Allow some time for result block to fully fade in
 
         for contents in [
                 'Test section', 'Test subsection', 'Test unit',
@@ -218,6 +220,7 @@ def test_pagination_multiple_results(self):
         start_button.click()
 
         self.wait_until_visible(result_block)
+        time.sleep(1)  # Allow some time for result block to fully fade in
 
         for contents in [
                 'Test section', 'Test subsection', 'Test unit',

From 6e6f31cd56c7984d9a3f56e4f34e82b78f4d3560 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Thu, 16 Jul 2015 23:10:28 +0200
Subject: [PATCH 07/21] Default to question ID (name) if question block does
 not have a "question" attribute.

---
 problem_builder/instructor_tool.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 84d42463..94360266 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -142,7 +142,7 @@ def build_tree(block, ancestors):
             block_name = getattr(block, "display_name", None)
             block_type = block.runtime.id_reader.get_block_type(block.scope_ids.def_id)
             if not block_name and block_type in block_types:
-                block_name = block.question
+                block_name = getattr(block, 'question', block.name)
             eligible = block_type in block_types
             if eligible:
                 # If this block is a question whose answers we can export,

From a2eb5c7b41ee980843c56ae578846efc317d607f Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Thu, 16 Jul 2015 23:50:38 +0200
Subject: [PATCH 08/21] Refactor to create better separation of concerns in
 client-side code.

For example, ResultView should not manipulate the status area (but it
should maintain meta info about result sets).
---
 problem_builder/public/js/instructor_tool.js | 106 +++++++++++++++----
 1 file changed, 83 insertions(+), 23 deletions(-)

diff --git a/problem_builder/public/js/instructor_tool.js b/problem_builder/public/js/instructor_tool.js
index c9cfdc68..d1eeb6c8 100644
--- a/problem_builder/public/js/instructor_tool.js
+++ b/problem_builder/public/js/instructor_tool.js
@@ -81,14 +81,17 @@ function InstructorToolBlock(runtime, element) {
 
         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._updateControls();
             this.$('#total-pages').text(this.collection.getTotalPages() || 0);
-            $('.data-export-status', $element).empty();
-            this.$el.show(700);
+            this.trigger('rendered');
             return this;
         },
 
@@ -109,6 +112,20 @@ function InstructorToolBlock(runtime, element) {
             }
         },
 
+        _show: function() {
+            this.$el.show(700);
+        },
+
+        _hide: function() {
+            this.$el.hide();
+        },
+
+        _updateInfo: function(info) {
+            var $exportInfo = this.$('.data-export-info');
+            $exportInfo.empty();
+            $exportInfo.append($('<p>').text(info));
+        },
+
         events: {
             'click #first-page': '_firstPage',
             'click #prev-page': '_prevPage',
@@ -162,6 +179,38 @@ function InstructorToolBlock(runtime, element) {
         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(
+                $('<i>').addClass('icon fa fa-spinner fa-spin')
+            ).css('text-align', 'center');
+        },
+
+        _displayMessage: function(message) {
+            this.$el.append($('<p>').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; };
@@ -198,18 +247,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(
-            $('<i>').addClass('icon fa fa-spinner fa-spin')
-        ).css("text-align", "center");
     }
 
-    function hideResults() {
-        $resultTable.hide();
+    function showInfo(info) {
+        resultsView.trigger('update', info);
     }
 
     function showResults() {
@@ -218,6 +264,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};
@@ -225,25 +287,22 @@ function InstructorToolBlock(runtime, element) {
     }
 
     function updateView() {
-        var $exportInfo = $('.data-export-info', $element),
-            $statusArea = $('.data-export-status', $element), startTime;
-        $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($('<p>').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($('<p>').text(
+                showInfo(
                     _.template(
                         ngettext(
                             'Results retrieved on <%= creation_time %> (<%= seconds %> second).',
@@ -254,15 +313,14 @@ function InstructorToolBlock(runtime, element) {
                             'creation_time': startTime.toString(),
                             'seconds': status.last_export_result.generation_time_s.toFixed(1)
                         }
-                    )
-                ));
+                    ));
                 resultsView.collection.getFirstPage();
             }
         } else {
             if (status.export_pending) {
-                $statusArea.append($('<p>').text(
-                    gettext('The report is currently being generated…')
-                ));
+                showStatusMessage(gettext('The report is currently being generated…'));
+            } else {
+                hideSpinner();
             }
         }
     }
@@ -290,6 +348,7 @@ function InstructorToolBlock(runtime, element) {
                 dataType: 'json'
             });
             showSpinner();
+            disableActions();
         });
     }
 
@@ -306,6 +365,7 @@ function InstructorToolBlock(runtime, element) {
     });
 
     showSpinner();
+    disableActions();
     getStatus();
 
 }

From 466b172b5a09002b946b87b2389dee29f17c1b66 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 01:07:49 +0200
Subject: [PATCH 09/21] Add integration test for deleting export results.

---
 .../tests/integration/test_instructor_tool.py | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/problem_builder/tests/integration/test_instructor_tool.py b/problem_builder/tests/integration/test_instructor_tool.py
index 75363fc7..f7f9b5f1 100644
--- a/problem_builder/tests/integration/test_instructor_tool.py
+++ b/problem_builder/tests/integration/test_instructor_tool.py
@@ -56,6 +56,36 @@ def test_students_dont_see_interface(self):
         data_export = self.go_to_view()
         self.assertIn('This interface can only be used by course staff.', data_export.text)
 
+    @patch.dict('sys.modules', {
+        'problem_builder.tasks': MockTasksModule(successful=True),
+        'instructor_task': True,
+        'instructor_task.models': MockInstructorTaskModelsModule(),
+    })
+    @patch.object(InstructorToolBlock, 'user_is_staff', Mock(return_value=True))
+    def test_data_export_delete(self):
+        instructor_tool = self.go_to_view()
+        start_button = instructor_tool.find_element_by_class_name('data-export-start')
+        result_block = instructor_tool.find_element_by_class_name('data-export-results')
+        status_area = instructor_tool.find_element_by_class_name('data-export-status')
+        download_button = instructor_tool.find_element_by_class_name('data-export-download')
+        cancel_button = instructor_tool.find_element_by_class_name('data-export-cancel')
+        delete_button = instructor_tool.find_element_by_class_name('data-export-delete')
+
+        start_button.click()
+
+        self.wait_until_visible(result_block)
+        self.wait_until_visible(delete_button)
+
+        delete_button.click()
+
+        self.wait_until_hidden(result_block)
+        self.wait_until_hidden(delete_button)
+
+        self.assertTrue(start_button.is_enabled())
+        self.assertEqual('', status_area.text)
+        self.assertFalse(download_button.is_displayed())
+        self.assertFalse(cancel_button.is_displayed())
+
     @patch.dict('sys.modules', {
         'problem_builder.tasks': MockTasksModule(successful=True),
         'instructor_task': True,

From 5822565513a9c8d77ed446b790844189eb7b444d Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 15:55:54 +0200
Subject: [PATCH 10/21] Make sure not to override global beforeSend handler.

---
 problem_builder/public/js/instructor_tool.js | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/problem_builder/public/js/instructor_tool.js b/problem_builder/public/js/instructor_tool.js
index d1eeb6c8..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) {
@@ -41,10 +47,7 @@ function InstructorToolBlock(runtime, element) {
             reset: true,
             type: 'POST',
             contentType: 'application/json',
-            processData: false,
-            beforeSend: function(jqXHR, options) {
-                options.data = JSON.stringify(options.data);
-            }
+            processData: false
         },
 
         getFirstPage: function() {

From 74e027cfa44fdb825b0c20bd24c9c55abef938fe Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 17:00:26 +0200
Subject: [PATCH 11/21] Get rid of "Unable to find view u'studio_view' on block
 InstructorToolBlockWithMixins" error that gets thrown when trying to edit
 Instructor Tool blocks in Studio.

---
 problem_builder/instructor_tool.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 94360266..0f770642 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -84,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'<p>Instructor Tool Block</p><p>This block only works from the LMS.</p>')
 
+    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'<p>This is a preconfigured block. It is not editable.</p>')
+
     def check_pending_export(self):
         """
         If we're waiting for an export, see if it has finished, and if so, get the result.

From a689bd2a716b9432929649b02d22a9be09afd0c6 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 18:13:00 +0200
Subject: [PATCH 12/21] Move code that computes block ID into separate method.

---
 problem_builder/instructor_tool.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 0f770642..c3d6afa8 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -137,13 +137,18 @@ def student_view(self, context=None):
         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 build_tree(block, ancestors):
             """
             Build up a tree of information about the XBlocks descending from root_block
             """
-            block_id = block.scope_ids.usage_id
-            # Block ID not in workbench runtime.
-            block_id = unicode(getattr(block_id, 'block_id', block_id))
             block_name = getattr(block, "display_name", None)
             block_type = block.runtime.id_reader.get_block_type(block.scope_ids.def_id)
             if not block_name and block_type in block_types:
@@ -157,7 +162,7 @@ def build_tree(block, ancestors):
                         ancestor["eligible"] = True
             new_entry = {
                 "depth": len(ancestors),
-                "id": block_id,
+                "id": get_block_id(block),
                 "name": block_name,
                 "eligible": eligible,
             }
@@ -170,9 +175,7 @@ def build_tree(block, ancestors):
         root_block = self
         while root_block.parent:
             root_block = root_block.get_parent()
-        root_block_id = root_block.scope_ids.usage_id
-        # Block ID not in workbench runtime.
-        root_block_id = unicode(getattr(root_block_id, 'block_id', root_block_id))
+        root_block_id = get_block_id(root_block)
         root_entry = {
             "depth": 0,
             "id": root_block_id,

From 69b0a808f06dc3f02f1e78da27c4c88a64cc1538 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 18:15:20 +0200
Subject: [PATCH 13/21] Take into account different key styles when computing
 block_type.

---
 problem_builder/instructor_tool.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index c3d6afa8..91f752fa 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -145,12 +145,22 @@ def get_block_id(block):
             # 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_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_name = getattr(block, "display_name", None)
-            block_type = block.runtime.id_reader.get_block_type(block.scope_ids.def_id)
+            block_type = get_block_type(block)
             if not block_name and block_type in block_types:
                 block_name = getattr(block, 'question', block.name)
             eligible = block_type in block_types

From 4cc0b2e6e701053dba2265b408635c8a74bc2dd8 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 18:46:13 +0200
Subject: [PATCH 14/21] Move code that computes names of blocks to separate
 method and make sure it works for non-eligible blocks with empty "Question
 title" (display_name).

---
 problem_builder/instructor_tool.py | 34 ++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 91f752fa..66e6398c 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -145,6 +145,33 @@ def get_block_id(block):
             # 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`.
+
+            - For MCQs, Ratings, Answer blocks this is one of:
+              - block.question
+              - block.name (fallback for old courses)
+            - For other types of (non-eligible) blocks this is one of:
+              - block.question
+              - block.display_name
+              - block ID (fallback if neither 'question' nor 'display_name' is suitable)
+            """
+            block_type = get_block_type(block)
+            # Eligible block (question)
+            if block_type in block_types:
+                return getattr(block, 'question', block.name)
+            # Non-eligible block (question or section/subsection/unit)
+            # - Try "question" attribute:
+            block_name = getattr(block, 'question', None)
+            if not block_name:
+                # - Try display_name:
+                block_name = getattr(block, 'display_name', None)
+            if not block_name:
+                # - Default to ID:
+                block_name = get_block_id(block)
+            return block_name
+
         def get_block_type(block):
             """
             Return type of `block`, taking into account different key styles that might be in use.
@@ -159,10 +186,9 @@ def build_tree(block, ancestors):
             """
             Build up a tree of information about the XBlocks descending from root_block
             """
-            block_name = getattr(block, "display_name", None)
+            block_id = get_block_id(block)
+            block_name = get_block_name(block)
             block_type = get_block_type(block)
-            if not block_name and block_type in block_types:
-                block_name = getattr(block, 'question', block.name)
             eligible = block_type in block_types
             if eligible:
                 # If this block is a question whose answers we can export,
@@ -172,7 +198,7 @@ def build_tree(block, ancestors):
                         ancestor["eligible"] = True
             new_entry = {
                 "depth": len(ancestors),
-                "id": get_block_id(block),
+                "id": block_id,
                 "name": block_name,
                 "eligible": eligible,
             }

From a856c87de16ee07b891a0c617145dbea44ff171e Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 20:58:45 +0200
Subject: [PATCH 15/21] Make sure dropdown excludes all choice fields (not just
 children of MRQs).

---
 problem_builder/instructor_tool.py | 37 +++++++++++++++---------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 66e6398c..1d97d4a3 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -189,24 +189,25 @@ def build_tree(block, ancestors):
             block_id = get_block_id(block)
             block_name = get_block_name(block)
             block_type = get_block_type(block)
-            eligible = block_type in 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 block_type == 'pb-mcq' 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]))
+            if not block_type == 'pb-choice':
+                eligible = block_type in 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:

From 418c55f754bde88bbfd89a4648152ebdc8c194b2 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 21:25:29 +0200
Subject: [PATCH 16/21] Make sure long block names can't break layout of export
 options.

---
 problem_builder/public/css/instructor_tool.css | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/problem_builder/public/css/instructor_tool.css b/problem_builder/public/css/instructor_tool.css
index 3db5c1d5..60d576e4 100644
--- a/problem_builder/public/css/instructor_tool.css
+++ b/problem_builder/public/css/instructor_tool.css
@@ -26,7 +26,7 @@
     padding-left: 1em;
 }
 .data-export-field-container {
-    min-width: 45%;
+    width: 43%;
 }
 .data-export-options .data-export-actions {
     max-width: 10%;
@@ -40,7 +40,7 @@
     vertical-align: middle;
 }
 .data-export-field input, .data-export-field select {
-    max-width: 55%;
+    width: 55%;
     float: right;
 }
 .data-export-results, .data-export-download, .data-export-cancel, .data-export-delete {

From 435382f56b9d748c4ca707337a746210aa9b70ce Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Fri, 17 Jul 2015 21:59:43 +0200
Subject: [PATCH 17/21] Handle cases where "Question title" is set but
 "question" isn't.

---
 problem_builder/instructor_tool.py | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 1d97d4a3..29c18f1a 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -149,21 +149,14 @@ def get_block_name(block):
             """
             Return name of `block`.
 
-            - For MCQs, Ratings, Answer blocks this is one of:
+            Try attributes in the following order:
               - block.question
               - block.name (fallback for old courses)
-            - For other types of (non-eligible) blocks this is one of:
-              - block.question
               - block.display_name
-              - block ID (fallback if neither 'question' nor 'display_name' is suitable)
+              - block ID
             """
-            block_type = get_block_type(block)
-            # Eligible block (question)
-            if block_type in block_types:
-                return getattr(block, 'question', block.name)
-            # Non-eligible block (question or section/subsection/unit)
             # - Try "question" attribute:
-            block_name = getattr(block, 'question', None)
+            block_name = getattr(block, 'question', block.name)
             if not block_name:
                 # - Try display_name:
                 block_name = getattr(block, 'display_name', None)

From 51d02cf695986832e89e8fc3733550ea1617ddbf Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Mon, 20 Jul 2015 22:31:51 +0200
Subject: [PATCH 18/21] Look up "Long Answer" submissions by question ID
 instead of block ID.

This ensures that export results correctly reflect changes to answers
associated with Long Answer blocks.
---
 problem_builder/tasks.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/problem_builder/tasks.py b/problem_builder/tasks.py
index cfc91bba..c5b504e4 100644
--- a/problem_builder/tasks.py
+++ b/problem_builder/tasks.py
@@ -149,6 +149,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:

From bb2ea2677f504bfa8a41dfa01bc54f62120c15b1 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Tue, 21 Jul 2015 09:43:12 +0200
Subject: [PATCH 19/21] Store question ID in "Question" column if "question"
 attribute is not set on block.

---
 problem_builder/tasks.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/problem_builder/tasks.py b/problem_builder/tasks.py
index c5b504e4..5a3380d8 100644
--- a/problem_builder/tasks.py
+++ b/problem_builder/tasks.py
@@ -97,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:
@@ -141,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`.

From e0fbd14c088c3d9ce57045e7a23531659650f9c0 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Tue, 21 Jul 2015 10:36:43 +0200
Subject: [PATCH 20/21] In dropdown listing blocks, show question ID (name)
 instead of block ID if "question" attribute is not set to a meaningful value.

---
 problem_builder/instructor_tool.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index 29c18f1a..d7c4e556 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -156,7 +156,10 @@ def get_block_name(block):
               - block ID
             """
             # - Try "question" attribute:
-            block_name = getattr(block, 'question', block.name)
+            block_name = getattr(block, 'question', None)
+            if not block_name:
+                # Try question ID (name):
+                block_name = getattr(block, 'name', None)
             if not block_name:
                 # - Try display_name:
                 block_name = getattr(block, 'display_name', None)

From c6ab809679338858e5ec73044023e3eba75ad969 Mon Sep 17 00:00:00 2001
From: Tim Krones <t.krones@gmx.net>
Date: Tue, 21 Jul 2015 14:39:49 +0200
Subject: [PATCH 21/21] Address review comments.

---
 problem_builder/instructor_tool.py | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/problem_builder/instructor_tool.py b/problem_builder/instructor_tool.py
index d7c4e556..5bc795ab 100644
--- a/problem_builder/instructor_tool.py
+++ b/problem_builder/instructor_tool.py
@@ -134,7 +134,7 @@ def student_view(self, context=None):
             _('Rating Question'): 'RatingBlock',
             _('Long Answer'): 'AnswerBlock',
         }
-        block_types = ('pb-mcq', 'pb-rating', 'pb-answer')
+        eligible_block_types = ('pb-mcq', 'pb-rating', 'pb-answer')
         flat_block_tree = []
 
         def get_block_id(block):
@@ -155,18 +155,10 @@ def get_block_name(block):
               - block.display_name
               - block ID
             """
-            # - Try "question" attribute:
-            block_name = getattr(block, 'question', None)
-            if not block_name:
-                # Try question ID (name):
-                block_name = getattr(block, 'name', None)
-            if not block_name:
-                # - Try display_name:
-                block_name = getattr(block, 'display_name', None)
-            if not block_name:
-                # - Default to ID:
-                block_name = get_block_id(block)
-            return block_name
+            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):
             """
@@ -185,8 +177,8 @@ def build_tree(block, ancestors):
             block_id = get_block_id(block)
             block_name = get_block_name(block)
             block_type = get_block_type(block)
-            if not block_type == 'pb-choice':
-                eligible = block_type in block_types
+            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