Merge pull request #5888 from pretendWhale/2.0.8

2.0.8

Changelog.md

@@ -3,6 +3,14 @@
 ## [unreleased]
 - Remove unmaintained locales (#5727)
 - Added the ability to submit URLs (#5822)
+- Switch Javascript bundling to jsbundling-rails gem, and update to webpack v5 (#5833)
+- Remove group name displayed attribute from assignment properties table (#5834)
+- Replace uses of first name and last name attributes with display name (#5844)
+
+
+## [v2.0.8]
+- Fix bug where "run tests" grader permission was not working for submission table (#5860)
+- Fix bug for replacing exam template files (#5863)
 
 ## [v2.0.7]
 - Fix bugs in starter files when assigning by section (#5846)

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=2.0.7,PATCH_LEVEL=DEV
+VERSION=2.0.8,PATCH_LEVEL=DEV

app/controllers/exam_templates_controller.rb

@@ -58,10 +58,7 @@ def update
       if new_uploaded_io.content_type != 'application/pdf'
         flash_message(:error, t('exam_templates.update.failure'))
       else
-        old_template_filename = old_exam_template.filename
         old_exam_template.replace_with_file(new_uploaded_io.read,
-                                            assignment_id: assignment.id,
-                                            old_filename: old_template_filename,
                                             new_filename: new_template_filename)
         old_exam_template.update(exam_template_params)
         respond_with(old_exam_template,

app/models/exam_template.rb

@@ -60,12 +60,8 @@ def self.create_with_file(blob, attributes={})
   end
 
   # Replace an ExamTemplate with the correct file
-  def replace_with_file(blob, attributes={})
-    return unless attributes.key? :assessment_id
-
-    File.open(File.join(base_bath, attributes[:new_filename].tr(' ', '_')), 'wb') do |f|
-      f.write blob
-    end
+  def replace_with_file(blob, attributes = {})
+    File.binwrite(File.join(base_path, attributes[:new_filename].tr(' ', '_')), blob)
 
     pdf = CombinePDF.parse blob
     self.update(num_pages: pdf.pages.length, filename: attributes[:new_filename])

app/policies/submission_policy.rb

@@ -26,6 +26,10 @@ def view_files?
     true
   end
 
+  def run_tests?
+    check?(:run_tests?, role)
+  end
+
   def before_release?
     !record.current_result.released_to_students
   end

app/views/results/student/no_remark_result.html.erb

@@ -36,7 +36,7 @@
       <%= link_to t('results.remark.cancel'),
                   { controller: 'results',
                     action: 'cancel_remark_request',
-                    id: @assignment.id,
+                    id: @result.id,
                     submission_id: @submission.id },
                   class: 'button',
                   data: { confirm: t('results.remark.cancel_confirm') },

spec/controllers/exam_templates_controller_spec.rb

@@ -16,14 +16,63 @@
       before { post_as user, :create, params: params }
       it('should respond with 302') { expect(response.status).to eq 302 }
     end
+
     describe '#update' do
-      let(:params) do
-        { exam_template: { name: 'test template' },
-          id: exam_template.id, course_id: course.id }
-      end
       before { put_as user, :update, params: params }
-      it('should respond with 302') { expect(response.status).to eq 302 }
+
+      context 'when updating the exam template name' do
+        let(:params) do
+          { exam_template: { name: 'test-template' },
+            id: exam_template.id, course_id: course.id }
+        end
+
+        it 'updates the exam template name' do
+          expect(exam_template.reload.name).to eq 'test-template'
+        end
+
+        it 'responds with 302' do
+          expect(response).to have_http_status 302
+        end
+      end
+
+      context 'when replacing the exam template file with a PDF file' do
+        let(:file_io) { fixture_file_upload('scanned_exams/midterm1-v2-test.pdf') }
+        let(:params) do
+          { exam_template: { new_template: file_io },
+            id: exam_template.id, course_id: course.id }
+        end
+
+        it 'updates the exam template file' do
+          expect(exam_template.reload.filename).to eq 'midterm1-v2-test.pdf'
+        end
+
+        it 'responds with 302' do
+          expect(response).to have_http_status 302
+        end
+      end
+
+      context 'when replacing the exam template file with a non-PDF file' do
+        let(:file_io) { fixture_file_upload('page_white_text.png') }
+        let(:params) do
+          { exam_template: { new_template: file_io },
+            id: exam_template.id, course_id: course.id }
+        end
+        let!(:original_filename) { exam_template.filename }
+
+        it 'does not update the exam template file' do
+          expect(exam_template.reload.filename).to eq original_filename
+        end
+
+        it 'displays a flash error message' do
+          expect(flash[:error].map { |f| extract_text f }).to eq [I18n.t('exam_templates.update.failure')]
+        end
+
+        it 'responds with 302' do
+          expect(response).to have_http_status 302
+        end
+      end
     end
+
     describe '#destroy' do
       before { delete_as user, :destroy, params: { id: exam_template.id, course_id: course.id } }
       it('should respond with 302') { expect(response.status).to eq 302 }

spec/policies/submission_policy_spec.rb

@@ -29,4 +29,21 @@
       let(:role) { create(:student) }
     end
   end
+
+  describe_rule :run_tests? do
+    succeed 'role is an instructor' do
+      let(:role) { create(:instructor) }
+    end
+    context 'role is a ta' do
+      succeed 'that can run tests' do
+        let(:role) { create :ta, run_tests: true }
+      end
+      failed 'that cannot run tests' do
+        let(:role) { create :ta, run_tests: false }
+      end
+    end
+    failed 'role is a student' do
+      let(:role) { create(:student) }
+    end
+  end
 end