Allow deletion of Assignment's with no groups

Changelog.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## [unreleased]
+- Allow deletion of assignments with no groups (#6880)
 
 ## [v2.4.5]
 - Add workaround for CSP in Safari < 16 (#6947)

app/assets/stylesheets/common/_markus.scss

@@ -303,6 +303,12 @@ button,
   }
 }
 
+#assignment-delete {
+  background: $severe-error;
+  color: $background-main;
+  font-weight: bold;
+}
+
 /** Fieldset */
 
 fieldset {

app/controllers/api/assignments_controller.rb

@@ -299,6 +299,24 @@ def submit_file
       upload_file(grouping, only_required_files: assignment.only_required_files)
     end
 
+    def destroy
+      assignment = record
+      if assignment.nil?
+        render 'shared/http_status', locals: { code: '404', message:
+          I18n.t('assignments.assignment_not_found',
+                 invalid_id: assignment_id) }, status: :not_found
+      else
+        begin
+          assignment.destroy
+          render 'shared/http_status',
+                 locals: { code: '200', message: I18n.t('assignments.successful_deletion') }, status: :ok
+        rescue ActiveRecord::DeleteRestrictionError
+          render 'shared/http_status',
+                 locals: { code: :conflict, message: I18n.t('assignments.assignment_has_groupings') }, status: :conflict
+        end
+      end
+    end
+
     protected
 
     def implicit_authorization_target

app/controllers/assignments_controller.rb

@@ -657,6 +657,17 @@ def lti_settings
     render layout: 'assignment_content'
   end
 
+  def destroy
+    @assignment = @record
+    begin
+      @assignment.destroy
+      respond_with @assignment, location: -> { course_assignments_path(current_course, @assignment) }
+    rescue ActiveRecord::DeleteRestrictionError
+      flash_message(:error, I18n.t('assignments.assignment_has_groupings'))
+      redirect_back fallback_location: { action: :edit, id: @assignment.id }
+    end
+  end
+
   private
 
   # Configures the automated test files and settings for an +assignment+ provided in the +zip_file+

app/models/assignment.rb

@@ -74,7 +74,7 @@ class Assignment < Assessment
   has_many :student_memberships, through: :groupings
 
   has_many :submissions, through: :groupings
-  has_many :groups, through: :groupings
+  has_many :groups, through: :groupings, dependent: :restrict_with_exception
 
   has_many :notes, as: :noteable, dependent: :destroy
 

app/views/assignments/_form.html.erb

@@ -489,4 +489,22 @@
                    data: { disable_with: t('working') } %>
     </p>
   <% end %>
+
+  <% if (action_name=="edit") %>
+    <% has_groups = @assignment.groups.length != 0 %>
+    <%= button_to t(:delete),
+                  course_assignment_path(@current_course, @assignment),
+                  {
+                    data: { confirm: "Are you sure you want to delete this assignment?" },
+                    method: 'delete',
+                    class: has_groups ? nil : 'button',
+                    id: has_groups ? nil : 'assignment-delete',
+                    form_class: 'display-inline-block',
+                    title: has_groups ? I18n.t('assignments.assignment_has_groupings') :
+                             I18n.t('helpers.submit.delete', model: Assignment.model_name.human),
+                    disabled: has_groups
+                  }
+    %>
+  <%end %>
+
 <% end %>

config/locales/views/assignments/en.yml

@@ -1,6 +1,8 @@
 ---
 en:
   assignments:
+    assignment_has_groupings: Assignment has groupings.
+    assignment_not_found: No assignment with id '%{invalid_id}'.
     average_annotations: "%{average_annotations} annotations per marked submission"
     configuration_zip_file: Configuration Zip File
     deadline_with_extension: You have an extension until %{extension_deadline}.
@@ -63,6 +65,7 @@ en:
       title: Starter Files
       upload_confirmation: The starter files for this assignment have changed since you last downloaded them. Are you sure you want to continue?
       use_original_filename: Use original filename
+    successful_deletion: Assignment with id '%{invalid_id}' has been successfully deleted.
     timed:
       before_start_instructions: Once you start this timed assessment, you will have %{duration_string} to complete it without penalty.
       before_start_instructions_extension: Once you start this timed assessment, you will have %{duration_string} to complete it without penalty (this includes a duration extension of %{extension_string}).

config/routes.rb

@@ -209,7 +209,7 @@
 
     resources :annotation_categories, only: [:show, :destroy, :update]
 
-    resources :assignments, except: [:destroy] do
+    resources :assignments do
       collection do
         get 'delete_rejected'
         get 'batch_runs'

spec/controllers/api/assignments_controller_spec.rb

@@ -188,6 +188,11 @@
       put :update, params: { id: assignment.id, course_id: course.id }
       expect(response).to have_http_status(403)
     end
+
+    it 'should fail to authenticate a DELETE destroy request' do
+      delete :destroy, params: { id: assignment.id, course_id: course.id }
+      expect(response).to have_http_status(403)
+    end
   end
 
   context 'An authenticated instructor request requesting' do
@@ -502,6 +507,35 @@
         expect(response).to have_http_status(403)
       end
     end
+    context 'DELETE assignment' do
+      it 'should successfully delete assignment because the assignment has no groups' do
+        assignment # since lazy let is used for creating an assignment, I invoke it here to trigger its execution
+        expect(assignment.groups).to be_empty
+        delete :destroy, params: { id: assignment.id, course_id: course.id }
+        expect(response).to have_http_status(200)
+        expect(Assignment.exists?(assignment.id)).to eq(false)
+      end
+      it 'fails to delete assignment because assignment has groups' do
+        assignment # since lazy let is used for creating an assignment, I invoke it here to trigger its execution
+        # creates a grouping (and thus a group) for the assignment
+        create :grouping, assignment: assignment, start_time: nil
+        expect(assignment.groups).to_not be_empty
+        original_size = Assignment.all.length
+        delete :destroy, params: { id: assignment.id, course_id: course.id }
+        expect(response).to have_http_status(409)
+        expect(Assignment.all.length).to eq(original_size)
+        expect(assignment.persisted?).to eq(true)
+      end
+      it 'fails to delete assignment because of invalid id' do
+        assignment # since lazy let is used for creating an assignment, I invoke it here to trigger its execution
+        original_size = Assignment.all.length
+        # Since we only have one assignment, it is guaranteed that assignment.id + 1 is an invalid id
+        delete :destroy, params: { id: assignment.id + 1, course_id: course.id }
+        expect(response).to have_http_status(404)
+        expect(Assignment.all.length).to eq(original_size)
+        expect(assignment.persisted?).to eq(true)
+      end
+    end
   end
 
   context 'An authenticated student request' do
@@ -670,6 +704,13 @@
         end
       end
     end
+
+    context 'DELETE destroy' do
+      it 'should fail to authenticate a DELETE destroy request' do
+        delete :destroy, params: { id: assignment.id, course_id: course.id }
+        expect(response).to have_http_status(403)
+      end
+    end
   end
 
   context 'An authenticated ta request' do
@@ -704,5 +745,12 @@
         end
       end
     end
+
+    context 'DELETE destroy' do
+      it 'should fail to authenticate a DELETE destroy request' do
+        delete :destroy, params: { id: assignment.id, course_id: course.id }
+        expect(response).to have_http_status(403)
+      end
+    end
   end
 end

spec/controllers/assignments_controller_spec.rb

@@ -2038,4 +2038,32 @@
       expect(response).to have_http_status(200)
     end
   end
+
+  context '#destroy' do
+    let!(:instructor) { create :instructor }
+    let!(:course) { instructor.course }
+    let!(:assignment) { create :assignment }
+    let(:grouping) { create :grouping, assignment: assignment }
+    it 'should fail to DELETE because of unauthorized request' do
+      delete :destroy, params: { course_id: course.id, id: assignment.id }
+      expect(Assignment.exists?(assignment.id)).to be(true)
+    end
+    it 'should fail to DELETE because assignment has groups' do
+      grouping # lazy initialization
+      delete_as instructor, :destroy, params: { course_id: course.id, id: assignment.id }
+      expect(Assignment.exists?(assignment.id)).to be(true)
+      expect(flash[:error]).to eq(["<p>#{I18n.t('assignments.assignment_has_groupings')}</p>"])
+      expect(flash.to_hash.length).to eq(1)
+      expect(flash[:error].length).to eq(1)
+      expect(response).to have_http_status(302)
+    end
+    it 'should successfully DELETE assignment (no groups)' do
+      delete_as instructor, :destroy, params: { course_id: course.id, id: assignment.id }
+      expect(Assignment.exists?(assignment.id)).to be(false)
+      expect(flash[:success]).to eq(I18n.t('flash.actions.destroy.success',
+                                           resource_name: assignment.short_identifier))
+      expect(flash.to_hash.length).to eq(1)
+      expect(response).to have_http_status(302)
+    end
+  end
 end