[katello-devel] before_filter behavior and notices
Brad Buckingham
bbuckingham at redhat.com
Wed Jul 20 13:09:41 UTC 2011
Team,
While working on activation keys within a remote branch, I decided to do
some 'copy/paste' reuse
of a before_filter. In doing so, I observed a couple of issues with the
filter that I chose to 'reuse'. After looking further, I found that the
same issues existed in several controllers. This email is to share the
finding, in case others may benefit.
Observation/Issue 1 - several of the before_filters are used to fetch
ActiveRecord data (e.g. find); however, they are not catching the
exception if the ActiveRecord cannot be found. As a result, the error
logic contained in the filter is being skipped.
Fix - wrap the find in a begin/rescue block, catching the AR
exceptions.
Observation/Issue 2 - when the exception is caught, the error notice
(i.e. errors) is not being displayed to the user. The reason being, the
error handling logic in these filters generally performs a render or
redirect, which will halt the filter chain causing the flash_to_headers
after_filter not to be executed.
Fix - added a new method to the ApplicationController called
'execute_after_filters' and call this method from within the
before_filter's error leg. This method currently only calls
'flash_to_headers'; however, if other after_filters are introduced in
the ApplicationController in the future, they can also be added here.
I have gone through the code to address these observations in the UI
controllers, where appropriate. For details on the commit, it is:
http://git.fedorahosted.org/git/?p=katello.git;a=commit;h=5165c9b2cc463a80df0ed73b22d006b8ff65e6e3
For an example of the typical changes made to the filters, see:
http://git.fedorahosted.org/git/?p=katello.git;a=blobdiff;f=src/app/controllers/sync_plans_controller.rb;h=4e3fe1834d841fdfbe6c7b267574ca55b22bc146;hp=89b94e9e3957e351c1535c5fe98be8770fd71208;hb=5165c9b2cc463a80df0ed73b22d006b8ff65e6e3;hpb=b7cea35221795fa7a779e04fc524cd74b554596e
thanks,
Brad
More information about the katello-devel
mailing list