Merge pull request #5515 from mishaschwartz/v1.13.2

v1.13.2

Changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [unreleased]
+
+## [v1.13.2]
+- Ensure "Create all groups" button uses existing repos if they already exist (#5504)
+- Set criteria marks after autotest run (#5508)
+
 ## [v1.13.1]
 - Ensure "Create all groups" button creates repositories for students usering their user name (#5499)
 - Support rendering of Markdown in criterion descriptions (#5500)

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=1.13.1,PATCH_LEVEL=DEV
+VERSION=1.13.2,PATCH_LEVEL=DEV

app/assets/javascripts/Components/test_run_table.jsx

@@ -282,10 +282,12 @@ class TestGroupResultTable extends React.Component {
           pivotBy={['test_group_name']}
           getTdProps={ (state, rowInfo) => {
             if (rowInfo) {
-              let className = `test-result-${rowInfo.row['test_status']}`;
-              if (!rowInfo.aggregated &&
-                  (!this.state.show_output || !rowInfo.original['test_results.output'])) {
-                className += ' hide-rt-expander';
+              let className = `-wrap test-result-${rowInfo.row["test_status"]}`;
+              if (
+                !rowInfo.aggregated &&
+                (!this.state.show_output || !rowInfo.original["test_results.output"])
+              ) {
+                className += " hide-rt-expander";
               }
               return {className: className}
             } else {

app/assets/stylesheets/common/core.scss

@@ -358,6 +358,11 @@ label.required::after, th.required::after {
     padding-right: 22px;
   }
 
+  .rt-td.-wrap,
+  .rt-th.-wrap {
+    white-space: initial;
+  }
+
   &.auto-overflow {
     .rt-thead,
     .rt-tbody {

app/jobs/autotest_results_job.rb

@@ -43,11 +43,11 @@ def perform(_retry: 3)
         end
       end
     end
+    outstanding_results
   ensure
     redis = Redis::Namespace.new(Rails.root.to_s)
     if redis.get('autotest_results') == self.job_id
       redis.del('autotest_results')
     end
-    outstanding_results
   end
 end

app/jobs/create_groups_job.rb

@@ -29,10 +29,8 @@ def perform(assignment, data)
           if assignment.is_timed
             grouping = inviter.create_autogenerated_name_group(assignment.id)
           else
-            if assignment.group_max == 1 && group_name == inviter.user_name
-              group = Group.find_or_create_by(group_name: group_name, repo_name: group_name)
-            else
-              group = Group.find_or_create_by(group_name: group_name)
+            group = Group.find_or_create_by(group_name: group_name) do |gr|
+              gr.repo_name = group_name if assignment.group_max == 1 && group_name == inviter.user_name
             end
             grouping = Grouping.find_or_create_by(group: group, assignment: assignment)
             errors += grouping.invite(inviter.user_name, StudentMembership::STATUSES[:inviter], true)

app/models/test_run.rb

@@ -47,6 +47,7 @@ def update_results!(results)
         create_annotations(result['annotations'])
         create_feedback_file(result['feedback'], test_group_result)
       end
+      self.submission&.set_autotest_marks
     end
   end
 

app/views/automated_tests/student_interface.html.erb

@@ -1,9 +1,9 @@
 <% content_for :title, t('automated_tests.title') %>
 <%= stylesheet_link_tag 'result_main' %>
 <%= stylesheet_pack_tag 'result' %>
-<%= javascript_include_tag 'mathjax' %>
-<%= javascript_pack_tag 'result' %>
-<%= javascript_include_tag 'Results/main' %>
+<%= javascript_include_tag 'mathjax', nonce: true %>
+<%= javascript_pack_tag 'result', nonce: true %>
+<%= javascript_include_tag 'Results/main', nonce: true %>
 
 <% if @assignment.enable_student_tests && [email protected]? %>
   <p>

spec/jobs/create_groups_job_spec.rb

@@ -130,6 +130,28 @@
           expect { CreateGroupsJob.perform_now(create(:assignment), data) }.not_to(change { Grouping.count })
         end
       end
+
+      context 'and the assignment does not allow students to work in groups > 1' do
+        let(:assignment) { create :assignment, assignment_properties_attributes: { group_min: 1, group_max: 1 } }
+        context 'and the repo name is different and the group is named after the inviter' do
+          let(:group) { create :group, group_name: group_name, repo_name: 'some_other_repo' }
+          before do
+            create :grouping_with_inviter, group: group, assignment: assignment, inviter: student1
+          end
+          context 'and the group name is the same as the inviter user' do
+            let(:group_name) { student1.user_name }
+            it 'should create a new grouping' do
+              data = [[group.group_name, student1.user_name]]
+              expect { CreateGroupsJob.perform_now(create(:assignment), data) }.to(change { Grouping.count }.by(1))
+            end
+            it 'should use the old repo name' do
+              data = [[group.group_name, student1.user_name]]
+              CreateGroupsJob.perform_now(create(:assignment), data)
+              expect(Grouping.joins(:group).pluck(:repo_name)).to contain_exactly('some_other_repo', 'some_other_repo')
+            end
+          end
+        end
+      end
     end
   end
 end

spec/models/test_run_spec.rb

@@ -42,8 +42,11 @@
     end
   end
   describe '#update_results!' do
-    let(:test_run) { create :test_run, status: :in_progress }
-    let(:test_group) { create(:test_group) }
+    let(:assignment) { create :assignment }
+    let(:grouping) { create :grouping, assignment: assignment }
+    let(:test_run) { create :test_run, status: :in_progress, grouping: grouping }
+    let(:criterion) { create :flexible_criterion, max_mark: 2, assignment: assignment }
+    let(:test_group) { create :test_group, criterion: criterion, assignment: assignment }
     let(:results) do
       JSON.parse({ status: :finished,
                    error: nil,
@@ -75,6 +78,9 @@
       it 'should change the status to failure' do
         expect { test_run.update_results!(results) }.to change { test_run.status }.to('failed')
       end
+      it 'should not update criteria marks' do
+        expect { test_run.update_results!(results) }.not_to(change { criterion.reload.marks.count })
+      end
     end
     context 'there is a success reported' do
       let(:test_group_result) { TestGroupResult.find_by(test_group_id: test_group.id, test_run_id: test_run.id) }
@@ -233,6 +239,22 @@
           expect(TestResult.where(output: 'abc\u0000de')).not_to be_nil
         end
       end
+      context 'when it is associated with a submission' do
+        let(:submission) { create :version_used_submission, grouping: grouping }
+        let(:test_run) { create :test_run, status: :in_progress, submission: submission }
+        it 'should create now criteria marks' do
+          expect { test_run.update_results!(results) }.to(change { criterion.reload.marks })
+        end
+        it 'should set criteria marks' do
+          test_run.update_results!(results)
+          expect(submission.results.first.total_mark).to eq 1
+        end
+      end
+      context 'when it is associated with a grouping' do
+        it 'should not update criteria marks' do
+          expect { test_run.update_results!(results) }.not_to(change { criterion.reload.marks.count })
+        end
+      end
     end
   end
 end