Merge pull request #515 from edx/will/ora-673

Preserve the assessment order in the Studio editor.

openassessment/templates/openassessmentblock/edit/oa_edit.html

@@ -103,19 +103,11 @@
                 {% trans "Select the steps that students must complete. All steps are optional, but every assignment must include at least one step. To change the order in which students will complete the steps, drag them into the desired order." %}
             </p>
             <ol id="openassessment_assessment_module_settings_editors">
-
-                {% for assessment in used_assessments %}
+                {% for assessment in editor_assessments_order %}
                     {% with "openassessmentblock/edit/oa_edit_"|add:assessment|add:".html" as template %}
                         {% include template %}
                     {% endwith %}
                 {% endfor %}
-
-                {% for assessment in unused_assessments %}
-                    {% with "openassessmentblock/edit/oa_edit_"|add:assessment|add:".html" as template %}
-                        {% include template %}
-                    {% endwith %}
-                {% endfor %}
-
             </ol>
         </div>
     </div>

openassessment/xblock/data_conversion.py

@@ -73,3 +73,19 @@ def create_rubric_dict(prompt, criteria):
         "prompt": prompt,
         "criteria": criteria
     }
+
+
+def make_django_template_key(key):
+    """
+    Django templates access dictionary items using dot notation,
+    which means that dictionary keys with hyphens don't work.
+    This function sanitizes a key for use in Django templates
+    by replacing hyphens with underscores.
+
+    Args:
+        key (basestring): The key to sanitize.
+
+    Returns:
+        basestring
+    """
+    return key.replace('-', '_')

openassessment/xblock/defaults.py

@@ -132,3 +132,9 @@ DEFAULT_ASSESSMENT_MODULES = [
     DEFAULT_SELF_ASSESSMENT,
 ]
 
+DEFAULT_EDITOR_ASSESSMENTS_ORDER = [
+    "example-based-assessment",
+    "student-training",
+    "peer-assessment",
+    "self-assessment",
+]

openassessment/xblock/schema.py

@@ -56,6 +56,14 @@ def datetime_validator(value):
         raise Invalid(u"Could not parse datetime from value \"{val}\"".format(val=value))
 
 
+VALID_ASSESSMENT_TYPES = [
+    u'peer-assessment',
+    u'self-assessment',
+    u'example-based-assessment',
+    u'student-training'
+]
+
+
 # Schema definition for an update from the Studio JavaScript editor.
 EDITOR_UPDATE_SCHEMA = Schema({
     Required('prompt'): utf8_validator,
@@ -66,15 +74,7 @@ EDITOR_UPDATE_SCHEMA = Schema({
     Required('allow_file_upload'): bool,
     Required('assessments'): [
         Schema({
-            Required('name'): All(
-                utf8_validator,
-                In([
-                    u'peer-assessment',
-                    u'self-assessment',
-                    u'example-based-assessment',
-                    u'student-training'
-                ])
-            ),
+            Required('name'): All(utf8_validator, In(VALID_ASSESSMENT_TYPES)),
             Required('start', default=None): Any(datetime_validator, None),
             Required('due', default=None): Any(datetime_validator, None),
             'must_grade': All(int, Range(min=0)),
@@ -92,6 +92,9 @@ EDITOR_UPDATE_SCHEMA = Schema({
             ]
         })
     ],
+    Required('editor_assessments_order'): [
+        All(utf8_validator, In(VALID_ASSESSMENT_TYPES))
+    ],
     Required('feedbackprompt', default=u""): utf8_validator,
     Required('criteria'): [
         Schema({

openassessment/xblock/static/js/fixtures/templates.json

@@ -452,12 +452,10 @@
                     "due": ""
                 }
             },
-            "used_assessments": [
-                "peer_assessment",
-                "self_assessment"
-            ],
-            "unused_assessments": [
+            "editor_assessments_order": [
                 "student_training",
+                "peer_assessment",
+                "self_assessment",
                 "example_based_assessment"
             ]
         },

openassessment/xblock/static/js/spec/oa_server.js

@@ -75,6 +75,13 @@ describe("OpenAssessment.Server", function() {
         }
     ];
 
+    var EDITOR_ASSESSMENTS_ORDER = [
+        "student_training",
+        "peer_assessment",
+        "self_assessment",
+        "example_based_assessment"
+    ];
+
     var TITLE = 'This is the title.';
     var SUBMISSION_START = '2012-10-09T00:00:00';
     var SUBMISSION_DUE = '2015-10-10T00:00:00';
@@ -199,9 +206,8 @@ describe("OpenAssessment.Server", function() {
         });
     });
 
-    it("updates the XBlock's Context definition", function() {
+    it("updates the XBlock's editor context definition", function() {
         stubAjax(true, { success: true });
-
         server.updateEditorContext({
             prompt: PROMPT,
             feedbackPrompt: FEEDBACK_PROMPT,
@@ -209,7 +215,8 @@ describe("OpenAssessment.Server", function() {
             submissionStart: SUBMISSION_START,
             submissionDue: SUBMISSION_DUE,
             criteria: CRITERIA,
-            assessments: ASSESSMENTS
+            assessments: ASSESSMENTS,
+            editorAssessmentsOrder: EDITOR_ASSESSMENTS_ORDER
         });
         expect($.ajax).toHaveBeenCalledWith({
             type: "POST", url: '/update_editor_context',
@@ -220,7 +227,8 @@ describe("OpenAssessment.Server", function() {
                 submission_start: SUBMISSION_START,
                 submission_due: SUBMISSION_DUE,
                 criteria: CRITERIA,
-                assessments: ASSESSMENTS
+                assessments: ASSESSMENTS,
+                editor_assessments_order: EDITOR_ASSESSMENTS_ORDER
             })
         });
     });

openassessment/xblock/static/js/spec/studio/oa_edit.js

@@ -41,6 +41,84 @@ describe("OpenAssessment.StudioView", function() {
     var server = null;
     var view = null;
 
+    var EXPECTED_SERVER_DATA = {
+        title: "The most important of all questions.",
+        prompt: "How much do you like waffles?",
+        feedbackPrompt: "",
+        submissionStart: null,
+        submissionDue: null,
+        imageSubmissionEnabled: false,
+        criteria: [
+            {
+                order_num: 0,
+                label: "Criterion with two options",
+                name: "52bfbd0eb3044212b809564866e77079",
+                prompt: "Prompt for criterion with two options",
+                feedback: "disabled",
+                options: [
+                    {
+                        order_num: 0,
+                        points: 1,
+                        name: "85bbbecbb6a343f8a2146cde0e609ad0",
+                        label: "Fair",
+                        explanation: "Fair explanation"
+                    },
+                    {
+                        order_num: 1,
+                        points: 2,
+                        name: "5936d5b9e281403ca123964055d4719a",
+                        label: "Good",
+                        explanation: "Good explanation"
+                    }
+                ]
+            },
+            {
+                name: "d96bb68a69ee4ccb8f86c753b6924f75",
+                label: "Criterion with no options",
+                prompt: "Prompt for criterion with no options",
+                order_num: 1,
+                options: [],
+                feedback: "required",
+            },
+            {
+                name: "2ca052403b06424da714f7a80dfb954d",
+                label: "Criterion with optional feedback",
+                prompt: "Prompt for criterion with optional feedback",
+                order_num: 2,
+                feedback: "optional",
+                options: [
+                    {
+                        order_num: 0,
+                        points: 2,
+                        name: "d7445661a89b4b339b9788cb7225a603",
+                        label: "Good",
+                        explanation: "Good explanation"
+                    }
+                ],
+            }
+        ],
+        assessments: [
+            {
+                name: "peer-assessment",
+                start: null,
+                due: null,
+                must_grade: 5,
+                must_be_graded_by: 3
+            },
+            {
+                name: "self-assessment",
+                start: null,
+                due: null
+            }
+        ],
+        editorAssessmentsOrder: [
+            "student-training",
+            "peer-assessment",
+            "self-assessment",
+            "example-based-assessment"
+        ]
+    };
+
     beforeEach(function() {
         // Load the DOM fixture
         jasmine.getFixtures().fixturesPath = 'base/fixtures';
@@ -57,6 +135,40 @@ describe("OpenAssessment.StudioView", function() {
         view = new OpenAssessment.StudioView(runtime, el, server);
     });
 
+    it("sends the editor context to the server", function() {
+        // Save the current state of the problem
+        // (defined by the current state of the DOM),
+        // and verify that the correct information was sent
+        // to the server.  This depends on the HTML fixture
+        // used for this test.
+        view.save();
+
+        // Top-level attributes
+        expect(server.receivedData.title).toEqual(EXPECTED_SERVER_DATA.title);
+        expect(server.receivedData.prompt).toEqual(EXPECTED_SERVER_DATA.prompt);
+        expect(server.receivedData.feedbackPrompt).toEqual(EXPECTED_SERVER_DATA.feedbackPrompt);
+        expect(server.receivedData.submissionStart).toEqual(EXPECTED_SERVER_DATA.submissionStart);
+        expect(server.receivedData.submissionDue).toEqual(EXPECTED_SERVER_DATA.submissionDue);
+        expect(server.receivedData.imageSubmissionEnabled).toEqual(EXPECTED_SERVER_DATA.imageSubmissionEnabled);
+
+        // Criteria
+        for (var criterion_idx = 0; criterion_idx < EXPECTED_SERVER_DATA.criteria.length; criterion_idx++) {
+            var actual_criterion = server.receivedData.criteria[criterion_idx];
+            var expected_criterion = EXPECTED_SERVER_DATA.criteria[criterion_idx];
+            expect(actual_criterion).toEqual(expected_criterion);
+        }
+
+        // Assessments
+        for (var asmnt_idx = 0; asmnt_idx < EXPECTED_SERVER_DATA.assessments.length; asmnt_idx++) {
+            var actual_asmnt = server.receivedData.assessments[asmnt_idx];
+            var expected_asmnt = EXPECTED_SERVER_DATA.assessments[asmnt_idx];
+            expect(actual_asmnt).toEqual(expected_asmnt);
+        }
+
+        // Editor assessment order
+        expect(server.receivedData.editorAssessmentsOrder).toEqual(EXPECTED_SERVER_DATA.editorAssessmentsOrder);
+    });
+
     it("confirms changes for a released problem", function() {
         // Simulate an XBlock that has been released
         server.isReleased = true;

openassessment/xblock/static/js/spec/studio/oa_edit_settings.js

@@ -125,24 +125,5 @@ describe("OpenAssessment.EditSettingsView", function() {
                 dummy: "Self assessment description"
             }
         ]);
-
-        // NOTE: Because the fixtures are generated with the context that only peer and self assessments
-        // are enabled, student training is placed at the end.  Even when we enable it, we do not fundamentally
-        // change the order in the DOM, so peer assessment will come before student training.
-        // Enable Training and Peer assessments
-        assessmentViews[peerID].isEnabled(true);
-        assessmentViews[selfID].isEnabled(false);
-        assessmentViews[aiID].isEnabled(false);
-        assessmentViews[studentID].isEnabled(true);
-        expect(view.assessmentsDescription()).toEqual([
-            {
-                name: "peer-assessment",
-                dummy: "Peer assessment description"
-            },
-            {
-                name: "student-training",
-                dummy: "Student Training description"
-            }
-        ]);
     });
 });

openassessment/xblock/static/js/src/oa_server.js

@@ -430,6 +430,7 @@ if (typeof OpenAssessment.Server == "undefined" || !OpenAssessment.Server) {
                 submission_due: kwargs.submissionDue,
                 criteria: kwargs.criteria,
                 assessments: kwargs.assessments,
+                editor_assessments_order: kwargs.editorAssessmentsOrder,
                 allow_file_upload: kwargs.imageSubmissionEnabled
             });
             return $.Deferred(function(defer) {

openassessment/xblock/static/js/src/studio/oa_edit.js

@@ -58,8 +58,6 @@ OpenAssessment.StudioView = function(runtime, element, server) {
     // Install the save and cancel buttons
     $(".openassessment_save_button", this.element).click($.proxy(this.save, this));
     $(".openassessment_cancel_button", this.element).click($.proxy(this.cancel, this));
-
-    this.initializeSortableAssessments()
 };
 
 OpenAssessment.StudioView.prototype = {
@@ -112,44 +110,6 @@ OpenAssessment.StudioView.prototype = {
     },
 
     /**
-    Installs click listeners which initialize drag and drop functionality for assessment modules.
-    **/
-    initializeSortableAssessments: function () {
-        var view = this;
-        // Initialize Drag and Drop of Assessment Modules
-        $('#openassessment_assessment_module_settings_editors', view.element).sortable({
-            // On Start, we want to collapse all draggable items so that dragging is visually simple (no scrolling)
-            start: function(event, ui) {
-                // Hide all of the contents (not the headers) of the divs, to collapse during dragging.
-                $('.openassessment_assessment_module_editor', view.element).hide();
-
-                // Because of the way that JQuery actively resizes elements during dragging (directly setting
-                // the style property), the only way to over come it is to use an important tag ( :( ), or
-                // to tell JQuery to set the height to be Automatic (i.e. resize to the minimum nescesary size.)
-                // Because all of the information we don't want displayed is now hidden, an auto height will
-                // perform the apparent "collapse" that we are looking for in the Placeholder and Helper.
-                var targetHeight = 'auto';
-                // Shrink the blank area behind the dragged item.
-                ui.placeholder.height(targetHeight);
-                // Shrink the dragged item itself.
-                ui.helper.height(targetHeight);
-                // Update the sortable to reflect these changes.
-                $('#openassessment_assessment_module_settings_editors', view.element)
-                    .sortable('refresh').sortable('refreshPositions');
-            },
-            // On stop, we redisplay the divs to their original state
-            stop: function(event, ui){
-                $('.openassessment_assessment_module_editor', view.element).show();
-            },
-            snap: true,
-            axis: "y",
-            handle: ".drag-handle",
-            cursorAt: {top: 20}
-        });
-        $('#openassessment_assessment_module_settings_editors .drag-handle', view.element).disableSelection();
-    },
-
-    /**
     Save the problem's XML definition to the server.
     If the problem has been released, make the user confirm the save.
     **/
@@ -203,7 +163,8 @@ OpenAssessment.StudioView.prototype = {
             submissionStart: view.settingsView.submissionStart(),
             submissionDue: view.settingsView.submissionDue(),
             assessments: view.settingsView.assessmentsDescription(),
-            imageSubmissionEnabled: view.settingsView.imageSubmissionEnabled()
+            imageSubmissionEnabled: view.settingsView.imageSubmissionEnabled(),
+            editorAssessmentsOrder: view.settingsView.editorAssessmentsOrder()
         }).done(
             // Notify the client-side runtime that we finished saving
             // so it can hide the "Saving..." notification.

openassessment/xblock/static/js/src/studio/oa_edit_settings.js

@@ -26,12 +26,52 @@ OpenAssessment.EditSettingsView = function(element, assessmentViews) {
         "#openassessment_submission_due_date",
         "#openassessment_submission_due_time"
     ).install();
+
+    this.initializeSortableAssessments();
 };
 
 
 OpenAssessment.EditSettingsView.prototype = {
 
     /**
+    Installs click listeners which initialize drag and drop functionality for assessment modules.
+    **/
+    initializeSortableAssessments: function () {
+        var view = this;
+        // Initialize Drag and Drop of Assessment Modules
+        $('#openassessment_assessment_module_settings_editors', view.element).sortable({
+            // On Start, we want to collapse all draggable items so that dragging is visually simple (no scrolling)
+            start: function(event, ui) {
+                // Hide all of the contents (not the headers) of the divs, to collapse during dragging.
+                $('.openassessment_assessment_module_editor', view.element).hide();
+
+                // Because of the way that JQuery actively resizes elements during dragging (directly setting
+                // the style property), the only way to over come it is to use an important tag ( :( ), or
+                // to tell JQuery to set the height to be Automatic (i.e. resize to the minimum nescesary size.)
+                // Because all of the information we don't want displayed is now hidden, an auto height will
+                // perform the apparent "collapse" that we are looking for in the Placeholder and Helper.
+                var targetHeight = 'auto';
+                // Shrink the blank area behind the dragged item.
+                ui.placeholder.height(targetHeight);
+                // Shrink the dragged item itself.
+                ui.helper.height(targetHeight);
+                // Update the sortable to reflect these changes.
+                $('#openassessment_assessment_module_settings_editors', view.element)
+                    .sortable('refresh').sortable('refreshPositions');
+            },
+            // On stop, we redisplay the divs to their original state
+            stop: function(event, ui){
+                $('.openassessment_assessment_module_editor', view.element).show();
+            },
+            snap: true,
+            axis: "y",
+            handle: ".drag-handle",
+            cursorAt: {top: 20}
+        });
+        $('#openassessment_assessment_module_settings_editors .drag-handle', view.element).disableSelection();
+    },
+
+    /**
     Get or set the display name of the problem.
 
     Args:
@@ -98,6 +138,7 @@ OpenAssessment.EditSettingsView.prototype = {
     /**
     Construct a list of enabled assessments and their properties.
 
+
     Returns:
         list of object literals representing the assessments.
 
@@ -121,15 +162,40 @@ OpenAssessment.EditSettingsView.prototype = {
     assessmentsDescription: function() {
         var assessmentDescList = [];
         var view = this;
-        // Finds all assessment modules within our element in the DOM, and appends their definitions to the DescList
-        $('.openassessment_assessment_module_settings_editor', this.assessmentsElement).each( function () {
-            var asmntView = view.assessmentViews[$(this).attr('id')];
-            if (asmntView.isEnabled()) {
-                var description = asmntView.description();
-                description["name"] = asmntView.name;
-                assessmentDescList.push(description);
+
+        // Find all assessment modules within our element in the DOM,
+        // and append their definitions to the description
+        $('.openassessment_assessment_module_settings_editor', this.assessmentsElement).each(
+            function () {
+                var asmntView = view.assessmentViews[$(this).attr('id')];
+                if (asmntView.isEnabled()) {
+                    var description = asmntView.description();
+                    description["name"] = asmntView.name;
+                    assessmentDescList.push(description);
+                }
             }
-        });
+        );
         return assessmentDescList;
-    }
+    },
+
+    /**
+    Retrieve the names of all assessments in the editor,
+    in the order that the user defined,
+    including assessments that are not currently active.
+
+    Returns:
+        list of strings
+
+    **/
+    editorAssessmentsOrder: function() {
+        var editorAssessments = [];
+        var view = this;
+        $('.openassessment_assessment_module_settings_editor', this.assessmentsElement).each(
+            function () {
+                var asmntView = view.assessmentViews[$(this).attr('id')];
+                editorAssessments.push(asmntView.name);
+            }
+        );
+        return editorAssessments;
+    },
 };
\ No newline at end of file

openassessment/xblock/studio_mixin.py

@@ -8,14 +8,13 @@ from uuid import uuid4
 from django.template import Context
 from django.template.loader import get_template
 from django.utils.translation import ugettext as _
-from dateutil.parser import parse as parse_date
-import pytz
 from voluptuous import MultipleInvalid
 from xblock.core import XBlock
+from xblock.fields import List, Scope
 from xblock.fragment import Fragment
-from openassessment.xblock import xml
+from openassessment.xblock.defaults import DEFAULT_EDITOR_ASSESSMENTS_ORDER
 from openassessment.xblock.validation import validator
-from openassessment.xblock.data_conversion import create_rubric_dict
+from openassessment.xblock.data_conversion import create_rubric_dict, make_django_template_key
 from openassessment.xblock.schema import EDITOR_UPDATE_SCHEMA
 from openassessment.xblock.resolve_dates import resolve_dates
 
@@ -39,6 +38,17 @@ class StudioMixin(object):
         }
     ]
 
+    # Since the XBlock problem definition contains only assessment
+    # modules that are enabled, we need to keep track of the order
+    # that the user left assessments in the editor, including
+    # the ones that were disabled.  This allows us to keep the order
+    # that the user specified.
+    editor_assessments_order = List(
+        default=DEFAULT_EDITOR_ASSESSMENTS_ORDER,
+        scope=Scope.content,
+        help="The order to display assessments in the editor."
+    )
+
     def studio_view(self, context=None):
         """
         Render the OpenAssessment XBlock for editing in Studio.
@@ -79,8 +89,7 @@ class StudioMixin(object):
 
         submission_start, submission_due = date_ranges[0]
         assessments = self._assessments_editor_context(date_ranges[1:])
-
-        used_assessments, unused_assessments = self._get_assessment_order_and_use()
+        editor_assessments_order = self._editor_assessments_order_context()
 
         # Every rubric requires one criterion. If there is no criteria
         # configured for the XBlock, return one empty default criterion, with
@@ -97,9 +106,11 @@ class StudioMixin(object):
             'assessments': assessments,
             'criteria': criteria,
             'feedbackprompt': self.rubric_feedback_prompt,
-            'unused_assessments': unused_assessments,
-            'used_assessments': used_assessments,
-            'allow_file_upload': self.allow_file_upload
+            'allow_file_upload': self.allow_file_upload,
+            'editor_assessments_order': [
+                make_django_template_key(asmnt)
+                for asmnt in editor_assessments_order
+            ],
         }
 
     @XBlock.json_handler
@@ -108,14 +119,8 @@ class StudioMixin(object):
         Update the XBlock's configuration.
 
         Args:
-            data (dict): Data from the request; should have a value for the keys: 'rubric', 'prompt',
-            'title', 'submission_start', 'submission_due', and 'assessments'.
-                -- The 'rubric' should be an XML representation of the new rubric.
-                -- The 'prompt' and 'title' should be plain text.
-                -- The dates 'submission_start' and 'submission_due' are both ISO strings
-                -- The 'assessments' is a list of assessment dictionaries (much like self.rubric_assessments)
-                   with the notable exception that all examples (for Student Training and eventually AI)
-                   are in XML string format and need to be parsed into dictionaries.
+            data (dict): Data from the request; should have the format described
+            in the editor schema.
 
         Kwargs:
             suffix (str): Not used
@@ -133,6 +138,11 @@ class StudioMixin(object):
             logger.exception('Editor context is invalid')
             return {'success': False, 'msg': _('Error updating XBlock configuration')}
 
+        # Check that the editor assessment order contains all the assessments
+        if set(DEFAULT_EDITOR_ASSESSMENTS_ORDER) != set(data['editor_assessments_order']):
+            logger.exception('editor_assessments_order does not contain all expected assessment types')
+            return {'success': False, 'msg': _('Error updating XBlock configuration')}
+
         # Backwards compatibility: We used to treat "name" as both a user-facing label
         # and a unique identifier for criteria and options.
         # Now we treat "name" as a unique identifier, and we've added an additional "label"
@@ -162,6 +172,7 @@ class StudioMixin(object):
         self.prompt = data['prompt']
         self.rubric_criteria = data['criteria']
         self.rubric_assessments = data['assessments']
+        self.editor_assessments_order = data['editor_assessments_order']
         self.rubric_feedback_prompt = data['feedback_prompt']
         self.submission_start = data['submission_start']
         self.submission_due = data['submission_due']
@@ -206,8 +217,7 @@ class StudioMixin(object):
         for asmnt, date_range in zip(self.rubric_assessments, assessment_dates):
             # Django Templates cannot handle dict keys with dashes, so we'll convert
             # the dashes to underscores.
-            name = asmnt['name']
-            template_name = name.replace('-', '_')
+            template_name = make_django_template_key(asmnt['name'])
             assessments[template_name] = copy.deepcopy(asmnt)
             assessments[template_name]['start'] = date_range[0]
             assessments[template_name]['due'] = date_range[1]
@@ -246,25 +256,33 @@ class StudioMixin(object):
 
         return assessments
 
-    def _get_assessment_order_and_use(self):
+    def _editor_assessments_order_context(self):
         """
-        Returns the names of assessments that should be displayed to the author, in the format that
-        our template expects, and in the order in which they are currently stored in the OA problem
-        definition.
+        Create a list of assessment names in the order
+        the user last set in the editor, including
+        assessments that are not currently enabled.
 
         Returns:
-            Tuple of the form ((list of str), (list of str))
-                Between the two lists, all of the assessment modules that the author should see are displayed.
-        """
-        used_assessments = []
+            list of assessment names
 
-        for assessment in self.rubric_assessments:
-            used_assessments.append(
-                assessment['name'].replace('-','_')
-            )
-
-        # NOTE: This is a perfect opportunity to gate the author to allow or disallow Example Based Assessment.
-        all_assessments = {'student_training', 'peer_assessment', 'self_assessment', 'example_based_assessment'}
-        unused_assessments = list(all_assessments - set(used_assessments))
-
-        return used_assessments, unused_assessments
+        """
+        order = copy.deepcopy(self.editor_assessments_order)
+
+        # Backwards compatibility:
+        # If the editor assessments order doesn't match the problem order,
+        # fall back to the problem order.
+        # This handles the migration of problems created pre-authoring,
+        # which will have the default editor order.
+        used_assessments = [asmnt['name'] for asmnt in self.valid_assessments]
+        problem_order_indices = [
+            order.index(asmnt_name) for asmnt_name in used_assessments
+            if asmnt_name in order
+        ]
+        if problem_order_indices != sorted(problem_order_indices):
+            unused_assessments = list(set(DEFAULT_EDITOR_ASSESSMENTS_ORDER) - set(used_assessments))
+            return sorted(unused_assessments) + used_assessments
+
+        # Forwards compatibility:
+        # Include any additional assessments that may have been added since the problem was created.
+        else:
+            return order + list(set(DEFAULT_EDITOR_ASSESSMENTS_ORDER) - set(order))

openassessment/xblock/test/data/update_xblock.json

@@ -41,7 +41,8 @@
                 "start": null,
                 "due": null
             }
-        ]
+        ],
+        "editor_assessments_order": ["student-training", "peer-assessment", "self-assessment", "example-based-assessment"]
     },
 
     "unicode": {
@@ -86,7 +87,8 @@
                 "start": null,
                 "due": null
             }
-        ]
+        ],
+        "editor_assessments_order": ["student-training", "peer-assessment", "self-assessment", "example-based-assessment"]
     },
 
     "student_training": {
@@ -146,7 +148,8 @@
                 "start": null,
                 "due": "4014-03-10T00:00"
             }
-        ]
+        ],
+        "editor_assessments_order": ["student-training", "peer-assessment", "self-assessment", "example-based-assessment"]
     },
 
     "already_has_criteria_and_options_names_assigned": {
@@ -194,6 +197,7 @@
                 "start": null,
                 "due": null
             }
-        ]
+        ],
+        "editor_assessments_order": ["student-training", "peer-assessment", "self-assessment", "example-based-assessment"]
     }
 }

openassessment/xblock/test/test_studio.py

@@ -2,12 +2,12 @@
 View-level tests for Studio view of OpenAssessment XBlock.
 """
 
+import copy
 import json
 import datetime as dt
 import pytz
 from ddt import ddt, file_data
 from .base import scenario, XBlockHandlerTestCase
-import xml.etree.ElementTree as etree
 
 
 @ddt
@@ -15,6 +15,39 @@ class StudioViewTest(XBlockHandlerTestCase):
     """
     Test the view and handlers for editing the OpenAssessment XBlock in Studio.
     """
+    UPDATE_EDITOR_DATA = {
+        "title": "Test title",
+        "prompt": "Test prompt",
+        "feedback_prompt": "Test feedback prompt",
+        "submission_start": "4014-02-10T09:46",
+        "submission_due": "4014-02-27T09:46",
+        "allow_file_upload": False,
+        "assessments": [{"name": "self-assessment"}],
+        "editor_assessments_order": [
+            "example-based-assessment",
+            "student-training",
+            "peer-assessment",
+            "self-assessment",
+        ],
+        "criteria": [
+            {
+                "order_num": 0,
+                "name": "Test criterion",
+                "label": "Test criterion",
+                "prompt": "Test criterion prompt",
+                "feedback": "disabled",
+                "options": [
+                    {
+                        "order_num": 0,
+                        "points": 0,
+                        "name": "Test option",
+                        "label": "Test option",
+                        "explanation": "Test explanation"
+                    }
+                ]
+            },
+        ]
+    }
 
     RUBRIC_CRITERIA_WITH_AND_WITHOUT_NAMES = [
         {
@@ -55,6 +88,13 @@ class StudioViewTest(XBlockHandlerTestCase):
         }
     ]
 
+    ASSESSMENT_CSS_IDS = {
+        "example-based-assessment": "oa_ai_assessment_editor",
+        "peer-assessment": "oa_peer_assessment_editor",
+        "self-assessment": "oa_self_assessment_editor",
+        "student-training": "oa_student_training_editor"
+    }
+
     @scenario('data/basic_scenario.xml')
     def test_render_studio_view(self, xblock):
         frag = self.runtime.render(xblock, 'studio_view')
@@ -67,25 +107,33 @@ class StudioViewTest(XBlockHandlerTestCase):
 
     @file_data('data/update_xblock.json')
     @scenario('data/basic_scenario.xml')
-    def test_update_context(self, xblock, data):
+    def test_update_editor_context(self, xblock, data):
+        xblock.published_date = None
+        resp = self.request(xblock, 'update_editor_context', json.dumps(data), response_format='json')
+        self.assertTrue(resp['success'], msg=resp.get('msg'))
+
+    @scenario('data/basic_scenario.xml')
+    def test_update_editor_context_saves_assessment_order(self, xblock):
+        # Update the XBlock with a different editor assessment order
+        data = copy.deepcopy(self.UPDATE_EDITOR_DATA)
+        data['editor_assessments_order'] = [
+            "example-based-assessment",
+            "student-training",
+            "peer-assessment",
+            "self-assessment",
+        ]
         xblock.published_date = None
         resp = self.request(xblock, 'update_editor_context', json.dumps(data), response_format='json')
         self.assertTrue(resp['success'], msg=resp.get('msg'))
+        self.assertEqual(xblock.editor_assessments_order, data['editor_assessments_order'])
 
     @scenario('data/basic_scenario.xml')
     def test_update_editor_context_assign_unique_names(self, xblock):
         # Update the XBlock with a rubric that is missing
         # some of the (unique) names for rubric criteria/options.
-        data = {
-            "title": "Test title",
-            "prompt": "Test prompt",
-            "feedback_prompt": "Test feedback prompt",
-            "submission_start": "4014-02-10T09:46",
-            "submission_due": "4014-02-27T09:46",
-            "allow_file_upload": False,
-            "assessments": [{"name": "self-assessment"}],
-            "criteria": self.RUBRIC_CRITERIA_WITH_AND_WITHOUT_NAMES
-        }
+        data = copy.deepcopy(self.UPDATE_EDITOR_DATA)
+        data['criteria'] = self.RUBRIC_CRITERIA_WITH_AND_WITHOUT_NAMES
+
         xblock.published_date = None
         resp = self.request(xblock, 'update_editor_context', json.dumps(data), response_format='json')
         self.assertTrue(resp['success'], msg=resp.get('msg'))
@@ -158,69 +206,68 @@ class StudioViewTest(XBlockHandlerTestCase):
         self.assertFalse(resp['is_released'])
         self.assertIn('msg', resp)
 
-    @scenario('data/example_based_assessment.xml')
-    def test_assessment_module_ordering_example_based(self, xblock):
-        self.assert_assessment_order_(xblock)
-
-    @scenario('data/basic_scenario.xml')
-    def test_assessment_module_ordering_basic(self, xblock):
-        self.assert_assessment_order_(xblock)
-
     @scenario('data/self_then_peer.xml')
-    def test_assessment_module_ordering_self_peer(self, xblock):
-        self.assert_assessment_order_(xblock)
-
-    @scenario('data/student_training.xml')
-    def test_assessment_module_ordering_student_training(self, xblock):
-        self.assert_assessment_order_(xblock)
-
-    @scenario('data/self_only_scenario.xml')
-    def test_assessment_module_ordering_self_only(self, xblock):
-        self.assert_assessment_order_(xblock)
-
-    def assert_assessment_order_(self, xblock):
-        """
-        Asserts that the assessment module editors are rendered in the correct order.
-        Renders the Studio View, and then examines the html body for the tags that we anticipate
-        to be in the tag for each editor, and compare the order. If it is anything besides
-        strictly increasing, we say that they rendered in the incorrect order.
+    def test_render_editor_assessment_order(self, xblock):
+        # Initially, the editor assessment order should be the default
+        # (because we haven't set it yet by saving in Studio)
+        # However, the assessment order IS set when we import the problem from XML,
+        # and it differs from the default order (self->peer instead of peer->self)
+        # Expect that the editor uses the order defined by the problem.
+        self._assert_rendered_editor_order(xblock, [
+            'example-based-assessment',
+            'student-training',
+            'self-assessment',
+            'peer-assessment',
+        ])
+
+        # Change the order (simulates what would happen when the author saves).
+        xblock.editor_assessments_order = [
+            'student-training',
+            'example-based-assessment',
+            'peer-assessment',
+            'self-assessment',
+        ]
+        xblock.rubric_assessments = [
+            xblock.get_assessment_module('peer-assessment'),
+            xblock.get_assessment_module('self-assessment'),
+        ]
+
+        # Expect that the rendered view reflects the new order
+        self._assert_rendered_editor_order(xblock, [
+            'student-training',
+            'example-based-assessment',
+            'peer-assessment',
+            'self-assessment',
+        ])
+
+    def _assert_rendered_editor_order(self, xblock, expected_assessment_order):
         """
-        frag = self.runtime.render(xblock, 'studio_view')
-        frag = frag.body_html()
-
-        assessments_in_order = self._find_assessment_order(xblock)
+        Render the XBlock Studio editor view and verify that the
+        assessments were listed in a particular order.
 
-        assessment_indicies = [frag.find(assessment) for assessment in assessments_in_order]
+        Args:
+            xblock (OpenAssessmentBlock)
+            expected_assessment_order (list of string): The list of assessment names,
+                in the order we expect.
 
-        # Asserts that for any pairwise comparison of elements n and n-1 in the lookup of indicies
-        # the value at n will be greater than n-1 (i.e. the place we find one ID is after the one before it)
-        self.assertTrue(
-            all(a < b for a, b in zip(assessment_indicies, assessment_indicies[1:]))
-        )
+        Raises:
+            AssertionError
 
-    def _find_assessment_order(self, xblock):
         """
-        Finds the order that we anticipate HTML ID tags of the section editors within the settings editor.
-
-        Returns:
-            A list with the four setting editor IDs, in the the order that we would anticipate given
-            the Xblock's problem definition that is handed in.
-        """
-        assessments = []
-        for assessment in xblock.rubric_assessments:
-            assessments.append(assessment['name'].replace('-', '_'))
-
-        all_assessments = {'student_training', 'peer_assessment', 'self_assessment', 'example_based_assessment'}
-        unused_assessments = list(all_assessments - set(assessments))
-        assessments.extend(unused_assessments)
-
-        id_dictionary = {
-            "example_based_assessment": "oa_ai_assessment_editor",
-            "peer_assessment": "oa_peer_assessment_editor",
-            "self_assessment": "oa_self_assessment_editor",
-            "student_training": "oa_student_training_editor"
-        }
-        return [id_dictionary[name] for name in assessments]
+        rendered_html = self.runtime.render(xblock, 'studio_view').body_html()
+        assessment_indices = [
+            {
+                "name": asmnt_name,
+                "index": rendered_html.find(asmnt_css_id)
+            }
+            for asmnt_name, asmnt_css_id
+            in self.ASSESSMENT_CSS_IDS.iteritems()
+        ]
+        actual_assessment_order = [
+            index_dict['name']
+            for index_dict in sorted(assessment_indices, key=lambda d: d['index'])
+        ]
+        self.assertEqual(actual_assessment_order, expected_assessment_order)
 
     @scenario('data/basic_scenario.xml')
     def test_editor_context_assigns_labels(self, xblock):