Merge pull request #4580 from mishaschwartz/v1.9.1

v1.9.1

Changelog.md

@@ -1,6 +1,12 @@
 # Changelog
 
-## [1.9.0]
+## [v1.9.1]
+- Fixed bug where the output column was not shown in the test results table if the first row had no output (#4537)
+- Fixed N+1 queries in Assignment repo list methods (#4543)
+- Fixed submission download_repo_list file extension (#4543)
+- Fixed bug preventing creation of assignments with submission rules (#4557)
+
+## [v1.9.0]
 - Added option to anonymize group membership when viewed by graders (#4331)
 - Added option to only display assigned criteria to graders as opposed to showing unassigned criteria but making them
   ungradeable (#4331)

app/assets/javascripts/Components/Result/annotation_table.jsx

@@ -6,6 +6,7 @@ export class AnnotationTable extends React.Component {
   columns = [
     {
       Header: '#',
+      accessor: 'number',
       id: 'number',
       maxWidth: 50,
       resizeable: false,
@@ -31,6 +32,7 @@ export class AnnotationTable extends React.Component {
     },
     {
       Header: I18n.t('attributes.filename'),
+      accessor: 'filename',
       id: 'filename',
       Cell: row => {
         let full_path = row.original.path ? row.original.path + '/' + row.original.filename : row.original.filename;

app/assets/javascripts/Components/test_run_table.jsx

@@ -120,10 +120,18 @@ class TestGroupResultTable extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      show_output: props.data[0] && 'test_results.output' in props.data[0]
+      show_output: this.showOutput(props.data)
     };
   }
 
+  showOutput = (data) => {
+    if (data) {
+      return data.some((row) => 'test_results.output' in row);
+    } else {
+      return false;
+    }
+  };
+
   columns = () => [
     {
       id: 'test_group_name',

app/controllers/assignments_controller.rb

@@ -612,9 +612,6 @@ def process_assignment_form(assignment)
       # Is the instructor forming groups?
       if assignment_params[:assignment_properties_attributes][:student_form_groups] == '0'
         assignment.invalid_override = true
-        # Increase group_max so that create_all_groups button is not displayed
-        # in the groups view.
-        assignment.group_max = 2
       else
         assignment.student_form_groups = true
         assignment.invalid_override = false
@@ -759,18 +756,6 @@ def assignment_params
         :_destroy,
         :id,
         :filename
-      ],
-      submission_rule_attributes: [
-        :_destroy,
-        :id,
-        :type,
-        { periods_attributes: [
-          :id,
-          :deduction,
-          :interval,
-          :hours,
-          :_destroy
-        ] }
       ]
     )
   end

app/views/assignments/_form.html.erb

@@ -335,7 +335,7 @@
             </tr>
             </thead>
             <tbody>
-              <% if rule.object.type.to_s == 'PenaltyDecayPeriodSubmissionRule' %>
+              <% if rule.object.type.to_s == 'PenaltyDecayPeriodSubmissionRule' && !rule.object.periods.empty? %>
                   <%= rule.fields_for :periods do |period_form| %>
                       <%
                         # The below conditional is meant to filter out periods that do not match
@@ -376,7 +376,7 @@
             </tr>
             </thead>
             <tbody>
-              <% if rule.object.type.to_s == 'PenaltyPeriodSubmissionRule' %>
+              <% if rule.object.type.to_s == 'PenaltyPeriodSubmissionRule' && !rule.object.periods.empty? %>
                   <%= rule.fields_for :periods do |period_form| %>
                       <%
                         # The below conditional is meant to filter out periods that do not match

app/views/criteria/new.js.erb

@@ -6,13 +6,13 @@ document.getElementById('new_criterion_prompt').focus();
 
 $(document).ready(function() {
   $('input[name=criterion_type]:radio').change(function () {
-
+    const max_mark = document.getElementById('max_mark_prompt');
     if (this.value === 'RubricCriterion') {
-      document.getElementById('max_mark_prompt').value = <%= RubricCriterion::DEFAULT_MAX_MARK -%>;
+      max_mark.defaultValue = <%= RubricCriterion::DEFAULT_MAX_MARK -%>;
     } else if (this.value === 'FlexibleCriterion') {
-      document.getElementById('max_mark_prompt').value = <%= FlexibleCriterion::DEFAULT_MAX_MARK -%>;
+      max_mark.defaultValue = <%= FlexibleCriterion::DEFAULT_MAX_MARK -%>;
     } else if (this.value === 'CheckboxCriterion') {
-      document.getElementById('max_mark_prompt').value = <%= CheckboxCriterion::DEFAULT_MAX_MARK -%>;
+      max_mark.defaultValue = <%= CheckboxCriterion::DEFAULT_MAX_MARK -%>;
     }
   });
 });

spec/controllers/assignments_controller_spec.rb

@@ -414,4 +414,101 @@
       end
     end
   end
+
+  context '#update' do
+    let(:assignment) { create :assignment }
+    let(:admin) { create :admin }
+    let(:submission_rule) { create :penalty_decay_period_submission_rule, assignment: assignment }
+    let(:params) do
+      {
+        id: assignment.id,
+        is_group_assignment: true,
+        is_hidden: 0,
+        assignment: {
+          assignment_properties_attributes: {
+            scanned_exam: false,
+            section_due_dates_type: 0,
+            allow_web_submits: 0,
+            vcs_submit: 0,
+            display_median_to_students: 0,
+            display_grader_names_to_students: 0,
+            has_peer_review: 0,
+            student_form_groups: 0,
+            group_min: 1,
+            group_max: 1,
+            group_name_autogenerated: 1,
+            allow_remarks: 0,
+            id: assignment.id
+          },
+          short_identifier: assignment.short_identifier,
+          description: 'Test',
+          message: '',
+          due_date: Time.now.to_s
+        },
+        submission_rule_attributes: {
+          type: 'PenaltyDecayPeriodSubmissionRule',
+          periods_attributes: {
+            submission_rule.id => {
+              deduction: 10.0,
+              interval: 1.0,
+              hours: 10.0,
+              _destroy: 0,
+              id: submission_rule.id
+            }
+          }
+        }
+      }
+    end
+    it 'should update an assignment without errors' do
+      patch_as admin, :update, params: params
+    end
+    shared_examples 'update assignment_properties' do |property, before, after|
+      it "should update #{property}" do
+        assignment.update!(property => before)
+        params[:assignment][:assignment_properties_attributes][property] = after
+        patch_as admin, :update, params: params
+        expect(assignment.reload.assignment_properties[property]).to eq after
+      end
+    end
+    shared_examples 'update assignment' do |property, before, after|
+      it "should update #{property}" do
+        assignment.update!(property => before)
+        params[:assignment][property] = after
+        patch_as admin, :update, params: params
+        expect(assignment.reload[property]).to eq after
+      end
+    end
+    it_behaves_like 'update assignment_properties', :section_due_dates_type, false, true
+    it_behaves_like 'update assignment_properties', :allow_web_submits, false, true
+    it_behaves_like 'update assignment_properties', :vcs_submit, false, true
+    it_behaves_like 'update assignment_properties', :display_median_to_students, false, true
+    it_behaves_like 'update assignment_properties', :display_grader_names_to_students, false, true
+    it_behaves_like 'update assignment_properties', :has_peer_review, false, true
+    it_behaves_like 'update assignment_properties', :student_form_groups, false, true
+    it_behaves_like 'update assignment_properties', :group_name_autogenerated, false, true
+    it_behaves_like 'update assignment_properties', :allow_remarks, false, true
+    it_behaves_like 'update assignment', :description, 'AAA', 'BBB'
+    it_behaves_like 'update assignment', :message, 'AAA', 'BBB'
+    it_behaves_like 'update assignment', :due_date, Time.now.to_s, (Time.now - 1.hour).to_s
+    it 'should update group_min and group_max when is_group_assignment is true' do
+      assignment.update!(group_min: 1, group_max: 1)
+      params[:assignment][:assignment_properties_attributes][:group_min] = 2
+      params[:assignment][:assignment_properties_attributes][:group_max] = 3
+      params[:is_group_assignment] = true
+      patch_as admin, :update, params: params
+      assignment.reload
+      expect(assignment.assignment_properties[:group_min]).to eq 2
+      expect(assignment.assignment_properties[:group_max]).to eq 3
+    end
+    it 'should not update group_min and group_max when is_group_assignment is false' do
+      assignment.update!(group_min: 1, group_max: 1)
+      params[:assignment][:assignment_properties_attributes][:group_min] = 2
+      params[:assignment][:assignment_properties_attributes][:group_max] = 3
+      params[:is_group_assignment] = false
+      patch_as admin, :update, params: params
+      assignment.reload
+      expect(assignment.assignment_properties[:group_min]).to eq 1
+      expect(assignment.assignment_properties[:group_max]).to eq 1
+    end
+  end
 end

spec/support/authentication_helper.rb

@@ -30,6 +30,12 @@ def put_as(user, action, params: {}, format: nil, session: {})
     put action, xhr: true, params: params, format: format, session: session
   end
 
+  # Performs PATCH request as the supplied user for authentication
+  def patch_as(user, action, params: {}, format: nil, session: {})
+    sign_in user
+    patch action, xhr: true, params: params, format: format, session: session
+  end
+
   # Performs DELETE request as the supplied user for authentication
   def delete_as(user, action, params: {}, format: nil, session: {})
     sign_in user