Merge pull request #6748 from donny-wong/v2.3.1

V2.3.1

Changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [v2.3.1]
+- Add filter for empty/non-empty submissions in submissions table (#6711)
+- Fix bug where autotest settings would not appear if there were no assignment criteria (#6718)
+- Added API routes for Extensions API (#6743)
+- Fix premature page reloading when autotest settings are saved (#6744)
+
 ## [v2.3.0]
 - Do not destroy pending group memberships if the group is given an extension (#6582)
 - Add OCR for parsing scanned exam uploads (#6433)

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.3.0,PATCH_LEVEL=DEV
+VERSION=v2.3.1,PATCH_LEVEL=DEV

app/assets/javascripts/Components/autotest_manager.jsx

@@ -35,6 +35,11 @@ class AutotestManager extends React.Component {
                 category: {
                   "ui:title": I18n.t("automated_tests.category"),
                 },
+                extra_info: {
+                  criterion: {
+                    "ui:enumDisabled": [""],
+                  },
+                },
                 feedback_file_names: {
                   "ui:classNames": "feedback-file-names",
                   "ui:options": {orderable: false},
@@ -73,6 +78,7 @@ class AutotestManager extends React.Component {
       non_regenerating_tokens: false,
       unlimited_tokens: false,
       loading: true,
+      submitted: false,
       showFileUploadModal: false,
       showSpecUploadModal: false,
       uploadTarget: undefined,
@@ -93,7 +99,7 @@ class AutotestManager extends React.Component {
       )
     )
       .then(data => data.json())
-      .then(data => this.setState({...data, loading: false}));
+      .then(data => this.setState({...data, loading: false, submitted: false}));
   };
 
   fetchFileDataOnly = () => {
@@ -239,6 +245,7 @@ class AutotestManager extends React.Component {
       },
       schema_form_data: this.state.formData,
     };
+    this.setState({form_changed: false, submitted: true});
     $.post({
       url: Routes.course_assignment_automated_tests_path(
         this.props.course_id,
@@ -247,9 +254,8 @@ class AutotestManager extends React.Component {
       data: JSON.stringify(data),
       processData: false,
       contentType: "application/json",
-    }).then(() => {
-      this.toggleFormChanged(false);
-      window.location.reload();
+    }).then(res => {
+      poll_job(res["job_id"], this.fetchData);
     });
   };
 
@@ -465,7 +471,7 @@ class AutotestManager extends React.Component {
         <p>
           <input
             type="submit"
-            value={I18n.t("save")}
+            value={this.state.submitted ? I18n.t("working") : I18n.t("save")}
             onClick={this.onSubmit}
             disabled={!this.state.form_changed}
           ></input>

app/assets/javascripts/Components/submission_table.jsx

@@ -174,8 +174,29 @@ class RawSubmissionTable extends React.Component {
     {
       Header: I18n.t("submissions.commit_date"),
       accessor: "submission_time",
-      filterable: false,
+      id: "submission_time",
+      filterable: true,
       sortMethod: dateSort,
+      Filter: selectFilter,
+      filterMethod: (filter, row) => {
+        if (filter.value === "all") {
+          return true;
+        } else if (filter.value === "false") {
+          return !row[filter.id];
+        } else {
+          return !!row[filter.id];
+        }
+      },
+      filterOptions: [
+        {
+          value: "true",
+          text: I18n.t("submissions.commit_date_filter.collected"),
+        },
+        {
+          value: "false",
+          text: I18n.t("submissions.commit_date_filter.not_collected"),
+        },
+      ],
     },
     {
       Header: I18n.t("submissions.grace_credits_used"),

app/controllers/api/groups_controller.rb

@@ -305,6 +305,51 @@ def remove_tag
       render 'shared/http_status', locals: { code: '404', message: I18n.t('tags.not_found') }, status: :not_found
     end
 
+    def extension
+      grouping = Grouping.find_by(group_id: params[:id], assignment: params[:assignment_id])
+      case request.method
+      when 'DELETE'
+        if grouping.extension.present?
+          grouping.extension.destroy!
+          # Successfully deleted the extension; render success
+          render 'shared/http_status', locals: { code: '200', message:
+            HttpStatusHelper::ERROR_CODE['message']['200'] }, status: :ok
+        else
+          # cannot delete a non existent extension; render failure
+          render 'shared/http_status', locals: { code: '422', message:
+            HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+        end
+      when 'POST'
+        if grouping.extension.nil?
+          extension_values = extension_params
+          extension_values[:time_delta] = time_delta_params if extension_values[:time_delta].present?
+          grouping.create_extension!(extension_values)
+          # Successfully created the extension record; render success
+          render 'shared/http_status', locals: { code: '201', message:
+            HttpStatusHelper::ERROR_CODE['message']['201'] }, status: :created
+        else
+          # cannot create extension as it already exists; render failure
+          render 'shared/http_status', locals: { code: '422', message:
+            HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+        end
+      when 'PATCH'
+        if grouping.extension.present?
+          extension_values = extension_params
+          extension_values[:time_delta] = time_delta_params if extension_values[:time_delta].present?
+          grouping.extension.update!(extension_values)
+          # Successfully updated the extension record; render success
+          render 'shared/http_status', locals: { code: '200', message:
+            HttpStatusHelper::ERROR_CODE['message']['200'] }, status: :ok
+        else
+          # cannot update extension as it does not exists; render failure
+          render 'shared/http_status', locals: { code: '422', message:
+            HttpStatusHelper::ERROR_CODE['message']['422'] }, status: :unprocessable_entity
+        end
+      end
+    rescue ActiveRecord::RecordInvalid => e
+      render 'shared/http_status', locals: { code: '422', message: e.to_s }, status: :unprocessable_entity
+    end
+
     private
 
     def assignment
@@ -326,5 +371,14 @@ def annotations_params
         :line_start
       ])
     end
+
+    def time_delta_params
+      params = extension_params[:time_delta]
+      Extension::PARTS.sum { |part| params[part].to_i.public_send(part) }
+    end
+
+    def extension_params
+      params.require(:extension).permit({ time_delta: [:weeks, :days, :hours, :minutes] }, :apply_penalty, :note)
+    end
   end
 end

app/controllers/automated_tests_controller.rb

@@ -19,10 +19,8 @@ def update
       flash_message(:error, e.message)
       raise ActiveRecord::Rollback
     end
-    @current_job = AutotestSpecsJob.perform_later(request.protocol + request.host_with_port, assignment, test_specs)
-    session[:job_id] = @current_job.job_id
-    # TODO: the page is not correctly drawn when using render
-    redirect_to action: 'manage', assignment_id: params[:assignment_id]
+    current_job = AutotestSpecsJob.perform_later(request.protocol + request.host_with_port, assignment, test_specs)
+    render json: { job_id: current_job.job_id }
   end
 
   # Manage is called when the Automated Test UI is loaded

app/helpers/automated_tests_helper.rb

@@ -20,7 +20,7 @@ def extra_test_group_schema(assignment)
         },
         criterion: {
           type: :string,
-          enum: criterion_names,
+          enum: (criterion_names.presence || ['']),
           title: Criterion.model_name.human
         }
       },

app/jobs/autotest_specs_job.rb

@@ -4,18 +4,6 @@ def self.show_status(_status)
     I18n.t('poll_job.autotest_specs_job')
   end
 
-  def self.on_complete_js(status)
-    %(
-      window.addEventListener("load", () => {
-        window.autotestManagerComponent.setState({formData: JSON.parse('#{status[:test_specs].to_json}')});
-      })
-    )
-  end
-
-  before_enqueue do |job|
-    self.status.update(test_specs: job.arguments[2])
-  end
-
   def perform(host_with_port, assignment, test_specs)
     ApplicationRecord.transaction do
       update_test_groups_from_specs(assignment, test_specs)

app/views/main/_markus_contributors.html.erb

@@ -24,6 +24,7 @@ Andrew Hernandez,
 Andrew Louis,
 Andrey Kulakevich,
 Angelo Maralit,
+Anis Singh,
 Ante Zheng,
 Anthony Labaere,
 Anthony Le Jallé,
@@ -67,6 +68,7 @@ Dhairya Khara,
 Diane Tam,
 Dina Sabie,
 Dmitry Khabarov,
+Donny Wong,
 Dylan Runkel,
 Ealona Shmoel,
 Egor Philippov,

config/locales/views/submissions/en.yml

@@ -32,6 +32,9 @@ en:
       this_revision: Collect and Grade This Revision
       undo_results_loss_warning: 'This operation will undo the previous Collect All Submissions operation. This should only be done if the previous Collect All Submissions operation was done in error. WARNING: All grading work done since the previous Collect All Submissions operation will be lost.'
     commit_date: Submission Date
+    commit_date_filter:
+      collected: Has files collected
+      not_collected: No files collected
     download_zipped_file:
       file_missing: No file named %{zip_file} found. Please try preparing this file for download again.
     errors:

config/routes.rb

@@ -58,6 +58,11 @@
             put 'update_marking_state'
             delete 'remove_extra_marks'
           end
+          member do
+            post 'extension'
+            patch 'extension'
+            delete 'extension'
+          end
         end
         resources :starter_file_groups, only: [:index, :create]
         member do

doc/markus-contributors.txt

@@ -63,6 +63,7 @@ Dhairya Khara
 Diane Tam
 Dina Sabie
 Dmitry Khabarov
+Donny Wong
 Dylan Runkel
 Ealona Shmoel
 Egor Philippov

spec/controllers/api/groups_controller_spec.rb

@@ -3,6 +3,17 @@
   let(:assignment) { create :assignment }
   let(:group) { create :group }
   let(:tag) { create :tag, course: course }
+  let(:extension_params) do
+    {
+      time_delta: {
+        weeks: rand(1..10),
+        days: rand(1..10),
+        hours: rand(1..10)
+      },
+      apply_penalty: true,
+      note: 'global random notes'
+    }
+  end
   context 'An unauthenticated request' do
     before :each do
       request.env['HTTP_AUTHORIZATION'] = 'garbage http_header'
@@ -33,6 +44,12 @@
       get :add_tag, params: { assignment_id: assignment.id, id: group.id, course_id: course.id, tag_id: tag.id }
       expect(response).to have_http_status(403)
     end
+
+    it 'should fail to authenticate a POST extension request' do
+      post :extension,
+           params: { assignment_id: assignment.id, course_id: course.id, id: group.id, extension: extension_params }
+      expect(response).to have_http_status(403)
+    end
   end
   context 'An authenticated request requesting' do
     let(:grouping) { create :grouping_with_inviter, assignment: assignment }
@@ -658,5 +675,105 @@
         expect(grouping.tags.first).to be(nil)
       end
     end
+
+    context 'Extension' do
+      let!(:grouping) { create :grouping, group: group, assignment: assignment }
+      context 'When an Extension exist' do
+        let!(:extension) do
+          create :extension, grouping: grouping, time_delta: 99_999, note: 'local random notes', apply_penalty: false
+        end
+        context 'POST' do
+          it 'should not work with an existing extension' do
+            post :extension,
+                 params: { assignment_id: assignment.id, course_id: course.id, id: group.id,
+                           extension: extension_params }
+            expect(grouping.reload.extension.time_delta).to eq(99_999)
+          end
+        end
+        context 'PATCH' do
+          it 'should update extension - check note attribute' do
+            patch :extension,
+                  params: { assignment_id: assignment.id, course_id: course.id, id: group.id,
+                            extension: extension_params }
+            expect(grouping.reload.extension.note).to eq(extension_params[:note])
+          end
+          it 'should have correct conversion of time_delta' do
+            patch :extension,
+                  params: { assignment_id: assignment.id, course_id: course.id, id: group.id,
+                            extension: extension_params }
+            params = extension_params[:time_delta]
+            expected_duration = Extension::PARTS.sum { |part| params[part].to_i.public_send(part) }
+            expect(grouping.reload.extension.time_delta).to eq(expected_duration)
+          end
+          it 'should update apply_penalty' do
+            patch :extension,
+                  params: { assignment_id: assignment.id, course_id: course.id, id: group.id,
+                            extension: extension_params }
+            expect(grouping.reload.extension.apply_penalty).to eq(extension_params[:apply_penalty])
+          end
+        end
+        context 'DELETE' do
+          it 'should delete extension' do
+            delete :extension, params: { assignment_id: assignment.id, course_id: course.id, id: group.id }
+            expect(grouping.reload.extension).to be_nil
+          end
+        end
+      end
+
+      context 'When an Extension does not already exist' do
+        it 'POST a new extension should work' do
+          post :extension,
+               params: { assignment_id: assignment.id, course_id: course.id, id: group.id, extension: extension_params }
+          expect(grouping.reload.extension).not_to be_nil
+        end
+        it 'POST a new extension should work - response status check' do
+          post :extension,
+               params: { assignment_id: assignment.id, course_id: course.id, id: group.id, extension: extension_params }
+          expect(response).to have_http_status(201)
+        end
+      end
+      context 'PATCH' do
+        it 'should not work for a non existent extension' do
+          put :extension,
+              params: { assignment_id: assignment.id, course_id: course.id, id: group.id, extension: extension_params }
+          expect(grouping.reload.extension).to be_nil
+        end
+        it 'should not work for a non existent extension - response status check' do
+          patch :extension,
+                params: { assignment_id: assignment.id, course_id: course.id, id: group.id,
+                          extension: extension_params }
+          expect(response).to have_http_status(422)
+        end
+      end
+      context 'DELETE' do
+        it 'should not work for a non existent extension' do
+          delete :extension, params: { assignment_id: assignment.id, course_id: course.id, id: group.id }
+          expect(response).to have_http_status(422)
+        end
+      end
+      context 'POST - Time delta' do
+        it 'should not allow time delta to be nil' do
+          extension_params =
+            {
+              apply_penalty: true,
+              note: 'random notes'
+            }
+          post :extension,
+               params: { assignment_id: assignment.id, course_id: course.id, id: group.id, extension: extension_params }
+          expect(response).to have_http_status(422)
+        end
+        it 'should not allow time delta to be empty' do
+          extension_params =
+            {
+              time_delta: {},
+              apply_penalty: true,
+              note: 'random notes'
+            }
+          post :extension,
+               params: { assignment_id: assignment.id, course_id: course.id, id: group.id, extension: extension_params }
+          expect(response).to have_http_status(422)
+        end
+      end
+    end
   end
 end