Merge pull request #5912 from pretendWhale/2.0.10

2.0.10

Changelog.md

@@ -8,6 +8,11 @@
 - Replace uses of first name and last name attributes with display name (#5844)
 - Upgrade to Rails 7 (#5885)
 
+## [v2.0.10]
+- Fix bug when sorting batch test runs where sorting by date was not working (#5906)
+- Ensure tabs in result view do not reload when switching between them (#5910)
+- Fix bug where penalty periods were sometimes incorrectly ordered (#5908)
+
 ## [v2.0.9]
 - Fix bug when downloading all automated test files where the files were saved to a sub directory (#5864)
 - Fix bugs in assigning scanned exams and improve error message when assigning by name (#5895)

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=2.0.9,PATCH_LEVEL=DEV
+VERSION=2.0.10,PATCH_LEVEL=DEV

app/assets/javascripts/Components/Helpers/table_helpers.jsx

@@ -54,7 +54,7 @@ export function dateSort(a, b) {
   } else {
     let a_date = new Date(a);
     let b_date = new Date(b);
-    return (a_date || 0) > (b_date || 0);
+    return (a_date || 0) - (b_date || 0);
   }
 }
 

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

@@ -117,7 +117,7 @@ export class LeftPane extends React.Component {
             />
           </div>
         </TabPanel>
-        <TabPanel>
+        <TabPanel forceRender={!LeftPane.disableTestResultsPanel(this.props)}>
           <div id="testviewer">
             {/* student results page (with instructor tests released) does not need the button */}
             {!this.props.student_view && (
@@ -151,7 +151,7 @@ export class LeftPane extends React.Component {
             />
           </div>
         </TabPanel>
-        <TabPanel>
+        <TabPanel forceRender={!LeftPane.disableFeedbackFilesPanel(this.props)}>
           <FeedbackFilePanel
             assignment_id={this.props.assignment_id}
             feedbackFiles={this.props.feedback_files}
@@ -160,7 +160,7 @@ export class LeftPane extends React.Component {
             loading={this.props.loading}
           />
         </TabPanel>
-        <TabPanel>
+        <TabPanel forceRender={!LeftPane.disableRemarkPanel(this.props)}>
           <div id="remark_request_tab">
             <RemarkPanel
               result_id={this.props.result_id}

app/assets/javascripts/Components/batch_test_run_table.jsx

@@ -5,7 +5,6 @@ import {dateSort} from "./Helpers/table_helpers";
 
 const makeDefaultState = () => ({
   data: [],
-  sorted: [{id: "created_at", desc: true}],
   statuses: {},
   loading: true,
 });
@@ -153,23 +152,7 @@ class BatchTestRunTable extends React.Component {
             },
           ]}
           pivotBy={["created_at"]}
-          // Controlled props
-          sorted={this.state.sorted}
-          // Callbacks
-          onSortedChange={sorted => this.setState({sorted})}
-          // Custom Sort Method to sort by latest batch run
-          defaultSortMethod={(a, b) => {
-            // sorting for created_at_user_name to ensure it's sorted by date
-            if (this.state.sorted[0].id === "created_at") {
-              if (typeof a === "string" && typeof b === "string") {
-                let a_date = Date.parse(a);
-                let b_date = Date.parse(b);
-                return a_date > b_date ? 1 : -1;
-              }
-            } else {
-              return a > b ? 1 : -1;
-            }
-          }}
+          defaultSorted={[{id: "created_at", desc: true}]}
           loading={this.state.loading}
         />
       </div>

app/models/period.rb

@@ -4,6 +4,9 @@ class Period < ApplicationRecord
   validates_numericality_of :hours, greater_than: 0
   validates_numericality_of :deduction, greater_than_or_equal_to: 0, if: :check_deduction
   validates_numericality_of :interval, greater_than: 0, if: :check_interval
+  # Order by id to ensure consistent ordering of periods.
+  # TODO: change this to order by created_at or add explicit ordering in the database.
+  default_scope { order(id: :asc) }
 
   before_create -> { self.submission_rule_type = submission_rule.type }
 

app/views/assignments/_read.html.erb

@@ -46,7 +46,7 @@
         <% acc = 0 %>
         <% if @assignment.submission_rule.type == "GracePeriodSubmissionRule" %>
           <% remaining_credits = @grouping.available_grace_credits %>
-          <% @assignment.submission_rule.periods.order(:id).each do |p| %>
+          <% @assignment.submission_rule.periods.each do |p| %>
             <% unless remaining_credits <= 0 %>
               <% acc += p.hours %>
               <% remaining_credits -= 1 %>
@@ -64,7 +64,7 @@
             <strong>
               <%= t('penalty_period_submission_rules.deadline_html') %>:
             </strong>
-            <% @assignment.submission_rule.periods.order(:id).each { |p| acc += p.hours } %>
+            <% @assignment.submission_rule.periods.each { |p| acc += p.hours } %>
             <% if @grouping.nil? %>
               <%= I18n.l(@assignment.section_due_date(@current_role.try(:section)) + acc.hours) %>
             <% else %>
@@ -84,8 +84,8 @@
             </li>
           <% end %>
         <% elsif @assignment.submission_rule.type == "PenaltyDecayPeriodSubmissionRule" %>
-          <% @assignment.submission_rule.periods.order(:id).each do |p| %>
-            <% if p == @assignment.submission_rule.periods.order(:id).first %>
+          <% @assignment.submission_rule.periods.each do |p| %>
+            <% if p == @assignment.submission_rule.periods.first %>
               <li><%= t('penalty_decay_period_submission_rules.details_message_first',
                         deduction: p.deduction, interval: p.interval, hours: p.hours) %> </li>
             <% else %>
@@ -95,7 +95,7 @@
           <% end %>
         <% elsif @assignment.submission_rule.type == "PenaltyPeriodSubmissionRule" %>
           <% deduction = hours = 0 %>
-          <% @assignment.submission_rule.periods.order(:id).each do |p| %>
+          <% @assignment.submission_rule.periods.each do |p| %>
             <% deduction += p.deduction %>
             <% hours += p.hours %>
             <li><%= t('penalty_period_submission_rules.details_message',

spec/models/period_spec.rb

@@ -42,4 +42,14 @@
     it { is_expected.not_to validate_numericality_of(:interval) }
     include_examples 'has a course'
   end
+
+  context 'Multiple penalty decay periods' do
+    let(:rule) { create :penalty_period_submission_rule }
+    let!(:period) { create :period, id: 2, submission_rule: rule }
+    let!(:period2) { create :period, id: 3, deduction: 0.25, interval: 0.25, hours: 0.25, submission_rule: rule }
+    let!(:period3) { create :period, id: 1, deduction: 1, interval: 1, hours: 10, submission_rule: rule }
+    it 'returns the periods in order' do
+      expect(rule.reload.periods).to eq [period3, period, period2]
+    end
+  end
 end