feat(submissions): submission processing, DOCX feedback and upload rules

app/api/entities/task_definition_entity.rb

@@ -12,6 +12,7 @@ def staff?(my_role)
     expose :abbreviation
     expose :name
     expose :description
+    expose :resubmission_extensions_enabled
     expose :weighting
     expose :target_grade
 
@@ -39,6 +40,7 @@ def staff?(my_role)
     expose :restrict_status_updates, if: ->(unit, options) { staff?(options[:my_role]) }
     expose :group_set_id, expose_nil: false
     expose :has_task_sheet?, as: :has_task_sheet
+    expose :task_sheet_filename
     expose :has_task_resources?, as: :has_task_resources
     expose :has_task_assessment_resources?, as: :has_task_assessment_resources, if: ->(unit, options) { staff?(options[:my_role]) }
     expose :has_task_assessment_script?, as: :has_task_assessment_script, if: ->(unit, options) { staff?(options[:my_role]) }

app/api/entities/task_entity.rb

@@ -18,6 +18,11 @@ class TaskEntity < Grape::Entity
       expose :target_start_date, expose_nil: false
     end
 
+    expose :effective_deadline
+    expose :effective_deadline_reason
+    expose :effective_deadline_source_id
+    expose :effective_deadline_date, format_with: :date_only, expose_nil: false
+
     expose :extensions
     expose :scorm_extensions
 
@@ -32,6 +37,7 @@ class TaskEntity < Grape::Entity
     expose :similarity_flag, unless: :update_only
 
     expose :num_new_comments, unless: :update_only
+    expose :has_feedback, unless: :update_only
 
     # Attributes only included in "update only"
 

app/api/submission/portfolio_api.rb

@@ -13,8 +13,8 @@ class PortfolioApi < Grape::API
 
     desc "Upload documents for inclusion in a project's portfolio"
     params do
-      requires :name,  type: String,                        desc: 'Name of the part being uploaded'
-      requires :kind,  type: String,                        desc: 'The kind of file being uploaded: document, code, or image'
+      requires :name, type: String, desc: 'Name of the part being uploaded'
+      requires :kind,  type: String, values: %w[document code image], desc: 'The kind of file being uploaded: document, code, or image'
       requires :file0, type: File, desc: 'file 0.'
     end
     post '/submission/project/:id/portfolio' do
@@ -34,6 +34,14 @@ class PortfolioApi < Grape::API
         error!({ error: "'#{file[:filename]}': #{file_result[:msg]}" }, 403)
       end
 
+      max_file_size = Doubtfire::Application.config.max_file_size.to_i
+      max_file_size = 10_000_000 if max_file_size <= 0
+      size_in_mb = max_file_size / 1_000_000
+
+      if File.size(file[:tempfile].path) > max_file_size
+        error!({ error: "'#{file[:filename]}' exceeds the #{size_in_mb}MB file limit." }, 413)
+      end
+
       # Move file into place
       result = project.move_to_portfolio(file, name, kind) # returns details of file
 
@@ -43,7 +51,7 @@ class PortfolioApi < Grape::API
     desc 'Remove a file from the portfolio files for a unit'
     params do
       optional :idx,   type: Integer, desc: 'The index of the file'
-      optional :kind,  type: String, desc: 'The kind of file being removed: document, code, or image'
+      optional :kind,  type: String, values: %w[document code image], desc: 'The kind of file being removed: document, code, or image'
       optional :name,  type: String, desc: 'Name of file to remove'
     end
     delete '/submission/project/:id/portfolio' do

app/api/submission/portfolio_evidence_api.rb

@@ -53,6 +53,16 @@ def self.logger
         error!({ error: "This task requires a group submission. Ensure you are in a group for the unit's #{task_definition.group_set.name}" }, 403)
       end
 
+      # A finished task stops accepting new student uploads. Without this the
+      # upload lands, submission_date and file_uploaded_at are rewritten and the
+      # assessed pdf is deleted and regenerated, while only the status transition
+      # is skipped. Staff are still allowed through on purpose, because a tutor
+      # sometimes has to upload on a student's behalf when a file is corrupt or
+      # went to the wrong task.
+      if task.task_submission_closed? && !authorise?(current_user, project, :assess)
+        error!({ error: 'This task is closed for new submissions.' }, 403)
+      end
+
       # Check that prerequisite tasks are in the required minimum submitted state
       prerequisites = task_definition.task_prerequisites
       prerequisites.each do |prerequisite|
@@ -156,16 +166,50 @@ def self.logger
       end
 
       task = project.task_for_task_definition(task_definition)
+      error!({ error: 'A submission for this task was not found.' }, 404) unless task
 
-      if task && PortfolioEvidence.recreate_task_pdf(task)
+      begin
+        task.regenerate_submission!(current_user)
         result = 'done'
-      else
-        result = 'false'
+      rescue ArgumentError => e
+        error!({ error: e.message }, 409)
+      rescue StandardError
+        error!({ error: 'The submitted files are not available to regenerate.' }, 422)
       end
 
       present :result, result, with: Grape::Presenters::Presenter
     end # put
 
+    desc 'Retry a failed or timed-out submission conversion'
+    post '/projects/:id/task_def_id/:task_definition_id/submission/retry' do
+      project = Project.find(params[:id])
+      task_definition = project.unit.task_definitions.find(params[:task_definition_id])
+
+      unless authorise? current_user, project, :reprocess_submission
+        error!({ error: "Not authorised to retry task '#{task_definition.name}'" }, 401)
+      end
+
+      task = project.task_for_task_definition(task_definition)
+      error!({ error: 'A submission for this task was not found.' }, 404) unless task
+
+      begin
+        task.retry_submission_processing!(current_user)
+      rescue ArgumentError => e
+        error!({ error: e.message }, 409)
+      rescue StandardError
+        error!({ error: 'The submitted files are not available to retry.' }, 422)
+      end
+
+      # For a group task the new state was written through other instances.
+      task.reload
+      present task.submission_processing_snapshot.merge(
+        submission_date: task.submission_date,
+        processing_error_code: task.submission_processing_error_code,
+        processing_attempts: task.submission_processing_attempts,
+        task_status: task.task_status.status_key
+      ), with: Grape::Presenters::Presenter
+    end
+
     desc 'Get the timestamps of the last 10 submissions of a task'
     get '/projects/:id/task_def_id/:task_definition_id/submissions/timestamps' do
       project = Project.find(params[:id])
@@ -187,41 +231,89 @@ def self.logger
 
     desc 'Get all retained submission histories for a task'
     get '/projects/:id/task_def_id/:task_definition_id/submission_histories' do
-      project = Project.find(params[:id])
-      task_definition = project.unit.task_definitions.find(params[:task_definition_id])
+      project = Project.find_by(id: params[:id])
+      error!({ error: 'Submission history is not available' }, 404) unless project
 
-      unless authorise? current_user, project, :get_submission
-        error!({ error: "Not authorised to get submission history for task '#{task_definition.name}'" }, 401)
+      unless authorise?(current_user, project, :get_submission)
+        error!({ error: 'Submission history is not available' }, 404)
       end
 
-      task = project.task_for_task_definition(task_definition)
-      unless task
-        error!({ error: 'A submission for this task definition has never been created' }, 404)
+      task_definition = project.unit.task_definitions.find_by(id: params[:task_definition_id])
+      error!({ error: 'Submission history is not available' }, 404) unless task_definition
+
+      task = project.tasks.find_by(task_definition: task_definition)
+      error!({ error: 'Submission history is not available' }, 404) unless task
+
+      student_request = project.student == current_user
+
+      if student_request && !authorise?(current_user, task, :get_submission)
+        error!({ error: 'Submission history is not available' }, 404)
       end
 
-      present task.submission_histories.order(submission_timestamp: :desc),
-              with: Entities::SubmissionHistoryEntity
+      histories = task.submission_histories.sort_by { |history| [-history.submission_timestamp.to_i, -history.id] }
+
+      if student_request
+        archive_pending = SubmissionHistory.pending?(task)
+        latest_submission_at = task.submission_processing_started_at || task.submission_date
+        student_histories = histories.each_with_index.map do |history, index|
+          {
+            id: history.id,
+            version_order: index + 1,
+            current: index.zero? && !archive_pending && latest_submission_at.present? &&
+              history.submission_timestamp.to_i >= latest_submission_at.to_i,
+            submission_timestamp: history.submission_timestamp,
+            status: history.has_submission_files? ? 'available' : 'unavailable'
+          }
+        end
+
+        status 202 if archive_pending
+        present student_histories
+      else
+        present histories, with: Entities::SubmissionHistoryEntity
+      end
     end
 
     desc 'Download a retained submission history archive'
     get '/projects/:id/task_def_id/:task_definition_id/submission_histories/:history_id/files' do
-      project = Project.find(params[:id])
-      task_definition = project.unit.task_definitions.find(params[:task_definition_id])
+      project = Project.find_by(id: params[:id])
+      error!({ error: 'Submission history is not available' }, 404) unless project
 
-      unless authorise? current_user, project.unit, :provide_feedback
-        error!({ error: "Not authorised to get submission history for task '#{task_definition.name}'" }, 401)
+      staff_access = authorise?(current_user, project.unit, :provide_feedback)
+      student_access =
+        project.student == current_user && authorise?(current_user, project, :get_submission)
+
+      unless staff_access || student_access
+        error!({ error: 'Submission history is not available' }, 404)
       end
 
-      task = project.task_for_task_definition(task_definition)
-      history = task&.submission_histories&.find_by(id: params[:history_id])
-      error!({ error: 'Submission history was not found' }, 404) unless history
-      error!({ error: 'Submission history files are not available' }, 404) unless history.has_submission_files?
+      task_definition = project.unit.task_definitions.find_by(id: params[:task_definition_id])
+      error!({ error: 'Submission history is not available' }, 404) unless task_definition
+
+      task = project.tasks.find_by(task_definition: task_definition)
+      error!({ error: 'Submission history is not available' }, 404) unless task
+
+      student_access &&= authorise?(current_user, task, :get_submission)
+
+      unless staff_access || student_access
+        error!({ error: 'Submission history is not available' }, 404)
+      end
+
+      history = task.submission_histories.find_by(id: params[:history_id])
+      error!({ error: 'Submission history is not available' }, 404) unless history
+
+      unless history.has_submission_files?
+        error!({ error: 'Submission history files are not available' }, 404)
+      end
 
       filename = "#{project.student.username}-#{task_definition.abbreviation}-#{history.submission_timestamp}.zip"
 
       content_type 'application/octet-stream'
       header['Content-Disposition'] = "attachment; filename=#{filename}"
-      submission_zip_data = history.submission_zip_data
+      begin
+        submission_zip_data = history.submission_zip_data
+      rescue Zip::Error, Errno::ENOENT, Errno::EACCES
+        error!({ error: 'Submission history files are not available' }, 404)
+      end
       header['Content-Length'] = submission_zip_data.bytesize.to_s
       env['api.format'] = :binary
       body submission_zip_data
@@ -339,7 +431,11 @@ def self.logger
       content_type 'application/octet-stream'
       header['Content-Disposition'] = "attachment; filename=#{filename}"
 
-      submission_zip_data = history.submission_zip_data
+      begin
+        submission_zip_data = history.submission_zip_data
+      rescue Zip::Error, Errno::ENOENT, Errno::EACCES
+        error!({ error: 'Submission history files are not available' }, 404)
+      end
       header['Content-Length'] = submission_zip_data.bytesize.to_s
       env['api.format'] = :binary
       body submission_zip_data

app/helpers/comment_attachment_policy.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+# Shared by the authenticated policy response, validation and storage dispatch.
+# A category never authorises an arbitrary MIME type or filename extension.
+module CommentAttachmentPolicy
+  MAX_BYTES = 30_000_000 # Exclusive, matching the existing comment API.
+  MAX_SELECTION_COUNT = 5
+  CATEGORIES = [
+    { id: 'pdf', name: 'PDF', extensions: %w[pdf], mime_types: %w[application/pdf], preview: 'pdf' },
+    { id: 'document', name: 'Document', extensions: %w[docx], mime_types: [FileHelper::DOCX_MIME_TYPE], preview: 'download' },
+    { id: 'spreadsheet', name: 'Spreadsheet', extensions: %w[csv xlsx], mime_types: %w[text/csv application/csv text/plain application/vnd.openxmlformats-officedocument.spreadsheetml.sheet], preview: 'download' },
+    { id: 'image', name: 'Image', extensions: %w[png bmp tiff tif jpeg jpg gif], mime_types: %w[image/png image/bmp image/x-ms-bmp image/tiff image/jpeg image/gif], preview: 'image' },
+    { id: 'audio', name: 'Audio', extensions: %w[wav ogg mp3 mp4 webm aac pcm aiff flac wma alac], mime_types: %w[audio/ video/webm application/ogg], preview: 'audio' }
+  ].freeze
+
+  def self.category(filename)
+    extension = File.extname(filename.to_s).downcase.delete_prefix('.')
+    CATEGORIES.find { |item| item[:extensions].include?(extension) }
+  end
+
+  def self.public_policy
+    {
+      version: 1,
+      max_bytes_exclusive: MAX_BYTES,
+      max_selection_count: MAX_SELECTION_COUNT,
+      categories: CATEGORIES.map { |item| item.except(:mime_types) }
+    }
+  end
+
+  def self.validate(file)
+    path = file['tempfile'].path
+    filename = file['filename'] || file[:filename]
+    category = category(filename)
+    # MediaRecorder sends a Blob with the browser's default extensionless name.
+    category ||= CATEGORIES.find { |item| item[:id] == 'audio' } if filename == 'blob'
+    return rejected('UPLOAD_EMPTY', 'Attachment is empty.', file) unless File.size?(path)
+    return rejected('UPLOAD_TOO_LARGE', 'Attachment must be smaller than 30 MB.', file) if File.size(path) >= MAX_BYTES
+    return rejected('UPLOAD_EXTENSION_NOT_ALLOWED', 'Unsupported attachment format. Choose a format listed beside Attach a file.', file) unless category
+
+    extension = File.extname(filename).downcase.delete_prefix('.')
+    detected = MimeCheckHelpers.mime_type(path).split(';').first
+    permitted_mimes = case extension
+                      when 'pcm' then %w[audio/L16 audio/x-pcm application/octet-stream]
+                      when 'csv' then %w[text/csv application/csv text/plain]
+                      when 'xlsx' then %w[application/vnd.openxmlformats-officedocument.spreadsheetml.sheet application/zip]
+                      else category[:mime_types]
+                      end
+    return rejected('UPLOAD_MIME_INVALID', 'File contents do not match the selected format.', file) unless permitted_mimes.any? { |mime| detected == mime || (mime.end_with?('/') && detected.start_with?(mime)) }
+
+    result = case extension
+             when 'docx' then FileHelper.validate_docx(path, max_file_size: MAX_BYTES - 1)
+             when 'xlsx' then FileHelper.validate_docx(path, format: 'xlsx', max_file_size: MAX_BYTES - 1)
+             when 'pdf' then FileHelper.validate_pdf(path)
+             when 'csv' then validate_csv(path)
+             else { valid: true }
+             end
+    return rejected(result[:encrypted] ? 'UPLOAD_ENCRYPTED' : 'UPLOAD_CORRUPT', 'The attachment is malformed, encrypted or contains unsupported active content.', file) if !result[:valid] || result[:encrypted]
+
+    Rails.logger.debug('Uploaded file is accepted')
+    { accepted: true, msg: 'success', category: category[:id] }
+  end
+
+  def self.validate_csv(path)
+    # Stream records so a near-limit CSV does not build a second in-memory table.
+    CSV.foreach(path, encoding: 'bom|utf-8') { |row| return { valid: false } if row.any? { |cell| cell&.include?("\0") } }
+    { valid: true }
+  rescue CSV::MalformedCSVError, EncodingError, ArgumentError
+    { valid: false }
+  end
+
+  def self.rejected(code, message, file)
+    reason = { 'UPLOAD_EXTENSION_NOT_ALLOWED' => 'File extension check failed', 'UPLOAD_MIME_INVALID' => 'File MIME check failed' }.fetch(code, 'Upload rejected')
+    FileHelper.log_file_rejection(reason, 'comment_attachment', file, code: code)
+    { accepted: false, code: code, msg: message }
+  end
+end