Merge pull request #7035 from donny-wong/v2.4.9

V2.4.9

Changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## [v2.4.9]
+- Peer review table bug fix to display total marks (#7034)
+- Fix bug preventing deletion of autotest files (#7033)
+
 ## [v2.4.8]
 - Validate user-provided paths (#7025)
 

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.4.8,PATCH_LEVEL=DEV
+VERSION=v2.4.9,PATCH_LEVEL=DEV

app/controllers/automated_tests_controller.rb

@@ -129,7 +129,7 @@ def upload_files
     delete_files = params[:delete_files] || []
     new_files = params[:new_files] || []
     unzip = params[:unzip] == 'true'
-    autotest_files_path = FileHelper.checked_join(assignment.autotest_files_dir, params[:path])
+    autotest_files_path = FileHelper.checked_join(assignment.autotest_files_dir, params[:path] || '')
     if autotest_files_path.nil?
       flash_now(:error, I18n.t('errors.invalid_path'))
       render partial: 'update_files'

app/controllers/peer_reviews_controller.rb

@@ -64,7 +64,7 @@ def populate_table
 
     total_marks = Result.get_total_marks(peer_review_data.pluck('result_id'))
     peer_review_data.each do |data|
-      data[:final_grade] = total_marks['result_id']
+      data[:final_grade] = total_marks[data['result_id']]
       data[:marking_state] = data['results.released_to_students'] ? 'released' : data['results.marking_state']
       data[:max_mark] = assignment.peer_criteria.sum(&:max_mark)
     end

spec/controllers/automated_tests_controller_spec.rb

@@ -284,6 +284,41 @@
           expect(File).to exist(File.expand_path(File.join(assignment.autotest_files_dir, '../../../../../LICENSE')))
         end
       end
+
+      context 'when a valid filename to be deleted is provided' do
+        let(:unzip) { 'false' }
+        let(:zip_file) { fixture_file_upload('test_zip.zip', 'application/zip') }
+        let(:params) do
+          { course_id: assignment.course.id, assignment_id: assignment.id,
+            unzip: unzip, new_files: [zip_file] }
+        end
+        let(:delete_params) do
+          { course_id: assignment.course.id, assignment_id: assignment.id,
+            unzip: false, delete_files: ['test_zip.zip'] }
+        end
+
+        it 'does delete the file' do
+          post_as role, :upload_files, params: delete_params
+          expect(File).not_to exist(File.expand_path(File.join(assignment.autotest_files_dir, 'test_zip.zip')))
+        end
+      end
+
+      context 'when a valid directory to be deleted is provided' do
+        let(:unzip) { 'false' }
+        let(:params) do
+          { course_id: assignment.course.id, assignment_id: assignment.id,
+            unzip: unzip, new_folers: ['hello'], path: '/' }
+        end
+        let(:delete_params) do
+          { course_id: assignment.course.id, assignment_id: assignment.id,
+            unzip: false, delete_folders: ['hello'] }
+        end
+
+        it 'does delete the file' do
+          post_as role, :upload_files, params: delete_params
+          expect(Dir).not_to exist(File.expand_path(File.join(assignment.autotest_files_dir, 'hello')))
+        end
+      end
     end
     context 'GET download_specs' do
       context 'when the assignment has test settings' do

spec/controllers/peer_reviews_controller_spec.rb

@@ -285,4 +285,39 @@
       it('should respond with 403') { expect(response.status).to eq 403 }
     end
   end
+
+  describe 'When listing peer reviews in Peer Reviews tab' do
+    let(:instructor) { create(:instructor) }
+    let(:max_mark) { 3 }
+
+    before do
+      PeerReview.create(reviewer_id: @selected_reviewer_group_ids[0],
+                        result_id: Grouping.find(@selected_reviewee_group_ids[1]).current_result.id)
+      PeerReview.create(reviewer_id: @selected_reviewer_group_ids[1],
+                        result_id: Grouping.find(@selected_reviewee_group_ids[2]).current_result.id)
+      PeerReview.create(reviewer_id: @selected_reviewer_group_ids[2],
+                        result_id: Grouping.find(@selected_reviewee_group_ids[0]).current_result.id)
+    end
+
+    it 'should list out total marks for each peer review' do
+      create_list(:flexible_criterion, 1, assignment: @assignment_with_pr.pr_assignment)
+      @assignment_with_pr.pr_assignment.criteria.first.update(peer_visible: true)
+      @assignment_with_pr.pr_assignment.criteria.first.update(max_mark: max_mark)
+
+      @assignment_with_pr.pr_assignment.groupings.each do |grouping|
+        result = grouping.peer_reviews_to_others.first.result
+        @assignment_with_pr.pr_assignment.criteria.each do |c|
+          mark = c.marks.find_or_create_by(result_id: result.id)
+          mark.update(mark: max_mark)
+        end
+        result.update(marking_state: Result::MARKING_STATES[:complete])
+      end
+
+      response = get_as instructor, :populate_table,
+                        params: { course_id: @assignment_with_pr.pr_assignment.course.id, assignment_id: @pr_id }
+      response_hash = JSON.parse(response.body)
+      final_grades = response_hash.pluck('final_grade')
+      expect(final_grades).to all eq(max_mark)
+    end
+  end
 end