The only way, this line could fail, is when you have lost the db connection. I'm not sure if we are able to handle that gracefully.

This is also reflected in the "implementation of destroy":http://github.com/rails/rails/blob/2-3-stable/activerecord/lib/active_record/base.rb#L2612 The only thing that could bubble up is a general ActiveRecord error.

I think this is a 'won't fix'.
--------------------------------------------------------------------------------
Gregor Schmidt wrote:
> I think this is a 'won't fix'.

I disagree, it's easy for plugins to add callbacks to Models like before_delete. It's a UI inconsistency if Redmine is told to delete a record, it doesn't (for whatever reason), and the flash says it was deleted.

(Or alternatively) If Redmine itself ever did something in before_delete then someone will have to remember to change the controller because of the hard coded 'success' message. (e.g. if Redmine adds a permission for "Allowed to delete time entries".)
--------------------------------------------------------------------------------
patch coming in 5min
--------------------------------------------------------------------------------
patch which introduces an error message. includes a test that simulates a failing @before_destroy@ callback.

please check and commit if you think it's okay.
--------------------------------------------------------------------------------
(patch also adds check for successful flash message to the positive test)
--------------------------------------------------------------------------------
as Eric points out, a failing callback will rather return false than raise an error, adapted the patch
--------------------------------------------------------------------------------
Applied in r3805, thank you for the contribution.
--------------------------------------------------------------------------------