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

allow event arguments to be taken into account when selecting the event #227

Closed
wants to merge 1 commit into from

Conversation

aserafin
Copy link
Contributor

@aserafin aserafin commented May 20, 2022

This allows access to event arguments when we're evaluating which event to run in the scope of

state :off
  event :turn_on, :transition_to => :on, :if => :sufficient_battery_level?

  event :turn_on, :transition_to => :low_battery, :if => proc { |device, _power_adapter| device.battery_level > 0 }
end

# corresponding instance method
def sufficient_battery_level?(power_adapter)
  power_adapter || battery_level > 10
end

@geekq I imagine this potentially is a breaking change but I run into a situation when event arguments had to be taken into account for conditional event - let me know what you think 👍 🙇

@geekq
Copy link
Owner

geekq commented May 23, 2022

Thanks for your contribution!
Backward compatibility is very important (people have used this library for many years, and some do not upgrade to the newest Ruby or Rails versions). The compatibility should be covered (checked) by the test suite though. Good thing would be to switch from travis-CI (ceased) to GitHub actions, before integrating any new contributions. Maybe something inspired by https://github.com/geekq/workflow-activerecord/blob/develop/.github/workflows/test.yml so any regression can be prevented.

For the new behavior we'll need test cases as well.

@geekq
Copy link
Owner

geekq commented May 25, 2022

Blocked by gh-229

@geekq
Copy link
Owner

geekq commented May 26, 2022

Please rebase your PR, so we can see the results of the tests for different Ruby versions

geekq added a commit that referenced this pull request May 16, 2024
geekq added a commit that referenced this pull request May 16, 2024
geekq added a commit that referenced this pull request May 16, 2024
@geekq
Copy link
Owner

geekq commented May 16, 2024

Added unit tests, documentation, rebased and merged

@geekq geekq closed this May 16, 2024
geekq added a commit that referenced this pull request May 29, 2024
… ignore additional arguments

In case the method referenced by `:if` does not accept additional
arguments, but the transition action is called with some, avoid
exception "1 argument provided, but 0 were expected" by ignoring
superfluous arguments if needed.
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

Successfully merging this pull request may close these issues.

2 participants