Merge pull request #4659 from mishaschwartz/v1.9.3

v1.9.3

Changelog.md

@@ -1,4 +1,9 @@
 # Changelog
+## [v1.9.3]
+- Fixed inverse association bug with assignments (#4551)
+- Fixed bug preventing graders from downloading submission files from multiple students (#4658)
+- Fixed bug preventing downloading all submission files from git repo (#4658)
+
 ## [v1.9.2]
 - Fixed bug preventing all git hooks from being run in production (#4594)
 - Fixed bug preventing folders from being deleted in file managers (#4605)

Gemfile.lock

@@ -374,7 +374,7 @@ GEM
       semantic_range (>= 2.3.0)
     websocket-driver (0.7.2)
       websocket-extensions (>= 0.1.0)
-    websocket-extensions (0.1.4)
+    websocket-extensions (0.1.5)
     ya2yaml (0.31)
     zeitwerk (2.3.0)
     zxing_cpp (0.1.1)

app/controllers/job_messages_controller.rb

@@ -1,5 +1,5 @@
 class JobMessagesController < ApplicationController
-  before_action :authorize_only_for_admin
+  before_action :authorize_for_ta_and_admin
 
   def get
     status = ActiveJob::Status.get(params[:job_id])

app/controllers/results_controller.rb

@@ -494,7 +494,7 @@ def download_zip
         revision = repo.get_revision(revision_identifier)
         repo.send_tree_to_zip(assignment.repository_folder, zip_file, zip_name, revision) do |file|
           submission_file = files.find_by(filename: file.name, path: file.path)
-          submission_file.retrieve_file(params[:include_annotations] == 'true' && !submission_file.is_supported_image?)
+          submission_file&.retrieve_file(params[:include_annotations] == 'true' && !submission_file.is_supported_image?)
         end
       end
     end

app/models/assignment.rb

@@ -18,21 +18,24 @@ class Assignment < Assessment
            -> { order(:position) },
            class_name: 'RubricCriterion',
            dependent: :destroy,
+           inverse_of: :assignment,
            foreign_key: :assessment_id
 
   has_many :flexible_criteria,
            -> { order(:position) },
            class_name: 'FlexibleCriterion',
            dependent: :destroy,
+           inverse_of: :assignment,
            foreign_key: :assessment_id
 
   has_many :checkbox_criteria,
            -> { order(:position) },
            class_name: 'CheckboxCriterion',
            dependent: :destroy,
+           inverse_of: :assignment,
            foreign_key: :assessment_id
 
-  has_many :test_groups, dependent: :destroy, foreign_key: :assessment_id
+  has_many :test_groups, dependent: :destroy, inverse_of: :assignment, foreign_key: :assessment_id
   accepts_nested_attributes_for :test_groups, allow_destroy: true, reject_if: ->(attrs) { attrs[:name].blank? }
 
   has_many :annotation_categories,
@@ -42,7 +45,7 @@ class Assignment < Assessment
 
   has_many :criterion_ta_associations, dependent: :destroy, foreign_key: :assessment_id
 
-  has_many :assignment_files, dependent: :destroy, foreign_key: :assessment_id
+  has_many :assignment_files, dependent: :destroy, inverse_of: :assignment, foreign_key: :assessment_id
   accepts_nested_attributes_for :assignment_files, allow_destroy: true
   validates_associated :assignment_files
 

app/models/grade_entry_form.rb

@@ -11,6 +11,7 @@ class GradeEntryForm < Assessment
 
   has_many                  :grade_entry_students,
                             dependent: :destroy,
+                            inverse_of: :grade_entry_form,
                             foreign_key: :assessment_id
 
   has_many                  :grades, through: :grade_entry_items

spec/controllers/job_messages_controller_spec.rb

@@ -1,76 +1,85 @@
 describe JobMessagesController do
-  describe '.get' do
-    let(:admin) { create :admin }
-    let(:job) { ApplicationJob.perform_later }
-    let(:status) { ActiveJob::Status.get(job.job_id) }
-    context 'when a job has been enqueued' do
-      before :each do
-        status[:status] = status_type
-        params = { job_id: job.job_id }
-        get_as admin, :get, params: params, session: params
-      end
-      after :each do
-        flash.discard
-      end
-      context 'when the job failed' do
-        let(:status_type) { :failed }
-        it 'should flash an error message' do
-          expect(flash[:error]).not_to be_nil
+  shared_examples 'job messages' do
+    describe '.get' do
+      let(:job) { ApplicationJob.perform_later }
+      let(:status) { ActiveJob::Status.get(job.job_id) }
+      context 'when a job has been enqueued' do
+        before :each do
+          status[:status] = status_type
+          params = { job_id: job.job_id }
+          get_as user, :get, params: params, session: params
         end
-        it 'should set the session[:job_id] to nil' do
-          expect(request.session[:job_id]).to be_nil
+        after :each do
+          flash.discard
         end
-        it 'should remove any progress flash messages' do
-          expect(response.headers['X-Message-Discard']).to include 'notice'
+        context 'when the job failed' do
+          let(:status_type) { :failed }
+          it 'should flash an error message' do
+            expect(flash[:error]).not_to be_nil
+          end
+          it 'should set the session[:job_id] to nil' do
+            expect(request.session[:job_id]).to be_nil
+          end
+          it 'should remove any progress flash messages' do
+            expect(response.headers['X-Message-Discard']).to include 'notice'
+          end
         end
-      end
-      context 'when the job completed successfully' do
-        let(:status_type) { :completed }
-        it 'should flash an success message' do
-          expect(flash[:success]).not_to be_nil
+        context 'when the job completed successfully' do
+          let(:status_type) { :completed }
+          it 'should flash an success message' do
+            expect(flash[:success]).not_to be_nil
+          end
+          it 'should set the session[:job_id] to nil' do
+            expect(request.session[:job_id]).to be_nil
+          end
+          it 'should remove any progress flash messages' do
+            expect(response.headers['X-Message-Discard']).to include 'notice'
+          end
         end
-        it 'should set the session[:job_id] to nil' do
-          expect(request.session[:job_id]).to be_nil
+        context 'when the job is queued' do
+          let(:status_type) { :queued }
+          it 'should flash a notice message' do
+            expect(flash[:notice]).to include "<p>#{I18n.t('poll_job.queued')}</p>"
+          end
+          it 'should not set the session[:job_id] to nil' do
+            expect(request.session[:job_id]).to eq job.job_id
+          end
+          it 'should not remove any progress flash messages' do
+            expect(response.headers['X-Message-Discard']).to be_nil
+          end
         end
-        it 'should remove any progress flash messages' do
-          expect(response.headers['X-Message-Discard']).to include 'notice'
+        context 'when the job is working' do
+          let(:status_type) { :working }
+          it 'should flash a notice message' do
+            expect(flash[:notice]).to include "<p>#{ApplicationJob.show_status(status)}</p>"
+          end
+          it 'should not set the session[:job_id] to nil' do
+            expect(request.session[:job_id]).to eq job.job_id
+          end
+          it 'should not remove any progress flash messages' do
+            expect(response.headers['X-Message-Discard']).to be_nil
+          end
         end
       end
-      context 'when the job is queued' do
-        let(:status_type) { :queued }
-        it 'should flash a notice message' do
-          expect(flash[:notice]).to include "<p>#{I18n.t('poll_job.queued')}</p>"
+      context 'when no job has been enqueued' do
+        before :each do
+          get_as user, :get, params: { job_id: 'a' }
         end
-        it 'should not set the session[:job_id] to nil' do
-          expect(request.session[:job_id]).to eq job.job_id
-        end
-        it 'should not remove any progress flash messages' do
-          expect(response.headers['X-Message-Discard']).to be_nil
-        end
-      end
-      context 'when the job is working' do
-        let(:status_type) { :working }
-        it 'should flash a notice message' do
-          expect(flash[:notice]).to include "<p>#{ApplicationJob.show_status(status)}</p>"
-        end
-        it 'should not set the session[:job_id] to nil' do
-          expect(request.session[:job_id]).to eq job.job_id
+        it 'should flash an error message' do
+          expect(flash[:error]).not_to be_nil
         end
-        it 'should not remove any progress flash messages' do
-          expect(response.headers['X-Message-Discard']).to be_nil
+        it 'should set the session[:job_id] to nil' do
+          expect(request.session[:job_id]).to be_nil
         end
       end
     end
-    context 'when no job has been enqueued' do
-      before :each do
-        get_as admin, :get, params: { job_id: 'a' }
-      end
-      it 'should flash an error message' do
-        expect(flash[:error]).not_to be_nil
-      end
-      it 'should set the session[:job_id] to nil' do
-        expect(request.session[:job_id]).to be_nil
-      end
-    end
+  end
+  context 'as an admin' do
+    let(:user) { create :admin }
+    it_behaves_like 'job messages'
+  end
+  context 'as a grader' do
+    let(:user) { create :ta }
+    it_behaves_like 'job messages'
   end
 end

spec/models/assignment_spec.rb

@@ -103,6 +103,26 @@
     end
   end
 
+  describe 'nested attributes' do
+    xit 'accepts nested attributes for required files (assignment_files)' do # this works on master
+      attrs = {
+        short_identifier: 't',
+        description: 't',
+        due_date: Time.current + 1.hour,
+        assignment_files_attributes: [
+          { filename: 't.py' }
+        ]
+      }
+      a = Assignment.new(attrs)
+      a.repository_folder = 't'
+      a.build_assignment_stat
+      a.build_submission_rule
+      a.save!
+
+      expect(a.assignment_files.first.filename).to eq 't.py'
+    end
+  end
+
   describe '#clone_groupings_from' do
     it 'makes an attempt to update repository permissions when cloning groupings' do
       a1 = create(:assignment, assignment_properties_attributes: { vcs_submit: true })

yarn.lock

@@ -7901,9 +7901,9 @@ websocket-driver@>=0.5.1:
     websocket-extensions ">=0.1.1"
 
 websocket-extensions@>=0.1.1:
-  version "0.1.3"
-  resolved "https://registry.yarnpkg.com/websocket-extensions/-/websocket-extensions-0.1.3.tgz#5d2ff22977003ec687a4b87073dfbbac146ccf29"
-  integrity sha512-nqHUnMXmBzT0w570r2JpJxfiSD1IzoI+HGVdd3aZ0yNi3ngvQ4jv1dtHt5VGxfI2yj5yqImPhOK4vmIh2xMbGg==
+  version "0.1.4"
+  resolved "https://registry.yarnpkg.com/websocket-extensions/-/websocket-extensions-0.1.4.tgz#7f8473bc839dfd87608adb95d7eb075211578a42"
+  integrity sha512-OqedPIGOfsDlo31UNwYbCFMSaO9m9G/0faIHj5/dZFDMFqPTcx6UwqyOy3COEaEOg/9VsGIpdqn62W5KhoKSpg==
 
 which-module@^1.0.0:
   version "1.0.0"