From fe118d8973109a962f3699c9a8cbf4c7a03f92af Mon Sep 17 00:00:00 2001
From: epai <ericpai@berkeley.edu>
Date: Sun, 17 Dec 2017 03:10:33 -0800
Subject: [PATCH] Grade Correctness for Checkpoint Grading and Bug Fixes
 (#1210)

* allow autograder correctness grading for checkpoints

* minor tweaks in how backups are gathered

* fix composition comments not working
---
 server/autograder.py                        |  9 ++-
 server/controllers/admin.py                 |  8 ++-
 server/forms.py                             |  4 ++
 server/jobs/checkpoint.py                   | 62 ++++++++++++++-------
 server/static/css/code.css                  |  2 +-
 server/static/css/main.css                  |  9 +++
 server/templates/diff.html                  |  4 +-
 server/templates/staff/jobs/checkpoint.html |  1 +
 8 files changed, 72 insertions(+), 27 deletions(-)

diff --git a/server/autograder.py b/server/autograder.py
index 5593beacf..853658286 100644
--- a/server/autograder.py
+++ b/server/autograder.py
@@ -235,7 +235,6 @@ def retry_task(task):
     message = '{} graded, {} failed'.format(
         statuses[GradingStatus.DONE], statuses[GradingStatus.FAILED])
     logger.info(message)
-    return message
 
 
 @jobs.background_job
@@ -251,5 +250,11 @@ def autograde_assignment(assignment_id):
     assignment = Assignment.query.get(assignment_id)
     course_submissions = assignment.course_submissions(include_empty=False)
     backup_ids = set(fs['backup']['id'] for fs in course_submissions if fs['backup'])
-    return autograde_backups(assignment, jobs.get_current_job().user_id, backup_ids, logger)
+    try:
+        autograde_backups(assignment, current_user.id, backup_ids, logger)
+    except ValueError:
+        logger.info('Could not autograde backups - Please add an autograding key.')
+        return
+    return '/admin/course/{cid}/assignments/{aid}/scores'.format(
+                cid=jobs.get_current_job().course_id, aid=assignment.id)
 
diff --git a/server/controllers/admin.py b/server/controllers/admin.py
index d073231d7..8f52e0192 100644
--- a/server/controllers/admin.py
+++ b/server/controllers/admin.py
@@ -829,6 +829,7 @@ def autograde(cid, aid):
         job = jobs.enqueue_job(
             autograder.autograde_assignment,
             description='Autograde {}'.format(assign.display_name),
+            result_kind='link',
             timeout=2 * 60 * 60,  # 2 hours
             course_id=cid,
             user_id=current_user.id,
@@ -1151,10 +1152,12 @@ def checkpoint_grading(cid, aid):
 
     form = forms.CheckpointCreditForm()
     if form.validate_on_submit():
+        timeout = 3600 if form.grade_backups.data else 600
         job = jobs.enqueue_job(
             checkpoint.assign_scores,
             description='Checkpoint Scoring for {}'.format(assign.display_name),
-            timeout=600,
+            timeout=timeout,
+            result_kind='link',
             course_id=cid,
             user_id=current_user.id,
             assign_id=assign.id,
@@ -1162,7 +1165,8 @@ def checkpoint_grading(cid, aid):
             kind=form.kind.data,
             message=form.message.data,
             deadline=form.deadline.data,
-            include_backups=form.include_backups.data)
+            include_backups=form.include_backups.data,
+            grade_backups=form.grade_backups.data)
         return redirect(url_for('.course_job', cid=cid, job_id=job.id))
     else:
         if not form.kind.data:
diff --git a/server/forms.py b/server/forms.py
index 28564ec9f..c6856249e 100644
--- a/server/forms.py
+++ b/server/forms.py
@@ -317,6 +317,10 @@ class CheckpointCreditForm(GradeForm):
                              description="Award points to all submissions before this time")
     include_backups = BooleanField('Include Backups', default=True,
                                    description='Include backups (as well as submissions)')
+    grade_backups = BooleanField('Grade Backups', default=False,
+                                   description='Grade backups using the autograder')
+    kind = SelectField('Kind', choices=[(c, c.title()) for c in SCORE_KINDS if 'checkpoint' in c.lower()],
+                       validators=[validators.required()])
 
 class CreateTaskForm(BaseForm):
     kind = SelectField('Kind', choices=[(c, c.title()) for c in SCORE_KINDS],
diff --git a/server/jobs/checkpoint.py b/server/jobs/checkpoint.py
index 36c083249..4f539aaa4 100644
--- a/server/jobs/checkpoint.py
+++ b/server/jobs/checkpoint.py
@@ -4,10 +4,11 @@
 from server import jobs
 from server.models import Assignment, Score, Backup, db
 from server.utils import server_time_obj
+from server.autograder import autograde_backups
 
 @jobs.background_job
 def assign_scores(assign_id, score, kind, message, deadline,
-                     include_backups=True):
+                     include_backups=True, grade_backups=False):
     logger = jobs.get_job_logger()
     current_user = jobs.get_current_job().user
 
@@ -19,7 +20,7 @@ def assign_scores(assign_id, score, kind, message, deadline,
     backups = Backup.query.filter(
         Backup.assignment_id == assign_id,
         or_(Backup.created <= deadline, Backup.custom_submission_time <= deadline)
-    ).order_by(Backup.created.desc()).group_by(Backup.submitter_id)
+    ).group_by(Backup.submitter_id).order_by(Backup.created.desc())
 
     if not include_backups:
         backups = backups.filter(Backup.submit == True)
@@ -31,28 +32,49 @@ def assign_scores(assign_id, score, kind, message, deadline,
                     .format(deadline))
         return "No Scores Created"
 
-    total_count = len(all_backups)
-    logger.info("Found {} eligible submissions...".format(total_count))
-
     score_counter, seen = 0, set()
 
+    unique_backups = []
+
     for back in all_backups:
-        if back.creator in seen:
+        if back.creator not in seen:
+            unique_backups.append(back)
+            seen |= back.owners()
+
+    total_count = len(unique_backups)
+    logger.info("Found {} unique and eligible submissions...".format(total_count))
+
+    if grade_backups:
+        logger.info('\nAutograding {} backups'.format(total_count))
+        backup_ids = [back.id for back in unique_backups]
+        try:
+            autograde_backups(assignment, current_user.id, backup_ids, logger)
+        except ValueError:
+            logger.info('Could not autograde backups - Please add an autograding key.')
+    else:
+        for back in unique_backups:
+            new_score = Score(score=score, kind=kind, message=message,
+                              user_id=back.submitter_id,
+                              assignment=assignment, backup=back,
+                              grader=current_user)
+
+            db.session.add(new_score)
+            new_score.archive_duplicates()
+
             score_counter += 1
-            continue
-        new_score = Score(score=score, kind=kind, message=message,
-                          user_id=back.submitter_id,
-                          assignment=assignment, backup=back,
-                          grader=current_user)
-        db.session.add(new_score)
-        new_score.archive_duplicates()
+            if score_counter % 100 == 0:
+                logger.info("Scored {} of {}".format(score_counter, total_count))
+
+        # only commit if all scores were successfully added
         db.session.commit()
 
-        score_counter += 1
-        if score_counter % 5 == 0:
-            logger.info("Scored {} of {}".format(score_counter, total_count))
-        seen |= back.owners()
+    logger.info("Left {} '{}' scores of {}".format(score_counter, kind.title(), score))
+    return '/admin/course/{cid}/assignments/{aid}/scores'.format(
+                cid=jobs.get_current_job().course_id, aid=assignment.id)
+
+
+
+
+
+
 
-    result = "Left {} '{}' scores of {}".format(score_counter, kind.title(), score)
-    logger.info(result)
-    return result
diff --git a/server/static/css/code.css b/server/static/css/code.css
index 9642b85ef..54aada44d 100644
--- a/server/static/css/code.css
+++ b/server/static/css/code.css
@@ -1,5 +1,5 @@
 /* Source file and diff rendering */
-.source-file {
+.source-file-box {
   position: relative;
   font-family: Consolas, 'Liberation Mono', Menlo, Courier, monospace;
   font-size: 12px;
diff --git a/server/static/css/main.css b/server/static/css/main.css
index 2e960db4b..c3c0bb332 100755
--- a/server/static/css/main.css
+++ b/server/static/css/main.css
@@ -205,4 +205,13 @@ img.desaturate{
   display: block;
   margin: 0px auto;
   color: #717171;
+}
+
+.checkbox {
+  display: inline-block;
+}
+
+.checkbox + .help-block{
+  display: inline-block;
+  margin-left: 8px;
 }
\ No newline at end of file
diff --git a/server/templates/diff.html b/server/templates/diff.html
index 24a99f3e1..e2c23c725 100644
--- a/server/templates/diff.html
+++ b/server/templates/diff.html
@@ -137,7 +137,7 @@ <h6>This file could not be displayed</h6>
 {% endmacro %}
 
 {% macro source_file(backup, filename, file, moss_diff) %}
-  <div class="box source-file">
+  <div class="box source-file-box">
     <div class="box-header source-file-header">
       <div class="source-file-name fixed-width-font">
         {{ filename }}
@@ -160,7 +160,7 @@ <h6>This file could not be displayed</h6>
           </button>
       </div>
     </div>
-    <div class="box-body table-responsive"
+    <div class="box-body source-file table-responsive"
       data-backup-id={{ utils.encode_id(backup.id) }}
       data-filename={{ filename }}>
       <table class="highlight">
diff --git a/server/templates/staff/jobs/checkpoint.html b/server/templates/staff/jobs/checkpoint.html
index ac9362baa..1c43a8512 100644
--- a/server/templates/staff/jobs/checkpoint.html
+++ b/server/templates/staff/jobs/checkpoint.html
@@ -38,6 +38,7 @@ <h1>
             {{ forms.render_field(form.message) }}
             {{ forms.render_field(form.deadline) }}
             {{ forms.render_checkbox_field(form.include_backups) }}
+            {{ forms.render_checkbox_field(form.grade_backups) }}
             {% endcall %}
         </div>
       </div>