diff --git a/Changelog.md b/Changelog.md index 16b529dae5..92a721e683 100644 --- a/Changelog.md +++ b/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) diff --git a/app/MARKUS_VERSION b/app/MARKUS_VERSION index 8744190e25..a2374ac39d 100644 --- a/app/MARKUS_VERSION +++ b/app/MARKUS_VERSION @@ -1 +1 @@ -VERSION=v2.4.8,PATCH_LEVEL=DEV +VERSION=v2.4.9,PATCH_LEVEL=DEV diff --git a/app/controllers/automated_tests_controller.rb b/app/controllers/automated_tests_controller.rb index c7def48605..a8e1a64534 100644 --- a/app/controllers/automated_tests_controller.rb +++ b/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' diff --git a/app/controllers/peer_reviews_controller.rb b/app/controllers/peer_reviews_controller.rb index 5378513909..d3c61dfc93 100644 --- a/app/controllers/peer_reviews_controller.rb +++ b/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 diff --git a/spec/controllers/automated_tests_controller_spec.rb b/spec/controllers/automated_tests_controller_spec.rb index 0fbf851397..6603ad9759 100644 --- a/spec/controllers/automated_tests_controller_spec.rb +++ b/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 diff --git a/spec/controllers/peer_reviews_controller_spec.rb b/spec/controllers/peer_reviews_controller_spec.rb index 375ba1e64b..abc703aa0a 100644 --- a/spec/controllers/peer_reviews_controller_spec.rb +++ b/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