Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fire! and fire with unknown event, crash with NoMethodError #618

Open
pjmartorell opened this issue Apr 19, 2019 · 6 comments
Open

fire! and fire with unknown event, crash with NoMethodError #618

pjmartorell opened this issue Apr 19, 2019 · 6 comments

Comments

@pjmartorell
Copy link

When I run fire or fire! with an unknown event (i.e. Model.aasm.fire!(:whatever) ) it raises a NoMethodError (undefined method fire_callbacks' for nil:NilClass), seems that it tries to fire before callbacks even not knowing that the event exists, which is not the expected behaviour.

For me, fire and fire! with an unknown event should not fire callbacks and return an exception like "Unknown event", to be consistent.

@anilmaurya
Copy link
Member

anilmaurya commented Apr 20, 2019

I agree with your point, are you interested in raising a PR for this ?

@pjmartorell
Copy link
Author

Yes, I'll do it :) Regarding before callbacks, I'm not sure if they should be triggered or not for an unknown event. How do we handle this case? do we consider the same behaviour for firing a non permitted event than for firing an unknown event, in terms of triggering callbacks?

For me an unpermitted event should trigger before callbacks, but not an unknown event.

What do you think @anilmaurya?

@anilmaurya
Copy link
Member

I Agree, unpermitted event should trigger before callbacks but unknown event should not trigger callbacks.

@mck9
Copy link

mck9 commented Mar 27, 2020

this should be fixed, as it is possible to call arbitrary methods:

Car.aasm.fire("destroy")

@barisbalic
Copy link

@mck9 'should be fixed' meaning no longer a problem, or that you would like it to be fixed? I suspect the latter.

@anilmaurya was there anything wrong with the PR looks like it's been hanging around for a year?

@mck9
Copy link

mck9 commented Jul 21, 2020

@barisbalic yes, the latter would be nice. fire should only be allowed to call valid events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants