diff --git a/Changelog.md b/Changelog.md index 3ce8d4990f..7f3f0b4f37 100644 --- a/Changelog.md +++ b/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) diff --git a/app/MARKUS_VERSION b/app/MARKUS_VERSION index 0e04a6090c..e61fa9df70 100644 --- a/app/MARKUS_VERSION +++ b/app/MARKUS_VERSION @@ -1 +1 @@ -VERSION=2.0.7,PATCH_LEVEL=DEV +VERSION=2.0.8,PATCH_LEVEL=DEV diff --git a/app/controllers/exam_templates_controller.rb b/app/controllers/exam_templates_controller.rb index ec042604e7..b905ebed04 100644 --- a/app/controllers/exam_templates_controller.rb +++ b/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, diff --git a/app/models/exam_template.rb b/app/models/exam_template.rb index 2eab7433de..fa662ad1a2 100644 --- a/app/models/exam_template.rb +++ b/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]) diff --git a/app/policies/submission_policy.rb b/app/policies/submission_policy.rb index b5a65ba45d..c06f440ed3 100644 --- a/app/policies/submission_policy.rb +++ b/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 diff --git a/app/views/results/student/no_remark_result.html.erb b/app/views/results/student/no_remark_result.html.erb index a4e3a63c2b..2a37042a14 100644 --- a/app/views/results/student/no_remark_result.html.erb +++ b/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') }, diff --git a/spec/controllers/exam_templates_controller_spec.rb b/spec/controllers/exam_templates_controller_spec.rb index bfbb2b154d..d8a2ec9a08 100644 --- a/spec/controllers/exam_templates_controller_spec.rb +++ b/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 } diff --git a/spec/policies/submission_policy_spec.rb b/spec/policies/submission_policy_spec.rb index 368fa59fc2..cc78cb89f2 100644 --- a/spec/policies/submission_policy_spec.rb +++ b/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