Merge pull request #6798 from donny-wong/v2.3.3

V2.3.3

.github/workflows/test_ci.yml

@@ -85,8 +85,6 @@ jobs:
         run: bundle exec rails db:migrate
       - name: Install chrome and chromedriver
         uses: nanasess/setup-chromedriver@v2
-        with:
-          chromedriver-version: '115.0.5790.102'
       - name: Run chromedriver
         run: chromedriver --whitelisted-ips &
       - name: Run rspec tests

Changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## [v2.3.3]
+- Fix bug where uploading scanned exam pages with overwriting option selected did not update submission files (#6768)
+- Fix bug: "Download Submissions" download link was not being rendered from partial view (#6779)
+
 ## [v2.3.2]
 - Allow MathJAX to process environments (e.g., align) (#6762)
 

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.3.2,PATCH_LEVEL=DEV
+VERSION=v2.3.3,PATCH_LEVEL=DEV

app/controllers/job_messages_controller.rb

@@ -8,7 +8,12 @@ def get
       session[:job_id] = nil
     elsif status.completed?
       status[:progress] = status[:total]
-      flash_message(:success, status[:job_class].completed_message(status))
+      completed_message = status[:job_class].completed_message(status)
+      if completed_message.is_a?(Hash)
+        flash_message(:success, **completed_message)
+      else
+        flash_message(:success, completed_message)
+      end
       session[:job_id] = nil
     elsif status.read.empty?
       flash_message(:error, t('poll_job.failed'))

app/jobs/download_submissions_job.rb

@@ -10,8 +10,8 @@ def self.show_status(status)
   end
 
   def self.completed_message(status)
-    { partial: 'submissions/download_zip_file', locals: { assignment_id: status[:assignment_id],
-                                                          course_id: status[:course_id] } }
+    { partial: 'submissions/download_zip_file', formats: [:html], locals: { assignment_id: status[:assignment_id],
+                                                                            course_id: status[:course_id] } }
   end
 
   before_enqueue do |job|

app/jobs/split_pdf_job.rb

@@ -130,8 +130,8 @@ def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil
       )
       groupings << grouping
 
-      # Save raw pages
-      if pages.length == exam_template.num_pages
+      # Save raw pages.
+      if File.exist?(File.join(complete_dir, exam_num.to_s)) || pages.length == exam_template.num_pages
         destination = File.join complete_dir, exam_num.to_s
         num_complete += 1
       else
@@ -172,23 +172,31 @@ def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil
         split_page.update(status: status)
       end
 
+      # Get all pages in the destination. This lets us combine newly-scanned pages with
+      # any pages for this group that were previously scanned.
+      destination_pages = Dir.glob(File.join(destination, '*.pdf')).map do |path|
+        [Integer(File.basename(path, '.pdf')), CombinePDF.load(path)]
+      end
+
       grouping.access_repo do |repo|
         assignment_folder = exam_template.assignment.repository_folder
         txn = repo.get_transaction(exam_template.course.instructors.first.user_name)
 
+        revision = repo.get_latest_revision
         # Pages that belong to a division
         exam_template.template_divisions.each do |division|
           new_pdf = CombinePDF.new
-          pages.each do |page_num, page|
+          destination_pages.each do |page_num, page|
             if division.start <= page_num && page_num <= division.end
               new_pdf << page
             end
           end
-          if File.exist? File.join(assignment_folder, "#{division.label}.pdf")
+          if revision.path_exists? File.join(assignment_folder, "#{division.label}.pdf")
             txn.replace(File.join(assignment_folder,
                                   "#{division.label}.pdf"),
                         new_pdf.to_pdf,
-                        'application/pdf')
+                        'application/pdf',
+                        revision.revision_identifier)
           else
             txn.add(File.join(assignment_folder,
                               "#{division.label}.pdf"),
@@ -198,7 +206,7 @@ def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil
         end
 
         # Pages that don't belong to any division
-        extra_pages = pages.reject do |page_num, _|
+        extra_pages = destination_pages.reject do |page_num, _|
           exam_template.template_divisions.any? do |division|
             division.start <= page_num && page_num <= division.end
           end
@@ -211,25 +219,29 @@ def save_pages(exam_template, partial_exams, filename = nil, split_pdf_log = nil
           cover_pdf << extra_pages[0][1]
           start_page = 1
         end
-        extra_pdf << extra_pages[start_page..extra_pages.size].collect { |_, page| page }
+        extra_pages[start_page..extra_pages.size].each do |_, pdf|
+          extra_pdf << pdf
+        end
 
-        if File.exist? File.join(assignment_folder, 'EXTRA.pdf')
+        if revision.path_exists? File.join(assignment_folder, 'EXTRA.pdf')
           txn.replace(File.join(assignment_folder,
                                 'EXTRA.pdf'),
                       extra_pdf.to_pdf,
-                      'application/pdf')
+                      'application/pdf',
+                      revision.revision_identifier)
         else
           txn.add(File.join(assignment_folder,
                             'EXTRA.pdf'),
                   extra_pdf.to_pdf,
                   'application/pdf')
         end
 
-        if File.exist? File.join(assignment_folder, 'COVER.pdf')
+        if revision.path_exists? File.join(assignment_folder, 'COVER.pdf')
           txn.replace(File.join(assignment_folder,
                                 'COVER.pdf'),
                       cover_pdf.to_pdf,
-                      'application/pdf')
+                      'application/pdf',
+                      revision.revision_identifier)
         else
           txn.add(File.join(assignment_folder,
                             'COVER.pdf'),

spec/jobs/split_pdf_job_spec.rb

@@ -6,6 +6,8 @@
     FileUtils.mkdir_p File.join(exam_template.base_path, 'raw')
     FileUtils.mkdir_p File.join(exam_template.base_path, 'error')
     FileUtils.rm_rf(File.join(exam_template.base_path, 'error'))
+    FileUtils.rm_rf(File.join(exam_template.base_path, 'complete'))
+    FileUtils.rm_rf(File.join(exam_template.base_path, 'incomplete'))
   end
 
   it 'correctly splits a PDF with 20 valid test papers, where all pages are in order' do
@@ -197,6 +199,139 @@
     end
   end
 
+  context 'when an uploaded test is missing pages, including the cover page' do
+    # File contains pages 2, 4, 6
+    # Template division: Q1: 3-3, Q2: 4-4, Q3: 5-6
+    let(:group) { Group.find_by(group_name: 'midterm1-v2-test_paper_104') }
+    let(:grouping) { exam_template.assignment.groupings.find_by(group_id: group.id) }
+
+    let!(:_job) do
+      filename = 'midterm_scan_104_evens.pdf'
+      split_pdf_log = exam_template.split_pdf_logs.create(
+        filename: filename,
+        original_num_pages: 3,
+        num_groups_in_complete: 0,
+        num_groups_in_incomplete: 0,
+        num_pages_qr_scan_error: 0,
+        role: instructor
+      )
+      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
+
+    it 'creates a blank COVER.pdf' do
+      grouping.access_repo do |repo|
+        revision = repo.get_latest_revision
+        cover_pdf_raw = repo.download_as_string(
+          revision.files_at_path(exam_template.assignment.repository_folder)['COVER.pdf']
+        )
+        cover_pdf = CombinePDF.parse(cover_pdf_raw)
+        expect(cover_pdf.pages.size).to eq 0
+      end
+    end
+
+    it 'creates partial template division pdfs' do
+      grouping.access_repo do |repo|
+        revision = repo.get_latest_revision
+        expected_pages = { 'Q1.pdf' => 0, 'Q2.pdf' => 1, 'Q3.pdf' => 1 }
+        files = revision.files_at_path(exam_template.assignment.repository_folder)
+        expected_pages.each do |filename, expected_page_num|
+          expect(files.key?(filename)).to be true
+          pdf_raw = repo.download_as_string(
+            revision.files_at_path(exam_template.assignment.repository_folder)[filename]
+          )
+          pdf = CombinePDF.parse(pdf_raw)
+          expect(pdf.pages.size).to eq expected_page_num
+        end
+      end
+    end
+
+    it 'creates a complete EXTRA.pdf' do
+      grouping.access_repo do |repo|
+        revision = repo.get_latest_revision
+        pdf_raw = repo.download_as_string(
+          revision.files_at_path(exam_template.assignment.repository_folder)['EXTRA.pdf']
+        )
+        pdf = CombinePDF.parse(pdf_raw)
+        expect(pdf.pages.size).to eq 1
+      end
+    end
+  end
+
+  context 'when an uploaded test has pages in two different files' do
+    # Files contains pages [1, 3, 5] and [2, 4, 6]
+    # Template division: Q1: 3-3, Q2: 4-4, Q3: 5-6
+    let(:group) { Group.find_by(group_name: 'midterm1-v2-test_paper_104') }
+    let(:grouping) { exam_template.assignment.groupings.find_by(group_id: group.id) }
+
+    let!(:_job) do
+      filename = 'midterm_scan_104_evens.pdf'
+      split_pdf_log1 = exam_template.split_pdf_logs.create(
+        filename: filename,
+        original_num_pages: 3,
+        num_groups_in_complete: 0,
+        num_groups_in_incomplete: 0,
+        num_pages_qr_scan_error: 0,
+        role: instructor
+      )
+      FileUtils.cp "db/data/scanned_exams/#{filename}",
+                   File.join(exam_template.base_path, 'raw', "raw_upload_#{split_pdf_log1.id}.pdf")
+      SplitPdfJob.perform_now(exam_template, '', split_pdf_log1, filename, instructor)
+
+      filename = 'midterm_scan_104_odds.pdf'
+      split_pdf_log2 = exam_template.split_pdf_logs.create(
+        filename: filename,
+        original_num_pages: 3,
+        num_groups_in_complete: 0,
+        num_groups_in_incomplete: 0,
+        num_pages_qr_scan_error: 0,
+        role: instructor
+      )
+      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)
+    end
+
+    it 'creates a complete COVER.pdf' do
+      grouping.access_repo do |repo|
+        revision = repo.get_latest_revision
+        cover_pdf_raw = repo.download_as_string(
+          revision.files_at_path(exam_template.assignment.repository_folder)['COVER.pdf']
+        )
+        cover_pdf = CombinePDF.parse(cover_pdf_raw)
+        expect(cover_pdf.pages.size).to eq 1
+      end
+    end
+
+    it 'creates complete template division pdfs' do
+      grouping.access_repo do |repo|
+        revision = repo.get_latest_revision
+        expected_pages = { 'Q1.pdf' => 1, 'Q2.pdf' => 1, 'Q3.pdf' => 2 }
+        files = revision.files_at_path(exam_template.assignment.repository_folder)
+        expected_pages.each do |filename, expected_page_num|
+          expect(files.key?(filename)).to be true
+          pdf_raw = repo.download_as_string(
+            revision.files_at_path(exam_template.assignment.repository_folder)[filename]
+          )
+          pdf = CombinePDF.parse(pdf_raw)
+          expect(pdf.pages.size).to eq expected_page_num
+        end
+      end
+    end
+
+    it 'creates a complete EXTRA.pdf' do
+      grouping.access_repo do |repo|
+        revision = repo.get_latest_revision
+        pdf_raw = repo.download_as_string(
+          revision.files_at_path(exam_template.assignment.repository_folder)['EXTRA.pdf']
+        )
+        pdf = CombinePDF.parse(pdf_raw)
+        expect(pdf.pages.size).to eq 1
+      end
+    end
+  end
+
   context 'when automatic parsing is enabled' do
     let(:exam_template) { create(:exam_template_with_automatic_parsing) }
     let(:split_pdf_log) do