Merge pull request jupyter#738 from lgpage/index

Refactor the filtering of existing submission notebooks for formgrader

nbgrader/server_extensions/formgrader/base.py

@@ -1,3 +1,4 @@
+import os
 import json
 
 from tornado import web
@@ -38,6 +39,31 @@ def exporter(self):
     def notebook_url_prefix(self):
         return self.settings['nbgrader_notebook_url_prefix']
 
+    def _filter_existing_notebooks(self, assignment_id, notebooks):
+        path = os.path.join(
+            self.notebook_dir,
+            self.notebook_dir_format,
+            "{notebook_id}.ipynb"
+        )
+
+        submissions = list()
+        for nb in notebooks:
+            filename = path.format(
+                nbgrader_step=self.nbgrader_step,
+                assignment_id=assignment_id,
+                notebook_id=nb.name,
+                student_id=nb.student.id
+            )
+            if os.path.exists(filename):
+                submissions.append(nb)
+
+        return sorted(submissions, key=lambda x: x.id)
+
+    def _notebook_submission_indexes(self, assignment_id, notebook_id):
+        notebooks = self.gradebook.notebook_submissions(notebook_id, assignment_id)
+        submissions = self._filter_existing_notebooks(assignment_id, notebooks)
+        return dict([(x.id, i) for i, x in enumerate(submissions)])
+
     def render(self, name, **ns):
         template = self.settings['nbgrader_jinja2_env'].get_template(name)
         return template.render(**ns)

nbgrader/server_extensions/formgrader/handlers.py

@@ -60,34 +60,21 @@ def get(self, assignment_id, notebook_id):
         except MissingEntry:
             raise web.HTTPError(404, "Invalid notebook: {}/{}".format(assignment_id, notebook_id))
 
-        notebook_dir_format = os.path.join(self.notebook_dir_format, "{notebook_id}.ipynb")
-        notebook_dicts = self.gradebook.notebook_submission_dicts(notebook_id, assignment_id)
-
-        submissions = list()
-        for nb_dict in notebook_dicts:
-            filename = os.path.join(
-                self.notebook_dir,
-                notebook_dir_format.format(
-                    nbgrader_step=self.nbgrader_step,
-                    assignment_id=assignment_id,
-                    notebook_id=notebook_id,
-                    student_id=nb_dict['student']
-                )
-            )
-            if os.path.exists(filename):
-                submissions.append(nb_dict)
+        submissions = self.gradebook.notebook_submission_dicts(notebook_id, assignment_id)
+        indexes = self._notebook_submission_indexes(assignment_id, notebook_id)
+        for nb in submissions:
+            nb['index'] = indexes.get(nb['id'], None)
 
+        submissions = [x for x in submissions if x['index'] is not None]
         submissions.sort(key=lambda x: x["id"])
-        for i, submission in enumerate(submissions):
-            submission["index"] = i
 
         html = self.render(
             "notebook_submissions.tpl",
             notebook_id=notebook_id,
             assignment_id=assignment_id,
             submissions=submissions,
-            base_url=self.base_url)
-
+            base_url=self.base_url
+        )
         self.write(html)
 
 
@@ -156,32 +143,19 @@ def get(self, student_id, assignment_id):
         except MissingEntry:
             raise web.HTTPError(404, "Invalid assignment: {} for {}".format(assignment_id, student_id))
 
-
-        notebook_dir_format = os.path.join(self.notebook_dir_format, "{notebook_id}.ipynb")
-
-        submissions = list()
-        for notebook in assignment.notebooks:
-            filename = os.path.join(
-                self.notebook_dir,
-                notebook_dir_format.format(
-                    nbgrader_step=self.nbgrader_step,
-                    assignment_id=assignment_id,
-                    notebook_id=notebook.name,
-                    student_id=student_id
-                )
-            )
-            if os.path.exists(filename):
-                submissions.append(notebook.to_dict())
-
-        submissions.sort(key=lambda x: x['name'])
+        notebooks = assignment.notebooks
+        submissions = self._filter_existing_notebooks(assignment_id, notebooks)
+        submissions = [x.to_dict() for x in assignment.notebooks]
+        for i, nb in enumerate(submissions):
+            nb['index'] = i
 
         html = self.render(
             "student_submissions.tpl",
             assignment_id=assignment_id,
             student=assignment.student.to_dict(),
             submissions=submissions,
-            base_url=self.base_url)
-
+            base_url=self.base_url
+        )
         self.write(html)
 
 
@@ -208,19 +182,18 @@ def get(self, submission_id):
             nbgrader_step=self.nbgrader_step,
             assignment_id=assignment_id,
             notebook_id=notebook_id,
-            student_id=student_id))
-
-        submissions = self.gradebook.notebook_submissions(notebook_id, assignment_id)
-        submission_ids = sorted([x.id for x in submissions])
-        ix = submission_ids.index(submission.id)
-
+            student_id=student_id)
+        )
         relative_path = os.path.relpath(filename, self.notebook_dir)
+        indexes = self._notebook_submission_indexes(assignment_id, notebook_id)
+        ix = indexes.get(submission.id, -2)
+
         resources = {
             'assignment_id': assignment_id,
             'notebook_id': notebook_id,
             'submission_id': submission.id,
             'index': ix,
-            'total': len(submissions),
+            'total': len(indexes),
             'base_url': self.base_url,
             'mathjax_url': self.mathjax_url,
             'last_name': submission.student.last_name,
@@ -234,6 +207,7 @@ def get(self, submission_id):
             self.clear()
             self.set_status(404)
             self.write(html)
+
         else:
             html, _ = self.exporter.from_filename(filename, resources=resources)
             self.write(html)
@@ -252,11 +226,13 @@ def _submission_url(self, submission_id):
             return url
 
     def _get_submission_ids(self, assignment_id, notebook_id):
-        submissions = self.gradebook.notebook_submissions(notebook_id, assignment_id)
+        notebooks = self.gradebook.notebook_submissions(notebook_id, assignment_id)
+        submissions = self._filter_existing_notebooks(assignment_id, notebooks)
         return sorted([x.id for x in submissions])
 
     def _get_incorrect_submission_ids(self, assignment_id, notebook_id, submission):
-        submissions = self.gradebook.notebook_submissions(notebook_id, assignment_id)
+        notebooks = self.gradebook.notebook_submissions(notebook_id, assignment_id)
+        submissions = self._filter_existing_notebooks(assignment_id, notebooks)
         incorrect_ids = set([x.id for x in submissions if x.failed_tests])
         incorrect_ids.add(submission.id)
         incorrect_ids = sorted(incorrect_ids)