Merge pull request #7143 from donny-wong/v2.4.12

V2.4.12

Changelog.md

@@ -1,5 +1,16 @@
 # Changelog
 
+## [v2.4.12]
+
+### ✨ New features and improvements
+
+- Added a backend to check MIME type and file extension of uploaded files (#7083)
+
+### 🐛 Bug fixes
+
+- Fix bug in grade display for marks summary (#7125)
+- Remove peer reviews from grade summary table (#7126)
+
 ## [v2.4.11]
 
 ### 🚨 Breaking changes

Gemfile

@@ -63,6 +63,7 @@ gem 'activerecord-session_store'
 gem 'config'
 gem 'cookies_eu'
 gem 'exception_notification'
+gem 'marcel'
 gem 'rails-html-sanitizer'
 gem 'rails_performance'
 gem 'responders'

Gemfile.lock

@@ -496,6 +496,7 @@ DEPENDENCIES
   jwt
   listen
   machinist (< 3)
+  marcel
   mini_mime
   pg
   pluck_to_hash

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.4.11,PATCH_LEVEL=DEV
+VERSION=v2.4.12,PATCH_LEVEL=DEV

app/controllers/course_summaries_controller.rb

@@ -9,7 +9,11 @@ def index; end
 
   def populate
     table_data = get_table_json_data(current_role)
-    assessments = current_role.student? ? current_role.visible_assessments : current_course.assessments
+    if current_role.student?
+      assessments = current_role.visible_assessments.where(parent_assessment_id: nil)
+    else
+      assessments = current_course.assessments.where(parent_assessment_id: nil)
+    end
     marking_schemes = current_role.student? ? MarkingScheme.none : current_course.marking_schemes
 
     average, median, individual, assessment_columns, marking_scheme_columns, graph_labels = [], [], [], [], [], []

app/controllers/submissions_controller.rb

@@ -401,6 +401,13 @@ def update_files
             authorize! to: :manage_subdirectories? # ensure user is authorized for directories in zip files
             success, msgs = add_folder(f, current_role, repo, path: path, txn: txn, required_files: required_files)
           else
+            content_type = Marcel::MimeType.for Pathname.new(f)
+            file_extension = File.extname(f.original_filename).downcase
+            expected_mime_type = Marcel::MimeType.for extension: file_extension
+
+            if content_type != expected_mime_type && content_type != 'application/octet-stream'
+              flash_message(:warning, I18n.t('student.submission.file_extension_mismatch', extension: file_extension))
+            end
             success, msgs = add_file(f, current_role, repo,
                                      path: path, txn: txn, check_size: true, required_files: required_files)
           end

app/helpers/course_summaries_helper.rb

@@ -30,12 +30,13 @@ def get_table_json_data(current_role)
       target[:section_name] = target.delete('sections.name')
       target
     end
-
     assignment_grades = students.joins(accepted_groupings: :current_result)
                                 .where('results.released_to_students': released)
                                 .order(:'results.created_at')
                                 .pluck('roles.id', 'groupings.assessment_id', 'results.id')
-    result_marks = Result.get_total_marks(assignment_grades.map(&:third))
+    results = Result.where(id: assignment_grades.map(&:third)).where.missing(:peer_reviews).ids
+    assignment_grades.select! { |row| results.include?(row[2]) }
+    result_marks = Result.get_total_marks(results)
     assignment_grades.each do |role_id, assessment_id, result_id|
       max_mark = @max_marks[assessment_id]
       mark = result_marks[result_id]

app/models/result.rb

@@ -129,7 +129,6 @@ def self.get_total_extra_marks(result_ids, max_mark: nil, user_visibility: :ta_v
           max_mark_hash[assessment_id] ||= Assignment.find(assessment_id)&.max_mark(user_visibility)
           assignment_max_mark = max_mark_hash[assessment_id]
         end
-        max_mark = max_mark_hash[assessment_id]
         extra_marks_hash[id] += (extra_mark * assignment_max_mark / 100).round(2)
       end
     end

config/locales/en.yml

@@ -35,6 +35,7 @@ en:
       empty_file_warning: "%{file_name} is empty"
       external_submit_only: MarkUs is only accepting external submits
       file_conflicts: 'Your changes have not been made.  The following file conflicts were detected:'
+      file_extension_mismatch: The contents of the uploaded file do not match the file extension %{extension}.
       file_too_large: The size of the uploaded file %{file_name} exceeds the maximum of %{max_size} MB.
       invalid_file_name: 'Invalid file name on submitted file: %{file_name}'
       invalid_folder_name: 'Invalid folder name on submitted folder: %{folder_name}'

spec/controllers/course_summaries_controller_spec.rb

@@ -170,6 +170,75 @@
           end
         end
       end
+
+      context 'when there are peer reviews' do
+        before do
+          assignments = create_list(:assignment_with_criteria_and_results, 3)
+          @pr_assignment = create(:assignment_with_peer_review_and_groupings_results,
+                                  parent_assessment_id: assignments[0].id)
+          create(:complete_result, grouping: @pr_assignment.groupings.first)
+          get_as instructor, :populate, params: { course_id: course.id }, format: :json
+          @response_data = response.parsed_body.deep_symbolize_keys
+          @assessments = @response_data[:assessments]
+        end
+
+        it 'does not return the peer review mark' do
+          expect(@assessments.pluck(:id)).not_to include @pr_assignment.id # rubocop:disable Rails/PluckId
+        end
+      end
+
+      context 'when there are percentage extra_marks' do
+        before do
+          assignments = create_list(:assignment_with_criteria_and_results, 3)
+          create(:grouping_with_inviter_and_submission, assignment: assignments[0])
+          create_list(:grade_entry_form_with_data, 2)
+          create(:grade_entry_form)
+          create(:marking_scheme, assessments: Assessment.all)
+          assignments.first.criteria.first.update!(max_mark: 3.0)
+          assignments.second.criteria.first.update!(max_mark: 2.0)
+          assignments.third.criteria.first.update!(max_mark: 5.0)
+          assignments.each do |assignment|
+            create(:extra_mark, result: assignment.groupings.first.current_result)
+          end
+
+          get_as instructor, :populate, params: { course_id: course.id }, format: :json
+          @response_data = response.parsed_body.deep_symbolize_keys
+          @data = @response_data[:data]
+        end
+
+        it 'returns the correct grades' do
+          expect(@data.length).to eq Student.count
+          Student.find_each do |student|
+            expected = {
+              id: student.id,
+              id_number: student.id_number,
+              user_name: student.user_name,
+              first_name: student.first_name,
+              last_name: student.last_name,
+              section_name: student.section_name,
+              email: student.email,
+              hidden: student.hidden,
+              assessment_marks: GradeEntryForm.all.map do |ges|
+                total_grade = ges.grade_entry_students.find_by(role: student).get_total_grade
+                out_of = ges.grade_entry_items.sum(:out_of)
+                percent = total_grade.nil? || out_of.nil? ? nil : (total_grade * 100 / out_of).round(2)
+                [ges.id.to_s.to_sym, {
+                  mark: total_grade,
+                  percentage: percent
+                }]
+              end.to_h
+            }
+            student.accepted_groupings.each do |g|
+              mark = g.current_result.get_total_mark
+              expected[:assessment_marks][g.assessment_id.to_s.to_sym] = {
+                mark: mark,
+                percentage: (mark * 100 / g.assignment.max_mark).round(2).to_s
+              }
+            end
+            expect(@data.map { |h| h.except(:weighted_marks) }).to include expected
+          end
+        end
+      end
     end
   end
 
@@ -268,6 +337,22 @@
           end
         end
 
+        context 'when there are peer reviews' do
+          before do
+            assignments = create_list(:assignment_with_criteria_and_results, 3)
+            @pr_assignment = create(:assignment_with_peer_review_and_groupings_results,
+                                    parent_assessment_id: assignments[0].id)
+            create(:complete_result, grouping: @pr_assignment.groupings.first)
+            get_as student, :populate, params: { course_id: course.id }, format: :json
+            @response_data = response.parsed_body.deep_symbolize_keys
+            @assessments = @response_data[:assessments]
+          end
+
+          it 'does not return the peer review mark' do
+            expect(@assessments.pluck(:id)).not_to include @pr_assignment.id # rubocop:disable Rails/PluckId
+          end
+        end
+
         context 'when display_median_to_students not set for any assignment' do
           it_behaves_like 'check_graph_data'
         end

spec/controllers/submissions_controller_spec.rb

@@ -116,6 +116,55 @@
       end
     end
 
+    it 'should check for MIME type and extension that match' do
+      expect(@student).to have_accepted_grouping_for(@assignment.id)
+
+      valid_file = fixture_file_upload('docx_file.docx',
+                                       'application/vnd.openxmlformats-officedocument.wordprocessingml.document')
+      content_type = Marcel::MimeType.for Pathname.new(valid_file)
+      file_extension = File.extname(valid_file.original_filename).downcase
+      expected_mime_type = Marcel::MimeType.for extension: file_extension
+
+      expect(content_type).to eq(expected_mime_type)
+
+      post_as @student, :update_files,
+              params: { course_id: course.id, assignment_id: @assignment.id, new_files: [valid_file] }
+      expect(response).to have_http_status :ok
+
+      # Check to see if the file was added
+      @grouping.group.access_repo do |repo|
+        revision = repo.get_latest_revision
+        files = revision.files_at_path(@assignment.repository_folder)
+        expect(files['docx_file.docx']).not_to be_nil
+      end
+    end
+
+    it 'should check for MIME type and extension that do not match' do
+      expect(@student).to have_accepted_grouping_for(@assignment.id)
+
+      invalid_file = fixture_file_upload('docx_renamed_to_pdf.pdf')
+      content_type = Marcel::MimeType.for(Pathname.new(invalid_file))
+      file_extension = File.extname(invalid_file.original_filename).downcase
+      expected_mime_type = Marcel::MimeType.for(extension: file_extension)
+
+      expect(content_type).not_to eq(expected_mime_type)
+
+      post_as @student, :update_files,
+              params: { course_id: course.id, assignment_id: @assignment.id, new_files: [invalid_file] }
+
+      expect(response).to have_http_status :ok
+      sample_warning_message = I18n.t('student.submission.file_extension_mismatch', extension: file_extension)
+      flash[:warning] = I18n.t('student.submission.file_extension_mismatch', extension: file_extension)
+      expect(flash[:warning]).to eq sample_warning_message
+
+      # Check to see if the file was added
+      @grouping.group.access_repo do |repo|
+        revision = repo.get_latest_revision
+        files = revision.files_at_path(@assignment.repository_folder)
+        expect(files['docx_renamed_to_pdf.pdf']).not_to be_nil
+      end
+    end
+
     it 'cannot add files outside the repository when an invalid path is given' do
       file = fixture_file_upload('Shapes.java', 'text/java')
       bad_path = '../../'