Autograder improvements (autolab#2209)

* Autograder improvements

* Linting

* Sanitized path

* Reduce code duplication

* Fixed spacing and added auth check for method

(cherry picked from commit 342cc0f)

app/assets/stylesheets/style.css.scss

@@ -966,6 +966,11 @@ input[type="search"] {
   margin-bottom: 2px;
 }
 
+div.file-field.input-field {
+  padding-top: 0;
+  margin-top: -0.5rem !important;
+}
+
 .flatpickr-input.form-control.input {
   color: black;
 }

app/controllers/autograders_controller.rb

@@ -1,6 +1,8 @@
 ##
 # Each Assessment can have an autograder, which is modified with this controller
-#
+
+require 'pathname'
+
 class AutogradersController < ApplicationController
   before_action :set_assessment
   before_action :set_assessment_breadcrumb, only: [:edit]
@@ -27,11 +29,16 @@ def create
   end
 
   action_auth_level :edit, :instructor
-  def edit; end
+  def edit
+    makefile_path = Rails.root.join("courses", @course.name, @assessment.name, "autograde-Makefile")
+    tar_path = Rails.root.join("courses", @course.name, @assessment.name, "autograde.tar")
+    @makefile_exists = File.exist?(makefile_path) ? makefile_path : nil
+    @tar_exists = File.exist?(tar_path) ? tar_path : nil
+  end
 
   action_auth_level :update, :instructor
   def update
-    if @autograder.update(autograder_params)
+    if @autograder.update(autograder_params) && @assessment.update(assessment_params)
       flash[:success] = "Autograder saved."
       begin
         upload
@@ -78,6 +85,25 @@ def upload
     end
   end
 
+  action_auth_level :download_file, :instructor
+  def download_file
+    allowed_files = {
+      'makefile' => Rails.root.join('courses', @course.name, @assessment.name,
+                                    'autograde-Makefile'),
+      'tar' => Rails.root.join('courses', @course.name, @assessment.name, 'autograde.tar')
+    }
+
+    file_key = params[:file_key]
+    file_path = allowed_files[file_key]
+
+    if file_path && File.exist?(file_path)
+      send_file(file_path, disposition: 'attachment')
+    else
+      flash[:error] = 'File not found'
+      redirect_to(edit_course_assessment_autograder_path(@course, @assessment))
+    end
+  end
+
 private
 
   def set_autograder
@@ -88,4 +114,8 @@ def set_autograder
   def autograder_params
     params[:autograder].permit(:autograde_timeout, :autograde_image, :release_score)
   end
+
+  def assessment_params
+    params.fetch(:autograder, {}).fetch(:assessment, {}).permit(:disable_network)
+  end
 end

app/form_builders/form_builder_with_date_time_input.rb

@@ -27,7 +27,8 @@ def score_adjustment_input(name, *args)
     options = args.extract_options!
 
     fields = fields_for name do |f|
-      (f.vanilla_text_field :value, class: "score-box", placeholder: options[:placeholder] || "", disabled: options[:disabled]) +
+      (f.vanilla_text_field :value, class: "score-box", placeholder: options[:placeholder] || "",
+                                    disabled: options[:disabled]) +
         (@template.content_tag :div do
           f.select(:kind, { "points" => "points", "%" => "percent" }, {},
                    class: "carrot", disabled: options[:disabled])
@@ -82,8 +83,15 @@ def file_field(name, *args)
             vanilla_file_field(name, options)
         end) +
         (@template.content_tag :div, class: "file-path-wrapper" do
-          (@template.content_tag :input, nil, class: "file-path validate", type: "text", value: "No file selected") +
-            help_text(name, options[:help_text])
+          if options.include?(:file_exists) && options.include?(:file_exists_text) && options[:file_exists]
+            (@template.content_tag :input, nil, class: "file-path validate", type: "text",
+                                                value: options[:file_exists_text]) +
+              help_text(name, options[:help_text])
+          else
+            (@template.content_tag :input, nil, class: "file-path validate", type: "text",
+                                                value: "No file selected") +
+              help_text(name, options[:help_text])
+          end
         end)
     end
   end

app/views/assessments/_edit_advanced.html.erb

@@ -16,7 +16,7 @@
   <%#= f.text_field :remote_handin_path, placeholder: "(Optional)", help_text: "The directory outside the assessment directory where student submissions directly from local machines will be copied." %>
 <%# end %>
 
-<%= f.check_box :disable_network, disabled: true, help_text: "Disable network access for autograding containers. (This is not implemented in UB's Tango yet.)" %>
+<%#= f.check_box :disable_network, disabled: true, help_text: "Disable network access for autograding containers. (This is not implemented in UB's Tango yet.)" %>
 <%#= f.check_box :allow_unofficial,
                 help_text: "Allow unofficial submission. Unless you know what you're doing, leave this unchecked." %>
 <%= f.check_box :embedded_quiz,

app/views/autograders/_form.html.erb

@@ -13,15 +13,29 @@
                   display_name: "Release Scores?",
                   help_text: "Check to release autograded scores to students immediately after autograding (strongly recommended)." %>
 
-  <%= f.file_field :makefile, label_text: "Autograder Makefile", action: :upload %>
-  <%= f.file_field :tar, label_text: "Autograder Tar", action: :upload %>
+  <% help_tar_text = "Tar file exists, upload a file to override." %>
+  <% help_makefile_text = "Makefile exists, upload a file to override." %>
+  <%= f.file_field :makefile, label_text: "Autograder Makefile", action: :upload, file_exists_text: help_makefile_text, class: "form-file-field", file_exists: @makefile_exists %>
+  <%= f.file_field :tar, label_text: "Autograder Tar", action: :upload, file_exists_text: help_tar_text, class: "form-file-field", file_exists: @tar_exists %>
   <p class="help-block">
     Both of the above files will be renamed upon upload.
   </p>
 
+  <%= f.fields_for :assessment, @assessment do |af| %>
+    <%= af.check_box :disable_network, help_text: "Disable network access for autograding containers." %>
+  <% end %>
+
   <%= f.submit "Save Settings" %>
 
   <%= link_to "Delete Autograder", course_assessment_autograder_path(@course, @assessment),
               method: :delete, class: "btn danger",
               data: { confirm: "Are you sure you want to delete the Autograder for this assesssment?" } %>
+
+  <% unless @makefile_exists.nil? %>
+    <%= link_to "Download Makefile", download_file_course_assessment_autograder_path(file_path: @makefile_exists, file_key: 'makefile'), class: "btn" %>
+  <% end %>
+
+  <% unless @tar_exists.nil? %>
+    <%= link_to "Download Tar", download_file_course_assessment_autograder_path(file_path: @tar_exists, file_key: 'tar'), class: "btn" %>
+  <% end %>
 <% end %>

config/routes.rb

@@ -151,7 +151,9 @@
     resources :attachments
 
     resources :assessments, param: :name, except: :update do
-      resource :autograder, except: [:new, :show]
+      resource :autograder, except: [:new, :show] do
+        get "download_file"
+      end
       resources :assessment_user_data, only: [:edit, :update]
       resources :attachments
       resources :extensions, only: [:index, :create, :destroy]