fix(models): halt destroy callbacks for datebooks, doctors and admin users

app/models/datebook.rb

@@ -33,7 +33,7 @@ def is_deleteable
   def check_if_is_deleteable
     return if is_deleteable
 
-    errors[:base] << I18n.t('errors.messages.has_appointments')
-    false
+    errors.add(:base, I18n.t('errors.messages.has_appointments'))
+    throw :abort
   end
 end

app/models/doctor.rb

@@ -53,7 +53,7 @@ def ciphered_feed_url
   def check_if_is_deleteable
     return if is_deleteable
 
-    errors[:base] << I18n.t('errors.messages.has_appointments_or_treatments')
-    false
+    errors.add(:base, I18n.t('errors.messages.has_appointments_or_treatments'))
+    throw :abort
   end
 end

app/models/user.rb

@@ -87,8 +87,8 @@ def validate_password?
   def check_if_admin
     return unless is_admin?
 
-    errors[:base] << I18n.t('errors.messages.unauthorised')
-    false
+    errors.add(:base, I18n.t('errors.messages.unauthorised'))
+    throw :abort
   end
 
   def set_admin_role_for_first_user

test/functional/datebooks_controller_test.rb

@@ -63,11 +63,11 @@ class DatebooksControllerTest < ActionController::TestCase
     assert_redirected_to datebooks_url
   end
 
-  # test "should not destroy datebook with appointments" do
-  #   assert_no_difference('Datebook.count') do
-  #     delete :destroy, params: {id: datebooks(:playa_del_carmen).to_param}
-  #   end
+  test 'should not destroy datebook with appointments' do
+    assert_no_difference('Datebook.count') do
+      delete :destroy, params: { id: datebooks(:playa_del_carmen).to_param }
+    end
 
-  #   assert_redirected_to datebooks_url
-  # end
+    assert_redirected_to datebooks_url
+  end
 end

test/functional/doctors_controller_test.rb

@@ -20,6 +20,7 @@ class DoctorsControllerTest < ActionController::TestCase
 
   test 'shows empty state cta when practice has no doctors' do
     practice = practices(:complete)
+    Appointment.where(doctor_id: Doctor.with_practice(practice.id)).delete_all
     Doctor.with_practice(practice.id).destroy_all
     practice.update_columns(doctors_count: 0)
 

test/unit/models/datebook_test.rb

@@ -43,6 +43,16 @@ class DatebookTest < ActiveSupport::TestCase
     assert_equal I18n.t('errors.messages.less_than_or_equal_to', count: 23), datebook.errors[:ends_at].join('; ')
   end
 
+  test 'datebook with appointments cannot be destroyed' do
+    datebook = datebooks(:playa_del_carmen)
+
+    assert_no_difference 'Datebook.count' do
+      assert_not datebook.destroy
+    end
+
+    assert datebook.errors[:base].any?
+  end
+
   test 'datebook name should be less than 100 chars' do
     datebook = Datebook.new(name: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec vehicula arcu ante, nec eleifend ipsum. Proin vestibulum nisi sit amet diam mattis tempor.')
 

test/unit/models/doctor_test.rb

@@ -80,6 +80,16 @@ class DoctorTest < ActiveSupport::TestCase
     assert_equal doctor.initials, 'RR'
   end
 
+  test 'doctor with appointments cannot be destroyed' do
+    doctor = doctors(:rebecca)
+
+    assert_no_difference 'Doctor.count' do
+      assert_not doctor.destroy
+    end
+
+    assert doctor.errors[:base].any?
+  end
+
   test 'destroying doctor purges profile picture attachment' do
     doctor = Doctor.create!(
       practice: practices(:complete),

test/unit/models/user_test.rb

@@ -171,4 +171,14 @@ class UserTest < ActiveSupport::TestCase
     user.firstname = 'Super'
     assert user.save
   end
+
+  test 'admin user cannot be destroyed' do
+    admin = users(:founder)
+
+    assert_no_difference 'User.count' do
+      assert_not admin.destroy
+    end
+
+    assert admin.errors[:base].any?
+  end
 end