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

fix: missing subclass transitions #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ObscurePlatypoctopus
Copy link

Fixes #11

Caveat: The way it gets called changes with this, instead of creating an instance of the class with the state machine, call diagram on the class state machine instead.

@mihaimuntenas
Copy link
Collaborator

@Katee, @ObscurePlatypoctopus is correct and this PR fixes the issue.
Also, the calling convention seems to be "better" (the AASM DSL evaluates at class level) and does not break backward compatibility.

@ObscurePlatypoctopus welcome! And thank you for the PR!

Some quick notes/observations:

  • bundler version update might be too high; the lowest version of supported aasm is 4.12, which requires ruby at minimum 1.9.3, thus it might depend on bundler from 1.x branch.
  • you might want to also update the generate.rake to use the new calling convention (for completeness).

Again, thank you for the fix!

@ObscurePlatypoctopus
Copy link
Author

ObscurePlatypoctopus commented Apr 14, 2021

@mihaimuntenas Thank you for the notes! I think the Ruby version I was using did not support the older bundler, but I don't think the upgrade was required after all so I removed it.

I also adjusted the rake task as per your notes, but I did not manage to verify it yet. I think there may be another issue in the Rakefile where

puts 'Missing `model` argument.' unless args[:model].present?

fails if not using rails, because string.present? is missing. I'm assuming aasm is usually used with rails so maybe this is not a big deal.

@mihaimuntenas
Copy link
Collaborator

@ObscurePlatypoctopus you are so right!
I actually use it in a Rails project, but I should come up with a fix for that - in the end, it is a Rake task, not a Rails task :) ...
And, as pointed out, there is no dependency on Rails.

@ObscurePlatypoctopus
Copy link
Author

@Katee Is there anything this needs before it can be merged?

@adis-io
Copy link

adis-io commented Aug 9, 2022

@Katee Would be nice if it was merged

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.

Does not appear to map transitions of subclass with new states
4 participants