-
Notifications
You must be signed in to change notification settings - Fork 638
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
Use AASM::NO_VALUE as to_state
argument default value
#484
Conversation
@@ -39,18 +39,18 @@ | |||
it 'should be able to wait into waiting if polite' do | |||
auth.left_suspend! | |||
expect(auth.may_left_wait?(:waiting, :please)).to be true | |||
auth.left_wait!(nil, :please) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anilmaurya I noticed nil
is explicitly included in argument list when triggering event in the specs. Is this a common in real usage?
This is different from the usage examples written in https://github.com/aasm/aasm/wiki/State-transitions-with-guard-clauses-that-require-arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teohm Including nil
in argument list seems weird to me.
Thank you @teohm for contributing , I appreciate your help 🎖 |
@anilmaurya By any chance to release this fix some time this month August? 🙏 |
Hi @teohm , I am planning to release new version with this fix soon. |
That's great! thanks @anilmaurya !! 🎉 |
Hi @teohm , just release |
Thanks @anilmaurya !! |
Based on discussion in #449 (comment)
This PR attempts to fix the root cause of a few issues (#449, #416, #441) with this similar error:
This error happens because AASM incorrectly calls guard method with
one extra argument in certain scenarios. For example:
This happens because in
AASM::Core::Event#_fire(obj, options={}, to_state=nil, *args)
,the
to_state
argument default nil value is causing ambiguity, i.e. there is no way forAASM to tell whether a caller provides a) one argument with nil value, or b) no argument.
This PR removes this ambiguity by using a symbol as
to_state
default value.