Repository: jupyter/nbgrader

Issue:
Assignment names with a '+' character break autograding
I (perhaps foolishly) created an assignment with a '+' character in its name (the web interface did not complain). Now nbgrader will not autograde submitted assignments. From the log file, I gather that the + character is used as a separator between student id and assignment name.

Does anyone know of a workaround that lets me autograde assignments that have already been submitted?

Regards,
Axel


### Operating system
Kubuntu Linux 16.04.3 LTS

### `nbgrader --version`
Python version 3.5.2 (default, Nov 23 2017, 16:37:01) 
[GCC 5.4.0 20160609]
nbgrader version 0.5.4

### `jupyterhub --version` (if used with JupyterHub)
0.8.1

### `jupyter notebook --version` 
5.4.0

### Expected behavior
`nbgrader autograde PHY332-08-Charge+Plate_Free_Fall`should autograde submitted assignments

### Actual behavior
Autograding fails; .nbgrader.log says

```
[AutogradeApp | INFO] Copying /home/axel/PHY332/submitted/doe1j+PHY332-08-Charge/PHY332-08-Charge+Plate_Free_Fall/timestamp.txt -> /home/axel/PHY332/autograded/doe1j+PHY332-08-Charge/PHY332-08-Charge+Plate_Free_Fall/timestamp.txt
[AutogradeApp | INFO] Copying /home/axel/PHY332/submitted/doe1j+PHY332-08-Charge/PHY332-08-Charge+Plate_Free_Fall/image_charge.png -> /home/axel/PHY332/autograded/doe1j+PHY332-08-Charge/PHY332-08-Charge+Plate_Free_Fall/image_charge.png
[AutogradeApp | ERROR] No student with ID 'doe1j+PHY332-08-Charge' exists in the database
[AutogradeApp | ERROR] There was an error processing assignment: /home/axel/PHY332/submitted/doe1j+PHY332-08-Charge/PHY332-08-Charge+Plate_Free_Fall
[AutogradeApp | ERROR] Traceback (most recent call last):
      File "/usr/local/lib/python3.5/dist-packages/nbgrader/api.py", line 1122, in find_student
        .filter(Student.id == student_id)\
      File "/usr/local/lib/python3.5/dist-packages/sqlalchemy/orm/query.py", line 2843, in one
        raise orm_exc.NoResultFound("No row was found for one()")
    sqlalchemy.orm.exc.NoResultFound: No row was found for one()
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/usr/local/lib/python3.5/dist-packages/nbgrader/converters/autograde.py", line 78, in init_assignment
        gb.find_student(student_id)
      File "/usr/local/lib/python3.5/dist-packages/nbgrader/api.py", line 1125, in find_student
        raise MissingEntry("No such student: {}".format(student_id))
    nbgrader.api.MissingEntry: No such student: doe1j+PHY332-08-Charge
    
    During handling of the above exception, another exception occurred:
    
    Traceback (most recent call last):
      File "/usr/local/lib/python3.5/dist-packages/nbgrader/converters/base.py", line 289, in convert_notebooks
        self.init_assignment(gd['assignment_id'], gd['student_id'])
      File "/usr/local/lib/python3.5/dist-packages/nbgrader/converters/autograde.py", line 82, in init_assignment
        raise NbGraderException(msg)
    nbgrader.converters.base.NbGraderException: No student with ID 'doe1j+PHY332-08-Charge' exists in the database



```

### Steps to reproduce the behavior
1. Create assignment with `+` in its name, e.g. `PHY332-08-Charge+Plate_Free_Fall`
2. Collect submitted assignments using `nbgrader collect PHY332-08-Charge+Plate_Free_Fall --update`
3. Attempt to autograde with `nbgrader autograde PHY332-08-Charge+Plate_Free_Fall`

Discussion:

Patch:
diff --git a/nbgrader/coursedir.py b/nbgrader/coursedir.py
index 26207215..978afb3f 100644
--- a/nbgrader/coursedir.py
+++ b/nbgrader/coursedir.py
@@ -4,7 +4,7 @@ import re
 from textwrap import dedent
 
 from traitlets.config import LoggingConfigurable
-from traitlets import Unicode, List, default
+from traitlets import Unicode, List, default, validate, TraitError
 
 from .utils import full_split, parse_utc
 
@@ -42,6 +42,12 @@ class CourseDirectory(LoggingConfigurable):
         )
     ).tag(config=True)
 
+    @validate('assignment_id')
+    def _validate_assignment_id(self, proposal):
+        if '+' in proposal['value']:
+            raise TraitError('Assignment names should not contain the following characters: +')
+        return proposal['value']
+
     notebook_id = Unicode(
         "*",
         help=dedent(
diff --git a/nbgrader/server_extensions/formgrader/static/js/manage_assignments.js b/nbgrader/server_extensions/formgrader/static/js/manage_assignments.js
index e8e68b1f..dd2757e3 100644
--- a/nbgrader/server_extensions/formgrader/static/js/manage_assignments.js
+++ b/nbgrader/server_extensions/formgrader/static/js/manage_assignments.js
@@ -395,6 +395,15 @@ var createAssignmentModal = function () {
             modal.modal('hide');
             return;
         }
+        if (name.indexOf("+") != -1) {
+            var err = $("#create-error");
+            err.text("Assignment names may not include the '+' character.");
+            err.show();
+            return;
+        } else {
+            var err = $("#create-error");
+            err.hide();
+        }
 
         var model = new Assignment({
             "name": name,
@@ -417,19 +426,22 @@ var createAssignmentModal = function () {
         modal.modal('hide');
     };
 
-    var body = $("<table/>").addClass("table table-striped form-table");
+    var body = $("<p/>")
+    body.append($("<p id='create-error' class='alert alert-danger' style='display: none'/>"));
+    var table = $("<table/>").addClass("table table-striped form-table");
+    body.append(table)
     var name = $("<tr/>");
-    body.append(name);
+    table.append(name);
     name.append($("<td/>").addClass("align-middle").text("Name"));
     name.append($("<td/>").append($("<input/>").addClass("name").attr("type", "text").attr("size", "31")));
 
     var duedate = $("<tr/>");
-    body.append(duedate);
+    table.append(duedate);
     duedate.append($("<td/>").addClass("align-middle").text("Due date (optional)"));
     duedate.append($("<td/>").append($("<input/>").addClass("duedate").attr("type", "datetime-local")));
 
     var timezone = $("<tr/>");
-    body.append(timezone);
+    table.append(timezone);
     timezone.append($("<td/>").addClass("align-middle").text("Timezone as UTC offset (optional)"));
     timezone.append($("<td/>").append($("<input/>").addClass("timezone").attr("type", "text")));
 


Test patch:
diff --git a/nbgrader/tests/apps/test_nbgrader_assign.py b/nbgrader/tests/apps/test_nbgrader_assign.py
index 73575ef2..c39d91db 100644
--- a/nbgrader/tests/apps/test_nbgrader_assign.py
+++ b/nbgrader/tests/apps/test_nbgrader_assign.py
@@ -1,6 +1,7 @@
 import os
 import sys
 import pytest
+import traitlets
 
 from os.path import join
 from sqlalchemy.exc import InvalidRequestError
@@ -44,6 +45,13 @@ class TestNbGraderAssign(BaseTestApp):
         run_nbgrader(["assign", "ps1"])
         assert os.path.isfile(join(course_dir, "release", "ps1", "foo.ipynb"))
 
+    def test_single_file_bad_assignment_name(self, course_dir, temp_cwd):
+        """Test that an error is thrown when the assignment name is invalid."""
+        self._empty_notebook(join(course_dir, 'source', 'foo+bar', 'foo.ipynb'))
+        with pytest.raises(traitlets.TraitError):
+            run_nbgrader(["assign", "foo+bar", "--create"])
+        assert not os.path.isfile(join(course_dir, "release", "foo+bar", "foo.ipynb"))
+
     def test_multiple_files(self, course_dir):
         """Can multiple files be assigned?"""
         self._empty_notebook(join(course_dir, 'source', 'ps1', 'foo.ipynb'))
diff --git a/nbgrader/tests/nbextensions/test_formgrader.py b/nbgrader/tests/nbextensions/test_formgrader.py
index 5fcabca8..772a252f 100644
--- a/nbgrader/tests/nbextensions/test_formgrader.py
+++ b/nbgrader/tests/nbextensions/test_formgrader.py
@@ -825,7 +825,7 @@ def test_add_new_assignment(browser, port, gradebook):
     # set the name and dudedate
     elem = browser.find_element_by_css_selector("#add-assignment-modal .name")
     elem.click()
-    elem.send_keys("ps2")
+    elem.send_keys("ps2+a")
     elem = browser.find_element_by_css_selector("#add-assignment-modal .duedate")
     elem.click()
     elem.send_keys("2017-07-05T17:00")
@@ -833,6 +833,16 @@ def test_add_new_assignment(browser, port, gradebook):
     elem.click()
     elem.send_keys("UTC")
 
+    # click save and wait for the error message to appear
+    utils._click_element(browser, "#add-assignment-modal .save")
+    WebDriverWait(browser, 10).until(EC.visibility_of_element_located((By.CSS_SELECTOR, "#create-error")))
+
+    # set a valid name
+    elem = browser.find_element_by_css_selector("#add-assignment-modal .name")
+    elem.clear()
+    elem.click()
+    elem.send_keys("ps2")
+
     # click save and wait for the modal to close
     utils._click_element(browser, "#add-assignment-modal .save")
     modal_not_present = lambda browser: browser.execute_script("""return $("#add-assignment-modal").length === 0;""")