Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Tweak observer #6062

Merged
merged 2 commits into from

3 participants

@marcandre

Small fixes and improvements for Observers.
Thanks

marcandre added some commits
@marcandre marcandre Fix error message:
- can pass the class, not the instance
- "instance method" is confusing, use "method :instance" instead
85c056c
@marcandre marcandre Generate appropriate error more judiciously 569fb1f
@carlosantoniodasilva

Please ignore my other comment, I missed something in the review. Thanks!

@tenderlove
Owner

I know this is current behavior, but the way we code rails APIs to take three different parameters that mean the same thing really annoys me. FooObserver != :foo_observer != 'foo_observer', yet this api treats them all the same way.

/rant

@tenderlove tenderlove merged commit c19acab into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 29, 2012
  1. @marcandre

    Fix error message:

    marcandre authored
    - can pass the class, not the instance
    - "instance method" is confusing, use "method :instance" instead
  2. @marcandre
This page is out of date. Refresh to see the latest.
View
11 activemodel/lib/active_model/observing.rb
@@ -90,14 +90,15 @@ def count_observers
def instantiate_observer(observer) #:nodoc:
# string/symbol
if observer.respond_to?(:to_sym)
- observer.to_s.camelize.constantize.instance
- elsif observer.respond_to?(:instance)
+ observer = observer.to_s.camelize.constantize
+ end
+ if observer.respond_to?(:instance)
observer.instance
else
raise ArgumentError,
- "#{observer} must be a lowercase, underscored class name (or an " +
- "instance of the class itself) responding to the instance " +
- "method. Example: Person.observers = :big_brother # calls " +
+ "#{observer} must be a lowercase, underscored class name (or " +
+ "the class itself) responding to the method :instance. " +
+ "Example: Person.observers = :big_brother # calls " +
"BigBrother.instance"
end
end
View
15 activemodel/test/cases/observing_test.rb
@@ -70,6 +70,21 @@ def setup
ObservedModel.instantiate_observers
end
+ test "raises an appropriate error when a developer accidentally adds the wrong class (i.e. Widget instead of WidgetObserver)" do
+ assert_raise ArgumentError do
+ ObservedModel.observers = ['string']
+ ObservedModel.instantiate_observers
+ end
+ assert_raise ArgumentError do
+ ObservedModel.observers = [:string]
+ ObservedModel.instantiate_observers
+ end
+ assert_raise ArgumentError do
+ ObservedModel.observers = [String]
+ ObservedModel.instantiate_observers
+ end
+ end
+
test "passes observers to subclasses" do
FooObserver.instance
bar = Class.new(Foo)
Something went wrong with that request. Please try again.