Support more options for scanned exam duplicate pages

Changelog.md

@@ -29,6 +29,7 @@
 - Add icons to submission and result grading action buttons (#6666)
 - Remove group name maximum length constraint (#6668)
 - Fix bug where in some cases flash messages were not being rendered correctly (#6670)
+- Enable duplicate pages when uploading scanned exams to overwrite existing pages or be ignored (#6674)
 
 ## [v2.2.3]
 - Fix bug where in some circumstances the wrong result would be displayed to students (#6465)

app/controllers/exam_templates_controller.rb

@@ -154,7 +154,8 @@ def split
       head :bad_request
       return
     else
-      current_job = exam_template.split_pdf(split_exam.path, split_exam.original_filename, current_role)
+      current_job = exam_template.split_pdf(split_exam.path, split_exam.original_filename, current_role,
+                                            params[:on_duplicate])
       session[:job_id] = current_job.job_id
     end
     respond_to do |format|

app/jobs/split_pdf_job.rb

@@ -13,7 +13,7 @@ def self.show_status(status)
     status.update(exam_name: "#{job.arguments[0].name} (#{job.arguments[3]})")
   end
 
-  def perform(exam_template, _path, split_pdf_log, _original_filename = nil, _current_role = nil)
+  def perform(exam_template, _path, split_pdf_log, _original_filename = nil, _current_role = nil, on_duplicate = nil)
     m_logger = MarkusLogger.instance
     begin
       # Create directory for files whose QR code couldn't be parsed
@@ -79,7 +79,7 @@ def perform(exam_template, _path, split_pdf_log, _original_filename = nil, _curr
         end
         progress.increment
       end
-      num_complete = save_pages(exam_template, partial_exams, filename, split_pdf_log)
+      num_complete = save_pages(exam_template, partial_exams, filename, split_pdf_log, on_duplicate)
       num_incomplete = partial_exams.length - num_complete
 
       split_pdf_log.update(
@@ -98,7 +98,7 @@ def perform(exam_template, _path, split_pdf_log, _original_filename = nil, _curr
   end
 
   # Save the pages into groups for this assignment
-  def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil)
+  def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil, on_duplicate = nil)
     return unless exam_template.course.instructors.exists?
     complete_dir = File.join(exam_template.base_path, 'complete')
     incomplete_dir = File.join(exam_template.base_path, 'incomplete')
@@ -141,18 +141,25 @@ def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil
           group: group,
           split_pdf_log: split_pdf_log
         )
-        # if a page already exists, move the page to error directory instead of overwriting it
-        if File.exist?(File.join(destination, "#{page_num}.pdf"))
-          new_pdf.save File.join(error_dir, "#{split_page.id}.pdf")
-          status = "ERROR: #{exam_template.name}: exam number #{exam_num}, page #{page_num} already exists"
-        else
+        if !File.exist?(File.join(destination, "#{page_num}.pdf")) || on_duplicate == 'overwrite'
+          # if the page already exists and on_duplicate == 'overwrite', overwrite the page,
+          # and indicate in page status
+          status = File.exist?(File.join(destination, "#{page_num}.pdf")) ? '(Overwritten) ' : ''
           new_pdf.save File.join(destination, "#{page_num}.pdf")
+
           # set status depending on whether parent directory of destination is complete or incomplete
           if File.dirname(destination) == complete_dir
-            status = 'Saved to complete directory'
+            status += 'Saved to complete directory'
           else
-            status = 'Saved to incomplete directory'
+            status += 'Saved to incomplete directory'
           end
+        elsif File.exist?(File.join(destination, "#{page_num}.pdf")) && on_duplicate == 'ignore'
+          # if the page already exists and on_duplicate == 'ignore', ignore the page
+          status = 'Duplicate page ignored'
+        else
+          # if the page already exists and on_duplicate is anything else, move the page to error directory
+          new_pdf.save File.join(error_dir, "#{split_page.id}.pdf")
+          status = "ERROR: #{exam_template.name}: exam number #{exam_num}, page #{page_num} already exists"
         end
         # update status of page
         split_page.update(status: status)

app/models/exam_template.rb

@@ -65,7 +65,7 @@ def delete_with_file
   end
 
   # Split up PDF file based on this exam template.
-  def split_pdf(path, original_filename = nil, current_role = nil)
+  def split_pdf(path, original_filename = nil, current_role = nil, on_duplicate = nil)
     basename = File.basename path, '.pdf'
     filename = original_filename.nil? ? basename : File.basename(original_filename)
     pdf = CombinePDF.load path
@@ -86,7 +86,7 @@ def split_pdf(path, original_filename = nil, current_role = nil)
     FileUtils.mkdir_p raw_dir
     FileUtils.cp path, File.join(raw_dir, "raw_upload_#{split_pdf_log.id}.pdf")
 
-    SplitPdfJob.perform_later(self, path, split_pdf_log, original_filename, current_role)
+    SplitPdfJob.perform_later(self, path, split_pdf_log, original_filename, current_role, on_duplicate)
   end
 
   def fix_error(filename, exam_num, page_num, upside_down)

app/views/exam_templates/_split_form.html.erb

@@ -10,5 +10,20 @@
     <%= f.label :pdf_to_split, t('exam_templates.upload_scans.instruction') %>
     <%= f.file_field :pdf_to_split, required: true, accept: 'application/pdf' %>
   </div>
+  <p>
+    <%= I18n.t('exam_templates.upload_scans.on_duplicate.instruction') %>
+  </p>
+  <p>
+    <%= f.radio_button :on_duplicate, 'overwrite', checked: true %>
+    <%= f.label :on_duplicate, I18n.t('exam_templates.upload_scans.on_duplicate.overwrite'), value: 'overwrite' %>
+  </p>
+  <p>
+    <%= f.radio_button :on_duplicate, 'ignore' %>
+    <%= f.label :on_duplicate, I18n.t('exam_templates.upload_scans.on_duplicate.ignore'), value: 'ignore' %>
+  </p>
+  <p>
+    <%= f.radio_button :on_duplicate, 'error' %>
+    <%= f.label :on_duplicate, I18n.t('exam_templates.upload_scans.on_duplicate.error'), value: 'error' %>
+  </p>
   <p><%= f.submit t('upload'), id: 'split_exam' %></p>
 <% end %>

config/locales/views/exam_templates/en.yml

@@ -90,6 +90,11 @@ en:
       instruction: Papers to upload (.pdf)
       invalid: Invalid file type
       missing: Missing File
+      on_duplicate:
+        error: Duplicated pages are marked as errors
+        ignore: Duplicated pages are ignored
+        instruction: "When an uploaded page is a duplicate of an existing page:"
+        overwrite: Duplicated pages overwrite existing pages
       search_failure: Exam Template to format upload not found
       success: Scans successfully uploaded
       title: Upload Scans

spec/jobs/split_pdf_job_spec.rb

@@ -132,6 +132,71 @@
     expect(error_dir_entries.length).to eq 1
   end
 
+  context 'when there are duplicated pages' do
+    let(:filename) { 'midterm_scan_102.pdf' }
+    let(:split_pdf_log) do
+      exam_template.split_pdf_logs.create(
+        filename: filename,
+        original_num_pages: 6,
+        num_groups_in_complete: 0,
+        num_groups_in_incomplete: 0,
+        num_pages_qr_scan_error: 0,
+        role: instructor
+      )
+    end
+    let(:split_pdf_log2) do
+      exam_template.split_pdf_logs.create(
+        filename: filename,
+        original_num_pages: 6,
+        num_groups_in_complete: 0,
+        num_groups_in_incomplete: 0,
+        num_pages_qr_scan_error: 0,
+        role: instructor
+      )
+    end
+    before do
+      FileUtils.cp "db/data/scanned_exams/#{filename}",
+                   File.join(exam_template.base_path, 'raw', "raw_upload_#{split_pdf_log.id}.pdf")
+      SplitPdfJob.perform_now(exam_template, '', split_pdf_log, filename, instructor)
+    end
+
+    context 'and on_duplicate = "error"' do
+      it 'marks duplicated pages as errors' do
+        FileUtils.cp "db/data/scanned_exams/#{filename}",
+                     File.join(exam_template.base_path, 'raw', "raw_upload_#{split_pdf_log2.id}.pdf")
+        SplitPdfJob.perform_now(exam_template, '', split_pdf_log2, filename, instructor, on_duplicate: 'error')
+
+        expect(split_pdf_log2.split_pages.where('status LIKE ?', '%already exists').size).to eq 6
+        error_dir_entries = Dir.entries(File.join(exam_template.base_path, 'error')) - %w[. ..]
+        expect(error_dir_entries.length).to eq 6
+      end
+    end
+
+    context 'and on_duplicate = "overwrite"' do
+      it 'overwrites existing pages' do
+        FileUtils.cp "db/data/scanned_exams/#{filename}",
+                     File.join(exam_template.base_path, 'raw', "raw_upload_#{split_pdf_log2.id}.pdf")
+        SplitPdfJob.perform_now(exam_template, '', split_pdf_log2, filename, instructor, on_duplicate: 'overwrite')
+
+        expect(split_pdf_log2.split_pages.where('status LIKE ?', '%Overwritten%').size).to eq 6
+        error_dir_entries = Dir.entries(File.join(exam_template.base_path, 'error')) - %w[. ..]
+        expect(error_dir_entries.length).to eq 0
+      end
+    end
+
+    context 'and on_duplicate = "ignore"' do
+      it 'ignores duplicated pages' do
+        FileUtils.cp "db/data/scanned_exams/#{filename}",
+                     File.join(exam_template.base_path, 'raw', "raw_upload_#{split_pdf_log2.id}.pdf")
+        SplitPdfJob.perform_now(exam_template, '', split_pdf_log2, filename, instructor, on_duplicate: 'ignore')
+
+        expect(split_pdf_log2.split_pages.where(status: 'Duplicate page ignored').size).to eq 6
+        error_dir_entries = Dir.entries(File.join(exam_template.base_path, 'error')) - %w[. ..]
+        expect(error_dir_entries.length).to eq 0
+      end
+    end
+  end
+
   context 'when automatic parsing is enabled' do
     let(:exam_template) { create(:exam_template_with_automatic_parsing) }
     let(:split_pdf_log) do