don't allow terms with active courses to be deleted

also don't allow default terms to be deleted

test plan:
* create a enrollment term, and keep the tab open
* add a course in another tab and add it to the enrollment term
* switch back to the enrollment terms tab and try to delete
the term
* should not actually delete it

* try to delete the term through SIS
* should not be deleted

* delete the course or remove it from the term
* should be able to delete the term now

closes #CNVS-18032

Change-Id: I54480f80259ba41348801de4bda9609060f40fc1
Reviewed-on: https://gerrit.instructure.com/47944
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
Tested-by: Jenkins

app/controllers/terms_controller.rb

@@ -85,11 +85,16 @@ class TermsController < ApplicationController
   #
   def destroy
     @term = api_find(@context.enrollment_terms, params[:id])
-    @term.destroy
-    if api_request?
-      render :json => enrollment_term_json(@term, @current_user, session)
+    @term.workflow_state = 'deleted'
+
+    if @term.save
+      if api_request?
+        render :json => enrollment_term_json(@term, @current_user, session)
+      else
+        render :json => @term
+      end
     else
-      render :json => @term
+      render :json => @term.errors, :status => :bad_request
     end
   end
 

app/models/enrollment_term.rb

@@ -35,12 +35,24 @@ class EnrollmentTerm < ActiveRecord::Base
   EXPORTABLE_ASSOCIATIONS = [:root_account, :enrollment_dates_overrides, :courses, :course_sections]
 
   validates_presence_of :root_account_id, :workflow_state
+  validate :check_if_deletable
+
   before_validation :verify_unique_sis_source_id
   before_save :update_courses_later_if_necessary
 
   include StickySisFields
   are_sis_sticky :name, :start_at, :end_at
 
+  def check_if_deletable
+    if self.workflow_state_changed? && self.workflow_state == "deleted"
+      if self.default_term?
+        self.errors.add(:workflow_state, t('errors.delete_default_term', "Cannot delete the default term"))
+      elsif self.courses.active.exists?
+        self.errors.add(:workflow_state, t('errors.delete_term_with_courses', "Cannot delete a term with active courses"))
+      end
+    end
+  end
+
   def update_courses_later_if_necessary
     self.update_courses_later if !self.new_record? && (self.start_at_changed? || self.end_at_changed?)
   end
@@ -147,6 +159,6 @@ class EnrollmentTerm < ActiveRecord::Base
     self.workflow_state = 'deleted'
     save!
   end
-  
+
   scope :active, -> { where("enrollment_terms.workflow_state<>'deleted'") }
 end

app/views/terms/_term.html.erb

@@ -83,7 +83,7 @@
   </td>
   <td class="links">
     <a href="#" class="edit_term_link no-hover"><i class="icon-edit standalone-icon"></i></a>
-    <% if term && !term.courses.active.empty? %>
+    <% if term && term.courses.active.exists? %>
       <a href="#" class="cant_delete_term_link no-hover"><i class="icon-end standalone-icon"></i></a>
     <% elsif term != @context.default_enrollment_term %>
       <a href="#" class="delete_term_link no-hover"><i class="icon-trash standalone-icon"></i></a>

spec/controllers/terms_controller_spec.rb

@@ -36,4 +36,41 @@ describe TermsController do
       }}
   end
 
+  it "should not be able to delete a default term" do
+    account_model
+    account_admin_user(:account => @account)
+    user_session(@user)
+
+    delete 'destroy', :account_id => @account.id, :id => @account.default_enrollment_term.id
+
+    expect(response).to_not be_success
+    error = json_parse(response.body)["errors"]["workflow_state"].first["message"]
+    expect(error).to eq "Cannot delete the default term"
+  end
+
+  it "should not be able to delete an enrollment term with active courses" do
+    account_model
+    account_admin_user(:account => @account)
+    user_session(@user)
+
+    @term = @account.enrollment_terms.create!
+    course account: @account
+    @course.enrollment_term = @term
+    @course.save!
+
+    delete 'destroy', :account_id => @account.id, :id => @term.id
+
+    expect(response).to_not be_success
+    error = json_parse(response.body)["errors"]["workflow_state"].first["message"]
+    expect(error).to eq "Cannot delete a term with active courses"
+
+    @course.destroy
+
+    delete 'destroy', :account_id => @account.id, :id => @term.id
+
+    expect(response).to be_success
+
+    @term.reload
+    expect(@term).to be_deleted
+  end
 end

spec/lib/sis/csv/term_importer_spec.rb

@@ -101,4 +101,36 @@ describe SIS::CSV::TermImporter do
     end
   end
 
+  it 'should not delete terms with active courses' do
+    process_csv_data(
+      "term_id,name,status,start_date,end_date",
+      "T001,Winter11,active,2011-1-05 00:00:00,2011-4-14 00:00:00",
+    )
+
+    t1 = @account.enrollment_terms.where(sis_source_id: 'T001').first
+
+    course(:account => @account)
+    @course.enrollment_term = t1
+    @course.save!
+
+    importer = process_csv_data(
+      "term_id,name,status,start_date,end_date",
+      "T001,Winter11,deleted,2011-1-05 00:00:00,2011-4-14 00:00:00",
+    )
+
+    t1.reload
+    expect(t1).to_not be_deleted
+    expect(importer.warnings.map{|r|r.last}.first).to include "Cannot delete a term with active courses"
+
+    @course.destroy
+
+    importer = process_csv_data(
+        "term_id,name,status,start_date,end_date",
+        "T001,Winter11,deleted,2011-1-05 00:00:00,2011-4-14 00:00:00",
+    )
+
+    t1.reload
+    expect(t1).to be_deleted
+  end
+
 end

spec/models/enrollment_term_spec.rb

@@ -82,4 +82,26 @@ describe EnrollmentTerm do
       expect(@term.overridden_term_dates([student_enrollment, ta_enrollment])).to eq([nil, nil])
     end
   end
+
+  describe "deletion" do
+    it "should not be able to delete a default term" do
+      account_model
+      expect { @account.default_enrollment_term.destroy }.to raise_error
+    end
+
+    it "should not be able to delete an enrollment term with active courses" do
+      account_model
+      @term = @account.enrollment_terms.create!
+      course account: @account
+      @course.enrollment_term = @term
+      @course.save!
+
+      expect { @term.destroy }.to raise_error
+
+      @course.destroy
+
+      @term.destroy
+    end
+  end
+
 end